Previous to this change when both `normalize_emails` and `hide_email_address_taken`
is enabled the expected `account_exists` email was only sent on exact email
matches.
This expands it so it also sends an email to the canonical email owner.
Why this change?
Currently, we do not have an easy way to test themes and theme components
using Rails system tests. While we support QUnit acceptance tests for
themes and theme components, QUnit acceptance tests stubs out the server
and setting up the fixtures for server responses is difficult and can lead to a
frustrating experience. System tests on the other hand allow authors to
set up the test fixtures using our fabricator system which is much
easier to use.
What does this change do?
In order for us to allow authors to run system tests with their themes
installed, we are adding a `upload_theme` helper that is made available
when writing system tests. The `upload_theme` helper requires a single
`directory` parameter where `directory` is the directory of the theme
locally and returns a `Theme` record.
Until now, we have allowed testing themes in production environments via `/theme-qunit`. This was made possible by hacking the ember-cli build so that it would create the `tests.js` bundle in production. However, this is fundamentally problematic because a number of test-specific things are still optimized out of the Ember build in production mode. It also makes asset compilation significantly slower, and makes it more difficult for us to update our build pipeline (e.g. to introduce Embroider).
This commit removes the ability to run qunit tests in production builds of the JS app when the Embdroider flag is enabled. If a production instance of Discourse exists exclusively for the development of themes (e.g. discourse.theme-creator.io) then they can add `EMBER_ENV: development` to their `app.yml` file. This will build the entire app in development mode, and has a significant performance impact. This must not be used for real production sites.
This commit also refactors many of the request specs into system specs. This means that the tests are guaranteed to have Ember assets built, and is also a better end-to-end test than simply checking for the presence of certain `<script>` tags in the HTML.
Discourse core now builds and runs with Embroider! This commit adds
the Embroider-based build pipeline (`USE_EMBROIDER=1`) and start
testing it on CI.
The new pipeline uses Embroider's compat mode + webpack bundler to
build discourse code, and leave everything else (admin, wizard,
markdown-it, plugins, etc) exactly the same using the existing
Broccoli-based build as external bundles (<script> tags), passed
to the build as `extraPublicTress` (which just means they get
placed in the `/public` folder).
At runtime, these "external" bundles are glued back together with
`loader.js`. Specifically, the external bundles are compiled as
AMD modules (just as they were before) and registered with the
global `loader.js` instance. They expect their `import`s (outside
of whatever is included in the bundle) to be already available in
the `loader.js` runtime registry.
In the classic build, _every_ module gets compiled into AMD and
gets added to the `loader.js` runtime registry. In Embroider,
the goal is to do this as little as possible, to give the bundler
more flexibility to optimize modules, or omit them entirely if it
is confident that the module is unused (i.e. tree-shaking).
Even in the most compatible mode, there are cases where Embroider
is confident enough to omit modules in the runtime `loader.js`
registry (notably, "auto-imported" non-addon NPM packages). So we
have to be mindful of that an manage those dependencies ourselves,
as seen in #22703.
In the longer term, we will look into using modern features (such
as `import()`) to express these inter-dependencies.
This will only be behind a flag for a short period of time while we
perform some final testing. Within the next few weeks, we intend
to enable by default and remove the flag.
---------
Co-authored-by: David Taylor <david@taylorhq.com>
We're seeing a large number of log noise from this endpoint due to malicious scanners that are trying to send clever params and seeing if they can break something.
This change simply rescues any NoMethodError during parameter parsing and re-raises a Discourse::InvalidParameters exception, which will be caught and render a 400.
Previously we were patching ember-cli so that it would split the test bundle into two halves: the helpers, and the tests themselves. This was done so that we could use the helpers for `/theme-qunit` without needing to load all the core tests. This patch has proven problematic to maintain, and will become even harder under Embroider.
This commit removes the patch, so that ember-cli goes back to generating a single `tests.js` bundle. This means that core test definitions will now be included in the bundle when using `/theme-qunit`, and so this commit also updates our test module filter to exclude them from the run. This is the same way that we handle plugin tests on the regular `/tests` route, and is fully supported by qunit.
For now, this keeps `/theme-qunit` working in both development and production environments. However, we are very likely to drop support in production as part of the move to Embroider.
They're both constant per-instance values, there is no need to store them
in the session. This also makes the code a bit more readable by moving
the `session_challenge_key` method up to the `DiscourseWebauthn` module.
Why this change?
As part of our ongoing efforts to security harden the Discourse
application, we are adding the `cross_origin_opener_policy_header` site setting
which allows the `Cross-Origin-Opener-Policy` response header to be set on requests
that preloads the Discourse application. In more technical terms, only
GET requests that are not json or xhr will have the response header set.
The `cross_origin_opener_policy_header` site setting is hidden for now
for testing purposes and will either be released as a public site
setting or be remove if we decide to be opinionated and ship a default
for the `Cross-Origin-Opener-Policy` response header.
This commit adds some system specs to test uploads with
direct to S3 single and multipart uploads via uppy. This
is done with minio as a local S3 replacement. We are doing
this to catch regressions when uppy dependencies need to
be upgraded or we change uppy upload code, since before
this there was no way to know outside manual testing whether
these changes would cause regressions.
Minio's server lifecycle and the installed binaries are managed
by the https://github.com/discourse/minio_runner gem, though the
binaries are already installed on the discourse_test image we run
GitHub CI from.
These tests will only run in CI unless you specifically use the
CI=1 or RUN_S3_SYSTEM_SPECS=1 env vars.
For a history of experimentation here see https://github.com/discourse/discourse/pull/22381
Related PRs:
* https://github.com/discourse/minio_runner/pull/1
* https://github.com/discourse/minio_runner/pull/2
* https://github.com/discourse/minio_runner/pull/3
`ReviewableQueuedPost` got refactored a while back to use the more
appropriate `target_created_by` for the user of the post being queued
instead of `created_by`. The change was not extended to the `DELETE
/review/:id` endpoint leading to error responses for a user attempting
to deleting their own queued post.
This fix extends the `Reviewable` lookup implementation in
`ReviewablesController#destroy` and Guardian implementation to account
for this change.
This PR adds a new toggle to switch the (new) /new list between showing topics with new replies (a.k.a unread topics), new topics, or everything mixed together.
Our code assumed the content_range interval was inclusive, but they are open-ended due to Postgres' [discrete range types](https://www.postgresql.org/docs/current/rangetypes.html#RANGETYPES-DISCRETE), meaning [1,2] will be represented as [1,3).
It also fixes some flaky tests due to test data not being correctly setup and the registry not being resetted after each test.
When we receive the stream parameter, we'll queue a job that periodically publishes partial updates, and after the summarization finishes, a final one with the completed version, plus metadata.
`summary-box` listens to these updates via MessageBus, and updates state accordingly.
This commit removes any logic in the app and in specs around
enable_experimental_hashtag_autocomplete and deletes some
old category hashtag code that is no longer necessary.
It also adds a `slug_ref` category instance method, which
will generate a reference like `parent:child` for a category,
with an optional depth, which hashtags use. Also refactors
PostRevisor which was using CategoryHashtagDataSource directly
which is a no-no.
Deletes the old hashtag markdown rule as well.
In #20135 we prevented invalid inputs from being accepted in category setting form fields on the front-end. We didn't do anything on the back-end at that time, because we were still discussing which path we wanted to take. Eventually we decided we want to move this to a new CategorySetting model.
This PR moves the num_auto_bump_daily from custom fields to the new CategorySetting model.
In addition it sets the default value to 0, which exhibits the same behaviour as when the value is NULL.
This is a similar fix to 32d4810e2b
Why this change?
Prior to this change, there is a bug in `TopicsController#bulk`
where it does not dismiss new unred posts in sub-subcategories when the
`category_id` and `include_subcategories=true` params are present. This
is because the controller did not account for sub-subcategories when
fetching the category ids of the new topics that should be dismissed.
This commit fixes the problem by relying on the `Category.subcategory_ids` class
method which accounts for sub-subcategories.
What is the problem here?
In multiple controllers, we are accepting a `limit` params but do not
impose any upper bound on the values being accepted. Without an upper
bound, we may be allowing arbituary users from generating DB queries
which may end up exhausing the resources on the server.
What is the fix here?
A new `fetch_limit_from_params` helper method is introduced in
`ApplicationController` that can be used by controller actions to safely
get the limit from the params as a default limit and maximum limit has
to be set. When an invalid limit params is encountered, the server will
respond with the 400 response code.
What is the context for this change?
Prior to this change, there is a bug in `TopicsController#reset_new`
where it does not dismiss new topics in sub-subcategories when the
`category_id` and `include_subcategories=true` params are present. This
is because the controller did not account for sub-subcategories when
fetching the category ids of the new topics that should be dismissed.
This commit fixes the problem by relying on the `Category.subcategory_ids` class
method which accounts for sub-subcategories.
Context of this change:
There are two site settings which an admin can configured to set the
default categories and tags that are shown for a new user. `default_navigation_menu_categories`
is used to determine the default categories while
`default_navigation_menu_tags` is used to determine the default tags.
Prior to this change when seeding the defaults, we will filter out the
categories/tags that the user do not have permission to see. However,
this means that when the user does eventually gain permission down the
line, the default categories and tags do not appear.
What does this change do?
With this commit, we have changed it such that all the categories and tags
configured in the `default_navigation_menu_categories` and
`default_navigation_menu_tags` site settings are seeded regardless of
whether the user's visibility of the categories or tags. During
serialization, we will then filter out the categories and tags which the
user does not have visibility of.
Why this change?
We're already displaying a category's description as the title attribute
on the category section link. We should do the same for tags as well.
Recently we started giving admins a notice in the advice panel when their translations have become outdated due to changes in core. However, we didn't include any additional information.
This PR adds more information about the outdated translation inside the site text edit page, together with an option to dismiss the warning.
Performing a `Delete User`/`Delete and Block User` reviewable actions for a
queued post reviewable from the `review.show` route results in an error
popup even if the action completes successfully.
This happens because unlike other reviewable types, a user delete action
on a queued post reviewable results in the deletion of the reviewable
itself. A subsequent attempt to reload the reviewable record results in
404. The deletion happens as part of the call to `UserDestroyer` which
includes a step for destroying reviewables created by the user being
destroyed. At the root of this is the creator of the queued post
being set as the creator of the reviewable as instead of the system
user.
This change assigns the creator of the reviewable to the system user and
uses the more approapriate `target_created_by` column for the creator of the
post being queued.
Why this change?
Prior to this change, we would only return tags that are used in at
least one public topic. However, this is confusing for users because the
tag could be used in a restricted category and that is not considered a
"public" topic. Instead, we will just display all the tags in the edit
tags navigation modal as long as it is visible to the user.
Follow up to: 56e792d
Adds a test to check that there is an api scope for the t/external_id
route. Plus checks many other topic routes that should have scopes.
Why this change?
Prior to this change, dismissing unreads posts did not publish the
changes across clients for the same user. As a result, users can end up
seeing an unread count being present but saw no topics being loaded when
visiting the `/unread` route.
* FEATURE: Inline topic summary. Cached version accessible to everyone.
Anons and non-members of the `custom_summarization_allowed_groups_map` groups can see cached summaries for any accessible topic. After the first 12 hours and if the posts to summarize have changed, allowed users clicking on the button will automatically re-generate it.
* Ensure chat summaries work and prevent model hallucinations when there are no messages.
Recently, site setting watched_precedence_over_muted was introduced - https://github.com/discourse/discourse/pull/22252
In this PR, we are allowing users to override it. The option is only displayed when the user has watched categories and muted tags, or vice versa.
What is the problem?
Before this change, we were relying on the `/tags` endpoint which
returned all the tags that are visible to a give user on the site leading to potential performance problems.
The attribute keys of the response also changes based on the `tags_listed_by_group` site setting.
What is the fix?
This commit fixes the problems listed above by creating a dedicate `#list` action in the
`TagsController` to handle the listing of the tags in the edit
navigation menu tags modal. This is because the `TagsController#index`
action was created specifically for the `/tags` route and the response
body does not really map well to what we need. The `TagsController#list`
action added here is also much safer since the response is paginated and
we avoid loading a whole bunch of tags upfront.
]When changing fonts in the `/wizard/steps/styling` step of
the wizard, users would not see the font loaded straight away,
having to switch to another one then back to the original to
see the result. This is because we are using canvas to render
the style preview and this fails with a Chrome-based intervention
when font loading is taking too long:
> [Intervention] Slow network is detected. See
https://www.chromestatus.com/feature/5636954674692096 for more details.
Fallback font will be used while loading:
https://sea2.discourse-cdn.com/business7/fonts/Roboto-Bold.ttf?v=0.0.9
We can get around this by manually loading the fonts selected using
the FontFace JS API when the user selects them and before rerendering
the canvas. This just requires preloading more information about the
fonts if the user is admin so the wizard can query this data.
This is the first of a number of PRs aimed at helping admins manage their translation overrides. It simply adds a list of available interpolation keys below the input field when editing an override.
It also includes custom interpolation key.
This PR splits up the preference that controls the count vs dot and destination of sidebar links, which is really hard to understand, into 2 simpler checkboxes:
The new preferences/checkboxes are off by default, but there are database migrations to switch the old preference to the new ones so that existing users don't have to update their preferences to keep their preferred behavior of sidebar links when this changed is rolled out.
Internal topic: t/103529.
Don't cache user_fields on users separately from custom_fields, since they can get out of sync.
---------
Co-authored-by: Daniel Waterworth <me@danielwaterworth.com>
Upstream added a capital 'T' to the 'Translation missing' message in https://github.com/ruby-i18n/i18n/commit/c5c6e753f3. This caused our translate accelerator patch to diverge, and the change in case affected a number of our specs. This commit updates the translate accelerator to match the upstream casing, and introduces a spec to detect future divergence.
This method is a huge footgun in production, since it calls
the Redis KEYS command. From the Redis documentation at
https://redis.io/commands/keys/:
> Warning: consider KEYS as a command that should only be used in
production environments with extreme care. It may ruin performance when
it is executed against large databases. This command is intended for
debugging and special operations, such as changing your keyspace layout.
Don't use KEYS in your regular application code.
Since we were only using `delete_prefixed` in specs (now that we
removed the usage in production in 24ec06ff85)
we can remove this and instead rely on `use_redis_snapshotting` on the
particular tests that need this kind of clearing functionality.
Communities can use sidebar or header dropdown, therefore navigation menu is a better name settings in 2 places:
- Old user sidebar preferences;
- Site setting about default tags and categories.
* FEATURE: Content custom summarization strategies.
This PR establishes a pattern for plugins to register alternative ways of summarizing content by extending a class that defines an interface.
Core controls which strategy we'll use and who has access to it through the `summarization_strategy` and `custom_summarization_allowed_groups`. It also defines the UI for summarizing topics.
Other plugins can access this summarization mechanism and implement their features, removing cross-plugin customizations, as it currently happens between chat and the discourse-ai plugin.
* Group membership validation and rate limiting
* Work with objects instead of classes
* Port summarization feature from discourse-ai to chat
* Rename available summaries to 'Top Replies' and 'Summary'
* DEV: Implement staff logs for user columns edits
* deleted extra space in staff logger detail string, deleted string when no changes are made, added basic test coverage for EditDirectoryColumnsController
* fixed change made to #self.staff_actions un UserHistory
* implemented a method that builds the details, previous_values and new_values in a dynamic way
* removed details of changes
* refactored small merge
Display modal for combined new and unread view with options:
- [x] Dismiss new topics
- [x] Dismiss new posts
- [ ] Stop tracking these topics so they stop appearing in my new list
Prior to this commit, we didn't have RTL versions of our admin and plugins CSS bundles and we always served LTR versions of those bundles even when users used an RTL locale, causing admin and plugins UI elements to never look as good as when an LTR locale was used. Example of UI issues prior to this commit were: missing margins, borders on the wrong side and buttons too close to each other etc.
This commit creates an RTL version for the admin CSS bundle as well as RTL bundles for all the installed plugins and serves those RTL bundles to users/sites who use RTL locales.
* FEATURE: reduce avatar sizes to 6 from 20
This PR introduces 3 changes:
1. SiteSetting.avatar_sizes, now does what is says on the tin.
previously it would introduce a large number of extra sizes, to allow for
various DPIs. Instead we now trust the admin with the size list.
2. When `avatar_sizes` changes, we ensure consistency and remove resized
avatars that are not longer allowed per site setting. This happens on the
12 hourly job and limited out of the box to 20k cleanups per cycle, given
this may reach out to AWS 20k times to remove things.
3.Our default avatar sizes are now "24|48|72|96|144|288" these sizes were
very specifically picked to limit amount of bluriness introduced by webkit.
Our avatars are already blurry due to 1px border, so this corrects old blur.
This change heavily reduces storage required by forums which simplifies
site moves and more.
Co-authored-by: David Taylor <david@taylorhq.com>
This commit adds modifiers that allow plugins to change how categories and groups are prefetched into the application and listed in the respective controllers.
Possible use cases:
- prevent some categories/groups from being prefetched when the application loads for performance reasons.
- prevent some categories/groups from being listed in their respective index pages.
This commit introduces a new `within_user_updater_transaction` event that's triggered inside the transaction that saves user updates in `UserUpdater`. Plugins can hook into the transaction using the event to include custom changes in the transaction. Callbacks for this event receive 2 arguments:
1. the user being saved
2. the changed attributes that are passed to `UserUpdater`.
There's also new modifier in this commit called `users_controller_update_user_params` to allow plugins to allowlist custom params in the `UsersController` which eventually end up getting passed as attributes to the `UserUpdater` and the new `within_user_updater_transaction` event where they can be used to perform additional updates using the custom params.
-----
New API is used in https://github.com/discourse/discourse-mailinglist-integration/pull/1.
When a user chooses to move a topic/message to an existing topic/message, they can now opt to merge the posts chronologically (using a checkbox in the UI).
The field more_topic_url is already included in the response preloaded in categories#index
However this field was missing if a request was subsequently made to update the page using
the end-points /categories_and_latest or /categories_and_top. This could lead the client
app to display incorrect information if it relied on this information to update the UI.
What is the problem?
In the test environement, we were calling `SiteSetting.setting` directly
to introduce new site settings. However, this leads to changes in state of the SiteSettings
hash that is stored in memory as test runs. Changing or leaking states
when running tests is one of the major contributors of test flakiness.
An example of how this resulted in test flakiness is our `spec/integrity/i18n_spec.rb` spec file which
had a test case that would fail because a new "plugin_setting" site
setting was registered in another test case but the site setting did not
have translations for the site setting set.
What is the fix?
There are a couple of changes being introduced in this commit:
1. Make `SiteSetting.setting` a private method as it is not safe to be
exposed as a public method of the `SiteSetting` class
2. Change test cases to use existing site settings in Discourse instead
of creating custom site settings. Existing site settings are not
removed often so we don't really need to dynamically add new site
settings in test cases. Even if the site settings being used in test
cases are removed, updating the test cases to rely on other site
settings is a very easy change.
3. Set up a plugin instance in the test environment as a "fixture"
instead of having each test create its own plugin instance.
This commit makes some fundamental changes to how hashtag cooking and
icon generation works in the new experimental hashtag autocomplete mode.
Previously we cooked the appropriate SVG icon with the cooked hashtag,
though this has proved inflexible especially for theming purposes.
Instead, we now cook a data-ID attribute with the hashtag and add a new
span as an icon placeholder. This is replaced on the client side with an
icon (or a square span in the case of categories) on the client side via
the decorateCooked API for posts and chat messages.
This client side logic uses the generated hashtag, category, and channel
CSS classes added in a previous commit.
This is missing changes to the sidebar to use the new generated CSS
classes and also colors and the split square for categories in the
hashtag autocomplete menu -- I will tackle this in a separate PR so it
is clearer.
The welcome topic user tip was for admins only, but in general, user
tips should be used for guiding new users through the features that
Discourse offers. For this reason, we decided to remove the user tip.
This commit also includes a few more copy tweaks to the welcome topic.
When uploading images via direct to S3 upload, we were
assuming that we could not pre-emptively check the file
size because the client may do preprocessing to reduce
the size, and UploadCreator could also further reduce the
size.
This, however, is not true of gifs, so we would have an
issue where you upload a gif > the max_image_size_kb
setting and had to wait until the upload completed for
this error to show.
Now, instead, when we direct upload gifs to S3, we check
the size straight away and present a file size error to
the user rather than making them wait. This will increase
meme efficiency by approximately 1000%.
What is the problem?
In `SvgSpriteController#search` and `SvgSpriteController#icon_picker_search`, the controller actions
was using the `RailsMultisite::ConnectionManagement.with_hostname` API
but `params[:hostname]` was always `nil` because the routes does not
have a `:hostname` param component and the client does not ever pass the
`:hostname` param when making the request. When `RailsMultisite::ConnectionManagement.with_hostname` is
used with a `nil` argument, it ends up connecting to the default
multisite database. Usually this would be bad because we're allowing a
site in a multisite setup to connect to another site but thankfully no
private data is being leaked here.
What is the fix?
Since `SvgSpriteController#search` and `SvgSpriteController#icon_picker_search` are login required route,
there is no need for us to switch database connections. The fix here is
to simply remove the use of `RailsMultisite::ConnectionManagement.with_hostname`.
- Update welcome topic copy
- Edit the welcome topic automatically when the title or description changes
- Remove “Create your Welcome Topic” banner/CTA
- Add "edit welcome topic" user tip
### What is the problem?
It is possible to pass an arbitrary value to the limit parameter in `TagsController#search`, and have it flow through `DiscourseTagging.filter_allowed_tags` where it will raise an error deep in the database driver. MiniSql ensures there's no injection happening, but that ultimately results in an invalid query.
### How does this fix it?
This change checks more strictly that the parameter can be cleanly converted to an integer by replacing the loose `#to_i` conversion semantics with the stronger `Kernel#Integer` ones.
**Example:**
```ruby
"1; SELECT 1".to_i
#=> 1
Integer("1; SELECT 1")
#=> ArgumentError
```
As part of the change, I also went ahead to disallow a limit of "0", as that doesn't seem to be a useful option. Previously only negative limits were disallowed.
This fixes the api documentation for the create invite api endpoint so
that it uses the `group_ids` param when specifying the group instead of
the incorrect `group_id` param.
This commit adds support for excluding categories when using the
`category:` filter with the `-` prefix. For example,
`-category:category-slug` will exclude all topics that belong to the
category with slug "category-slug" and all of its sub-categories.
To only exclude a particular category and not all of its sub-categories,
the `-` prefix can be used with the `=` prefix. For example,
`-=category:category-slug` will only exclude topics that belong to the
category with slug "category-slug". Topics in the sub-categories of
"category-slug" will still be included.
* DEV: move sidebar community section to database
Before, community section was hard-coded. In the future, we are planning to allow admins to edit it. Therefore, it has to be moved to database to `custom_sections` table.
Few steps and simplifications has to be made:
- custom section was hidden behind `enable_custom_sidebar_sections` feature flag. It has to be deleted so all forums, see community section;
- migration to add `section_type` column to sidebar section to show it is a special type;
- migration to add `segment` column to sidebar links to determine if link should be displayed in primary section or in more section;
- simplify more section to have one level only (secondary section links are merged);
- ensure that links like `everything` are correctly tracking state;
- make user an anonymous links position consistence. For example, from now on `faq` link for user and anonymous is visible in more tab;
- delete old community-section template.
SearchIndexer is only automatically disabled in `before_all` and `before` blocks which means at the start
of test runs. Enabling the SearchIndexer in one `fab!` block will affect
all other `fab!` blocks which is not ideal as we may be indexing stuff
for search when we don't need to.
`TopicQuery#latest_results` which was being used by
`TopicQuery#list_filter` defaults to ordering by `Topic#bumped_at` in
descending order and that was taking precedent over the order scopes
being applied by `TopicsFilter`.
The value field of ThemeField is only used when viewing a diff in the staff action logs and local theme editing. value is being serialized into the theme index as well, which is not used. It's a huge amount of JSON that we can cut by removing it.
This also breaks up the various theme serializers into separate classes so they autoload properly (or at least restart the server on edit)
We are seeing issues with the composer not being able to close due to the addition of a error message when rescuing from `Draft::OutOfSequence`. This PR will revert to the original solution implemented prior to https://github.com/discourse/discourse/pull/21148 that just silently rescues from `Draft::OutOfSequence`
This PR adds the ability to destroy reviewables for a passed user via the API. This was not possible before as this action was reserved for reviewables for you created only.
If a user is an admin and calls the `#destroy` action from the API they are able to destroy a reviewable for a passed user. A user can be targeted by passed either their:
- username
- external_id (for SSO)
to the request.
In the case you attempt to destroy a non-personal reviewable and
- You are not an admin
- You do not access the `#destroy` action via the API
you will raise a `Discourse::InvalidAccess` (403) and will not succeed in destroying the reviewable.
This PR adds the ability to destroy drafts for a passed user via the API. This was not possible before as this action was reserved for only your personal drafts.
If a user is an admin and calls the `#destroy` action from the API they are able to destroy a draft for a passed user. A user can be targeted by passed either their:
- username
- external_id (for SSO)
to the request.
In the case you attempt to destroy a non-personal draft and
- You are not an admin
- You do not access the `#destroy` action via the API
you will raise a `Discourse::InvalidAccess` (403) and will not succeed in destroying the draft.
* FEATURE: add a setting to allowlist DiscourseConnect return path domains
This commit adds a site setting to allowlist DiscourseConnect return
path domains. The setting needs supports exact domain or wildcard
character (*) to allow for any domain as return path.
* Add more specs to clarify what is allowed in site setting
* Update setting description to explain what is allowed
This feature will allow sites to define which emoji are not allowed. Emoji in this list should be excluded from the set we show in the core emoji picker used in the composer for posts when emoji are enabled. And they should not be allowed to be chosen to be added to messages or as reactions in chat.
This feature prevents denied emoji from appearing in the following scenarios:
- topic title and page title
- private messages (topic title and body)
- inserting emojis into a chat
- reacting to chat messages
- using the emoji picker (composer, user status etc)
- using search within emoji picker
It also takes into account the various ways that emojis can be accessed, such as:
- emoji autocomplete suggestions
- emoji favourites (auto populates when adding to emoji deny list for example)
- emoji inline translations
- emoji skintones (ie. for certain hand gestures)
This commit adds support for the `created-by:<username>` query filter
which will return topics created by the specified user. Multiple
usernames can be specified by comma seperating the usernames like so:
`created-by:username1,username2`. This will filter for topics created by
either of the specified users. Multiple `created-by:<username>` can also
be composed together. `created-by:username1 created-by:username2` is
equivalent to `created-by:username1,username2`.
This commit adds support for the `in:<topic notification level>` query
filter. As an example, `in:tracking` will filter for topics that the
user is watching. Filtering for multiple topic notification levels can
be done by comma separating the topic notification level keys. For
example, `in:muted,tracking` or `in:muted,tracking,watching`.
Alternatively, the user can also compose multiple filters with `in:muted
in:tracking` which translates to the same behaviour as
`in:muted,tracking`.
This commit adds some more detailed information about how to actually
get the number of likes for a post.
Also refactors some requests and responses into json schema files to
clean up the specs a bit.
See https://meta.discourse.org/t/69017/4?u=blake
topic.url is a dangerous method to use, it is incompatible with our `sign_in`
helper.
This removes a bunch of places this was erroneously used and helps to avoid
future cargo culting.
The following are the changes being introduced in this commit:
1. Instead of mapping the query language to various query params on the
client side, we've decided that the benefits of having a more robust
query language far outweighs the benefits of having a more human readable query params in the URL.
As such, the `/filter` route will just accept a single `q` query param
and the query string will be parsed on the server side.
1. On the `/filter` route, the tags filtering query language is now
supported in the input per the example provided below:
```
tags:bug+feature tagged both bug and feature
tags:bug,feature tagged either bug or feature
-tags:bug+feature excluding topics tagged bug and feature
-tags:bug,feature excluding topics tagged bug or feature
```
The `tags` filter can also be specified multiple
times in the query string like so `tags:bug tags:feature` which will
filter topics that contain both the `bug` tag and `feature` tag. More
complex query like `tags:bug+feature -tags:experimental` will also work.
Our schema allows `category.topic_id` to be NULL. Null values shouldn't actually happen in production, but it is very common in tests because `Fabricate(:category)` skips creating the definition topic to improve performance. Before this commit, a NULL category.topic_id would cause all subcategory topics to be excluded from a TopicQuery result. This is because, in postgres, `NULL <> anything` is falsy. Instead, we can use `IS DISTINCT FROM`, which will return true when NULL is compared to a non-NULL value.
Why is this change required?
Prior to this change, we would list all group messages that a user
has access to in the user menu messages notifications panel dropdown.
However, this did not respect the topic's notification level setting and
group messages which the user has set to 'normal' notification level were
being displayed
What does this commit do?
With this commit, we no longer display all group messages that a user
has access to. Instead, we only display group messages that a user is
watching in the user menu messages notifications panel dropdown.
Internal Ref: /t/94392
Currently the auto-bump cooldown is hard-coded to 24 hours.
This change makes the highlighted 24 hours part configurable (defaulting to 24 hours), and the rest of the process remains the same.
This uses the new CategorySetting model associated with Category. We decided to add this because we want to move away from custom fields due to the lack of type casting and validations, but we want to keep the loading of these optional as they are not needed for almost all of the flows.
Category settings will be back-filled to all categories as part of this change, and creating a new category will now also create a category setting.
That column is obsolete since we added the `granted_title_badge_id` column in 2019 (56d3e29a69). Having both columns can lead to inconsistencies (mostly due to old data from before 2019).
For example, `BadgeGranter.revoke_ungranted_titles!` doesn't work correctly if `badge_granted_title` is `false` while `granted_title_badge_id` points to the badge that is used as title.
Way back when this was introduced way back in b96c10a903
I didn't have any frame of reference for what these max rate
limit numbers should be, so 10 seemed like a reasonable limit
until a real world case where this did not make sense came
along.
The time has come.
Moving these into site settings, which are hidden since in most
cases there is no need to change these.
Similar spirit to e195e6f614,
this moves the Bookmarkable registration to DiscoursePluginRegistry
so plugins which are not enabled do not register additional
bookmarkable classes.
* UX: add type tag and design update
* UX: clarify status copy in reviewQ
* DEV: switch to selectKit
* UX: color approve/reject buttons in RQ
* DEV: regroup actions
* UX: add type tag and design update
* UX: clarify status copy in reviewQ
* Join questions for flagged post with "or" with new I18n function
* Move ReviewableScores component out of context
* Add CSS classes to reviewable-item based on human type
* UX: add table header for scoring
* UX: don't display % score
* UX: prefix modifier class with dash
* UX: reviewQ flag table styling
* UX: consistent use of ignore icon
* DEV: only show context question on pending status
* UX: only show table headers on pending status
* DEV: reviewQ regroup actions for hidden posts
* UX: reviewQ > approve/reject buttons
* UX: reviewQ add fadeout
* UX: reviewQ styling
* DEV: move scores back into component
* UX: reviewQ mobile styling
* UX: score table on mobile
* UX: reviewQ > move meta info outside table
* UX: reviewQ > score layout fixes
* DEV: readd `agree_and_keep` and fix the spec tests.
* Fix the spec tests
* fix the quint test
* DEV: readd deleting replies
* UX: reviewQ copy tweaks
* DEV: readd test for ignore + delete replies
* Remove old
* FIX: Add perform_ignore back in for backwards compat
* DEV: add an action alias `ignore` for `ignore_and_do_nothing`.
---------
Co-authored-by: Martin Brennan <martin@discourse.org>
Co-authored-by: Vinoth Kannan <svkn.87@gmail.com>
This commit e62e93f83a slowed
down the spec, making it take ~4m in CI and a long time
locally.
This commit fixes the issue by just not doing the work in
the before block, it was not necessary, all we really need
to do is make sure everyone can PM.
54.37s -> 22.98s locally
This commit implements many changes to topic and comments embedding. It
deprecates the class_name field from EmbeddableHost and suggests using
the className parameter. discourse_username parameter has been
deprecated and it will fetch it from embedded site from the author or
discourse-username meta.
See the updated code sample from Admin > Customize > Embedding page.
* FEATURE: Add className parameter for Discourse embed
* DEV: Hide class_name from EmbeddableHost
* DEV: Deprecate class_name field of EmbeddableHost
* FEATURE: Use either author or discourse-username meta tag
* DEV: Deprecate discourse_username parameter
* DEV: Improve embed code sample
* FIX: Use pluralized string
* REFACTOR: Fix misuse of pluralized string
* REFACTOR: Fix misuse of pluralized string
* DEV: Remove linting of `one` key in MessageFormat string, it doesn't work
* REFACTOR: Fix misuse of pluralized string
This also ensures that the URL works on subfolder and shows the site setting link only for admins instead of staff. The string is quite complicated, so the best option was to switch to MessageFormat.
* REFACTOR: Fix misuse of pluralized string
* FIX: Use pluralized string
This also ensures that the URL works on subfolder and shows the site setting link only for admins instead of staff.
* REFACTOR: Correctly pluralize reaction tooltips in chat
This also ensures that maximum 5 usernames are shown and fixes the number of "others" which was off by 1 if the current user reacted on a message.
* REFACTOR: Use translatable string as comma separator
* DEV: Add comment to translation to clarify the meaning of `%{identifier}`
* REFACTOR: Use translatable comma separator and use explicit interpolation keys
* REFACTOR: Don't interpolate lowercase channel status
* REFACTOR: Fix misuse of pluralized string
* REFACTOR: Don't interpolate channel status
* REFACTOR: Use %{count} interpolation key
* REFACTOR: Fix misuse of pluralized string
* REFACTOR: Correctly pluralize DM chat channel titles
Currently, clicking on the unsubscribe link with a `key` associated with a
deleted topic results in an HTTP 500 response.
This change fixes that by skipping any attempt to run topic related flow if topic
isn't present.
Previously, a user avatar redirect had a lifetime of 24h. That means that a change to the S3 CDN URL would take up to 24h to propagate to clients and intermediate CDNs.
This commit reduces the max age to 1 hour, but also introduces a `stale-while-revalidate` directive. This allows clients and CDNs to use a 'stale' value if it was received between 1h and 24h ago, as long as they make a background request to update the cache. This should reduce the impact of S3 URL changes. 1 hour after the change, the CDN will start serving updated values. Plus, if users have cached bad responses, their browser will automatically fetch the correct version and use it on the next page load.
When a post is created using the API and goes into the review queue, we
would return a 'null' string in the response which isn't valid JSON.
Internal ref: /t/92419
Co-authored-by: Leonardo Mosquera <ldmosquera@gmail.com>
Improvements for this PR: https://github.com/discourse/discourse/pull/20057
What was fixed:
- [x] Use ember transitions instead of full reload
- [x] Link was inaccurately kept active
- [x] "+ save" renamed to just "save"
- [x] Render emojis in link name
- [x] UI to set icon
- [x] Delete link is trash icon instead of "x"
- [x] Add another link to on the left and rewording
- [x] Raname "link name" -> "name", "points to" -> link
- [x] Add limits to fields
- [x] Move add section button to the bottom
* FIX: Remove action buttons if post has already been reviewed
* Change the approve to reject test to expect an error
* Adds a controller spec to ensure you can't edit a non-pending review item
* Remove unnessary conditional
This commits adds the ability to add a header to the embedded comments
view. One use case for this is to allow `postMessage` communication
between the comments iframe and the parent frame, for example, when
toggling the theme of the parent webpage.
The algorithm failed to find the correct category by slug when there are multiple sub-sub-categories with the same child-category name and the first child doesn't have the correct grandchild.
So, searching for "child / grandchild" worked in the following case, it found (3):
- (1) parent 1
- (2) child
- (3) grandchild
- (4) parent 2
- (5) child
- (6) grandchild
But it failed to find the grandchild in the following case:
- (1) parent 1
- (2) child
- (4) parent 2
- (5) child
- (6) grandchild
And this also fixes a flaky spec by forcing categories to always order by by `parent_category_id` and `id`.
This makes it possible to partly revert 60990aab55
Allows users to configure their own custom sidebar sections with links withing Discourse instance. Links can be passed as relative path, for example "/tags" or full URL.
Only path is saved in DB, so when Discourse domain is changed, links will be still valid.
Feature is hidden behind SiteSetting.enable_custom_sidebar_sections. This hidden setting determines the group which members have access to this new feature.
1. What is the problem here?
When a user's reviewables count changes, the changes are published via
MessageBus in a background Sidekiq job which means there is a delay before the
client receives the MessageBus message with the updated count. During
the time the reviewables count for a user has been updated and the time
when the client receives the MessageBus message with the updated count,
a user may view the reviewables list in the user menu. When that happens, the number of
reviewables in the list may be out of sync with the count shown.
2. What is the fix?
Going forward, the response for the `ReviewablesController#user_menu_list` action will include the user's reviewables count as
the `reviewables_count` attribute. This is then used by the client side
to update the user's reviewables count to ensure that the reviewables
list and count are kept in sync.
When we introduce new color scheme colors, they are not immediately persisted to the database for all color schemes. Previously, this meant that they would be unavailable in the admin UI for editing. The only way to work with the new colors was to create a new color scheme.
This commit updates the serializer so that all colors are serialized, even if they are not yet persisted to the database for the current scheme. This means that they now show up in the admin UI and can be edited.
Currently `Topic#pm_topic_count` is a count of all personal messages tagged for a given tag. As a result, any user with access to PM tags can poll a sensitive tag to determine if a new personal message has been created using that tag even if the user does not have access to the personal message. We classify this as a minor leak in sensitive information.
With this commit, `Topic#pm_topic_count` is hidden from users by default unless the `display_personal_messages_tag_counts` site setting is enabled.
In some situations, these HTTP calls would cause some cache to warmup and send a `/distributed_hash` message-bus message. We can avoid tracking those by passing a specific channel name to `track_publish`.
This commits adds a database migration to limit the user status to 100
characters, limits the user status in the UI and makes sure that the
emoji is valid.
Follow up to commit b6f75e231c.
When the `tags_listed_by_group` site setting is disable, we were seeing
the N+1 queries problem when multiple `CategoryTag` records are listed.
This commit fixes that by ensuring that we are not filtering through the
category `tags` association after the association has been eager loaded.
This assertion was failing in internal builds. I can repro locally if I
set `foobarbaz` to be created after `quxbarbaz`.
For now, I think this complication in the test is unnecessary, hence this
removes the `quxbarbaz` case.
When creating a group membership request, there is no character
limit on the 'reason' field. This can be potentially be used by
an attacker to create enormous amount of data in the database.
Co-authored-by: Ted Johansson <ted@discourse.org>
This is just cleaning up a TODO I had to add more specs
to this controller -- there are more thorough tests on the
actual HashtagService class and the type-specific hashtag
classes.
This commit allows us to set the channel slug when creating new chat
channels. As well as this, it introduces a new `SlugsController` which can
generate a slug using `Slug.for` and a name string for input. We call this
after the user finishes typing the channel name (debounced) and fill in
the autogenerated slug in the background, and update the slug input
placeholder.
This autogenerated slug is used by default, but if the user writes anything
else in the input it will be used instead.
Currently, `Tag#topic_count` is a count of all regular topics regardless of whether the topic is in a read restricted category or not. As a result, any users can technically poll a sensitive tag to determine if a new topic is created in a category which the user has not excess to. We classify this as a minor leak in sensitive information.
The following changes are introduced in this commit:
1. Introduce `Tag#public_topic_count` which only count topics which have been tagged with a given tag in public categories.
2. Rename `Tag#topic_count` to `Tag#staff_topic_count` which counts the same way as `Tag#topic_count`. In other words, it counts all topics tagged with a given tag regardless of the category the topic is in. The rename is also done so that we indicate that this column contains sensitive information.
3. Change all previous spots which relied on `Topic#topic_count` to rely on `Tag.topic_column_count(guardian)` which will return the right "topic count" column to use based on the current scope.
4. Introduce `SiteSetting.include_secure_categories_in_tag_counts` site setting to allow site administrators to always display the tag topics count using `Tag#staff_topic_count` instead.
Prior to this change, we were parsing `Post#cooked` every time we
serialize a post to extract the usernames of mentioned users in the
post. However, the only reason we have to do this is to support
displaying a user's status beside each mention in a post on the client side when
the `enable_user_status` site setting is enabled. When
`enable_user_status` is disabled, we should avoid having to parse
`Post#cooked` since there is no point in doing so.
Our JS files reference sourcemaps relative to their current path. On sites with non-S3 CDN setups, we use a special path for brotli assets (39a524aa). This caused the sourcemap requests to 404.
This commit fixes the issue by allowing the `.map` files to be accessed under `/brotli_asset/*`.
When the `tags_listed_by_group` site setting is enabled, we were seeing
the N+1 queries problem when multiple `TagGroup` records are listed.
This commit fixes that by ensuring that we are not filtering through the
`tags` association after the association has been eager loaded.
Added in c2013865d7,
this migration was supposed to only turn off the hashtag
setting for existing sites (since that was the old default)
but its doing it for new ones too because we run all migrations
on new sites.
Instead, we should only run this if the first migration was
only just created, meaning its a new site.
In "GlobalSetting.redirect_avatar_requests" mode, when the application gets
an avatar request it returns a "redirect" to the S3 CDN.
This shields the application from caching avatars and downloading from S3.
However clients will make 2 requests per avatar. (one to get redirect,
second to get avatar)
A one hour cache on a redirect means there may be an increase in CDN
traffic, given more clients will ask for the redirect every hour.
This may also lead to an increase in origin requests to the application.
To mitigate lets cache the CDN URL for 1 day.
The downside is that any changes to S3 CDN need extra care to allow for
the extra 1 day delay. (leave data around for 1 extra day)
If a secure upload's access_control_post was trashed, and an anon user
tried to look at that upload, they would get a 500 error rather than
the correct 403 because of an error inside the PostGuardian logic.
We show live user status on mentions starting from a76d864. But status didn’t appear on the post that appears on the bottom of the topic just after a user posted it (status appeared only after page reloading). This adds status to just posted posts.
A few specs in `dashboard_controller_spec.rb` set some state in redis but don't clean it up afterwards which causes other specs to fail when they're ran after `dashboard_controller_spec.rb`.
Related commit: 18467d4.
Featured topics are eventually serialized by `ListableTopicSerializer`
which calls `Topic#image_url` which requires us to preload
`Topic#topic_thumbnails`.
At the time of writing, this is how the `TopicPosterSerializer` looks
like:
```
class TopicPosterSerializer < ApplicationSerializer
attributes :extras, :description
has_one :user, serializer: PosterSerializer
has_one :primary_group, serializer: PrimaryGroupSerializer
has_one :flair_group, serializer: FlairGroupSerializer
end
```
Within `PosterSerializer`, the `primary_group` and `flair_group`
association is requested on the `user` object. However, the associations
have not been loaded on the `user` object at this point leading to the
N+1 queries problem. One may wonder
why the associations have not been loaded when the `TopicPosterSerializer`
has `has_one :primary_group` and `has_one :flair_group`. It turns out that `TopicPoster`
is just a struct containing the `user`, `primary_group` and
`flair_group` objects. The `primary_group` and `flair_group`
ActiveRecord objects are loaded seperately in `UserLookup` and not preloaded when querying for
the users. This is done for performance reason so that we are able to
load the `primary_group` and `flair_group` records in a single query
without duplication.
It was redefining rather than including them. It was causing this warning:
```
WARNING: Shared example group suspension of active user possible was defined without a block and will have no effect. Please define a block or remove the definition
```
* UX: Wizard Step Enhancements
- Remove illustrations
- Add Emoji graphic to top of steps
- Add description below step title
- Move point of contact to last step
* Move step count to header, plus some button navigation tweaks
* add remaining emoji to step headers
* fix button logic on steps
* Update Point of Contact
* remove automated messages field
* adjust styling for counter, title, and emoji
* Update wording for logos
* Fix tests
* fix prettier
* fix specs
* set same with for steps except for styling screen
* use sentence case; remove duplicate copy under your organization fields
* fix missing buttons on small screens
* add spacing to buttons; adjust font weight to labels
* adjust styling for community logo step; use sentence case for button
* update copy for point of contact text helper
* use sentence case for field labels
* fix ui tests
* use btn-back class to fix ui tests
* reduce bottom margin for toggle fields
* clean up
Co-authored-by: Ella <ella.estigoy@gmail.com>
* DEV: Remove enable_whispers site setting
Whispers are enabled as long as there is at least one group allowed to
whisper, see whispers_allowed_groups site setting.
* DEV: Always enable whispers for admins if at least one group is allowed.
We were changing the user's user_option.bookmark_auto_delete_preference
to whatever they changed it to in the bookmark modal to use as default
for future bookmarks. However this was leading to a lot of confusion
since if you wanted to set it for one bookmark you had to remember to
change it back on the next one.
This commit removes that automatic functionality, and instead moves
the bookmark auto delete preference to User Preferences > Interface
in an explicit dropdown.
This commit adds a new notification that gets sent to admins when the site gets new features after an upgrade/deploy. Clicking on the notification takes the admin to the admin dashboard at `/admin` where they can see the new features under the "New Features" section.
Internal topic: t/87166.
FEATURE: Chat and Sidebar are now on by default
- Set the sidebar site setting to be enabled by default
- Set the chat site setting to be enabled by default
- Updated existing specs that assumed the original default
- Use a migration to keep old defaults for existing sites
When user is watching category or tag (watching or watching first post) notifications are moved to other tab.
To achieve that and distinguish between post create to directly watched topics and indirectly watched topics, new notification type called `watching_category_or_tag` was introduced.
* FEATURE: Show similar users when penalizing a user
Moderators will be notified if other users with the same IP address
exist before penalizing a user.
* FEATURE: Allow staff to penalize multiple users
This allows staff members to suspend or silence multiple users belonging
to the same person.
This new site setting replaces the
`enable_experimental_sidebar_hamburger` and `enable_sidebar` site
settings as the sidebar feature exits the experimental phase.
Note that we're replacing this without depreciation since the previous
site setting was considered experimental.
Internal Ref: /t/86563
The default behavior for Rails is to vary the response of an endpoint based on the `Accept:` header, and therefore it returns a `Vary:` header on responses. This instructs browsers and intermediate proxies to key their caches based on the value of the request's `Accept` header. In some cases (e.g. Akamai), the presence of a `Vary` header is enough to prevent caching entirely.
This commit restructures the Rails route definitions so that:
1. The "format" segment of the route is 'required'
2. The "format" segment of the route is constrained to a single value (e.g. `js` or `css`)
Now that the routes are guaranteed to have a `:format` segment, Rails will always prioritize that over the `Accept` header, and will therefore omit the `Vary` header.
Request specs are also added to test this behaviour for both stylesheets and theme-javascripts.
Note that we don't have a database table and a model for post mentions yet, and I decided to implement it without adding one to avoid heavy data migrations. Still, we may want to add such a model later, that would be convenient, we have such a model for mentions in chat.
Note that status appears on all mentions on all posts in a topic except of the case when you just posted a new post, and it appeared on the bottom of the topic. On such posts, status won't be shown immediately for now (you'll need to reload the page to see the status). I'll take care of it in one of the following PRs.
* FEATURE: Show warning if group cannot be mentioned
A similar warning is displayed when the user cannot be mentioned because
they have not been invited to the topic.
* FEATURE: Resolve mentions for new topic
This commit improves several improvements and refactors
/u/is_local_username route to a better /composer/mentions route that
can handle new topics too.
* FEATURE: Show warning if only some are notified
Sometimes users are still notified even if the group that was mentioned
was not invited to the message. This happens because its members were
invited directly or are members of other groups that were invited.
* DEV: Refactor _warnCannotSeeMention
When uploads are stored on S3, by default Discourse will fetch the avatars and proxy them through to the requesting client. This is simple, but it can lead to significant inbound/outbound network load in the hosting environment.
This commit adds an optional redirect_avatar_requests GlobalSetting. When enabled, requests for user avatars will be redirected to the S3 asset instead of being proxied. This adds an extra round-trip for clients, but it should significantly reduce server load. To mitigate that extra round-trip for clients, a CDN with 'follow redirect' capability could be used.
Users who can access the review queue can claim a pending reviewable(s) which means that the claimed reviewable(s) can only be handled by the user who claimed it. Currently, we show claimed reviewables in the user menu, but this can be annoying for other reviewers because they can't do anything about a reviewable claimed by someone. So this PR makes sure that we only show in the user menu reviewables that are claimed by nobody or claimed by the current user.
Internal topic: t/77235.
Follow up to 40e8912395
In this previous commit I introduced a bug that prevented
a legitimate case for an existing user to redeem an invite,
where the email/domain were both blank and the invite was
still redeemable by the user. Fixes the issue and adds more
specs for that case.
I'm hesitant to call this a performance improvement since claiming a
reviewable is probably rare. However, this commit cuts out two DB
queries each time we have to publish a reviewable claimed message. More
importantly, publishing to groups scales much better than publishing to
users because we esstentially cap the number ids we have to load into
memory.
This adds API scope for the user status. This also adds a get method to the user status controller. We didn't need a dedicated method that returns status before because the server returns status with user objects, but I think we need to provide this method for API clients.
This commit fleshes out and adds functionality for the new `#hashtag` search and
lookup system, still hidden behind the `enable_experimental_hashtag_autocomplete`
feature flag.
**Serverside**
We have two plugin API registration methods that are used to define data sources
(`register_hashtag_data_source`) and hashtag result type priorities depending on
the context (`register_hashtag_type_in_context`). Reading the comments in plugin.rb
should make it clear what these are doing. Reading the `HashtagAutocompleteService`
in full will likely help a lot as well.
Each data source is responsible for providing its own **lookup** and **search**
method that returns hashtag results based on the arguments provided. For example,
the category hashtag data source has to take into account parent categories and
how they relate, and each data source has to define their own icon to use for the
hashtag, and so on.
The `Site` serializer has two new attributes that source data from `HashtagAutocompleteService`.
There is `hashtag_icons` that is just a simple array of all the different icons that
can be used for allowlisting in our markdown pipeline, and there is `hashtag_context_configurations`
that is used to store the type priority orders for each registered context.
When sending emails, we cannot render the SVG icons for hashtags, so
we need to change the HTML hashtags to the normal `#hashtag` text.
**Markdown**
The `hashtag-autocomplete.js` file is where I have added the new `hashtag-autocomplete`
markdown rule, and like all of our rules this is used to cook the raw text on both the clientside
and on the serverside using MiniRacer. Only on the server side do we actually reach out to
the database with the `hashtagLookup` function, on the clientside we just render a plainer
version of the hashtag HTML. Only in the composer preview do we do further lookups based
on this.
This rule is the first one (that I can find) that uses the `currentUser` based on a passed
in `user_id` for guardian checks in markdown rendering code. This is the `last_editor_id`
for both the post and chat message. In some cases we need to cook without a user present,
so the `Discourse.system_user` is used in this case.
**Chat Channels**
This also contains the changes required for chat so that chat channels can be used
as a data source for hashtag searches and lookups. This data source will only be
used when `enable_experimental_hashtag_autocomplete` is `true`, so we don't have
to worry about channel results suddenly turning up.
------
**Known Rough Edges**
- Onebox excerpts will not render the icon svg/use tags, I plan to address that in a follow up PR
- Selecting a hashtag + pressing the Quote button will result in weird behaviour, I plan to address that in a follow up PR
- Mixed hashtag contexts for hashtags without a type suffix will not work correctly, e.g. #ux which is both a category and a channel slug will resolve to a category when used inside a post or within a [chat] transcript in that post. Users can get around this manually by adding the correct suffix, for example ::channel. We may get to this at some point in future
- Icons will not show for the hashtags in emails since SVG support is so terrible in email (this is not likely to be resolved, but still noting for posterity)
- Additional refinements and review fixes wil
* FEATURE: API to update user's discourse connect external id
This adds a special handling of updates to DiscourseConnect external_id
in the general user update API endpoint.
Admins can create, update or delete a user SingleSignOn record using
PUT /u/:username.json
{
"external_ids": {
"discourse_connect": "new-external-id"
}
}
When opening the invite acceptance page when the user
was already logged in, we were still showing the Accept
Invitation prompt even if the user had already redeemed
the invitation and was present in the `InvitedUser` table.
This would lead to errors when the user clicked on the button.
This commit fixes the issue by hiding the Accept Invitation
button and showing an error message instead indicating that
the user had already redeemed the invitation. This only applies
to multi-use invite links.
The problem was reported as a problem with changing theme in user preferences, after saving a new theme the previously set user status was disappearing (https://meta.discourse.org/t/user-status/240335/42). Turned out though that the problem was more wide, changing pretty much any setting in user preferences apart from user status itself led to clearing the status.
This commit adds some protections in InviteRedeemer to ensure that email
can never be nil, which could cause issues with inviting the invited
person to private topics since there was an incorrect inner join.
If the email is nil and the invite is scoped to an email, we just use
that invite.email unconditionally. If a redeeming_user (an existing
user) is passed in when redeeming an email, we use their email to
override the passed in email. Otherwise we just use the passed in
email. We now raise an error after all this if the email is still nil.
This commit also adds some tests to catch the private topic fix, and
some general improvements and comments around the invite code.
This commit also includes a migration to delete TopicAllowedUser records
for users who were mistakenly added to topics as part of the invite
redemption process.
Currently, moderators are able to set primary group for users
irrespective of the of the `moderators_manage_categories_and_groups` site
setting value.
This change updates Guardian implementation to honour it.
Depends on: #18806
We have a banner that prompts to edit the welcome topic, so let's not
show it in the topic list until it has been edited. Previously this
banner covered the welcome topic, now the banner will be above the topic
list, so we need to hide the welcome topic.
* DEV: Add a dedicated Admin::StaffController base controller
The current parent(Admin:AdminController) for all admin-related controllers
uses a filter that allows only staff(admin, moderator) users.
This refactor makes Admin::AdminController filter for only admins as the name suggests and
introduces a base controller dedicated for staff-related endpoints.
* DEV: Set staff-only controllers parent to Admin::StaffController
Refactor staff-only controllers to inherit newly introduced
Admin::StaffController abstract controller. This conveys the
purpose of the parent controller better unlike the previously used parent
controller.
* FIX: Follow up fixes for password-reset error page
Pass in `base_url` to the template
Use `.html_safe` since the message now contains html
Follow up to: 9b1536fb83
* Update specs to pass in the base_url
Before this commit, we did not have guardian checks in place to determine if a
topic's title associated with a user badge should be displayed or not.
This means that the topic title of topics with restricted access
could be leaked to anon and users without access if certain conditions
are met. While we will not specify the conditions required, we have internally
assessed that the odds of meeting such conditions are low.
With this commit, we will now apply a guardian check to ensure that the
current user is able to see a topic before the topic's title is included
in the serialized object of a `UserBadge`.
The previous sidebar default tags and categories implementation did not
allow for a user to configure their sidebar to have no categories or
tags. This commit changes how the defaults are applied. When a user is being created,
we create the SidebarSectionLink records based on the `default_sidebar_categories` and
`default_sidebar_tags` site settings. SidebarSectionLink records are
only created for categories and tags which the user has visibility on at
the point of user creation.
With this change, we're also adding the ability for admins to apply
changes to the `default_sidebar_categories` and `default_sidebar_tags`
site settings historically when changing their site setting. When a new
category/tag has been added to the default, the new category/tag will be
added to the sidebar for all users if the admin elects to apply the changes historically.
Like wise when a tag/category is removed, the tag/category will be
removed from the sidebar for all users if the admin elects to apply the
changes historically.
Internal Ref: /t/73500
The `active` param on the create user endpoint requires that an api key
is used in the request header otherwise it is ignored, so adding this
distinction to the api docs.
* DEV: Use list controller and action
It used an empty action handler which just returned the app and it
required another request to get the topic list. By using the correct
controller and action we can preload the topic list.
Our theme system injects a magical `settings` object at the top of themes JS modules to allow theme authors to access the settings as configured by admins in the UI. Within this `settings` object, there are a couple of special objects `theme_uploads` and `theme_uploads_local` that contain URLs for all the assets/uploads that the theme has.
For test modules/files, the theme system also injects a `settings` object at the top of tests modules, but it's not the same object as the object that's injected in non-test files. The difference is that in tests we want the settings to have their default values as opposed to any custom values that may exist in the site's database. This ensures that test results are consistent no matter the site that runs them.
However, the `settings` object in tests files currently doesn't have the special objects `theme_uploads` and `theme_uploads_local` which means that if a theme includes an asset that's lazy-loaded, it's not possible to write tests for anything that depends on the lazy-loaded asset because the theme will not be able to load the asset during the tests since `theme_uploads_local` and `theme_uploads` don't exist. This PR adds these special objects inside the `settings` object for test files.
Internal topic: t/71825/52.
This commit excludes posts from hidden topics from the latest posts and user activity RSS feeds. Additionally, it also excludes small actions from the first one.
Theme javascript is now minified using Terser, just like our core/plugin JS bundles. This reduces the amount of data sent over the network.
This commit also introduces sourcemaps for theme JS. Browser developer tools will now be able show each source file separately when browsing, and also in backtraces.
For theme test JS, the sourcemap is inlined for simplicity. Network load is not a concern for tests.
The previous implementation would attempt to fetch groups using the end-user's Google auth token. This only worked for admin accounts, or users with 'delegated' access to the `admin.directory.group.readonly` API.
This commit changes the approach to use a single 'service account' for fetching the groups. This removes the need to add permissions to all regular user accounts. I'll be updating the [meta docs](https://meta.discourse.org/t/226850) with instructions on setting up the service account.
This is technically a breaking change in behavior, but the existing implementation was marked experimental, and is currently unusable in production google workspace environments.
* Revert "Revert "FEATURE: Preload resources via link header (#18475)" (#18511)"
This reverts commit 95a57f7e0c.
* put behind feature flag
* env -> global setting
* declare global setting
* forgot one spot
Unless we have specified `override = true` in the DeprecatedSettings
class for an old -> new settings map, we should not allow people
to change the old setting in the UI and have it affect the new
setting.
* FEATURE: Hide Privacy Policy and TOS topics
As a way to simplify new sites this change will hide the privacy policy
and the TOS topics from the topic list. They can still be accessed and
edited though.
* add tests
This can no longer be used from the user interface and could be used to
generate useless topic invites notifications. This commit adds site
setting max_topic_invitations_per_minute to prevent invite spam.
Experiment moving from preload tags in the document head to preload information the the response headers.
While this is a minor improvement in most browsers (headers are parsed before the response body), this allows smart proxies like Cloudflare to "learn" from those headers and build HTTP 103 Early Hints for subsequent requests to the same URI, which will allow the user agent to download and parse our JS/CSS while we are waiting for the server to generate and stream the HTML response.
Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
Adds a new upload field for a second dark mode category logo.
This alternative will be used when the browser is in dark mode (similar to the global site setting for a dark logo).
This commit makes pending reviewables show up in the main tab (a.k.a. "all notifications" tab). Pending reviewables along with unread notifications are always shown first and they're sorted based on their creation date (most recent comes first).
The dismiss button currently only shows up if there are unread notifications and it doesn't dismiss pending reviewables. We may follow up with another change soon that allows makes the dismiss button work with reviewables and remove them from the list without taking any action on them.
Follow-up to 079450c9e4.
cf. e62e93f83a
This PR also makes it so `bot` (negative ID) and `system` users are always allowed
to send PMs, since the old conditional was just based on `enable_personal_messages`
This commit adds non-archived group messages and `group_message_summary` notifications in the messages tab in the user menu. With this change, the messages tab in the user menu now includes 3 types of items:
1. Unread `private_message` notifications (notifications when you receive a reply in a PM)
2. Unread and read `group_message_summary` notifications (notifications when there's a new message in a group inbox that you track)
3. Non-archived personal and group messages
Unread `private_message` notifications are always shown first, followed by unread `group_message_summary` notifications, and then everything else (messages and read `group_message_summary` notifications) sorted by recency (most recent first).
Internal topic: t/72976.
Currently, the reviewables tab in the user menu shows pending reviewables at the top of the menu and fills the remaining space in the menu with old/handled reviewables. This PR makes the revieables tab show only pending reviewables and hides the tab altogether from the menu if there are no pending reviewables. We're going to follow-up with another change soon that will show pending reviewables in the main tab of the user menu.
Internal topic: t/73220.
* SECURITY: moderator shouldn't be able to import a theme via API.
* DEV: apply `AdminConstraint` for all the "themes" routes.
Co-authored-by: Vinoth Kannan <svkn.87@gmail.com>
This commit renames all secure_media related settings to secure_uploads_* along with the associated functionality.
This is being done because "media" does not really cover it, we aren't just doing this for images and videos etc. but for all uploads in the site.
Additionally, in future we want to secure more types of uploads, and enable a kind of "mixed mode" where some uploads are secure and some are not, so keeping media in the name is just confusing.
This also keeps compatibility with the `secure-media-uploads` path, and changes new
secure URLs to be `secure-uploads`.
Deprecated settings:
* secure_media -> secure_uploads
* secure_media_allow_embed_images_in_emails -> secure_uploads_allow_embed_images_in_emails
* secure_media_max_email_embed_image_size_kb -> secure_uploads_max_email_embed_image_size_kb
* FEATURE: add composer warning when user haven't been seen in a long time
When a user creates a PM and adds a recipient that hasn't been seen in a
long time then we'll now show a warning in composer indicating that the
user hasn't been seen in a long time.
This will replace `enable_personal_messages` and
`min_trust_to_send_messages`, this commit introduces
the setting `personal_message_enabled_groups`
and uses it in all places that `enable_personal_messages`
and `min_trust_to_send_messages` currently apply.
A migration is included to set `personal_message_enabled_groups`
based on the following rules:
* If `enable_personal_messages` was false, then set
`personal_message_enabled_groups` to `3`, which is
the staff auto group
* If `min_trust_to_send_messages` is not default (1)
and the above condition is false, then set the
`personal_message_enabled_groups` setting to
the appropriate auto group based on the trust level
* Otherwise just set `personal_message_enabled_groups` to
11 which is the TL1 auto group
After follow-up PRs to plugins using these old settings, we will be
able to drop the old settings from core, in the meantime I've added
DEPRECATED notices to their descriptions and added them
to the deprecated site settings list.
This commit also introduces a `_map` shortcut method definition
for all `group_list` site settings, e.g. `SiteSetting.personal_message_enabled_groups`
also has `SiteSetting.personal_message_enabled_groups_map` available,
which automatically splits the setting by `|` and converts it into
an array of integers.
We previously had a system which would generate a 10x10px preview of images and add their URLs in a data-small-upload attribute. The client would then use that as the background-image of the `<img>` element. This works reasonably well on fast connections, but on slower connections it can take a few seconds for the placeholders to appear. The act of loading the placeholders can also break or delay the loading of the 'real' images.
This commit replaces the placeholder logic with a new approach. Instead of a 10x10px preview, we use imagemagick to calculate the average color of an image and store it in the database. The hex color value then added as a `data-dominant-color` attribute on the `<img>` element, and the client can use this as a `background-color` on the element while the real image is loading. That means no extra HTTP request is required, and so the placeholder color can appear instantly.
Dominant color will be calculated:
1. When a new upload is created
2. During a post rebake, if the dominant color is missing from an upload, it will be calculated and stored
3. Every 15 minutes, 25 old upload records are fetched and their dominant color calculated and stored. (part of the existing PeriodicalUpdates job)
Existing posts will continue to use the old 10x10px placeholder system until they are next rebaked
When tagging is enabled, we were correctly serializing tags by their name. However, when tagging was disabled we were attempting to serialize an entire Tag object which raises an error since ee07f6da7d.
https://meta.discourse.org/t/232885
Right now the experimental user menu sorts notifications the same way that the old menu does: unread high-priority notifications are shown first in reverse-chronological order followed by everything else also in reverse-chronological order. However, since the experimental user menu has dedicated tabs for some notification types and each tab displays a badge with the count of unread notifications in the tab, we feel like it makes sense to change how notifications are sorted in the experimental user menu to this:
1. unread high-priority notifications
2. unread regular notifications
3. all read notifications (both high-priority and regular)
4. within each group, notifications are sorted in reverse-chronological order (i.e. newest is shown first).
This new sorting logic applies to all tabs in the experimental user menu, however it doesn't change anything in the old menu. With this change, if a tab in the experimental user menu shows an unread notification badge for a really old notification, it will be surfaced to the top and prevents confusing scenarios where a user sees an unread notification badge on a tab, but the tab doesn't show the unread notification because it's too old to make it to the list.
Internal topic: t72199.
The generate RSA key and import theme routes worked separate from each
other. The RSA key returned both the public and private key and it was
the frontend which posted the private key back to the server. With this
commit, only the public key is necessary as the server keeps a map of
public and private keys that is used to get the private key back from
a public key.
This lets us use all our normal JS tooling like prettier, esline and babel on the splash screen JS. At runtime the JS file is read and inlined into the HTML. This commit also switches us to use a CSP hash rather than a nonce for the splash screen.
When preloading topic_list data we were giving it a 'preload key' which was loosely based on the parameters of the list. However, it did not include all parameters, and mismatches between client/server-side logic would cause the preloaded data to be ignored.
This commit simplifies things by using a single key for all topic_list preloading. This works on the assumption that "The first topic_list the JS app will load is the one which was preloaded". That assumption also existed to some extent in the old design, so we don't expect any regressions here.
The logic to determine what post excerpt to show for
a topic-level bookmark based on the last unread post
was complex and slow, so we decided to remove it and
always just use the first post excerpt.
This commit also fixes an issue where a couple of
instances of for_topic were missed when doing the
Bookmarkable refactors, so:
1. Clicking the topic bookmark link was not taking
the user to the last unread post
2. When replying to a topic where there was a topic
level bookmark with the auto delete preference
of "on owner reply", we were not removing the
bookmark from the UI correctly.
A test has been added for the former, the latter would
be quite time-consuming to test and not really worth
it considering it's quite an edge case UI bug.
Previously, for every bookmarked topic, all topic_user records were being preloaded. Only the current user's record is actually required.
This commit introduces a new `perform_custom_preload!` API which bookmarkables can use to add custom preloading logic. We use this in topic_bookmarkable to load just the topic_user data we need (in the same way as `topic_list.rb`).
Co-authored-by: Blake Erickson <o.blakeerickson@gmail.com>
Previously, this would require manually adding `?safe_mode=...` multiple times during the email-based login flow. `/u/admin-login` is often used when debugging a site, so it makes sense for this to be easier.
This commit introduces a new checkbox on the `/u/admin-login` screen. When checked, it'll set the safe_mode parameter on the `/email-login` link, and then pass it all the way through to the homepage redirect.
A public key must be added to GitHub when installing private themes.
When the process happens asynchronously (for example if the admin does
not have admin permissions to the GitHub repository), installing
private themes becomes very difficult.
In this case, the Discourse admin can partially install the theme by
letting Discourse save the private key, create a placeholder theme and
give the admin a public key to be used as a deploy key. After the key
is installed, the admin can finish theme installation by pressing a
button on the theme page.
Hard deleting topics that contained soft deleted posts or small actions
used to create orphan posts because only the first post was hard
deleted. This commit adds an error message if there are still posts left
in the topic that must be hard deleted first or hard deletes all small
actions too immediately (there is no other way of hard deleting a small
action because there is no wrench menu).
Some of the changes in this PR are extracted from https://github.com/discourse/discourse/pull/17379.
Similar to the bookmarks tab in the new user menu, the messages tab also displays a mix of notifications and messages. When there are unread message notifications, the tab displays all of these notifications at the top and fills the remaining space in the menu with a list of the user's messages. The bubble/badge count on the messages tab indicates how many unread message notifications there are.
Some of the changes in this commit are extracted from https://github.com/discourse/discourse/pull/17379.
The bookmarks tab in the new user menu is different from the other tabs in that it can display a mixture of notifications and bookmarks. When there are unread bookmark reminder notifications, the tab displays all of these notifications at the top and fills the remaining space in the menu with the rest of the bookmarks. The bubble/badge count on the bookmarks tab indicates how many unread bookmark reminder notifications there are.
On the technical aspect, since this commit introduces a new `bookmark-item` component, we've done some refactoring so that all 3 "item" components (`notification-item`, `reviewable-item` and the new `bookmark-item`) inherit from a base component and get identical HTML structure so they all look consistent.
Internal tickets: t70584 and t65045.
Follow up to 4d3c1ceb44, this commit
shows the SMTP response in the admin email sent list and also moves the
topic/post link into a new column. Reply key is now in its own column.
* FEATURE: Add case-sensitivity flag to watched_words
Currently, all watched words are matched case-insensitively. This flag
allows a watched word to be flagged for case-sensitive matching.
To allow allow for backwards compatibility the flag is set to false by
default.
* FEATURE: Support case-sensitive creation of Watched Words via API
Extend admin creation and upload of Watched Words to support case
sensitive flag. This lays the ground work for supporting
case-insensitive matching of Watched Words.
Support for an extra column has also been introduced for the Watched
Words upload CSV file. The new column structure is as follows:
word,replacement,case_sentive
* FEATURE: Enable case-sensitive matching of Watched Words
WordWatcher's word_matcher_regexp now returns a list of regular
expressions instead of one case-insensitive regular expression.
With the ability to flag a Watched Word as case-sensitive, an action
can have words of both sensitivities.This makes the use of the global
Regexp::IGNORECASE flag added to all words problematic.
To get around platform limitations around the use of subexpression level
switches/flags, a list of regular expressions is returned instead, one for each
case sensitivity.
Word matching has also been updated to use this list of regular expressions
instead of one.
* FEATURE: Use case-sensitive regular expressions for Watched Words
Update Watched Words regular expressions matching and processing to handle
the extra metadata which comes along with the introduction of
case-sensitive Watched Words.
This allows case-sensitive Watched Words to matched as such.
* DEV: Simplify type casting of case-sensitive flag from uploads
Use builtin semantics instead of a custom method for converting
string case flags in uploaded Watched Words to boolean.
* UX: Add case-sensitivity details to Admin Watched Words UI
Update Watched Word form to include a toggle for case-sensitivity.
This also adds support for, case-sensitive testing and matching of Watched Word
in the admin UI.
* DEV: Code improvements from review feedback
- Extract watched word regex creation out to a utility function
- Make JS array presence check more explicit and readable
* DEV: Extract Watched Word regex creation to utility function
Clean-up work from review feedback. Reduce code duplication.
* DEV: Rename word_matcher_regexp to word_matcher_regexp_list
Since a list is returned now instead of a single regular expression,
change `word_matcher_regexp` to `word_matcher_regexp_list` to better communicate
this change.
* DEV: Incorporate WordWatcher updates from upstream
Resolve conflicts and ensure apply_to_text does not remove non-word characters in matches
that aren't at the beginning of the line.
Some errors (e.g. InvalidAccess) are rendered with `include_ember: true`. Booting the ember app requires that the 'preload' data is rendered in the HTML.
If a particular route was configured to `skip_before_action :preload_json`, and then went on to raise an InvalidAccess error, then we'd attempt to render the Ember app without the preload json. This led to a blank screen and a client-side error.
This commit ensures that error pages will fallback to the no_ember view if there is no preload data. It also adds a sanity check in `discourse-bootstrap` so that it's easier for us to identify similar errors in future.
This commit removes the ability to enable/disable the Sidebar on a per
user basis and introduces a site wide setting. For testing purposes, sidebar can be enabled/disabled via the `enable_sidebar=1` or `enable_sidebar=0` query param.
* FEATURE: revamped wizard
* UX: Wizard redesign (#17381)
* UX: Step 1-2
* swap out images
* UX: Finalize all steps
* UX: mobile
* UX: Fix test
* more test
* DEV: remove unneeded wizard components
* DEV: fix wizard tests
* DEV: update rails tests for new wizard
* Remove empty hbs files that were created because of rebase
* Fixes for rebase
* Fix wizard image link
* More rebase fixes
* Fix rails tests
* FIX: Update preview for new color schemes: (#17481)
* UX: make layout more responsive, update images
* fix typo
* DEV: move discourse logo svg to template only component
* DEV: formatting improvements
* Remove unneeded files
* Add tests for privacy step
* Fix banner image height for step "ready"
Co-authored-by: Jordan Vidrine <30537603+jordanvidrine@users.noreply.github.com>
Co-authored-by: awesomerobot <kris.aubuchon@discourse.org>
This commit introduces a new plugin API to register
a group of stats that will be included in about.json
and also conditionally in the site about UI at /about.
The usage is like this:
```ruby
register_about_stat_group("chat_messages", show_in_ui: true) do
{
last_day: 1,
"7_days" => 10,
"30_days" => 100,
count: 1000,
previous_30_days: 120
}
end
```
In reality the stats will be generated any way the implementer
chooses within the plugin. The `last_day`, `7_days`, `30_days,` and `count`
keys must be present but apart from that additional stats may be added.
Only those core 4 stat keys will be shown in the UI, but everything will be shown
in about.json.
The stat group name is used to prefix the stats in about.json like so:
```json
"chat_messages_last_day": 2322,
"chat_messages_7_days": 2322,
"chat_messages_30_days": 2322,
"chat_messages_count": 2322,
```
The `show_in_ui` option (default false) is used to determine whether the
group of stats is shown on the site About page in the Site Statistics
table. Some stats may be needed purely for reporting purposes and thus
do not need to be shown in the UI to admins/users. An extension to the Site
serializer, `displayed_about_plugin_stat_groups`, has been added so this
can be inspected on the client-side.
The site.json endpoint was missing some optional fields on the
categories property that is returned. This commit documents the
`parent_category_id` which is present if there are subcategories and it
also documents the `custom_fields` property which the chat plugin is
using causing some flaky tests.
* FIX: Posts can belong to hard-deleted topics
This was a problem when serializing deleted posts because they might
belong to a topic that was permanently deleted. This caused to DB
lookup to fail immediately and raise an exception. In this case, the
endpoint returned a 404.
* FIX: Remove N+1 queries
Deleted topics were not loaded because of the default scope that
filters out all deleted topics. It executed a query for each deleted
topic.
We previously relied on CSS animation-delay for the splash. This means that we can get inconsistent results based on device/network conditions.
This PR moves us to a more consistent timing based on {request time + 2 seconds}
Internal topic: /t/65378/65
Before, whispers were only available for staff members.
Config has been changed to allow to configure privileged groups with access to whispers. Post migration was added to move from the old setting into the new one.
I considered having a boolean column `whisperer` on user model similar to `admin/moderator` for performance reason. Finally, I decided to keep looking for groups as queries are only done for current user and didn't notice any N+1 queries.
* DEV: Fix flaky admin badges.json api docs spec
This commit is to fix this incredibly vague error message:
```
Failure/Error: expect(valid).to eq(true)
expected: true
got: false
```
From this test:
> Assertion: badges /admin/badges.json get success response behaves like
> a JSON endpoint response body matches the documented response schema
I was finally able to repro locally using parallel tests:
```
RAILS_ENV=test bundle exec ./bin/turbo_rspec
```
I *think* the parallel tests might be swallowing the `puts` output, but
when I also specified the individual spec file
```
RAILS_ENV=test bundle exec ./bin/turbo_rspec spec/requests/api/badges_spec.rb
```
It revealed the issue:
```
VALIDATION DETAILS: {"missing_keys"=>["i18n_name"]}
```
``` ruby
...
def include_i18n_name?
object.system?
end
```
Looks like if the "system" user isn't being used the `i18n_name` won't
be returned in the json response so we shouldn't mark it as a required
attribute.
* Switch to using fab!
When using `let(:badge)` to fabricate a test badge it wouldn't be
returned from the controller, but switching to using `fab!` allows it to
be returned in the json data giving us a non-system badge to test
against.
When calling the API to delete a user:
```
curl -X DELETE "https://discourse.example.com/admin/users/159.json" \
-H "Content-Type: multipart/form-data;" \
-H "Api-Key: ***" \
-H "Api-Username: ***" \
-F "delete_posts=true" \
-F "block_email=false" \
-F "block_urls=false" \
-F "block_ip=false"
```
Setting the parameters `block_email`, `block_urls` and `block_ip`explicitly to `false` did not work because the values weren't being parsed to boolean.
This PR introduces a new hidden site setting that allows admins to display a splash screen while site assets load.
The splash screen can be enabled via the `splash_screen` hidden site setting.
This is what the splash screen currently looks like
5ceb72f085.mp4
Once site assets load, the splash screen is automatically removed.
To control the loading text that shows in the splash screen, you can change the preloader_text translation string in admin > customize > text
In certain situations, a logged in user can redeem an invite with an email that
either doesn't match the invite's email or does not adhere to the email domain
restriction of an invite link. The impact of this flaw is aggrevated
when the invite has been configured to add the user that accepts the
invite into restricted groups.
The category description fields as part of the rswag specs for the
site.json endpoint were flakey. Removing the `required` attribute allows
us to still document that these fields exists, but that depending on
certain site settings they may not be present in the response.
Certain rogue bots such as Yandex may send across invalid CSP reports
when CSP report collection is enabled.
This ensures that invalid reports will not cause log floods and simply
returns a 422 error.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
This reverts commit 94c3bbc2d1.
At this current point in time, we do not have enough data on whether
this centralisation is the trade-offs of coupling features into a single
channel.
Presence endpoints are often called asynchronously at the same time as other request, and never need to modify the session. Skipping ensures that an unneeded cookie rotation doesn't race against another request and cause issues.
This change brings presence in line with message-bus's behaviour.
As part of this commit, a bug where updating a tag's notification level on the server side does not update the state of the user's tag notification levels on the client side is fixed too.
The `WebhookController` inherits directly from `ActionController::Base`. Since Rails 5.2, forgery protection has been enabled by default. When we applied those new defaults in 0403a8633b, it took effect on this controller and broke integrations.
This commit explicitly disables CSRF protection on these webhook routes, and updates the specs so they'll catch this kind of regression in future.
This commit resolves a bug where users are not auto approved based on
`SiteSetting.auto_approve_email_domains` when
`SiteSetting.must_approve_users` has been enabled.
This security fix affects sites which have `SiteSetting.must_approve_users`
enabled. There are intentional and unintentional cases where invited
users can be auto approved and are deemed to have skipped the staff approval process.
Instead of trying to reason about when auto-approval should happen, we have decided that
enabling the `must_approve_users` setting going forward will just mean that all new users
must be explicitly approved by a staff user in the review queue. The only case where users are auto
approved is when the `auto_approve_email_domains` site setting is used.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
The server-side implementation had unintentionally changed to include `-{id}` at the end of the body class name. This change meant that the JS client was unaware of the class, and didn't remove it when navigating away from the category page.
This commit fixes the server-side implementation to match the client
Previously we limited Discourse Connect provider to 1 secret per domain.
This made it pretty awkward to cycle secrets in environments where config
takes time to propagate
This change allows for the same domain to have multiple secrets
Also fixes internal implementation on DiscourseConnectProvider which was
not thread safe as it leaned on class variables to ferry data around
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Co-authored-by: David Taylor <david@taylorhq.com>
Previously we hardcoded the DOWNLOAD_URL_EXPIRES_AFTER_SECONDS const
inside S3Helper to be 5 minutes (300 seconds). For various reasons,
some hosted sites may need this to be longer for other integrations.
The maximum expiry time for presigned URLs is 1 week (which is
604800 seconds), so that has been added as a validation on the
setting as well. The setting is hidden because 99% of the time
it should not be changed.
When searching for PMs or PMs in a group inbox, results in the header search were not being limited to 5 with a "More" link to the full page search. This PR fixes that.
It also simplifies the logic and updates the search API docs to include recently added `in:messages` and `group_messages:groupname` options.
This commit migrates all bookmarks to be polymorphic (using the
bookmarkable_id and bookmarkable_type) columns. It also deletes
all the old code guarded behind the use_polymorphic_bookmarks setting
and changes that setting to true for all sites and by default for
the sake of plugins.
No data is deleted in the migrations, the old post_id and for_topic
columns for bookmarks will be dropped later on.
The title had to be added both on the 404 page generated by the server
side, displayed when the user reaches a bad page directly and the 404
page rendered by Ember when a user reaches a missing topic while
navigating the forum.
The logic in 06893380 only works for `.js` files. It breaks down for `.br.js` and `.gz.js` files. This commit makes things more robust by extracting only the base_url from the service-worker JS, and taking the map filename from the original `sourceMappingURL` comment.
`script_asset_path('.../blah.js.map')` was appending `.js`, which would result in a filename like `.js.map.js`. It would also lose the `/assets` prefix, since the map files are not included in the sprockets manifest.
This commit updates the sourceMappingURL rewriting logic to calculate the service-worker's own JS url, and then append `.map`.
We have a .ics endpoint for user bookmarks, this
commit makes it so polymorphic bookmarks work on
that endpoint, using the serializer associated with
the RegisteredBookmarkable.
Currently the only way to allow tagging on pms is to use the `allow_staff_to_tag_pms` site setting. We are removing that site setting and replacing it with `pm_tags_allowed_for_groups` which will allow for non staff tagging. It will be group based permissions instead of requiring the user to be staff.
If the existing value of `allow_staff_to_tag_pms` is `true` then we include the `staff` groups as a default for `pm_tags_allowed_for_groups`.
This was causing issues on some sites, having the const, because this really is heavily
dependent on upload speed. We request 5-10 URLs at a time with this endpoint; for
a 1.5GB upload with 5mb parts this could mean 60 requests to the server to get all
the part URLs. If the user's upload speed is super fast they may request all 60
batches in a minute, if it is slow they may request 5 batches in a minute.
The other external upload endpoints are not hit as often, so they can stay as constant
values for now. This commit also increases the default to 20 requests/minute.
We have not used anything related to bookmarks for PostAction
or UserAction records since 2020, bookmarks are their own thing
now. Deleting all this is just cleaning up old cruft.
A bit of a mixed bag, this addresses several edge areas of bookmarks and makes them compatible with polymorphic bookmarks (hidden behind the `use_polymorphic_bookmarks` site setting). The main ones are:
* ExportUserArchive compatibility
* SyncTopicUserBookmarked job compatibility
* Sending different notifications for the bookmark reminders based on the bookmarkable type
* Import scripts compatibility
* BookmarkReminderNotificationHandler compatibility
This PR also refactors the `register_bookmarkable` API so it accepts a class descended from a `BaseBookmarkable` class instead. This was done because we kept having to add more and more lambdas/properties inline and it was very messy, so a factory pattern is cleaner. The classes can be tested independently as well.
Some later PRs will address some other areas like the discourse narrative bot, advanced search, reports, and the .ics endpoint for bookmarks.
`TestLogger` was responsible for some flaky specs runs:
```
Error during failsafe response: undefined method `debug' for #<TestLogger:0x0000556c4b942cf0 @warnings=1>
Did you mean? debugger
```
This commit also cleans up other uses of `FakeLogger`
This was due to a server side bug when unicode usernames have been
enabled. We were double encoding the unicode username in the URL
resulting in a invalid URL.
This will make future changes to the 'pull hotlinked images' system easier. This commit should not introduce any functional change.
For now, the old post_custom_field data is kept in the database. This will be dropped in a future commit.
This commit introduces a new site setting: `use_name_for_username_suggestions` (default true)
Admins can disable it if they want to stop using Name values when generating usernames for users. This can be useful if you want to keep real names private-by-default or, when used in conjunction with the `use_email_for_username_and_name_suggestions` setting, you would prefer to use email-based username suggestions.
