Using the CategoryDrop on the categories page redirected the user to the
"latest topics" page with topics only from that category. With these
changes, selecting a category will take the user to a "subcategories
page" where only the subcategories of the selected property will be
displayed.
This commit adds a different message to the users directory (`/u`) that's shown when there are no results for the search term instead of showing the one for when there are no users on the site yet.
Internal topic: t/126076.
Per https://developers.google.com/search/docs/crawling-indexing/overview-google-crawlers
> GoogleOther is the generic crawler that may be used by various product teams for fetching publicly accessible content from sites. For example, it may be used for one-off crawls for internal research and development.
This commit will ensure it's served the crawler view, and included in crawler metrics
This commit changes request method for "categories/search" from GET to
POST to make sure that long filters can be passed to the server. For
example, category selectors with many categories are setting the full
list of selected category IDs to ensure these are filtered out from the
list of choices. This can result in a long URL that exceeds the maximum
length.
The watched word group's create, update and delete action logs were missing the translations. This PR will add those strings and will use the group key instead of watched word key where needed.
This commit switches `DiscourseIpInfo.mmdb_download` to use the
permalinks supplied by MaxMind to download the MaxMind databases as
specified in
https://dev.maxmind.com/geoip/updating-databases#directly-downloading-databases
which states:
```
To directly download databases, follow these steps:
1. In the "Download Links" column, click "Get Permalink(s)" for the desired database.
2. Copy the permalink(s) provided in the modal window.
3. Provide your account ID and your license key using Basic Authentication to authenticate.
```
Previously we are downloading from `https://download.maxmind.com/app/geoip_download` but this is not
documented anyway on MaxMind's docs so this URL can in theory break
in the future without warning. Therefore, we are taking a proactive
approach to download the databases from MaxMind the recommended way
instead of relying on a hidden URL. This old way of downloading the
databases with only a license key will be deprecated in 3.3 and be
removed in 3.4.
This keeps coming up in user testing as something
we want to get rid of. The `navigation_menu` setting
has been set to sidebar by default for some time now,
and we are rolling out admin sidebar widely. It just
doesn't make sense to let people turn this off in
the first step of the wizard -- we _want_ people to
use the sidebar.
At the moment, there is no way to create a group of related watched words together. If a user needed a set of words to be created together, they'll have to create them individually one at a time.
This change attempts to allow related watched words to be created as a group. The idea here is to have a list of words be tied together via a common `WatchedWordGroup` record. Given a list of words, a `WatchedWordGroup` record is created and assigned to each `WatchedWord` record. The existing WatchedWord creation behaviour remains largely unchanged.
Co-authored-by: Selase Krakani <skrakani@gmail.com>
Co-authored-by: Martin Brennan <martin@discourse.org>
This commit introduces a few changes as a result of
customer issues with finding why a topic was relisted.
In one case, if a user edited the OP of a topic that was
unlisted and hidden because of too many flags, the topic
would get relisted by directly changing topic.visible,
instead of going via TopicStatusUpdater.
To improve tracking we:
* Introduce a visibility_reason_id to topic which functions
in a similar way to hidden_reason_id on post, this column is
set from the various places we change topic visibility
* Fix Post#unhide! which was directly modifying topic.visible,
instead we use TopicStatusUpdater which sets visibility_reason_id
and also makes a small action post
* Show the reason topic visibility changed when hovering the
unlisted icon in topic status on topic titles
Our 'page_view_crawler' / 'page_view_anon' metrics are based purely on the User Agent sent by clients. This means that 'badly behaved' bots which are imitating real user agents are counted towards 'anon' page views.
This commit introduces a new method of tracking visitors. When an initial HTML request is made, we assume it is a 'non-browser' request (i.e. a bot). Then, once the JS application has booted, we notify the server to count it as a 'browser' request. This reliance on a JavaScript-capable browser matches up more closely to dedicated analytics systems like Google Analytics.
Existing data collection and graphs are unchanged. Data collected via the new technique is available in a new 'experimental' report.
Made another effort to improve the description of the personal_message_enabled _groups site setting, to clarify who can already create and reply to messages no matter which groups are specified in this setting.
This service-worker caching functionality was disabled by default in 1c58395bca, and the setting to re-enable was marked as experimental. Now we are dropping all the related logic.
- Run the CSP-nonce-related middlewares on the generated response
- Fix the readonly mode checking to avoid empty strings being passed (the `check_readonly_mode` before_action will not execute in the case of these re-dispatched exceptions)
- Move the BlockRequestsMiddleware cookie-setting to the middleware, so that it is included even for unusual HTML responses like these exceptions
This commit adds the most common AI bot crawlers seen
on our hosting (claudebot, gptbot, anthropic-ai, brightbot)
to our `slow_down_crawler_user_agents` and `crawler_user_agents`
site settings by default.
This means these AI bots will be rate limited by default instead
of site admins having to remember to do it for themselves.
LinkedIn has grandfathered its old OAuth2 provider. This can only be used by existing apps. New apps have to use the new OIDC provider.
This PR adds a linkedin_oidc provider to core. This will exist alongside the discourse-linkedin-auth plugin, which will be kept for those still using the deprecated provider.
When the user sees no results in their admin sidebar query,
we are adding two additional links:
* "Search site settings" - Navigates to the site settings page
with the filter prefilled in the search
* "Admin user list" - Navigates to the user list with the filter
prefilled in the username search
This will bridge the gap until we have a full admin-wide search.
Also make admin site setting search param refresh on filter changes
---------
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
This commit ensures that additional keywords for admin
sidebar links (which are also stored in the admin sidebar state
manager) are translated with I18n, which was discussed
in https://meta.discourse.org/t/introducing-experimental-admin-sidebar-navigation/289281/58?u=martin
This also changes the admin sidebar state manager keywords to
not be a TrackedObject -- this is not necessary as keywords are
only set once, and it was causing rendering issues because
the keywords were being set at the same time they were read.
Finally this adds a "theme" keyword to the "Components" link
because we often refer to components as Theme Components
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
Automatically add `moderators` and `admins` auto groups to specific site settings.
In the new group-based permissions systems, we just want to check the user’s groups since it more accurately reflects reality
Affected settings:
- tag_topic_allowed_groups
- create_tag_allowed_groups
- send_email_messages_allowed_groups
- personal_message_enabled_groups
- here_mention_allowed_groups
- approve_unless_allowed_groups
- approve_new_topics_unless_allowed_groups
- skip_review_media_groups
- email_in_allowed_groups
- create_topic_allowed_groups
- edit_wiki_post_allowed_groups
- edit_post_allowed_groups
- self_wiki_allowed_groups
- flag_post_allowed_groups
- post_links_allowed_groups
- embedded_media_post_allowed_groups
- profile_background_allowed_groups
- user_card_background_allowed_groups
- invite_allowed_groups
- ignore_allowed_groups
- user_api_key_allowed_groups
To add a components link to the sidebar refactoring was required to create unique URLs for themes and components. Before the query param was used. After changes, we have two URLs `/admin/customize/themes` and `/admin/customize/components`.
This is to enable :array type attributes for Contract
attributes in services, this is a followup to the move
of services from chat to core here:
cab178a405
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
The bulk actions menu for topics has multiple options to work
with tags on topics (append, replace, remove). Our tagging system
along with categories allows for some complicated tag restrictions
to be applied via tag groups. This was a problem for the topic bulk
actions because you couldn't append restricted tags to topics.
This commit allows restricted tags to be used in bulk tagging actions
as long as all selected topics are for a sole category. The category
information will be shown in the modal, and the category ID is used
for the tag search.
This will automatically enable the glimmer header when all installed themes/plugins are ready. This replaces the old group-based site setting.
In 'auto' mode, we check for calls to deprecated APIs (e.g. decorateWidget) which affect the old header. If any are present, we stick to the old header implementation and print a message to the console alongside the normal deprecation messages.
To override this automatic behavior, a new `glimmer_header_mode` site setting can be set to 'disabled' or 'enabled'.
This change also means that our test suite is running with the glimmer header. This unveiled a couple of small issues (e.g. some incorrect `aria-*` and `alt` text) which are now fixed. A number of selectors had to be updated to ensure the tests were clicking the actual `<button>` elements rather than the surrounding `<li>` elements.
Display additional confirmation when:
- The public section is going to be updated;
- The public section is going to be deleted;
- The public section is going to be marked as private.
It's mostly fine to use the plural form instead of writing something like "topic(s)" when one or more topics could be meant, but the actual count is not known.
This also removes some unused strings from the locale files.
This commmit removes the unused `/u/:username/preferences/categories`
route which was merged into the `/u/:username/preferences/tracking`
route in 2fc2d7d828.
Adds the new quick menu for bookmarking. When you bookmark
a post (chat message behaviour will come later) we show this new quick
menu and bookmark the item straight away.
You can then choose a reminder quick option, or choose Custom... to open
the old modal. If you click on an existing bookmark, we show the same quick menu
but with Edit and Delete options.
A later PR will introduce a new bookmark modal, but for now we
are using the old modal for Edit and Custom... options.
To remove the Getting Started button manually, you have to disable bootstrap mode by setting bootstrap_mode_min_users to 0. I clarified this in the description for the setting.
- Add a "Skip tips" button to first notification tip
- Add a "Skip tips" button to the admin guide tip
- Fixes the timeline tip showing when no timeline was present
- Fixes post menu tip showing when no "..." button is present
- Adds system tests
- Marks each tip as seen as soon as it is displayed so that refreshing,
clicking outside, etc. won't show it again
- Change just above means we no longer need a MessageBus track
Co-authored-by: Bianca Nenciu <nbianca@users.noreply.github.com>
When a user is manually deactivated, they should not be deleted by our background job that purges inactive users.
In addition, site settings keywords should accept an array of keywords.
In this PR, all references in the UI to the word "`upgrade`" are changed to "`update`". This is to differentiate the update process in self-hosted sites from the plan "upgrade" process in hosted sites.
Follow-up to the PR: https://github.com/discourse/docker_manager/pull/208
Why this change?
This allows downloading the MaxMind databases from a mirror in cases
where downloading directly from MaxMind's API endpoint is problematic
due to API limits.
* DEV: Various bulk-select dropdown tweaks
- Setting is no longer hidden
- descriptions have been moved to the modal
- Removed ... from one of the dropdown titles
This adds a hidden site setting of `skip_email_bulk_invites`
If set to `true`, the `BulkInvite` job will pass the value to `Invite`, meaning the generated invite wont trigger an email notification being sent to the newly invited user.
(This is useful if you want to manage the sending of the invite emails outside of Discourse.)
The personal message enabled groups site setting is overridden by the group interaction settings for specifying who is allowed to write to groups. This was not clearly explained in the description here and I think I fixed it!
Why this change?
This is a follow-up to 86b2e3a.
Basically, we want to allow people to select more than 1 group as well.
What does this change do?
1. Change `type: group` to `type: groups` and support `min` and `max`
validations for `type: groups`.
2. Fix the `<SchemaThemeSetting::Types::Groups>` component to support the
`min` and `max` validations and switch it to use the `<GroupChooser>` component
instead of the `<ComboBoxComponent>` component which previously only supported
selecting a single group.
It's not really intentional to have regular admins change
this in all but pathological cases. It deletes all notifications
over this threshold for users without warning. If admins
really want to turn this on, they can do it via the app.yml file
Why this change?
This is a follow up to 897be75941.
When updating `net-smtp` from `0.4.x` to `0.5.x`, our test suite passed
but the error `ArgumentError: SMTP-AUTH requested but missing user name`
was being thrown in production leading to emails being failed to send
out via SMTP.
This commit adds a test to ensure that our production SMTP settings will
at least attemp to connect to an SMTP server.
When a topic fails to be created due to the user not having permission to add tags to the topic, the error message that you get is `There was an error tagging the topic` which is very generic and doesn't explain where/what the problem is. This commit adds a clearer error message for this scenario.
This commit removes the 'experimental_preconnect_link_header' site setting, and the 'preload_link_header' site setting, and introduces two new global settings: early_hint_header_mode and early_hint_header_name.
We don't actually send 103 Early Hint responses from Discourse. However, upstream proxies can be configured to cache a response header from the app and use that to send an Early Hint response to future clients.
- `early_hint_header_mode` specifies the mode for the early hint header. Can be nil (disabled), "preconnect" (lists just CDN domains) or "preload" (lists all assets).
- `early_hint_header_name` specifies which header name to use for the early hint. Defaults to "Link", but can be changed to support different proxy mechanisms.
Why this change?
This is a follow-up to 86b2e3aa3e.
Basically, we want to allow people to select more than 1 category as well.
What does this change do?
1. Change `type: category` to `type: categories` and support `min` and `max`
validations for `type: categories`.
2. Fix the `<SchemaThemeSetting::Types::Categories>` component to support the
`min` and `max` validations and switch it to use the `<CategorySelector>` component
instead of the `<CategoryChooser>` component which only supports selecting one category.
This user-agent is sent when URLs are inspected via the UI of Google's search console. It makes sense for us to serve it the same content as other bots, including GoogleBot.
When crawlers visit a post-specific URL like `/t/-/{topic-id}/{post-number}`, we use the canonical to direct them to the appropriate crawler-optimised paginated view (e.g. `?page=3`).
However, analysis of google results shows that the post-specific URLs are still being included in the index. Google doesn't tell us exactly why this is happening. However, as a general rule, 'A large portion of the duplicate page's content should be present on the canonical version'.
In our previous implementation, this wasn't 100% true all the time. That's because a request for a post-specific URL would include posts 'surrounding' that post, and won't exactly conform to the page boundaries which are used in the canonical version of the page. Essentially: in some cases, the content of the post-specific pages would include many posts which were not present on the canonical paginated version.
This commit aims to resolve that problem by simplifying the implementation. Instead of rendering posts surrounding the target post_number, we will only render the target post, and include a link to 'show post in topic'. With this new implementation, 100% of the post-specific page content will be present on the canonical paginated version, which will hopefully mean google reduces their indexing of the non-canonical post-specific pages.
Why this change?
Previously, we were preloading the necessary metadata for
`adminCustomizeThemes.show.schema` route in the
`adminCustomizeThemes.show` route. This is wasteful because we're
loading data upfront when the objects setting editor may not be used.
This change also lays the ground work for a future commit where we need
to be shipping down additional metadata which may further add to the
payload.
Why this change?
When a property of `type: tags` is required, we should be displaying the
"at least 1 tag is required" validation error message when there are no
tags selected in the `TagChooser` compoment. However, we were passing
`this.min` as the `count` attribute when generating the translation
string which is incorrect as `this.min` is not always set.
To improve performance, we omit the basic-HTML version of pages when users are logged in, or when they are using a modern mobile device. This can be confusing when analysing the SEO of sites, so this commit adds a short static message when content is omitted.
Why this change?
While working on the tag selector for the theme object editor, I
realised that there is an extremely high possibility that users might want to select
more than one tag. By supporting the ability to select more than one
tag, it also means that we get support for a single tag for free as
well.
What does this change do?
1. Change `type: tag` to `type: tags` and support `min` and `max`
validations for `type: tags`.
2. Fix the `<SchemaThemeSetting::Types::Tags>` component to support the
`min` and `max` validations
Why this change?
Fortunately or unfortunately in Discourse core, we mainly use `Tag#name`
to look up tags and not its id. This assumption is built into the
frontend as well so we need to use the tag's name instead of the id
here.
Previously, we had an instant redirect back to the homepage, and clicking avatars would do nothing. This made things feel 'broken' for anon when 'hide_user_profiles_from_public' was enabled.
This commit does a few things to resolve this:
1. Improve our 'exception' system for routes so that developers can deliberately trigger it without an ajax error
2. Improve 'exception' system so that the browser URL bar is updated correctly, and the 'back' button works as expected
3. Replace the redirect-to-home with an 'access denied' error page, with specific copy for 'You must log in to view user profiles'
4. Update user-card logic to display this new page instead of doing nothing on click
Why this change?
This is a continuation of 8de869630f.
In our schema, we support the `min` and `max` validation
rules like so:
```
some_objects_setting
type: objects
schema:
name: some_object
properties:
id:
type: integer
validations:
min: 5
max: 10
```
While the validations used to validate the objects on the server side,
we should also add client side validation for better UX.
Why this change?
In our schema, we support the `min_length` and `max_length` validation
rules like so:
```
some_objects_setting
type: objects
schema:
name: some_object
properties:
title:
type: string
validations:
min_length: 1
max_length: 10
```
While the validations used to validate the objects on the server side,
we should also add client side validation for better UX.
Currently, a new sidebar link for what's new and reports is going to the main dashboard page and activates the proper tab.
It might be problematic, especially, when the instance has a lot of problems. In that case, it would be difficult for admin to find reports or what’s new which is rendered at the bottom of the page.
Therefore separate pages for reports and what's new were created.
Reports were moved to a component that is shared between a separate page and the dashboard.
Why this change?
In https://web.dev/articles/preconnect-and-dns-prefetch, it describes
how hinting to the browser to preconnect to domains which we will
eventually use the connection for can help improve the time it takes to
load a page.
We are putting this behind an experimental flag so that we can test and
profile this in a production environment.
What does this change introduce?
Introduce a hidden experimental `experimental_preconnect_link_header`
site setting which when enabled will add the `preconnect` and
`dns-prefetch` resource hints to the response headers for full page load
requests.
Why this change?
This is a first pass at styling the editor for creating/editing/updating
an objects typed theme setting. Only the desktop view is being
considered at the current moment.
The objects typed theme setting is still behind a feature flag at this moment so there is no need for us to get the styling perfect. The purpose of this PR is to get us to a state which we can quickly iterate with a designer on.
This commit operates at three levels of abstraction:
1. We want to prevent user history rows from being unbounded in size.
This commit adds rails validations to limit the sizes of columns on
user_histories,
2. However, we don't want to prevent certain actions from being
completed if these columns are too long. In those cases, we truncate
the values that are given and store the truncated versions,
3. For endpoints that perform staff actions, we can further control
what is permitted by explicitly validating the params that are given
before attempting the action,
In AdminDashboardData we have a bunch of problem checks implemented as methods on that class. This PR absolves it of the responsibility by promoting each of those checks to a first class ProblemCheck. This way each of them can have their own priority and arbitrary functionality can be isolated in its own class.
Think "extract class" refactoring over and over. Since they were all moved we can also get rid of the @@problem_syms class variable which was basically the old version of the registry now replaced by ProblemCheck.realtime.
In addition AdminDashboardData::Problem value object has been entirely replaced with the new ProblemCheck::Problem (with compatible API).
Lastly, I added some RSpec matchers to simplify testing of problem checks and provide helpful error messages when assertions fail.
This commit adds new plugin show routes (`/admin/plugins/:plugin_id`) as we move
towards every plugin having a consistent UI/landing page.
As part of this, we are introducing a consistent way for plugins
to show an inner sidebar in their config page, via a new plugin
API `register_admin_config_nav_routes`
This accepts an array of links with a label/text, and an
ember route. Once this commit is merged we can start the process
of conforming other plugins to follow this pattern, as well
as supporting a single-page version of this for simpler plugins
that don't require an inner sidebar.
Part of /t/122841 internally
Previously, if the sso= payload was invalid Base64, but signed correctly, there would be no useful log or error. This commit improves things by:
- moving the base64 check before the signature checking so that it's properly surfaced
- split the ParseError exception into PayloadParseError and SignatureError
- add user-facing errors for both of those
- add/improve spec for both
This change creates a user setting that they can toggle if
they don't want to receive unread notifications when someone closes a
topic they have read and are watching/tracking it.
* A11Y: Update bulk selection keyboard shortcuts
Still a draft, but in current state this:
- adds `shift+b` as a keyboard shortcut to toggle bulk select
- adds `shift+d` as a keyboard shortcut to dismiss selected topic(s) (this
replaces `x r` and `x t` shortcuts)
- adds `x` as a keyboard shortcut to toggle selection (while in bulk select mode)
- fixes a bug with the `shift+a` shortcut, which was not working properly
Note that there is a breaking change here. Previously we had:
- `x r` to dismiss new topics
- `x t` to dismiss unread topics
However, this meant that we couldn't use `x` for selection, because the
itsatrap library does not allow the same character to be used both as a
single character shortcut and as the start of a sequence. The proposed
solution here is more consistent with other apps (Gmail, Github) that use
`x` to toggle selection.
Also, we never show both "Dismiss New" and "Dismiss Unread" in the same
screen, hence it makes sense to consolidate both actions under `shift+d`.
* Address review
When "lazy load categories" is enabled, the CategoryDrop component will
render at most 15 categories. If there are more categories, a "Show
more" link pointing to the categories page will be displayed.
This option was introduced at some point in the past, but was removed
during the work necessary to make Discourse work with a large number of
categories.
Follow up to commit 2e68ead45b.
Why this change?
Instead of manually loading files, we should just structure the plugin
so that it relies on Rails autoload strategy and avoid all the manual
`require_relative`s.
What does this change do?
1. Structure the plugin to use Rails autoloading convention
2. Remove onceff jobs that were added 5-6 years ago. There is no need to
carry these jobs anymore after such a long time.
3. Move setting of `SiteSetting.discourse_narrative_bot_enabled` to
`false` in the test environment from core into the plugin.
Why this change?
The `/admin/customize/themes/:id/schema/name` route is a work in
progress but we want to be able to start navigating to it from the
`/admin/customize/themes/:id` route.
What does this change do?
1. Move `adminCustomizeThemes.schema` to a child route of
`adminCustomizeThemes.show`. This is because we need the model
from the parent route and if it isn't a child route we end up
having to load the theme model again from the server.
1. Add the `objects_schema` attribute to `ThemeSettingsSerializer`
1. Refactor `SiteSettingComponent` to be able to render a button
so that we don't have to hardcode the button rendering into the
`SiteSettings::String` component
Adds a site setting to include a post's content in penalty message.
When silencing/suspending a user from a post, or a reviewable with
a post, adds an option to include a post's content in the email
message by default.
This commit changes the wording of the 50 site settings that
previously had the shortest descriptions (e.g. City for Disputes
was described as City for Disputes...) using AI-generated and then
human curated descriptions based on the Forum Helper persona
on Discourse Meta.
In future we may want to do more of these, this is only a first pass.
Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
When a post is created by an incoming email, we show
an envelope icon on it which then opens a modal with the
raw email contents. Previously this was staff (admin+mod)
only, but now this commit adds the `view_raw_email_allowed_groups`
site setting, so any group can be added to give users permission
to see this.
Why this change?
Previously in cac60a2c6b, I added support
for `type: "category"` for a property in the theme objects schema. This
commit extend the work previously to add support for types `topic`,
`post`, `group`, `upload` and `tag`.
Why this change?
This change adds validation for the default value for `type: objects` theme
settings when a setting theme field is uploaded. This helps the theme
author to ensure that the objects which they specifc in the default
value adhere to the schema which they have declared.
When an error is encountered in one of the objects, the error
message will look something like:
`"The property at JSON Pointer '/0/title' must be at least 5 characters
long."`
We use a JSON Pointer to reference the property in the object which is
something most json-schema validator uses as well.
What does this change do?
1. This commit once again changes the shape of hash returned by
`ThemeSettingsObjectValidator.validate`. Instead of using the
property name as the key previously, we have decided to avoid
multiple levels of nesting and instead use a JSON Pointer as the key
which helps to simplify the implementation.
2 Introduces `ThemeSettingsObjectValidator.validate_objects` which
returns an array of validation error messages for all the objects
passed to the method.
This commit adds a loading spinner when installing a theme as sometimes
installing a theme can take quite a bit of time this way we have some
indication that things are still working as the theme is being
installed.
This PR adds a new scheduled problem check that simply tries to connect to Twitter OAuth endpoint to check that it's working. It is using the default retry strategy of 2 retries 30 seconds apart.
Why this change?
This regressed in 6e9fbb5bab because we
had a `request.xhr?` check before we decide to block requests. However,
there could not none-xhr requests which we need to block as well at the
end of each system test when `@@block_requests` is true.
This also reverts commit 6437f27f90.
Also, remove experimental setting and simply use top_menu for feature detection
This means that when people eventually enable the hot top menu, there will
be topics in it
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Why this change?
This reverts 725561cf4b as it did not
address the root cause of the problem even though it fixed the failing tests we were seeing
when running `bundle exec rspec --tag ~type:multisite --order random:776 spec/system/admin_customize_form_templates_spec.rb spec/system/admin_sidebar_navigation_spec.rb spec/system/admin_site_setting_search_spec.rb spec/system/composer/dont_feed_the_trolls_popup_spec.rb spec/system/composer/review_media_unless_trust_level_spec.rb spec/system/create_account_spec.rb spec/system/editing_sidebar_tags_navigation_spec.rb spec/system/email_change_spec.rb spec/system/emojis/emoji_deny_list_spec.rb spec/system/group_activity_spec.rb spec/system/hashtag_autocomplete_spec.rb spec/system/network_disconnected_spec.rb spec/system/post_menu_spec.rb spec/system/post_small_action_spec.rb spec/system/tags_intersection_spec.rb spec/system/topic_list_focus_spec.rb spec/system/topic_page_spec.rb spec/system/user_page/user_profile_info_panel_spec.rb spec/system/viewing_group_members_spec.rb spec/system/viewing_navigation_menu_preferences_spec.rb`.
The root cause here is that `before_action`s added to a controller is
order dependent. As such, some requests were not setting the cookie
because the `before_action` callback was not even hit as a prior
`before_action` callbacks has raised an error such as the `check_xhr`
`before_action` callback.
To resolve the problem, we need to add the `prepend: true` option in
our monkey patch of `ApplicationController` to ensure that the
`before_action` callback which we have added is always run first.
This change also makes a couple of changes:
1. Improve the response body when a request is blocked by the `BlockRequestsMiddleware` middleware
so that it makes debugging easier.
2. Only set the cookies for non-xhr HTML format requests. Setting it for
other formats is kind of pointless.
Why this change?
We noticed that running `LOAD_PLUGINS=1 rspec --seed=38855 plugins/chat/spec/system/chat_new_message_spec.rb` locally
results in the system tests randomly failing. When we inspected the
request logs closely, we noticed that a `/presence/get` request from a
previous rspec example was being processed when a new rspec example is
already being run. We know it was from the previous rspec example
because inspecting the auth token showed the request using the auth
token of a user from the previous example. However, when a request using
an auth token from a previous example is used it ends up logging out the
same user on the server side because the user id in the cookie is the same
due to the use of `fab!`.
I did some research and there is apparently no way to wait until all
inflight requests by the browser has completed through capybara or
selenium. Therefore, we will add an identifier by attaching a cookie to all non-xhr requests so that
xhr requests which are triggered subsequently will contain the cookie in the request.
In the `BlockRequestsMiddleware` middleware, we will then reject any
requests when the value of the identifier in the cookie does not match the current rspec's example
location.
To see the problem locally, change `Auth::DefaultCurrentUserProvider.find_v1_auth_cookie` to the following:
```
def self.find_v1_auth_cookie(env)
return env[DECRYPTED_AUTH_COOKIE] if env.key?(DECRYPTED_AUTH_COOKIE)
env[DECRYPTED_AUTH_COOKIE] = begin
request = ActionDispatch::Request.new(env)
cookie = request.cookies[TOKEN_COOKIE]
# don't even initialize a cookie jar if we don't have a cookie at all
if cookie&.valid_encoding? && cookie.present?
puts "#{env["REQUEST_PATH"]} #{request.cookie_jar.encrypted[TOKEN_COOKIE]&.with_indifferent_access}"
request.cookie_jar.encrypted[TOKEN_COOKIE]&.with_indifferent_access
end
end
end
```
After which run the following command: `LOAD_PLUGINS=1 rspec --format documentation --seed=38855 plugins/chat/spec/system/chat_new_message_spec.rb`
It takes a few tries but the last spec should fail and you should see something like this:
```
assets/chunk.c16f6ba8b6824baa47ac.d41d8cd9.js {"token"=>"37d995a4b65395d3b343ec70fff915b4", "user_id"=>3382, "username"=>"bruce0", "trust_level"=>1, "issued_at"=>1708591735}
/assets/chunk.050148142e1d2dc992dd.d41d8cd9.js {"token"=>"37d995a4b65395d3b343ec70fff915b4", "user_id"=>3382, "username"=>"bruce0", "trust_level"=>1, "issued_at"=>1708591735}
/chat/api/channels/527/messages {"token"=>"37d995a4b65395d3b343ec70fff915b4", "user_id"=>3382, "username"=>"bruce0", "trust_level"=>1, "issued_at"=>1708591735}
/uploads/default/test_0/optimized/1X/_129430568242d1b7f853bb13ebea28b3f6af4e7_2_512x512.png {"token"=>"37d995a4b65395d3b343ec70fff915b4", "user_id"=>3382, "username"=>"bruce0", "trust_level"=>1, "issued_at"=>1708591735}
redirects to existing chat channel
redirects to chat channel if recipients param is missing (PENDING: Temporarily skipped with xit)
with multiple users
/favicon.ico {"token"=>"9a75c114c4d3401509a23d240f0a46d4", "user_id"=>3382, "username"=>"bruce0", "trust_level"=>1, "issued_at"=>1708591736}
/chat/new-message {"token"=>"9a75c114c4d3401509a23d240f0a46d4", "user_id"=>3382, "username"=>"bruce0", "trust_level"=>1, "issued_at"=>1708591736}
/presence/get {"token"=>"37d995a4b65395d3b343ec70fff915b4", "user_id"=>3382, "username"=>"bruce0", "trust_level"=>1, "issued_at"=>1708591735}
```
Note how the `/presence/get` request is using a token from the previous example.
Co-authored-by: David Taylor <david@taylorhq.com>
We're starting to use this system for non-ember-5 deprecations, so linking to the Ember 5 topic doesn't make sense. Instead, we can include the deprecation ID to help with identifying the issue.
The Digital Services Act requires a checkbox for any user who's flagging a post as illegal to confirm that they are flagging in good faith. This PR adds that.
Why this change?
This change supports a property of `type: category` in the schema that
is declared for a theme setting object. Example:
```
sections:
type: objects
schema:
name: section
properties:
category_property:
type: category
```
The value of a property declared as `type: category` will have to be a
valid id of a row in the `categories` table.
What does this change do?
Adds a property value validation step for `type: category`. Care has
been taken to ensure that we do not spam the database with a ton of
requests if there are alot of category typed properties. This is done by
walking through the entire object and collecting all the values for
properties typed category. After which, a single database query is
executed to validate which values are valid.
Why this change?
Firstly, note that this is not a security commit because this feature is
still in development and should not be used anywhere.
The reason we want to set a limit here is to greatly reduce the
possibility of a DoS attack in the future via `ThemeSetting` where
someone would set an arbituary large json string in
`ThemeSetting#json_value` and causing the server to run out of resources
trying to serialize/deserialize the value.
What does this change do?
Adds an ActiveRecord validation to ensure that the bytesize of the json
string being stored is smaller than or equal to 0.5mb. We believe 0.5mb
is a decent limit for now but we can review the limit in the future if
we believe it is too small.
Why this change?
The logic for validating a theme setting's value and default value was
not consistent as each part of the code would implement its own logic.
This is not ideal as the default value may be validated differently than
when we are setting a new value. Therefore, this commit seeks to
refactor all the validation logic for a theme setting's value into a
single service class.
What does this change do?
Introduce the `ThemeSettingsValidator` service class which holds all the
necessary helper methods required to validate a theme setting's value
A while ago we increased group SMTP read and open timeouts
to address issues we were seeing with Gmail sometimes giving
really long timeouts for these values. The commit was:
3e639e4aa7
Now, we want to increase all SMTP read timeouts to 30s,
since the 5s is too low sometimes, and the ruby Net::SMTP
stdlib also defaults to 30s.
Also, we want to slightly tweak the group smtp email job
not to fail if the IncomingEmail log fails to create, or if
a ReadTimeout is encountered, to avoid retrying the job in sidekiq
again and sending the same email out.
This commit introduces the possibility to stream messages. To allow plugins to use streaming this commit also ships a `ChatSDK` library to allow to interact with few parts of discourse chat.
```ruby
ChatSDK::Message.create_with_stream(raw: "test") do |helper|
5.times do |i|
is_streaming = helper.stream(raw: "more #{i}")
next if !is_streaming
sleep 2
end
end
```
This commit also introduces all the frontend parts:
- messages can now be marked as streaming
- when streaming their content will be updated when a new content is appended
- a special UI will be showing (a blinking indicator)
- a cancel button allows the user to stop the streaming, when cancelled `helper.stream(...)` will return `false`, and the plugin can decide exit early
Why this change?
This commit updates `ThemeSettingsObjectValidator` to validate a
property's value against the validations listed in the schema.
For string types, `min_length`, `max_length` and `url` are supported.
For integer and float types, `min` and `max` are supported.
Why this change?
This change adds property value validation to `ThemeSettingsObjectValidator`
for the following types: "string", "integer", "float", "boolean", "enum". Note
that this class is not being used anywhere yet and is still in
development.
Why this change?
For some reason, we were setting up bootsnap manually even though the
official documentation suggests requiring `bootsnap/setup` which will
setup bootsnap using the default configuration. Because we were calling
`Bootsnap.setup` manually, we did not set the `development_mode` option
which defaults to `true`. Hence, we were running bootsnap in development
mode even in the production environment which I suppose is not ideal.
What does this change do?
Instead of calling `Bootsnap.setup` manually, we can just use `require
'bootsnap/setup' instead.`
Affects the following settings:
delete_all_posts_and_topics_allowed_groups
experimental_new_new_view_groups
enable_experimental_admin_ui_groups
custom_summarization_allowed_groups
pm_tags_allowed_for_groups
chat_allowed_groups
direct_message_enabled_groups
chat_message_flag_allowed_groups
This turns off client: true for these group-based settings,
because there is no guarantee that the current user gets all
their group memberships serialized to the client. Better to check
server-side first.
The strict-dynamic CSP directive is supported in all our target browsers, and makes for a much simpler configuration. Instead of allowlisting paths, we use a per-request nonce to authorize `<script>` tags, and then those scripts are allowed to load additional scripts (or add additional inline scripts) without restriction.
This becomes especially useful when admins want to add external scripts like Google Tag Manager, or advertising scripts, which then go on to load a ton of other scripts.
All script tags introduced via themes will automatically have the nonce attribute applied, so it should be zero-effort for theme developers. Plugins *may* need some changes if they are inserting their own script tags.
This commit introduces a strict-dynamic-based CSP behind an experimental `content_security_policy_strict_dynamic` site setting.
This commit is the first of a series of commits that will allow themes to define complex settings types by declaring a schema of the setting structure that Discourse core will use to build a UI for the setting automatically. We implement the navigation logic and support for multiple levels of nesting in this commit and we'll continue building this new system gradually in future commits.
Internal topic: t/116870.
Why this change?
This is a first pass at adding an objects validator which main's job is
to validate an object against a defined schema which we will support. In
this pass, we are simply validating that properties that has been marked
as required are present in the object.
Why this change?
We have been debugging flaky system tests and noticed in https://github.com/discourse/discourse/actions/runs/7911902047/job/21596791343?pr=25690
that ActiveRecord connection checkout timeouts are encountered because
the Capybara server thread is processing requests even after
`Capybara.reset_session!` and ActiveRecord's `teardown_fixtures` have already been call.
The theory here is that an inflight request can still hit the Capybara
server even after `Capybara.reset_session!` has been called and end up
eating up an ActiveRecord connection for too long and also messing with
the database outside of a transaction.
What does this change do?
This change adds a `BlockRequestsMiddleware` middleware in the test
environment which is enabled to reject all incoming requests at the end
of each system test and before `Capybara.reset_session!` is called. At
the start of each RSpec test, the middleware is disabled again.
This would allow a theme component (or an API call) to reset the bump
date of a topic to a given post's created_at date.
I picked `post_id` as the parameter here because it provides a bit of
extra protection against accidentally resetting the bump date to a date
that doesn't make sense.
When we launched the new illegal flag type, there were a few problems with the translations:
The translation for the message in the e-mail was missing and in the review queue, the message read: "Is this it's illegal?"
In this PR the missing translation key has been added. For the review queue there was a coupling of the name rendering to whether the flag is of "custom" type, but this is also used for deciding whether we render the textbox for additional details. I think these two things should not be coupled together. For now I have instead hard-coded the existing "custom" types when formatting the name. We can potentially improve this later.
'please refresh, or you may experience unexpected behavior' sounds quite threatening to me. 'please refresh to to keep things working smoothly' conveys the same information in a more friendly way
This commit includes several changes to make hashtags work when "lazy
load categories" is enabled. The previous hashtag implementation use the
category colors CSS variables, but these are not defined when the site
setting is enabled because categories are no longer preloaded.
This commit implements two fundamental changes:
1. load colors together with the other hashtag information
2. load cooked hashtag data asynchronously
The first change is implemented by adding "colors" to the HashtagItem
model. It is a list because two colors are returned for subcategories:
the color of the parent category and subcategory.
The second change is implemented on the server-side in a new route
/hashtags/by-ids and on the client side by loading previously unseen
hashtags, generating the CSS on the fly and injecting it into the page.
There have been minimal changes outside of these two fundamental ones,
but a refactoring will be coming soon to reuse as much of the code
and maybe favor use of `style` rather than injecting CSS into the page,
which can lead to page rerenders and indefinite grow of the styles.
This commit changes `max_image_megapixels` to be used
as is without multiplying by 2 to give extra leway.
We found in reality this was just causing confusion
for admins, especially with the already permissive
40MP default.
When we show the links to installed plugins in the admin
sidebar (for plugins that have custom admin routes) we were
previously only doing this if you opened /admin, not if you
navigated there from the main forum. We should just always
preload this data if the user is admin.
This commit also changes `admin_sidebar_enabled_groups` to
not be sent to the client as part of ongoing efforts to
not check groups on the client, since not all a user's groups
may be serialized.
Why this change?
We have been seeing checkout timeouts happening on CI when using the
default of 5 seconds. This can happen in system tests when the server
has to process many requests using the same database connection.
Therefore, we will double the timeout for now and monitor if stuff
continues to timeout.
Why this change?
This commit introduces an experimental `type: objects` theme setting
which will allow theme developers to store a collection of objects as
JSON in the database. Currently, the feature is still in development and
this commit is simply setting up the ground work for us to introduce the
feature in smaller pieces.
What does this change do?
1. Adds a `json_value` column as `jsonb` data type to the `theme_settings` table.
2. Adds a `experimental_objects_type_for_theme_settings` site setting to
determine whether `ThemeSetting` records of with the `objects` data
type can be created.
3. Updates `ThemeSettingsManager` to support read/write access from the
`ThemeSettings#json_value` column.
Followup fb087b7ff6
post_links_allowed_groups is an odd check tied to
unrestricted_link_posting? in PostGuardian, in that
it doesn't have an escape hatch for staff like most
of the rest of these group based settings.
It doesn't make sense to exclude admins or mods from
posting links, so just always allow them to avoid confusion.
Affects the following settings:
* whispers_allowed_groups
* anonymous_posting_allowed_groups
* personal_message_enabled_groups
* shared_drafts_allowed_groups
* here_mention_allowed_groups
* uploaded_avatars_allowed_groups
* ignore_allowed_groups
This turns off `client: true` for these group-based settings,
because there is no guarantee that the current user gets all
their group memberships serialized to the client. Better to check
server-side first.
When enabled, the workbox caching logic in the service worker will be replaced with a very simple offline error page. We plan to use this as an experiment to see how it affects performance and stability of Discourse.
Why this change?
I have been investigating transaction related issues with our system
tests and I have a hard time figuring out what is causing the problem.
To help simplify our environment further, we will set the pool size in
the test environment to 1 so that it is impossible for us to be fetching
a different connection between the threads since they all share the
connection pool.
Also set `reaping_frequency` to `0` to ensure we don't reap any
connection ensuring the same connection is always used.
- Created a new migration for here_mention
- Updated existing migration for here_mention
- Updated site settings for here_mention, create_tag, and
send_email_messages
* DEV: Update min trust level to tag topics migration to groups
- Update the existing migration to include staff and admin
- Update default values
- Added migration to include staff and admin cases
Checking group permissions on the client does not work,
since not all groups are serialized to the client all
the time. We can check `uploaded_avatars_allowed_groups`
on the server side and serialize to the current user
instead.
There are some cases where staff (admins/mods) can
be in lower trust levels, so some of these checks will
fail for them. Since we want to keep allowing this (for now)
we should set most settings to also default to be allowed
for staff too, since the old `has_trust_level?` check
worked in this way.
