What is this change required?
The `enable_offline_indicator` site setting is disabled by default so
there is no need for us to be rendering an extra Ember component when
the site setting is not enabled.
Why this change?
Before this change, the `GroupNotificationsButton` is rendered in the
template of `userPrivateMessages` route based on a conditional that
checks if the `isGroup` property is true. However, the `isGroup`
property is determined based on the child route that is rendered.
However, this leads to "jankiness" in the UI because the
`GroupNotificationsButton` will be rendered once the route is entered
even if the model for the child route has not been resolved yet.
What is the solution?
In order to avoid this, we move the rendering of the
`GroupNotificationsButton` into the template of the
`userPrivateMessages.group` route and rely on the `in-element` helper to
render it into the right spot in the template of the
`userPrivateMessages` route.
This removes the modal container named-outlet/controller/template and replaces it with a component. Named outlets will be removed in Ember 4.x, so this change is part of that upgrade project.
Smaller changes include:
- update some of the computed values to be getters rather than calculated during `show()`.
- update tests which were previously depending on the modal class persisting after the modal was closed
Much of the logic in the service will be deprecated once we introduce component-based modals.
This work is split out from https://github.com/discourse/discourse/pull/21304
Why does this change do?
This commit updates the educate message displayed when there are no new
topics on the `/new` route when the experimental new new view site setting is enabled.
The commit also fixes a couple of bugs:
1. Correct default auto track minutes used in the copy for unread
topics from the 4 minutes to 5 minutes.
2. Correct link to user's preference in copy to go to tracking tab
instead of notifications tab.
What is the problem?
The user messages routes are currently routed by the server to
`UserActionsController#private_messages`. However, the method is
essentially a no-op and does not do any preloading. As a result, when we
load the user private messages routes, the client ends up having to
issue another request to the server to get more information about the
user profile currently being viewed. This extra request is triggered by
the `user` model's `findDetails` method that is called from the `user`
route in the `afterModel` hook.
What is the solution?
The `user` model's `findDetails` method actually checks the preload
store to see if the `user_${username}` key is present in the store and
if it is, it will use the preloaded data instead of triggering another
request. Since the user private messages routes are nested under the
user route on the client side, we have to rely on the
`UsersController#show` controller action on the server side for the user private
messages route as the `UsersController#show` controller action preloads
the required user information for the client side.
Before, the review button was shown in `primary section` when there were items to review. Otherwise, it was hidden in `more section`.
Because we are allowing admins to customize community section and reorder link, it makes sense to simplify that logic and review link should follow admin's decision.
What is the problem?
When opening the composer, we are seeing multiple requests made to
the `/composer_messages` endpoint. This is due to our use of the
`transitionend` event on the `#reply-control` element. The event is
fired once for each transition event and the `#reply-control` element
has multiple transition events.
What is the solution?
Since are only interested in the `height` transition event, we add a
condition to check that the callback function is only triggered when the
`propertyName` of the `transitionend` event is `height`.
Why is there no tests for this change?
In QUnit, we have `transition: none !important` set in the stylesheet
with no easy way to disable. We'll have to accept the risk of not
writing test for this performance fix.
Meta topic: https://meta.discourse.org/t/mention-suggestion-list-box-in-the-rtl-website-in-wrong-place/266763?u=osama.
Our autocomplete box doesn't currently take into account the user's locale and places itself off-screen when using an RTL locale. This commit changes the placement logic for the autocomplete box when an RTL locale is used to make sure that:
1. the autocomplete box's right side is near and to the left of the caret
2. the autocomplete box doesn't go beyond the composer's left side.
Prior to this commit, we didn't have RTL versions of our admin and plugins CSS bundles and we always served LTR versions of those bundles even when users used an RTL locale, causing admin and plugins UI elements to never look as good as when an LTR locale was used. Example of UI issues prior to this commit were: missing margins, borders on the wrong side and buttons too close to each other etc.
This commit creates an RTL version for the admin CSS bundle as well as RTL bundles for all the installed plugins and serves those RTL bundles to users/sites who use RTL locales.
* FEATURE: reduce avatar sizes to 6 from 20
This PR introduces 3 changes:
1. SiteSetting.avatar_sizes, now does what is says on the tin.
previously it would introduce a large number of extra sizes, to allow for
various DPIs. Instead we now trust the admin with the size list.
2. When `avatar_sizes` changes, we ensure consistency and remove resized
avatars that are not longer allowed per site setting. This happens on the
12 hourly job and limited out of the box to 20k cleanups per cycle, given
this may reach out to AWS 20k times to remove things.
3.Our default avatar sizes are now "24|48|72|96|144|288" these sizes were
very specifically picked to limit amount of bluriness introduced by webkit.
Our avatars are already blurry due to 1px border, so this corrects old blur.
This change heavily reduces storage required by forums which simplifies
site moves and more.
Co-authored-by: David Taylor <david@taylorhq.com>
What is the problem?
There are two problems being fixed here:
1. When opening the composer, we are seeing multiple requests made to
the `/composer_messages` endpoint. This is due to our use of the
`transitionend` event on the `#reply-control` element. The event is
fired once for each transition event and the `#reply-control` element
has multiple transition events.
2. System tests have animations disabled so the `transitionend` event
does not fire at all.
What is the solution?
Instead of relying on the `transitionend` event, we can instead just
observer the `composerState` property of the `ComposerBody` component
and trigger the `composer:opened` appEvent with a delay that is similar
to the transition duration used for the `ComposerBody` component.
We currently have some CSS rules in `common/base/rtl.scss` that were added to workaround shortcomings of the R2 gem that we used to use to generate versions of our CSS that are suitable for RTL layouts. Those workarounds are mostly duplicates of existing rules with the only difference being that they're flipped to suit RTL layouts (e.g. `padding-left` is changed to `padding-right` and vice versa).
However, we've recently replaced R2 with `rtlcss` which doesn't have those shortcomings of R2 (see f94951147e) which means those workarounds/duplicate rules need to be removed because they're getting flipped by `rltcss`, essentially reverting them to their original LTR version and causing issues with RTL layouts.
This commit removes those workarounds that are no longer needed, and cleans up the the file that contains our RTL-specific CSS.
Meta topic: https://meta.discourse.org/t/avatar-in-rtl-website-in-wrong-place/264676?u=osama.
This commit adds modifiers that allow plugins to change how categories and groups are prefetched into the application and listed in the respective controllers.
Possible use cases:
- prevent some categories/groups from being prefetched when the application loads for performance reasons.
- prevent some categories/groups from being listed in their respective index pages.
We were calling reset without the proper params which was causing errors in the console. This commit does the following changes:
- ensures `composer.cancel()` is the only way to cancel editing/reply
- adds a `draftSaved` property to chat message to allow for better tests
- writes a spec to ensure the flow is correct
- adds more page objects for better tests
- homogenize the default state of objects on chat message
Co-authored-by: Martin Brennan <martin@discourse.org>
Privacy Policy and Terms of Service topics are no longer created by
default for communities that have not set a company name. For this
reason, some URLs were pointing to 404 page.
Allow admins to edit Community section. This includes drag and drop reorder, change names, delete and reset to default.
Visual improvements introduced in edit community section modal are available in edit custom section form as well. For example:
- drag and drop links to change their position;
- smaller icon picker.
Why is this change required?
The flaky system test was due to the fact that we had to poll for the
user preferences interface page to reload after saving. However, this
turns out to be a bug on the user perferences interface page because the
page should only reload if the user has selected a new theme that is
different from the site's default but we were reloading the page for
users that did not have any user theme selected. Therefore there was an
unnecessary reload happening when saving other fields on the user
preferences interface page.
Followup to eae47d82e2,
we removed some specificity from the hashtag color
CSS classes, but now the color is being overridden
by the base hashtag-cooked.d-icon color. This color
is no longer needed, so we just remove that and
the specificity.
New headless shares the same implementation as the chrome browser
instead of being a separate implementation of its own.
See https://developer.chrome.com/articles/new-headless/ for more
details
Co-authored-by: Rafael dos Santos Silva <xfalcox@gmail.com>
Why are we making this change?
Currently, we are displaying the value of the `short_site_description`
site setting in the sidebar only for anonymous user. However, the
display of the description seems out of place in both the `sidebar` and
`header dropdown` navigation menu and do not think the sidebar is the
right place to display it anymore.
This commit introduces a new `within_user_updater_transaction` event that's triggered inside the transaction that saves user updates in `UserUpdater`. Plugins can hook into the transaction using the event to include custom changes in the transaction. Callbacks for this event receive 2 arguments:
1. the user being saved
2. the changed attributes that are passed to `UserUpdater`.
There's also new modifier in this commit called `users_controller_update_user_params` to allow plugins to allowlist custom params in the `UsersController` which eventually end up getting passed as attributes to the `UserUpdater` and the new `within_user_updater_transaction` event where they can be used to perform additional updates using the custom params.
-----
New API is used in https://github.com/discourse/discourse-mailinglist-integration/pull/1.
When a user chooses to move a topic/message to an existing topic/message, they can now opt to merge the posts chronologically (using a checkbox in the UI).
The field more_topic_url is already included in the response preloaded in categories#index
However this field was missing if a request was subsequently made to update the page using
the end-points /categories_and_latest or /categories_and_top. This could lead the client
app to display incorrect information if it relied on this information to update the UI.
This brings the behaviour in line with our other widget-related APIs like `decorateWidget` and `reopenWidget`. This commit also adds a theme/plugin prefix to the console messages.
For now, state is still stored in the modal controller. Eventually the controller will be replaced with a component, and the state will be stored in the service.
(extracted from https://github.com/discourse/discourse/pull/21304)
This commit deprecates the `modify_user_params` method in `UsersController` in favor of a new modifier that replaces that method whose entire purpose is to allow plugins to monkey-patch it to permit custom params in the controller. We now have the "modifier" system which can achieve the same results but in a safer and easier way. The modifier that replaces the deprecated method is included in PR https://github.com/discourse/discourse/pull/21737.
What is this change required?
I noticed that actions in `SidebarSectionsController` resulted in
lots of N+1 queries problem and I wanted a solution to
prevent such problems without having to write N+1 queries tests. I have
also used strict loading for `SidebarSection` queries in performance
sensitive spots.
Note that in this commit, I have also set `config.active_record.action_on_strict_loading_violation = :log`
for the production environment so that we have more visibility of
potential N+1 queries problem in the logs. In development and test
environment, we're sticking with the default of raising an error.
What is the problem?
In the test environement, we were calling `SiteSetting.setting` directly
to introduce new site settings. However, this leads to changes in state of the SiteSettings
hash that is stored in memory as test runs. Changing or leaking states
when running tests is one of the major contributors of test flakiness.
An example of how this resulted in test flakiness is our `spec/integrity/i18n_spec.rb` spec file which
had a test case that would fail because a new "plugin_setting" site
setting was registered in another test case but the site setting did not
have translations for the site setting set.
What is the fix?
There are a couple of changes being introduced in this commit:
1. Make `SiteSetting.setting` a private method as it is not safe to be
exposed as a public method of the `SiteSetting` class
2. Change test cases to use existing site settings in Discourse instead
of creating custom site settings. Existing site settings are not
removed often so we don't really need to dynamically add new site
settings in test cases. Even if the site settings being used in test
cases are removed, updating the test cases to rely on other site
settings is a very easy change.
3. Set up a plugin instance in the test environment as a "fixture"
instead of having each test create its own plugin instance.
Legal topics, such as the Terms of Service and Privacy Policy topics
do not make sense if the entity creating the community is not a company.
These topics will be created and updated only when the company name is
present and deleted when it is not.
This patch is a followup of
https://github.com/discourse/discourse/pull/21504 where limits on custom
message for an invite were introduced.
This had a side effect of making some existing invites invalid and with
the current code, they can’t be invalidated anymore.
This patch takes the approach of skipping the validations when invites
are invalidated since the important thing here is to mark the invite as
invalidated regardless of its actual state in the DB. (no other
attributes are updated at the same time anyway)
This PR adds status to mentions in chat and makes those mentions receive live updates.
There are known unfinished part in this implementation: when posting a message, status on mentions on that message appears immediately, but only if a user used autocomplete when typing the message. If user copy and paste a message with mentions into chat composer, those mentions won't have user status on them.
PRs with fixes for both problems are following soon.
Preparations for this PR that were made previously include:
- DEV: correct a relationship – a chat message may have several mentions 0dcfd7ddec
- DEV: extract the logic for extracting and expanding mentions from ChatNotifier 75b81b6854
- DEV: Always create chat mention records fa543cda06
- DEV: better split create_notification! and send_notifications logic e292c45924
- DEV: more tests for mentions when updating chat messages e7292e1682
- DEV: extract updating status on mentions into a lib function e49d338c21
- DEV: Create and update chat message mentions earlier 35a414bb38
- DEV: Create a chat_mention record when self mentioning 2703f2311a
- DEV: When deleting a chat message, do not delete mention records f4fde4e49b
This commit prevents unallowed URLs in iframe src by adding a relative path like `https://bob.com/abc/def/../ghi`. Currently, the iframe linking to the site uses the current_user, not the post's author, so users who have no access to a certain path are not able to view anything they shouldn't.
Why this change?
This change allows plugins or themes to replace the tag icon in the
sidebar. The color of the icon can be customised as well.
However, do note that this change is marked experimental as we intend to
support custom icons for tags in the near term as part of Discourse core.
Therefore, the plugin API will become obsolete once that happens and we
are marking it experimental to avoid having to deprecate it.
* FIX: Video thumbnails can have duplicates
It's possible that a duplicate video or even a very similar video could
generate the same video thumbnail. Because video thumbnails are mapped
to their corresponding video by using the video sha1 in the thumbnail
filename we need to allow for duplicate thumbnails otherwise even when a
thumbnail has been generated for a topic it will not be mapped
correctly.
This will also allow you to re-upload a video on the same topic to
regenerate the thumbnail.
* fix typo
This commit makes some fundamental changes to how hashtag cooking and
icon generation works in the new experimental hashtag autocomplete mode.
Previously we cooked the appropriate SVG icon with the cooked hashtag,
though this has proved inflexible especially for theming purposes.
Instead, we now cook a data-ID attribute with the hashtag and add a new
span as an icon placeholder. This is replaced on the client side with an
icon (or a square span in the case of categories) on the client side via
the decorateCooked API for posts and chat messages.
This client side logic uses the generated hashtag, category, and channel
CSS classes added in a previous commit.
This is missing changes to the sidebar to use the new generated CSS
classes and also colors and the split square for categories in the
hashtag autocomplete menu -- I will tackle this in a separate PR so it
is clearer.
This might soon become a first class feature in Discourse core in the
short term so marking it as experimental for now to bridge
certain Discourse own-ed plugins and themes.
This commit adds the experimental `registerCustomCategorySectionLinkPrefix` client side
plugin API that allows themes or plugins to override the prefix of a
category section link.
This is marked experimental because we might be introducing a core
feature where category icons are supported. This is currently use as a
bridge for the https://github.com/discourse/discourse-category-icons
theme component.
* FIX: Fix for Default to subcategory when parent category does not allow posting
* added tests for edge case scenario
* implemented correct behaviour when parent category doesn't have subcategories
* implemented new fabricator for categories and suggested changes
New client side plugin API that allows plugins or themes to customize
the fontawesome 5 icon used to indicate that a category is locked/read
restricted.
The welcome topic user tip was for admins only, but in general, user
tips should be used for guiding new users through the features that
Discourse offers. For this reason, we decided to remove the user tip.
This commit also includes a few more copy tweaks to the welcome topic.
The old method updated only existing records, without considering that
new tags might have been created or some tags might not exist anymore.
This was usually not a problem because the stats were also updated by
other code paths.
However, the ensure consistency job should be more solid and help when
other code paths fail or after importing data.
Also, update category tag stats too should happen when updating other
category stats as well.
When uploading images via direct to S3 upload, we were
assuming that we could not pre-emptively check the file
size because the client may do preprocessing to reduce
the size, and UploadCreator could also further reduce the
size.
This, however, is not true of gifs, so we would have an
issue where you upload a gif > the max_image_size_kb
setting and had to wait until the upload completed for
this error to show.
Now, instead, when we direct upload gifs to S3, we check
the size straight away and present a file size error to
the user rather than making them wait. This will increase
meme efficiency by approximately 1000%.
### What is this change?
The lounge category was replaced with the general category in https://github.com/discourse/discourse/pull/18097.
However, there are still a few references to the lounge category in code. In particular, `Category#seeded?` is erroring out in production looking for `SiteSetting.lounge_category_id`.
In #21498, we split `BaseStore#download` into a "safe" version which returns nil on errors, and an "unsafe" version which raises an exception, which was the old behaviour of `#download`.
This change updates call sites that used the old `#download`, which raised exceptions, to use the new `#download!` to preserve behaviour (and silence deprecation warnings.)
It also silences the deprecation warning in tests.
What is the problem?
This is a follow up to 4cca7de22d. In the
commit, CSS was used to disable the collapsing of sections in the header
dropdown navigation menu when the `navigation_menu` site setting is set
to `header dropdown`. However, using CSS is not the correct approach as
the underlying code is still marking the section as collapsable which
means that the sections will still be displayed as collapsed with no way
to "uncollapse" if the local store has already marked the section as
collapsed.
What is the fix?
This commit removes the usage of CSS to hide the collapsabe button and
instead correctly marks the section as not collapsable in the code.
What is the problem?
In `SvgSpriteController#search` and `SvgSpriteController#icon_picker_search`, the controller actions
was using the `RailsMultisite::ConnectionManagement.with_hostname` API
but `params[:hostname]` was always `nil` because the routes does not
have a `:hostname` param component and the client does not ever pass the
`:hostname` param when making the request. When `RailsMultisite::ConnectionManagement.with_hostname` is
used with a `nil` argument, it ends up connecting to the default
multisite database. Usually this would be bad because we're allowing a
site in a multisite setup to connect to another site but thankfully no
private data is being leaked here.
What is the fix?
Since `SvgSpriteController#search` and `SvgSpriteController#icon_picker_search` are login required route,
there is no need for us to switch database connections. The fix here is
to simply remove the use of `RailsMultisite::ConnectionManagement.with_hostname`.
Context of the problem
When viewing the topic list for either the personal inbox or the group
PM inbox, we store a cache of the topic list if the user has loaded more
topics in the topic list. This cache is used to improve the experience
for users so that navigating to a topic and then back would not make
them lose their "last read" position in the topic list. Without this
cache, users will have to start from the top of the topic list each time
they navigate back after reading a topic.
What is the problem?
After archiving a PM, the user is redirected to either the personal
inbox or the group PM inbox. The problem is that if a topic list cache
exists, we will render the topic list using the cache. However, this
means that the archived PM will still appear in the list leading to
confusion for our users.
What is the fix?
To fix this, we will simply clear the topic list cache after a user
archives a topic.
It wasn't possible (at least in any reasonable way) to pass params like `tags`. Also removes the export and inlines the function as that was used only to test the function and the test is gone.
This test was passing, but the environment it was testing was incorrect.
The `image-controls` markdown rule allowlists several svgs when previewing.
But since `previewing: true` is only set on the parent `ComposerEditor`
component, the test in `DEditor` wasn't aware of that, so the output was
ignoring the `previewing` option.
This moves the test one level higher, to `ComposerEditor`, and because
now `previewing: true` is correctly used, it updates the test to show
that the svg element is present, but an `onload` attributes is stripped.
We have been struggling lately finding site settings due to 30 setting limit
This was introduced for performance reasons a while back but is no longer as
needed given that ember is faster.
Additionally searching is hard, so allow people to use fuzzy search against
setting name.
What is the problem?
Previously the `sections` getter was initializing duplicate `lib/sidebar/(community-)section` instances every time it was evaluated. This change in identity was causing Ember's `{{#each` helper to totally rerender every section whenever the getter was evaluated.
What is the fix?
This commit refactors things to lean on Ember's components for state/lifecycle management. The `{{#each` loop is done over the source data, which is guaranteed to only change identity when there is a real config change. Individual section components are initialized for each section, and are responsible for constructing and tearing down their own `lib/sidebar/(community-)section` instances.
This commit also updates `lib/sidebar/(community-)section` to support service injection rather than passing service references around.
Co-authored-by: David Taylor <david@taylorhq.com>
We use the `:empty` css selector on `#modal-alert`, so we need to strip any whitespace from the contents to ensure the selector functions correctly. Followup to ad431ab03a