The hidden site setting `suppress_secured_categories_from_admin` will
suppress visibility of categories without explicit access from admins
in a few key areas (category drop downs and topic lists)
It is not intended to be a security wall since admins can amend any site
setting. Instead it is feature that allows hiding the categories from the
UI.
Admins will still be able to see topics in categories without explicit
access using direct URLs or flags.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
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.
* FEATURE: Default Composer Category Site Setting
- Create the default_composer_category site setting
- Replace general_category_id logic for auto selecting the composer
category
- Prevent Uncategorized from being selected if not allowed
- Add default_composer_category option to seeded categories
- Create a migration to populate the default_composer_category site
setting if there is a general_category_id populated
- Added some tests
* Add missing translation for the new site setting
* fix some js tests
* Just check that the header value is null
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.
Currently, we have available three 2fa methods:
- Token-Based Authenticators
- Physical Security Keys
- Two-Factor Backup Codes
If the first two are deleted, user lose visibility of their backup codes, which suggests that 2fa is disabled.
However, when they try to authenticate, the account is locked, and they have to ask admin to fix that problem.
This PR is fixing the issue. User still sees backup codes in their panel and can use them to authenticate.
In next PR, I will improve UI to clearly notify the user when 2fa is fully disabled and when it is still active.
* Remove old bookmark column ignores to follow up b22450c7a8
* Change some group site setting checks to use the _map helper
* Remove old secure_media helper stub for chat
* Change attr_accessor to attr_reader for preloaded_custom_fields to follow up 70af45055a
It is likely that a new admin user was created as just a regular user
before being promoted to admin so this change will update the sidebar
link records for any users that are promoted to admin. This way if any
of the default side bar categories or tags are restricted to admins
these new admins will have those added to their sidebar as well.
You can easily replicate this issue locally (prior to this fix) by using
`rails admin:create` where it creates a user first, then it is promoted
to admin. This means it would receive the default categories of regular
user, but never receive the ones they should have access to as an admin.
As part of this change I did drop the `!` from
`SidebarSectionLink.insert_all` so that it would add any new records
that were missing, but not throw a unique constraint error trying to add
any existing records.
Follow up to: 1b56a55f50
And: e320bbe513
Previously the stylesheet cachebusting hash was based on the maximum mtime of files. This works well in development and during in-container updates (e.g. via docker_manager). However, when a fresh docker image is created for each deploy, the file mtimes will change even if the contents has not.
This commit changes the production logic to calculate the cachebuster from the filenames and contents of the relevant assets. This should be consistent across deploys, thereby improving cache hits and improving page load times.
Meta topic: https://meta.discourse.org/t/meta-theme-color-is-not-respecting-current-color-scheme/239815
Currently, the dark mode theme-color `<meta>` tag doesn't apply because the light mode tag has `media="all"`. This means that the dark mode `<meta>` tag with `media="(prefers-color-scheme: dark)"` won't override it. This PR updates the light mode tag to `media="(prefers-color-scheme: light)"` if `dark_scheme_id` is defined and leaves it as `media="all"` otherwise.
Since the system user is a regular user, it can have its
`allow_private_messages` user option turned off, which
with our current `can_send_private_message?(Discourse.system_user)`
check inside the CurrentUserSerializer, will prevent any
user from sending messages in the UI if the system user is not
accepting PMs.
This commit adds a new `can_send_private_messages?` method to
the Guardian, which can be used in serializers and not depend
on the system user. When the user actually sends a message
we still rely on the old `can_send_private_message?(target)`
call to see if they are allowed to send the message to the target.
The new method is just to say they can "generally" send
private messages.
Previously, we didn't have a site-wide setting to set the default behavior for user profile visibility and user presence features. But we already have a user preference for that.
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
This commit fixes a regression introduced in 8979adc where under certain conditions the groups syncing logic in Discourse Connect would try to add users to groups they're already members of and cause errors when users try to sign in using Discourse Connect.
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
Before this commit, there was no way for us to efficiently check an
array of topics for which a user can see. Therefore, this commit
introduces the `TopicGuardian#can_see_topic_ids` method which accepts an
array of `Topic#id`s and filters out the ids which the user is not
allowed to see. The `TopicGuardian#can_see_topic_ids` method is meant to
maintain feature parity with `TopicGuardian#can_see_topic?` at all
times so a consistency check has been added in our tests to ensure that
`TopicGuardian#can_see_topic_ids` returns the same result as
`TopicGuardian#can_see_topic?`. In the near future, the plan is for us
to switch to `TopicGuardian#can_see_topic_ids` completely but I'm not
doing that in this commit as we have to be careful with the performance
impact of such a change.
This method is currently not being used in the current commit but will
be relied on in a subsequent commit.
Meta topic: https://meta.discourse.org/t/meta-theme-color-is-not-respecting-current-color-scheme/239815/7?u=osama.
This commit renders an additional `theme-color` `<meta>` tag for the dark scheme if the current user/request has a scheme selected for dark mode. We currently only render one `theme-color` tag which is always based on the user's selected scheme for light mode, but if the user also selects a scheme for dark mode and uses a device that's configured to use/prefer dark mode, the Discourse UI will be in dark mode, but any parts of the browser/OS UI that's colored based on the `theme-color` tag, would use a color from the user's selected light scheme and look inconsistent with the Discourse UI because the `theme-color` tag is based on the user's selected light scheme.
The additional `theme-color` tag has `media="(prefers-color-scheme: dark)"` and is based on the user's selected dark scheme which means any browser UI that's colored based on `theme-color` tags should be able to pick the right tag based on the user's preference for light/dark mode.
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.
Related to aeee7ed.
Before the change in aeee7ed, notifications for direct replies to your posts and notifications for replies in watched topics looked the same in the notifications menu -- they both used the arrow icon.
We decided in aeee7ed to distinguish them by changing "watched topics" notifications to use the bell icon because it was confusing for users who watch topics to see the same icon for direct replies and "watched topics". However, that change also means that non-power/new users who receive replies to topics _they create_ will get notifications with the bell icon because technically they're watching the topic, but the arrow icon is more appropriate for this case because we use it throughout the app to indicate "replies".
This commit adds a special-case so that if a user is watching a topic AND the topic is created by them, they receive notifications with the arrow icon (type `replied`) instead of the bell icon (type `posted`) for new posts in the topic.
Internal topic: t/79051.
Discourse Connect can be used to manage group memberships of users by including a `add_groups`, `remove_groups` or `groups` attribute in the Discourse Connect payload. However, additions/deletions of users from groups aren't logged to the groups logs (available at `/g/<group>/manage/logs`) which can cause confusions to admins they try to figure out when/how users were added or removed from a group. This commit makes Discourse Connect add entries to the groups logs when it makes changes to users' group memberships.
This test flakes occassionally, possibly because
of the arg ordering which we do not guarantee.
In future if this keeps occurring we may want to
try make expect_enqueued_with not care about argument
orders or the order of arrays etc within those arguments.
Adds sorting for the HashtagAutocompleteService to
sort the results by case-insensitive text _within_
the type sort order specified by the params. This
should fix some flaky specs as well.
Linking a commit from a GitHub pull request included the complete commit
message, instead of just the first line. The rest of the commit message
will be added to the body of the Onebox.
Building does not persist the object in the database which is
unrealistic since we're mostly dealing with persisted objects in
production.
In theory, this will result our test suite taking longer to run since we
now have to write to the database. However, I don't expect the increase
to be significant and it is actually no different than us adding new
tests which fabricates more objects.
Staged users are allowed to view topics they created in a read restricted category
when category has `Category#email_in` and
`Category#email_in_allow_strangers` configured.
* 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.
When PostRevisor is called with 'skip_validations: true' it can save
the post twice and one of the calls passes the correct 'validate: false'
argument, but the other one does not.
The filenames (minus the extensions) were being used as keys in a hash to pass to Terser, which meant that colocated connector files would overwrite each other. This commit moves the un-colocating earlier in the pipeline so that the fixed filenames are passed to Terser.
Followup to be3d6a56ce
This commit adds a new `/hashtag/search` endpoint and both
relevant JS and ruby plugin APIs to handle plugins adding their
own data sources and priority orders for types of things to search
when `#` is pressed.
A `context` param is added to `setupHashtagAutocomplete` which
a corresponding chat PR https://github.com/discourse/discourse-chat/pull/1302
will now use.
The UI calls `registerHashtagSearchParam` for each context that will
require a `#` search (e.g. the topic composer), for each type of record that
the context needs to search for, as well as a priority order for that type. Core
uses this call to add the `category` and `tag` data sources to the topic composer.
The `register_hashtag_data_source` ruby plugin API call is for plugins to
add a new data source for the hashtag searching endpoint, e.g. discourse-chat
may add a `channel` data source.
This functionality is hidden behind the `enable_experimental_hashtag_autocomplete`
flag, except for the change to `setupHashtagAutocomplete` since only core and
discourse-chat are using that function. Note this PR does **not** include required
changes for hashtag lookup or new styling.
Before, `sidebar_list_destination` was an attribute on UserOptionSerializer. The problem was that this attribute was added to user model only when the user entered the preferences panel. We want that attribute to be available all the time, therefore it was moved to CurrentUserSerializer.
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.
Previously, compiling theme 'extra_js' was done with a number of steps. Each theme_field would be compiled into its own value_baked column, and then the JavascriptCache content would be built by concatenating all of those compiled values.
This commit streamlines things by removing the value_baked step. The raw value of all extra_js theme_fields are passed directly to the ThemeJavascriptCompiler, and then the result is stored in the JavascriptCache.
In itself, this commit should not cause any behavior change. It is designed to open the door to more advanced compilation features which have interdependencies between different source files (e.g. template colocation, sourcemaps).
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.
This commit fixes an issue where we had a typo in the
UserAction.stream query which meant that action_code_path
was not loaded correctly. Once that was fixed, we were also
not actually using the action_code_path in the user-stream-item,
so that has been fixed here too.
The bug this caused was that, when the link for the action was
clicked within the user-stream-item, the user would be redirected
to a URL ending with `[missing%20%%7Bpath%7D%20value]` because
the I18n call did not have the path present.
Previously, when the array had both nil and string values it returned the error "comparison of NilClass with String failed". Now I added the `.compact` method to prevent this issue as per @martin-brennan's suggestion https://github.com/discourse/discourse/pull/18431#discussion_r984204788
* FIX: Reset related site settings on general category delete
If the new seeded General category is deleted we also need to delete the
corresponding site setting for it so that we don't try and reference it.
This fixes a bug in the category dropdown composer.
This change creates the `clear_related_site_settings` after destroy
hook that could also be used by other features in the future, like maybe
when we have a `default_category_id` site_setting.
Looks like if `nil` out a site setting it is set to `0`?
```
[9] pry(main)> SiteSetting.general_category_id = nil
SiteSetting Load (0.4ms) SELECT "site_settings".* FROM "site_settings" WHERE "site_settings"."name" = 'general_category_id' LIMIT 1
=> nil
[10] pry(main)> SiteSetting.general_category_id
=> 0
```
That is why the tests check if the value is `< 1` and not `nil`.
* Use -1 instead of nil because it is the default
This commit introduces a new framework for building user tutorials as
popups using the Tippy JS library. Currently, the new framework is used
to replace the old notification spotlight and tips and show a new one
related to the topic timeline.
All popups follow the same structure and have a title, a description and
two buttons for either dismissing just the current tip or all of them
at once.
The state of all seen popups is stored in a user option. Updating
skip_new_user_tips will automatically update the list of seen popups
accordingly.
The previous fix in e83d35d6 was incorrect, and the stub in the test was never actually hit. This commit moves the error handling to the right place and updates the specs to ensure the stub is always used.
* 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).
These errors tend to indicate that the upload is missing on the remote store. This is bad, but we don't want it to block the dominant-color calculation process. This commit catches errors when there is an HTTP error, and fixes the `base_store.rb` implementation when `FileHelper.download` returns nil.
When a user with an email matching those inside the
DISCOURSE_DEVELOPER_EMAILS env var log in, we make
them into admin users if they are not already. This
is used when setting up the first admin user for
self-hosters, since the discourse-setup script sets
the provided admin emails into DISCOURSE_DEVELOPER_EMAILS.
The issue being fixed here is that the new admins were
not being automatically added to the staff and admins
automatic groups, which was causing issues with the site
settings that are group_list based that don't have an explicit
staff override. All we need to do is refresh the automatic
staff, admin groups when admin is granted for the user.
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`
Static topics are the seeded topics that are automatically created for every Discourse instance to hold the content for the FAQ, ToS and Privacy pages. These topics are allowed to bypass the minimum title length checks when they're edited by admins:
ba27ee1637/app/assets/javascripts/discourse/app/models/composer.js (L487-L496)
However, on the server-side, the "quality title" validations aren't skipped for static topics and that can cause confusion for admins when they change the title of a static topic to something that's short enough to fail the quality title validations. This commit ignores all quality title validations on static topics when they're edited by admins.
Internal topic: t/75745.
Skipped invites were not counted at all and some invites could generate
more than one error and resulted in a grand total that was not equal to
the count of bulk invites.
* FEATURE: Make General the default category
* Set general as the default category in the composer model instead
* use semicolon
* Enable allow_uncategorized_topics in create_post spec helper for now
* Check if general_category_id is set
* Enable allow_uncategorized_topics for test env
* Provide an option to the create_post helper to not set allow_uncategorized_topics
* Add tests to check that category… is not present and that General is selected automatically
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.
By default, only staff members have to confirm their old email when
changing it. This commit adds a site setting that when enabled will
always ask the user to confirm old email.
* 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
This commit introduces rails system tests run with chromedriver, selenium,
and headless chrome to our testing toolbox.
We use the `webdrivers` gem and `selenium-webdriver` which is what
the latest Rails uses so the tests run locally and in CI out of the box.
You can use `SELENIUM_VERBOSE_DRIVER_LOGS=1` to show extra
verbose logs of what selenium is doing to communicate with the system
tests.
By default JS logs are verbose so errors from JS are shown when
running system tests, you can disable this with
`SELENIUM_DISABLE_VERBOSE_JS_LOGS=1`
You can use `SELENIUM_HEADLESS=0` to run the system
tests inside a chrome browser instead of headless, which can be useful to debug things
and see what the spec sees. See note above about `bin/ember-cli` to avoid
surprises.
I have modified `bin/turbo_rspec` to exclude `spec/system` by default,
support for parallel system specs is a little shaky right now and we don't
want them slowing down the turbo by default either.
### PageObjects and System Tests
To make querying and inspecting parts of the page easier
and more reusable inbetween system tests, we are using the
concept of [PageObjects](https://www.selenium.dev/documentation/test_practices/encouraged/page_object_models/) in
our system tests. A "Page" here is generally corresponds to
an overarching ember route, e.g. "Topic" for `/t/324345/some-topic`,
and this contains logic for querying components within the topic
such as "Posts".
I have also split "Modals" into their own entity. Further down the
line we may want to explore creating independent "Component"
contexts.
Capybara DSL should be included in each PageObject class,
reference for this can be found at https://rubydoc.info/github/teamcapybara/capybara/master#the-dsl
For system tests, since they are so slow, we want to focus on
the "happy path" and not do every different possible context
and branch check using them. They are meant to be overarching
tests that check a number of things are correct using the full stack
from JS and ember to rails to ruby and then the database.
### CI Setup
Whenever a system spec fails, a screenshot
is taken and a build artifact is produced _after the entire CI run is complete_,
which can be downloaded from the Actions UI in the repo.
Most importantly, a step to build the Ember app using Ember CLI
is needed, otherwise the JS assets cannot be found by capybara:
```
- name: Build Ember CLI
run: bin/ember-cli --build
```
A new `--build` argument has been added to `bin/ember-cli` for this
case, which is not needed locally if you already have the discourse
rails server running via `bin/ember-cli -u` since the whole server is built and
set up by default.
Co-authored-by: David Taylor <david@taylorhq.com>
* 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.
* FIX: Recursively tag topics with missing ancestor tags
Given only a child tag, walk up the ancestry chain, get all of it's
ancestors for use in tagging a topic
* FIX: Ensure only one parent tag is returned for topic tagging
Current implementation selects and return first parent tag if child tag
has multiple parents.
This change updates recursive parent tag implementation to only return
parent tags via only one ancestry line.
* DEV: Add test case for tag cycles
Given we aren't performing a strict graph traversal to get a tag's
parent, cycles do not have any effect on the tags returned for topic
tagging.
By default, we won't include associated account ids in current user serializer. If the new hidden site setting `include_associated_account_ids` is enabled then we will add it in the serializer.
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.
See https://meta.discourse.org/t/discourse-email-messages-are-incorrectly-threaded/233499
for thorough reasoning.
This commit changes how we generate Message-IDs and do email
threading for emails sent from Discourse. The main changes are
as follows:
* Introduce an outbound_message_id column on Post that
is either a) filled with a Discourse-generated Message-ID
the first time that post is used for an outbound email
or b) filled with an original Message-ID from an external
mail client or service if the post was created from an
incoming email.
* Change Discourse-generated Message-IDs to be more consistent
and static, in the format `discourse/post/:post_id@:host`
* Do not send References or In-Reply-To headers for emails sent
for the OP of topics.
* Make sure that In-Reply-To is filled with either a) the OP's
Message-ID if the post is not a direct reply or b) the parent
post's Message-ID
* Make sure that In-Reply-To has all referenced post's Message-IDs
* Make sure that References is filled with a chain of Message-IDs
from the OP down to the parent post of the new post.
We also are keeping X-Discourse-Post-Id and X-Discourse-Topic-Id,
headers that we previously removed, for easier visual debugging
of outbound emails.
Finally, we backfill the `outbound_message_id` for posts that have
a linked `IncomingEmail` record, using the `message_id` of that record.
We do not need to do that for posts that don't have an incoming email
since they are backfilled at runtime if `outbound_message_id` is missing.
The `add_column` `limit` parameter has no effect on a postgres `text` column. Instead we can perform the check in ActiveRecord.
We never expect this condition to be hit - users cannot control this value. It's just a safety net.
This commits introduces a new SiteSetting.enable_new_user_profile_nav_groups
feature flag. When configured, users of the configured groups will see
the new user page navigation links.
As of this commit, only the user activity navigation link has been
converted to the newly proposed dropdown of navigation links.
Mobile support has not been considered.
Adds limits to location and website fields at model and DB level
to match the bio_raw field limits. A limit cannot be added at the
DB level for bio_raw because it is a postgres text field.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
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
Upgrading to Markdown.it v13 broke empty inline BBCodes. This works around the problem by adding an empty token before a closing token if the previous token was a BBCode token.
It also removes the unused `jump` attribute which was removed in Markdown.it v12.3
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
`delete_previous!` deletes existing topics even when we cannot send a new one due to the `limit_once_per` option. The dashboard problems PM gets deleted the next time the job runs (30 minutes), so the inbox could be empty when
admins click on the summary notification.
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.
A user could receive more than a notification for the same post if they
watched both the categories and tags at different levels. This commit
makes sure that only the watching notification is created.
* Add DiscourseEvent before post notifications are created
Previously we were relying on a highly-customized version of the unmaintained Barber gem for theme template compilation. This commit switches us to use our own DiscourseJsProcessor, which makes use of more modern patterns and will be easier to maintain going forward.
In summary:
- Refactors DiscourseJsProcessor to move multiline JS heredocs into a companion `discourse-js-processor.js` file
- Use MiniRacer's `.call` method to avoid manually escaping JS strings
- Move Theme template AST transformers into DiscourseJsProcessor, and formalise interface for extending RawHandlebars AST transformations
- Update Ember template compilation to use a babel-based approach, just like Ember CLI. This gives each template its own ES6 module rather than directly assigning `Ember.TEMPLATES` values
- Improve testing of template compilation (and move some tests from `theme_javascript_compiler_spec.rb` to `discourse_js_processor_spec.rb`
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.
It used to return the next URL anyway which lead to an additional
request. On the frontend, if the result set was empty, it kept retrying
until at least one result was returned. This bug is fixed in this commit
too.
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.
* FIX: hide welcome topic banner as soon as the welcome topic is edited
This commit adds a message bus listener on client to hide the welcome
topic banner as soon as the welcome topic is edited.
* update test
* only subscribe when show_welcome_topic_banner is true
* Do not lookup for messageBus service if it's not required
* Remove unneeded code
* Cache result for Site.show_welcome_topic_banner
* Update tests per latest changes
* Changes per PR review
Each new user menu notifications should have their own count. Therefore, we need to include all types to serializer and not only `grouped_unread_high_priority_notifications`
Additional PR will be created for chat and assign plugin, as they will have to switch to `grouped_unread_notifications` as well.
* FIX: Only seed general category on new sites
If the site already has human users (users with an id > 0) don't seed
the categories.
Follow up to: a6ad74c759
* use human_users scope
We don't want to save the auto_delete_preference for bookmarks to the
user options if it was passed through as nil from the frontend,
this leads to confusion for the end user since they did not explicitly set it.
It's fine to create the bookmark with the default of "never" if no
auto_delete_preference is provided since it applies only to the
single bookmark, not future bookmarks.
- Seed the General category so that the general chat channel will have
a home
- Do not seed the Lounge category anymore
- Move the "Welcome to Site" topic to the General category
This commit makes a number of improvements to the DiscourseJsProcessor:
1. Remove dependence on the out-of-date Ember template compiler from the ember-rails gem; switch to modern template compiler
2. Refactor to make use of a proper module system with `define`/`require`
3. Introduce `babel-plugin-ember-template-compilation` to enable inline hbs compilation
The `mini-loader` is upgraded to support relative lookup and `require.has`, so that these new JS packages work correctly.
If a topic is converted to a private message, all posters were invited
to the new private message. This included users who only whispered or
posted small actions.
Topic allowed user records were created for small actions, which lead to
the system user being invited in many private topics when the user
removed themselves or if a group was invited but some members already
had access.
This commits skips creating topic allowed user. They are already skipped
for the whisper posts.
If a user was granted a trust level, joined a group that granted a trust
level and left the group, the trust level was reset. This commit tries
to restore the last known trust level before joining the group by
looking into staff logs.
This commit also migrates old :change_trust_level user history records
to use previous_value and new_value fields.
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.
In a multisite Discourse reported that no backup is running after 60 seconds because the Redis key expired. Also, the thread that listens for a shutdown signal stopped running immediately because it didn't detect a running operation.
Default sidebar tags for not authenticated users can be defined in admin panel. Otherwise, top 5 categories and tags are taken.
Optionally, if categories are set up in permanent order, then the first 5 categories are taken.
Certain HTML can be rejected by nokogumbo, specifically cases where there
are enormous amounts of attributes
This ensures that malformed HTML is simply skipped instead of leaking out
an exception and terminating downstream processes.
The logic was added in commit ec8306835d,
to show the like action even if the user could not like the post. It is
not necessary for this logic to be implemented on the server side.
* FIX: Do not allow to remove like if topic is archived
* FIX: Always show like button
The like button used to be hidden if the topic was archived and it had
no likes. This commit changes that to always show the like button, but
with a not-allowed cursor if the topic is archived.
`TopicQueryParams` allows for `match_all_tags` to be passed as a query parameter. `TagsController` forces the value to be true.
This change allows a value to be passed, and only sets it to true if no value has been set. It then uses `ActiveModel::Type::Boolean.new.cast` to compare the value.
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.
The maximum_staged_users_per_email site setting controls how many
staged users will be invited to the topic created from an incoming
email. Previously, it counted only the new staged users.
Twitter removed OpenGraph tags from their pages. We can no longer
extract all the information (for example, the quoted tweet) we need
to render Oneboxes without using their API.
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>
Handles edge-case when a user is an admin and has an associated reviewable. Hitting this exception should be rare since we clear the reviewable when
granting staff to the user.
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.
Previously, PM only tags were being routed to the public topic list with
the tag added as a filter. However, the public topic list does not fetch
PMs and hence PM only tags did not provide any value when added to the
Sidebar. This commit changes that by allowing the client to
differentiate PM only tag and thus routes the link to the PM tags show
route.
Counts for PM only tags section links are not supported as of this
commit and will be added in a follow up commit.
Dead and large images are replaced with a placeholder, either a broken
chain icon or a short text. This commit no longer applies this
transformation for images inside Oneboxes, but removes them instead.
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.
* FEATURE: update bootstrap mode notice to add invite and wizard links
* Updates per feedback on PR
* Fix the wizard link not showing
* Remove unneeded function
* Remove router service injection
This is a much better description of its function. It performs idempotent normalization of a URL. If consumers truly need to `encode` a URL (including double-encoding of existing encoded entities), they can use the existing `.encode` method.
normalized_encode in addressable has a number of issues, including https://github.com/sporkmonger/addressable/issues/472
To temporaily work around those issues for the majority of cases, we try parsing with `::URI`. If that fails (e.g. due to non-ascii characters) then we will fall back to addressable.
Hopefully we can simplify this back to `Addressable::URI.normalized_encode` in the future.
This commit also adds support for unicode domain names and emoji domain names with escape_uri.
This removes an unneeded hack checking for pre-signed urls, which are now handled by the general case due to starting off valid and only being minimally normalized. Previous test case continues to pass.
UrlHelper.s3_presigned_url? which was somewhat wide was removed.
The new plugin list is based on the ones currently used in our ember-cli pipeline, and are based on our official browser support policy.
This commit includes an update to the raw-handlebars compiler to remove the 'very hacky but lets us use ES6' code. It's served us well for the last 6 years, but the babel config changes broke it (`const` -> `let`). This commit takes the opportunity to refactor it to take a similar approach to PrettyText, by leaning on `mini-loader.js`.
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.
Currently we can’t add a case-sensitive watched word if another one
exists with a different case. For example, the existing watched word
`Meta` has been created and is case-sensitive. Now an admin tries to add
`metA` while marking it as case-sensitive too, this won’t work and the
word won’t be added.
This patch changes this behavior by allowing to add same words that have
different cases, so the example above will now work as expected.
We still check for uniqueness but case-sensitivy is now taken
into account. It means that if the watched word `meta` already exists
and is not case-sensitive then it will not be possible to add `Meta`
(case-sensitive or not) as `meta` already matches every possible
variations of this word.
Our internal implementation of #perform on jobs performs remapping.
This happens cause we do "exception aggregation".
Scheduled jobs run on every site in the multisite cluster, and we report
one error per site that failed. During this aggregation we reshape the
context from the original object shape returned by mini_scheduler
The new integration test ensures this interface will remain stable even if
decoupled parts of the code change shapes.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Followup to d66115d918
* Makes sure the `actor_preferences` all initialize with an empty array instead of nil if there are no preferences e.g. the actor is not ignoring anyone
* If the actor has disabled all PMs make `actor_disallowing_pms?` always return true
* FIX: don't memoize site setting in guardian
Memoizing site settings can make tests more fragile and harder to debug
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
This commit introduces several fine-grained methods
to UserCommScreener which can be used to show the actor
who they are ignoring/muting/blocking DMs from in order
to prevent them initiating conversation with those users
or to display relevant information in the UI to the
actor.
This will be used in a companion PR in discourse-chat,
and is a follow up to 74584ff3ca
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Co-authored-by: Osama Sayegh <asooomaasoooma90@gmail.com>
Follow-up to ce9eec8606.
I did a last-minute refactoring before merging the commit above where I extracted the Message Bus publish call into a new method, but forgot to delete the publish call after adding a call to the new method.
* FEATURE: track stats around failing scheduled jobs
Discourse.job_exception_stats can now be used to gather stats around how
many regular scheduled jobs failed in the current process.
This will be consumed by the Prometheus plugin and potentially other
monitoring plugins.
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.
The content from the remote site and the footer get cached for 10 minutes, so Discourse should use the default locale instead of the user locale for the footer. Otherwise Discourse might cache the message in a different language.
* 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.
* FIX: bots could generate errors when slug generation method is encoded
When slug generation method is encoded (non default) then bots could
cause errors in the logs for urls containing special chars.
ó for example in a URL can be requested in a valid ASCII-8BIT string, and
later when joined to UTF-8 would result in encoding issues.
Fix here ensures we force encoding correctly for outlier cases.
Browser tend to always encode these chars, hence we did not notice this.
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
Fixes edge case from fa5f3e228c.
In case the acting user is sent in with the target_user_ids,
we do not need to load those preferences, because even if the
acting user is preventing PMs or muting etc they need to always be able to
send themselves messages.
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.
The "everyone" group is an automatic group and GroupUser records do not
exist for it. This commit allows all users if the group everyone is one
of the groups in the setting "pm_tags_allowed_for_groups".
* FIX: Rejected emails should not be cleaned up before their logs
If we delete the rejected emails before we delete their associated logs
we will receive 404 errors trying to inspect an email message for that
log.
* don't add a blank line
* test for max value as well
* pr cleanup and add migration
* Fix failing test
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>
The previous method for reused the PrettyText logic which applied the
watched word logic, but had the unwanted effect of cooking the text too.
This meant that regular text values were converted to HTML.
Follow up to commit 5a4c35f627.
Currently when generating oneboxes if the connection timeouts and we’re
using the `FinalDestination#get` method, then it raises an exception.
We already catch this exception when using the
`FinalDestination#resolve` method so this patch just applies the same
logic to `FinalDestination#get`.
All other tests that are setting grade_period use either unitless `0`, `1.minute` or `5.minutes` so it wasn't clear if `5` was meant to be seconds (it was)
Our theme system is very complex and it can take a while to figure out how to invalidate the various types of caches that are used throughout the theme system. So, having a single helper method that invalidates everything can be useful in emergency situations where there is no time to read through the code and figure out how to clear the various caches.
Internal ticket: t64732.
When a topic was published from a shared draft and it had tags, the
users watching the tags were not notified. The problem was that the
topics are usually created in a secret category and publishing it just
moves an existent topic to the target category, without making any
changes to the tags.
The invites should be redeemed during the signup process. This was a
problem because when user tried to redeem an admin invite it tried to
authenticate the user using information from the session that was not
available.
This is so we can join the Notification table onto the
Bookmark table. A slight refactor was needed to ensure
that the required values are always included and the
consumer does not need to think about this.
The discourse-chat and discourse-data-explorer plugins
will be updated to take advantage of this commit.
Tag edit notifications are either created by PostActionNotifier or
PostRevisor. PostActionNotifier already checks if the site setting is
enabled. but PostRevisor scheduled a NotifyTagChange job without
checking disable_tags_edit_notifications.
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.
* FIX: properly validate multiselect user fields on user creation
* Add test cases
* FIX: don't check multiselect user fields for watched words
* Clarifiy/simplify tests
* Roll back apply_watched_words changes
Since this method no longer needs to deal with arrays for now. If/when
we add new user fields which uses them, we can deal with it then.
The `unread_not_too_old` attribute is a little odd because there should never be a case where
the user's first_unread_at column is less than the `Topic#updated_at`
column of an unread topic. The `unread_not_too_old` attribute is causing
a bug where topic states synced into `TopicTrackingState` do not appear
as unread because the attribute does not exsist on a normal `Topic`
object and hence never set.
It makes more sense to use user_ids for the UserCommScreener
introduced in fa5f3e228c since
in most cases the ID will be available, not the username. This
was discovered while starting work on a plugin that will
use this. In the cases where only usernames are available
the extra query is negligble.
The idea behind this refactor is to centralise all of the user ignoring / muting / disallow PM checks in a single place, so they can be used consistently in core as well as for plugins like chat, while improving the main bulk of the checks to run in a single fast non-AR query.
Also fixed up the invite error when someone is muting/ignoring the user that is trying to invite them to the topic.
Currently we only apply watched words of the `Block` type to custom user
fields and user profile fields.
This patch enables all rules to be applied such as `Censor` or
`Replace`.
Previously the spec was hardcoded to expect a certain compiled result. The precise result will change as we update to more recent Ember versions.
Instead, we can compile via the special theme compiler, and then compile our expected result via the normal compiler. If they match, then things are working as intended. This technique should be safe across template-compiler upgrades.
* FIX: min/max username length limits weren't validated
The custom validators introduced in e0d7cda made so we ignored the mix
and max values set on site_settings.yml. That change allowed admins to
set values outside of the range defined on the yaml file.
Related to https://meta.discourse.org/t/group-names-with-more-than-60-characters-broken/232115?u=falco
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
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.
Mutating the `raw` variable like this would cause issues upstream, meaning that the modification is not persisted. Instead, we should allocate a new string like the other replacement methods.
* 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.
If an image is oneboxed directly, then we should replace the onebox URL with a markdown image tag. This ensures that the wrapper link points to the downloaded version rather than the original.
This regressed in bf6f8299
Logging out failed when the current user was cached by an instance of `Auth::DefaultCurrentUserProvider` and `#log_off_user` was called on a different instance of that class.
Co-authored-by: Sam <sam.saffron@gmail.com>
This happened when a middleware accessed the `currentUser` before a controller had a chance to populate the `action_dispatch.request.path_parameters` env variable. In that case Discourse would always cache `nil` as `currentUser`.
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.
Updates automatically data on the stats section of the topic.
It will update automatically the following information: likes, replies and last reply (timestamp and user)
* 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.
We have a `cleanup!` class method on bookmarks that deletes
bookmarks X days after their related record (post/topic) are
deleted. This commit changes this method to use the
registered_bookmarkables for this instead, and each bookmarkable
type can delete related bookmarks in their own way.
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
Future versions of redis will validate this port number causing the tests
relying on this to fail with:
```
Redis::CommandError:
ERR Invalid master port
```
Also change from an IPv4 address that might feasibly be in use to an IPv6
random ULA address that almost *certainly* won't be.
It's already included in the `ignored_columns` list in the group model. 03ffb0bf27/app/models/group.rb (L9)
Also, removed the `MigrateGroupFlairImages` onceoff job and spec.
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>
At some point in the past we decided to rename the 'regular' notification state of topics/categories to 'normal'. However, some UI copy was missed when the initial renaming was done so this commit changes the spots that were missed to the new name.
This is pre-request work to introduce a splash screen while site assets load.
The only change this commit introduces is that it ensures we add the defer attribute to core/plugin/theme .JS files. This will allow us to insert markup before the browser starts evaluating those scripts later on. It has no visual or functional impact on core.
This will not have any impact on how themes and plugins work. The only exception is themes loading external scripts in the </head> theme field directly via script tags. Everything will work the same but those would need to add the defer attribute if they want to keep the benefits introduced in this PR.
```
WARNING: Using `expect { }.not_to raise_error(SpecificErrorClass)` risks false positives, since literally any other error would cause the expectation to pass, including those raised by Ruby (e.g. `NoMethodError`, `NameError` and `ArgumentError`), meaning the code you are intending to test may not even get reached. Instead consider using `expect { }.not_to raise_error` or `expect { }.to raise_error(DifferentSpecificErrorClass)`. This message can be suppressed by setting: `RSpec::Expectations.configuration.on_potential_false_positives = :nothing`. Called from /var/www/discourse/spec/lib/retrieve_title_spec.rb:155:in `block (3 levels) in <main>'.
```
This doesn't cope well with gzipped, binary, or large responses. Ideally we would teach FinalDestination to safely retrieve and decode some of the response body. But for now, let's remove the broken implementation.
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.
When sending emails with delivery_method_options -> return_response
set to true, the SMTP sending code inside Mail will return the SMTP
response when calling deliver! for mail within the app. This commit
ensures that Email::Sender captures this response if it is returned
and stores it against the EmailLog created for the sent email.
A follow up PR will make this visible within the admin email UI.
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.
We do not zero-pad our base62 short URLs, so there is no guarantee that the length is 27. Instead, let's greedily match all consecutive base62 characters and look for a matching upload.
This reverts bd32656157 and 36f5d5eada.
* FIX: Fix a bug that is accessing the values in a hash wrongly and write tests
I decided to write tests in order to be confident in my refactor that's in the next commit.
Meanwhile I have discovered a potential bug. The `title_attr` key was accessed as a string,
but all the keys are actually symbols so it was never evaluated to be true.
irb(main):025:0> d = {key: 'value'}
=> {:key=>"value"}
irb(main):026:0> d['key']
=> nil
irb(main):027:0> d[:key]
=> "value"
* DEV: Extract methods for readability
I will be adding a new method following the conventions in place for adding a new normalizer. And this will make the readability of the `raw` block even more difficult; so I am extracting self contained private methods beforehand.
* FEATURE: Parse JSON-LD and introduce Movie object
JSON LD data is very easily transferable to Ruby objects because they contain types. If these types are mapped to Ruby objects, it is also better to make all the parsed data very explicit and easily extendable.
JSON-LD has many more standardized item types, with a full list here: https://schema.org/docs/full.html
However in order to decrease the scope, I only adapted the movie type.
* DEV: Change inheritance between normalizers
Normalizers are not supposed to have an inheritance relationships amongst each other. They are all normalizers, but all normalizing separate protocols. This is why I chose to extract a parent class and relieve Open Graph off that responsibility. Removing the parent class altogether could also a possibility, but I am keeping the scope limited to having a more accurate representation of the normalizers while making it easier to add a new one.
* Lint changes
* Bring back the Oembed OpenGraph inheritance
There is one test that caught that this inheritance was necessary. I still think modelling wise this inheritance shouldn't exist, but this can be tackled separately.
* Return empty hash if the json received is invalid
Before this change if there was a parsing error with JSON it would throw an exception. The goal of this commit is to rescue that exception and then log a warning. I chose to use Discourse's logger wrapper `warn_exception` to have the backtrace and not just used Rails logger. I considered raising an `InvalidParameters` error however if the JSON here is invalid it should not block showing of the Onebox, so logging is enough.
* Prep to support more JSONLD schema types with case
* Extract mustache template object created from JSONLD
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 PR changes the rescue block to rescue only Net::TimeoutError exceptions and removes the log line to prevent clutter the logs with errors that are ignored. Other errors can bubble up because they're errors we probably want to know about
This table holds associations between uploads and other models. This can be used to prevent removing uploads that are still in use.
* DEV: Create upload_references
* DEV: Use UploadReference instead of PostUpload
* DEV: Use UploadReference for SiteSetting
* DEV: Use UploadReference for Badge
* DEV: Use UploadReference for Category
* DEV: Use UploadReference for CustomEmoji
* DEV: Use UploadReference for Group
* DEV: Use UploadReference for ThemeField
* DEV: Use UploadReference for ThemeSetting
* DEV: Use UploadReference for User
* DEV: Use UploadReference for UserAvatar
* DEV: Use UploadReference for UserExport
* DEV: Use UploadReference for UserProfile
* DEV: Add method to extract uploads from raw text
* DEV: Use UploadReference for Draft
* DEV: Use UploadReference for ReviewableQueuedPost
* DEV: Use UploadReference for UserProfile's bio_raw
* DEV: Do not copy user uploads to upload references
* DEV: Copy post uploads again after deploy
* DEV: Use created_at and updated_at from uploads table
* FIX: Check if upload site setting is empty
* DEV: Copy user uploads to upload references
* DEV: Make upload extraction less strict
Following the Rails 7 upgrade, the `DISCOURSE_SMTP_ENABLE_START_TLS`
setting doesn’t work anymore. This is because Rails upgraded the
`net-smtp` gem to the 0.3.1 version which enables `starttls` by default.
The `mail` gem doesn’t support this new behavior yet and doesn’t know
how to disable TLS. This should be fixed in an upcoming release.
Meanwhile applying this patch allows us to get back the previous
behavior which is expected by many.
This commit introduces a new site setting: `block_hotlinked_media`. When enabled, all attempts to hotlink media (images, videos, and audio) will fail, and be replaced with a linked placeholder. Exceptions to the rule can be added via `block_hotlinked_media_exceptions`.
`download_remote_image_to_local` can be used alongside this feature. In that case, hotlinked images will be blocked immediately when the post is created, but will then be replaced with the downloaded version a few seconds later.
This implementation is purely server-side, and does not impact the composer preview.
Technically, there are two stages to this feature:
1. `PrettyText.sanitize_hotlinked_media` is called during `PrettyText.cook`, and whenever new images are introduced by Onebox. It will iterate over all src/srcset attributes in the post HTML and check if they're allowed. If not, the attributes will be removed and replaced with a `data-blocked-hotlinked-src(set)` attribute
2. In the `CookedPostProcessor`, we iterate over all `data-blocked-hotlinked-src(set)` attributes and check whether we have a downloaded version of the media. If yes, we update the src to use the downloaded version. If not, the entire media element is replaced with a placeholder. The placeholder is labelled 'external media', and is a link to the offsite media.
* FIX: Email Send post has already been taken error
Adding a failing test first before coming up with a good solution.
Related: 357011eb3b
The above commit changed
```
PostReplyKey.find_or_create_by_safe!
```
to
```
PostReplyKey.create_or_find_by!
```
But I don't think it is working as a 1-1 replacement because of the
`Validation failed: Post has already been taken` error we are receiving
with this change. Also we need to make sure we don't re-introduce any
concurrency issues.
Reported: https://meta.discourse.org/t/224706/13
* Remove rails unique constraint and rely on db index
I believe this is what is causing `create_or_find_by!` to fail. Because
we have a unique constraint in the db I think we can remove this rails
unique constraint?
* clean up spec wording
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.
When a site has `SiteSetting.invite_only` enabled, we create a
`ReviewableUser`record when activating a user if the user is not
approved. Therefore, we need to approve the user when redeeming an
invite.
There are some uncertainties surrounding why a `ReviewableRecord` is
created for a user in an invites only site but this commit does not seek
to address that.
Follow-up to 7c4e2d33fa
Twitter does not allow SVGs to be used for twitter:image
metadata (see https://developer.twitter.com/en/docs/twitter-for-websites/cards/overview/markup)
so we should fall back to the site logo if the image option
provided to `crawlable_meta_data` or SiteSetting.site_twitter_summary_large_image_url
is an SVG, and do not add the meta tag for twitter:image at all
if the site logo is an SVG.
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
Due to some changes we started notifying via push notifications on other
families of notifications. There are a total of about 30 or so possible
notification you could get, some can be pushed.
This fallback means that if for any reason we are unable to find an icon
for a push notification we just fallback to the Discourse logo.
Also go with a simple reply icon for watching first post.
Note, that in production `image_url` can return an exception if an image is
missing. This is not the case in test / development.
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>
In 7328a2bfb0 we changed the
InlineOneboxer#onebox_for method to run the title of the
onebox through WatchedWord#censor_text. However, it is
allowable for the title to be nil, which was causing this
error in production:
> NoMethodError : undefined method gsub for nil:NilClass
We just need to check whether the title is nil before trying
to censor it.
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.
Censored watched words were not censored inside the title of an inline
oneboxes. Malicious users could exploit this behaviour to insert bad
words. The same issue has been fixed for regular Oneboxes in commit
d184fe59ca.
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.
When saving / creating bookmarks, we have code to save
the user's preference of bookmark_auto_delete_preference
to their user_options.
Unfortunately this can cause weirdness when plugins
have code using BookmarkManager to set the auto delete preference for
only a specific bookmark.
This commit introduces a save_user_preferences option (false
by default) so that this user preference is not saved unless
specified by the consumer of BookmarkManager, so plugins will
not have to worry about it.
Previously, with the default `editing_grace_period`, hotlinked images were pulled 5 minutes after a post is created. This delay was added to reduce the chance of automated edits clashing with user edits.
This commit refactors things so that we can pull hotlinked images immediately. URLs are immediately updated in the post's `cooked` HTML. The post's raw markdown is updated later, after the `editing_grace_period`.
This involves a number of behind-the-scenes changes including:
- Schedule Jobs::PullHotlinkedImages immediately after Jobs::ProcessPost. Move scheduling to after the `update_column` call to avoid race conditions
- Move raw changes into a separate job, which is delayed until after the ninja-edit window
- Move disable_if_low_on_disk_space logic into the `pull_hotlinked_images` job
- Move raw-parsing/replacing logic into `InlineUpload` so it can be easily be shared between `UpdateHotlinkedRaw` and `PullUserProfileHotlinkedImages`
Meta topic: https://meta.discourse.org/t/prevent-to-linkify-when-there-is-a-redirect/226964/2?u=osama.
This commit adds a new site setting `block_onebox_on_redirect` (default off) for blocking oneboxes (full and inline) of URLs that redirect. Note that an initial http → https redirect is still allowed if the redirect location is identical to the source (minus the scheme of course). For example, if a user includes a link to `http://example.com/page` and the link resolves to `https://example.com/page`, then the link will onebox (assuming it can be oneboxed) even if the setting is enabled. The reason for this is a user may type out a URL (i.e. the URL is short and memorizable) with http and since a lot of sites support TLS with http traffic automatically redirected to https, so we should still allow the URL to onebox.
Incorporates learnings from /t/64227:
* Changes the code to set access control posts in the rake
task to be an efficient UPDATE SQL query.
The original version was timing out with 312017 post uploads,
the new query took ~3s to run.
* Changes the code to mark uploads as secure/not secure in
the rake task to be an efficient UPDATE SQL query rather than
using UploadSecurity. This took a very long time previously,
and now takes only a few seconds.
* Spread out ACL syncing for uploads into jobs with batches of
100 uploads at a time, so they can be parallelized instead
of having to wait ~1.25 seconds for each ACL to be changed
in S3 serially.
One issue that still remains is post rebaking. Doing this serially
is painfully slow. We have a way to do this in sidekiq via PeriodicalUpdates
but this is limited by max_old_rebakes_per_15_minutes. It would
be better to fan this rebaking out into jobs like we did for the
ACL sync, but that should be done in another PR.
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.
Previously we were only applying the restriction to `a[href]` and `img[src]`. This commit ensures we apply the same logic to all allowlisted media src attributes.
This makes it easier to find PMs involving a particular user, for
example by searching for `in:messages thisUser` (previously, that query
would only return results in posts where `thisUser` was in the post body).
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.
- Only validate if custom_fields are loaded, so that we don't trigger a db query
- Only validate public user fields, not all custom_fields
This commit also reverts the unrelated spec changes in ba148e08, which were required to work around these issues
The cache was causing state to leak between tests since the `WatchedWord` record in the DB would have been rolled back but `WordWatcher` still had the word in the cache.
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.
7a284164 previously switched the UserDestroyer to use find_each when iterating over UserHistory records. Unfortunately, since this logic is wrapped in a transaction, this didn't actually solve the memory usage problem. ActiveRecord maintains references to all modified models within a transaction.
This commit updates the logic to use a single SQL query, rather than updating models one-by-one
22a7905f restructured how we load Ember CLI assets in production. Unfortunately, it also broke sourcemaps for those assets. This commit fixes that regression via a couple of changes:
- It adds the necessary `.map` paths to `config.assets.precompile`
- It swaps Sprockets' default `SourcemappingUrlProcessor` with an extended version which maintains relative URLs of maps
`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`.
When we build and send emails using MessageBuilder and Email::Sender
we add custom headers defined in SiteSetting.email_custom_headers.
However this was causing errors in cases where the custom headers
defined a header that we already specify in outbound emails (e.g.
the Precedence: list header for topic/post emails).
This commit makes it so we always use the header value defined in Discourse
core if there is a duplicate, discarding the custom header value
from the site setting.
cf. https://meta.discourse.org/t/email-notifications-fail-if-duplicate-headers-exist/222960/14
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`.
Currently we don’t apply watched words to custom user fields nor user
profile fields.
This led to users being able to use blocked words in their bio, location
or some custom user fields.
This patch addresses this issue by adding some validations so it’s not
possible anymore to save the User model or the UserProfile model if they
contain blocked words.
This allows the category_id filter for the bookmark
report to work with polymorphic bookmarks. Honestly this
is a little hardcode-y at the moment but until we go and
make this report a lot more flexible with more filters
I don't think it's worth the work to add extra interfaces
to RegisteredBookmarkable and BaseBookmarkable to make
this more flexible. This is enough for now.
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.
Latest redis interoduces a block form of multi / pipelined, this was incorrectly
passed through and not namespaced.
Fix also updates logster, we held off on upgrading it due to missing functions
This fixes a corner case of the perf optimization in d4e35f5.
When you have the the same post showing in multiple tab/devices and like
said post in one place, we updated the like count but didn't flip the
`acted` bool in the front-end. This caused a small visual desync.
Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
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.
Admins won't be able to disable strip_image_metadata if they don't
disable composer_media_optimization_image_enabled first since the later
will strip the same metadata on client during upload, making disabling
the former have no effect.
Bug report at https://meta.discourse.org/t/-/223350
When a user views a topic that contains a topic timer to publish to a
restricted category, an error occurs on the client side because the user
does not have access to information about the category.
This commit fixes it such that the topic timer is not shown to the user
if the user does not have access to the category.
`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 happened only for languages other than "en" and when `I18n.t` was called without any interpolation keys. The lib still tried to interpolate keys because it interpreted the `overrides` option as interpolation key.
When an admin enters a badly formed regular expression in the
permalink_normalizations site setting, a RegexpError exception is
generated everytime a URL is normalized (see Permalink.normalize_url).
The new validator validates every regular expression present in the
setting value (delimited by '|').
If the identity provider does not provide a precise username value, then we should use our UserNameSuggester to generate one and use it for the override. This makes the override consistent with initial account creation.
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.
* hidden siteSetting to enable experimental sidebar
* user preference to enable experimental sidebar
* `experimental_sidebar_enabled` attribute for current user
* Empty glimmer component for Sidebar
We were calling `dup` on the hash and using that to check for changes. However, we were not duplicating the values, so changes to arrays or nested hashes would not be detected.
This pull request follows on from https://github.com/discourse/discourse/pull/16308. This one does the following:
* Changes `BookmarkQuery` to allow for querying more than just Post and Topic bookmarkables
* Introduces a `Bookmark.register_bookmarkable` method which requires a model, serializer, fields and preload includes for searching. These registered `Bookmarkable` types are then used when validating new bookmarks, and also when determining which serializer to use for the bookmark list. The `Post` and `Topic` bookmarkables are registered by default.
* Adds new specific types for Post and Topic bookmark serializers along with preloading of associations in `UserBookmarkList`
* Changes to the user bookmark list template to allow for more generic bookmarkable types alongside the Post and Topic ones which need to display in a particular way
All of these changes are gated behind the `use_polymorphic_bookmarks` site setting, apart from the .hbs changes where I have updated the original `UserBookmarkSerializer` with some stub methods.
Following this PR will be several plugin PRs (for assign, chat, encrypt) that will register their own bookmarkable types or otherwise alter the bookmark serializers in their own way, also gated behind `use_polymorphic_bookmarks`.
This commit also removes `BookmarkQuery.preloaded_custom_fields` and the functionality surrounding it. It was added in 0cd502a558 but only used by one plugin (discourse-assign) where it has since been removed, and is now used by no plugins. We don't need it anymore.
A category_required_tag_group should always have an associated tag_group. However, this is only enforced at the application layer, so it's technically possible for the database to include a category_required_tag_group without a matching tag_group.
Previously that situation would cause the whole site to go offline. With this change, it will cause some unexpected behavior, but the site serializer will not raise an error.
Previously, accessing the Rails app directly in development mode would give you assets from our 'legacy' Ember asset pipeline. The only way to run with Ember CLI assets was to run ember-cli as a proxy. This was quite limiting when working on things which are bypassed when using the ember-cli proxy (e.g. changes to `application.html.erb`). Also, since `ember-auto-import` introduced chunking, visiting `/theme-qunit` under Ember CLI was failing to include all necessary chunks.
This commit teaches Sprockets about our Ember CLI assets so that they can be used in development mode, and are automatically collected up under `/public/assets` during `assets:precompile`. As a bonus, this allows us to remove all the custom manifest modification from `assets:precompile`.
The key changes are:
- Introduce a shared `EmberCli.enabled?` helper
- When ember-cli is enabled, add ember-cli `/dist/assets` as the top-priority Rails asset directory
- Have ember-cli output a `chunks.json` manifest, and teach `preload_script` to read it and append the correct chunks to their associated `afterFile`
- Remove most custom ember-cli logic from the `assets:precompile` step. Instead, rely on Rails to take care of pulling the 'precompiled' assets into the `public/assets` directory. Move the 'renaming' logic to runtime, so it can be used in development mode as well.
- Remove fingerprinting from `ember-cli-build`, and allow Rails to take care of things
Long-term, we may want to replace Sprockets with the lighter-weight Propshaft. The changes made in this commit have been made with that long-term goal in mind.
tldr: when you visit the rails app directly, you'll now be served the current ember-cli assets. To keep these up-to-date make sure either `ember serve`, or `ember build --watch` is running. If you really want to load the old non-ember-cli assets, then you should start the server with `EMBER_CLI_PROD_ASSETS=0`. (the legacy asset pipeline will be removed very soon)
* Adds a hidden site setting: `max_participant_names`
* Replaces duplicate code in `GroupSmtpMailer` and `UserNotifications`
* Groups are sorted by the number of users (decreasing)
* Replaces the query to count users of each group with `Group#user_count`)
* Users are sorted by their last reply in the topic (most recent first)
* Adds lots of tests
We intend to switch to the `:json` serializer, which will stringify all keys. However, we need a clean revert path. This commit ensures that our `_t` cookie handling works with both marshal (the current default) and json (the new default) serialization.
This PR enables custom email dark mode styles by default that were added here.
There is currently poor support for dark mode queries in mail clients. The main beneficiary of these changes will be Apple Mail and Outlook.
Enjoy the darkness 🕶️
When changing upload security using `Upload#update_secure_status`,
we may not have the context of how an upload is being created, because
this code path can be run through scheduled jobs. When calling
update_secure_status, the normal ActiveRecord validations are run,
and ours include validating extensions. In some cases the upload
is created in an automated way, such as user export zips, and the
security is applied later, with the extension prohibited from
use when normally uploading.
This caused the upload to fail validation on `update_secure_status`,
causing the security change to silently fail. This fixes the issue
by skipping the file extension validation when the upload security
is being changed.
Previously, 'crop' would resize the image to have the requested width, then crop the height to the requested value. This works when cropping images vertically, but not when cropping them horizontally.
For example, trying to crop a 500x500 image to 200x500 was actually resulting in a 200x200 image. Having an OptimizedImage with width/height columns mismatching the actual OptimizedImage width/height causes some unusual issues.
This commit ensures that a call to `OptimizedImage.crop(from, to, width, height)` will always return an image of the requested width/height. The `w x h^` syntax defines minimum width/height, while maintaining aspect ratio.
This commit fixes two issues at play. The first was introduced
in f6c852b (or maybe not introduced
but rather revealed). When a user posted a new message in a topic,
they received the unread topic tracking state MessageBus message,
and the Unread (X) indicator was incremented by one, because with the
aforementioned perf commit we "guess" the correct last read post
for the user, because we no longer calculate individual users' read
status there. This meant that every time a user posted in a topic
they tracked, the unread indicator was incremented. To get around
this, we can just exclude the user who created the post from the
target users of the unread state message.
The second issue was related to the private message topic tracking
state, and was somewhat similar. Whenever a user created a new private
message, the New (X) indicator was incremented, and could not be
cleared until the page was refreshed. To solve this, we just don't
update the topic state for the user when the new_topic tracking state
message comes through if the user who created the topic is the
same as the current user.
cf. https://meta.discourse.org/t/bottom-of-topic-shows-there-is-1-unread-remaining-when-there-are-actually-0-unread-topics-remaining/220817
It used to show the warning that said only members of certain groups
could view the topic even if the group "everyone" was listed in
category's permission list.
This PR will include `suspended` attribute in post serializer to check it in post widget and add a CSS class name.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Discourse has the Discourse Connect Provider protocol that makes it possible to
use a Discourse instance as an identity provider for external sites. As a
natural extension to this protocol, this PR adds a new feature that makes it
possible to use Discourse as a 2FA provider as well as an identity provider.
The rationale for this change is that it's very difficult to implement 2FA
support in a website and if you have multiple websites that need to have 2FA,
it's unrealistic to build and maintain a separate 2FA implementation for each
one. But with this change, you can piggyback on Discourse to take care of all
the 2FA details for you for as many sites as you wish.
To use Discourse as a 2FA provider, you'll need to follow this guide:
https://meta.discourse.org/t/-/32974. It walks you through what you need to
implement on your end/site and how to configure your Discourse instance. Once
you're done, there is only one additional thing you need to do which is to
include `require_2fa=true` in the payload that you send to Discourse.
When Discourse sees `require_2fa=true`, it'll prompt the user to confirm their
2FA using whatever methods they've enabled (TOTP or security keys), and once
they confirm they'll be redirected back to the return URL you've configured and
the payload will contain `confirmed_2fa=true`. If the user has no 2FA methods
enabled however, the payload will not contain `confirmed_2fa`, but it will
contain `no_2fa_methods=true`.
You'll need to be careful to re-run all the security checks and ensure the user
can still access the resource on your site after they return from Discourse.
This is very important because there's nothing that guarantees the user that
will come back from Discourse after they confirm 2FA is the same user that
you've redirected to Discourse.
Internal ticket: t62183.
This commit improves the logic for rolling up IPv4 screened IP
addresses and extending it for IPv6. IPv4 addresses will roll up only
up to /24. IPv6 can rollup to /48 at most. The log message that is
generated contains the list of original IPs and new subnet.
* FEATURE: Let sites add a sitemap.xml file.
This PR adds the same features discourse-sitemap provides to core. Sitemaps are only added to the robots.txt file if the `enable_sitemap` setting is enabled and `login_required` disabled.
After merging discourse/discourse-sitemap#34, this change will take priority over the sitemap plugin because it will disable itself. We're also using the same sitemaps table, so our migration won't try to create it
again using `if_not_exists: true`.
Sometimes we need to update a _lot_ of ACLs on S3 (such as when secure media
is enabled), and since it takes ~1s per upload to update the ACL, this is
best spread out over many jobs instead of having to do the whole thing serially.
In future, it will be better to have a job that can be run based on
a column on uploads (e.g. acl_stale) so we can track progress, similar
to how we can set the baked_version to nil to rebake posts.
After this commit, category group permissions can only be seen by users
that are allowed to manage a category. In the past, we inadvertently
included a category's group permissions settings in `CategoriesController#show`
and `CategoriesController#find_by_slug` endpoints for normal users when
those settings are only a concern to users that can manage a category.
The permissions for the 'everyone' group were not serialized because
the list of groups a user can view did not include it. This bug was
introduced in commit dfaf9831f7.
Due to default CSP web workers instantiated from CDN based assets are still
treated as "same-origin" meaning that we had no way of safely instansiating
a web worker from a theme.
This limits the theme system and adds the arbitrary restriction that WASM
based components can not be safely used.
To resolve this limitation all js assets in about.json are also cached on
local domain.
{
"name": "Header Icons",
"assets" : {
"worker" : "assets/worker.js"
}
}
This can then be referenced in JS via:
settings.theme_uploads_local.worker
local_js_assets are unconditionally served from the site directly and
bypass the entire CDN, using the pre-existing JavascriptCache
Previous to this change this code was completely dormant on sites which
used s3 based uploads, this reuses the very well tested and cached asset
system on s3 based sites.
Note, when creating local_js_assets it is highly recommended to keep the
assets lean and keep all the heavy working in CDN based assets. For example
wasm files can still live on the CDN but the lean worker that loads it can
live on local.
This change unlocks wasm in theme components, so wasm is now also allowed
in `theme_authorized_extensions`
* more usages of upload.content
* add a specific test for upload.content
* Adjust logic to ensure that after upgrades we still get a cached local js
on save
There are still some, but those are in actual code that's used outside core, so the change there would need to go through the deprecation cycle. That's a task for another day.
Previously we only supported a single 'required tag group' for a category. This commit allows admins to specify multiple required tag groups, each with their own minimum tag count.
A new category_required_tag_groups database table replaces the existing columns on the categories table. Data is automatically migrated.
Previous to this change an optimisation stripped crawler content from
all mobile browsers.
This had a side effect that meant that when we dropped support for an old
mobile platform we would stop rendering topic and topic list pages.
The new implementation ensures we only perform the optimisation on modern
mobile browsers.
This patch removes some of our freedom patches that have been deprecated
for some time now.
Some of them have been updated so we’re not shipping code based on an
old version of Rails.
All users are members of the EVERYONE group, but this group is special and
is omitted from the group_users table. When checking permission we need to
make sure we also add a bypass.
This also fixes a very buggy test in post_alerter, it was confirming the
broken behavior due to fabricator flow.
When it defined the tag group the everyone group automatically had full access
then the additional permission fabricated just added one more group. After
fix was made to code the test started failing. Fabricators can be risky.
This did not work properly everytime because the destination URL was
saved in a cookie and that can be lost for various reasons. This commit
redirects the user to invited topic if it exists.
raw_html posts (i.e. those which are pulled as part of our comments integration) don't go through our markdown pipeline, so `upload://` URLs are not supported. Running pull_hotlinked_images will break any images in the post.
In future we may add support for pulling hotlinked images in these posts. But for now, disabling it will stop it breaking images.
When emailing a group inbox and including other support-type
emails (or even just regular ones with autoresponders) in the
CC field, each automated reply to the group inbox triggered
more emails to be sent out to all CC addresses to notify them
of the new reply, which in turn caused more automated emails
to be sent to the group inbox.
This commit fixes the issue by preventing any emails being sent
by the PostAlerter when the new post has an incoming email record
which is_auto_generated, which we detect in Email::Receiver.
Via the API it is possible to create a user with an integer username. So
123 instead of "123". This causes the following 500 error:
```
NoMethodError (undefined method `unicode_normalize' for 1:Integer)
app/models/user.rb:276:in `normalize_username'
```
See: https://meta.discourse.org/t/222281
Fixes the issue where making a user x as owner of a post doesn't
cause the concerned topic to be listed in new owner's `My Posts`
top menu filter
per https://meta.discourse.org/t/199369
can_permanently_delete field in Post and TopicViewDetails serializers
cannot use Guardian's can_permanently_delete beause their use is
different. The field from the serializers is used to show the button
and the button is shown even if the post cannot be removed forever
because not enough time has passed since it was first deleted. The
guardian method is used by the controller to check that the post can
really be deleted.
Previous to this change if any of the assets were not allowed extensions
they would simply be silently ignored, this could lead to broken themes
that are very hard to debug
Our group fabrication creates groups with name "my_group_#{n}" where n
is the sequence number of the group being created. However, this can
cause the test to be flaky if and when a group with name `my_group_10`
is created as it will be ordered before
`my_group_9`. This commits makes the group names determinstic to
eliminate any flakiness.
This reverts commit 558bc6b746.
In certain instances when viewing a category, the name of a group with
restricted visilbity may be revealed to users which do not have the
required permission.
This commit introduces a new use_polymorphic_bookmarks site setting
that is default false and hidden, that will be used to help continuous
development of polymorphic bookmarks. This setting **should not** be
enabled anywhere in production yet, it is purely for local development.
This commit uses the setting to enable create/update/delete actions
for polymorphic bookmarks on the server and client side. The bookmark
interactions on topics/posts are all usable. Listing, searching,
sending bookmark reminders, and other edge cases will be handled
in subsequent PRs.
Comprehensive UI tests will be added in the final PR -- we already
have them for regular bookmarks, so it will just be a matter of
changing them to be for polymorphic bookmarks.
Under some conditions, replacing an `<img` with `![]()` can break rendering, and make the image disappear.
Context at https://meta.discourse.org/t/152801
* DEV: add testing for multi del on keys
Following #15905 we were missing some tests, this covers cases where
del is used in the form of .del(key1,key2)
Sam
Meghna
Martin Brennan
Andrei Prigorshnev
Jarek Radosz
Du Jiajun
Krzysztof Kotlarek
Daniel Waterworth
Blake Erickson
Selase Krakani
Osama Sayegh
Natalie Tay
Bianca Nenciu
David Taylor
kaden-stytch
Vinoth Kannan
Alan Guo Xiang Tan
Jan Cernik
dependabot[bot]
Joffrey JAFFEUX
Roman Rizzi
chapoi
Rafael dos Santos Silva
Jacob Michalskie
Arpit Jalan
Jan Cernik
Penar Musaraj
Ayke Halder
Loïc Guitaut
Gerhard Schlager
jbrw
Frank
Isaac Janzen
Wolftallemo
Phil Pirozhkov
Jean
Leonardo Mosquera
Jordan Vidrine
Joe
Sérgio Saquetim
Michael Brown
J. David Lowe
sansnumero
Wolftallemo
Mayfield
M.K. Safi
Johannes Faigle
Angus McLeod
Mark VanLandingham
Bianca Nenciu
Dan Ungureanu
Faizaan Gagan