The primary motivation is to simplify `eagerLoadRawTemplateModules` which curently introspects the module dependencies (the `imports` at runtime). This is no longer supported in Embroider as the AMD shims do not have any dependencies (since it's managed internally with webpack).
Prior to this commit the `setSiteThemeColor` could mistakenly receive a color with a leading `#` which would cause an invalid color to be send to `postRNWebviewMessage` and would eventually cause a crash if we try to interpolate between this color and another.
The attribute Reviewable#post_options was deprecated (and replaced by #payload) four years ago, and marked for deletion in 2.9.0. This commit removes it.
Using the lastViewedTopicId indiscriminately can cause strange scrolling behavior when navigating to a **different** topic list after viewing a topic. We only want to refocus the topic when going 'back' to the same topic list which originally triggered the navigation.
Previously we had three query parameters to control which tests would be run. The default was to run all core/plugin tests together, which would almost always lead to errors and does not match the way we run tests in CI.
This commit removes the three old parameters (skip_core, skip_plugins and single_plugin), and introduces a new 'target' parameter. This can have a value of 'core', 'plugins', 'all', or a specific plugin name. The default is 'core'. Attempting to use the old parameters will raise an error.
Previously we were implementing scroll reset/memorization on a per-page basis. Many of these approaches relied on the `didInsertElement` hook, which is no longer appropriate since Discourse changed to use the 'loading slider' strategy for page transitions.
This commit rips out all of our custom scroll resetting/memorizing, and implements those things in a generic service. There are two features:
1. After every route transition, scroll to the top of the page
2. When using browser back/forward buttons, restore the last known scroll position for those routes
To opt-out of the behaviour, individual routes can add a scrollOnTransition boolean to their RouteInfo metadata using Ember's `buildRouteInfoMetadata` hook.
The new lightbox was missing the tracked property for items when it was launched earlier as experimental feature flag.
This PR should fix issues experienced when the user clicks between multiple galleries causing the carousel images not to be updated as they were previously not tracked.
Why this change?
Prior to this change, dismissing unreads posts did not publish the
changes across clients for the same user. As a result, users can end up
seeing an unread count being present but saw no topics being loaded when
visiting the `/unread` route.
Why this change?
Prior to this change, the ordering of the tags shown in the email subject
was non-deterministic as there was no specific order specified. This
problem was exposed by a flaky test which we had.
What is the fix?
This commit orders the tags used in the email subject first by the
`Tag#public_topic_count` column in descending order and then the `Tag#name`
column in ascending order.
Simplified query based on SiteSettings to join only relevant user_options rows.
In addition, index was added to 'watched_precedence_over_muted` column in `user_options` table to speed up query
Why this change?
Group mention notifications are currently placed in the "Others" tab
of the user menu which is odd considering that mentioned notifications
are in the reply tab. This commit changes it such that group mention
notifications are displayed in the reply tab as well.
This commit makes sure we don't load all data into memory when doing CSV exports.
The most important change here made to the recently introduced export of chat
messages (3ea31f4). We were loading all data into memory in the first version, with
this commit it's not the case anymore.
Speaking of old exports. Some of them already use find_each, and it worked as
expected, without loading all data into memory. And it will proceed working as
expected after this commit.
In general, I made sure this change didn't break other CSV exports, first manually, and
then by writing system specs for them. Sadly, I haven't managed yet to make those
specs stable, they work fine locally, but flaky in GitHub actions, so I've disabled them
for now.
I'll be making more changes to the CSV exports code soon, those system specs will be
very helpful. I'll be running them locally, and I hope I'll manage to make them stable
while doing that work.
Instead of having to remember every time, just always wait until the
current transaction (if it exists) has committed before clearing any
DistributedCache.
The only exception to this is caches that aren't caching things from
postgres.
This means we have to do the test setup after setting the test
transaction, because doing the test setup involves clearing caches.
Reapplying this - it now doesn't use after_commit if skip_db is set
* FEATURE: Inline topic summary. Cached version accessible to everyone.
Anons and non-members of the `custom_summarization_allowed_groups_map` groups can see cached summaries for any accessible topic. After the first 12 hours and if the posts to summarize have changed, allowed users clicking on the button will automatically re-generate it.
* Ensure chat summaries work and prevent model hallucinations when there are no messages.
In the past, widget implementors would have to subclass the MountWidget component and wire up `didUpdateAttrs` or an observer to trigger a re-render. If that wasn't done, then it could lead to weird behaviors, especially now that page transitions in Discourse do not de-render/re-render components by default.
This commit updates MountWidget so that it re-renders whenever any input arguments change.
Browser capabilities are inherently unconnected to the lifecycle of our app. Making them formally available outside of the service means that they can safely be used in non-app-linked functions without needing risky hacks like `helperContext()` or `discourse-common/lib/get-owner`.
One example of where the old hacks were problematic is the `translateModKey()` utility function. This is called in the root of the `discourse/components/modal/keyboard-shortcuts-help` es6 module. If anything (e.g. a theme/plugin) caused that es6 module to be `require()`d before the application was booted, a fatal error would occur.
Following this commit, `translateModKey()` can safely import and access `capabilities` without needing to worry about the app lifecycle.
The only potential downside to this approach is that the capabilities data now persists across tests. If any tests need to 'stub' capabilities, they will need to revert their changes at the end of the test (e.g. by using Sinon to stub a property).
This commit also updates some legacy references from `capabilities:main` to `service:capabilities`.
These avatar-related helper functions are used in pretty-text, which currently means we load the entire `discourse/lib/utilities` module into the mini-racer when running pretty-text on the server side. This stops us adding any logic or imports to discourse/lib/utilities which may depend on other `discourse/` namespace features.
This commit moves the avatar-related utils into a dedicated module in the `discourse-common` namespace, adds backwards-compatibility shims, and updates the pretty-text config accordingly.
- Unify the silencing methods, use a WeakMap to remember the seen objects
- Export a proper plugin and use the absolute path in the config, instead
of the proprietary config from `broccoli-babel-transpiler`
The latter causes problems in Embroider which doesn't use the broccoli
based babel pipeline.
Some themes were doing `require("i18n").t()`, which was never recommended, but did work prior to f8483295. This commit restores that functionality with a deprecation notice.
What does this commit do??
This commit introduces two changes:
1. As a follow up review comment to
cc463c3e9b, we remove the top level
recipientNames cache in composer message to be a property of the
`ComposerMessage` component instead. Across components, we're more
likely to get a cache miss than a hit since we're caching the entire
recipient array so we can just drop it. If we really need this
optimisation, we should probably use a map and cache the information for
each user instead. However, the request is fairly cheap so we avoid that
optimisation for now.
2. This commit adds a debounce to `_typeReply` as well since we were not
debouncing and the method was being called each time we received the
event.
Why is this change being made?
We've decided that the previous "community" section should look more
like a primary section that holds the most important navigation links
for the site and the word "community" doesn't quite fit that
description. Therefore, we've made the decision to drop the
section heading for the community section.
As part of removing the section heading, the following changes are made
as well:
1. Button to customize the section has been moved to the "footer" of the
"More..." section when `navigation_menu` site setting is set to `sidebar`.
When `navigation_menu` is set to `header dropdown`, a button to customize
the section is shown inline.
2. The section will no longer be collapsable.
3. The title of the section is no longer customisable as it is no longer
displayed. As a technical note, we have not dropped any previous
customisations of the section's title previously in case we have to
bring back the header in the future.
4. The new topic button that was previously present in the header has
been removed alongside the header. Admins can add a custom section
link to the `/new-topic` route if there would like to make it easier for
users to create a new topic in the sidebar.
Generally follows the same pattern as #22520
There are some changes here, notably it uses the addon's babel
settings rather than the app's, and it goes through the same
treatment as the rest of the addon code (which may include more
than just babel).
However, this probably brings us closer to the normal expectations
you have around developing addon code, and in any case, does not
seem to have any effect on the final output:
```
$ diff dist/assets/markdown-it-bundle.js /tmp/dist-before/assets/markdown-it-bundle.js
```
* FIX: Default parameter recipients to create new message via params must be a string
The default parameter recipients was defined as an empty array in:
- route:application#createNewMessageViaParams
- mixin:open-composer#openComposerWithMessageParams
However, in model:composer, targetRecipient is handled as a string as can be
verified due to the existence of the #targetRecipientsArray computed property.
Using the default parameter defined as an array was causing issues with
the `discourse-bcc` plugin when opening the composer using the route
/new-message.
* DEV: Added tests for the composer messages for private messages
* Fix test naming
Co-authored-by: Mark VanLandingham <markvanlan@gmail.com>
---------
Co-authored-by: Mark VanLandingham <markvanlan@gmail.com>
Currently, the admin/wizard build relies on the addon build getting
triggered first, so that its `treeForAddon()` hook will be called,
and then it can stash the result on the app's options, which is
super fragile. In Embroider the timing works differently so the
trees end up being `undefined`.
This inverts the logic so that it will be discourse core's build
calling these hooks at a specific timing and return the result
rather than coordinating through the options bag.
```
$ diff dist/assets/admin.js dist-after/assets/admin.js
$ diff dist/assets/wizard.js dist-after/assets/wizard.js
```
Currently the I18n module shim return an object. Per AMD/loader.js,
the properties on the object becomes named exports of the module,
i.e. `import { t } from 'I18n';`.
However, this is not how we actually consume this module. We always
do `import I18n from 'I18n';`.
The returned object from the shim (`window.I18n`) does NOT have a
`default` property on it. This is only working because loader.js
has a `makeDefaultExport` feature that defaults to true, which we
are relying on to synthesize the default export for us.
That feature has been noted as undesirable and may some day be
deprecated. In Embroider, it specifically disables the feature in
loader.js.
https://github.com/embroider-build/embroider/issues/539
This commit also standardize the naming pattern of modals: `<Chat::Modal::FooBar />` and changes css class accordingly.
Co-authored-by: David Taylor <david@taylorhq.com>
When the loading slider is enabled, the rendering of `application.hbs` is slightly delayed compared to the old 'spinner' strategy. This means that if a route tried to render a dialog during its `model()` hook, the dialog wrapper element would not be present and an error would occur.
This commit detects that situation and delays rendering the error until the next runloop iteration. If the element is still not found, we print a useful error to the console.
In the long term, we should ideally convert the dialog service to use a pure-ember rendering strategy instead of leaning on a11y-dialog. But for now, this workaround should resolve the problems identified by the chat system specs.
This PR adds a feature to help admins stay up-to-date with their translations. We already have protections preventing admins from problems when they update their overrides. This change adds some protection in the other direction (where translations change in core due to an upgrade) by creating a notice for admins when defaults have changed.
Terms:
- In the case where Discourse core changes the default translation, the translation override is considered "outdated".
- In the case above where interpolation keys were changed from the ones the override is using, it is considered "invalid".
- If none of the above applies, the override is considered "up to date".
How does it work?
There are a few pieces that makes this work:
- When an admin creates or updates a translation override, we store the original translation at the time of write. (This is used to detect changes later on.)
- There is a background job that runs once every day and checks for outdated and invalid overrides, and marks them as such.
- When there are any outdated or invalid overrides, a notice is shown in admin dashboard with a link to the text customization page.
Known limitations
The link from the dashboard links to the default locale text customization page. Given there might be invalid overrides in multiple languages, I'm not sure what we could do here. Consideration for future improvement.
Instead of having to remember every time, just always wait until the
current transaction (if it exists) has committed before clearing any
DistributedCache.
The only exception to this is caches that aren't caching things from
postgres.
This means we have to do the test setup after setting the test
transaction, because doing the test setup involves clearing caches.
Some plugins call require_plugin with a wrong argument instead of the
plugin name. In a production environment that used to be a warning, but
there is no reason to keep it like that in a testing environment
because the issue will continue to be ignored.
Follow-up to b27e12445d
This commit adds 2 new site settings `default_sidebar_link_to_filtered_list` and `default_sidebar_show_count_of_new_items` to control the default values for the navigation menu preferences that were added in the linked commit (`sidebar_link_to_filtered_list` and `sidebar_show_count_of_new_items` respectively).
Chat drawer was using the `DiscourseURL` hook `afterRouteComplete`. This hook suffer from a very poor implementation which makes it very unreliable:
```javascript
if (typeof opts.afterRouteComplete === "function") {
schedule("afterRender", opts.afterRouteComplete);
}
```
This commit attempts to return the promise from `handleURL` to directly use it and have a very reliable after transition hook.
This PR converts the following modals:
- `dismiss-new`
- `dismiss-read`
- `dismiss-notification-confirmation`
to make use of the new component-based API
# Additional Changes
## Before
By default we display a warning modal when dismissing a notification however we bypass the warning modal for specific notification types when they are a 'low priority' type of notification (eg. likes). To do this we were overwriting `dismissWarningModal` on a given notification type component
```javascript
dismissWarningModal() {
return null
}
```
but in the case we wanted to change the text within the modal we were calling `showModal` and then passing in the respective options all over again, putting the logic of rendering the modal in multiple places.
```javascript
dismissWarningModal() {
const modalController = showModal("dismiss-notification-confirmation");
modalController.set(
"confirmationMessage",
I18n.t("notifications.dismiss_confirmation.body.assigns", {
count: this._unreadAssignedNotificationsCount,
})
);
return modalController;
}
```
## After
I simplified this by adding an extensible `dismissConfirmationText` function that can be updated on a per component basis as that was the only option being overridden.
eg
```javascript
get dismissConfirmationText() {
return I18n.t("notifications.dismiss_confirmation.body.bookmarks", {
count: this.#unreadBookmarkRemindersCount,
});
```
This saves us from importing the entire modal again and keeps the core logic in one place.
Instead of overwriting the `dismissWarningModal` function and returning `null` to bypass the confirmation modal, I added another extension point of `renderDismissConfirmation` (defaults to true) to _toggle_ whether we should display a confirmation when dismissing notifications.
eg
```javascript
get renderDismissConfirmation() {
return false;
}
```
we utilize this in core for specific _low priority_ notification types. When you need the confirmation modal to be displayed no matter the case you can set `alwaysRenderDismissConfirmation` to `true`
```
get alwaysRenderDismissConfirmation(){
return true
}
```
This can be useful when you want to render the confirmation modal on a custom notification type that is not deemed as _high priority_, leading to the confirmation modal never being rendered.
You can see this in use in [Discourse Assign](https://github.com/discourse/discourse-assign/pull/481)
Followup to d51baa3bb3
Also includes: Force full rerender of post-stream widget when switching topics. This ensures that plugin/theme decorators are re-run when we switch between topics with the loading slider enabled.
Previously we were using the `didInsertElement` hook and querying the DOM to check whether the other button was visible. This is problematic from a performance point of view because it forces the browser to render the layout prematurely. It can also lead to subtle bugs based on the current scroll position.
In addition, having this logic on a `didInsertElement` hook makes it totally incompatible with the new 'loading slider' feature (because the component is not re-rendered between different topic lists).
This commit updates the logic to be based simply on the count of topics in the list. If there are fewer than 5 topics, the top button is hidden.
Under certain conditions, this `afterRender` hook can be triggered after the topic-list-item has been removed from the DOM. This is more likely when the 'loading slider' strategy is used on a site.
1) Edit Category when editing a category with form templates set should have form templates enabled and showing the selected templates
Failure/Error: expect(category_page).to have_selected_template(selected_templates)
expected `#<PageObjects::Pages::Category:0x00007fdb278fbd30>.has_selected_template?("template_0,template_1")` to be truthy, got false
Wait for CSS rather than trying to compare attr directly
and also make sure the ids are always in order.
This introduces a PLATFORM_KEY_MODIFIER const that
can be used both client and server side, to determine
whether we should be using the Meta or Ctrl key based
on whether the user is on Windows/Linux or Mac.
Why this change?
A new component based API for modals was introduced in
b3a23bd9d6. This commit moves the edit
sidebar section modal to the new API.
Reviewer notes
No functionality or visual change is introduced in this PR.
In previous PR https://github.com/discourse/discourse/pull/22340 bug was introduced. Notifications were blocked when, even if topic was watched directly. New query is taking TopicUser into consideration.
In addition, in user interface, when `watched_precedence_over_muted` is not set, then value from SiteSetting should be displayed.
This brings the functionality from https://github.com/discourse/discourse-loading-slider into Discourse core. Default behaviour remains the same - the new slider mode can be enabled using the new 'page_loading_indicator' site setting.
A follow-up to 585a2e4e. A couple of tests with the new rich tooltip were flaky.
We suppose the reason is some problem related to widgets lifecycle. This PR
doesn't fix the issue, but isolates testing of the tooltip related logic related
inside its own test, which should make it not flaky.
This is a temporal solution, we're going to move all these code to using
glimmer components.
Faizaan Gagan
Roman Rizzi
Godfrey Chan
Joffrey JAFFEUX
Jamie Dunstan
Ted Johansson
David Taylor
David Battersby
dependabot[bot]
Alan Guo Xiang Tan
Joe
Krzysztof Kotlarek
Kris
Andrei Prigorshnev
Daniel Waterworth
Blake Erickson
Bianca Nenciu
Jordan Vidrine
chapoi
Selase Krakani
Sérgio Saquetim
Isaac Janzen
Meghna
Martin Brennan
Osama Sayegh
Rafael dos Santos Silva
Jarek Radosz
Keegan George
Renato Atilio