This commit adds support for an optional `prompt` parameter in the
payload of the /session/sso_provider endpoint. If an SSO Consumer
adds a `prompt=none` parameter to the encoded/signed `sso` payload,
then Discourse will avoid trying to login a not-logged-in user:
* If the user is already logged in, Discourse will immediately
redirect back to the Consumer with the user's credentials in a
signed payload, as usual.
* If the user is not logged in, Discourse will immediately redirect
back to the Consumer with a signed payload bearing the parameter
`failed=true`.
This allows the SSO Consumer to simply test whether or not a user is
logged in, without forcing the user to try to log in. This is useful
when the SSO Consumer allows both anonymous and authenticated access.
(E.g., users that are already logged-in to Discourse can be seamlessly
logged-in to the Consumer site, and anonymous users can remain
anonymous until they explicitly ask to log in.)
This feature is similar to the `prompt=none` functionality in an
OpenID Connect Authentication Request; see
https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
* UX: selected indicator more topics spacing
In relation to the selected indicator, this PR addresses a vertical and horizontal issue with the indicator touching the table header and its left alignment to be more consistent with the regular topic list feed.
* UX: border for pill navigation on mobile
Makes UI more consistent desktop -> mobile, by adding a border to the bottom of the Suggested/Related.
Currently moderators can see the custom public sidebar section edit button, but they are prevented from making any changes by an error. According to the back-end, moderators can not access these.
This PR hides the custom public sidebar section edit button, as well as the "make public" checkbox of the create modal, if the user is not an admin, bringing the UI in line with the back-end.
If needed, we can add a site setting to allow moderator access when the need arises.
JS tests expect `show_copy_button_on_codeblocks` to be false (because
default before #81f3f56 was false). There is probably a different
issue at play here with JS tests, I haven't dug into it yet.
Instead, this PR adds a system test to ensure copy button is present
for code blocks with default site settings enabled.
1. actually call `popupAjaxError`, thanks :P
2. don't close a modal on error
3. use `extractError()` instead of manually joining error messages
4. …or passing just the error object to `this.flash`
Discourse has a custom stylesheet pipeline which compiles things 'just in time'. The only place we were still running sass files through sprockets was for the `/tests` route in development mode. This use can be removed by compiling the relevant stylesheets through ember-cli instead (which we were already doing for testem runs)
This work was prompted by the incompatibility of dartsass-sprockets with the latest sass-embedded release (https://github.com/tablecheck/dartsass-sprockets/issues/13)
Our custom implementation of `getOwner` includes a fallback which returns an owner, even if the passed object does not have one set. This is confusing and creates a false sense of security. Generally if the fallback is used, it means there is a problem with the patterns being used.
This commit renames our custom implementation to `getOwnerWithFallback`, while maintaining the old `getOwner` export with a deprecation notice. Core code is updated to use the official `@ember/application` implementation, or the new `getOwnerWithFallback` function.
This commit updates all core uses of `{ getOwner } from discourse-common/lib/get-owner` to use `getOwnerWithFallback`. Future commits will work through and convert many of these to use the official `@ember/application` implementation
A new `rawRenderGlimmer` function is introduced which can be used to render glimmer components inside our legacy 'raw hbs' views. See discourse/lib/raw-render-glimmer for more information. This will help as we work to move away from raw-hbs use.
This PR introduces three new concepts to Discourse codebase through an addon called "FloatKit":
- menu
- tooltip
- toast
## Tooltips
### Component
Simple cases can be express with an API similar to DButton:
```hbs
<DTooltip
@Label={{i18n "foo.bar"}}
@ICON="check"
@content="Something"
/>
```
More complex cases can use blocks:
```hbs
<DTooltip>
<:trigger>
{{d-icon "check"}}
<span>{{i18n "foo.bar"}}</span>
</:trigger>
<:content>
Something
</:content>
</DTooltip>
```
### Service
You can manually show a tooltip using the `tooltip` service:
```javascript
const tooltipInstance = await this.tooltip.show(
document.querySelector(".my-span"),
options
)
// and later manual close or destroy it
tooltipInstance.close();
tooltipInstance.destroy();
// you can also just close any open tooltip through the service
this.tooltip.close();
```
The service also allows you to register event listeners on a trigger, it removes the need for you to manage open/close of a tooltip started through the service:
```javascript
const tooltipInstance = this.tooltip.register(
document.querySelector(".my-span"),
options
)
// when done you can destroy the instance to remove the listeners
tooltipInstance.destroy();
```
Note that the service also allows you to use a custom component as content which will receive `@data` and `@close` as args:
```javascript
const tooltipInstance = await this.tooltip.show(
document.querySelector(".my-span"),
{
component: MyComponent,
data: { foo: 1 }
}
)
```
## Menus
Menus are very similar to tooltips and provide the same kind of APIs:
### Component
```hbs
<DMenu @ICON="plus" @Label={{i18n "foo.bar"}}>
<ul>
<li>Foo</li>
<li>Bat</li>
<li>Baz</li>
</ul>
</DMenu>
```
They also support blocks:
```hbs
<DMenu>
<:trigger>
{{d-icon "plus"}}
<span>{{i18n "foo.bar"}}</span>
</:trigger>
<:content>
<ul>
<li>Foo</li>
<li>Bat</li>
<li>Baz</li>
</ul>
</:content>
</DMenu>
```
### Service
You can manually show a menu using the `menu` service:
```javascript
const menuInstance = await this.menu.show(
document.querySelector(".my-span"),
options
)
// and later manual close or destroy it
menuInstance.close();
menuInstance.destroy();
// you can also just close any open tooltip through the service
this.menu.close();
```
The service also allows you to register event listeners on a trigger, it removes the need for you to manage open/close of a tooltip started through the service:
```javascript
const menuInstance = this.menu.register(
document.querySelector(".my-span"),
options
)
// when done you can destroy the instance to remove the listeners
menuInstance.destroy();
```
Note that the service also allows you to use a custom component as content which will receive `@data` and `@close` as args:
```javascript
const menuInstance = await this.menu.show(
document.querySelector(".my-span"),
{
component: MyComponent,
data: { foo: 1 }
}
)
```
## Toasts
Interacting with toasts is made only through the `toasts` service.
A default component is provided (DDefaultToast) and can be used through dedicated service methods:
- this.toasts.success({ ... });
- this.toasts.warning({ ... });
- this.toasts.info({ ... });
- this.toasts.error({ ... });
- this.toasts.default({ ... });
```javascript
this.toasts.success({
data: {
title: "Foo",
message: "Bar",
actions: [
{
label: "Ok",
class: "btn-primary",
action: (componentArgs) => {
// eslint-disable-next-line no-alert
alert("Closing toast:" + componentArgs.data.title);
componentArgs.close();
},
}
]
},
});
```
You can also provide your own component:
```javascript
this.toasts.show(MyComponent, {
autoClose: false,
class: "foo",
data: { baz: 1 },
})
```
Co-authored-by: Martin Brennan <mjrbrennan@gmail.com>
Co-authored-by: Isaac Janzen <50783505+janzenisaac@users.noreply.github.com>
Co-authored-by: David Taylor <david@taylorhq.com>
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
favicons were removed in #17477
theme-preview appears to once be a `.hbs` file shared by several
components, which went away with the refactor in #20282. It must
have accidentally came back with some rebase error, and then got
picked up by the template-only component codemod. It's unused and
does nothing anyway.
It seems like the intention is to update the tab selection at the bottom when the scrollable pane changes enough. In my testing (and I think by definition?), it doesn't seem like `scrollLeft` ever exceeds `offsetWidth`, so that tab-switching behavior doesn't ever happen
What is the problem we are trying to solve here?
The `/` path in our Ember app leads to the `discovery.index` route but
we actually don't render anything on that route leading to a blank page
if the Ember app were to transition to it which is what was happening
when a user adds a custom sidebar section link with the `/` path.
What is the fix there?
Instead of generating a link for the `discovery.index` route when
creating the sidebar section link, we detect if the Ember route is
`discovery.index` and change it to the `discovery.${defaultHomepage()}`
route instead.
Some sites have a large number of categories and fetching the category
IDs or category topic IDs just to build another query can take a long
time or resources (i.e. memory).
Our existing PluginOutlet system allows theme/plugin developers to easily insert new content into Discourse.
Another common requirement is to **replace** existing content in Discourse. Previously this could be achieved either using template overrides, or by introducing new content via a PluginOutlet and then hiding the old implementation with CSS. Neither of these patterns are ideal from a maintainability or performance standpoint.
This commit introduces a new mode for PluginOutlets. They can now be used to 'wrap' blocks of content in core. If a plugin/theme registers a connector for the outlet, then it will be rendered **instead of** the core implementation. If needed, outlets can use `{{yield}}` to render the core implementation inside their own implementation (e.g. to add a wrapper element).
In this 'wrapper' mode, only one connector can be registered for each outlet. If more than one is registered, only one will be used, and an error will be printed to the console.
To introduce a new PluginOutlet wrapper, this kind of thing can be added to a core template:
```hbs
<PluginOutlet @name="site-logo" @defaultGlimmer={{true}} @outletArgs={{hash title=title}}>
<h1>This is the default core implementation: {{title}}</h1>
</PluginOutlet>
```
A plugin/theme can then register a connector for the `site-logo` outlet:
```hbs
{{! connectors/site-logo/my-site-logo-override.hbs }}
<h2>This is the plugin implementation: {{@outletArgs.title}}</h2>
```
Care should be taken when introducing new wrapper PluginOutlets. We need to ensure that
1) They are properly sized. In general it's preferable for each outlet to wrap a small amount of core code, so that plugin/themes only need to re-implement what they want to change
2) The `@outletArgs` are carefully chosen. It may be tempting to pass through lots of core implementation into the outletArgs (or worse, use `this` to pass a reference to the wrapping component/controller). Doing this will significantly increase the API surface area, and make it hard to refactor core. Instead, we should aim to keep `@outletArgs` to a minimum, even if that means re-implementing some very simple things in themes/plugins.
Previously we were using 'mouseup', which meant that if you started the click inside, and then dragged to outside the modal, it would still close. This kind of dragging action is common when selecting text, and having it close the modal can be very frustrating.
Simply switching to a 'click' listener doesn't totally solve the problem, because when a click event involves dragging from one element to another, the browser will fire the event on "the most specific ancestor element that contained both elements". For modals, the most specific common ancestor was still the `modal-middle-container`, which would cause the modal to close.
Therefore, this commit sets the modal containers to have `pointer-events: none`, and sets up the click listener on the `.modal-backdrop` element, which is **adjacent** to the modal in the DOM. That means that click events fired on any ancestors of the modal will not accidentally trigger closure.
This is an aesthetic change. Currently, if one of the scores involved in the reviewable score explanation is negative, we display it as: + -value. This changes that.
I also made an attempt at converting the component into GJS format. This is done as a separate commit.
Meta topic: https://meta.discourse.org/t/suppress-these-tags-from-summary-emails-settings-is-not-working-in-preview-digest-email/279196?u=osama
Follow-up to 477a5dd371
The `digest_suppress_tags` setting is designed to be a list of pipe-delimited tag names, but the tag-based topic suppression logic assumes (incorrectly) that the setting contains pipe-delimited tag IDs. This mismatch in expectations led to the setting not working as expected.
This PR adds a step that converts the list of tag names in the setting to their corresponding IDs, which is then used to suppress topics tagged with those specific tags.
See https://github.com/discourse/discourse-encrypt/pull/282
> `cooked` was an Ember SafeString. The internal storage of the string changed from `.string` to `.__string` at some point between Ember 3.28 and Ember 5. Instead, we can use `toString()` which is guaranteed to work in all situations
DEV: Adjust site setting search limiter
This opens up the site setting search limiter some more so that when
searching for "min length" it will contain
"min_personal_message_post_length" as one of the results, but not open
it up so much so that when searching for "digest",
"pending_users_reminder_delay_minutes" won't show up in the results
because it isn't really related.
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
We are seeing occasional flakes in `patch-package`, possibly caused by https://github.com/ds300/patch-package/issues/484. This wrapper script will retry patch-package three times before giving up. Longer-term we hope to upgrade to a package manager with built-in patch support.
- Switch to `@tracked` and native getters
- Remove queryParam defaults which are awkward to work with. Instead, add `resolvedBlah` getters
- Add 'no results found' text
- Use standard 'model' key instead of a custom `setupController` method
- Remove use of `route-action`
- Remove `{{action` helper
Default queryParams in ember controllers are tricky to work with, especially when combined with the new router service. Instead, we can handle defaults ourselves
* DEV: upgrade grant badge modal to glimmer
* DEV: add unit tests for grant badge utils
* DEV: replace grant-badge-controller mixin with grant-badge-utils in admin-user-badges controller
* DEV: remove GrantBadgeController mixin
* FIX: Return 403 instead of redirect on username routes when hidding profiles
* Updated raised error to better reflect the problem to the user
* implemented suggested changes
This will allow initializing the glimmer search menu without having to pass args directly from header.js widget, to help themes and plugins with search customizations
---------
Co-authored-by: Mark VanLandingham <markvanlan@gmail.com>
Previously, a "`some_not_allowed`" warning message was appeared in composer even when all the users mentioned via group are already invited to the private message directly or via other groups.
Why this change?
We have been bitten by bugs where tests are not catching missing
interpolate argument in our client side code because the JavaScript
tests are also using `I18n.translate` to assert that the right message
is shown. Before this change, `I18n.interpolate` will just replace the
missing interpolation argument in the final translation with some
placeholder. As a result, we ended up comparing a broken translation
with another broken translation in the test environment.
Why does this change do?
This change introduces the `I18n.testing` property which when set to
`true` will cause `I18n.translate` to throw an error when an interpolate
argument is missing. With this commit, we also set `I18n.testing = true`
when running qunit acceptance test.
This commit adds limits to themes and theme components on the:
- file size of about.json and .discourse-compatibility
- file size of theme assets
- number of files in a theme
The hidden site setting max_drafts_per_user defaults to 10_000 drafts per user.
The longest key should be "topic_<MAX_BIG_INT>" which is 25 characters.
On `mousedown` if the click is outside a cooked element cancel the `mousedown`/`mouseup` sequence and only rely on the `selectionchange` event.
This change ensures a click on avatar for example will work, even if user is doing a rather slow click (meaning: the mousedown has been hold for more than 100ms).
Due to server upload limits backups may receive a 413 error so we need
to display a different error message than the default one we have set
for attachments.
Previously, calling `decorateCookedElement` would re-open a number of components and introduce new event listeners. This kind of thing cannot be undone, and so we were forced to introduce the unique 'id' parameter. If a given decorator id had already been applied, we would skip re-applying it. This helped, but it was still problematic because all tests would be using the callback which was registered in the first test. If its closure had any references to the ApplicationInstance, then those references would be destroyed and useless in future tests.
This commit switches strategy to use `appEvents` instead of `klass.reopen`. This is a much more obvious system and, since appEvent registrations are reset for every ApplicationInstance, we can drop the requirement for unique ids on `decorateCookedElement` calls. The callback used will always be the one registered against the current ApplicationInstance.
This commit also updates our `wrapWithErrorHandler` implementation so that it throws errors in tests. This ensures that errors are not silently swallowed in CI.
Second iteration of https://github.com/discourse/discourse/pull/23312 with a fix for embroider not resolving an export file using .gjs extension.
---
This PR introduces three new concepts to Discourse codebase through an addon called "FloatKit":
- menu
- tooltip
- toast
## Tooltips
### Component
Simple cases can be express with an API similar to DButton:
```hbs
<DTooltip
@label={{i18n "foo.bar"}}
@icon="check"
@content="Something"
/>
```
More complex cases can use blocks:
```hbs
<DTooltip>
<:trigger>
{{d-icon "check"}}
<span>{{i18n "foo.bar"}}</span>
</:trigger>
<:content>
Something
</:content>
</DTooltip>
```
### Service
You can manually show a tooltip using the `tooltip` service:
```javascript
const tooltipInstance = await this.tooltip.show(
document.querySelector(".my-span"),
options
)
// and later manual close or destroy it
tooltipInstance.close();
tooltipInstance.destroy();
// you can also just close any open tooltip through the service
this.tooltip.close();
```
The service also allows you to register event listeners on a trigger, it removes the need for you to manage open/close of a tooltip started through the service:
```javascript
const tooltipInstance = this.tooltip.register(
document.querySelector(".my-span"),
options
)
// when done you can destroy the instance to remove the listeners
tooltipInstance.destroy();
```
Note that the service also allows you to use a custom component as content which will receive `@data` and `@close` as args:
```javascript
const tooltipInstance = await this.tooltip.show(
document.querySelector(".my-span"),
{
component: MyComponent,
data: { foo: 1 }
}
)
```
## Menus
Menus are very similar to tooltips and provide the same kind of APIs:
### Component
```hbs
<DMenu @icon="plus" @label={{i18n "foo.bar"}}>
<ul>
<li>Foo</li>
<li>Bat</li>
<li>Baz</li>
</ul>
</DMenu>
```
They also support blocks:
```hbs
<DMenu>
<:trigger>
{{d-icon "plus"}}
<span>{{i18n "foo.bar"}}</span>
</:trigger>
<:content>
<ul>
<li>Foo</li>
<li>Bat</li>
<li>Baz</li>
</ul>
</:content>
</DMenu>
```
### Service
You can manually show a menu using the `menu` service:
```javascript
const menuInstance = await this.menu.show(
document.querySelector(".my-span"),
options
)
// and later manual close or destroy it
menuInstance.close();
menuInstance.destroy();
// you can also just close any open tooltip through the service
this.menu.close();
```
The service also allows you to register event listeners on a trigger, it removes the need for you to manage open/close of a tooltip started through the service:
```javascript
const menuInstance = this.menu.register(
document.querySelector(".my-span"),
options
)
// when done you can destroy the instance to remove the listeners
menuInstance.destroy();
```
Note that the service also allows you to use a custom component as content which will receive `@data` and `@close` as args:
```javascript
const menuInstance = await this.menu.show(
document.querySelector(".my-span"),
{
component: MyComponent,
data: { foo: 1 }
}
)
```
## Toasts
Interacting with toasts is made only through the `toasts` service.
A default component is provided (DDefaultToast) and can be used through dedicated service methods:
- this.toasts.success({ ... });
- this.toasts.warning({ ... });
- this.toasts.info({ ... });
- this.toasts.error({ ... });
- this.toasts.default({ ... });
```javascript
this.toasts.success({
data: {
title: "Foo",
message: "Bar",
actions: [
{
label: "Ok",
class: "btn-primary",
action: (componentArgs) => {
// eslint-disable-next-line no-alert
alert("Closing toast:" + componentArgs.data.title);
componentArgs.close();
},
}
]
},
});
```
You can also provide your own component:
```javascript
this.toasts.show(MyComponent, {
autoClose: false,
class: "foo",
data: { baz: 1 },
})
```
Co-authored-by: Martin Brennan <mjrbrennan@gmail.com>
Co-authored-by: Isaac Janzen <50783505+janzenisaac@users.noreply.github.com>
Co-authored-by: David Taylor <david@taylorhq.com>
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
This PR introduces three new UI elements to Discourse codebase through an addon called "FloatKit":
- menu
- tooltip
- toast
Simple cases can be express with an API similar to DButton:
```hbs
<DTooltip
@label={{i18n "foo.bar"}}
@icon="check"
@content="Something"
/>
```
More complex cases can use blocks:
```hbs
<DTooltip>
<:trigger>
{{d-icon "check"}}
<span>{{i18n "foo.bar"}}</span>
</:trigger>
<:content>
Something
</:content>
</DTooltip>
```
You can manually show a tooltip using the `tooltip` service:
```javascript
const tooltipInstance = await this.tooltip.show(
document.querySelector(".my-span"),
options
)
// and later manually close or destroy it
tooltipInstance.close();
tooltipInstance.destroy();
// you can also just close any open tooltip through the service
this.tooltip.close();
```
The service also allows you to register event listeners on a trigger, it removes the need for you to manage open/close of a tooltip started through the service:
```javascript
const tooltipInstance = this.tooltip.register(
document.querySelector(".my-span"),
options
)
// when done you can destroy the instance to remove the listeners
tooltipInstance.destroy();
```
Note that the service also allows you to use a custom component as content which will receive `@data` and `@close` as args:
```javascript
const tooltipInstance = await this.tooltip.show(
document.querySelector(".my-span"),
{
component: MyComponent,
data: { foo: 1 }
}
)
```
Menus are very similar to tooltips and provide the same kind of APIs:
```hbs
<DMenu @icon="plus" @label={{i18n "foo.bar"}}>
<ul>
<li>Foo</li>
<li>Bat</li>
<li>Baz</li>
</ul>
</DMenu>
```
They also support blocks:
```hbs
<DMenu>
<:trigger>
{{d-icon "plus"}}
<span>{{i18n "foo.bar"}}</span>
</:trigger>
<:content>
<ul>
<li>Foo</li>
<li>Bat</li>
<li>Baz</li>
</ul>
</:content>
</DMenu>
```
You can manually show a menu using the `menu` service:
```javascript
const menuInstance = await this.menu.show(
document.querySelector(".my-span"),
options
)
// and later manually close or destroy it
menuInstance.close();
menuInstance.destroy();
// you can also just close any open tooltip through the service
this.menu.close();
```
The service also allows you to register event listeners on a trigger, it removes the need for you to manage open/close of a tooltip started through the service:
```javascript
const menuInstance = this.menu.register(
document.querySelector(".my-span"),
options
)
// when done you can destroy the instance to remove the listeners
menuInstance.destroy();
```
Note that the service also allows you to use a custom component as content which will receive `@data` and `@close` as args:
```javascript
const menuInstance = await this.menu.show(
document.querySelector(".my-span"),
{
component: MyComponent,
data: { foo: 1 }
}
)
```
Interacting with toasts is made only through the `toasts` service.
A default component is provided (DDefaultToast) and can be used through dedicated service methods:
- this.toasts.success({ ... });
- this.toasts.warning({ ... });
- this.toasts.info({ ... });
- this.toasts.error({ ... });
- this.toasts.default({ ... });
```javascript
this.toasts.success({
data: {
title: "Foo",
message: "Bar",
actions: [
{
label: "Ok",
class: "btn-primary",
action: (componentArgs) => {
// eslint-disable-next-line no-alert
alert("Closing toast:" + componentArgs.data.title);
componentArgs.close();
},
}
]
},
});
```
You can also provide your own component:
```javascript
this.toasts.show(MyComponent, {
autoClose: false,
class: "foo",
data: { baz: 1 },
})
```
Co-authored-by: Martin Brennan <mjrbrennan@gmail.com>
Co-authored-by: Isaac Janzen <50783505+janzenisaac@users.noreply.github.com>
Co-authored-by: David Taylor <david@taylorhq.com>
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
In #20135 we prevented invalid inputs from being accepted in category setting form fields on the front-end. We didn't do anything on the back-end at that time, because we were still discussing which path we wanted to take. Eventually we decided we want to move this to a new CategorySetting model.
This PR moves the require_topic_approval and require_reply_approval from custom fields to the new CategorySetting model.
This PR is nearly identical to #20580, which migrated num_auto_bump_daily, but since these are slightly more sensitive, they are moved after the previous one is verified.
Previous to this change when both `normalize_emails` and `hide_email_address_taken`
is enabled the expected `account_exists` email was only sent on exact email
matches.
This expands it so it also sends an email to the canonical email owner.
Why this change?
Currently, we do not have an easy way to test themes and theme components
using Rails system tests. While we support QUnit acceptance tests for
themes and theme components, QUnit acceptance tests stubs out the server
and setting up the fixtures for server responses is difficult and can lead to a
frustrating experience. System tests on the other hand allow authors to
set up the test fixtures using our fabricator system which is much
easier to use.
What does this change do?
In order for us to allow authors to run system tests with their themes
installed, we are adding a `upload_theme` helper that is made available
when writing system tests. The `upload_theme` helper requires a single
`directory` parameter where `directory` is the directory of the theme
locally and returns a `Theme` record.
When using ember-template-tag (.gjs), templates are much more interlinked with the JS file they're defined in. Therefore, attempting to override their template with a 'non-strict-mode' template doesn't make sense, and will likely lead to problems.
This commit skips any such overrides, and introduces a clear console warning. In theme/plugin tests, an error will be thrown during app boot.
Going forward, we aim to provide alternative APIs to achieve the customizations people currently implement with template overrides. (e.g. https://github.com/discourse/discourse/pull/23110)
While it's generally not recommended from a UX perspective, the DModal system is intended to allow multiple modals to be rendered simultaneously when using the declarative API. This wasn't working because `{{#in-element` was configured to replace the content of the container rather than append a new modal.
This commit fixes that and adds a test for the functionality.
By default this is linked to the `tests` boolean, which we disabled for Embroider builds in 96674859. We want deprecation-workflow features to be available in production builds, so let's enable it unconditionally.
Until now, we have allowed testing themes in production environments via `/theme-qunit`. This was made possible by hacking the ember-cli build so that it would create the `tests.js` bundle in production. However, this is fundamentally problematic because a number of test-specific things are still optimized out of the Ember build in production mode. It also makes asset compilation significantly slower, and makes it more difficult for us to update our build pipeline (e.g. to introduce Embroider).
This commit removes the ability to run qunit tests in production builds of the JS app when the Embdroider flag is enabled. If a production instance of Discourse exists exclusively for the development of themes (e.g. discourse.theme-creator.io) then they can add `EMBER_ENV: development` to their `app.yml` file. This will build the entire app in development mode, and has a significant performance impact. This must not be used for real production sites.
This commit also refactors many of the request specs into system specs. This means that the tests are guaranteed to have Ember assets built, and is also a better end-to-end test than simply checking for the presence of certain `<script>` tags in the HTML.
Mixins are considered deprecated by Ember, and cannot be applied to modern framework objects. Also, the coupling they introduce can make things very difficult to refactor.
This commit takes the state/logic from BulkTopicSelection and turns it into a helper object. This avoids it polluting any controllers/components its included in.
In future, the entire helper object can be passed down to child components so that they don't need to lookup controllers using the resolver. Those kinds of changes would involve changing some very heavily-overridden templates, so they are not included in this PR. It will likely be done as part of the larger refactor in https://github.com/discourse/discourse/pull/22622.
- Add data-embroider-ignore to all script tags which are not currently being compiled by embroider
- Ensure all remaining script tags are wrapped in `<discourse-chunked-script>` so that Rails will follow any renames which Embroider makes (e.g. when it adds fingerprints to filenames)
Discourse core now builds and runs with Embroider! This commit adds
the Embroider-based build pipeline (`USE_EMBROIDER=1`) and start
testing it on CI.
The new pipeline uses Embroider's compat mode + webpack bundler to
build discourse code, and leave everything else (admin, wizard,
markdown-it, plugins, etc) exactly the same using the existing
Broccoli-based build as external bundles (<script> tags), passed
to the build as `extraPublicTress` (which just means they get
placed in the `/public` folder).
At runtime, these "external" bundles are glued back together with
`loader.js`. Specifically, the external bundles are compiled as
AMD modules (just as they were before) and registered with the
global `loader.js` instance. They expect their `import`s (outside
of whatever is included in the bundle) to be already available in
the `loader.js` runtime registry.
In the classic build, _every_ module gets compiled into AMD and
gets added to the `loader.js` runtime registry. In Embroider,
the goal is to do this as little as possible, to give the bundler
more flexibility to optimize modules, or omit them entirely if it
is confident that the module is unused (i.e. tree-shaking).
Even in the most compatible mode, there are cases where Embroider
is confident enough to omit modules in the runtime `loader.js`
registry (notably, "auto-imported" non-addon NPM packages). So we
have to be mindful of that an manage those dependencies ourselves,
as seen in #22703.
In the longer term, we will look into using modern features (such
as `import()`) to express these inter-dependencies.
This will only be behind a flag for a short period of time while we
perform some final testing. Within the next few weeks, we intend
to enable by default and remove the flag.
---------
Co-authored-by: David Taylor <david@taylorhq.com>
We have the max_mentions_per_chat_message site settings; when a user tries
to mention more users than allowed, no one gets mentioned.
Chat messages may contain code-blocks with strings that look like mentions:
def foo
@bar + @baz
end
The problem is that the parsing code considers these as real mentions and counts
them when checking the limit. This commit fixes the problem.
`badge.save(["name", "description", "badge_type_id"])` api it was testing isn't a thing anymore.
Also: replaces `assert.expect(0)` with more useful assertions
We have a workaround so that currentUser/siteSettings/appEvents work properly on RestModel instances which are created without an owner. This is not ideal, but fixing this properly is not trivial. This commit improves the workaround to be more robust and support all service injections.
Currently, if the review queue has both a flagged post and a flagged chat message, one of the two will have some of the labels of their actions replaced by those of the other. In other words, the labels are getting mixed up. For example, a flagged chat message might show up with an action labelled "Delete post".
This is happening because when using bundles, we are sending along the actions in a separate part of the response, so they can be shared by many reviewables. The bundles then index into this bag of actions by their ID, which is something generic describing the server action, e.g. "agree_and_delete".
The problem here is the same action can have different labels depending on the type of reviewable. Now that the bag of actions contains multiple actions with the same ID, which one is chosen is arbitrary. I.e. it doesn't distinguish based on the type of the reviewable.
This change adds an additional field to the actions, server_action, which now contains what used to be the ID. Meanwhile, the ID has been turned into a concatenation of the reviewable type and the server action, e.g. post-agree_and_delete.
This still provides the upside of denormalizing the actions while allowing for different reviewable types to have different labels and descriptions.
At first I thought I would prepend the reviewable type to the ID, but this doesn't work well because the ID is used on the server-side to determine which actions are possible, and these need to be shared between different reviewables. Hence the introduction of server_action, which now serves that purpose.
I also thought about changing the way that the bundle indexes into the bag of actions, but this is happening through some EmberJS mechanism, so we don't own that code.
This adds a new secure_uploads_pm_only site setting. When secure_uploads
is true with this setting, only uploads created in PMs will be marked
secure; no uploads in secure categories will be marked as secure, and
the login_required site setting has no bearing on upload security
either.
This is meant to be a stopgap solution to prevent secure uploads
in a single place (private messages) for sensitive admin data exports.
Ideally we would want a more comprehensive way of saying that certain
upload types get secured which is a hybrid/mixed mode secure uploads,
but for now this will do the trick.
We're seeing a large number of log noise from this endpoint due to malicious scanners that are trying to send clever params and seeing if they can break something.
This change simply rescues any NoMethodError during parameter parsing and re-raises a Discourse::InvalidParameters exception, which will be caught and render a 400.
This patch adds a new shortcut to allow archiving private messages. When
on a private message page, just type `a` to archive it. Typing `a` on an
already archived message will move it back to inbox.
Chat review queue flags were missing the context message above the actions.
This is probably because the (reasonably complex) logic was somewhat hard-coded to posts. After some investigation I concluded we can reuse this logic with some small amendments.
Previously we were patching ember-cli so that it would split the test bundle into two halves: the helpers, and the tests themselves. This was done so that we could use the helpers for `/theme-qunit` without needing to load all the core tests. This patch has proven problematic to maintain, and will become even harder under Embroider.
This commit removes the patch, so that ember-cli goes back to generating a single `tests.js` bundle. This means that core test definitions will now be included in the bundle when using `/theme-qunit`, and so this commit also updates our test module filter to exclude them from the run. This is the same way that we handle plugin tests on the regular `/tests` route, and is fully supported by qunit.
For now, this keeps `/theme-qunit` working in both development and production environments. However, we are very likely to drop support in production as part of the move to Embroider.
The changes in e1d27400f5 slightly changed the sourcemap paths of our workbox assets. The sourcemaps now have the extension `.prod.map` instead of `prod.js.map`. However, since the version number of workbox didn't change, the directory digest remained the same, and so cached versions of the JS were pointing to the now-nonexistant map files.
This commit introduces a cachebusting constant which we can bump for these kinds of changes in future.
Our Ember build compiles assets into multiple chunks. In the past, we used the output from ember-auto-import-chunks-json-generator to give Rails a map of those chunks. However, that addon is specific to ember-auto-import, and is not compatible with Embroider.
Instead, we can switch to parsing the html files which are output by ember-cli. These are guaranteed to have the correct JS files in the correct place. A <discourse-chunked-script> will allow us to easily identify which chunks belong to which entrypoint.
In future, as we update more entrypoints to be compiled by Embroider/Webpack, we can easily introduce new wrappers.
Previously applied in 2c58d45 and reverted in 24d46fd. This version has been updated for subfolder support.
This will allow us to extend the deprecation period for this-property-fallback beyond Ember 4.x, to give more time for plugin developers to update their templates.
This fixes a regression introduced by an earlier change which changed `ReviewableQueuedPost`
record creation to use the more appropriate `target_created_by_id` for the author of the post
being queued instead of setting it to the creator(system user) of the `ReviewableQueuedPost` record.
They're both constant per-instance values, there is no need to store them
in the session. This also makes the code a bit more readable by moving
the `session_challenge_key` method up to the `DiscourseWebauthn` module.
Why this change?
As part of our ongoing efforts to security harden the Discourse
application, we are adding the `cross_origin_opener_policy_header` site setting
which allows the `Cross-Origin-Opener-Policy` response header to be set on requests
that preloads the Discourse application. In more technical terms, only
GET requests that are not json or xhr will have the response header set.
The `cross_origin_opener_policy_header` site setting is hidden for now
for testing purposes and will either be released as a public site
setting or be remove if we decide to be opinionated and ship a default
for the `Cross-Origin-Opener-Policy` response header.
1. Use `this.` instead of `{{action}}` where applicable
2. Use `{{fn}}` instead of `@actionParam` where applicable
3. Use non-`@` versions of class/type/tabindex/aria-controls/aria-expanded
4. Remove `btn` class (it's added automatically to all DButtons)
5. Remove `type="button"` (it's the default)
6. Use `concat-class` helper
When an upload fails and we don't have a specific error, we
show a generic one. But it's a little too generic -- it doesn't
even include the file name.
This commit shows the file name so you at least know which of your
uploads failed.
This tab doesn't really provide anything useful, and can be quite
confusing in some cases. Each plugin is already listed below, and
you can navigate to their settings from there. We want to move away
from the catch-all Plugins category for site settings. Core plugins are
not shown in this list as at 97a812f022.
Our Ember build compiles assets into multiple chunks. In the past, we used the output from `ember-auto-import-chunks-json-generator` to give Rails a map of those chunks. However, that addon is specific to ember-auto-import, and is not compatible with Embroider.
Instead, we can switch to parsing the html files which are output by ember-cli. These are guaranteed to have the correct JS files in the correct place. A `<discourse-chunked-script>` will allow us to easily identify which chunks belong to which entrypoint.
In future, as we update more entrypoints to be compiled by Embroider/Webpack, we can easily introduce new wrappers.
This commit fixes an issue from 2ecc8291e8
where the user sees an ugly plain #hashtag when sending a chat
message. Now, we add a basic placeholder that looks like the
cooked hashtag with a grey square, which is then filled in
once the "sent" message bus event for the message comes back,
and we do decorateCooked on the message to fill in the proper
hashtag details.
* DEV: Add rake command to help detect dead settings
Some Site Settings may still exist but are no longer being used in the
core discourse code or in related plugins. This rake task will help
identify any unused (aka: dead) settings by using the `rg` command to
search for them.
You can execute the rake task by using this command:
`LOAD_PLUGINS=1 bin/rails "site_settings:find_dead"`
* Add env variable, apply feedback
When flagging a chat message, and options included both notifying user and notifying staff, the modal was missing the separating text. This was happening because the #staffFlagsAvailable method was based on post flags, and the model for chat flags is slightly different. This fixes that by delegating to the relevant flag target object.
Streaming doesn't work for anonymous users because we scope updates to the current user. Since they can only see cached summaries, we can skip the streaming parameter and update it directly with the ajax response.
Previously, when dragging the timeline scroller, we would position it purely based on the current cursor position. That means that if you started dragging it from anywhere other than the centre, the scroller will 'jump' suddenly to centre itself on the cursor.
This commit measures the offset of your cursor when the drag starts, and maintains it throughout the drag. So for example, if you start dragging at the bottom of the scroller and drag one pixel up, the scroller will only move by 1px.
This should make the UX much smoother, especially on large topics.
When the 'loading slider' navigation indicator is enabled, and a connection is very slow, we `display: none` most of the page and display a spinner. The `still-loading` body class for this was being added in the `afterRender` step in the Ember runloop. This meant that, depending on the order they were scheduled, other `afterRender` jobs may run before it. This caused an issue with topic scroll locations because we would attempt to scroll to an element which was `display: none` at the point its position was calculated.
This commit moves the `still-loading` class manipulations to the `render` step of the runloop, which is technically more correct, and means that anything scheduled in the `afterRender` step is guaranteed to run without the `display: none` CSS.
https://meta.discourse.org/t/276305/29
This commit contains a few improvements:
* Use LinkTo instead of a button with a weird action referencing the
controller to navigate to the filtered settings for a plugin
* Add an AdminPlugin model with tracked properties and use that when
toggling the setting on/off and in the templates
* Make it so the Settings button for a plugin navigates to the correct
site setting category instead of always just going to the generic
"plugins" one if possible
Follow-up to #23199 in which we moved the "delete user" options under the relevant action menu for flagged post. This change does the same, but to queued posts.
This provides a `refresh()` function on Ember's public router service. The feature was added to Ember in v4.1.0, but this polyfill will allow us to start using it straight away under 3.28
As per discussion, we want to move the options to delete the user under the "Yes" menu of the review queue, since these options are often the most recommended, but also frequently missed because they are tucked away under their own menu.
In addition to the move, I removed the title case of the options so that it matches the other options in the "Yes" menu.
DEV: Display fuzzy site setting search results below direct matches
When searching for site settings, in the results under the ALL category
all the fuzzy search results were showing first followed by any direct
matches. This change adjusts that so that fuzzy searches show below
direct matches.
Fuzzy results are now also sorted based on their gap calculation in
ascending order.
* UX: update styling for related/suggested
This PR addresses state issues for icons of the Related & Suggested buttons, as well as well as fixes alignment issues for folding phones / tablets, wider mobile devices by moving styling to the desktop scss file; also replaces border with box-shadow.
It is hard to catch and debug potential bugs related to live updates of
user status (though, we haven't seen many such bugs so far). We have
this `console.warn` statement that should help us to catch one class of
such bugs:
70f1cc5552/app/assets/javascripts/discourse/app/models/user.js (L1384-L1389)
But in tests, we quite often operate with stubs of users that don't have ID,
and this warning create unnecessary noise. This PR disable the warning in
the testing environment.
This could happen after you had already change the separation mode and would cause unexpected bugs.
This PR also adds more tests around using switch buttons with chat.
Reverts e2705df and re-lands #23187 and #23219.
The issue was incorrect order of execution of Rails' `assets:precompile` task in our own precompilation stack.
Co-authored-by: David Taylor <david@taylorhq.com>
* UX: update styling for related/suggested
This PR fixes styling for previous related/suggested changes' positioning being off for topics and updates the active icon color by removing a line that changed its active color.
Short answer -- the problem is the video thumbnail generator & uploader
code added a couple of months back in f144c64e13.
It was implemented as another Mixin which overrides `this._uppyInstance`
when uploading the video thumbnail after the initial upload is complete,
which means the composer's `this._uppyInstance` value is overridden,
and it loses all of its preprocessors & upload code.
This is generally a problem with the Mixin based architecture that I
used for the Uppy code, which we need to remove at some point and
refacotr.
The most ideal thing to do here would be to convert this video thumbnail
code into an Uppy
[postprocessor](https://uppy.io/docs/uppy/#addpostprocessorfn) plugin,
which runs on each upload after they are complete. I started looking
into this, and the main hurdle here is adding support to tracking the
progress of postprocessors to
[ExtendableUploader](cf42466dea/app/assets/javascripts/discourse/app/mixins/extendable-uploader.js)
so that is out of scope at this time.
The fix here makes it so the ComposerVideoThumbnailUppy code is no
longer a Mixin, but acts more like a normal class, a pattern which
we have used in chat. I also clean up a lot of the thumbnail uploader
code and remove some unnecessary things.
Attempted to add a system spec, but video streaming does not work
in Chrome for Testing at this time, and it is needed for the
onloadedmetadata event.
This PR updates the styling for the related & suggested topics to distract less from the Reply buttons and clearly indicate its usage as tabs, also referred to as pills.
`Window`'s `resize` event was unreliable. You could shrink down the browser window so that the timeline would disappear but the progress element would not render to replace it.
This commit makes it rely on a media query listener instead so it 1. matches the css 2. fires only when that query result changes (perf win)
By default, the workbox-expiration plugin will not expire cache entries which include a `Vary` header in the response. This means that cached entries can build up until the browser storage quota is hit.
This commit introduces the `ignoreVary: true` option, so that deletion is performed correctly. This will only apply going forward, so this commit also bumps the cache version and deletes the old caches.
Ref https://github.com/GoogleChrome/workbox/issues/2206
This fixes the problem reported in
https://meta.discourse.org/t/custom-status-message-in-front-of-by-header-on-scroll/273320.
This problem can be reproduced with any tooltip created using the DTooltip
component or the createDTooltip function.
The problem happens because the trigger for tooltip on mobile is click, and for tooltip
to disappear the user has to click outside the tooltip. This is the default behavior
of tippy.js – the library we use under the hood.
Note that this PR fixes the problem in topics, but not in chat. I'm going to investigate and
address it in chat in a following PR.
To fix it for tooltips created with the createDTooltip function, I had to make a refactoring.
We had a somewhat not ideal solution there, we were leaking an implementation detail
by passing tippy instances to calling sides, so they could then destroy them. With this fix,
I would have to make it more complex, because now we need to also remove onScrool
handlers, and I would need to leak this implementation detail too. So, I firstly refactored
the current solution in 5a4af05 and then added onScroll handlers in fb4aabe.
When refactoring this, I was running locally some temporarily skipped flaky tests. Turned out
they got a bit outdated, so I fixed them. Note that I'm not unskipping them in this commit,
we'll address them separately later.
This commit adds some system specs to test uploads with
direct to S3 single and multipart uploads via uppy. This
is done with minio as a local S3 replacement. We are doing
this to catch regressions when uppy dependencies need to
be upgraded or we change uppy upload code, since before
this there was no way to know outside manual testing whether
these changes would cause regressions.
Minio's server lifecycle and the installed binaries are managed
by the https://github.com/discourse/minio_runner gem, though the
binaries are already installed on the discourse_test image we run
GitHub CI from.
These tests will only run in CI unless you specifically use the
CI=1 or RUN_S3_SYSTEM_SPECS=1 env vars.
For a history of experimentation here see https://github.com/discourse/discourse/pull/22381
Related PRs:
* https://github.com/discourse/minio_runner/pull/1
* https://github.com/discourse/minio_runner/pull/2
* https://github.com/discourse/minio_runner/pull/3
This PR changes how we track which lists are available for a topic and how we decide which is the active one. The new approach centralizes everything in the service, and exposes functions for adding/removing a list, which each calls via `did-insert/will-destroy` modifiers.
It makes it much easier to track and update state when navigated to another topic or PM, ensuring things get updated correctly.
71ff3417 removed the mobile-specific template for discovery/topics. It was noted that this would introduce an additional `<div class='show-more'>` wrapper around the new topic indicator on mobile, but I didn't realise that it would cause positioning problems on mobile.
This commit moves the desktop-specific CSS into the desktop SCSS file so that it does not apply to mobile.
Separate mobile templates are a pattern we're moving away from. They are not supported by Ember, and make things more difficult to develop/test. The differences between the mobile and desktop templates for `discovery/topics` are very minimal, so they can be easily integrated.
The only feature missing from the main template was the new 'list header controls' UI. This commit introduces that to the main template inside an `mobileView` conditional.
Key changes in behavior, many of which could be considered bug fixes, are:
- Mobile will now include 'redirected reason'
- Mobile will now include shared drafts
- Mobile will now include before-topic-list and after-topic-list Plugin Outlets
- Mobile will now have a `<div class="show-more">` wrapper around the 'new or updated' UI, to match desktop. This does not seem to cause any visual change.
Mobile-specific template overrides of `discovery/topics` will continue to function as before - this should not be a breaking change for any themes/plugins.
Mobile-specific templates for the topic list and topic-list-item remain in place.
Sometimes the fuzzy search would return too many site setting results
making it hard to find what you are searching for. This change still
allows for fuzzy searching but tightens up the criteria for being a
fuzzy match.
One example is searching for 'cheer', a term associated with a plugin,
previously returned ~55 search results. With this change it will return
~13 (Actual numbers depend on how many plugins your instance has).
Another example is searching for 'digest'. Previously returned ~37
results and now will return ~14.
Follow up to: e63e193a0a
See also: https://meta.discourse.org/t/276013
Transpiling to `/raw-templates` is important so that they are detected by the `eager-load-raw-templates` initializer. Prior to 16c6ab86 this wasn't a problem because all connector modules were being eager-loaded. Now that connectors are lazily loaded, it's critical that `eager-load-raw-templates` does the eager loading. This problem doesn't affect themes because they compile raw templates into an iife instead of a `define()` module.
Unfortunately we don't have any way to introduce automated testing for this part of our compilation pipeline. However, discourse-calendar will begin depending on this functionality imminently, so its tests will warn us of future regressions.
This commit introduces a 'shortcut' when rendering PluginOutlets which have no registered connectors. On my machine, this improves rendering performance of empty PluginOutlets by around 30-40% (tested by running tachometer on a `/latest` route with 600 plugin outlets).
In most cases, deleting a user from outside the review UI will also delete any pending reviewables for that user. This was not working in some cases, e.g. for reviewables created due to "fast typer" violations.
This was happening because UserDestroyer only automatically resolves flagged posts.
After this change, in addition to existing checks, look for ReviewablePost where the post was created by the user and reject them if present.
* Minor style adjustments
* Removes "all" count because it's redundant to the count on New
* Updates generic class names with -- modifier to follow BEM and help avoid class name collisions
* Hides the toggle when bulk select is enabled (the UI ends up being too busy)
Manipulating theme module paths means that the paths you author are not the ones used at runtime. This can lead to some very unexpected behavior and potential module name clashes. It also meant that the refactor in 16c6ab8661 was unable to correctly match up theme connector js/templates.
While this could technically be a breaking change, I think it is reasonably safe because:
1. Themes are already forced to use relative paths when referencing their own modules (since they're namespaced based on the site-specific id). The only time this might be problematic is when theme tests reference modules in the theme's main `javascripts` directory
2. For things like components/services/controllers/etc. our custom Ember resolver works backwards from the end of the path, so adding `discourse/` in the middle will not affect resolution.
This is a bug that happens only when the current date is less than 90 days from a date on which the time zone transitions into or out of Daylight Savings Time.
In these conditions, bulk invites show the time of day of their expiration as being 1 hour later than the current time.
Whereas it should match the time of day the invite was generated.
This is because the server has not been using the user's timezone in calculating the expiration time of day. This PR fixes issue by considering the user's timezone when doing the date math.
https://meta.discourse.org/t/bulk-invite-logic-to-generate-expire-date-bug/274689
`ReviewableQueuedPost` got refactored a while back to use the more
appropriate `target_created_by` for the user of the post being queued
instead of `created_by`. The change was not extended to the `DELETE
/review/:id` endpoint leading to error responses for a user attempting
to deleting their own queued post.
This fix extends the `Reviewable` lookup implementation in
`ReviewablesController#destroy` and Guardian implementation to account
for this change.
Previously we were discovering plugin outlets by checking first for dedicated template files, and then looking for classes to match them. This doesn't work for components which are entirely defined in JS (e.g. those authored with gjs, or those which are re-exports of a colocated component).
This commit refactors our detection logic to look for both class and template modules in a single pass. It also refactors things so that the modules themselves are required lazily when needd, rather than all being loaded during app boot.
This PR adds a new toggle to switch the (new) /new list between showing topics with new replies (a.k.a unread topics), new topics, or everything mixed together.
When hiding a post (essentially updating hidden, hidden_at, and hidden_reason_id) our callbacks are running the whole battery of post validations. This can cause the hiding to fail in a number of edge cases. The issue is similar to the one fixed in #11680, but applies to all post validations, none of which should apply when hiding a post.
After some code reading and discussion, none of the validations in PostValidator seem to be relevant when hiding posts, so instead of just skipping unique check, we skip all post validator checks.
After fbe0e4c we always pass a block into these methods.
So yield inside the export methods works and there is no need
anymore to wrap them into enumerators.
Prior to this fix we would always re-set `this.attrs` with `this.attrs` when defined, which is both wasteful but also dangerous as `this.attrs` can possibly error when mutated.
Previously, we had a `showFooter` boolean on the application controller which would be set true/false in various routes by different routes/controllers. A global `routeWillChange` hook would set it `false` before every route transition, and the destination route/controller would have to set it `true` for the footer to show correctly.
This commit replaces that with a new 'declarative' system. Instead of having to set the value true/false manually, UIs which need the footer to be hidden can simply include the `{{hide-application-footer}}` helper in their template when needed. The helper/service will automatically keep track of all the current invocations of that helper, and only show the footer when there are 0 invocations.
This significantly simplifies things, and removes the need for many observers and controller injections, both of which are considered 'code smells' in modern Ember applications.
So we have to order by calling `find_each(order: :desc)`.
Note that that will order rows by Id, not by `last_match_at`
as we tried before (though that didn't work).
What is the problem here?
When transiting between `/filter` routes with different `q` query
params, the input field is not updating to include the values in the `q`
query param. This was because we were setting the value of the input
field in the constructor of the controller but controllers are actually
singletons in Ember so setting the value of the input field is only done
once when the controller is initialised.
What is the fix here?
Instead of setting the value of the input field in the controller, we
set the value in the `setupController` hook in the route file.
* scrub non-a html tags from tag descriptions on create, strips all tags from tag description when displayed in tag hover
* test for tag description links
* UX: basic render-tag test
* UX: fix linting
* UX: fix linting
* fix broken tests
* Update spec/models/tag_spec.rb
Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
* UX: use has_sanitizable_fields instead of has_scrubbable_fields to ensafen tag.description
---------
Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
104baab5 fixed double-counted pageviews for the initial page load. Under the default 'loading slider' implementation, that resolved all the known problems.
However, under the 'loading spinner', there is an additional problem. In 'spinner' mode, each navigation within the JS app involves transitioning to an intermediate 'loading' route. Previously, this intermediate state was being treated as a separate page by the app, and so any ajax requests fired during it would be counted as a distinct pageview. One known case of this is the `/presence/get` request which is made when logged-in users visit a topic.
This commit updates the logic to ignore 'intermediate' transitions, and introduces regression tests for both the 'spinner' and 'slider' modes.
In this commit 2.5 years ago, variables for showOnUserCard and showOnProfile were removed, but we still used them in the component. e29605b
This corrects the variable names and adds a test to confirm the text is now shown.
* DEV: allow `formTemplateIds` to be explicitly set via the composer service
* DEV: allow to skip the configured form template via the composer service
Our code assumed the content_range interval was inclusive, but they are open-ended due to Postgres' [discrete range types](https://www.postgresql.org/docs/current/rangetypes.html#RANGETYPES-DISCRETE), meaning [1,2] will be represented as [1,3).
It also fixes some flaky tests due to test data not being correctly setup and the registry not being resetted after each test.
This resolves the issue in #23064.
This issue arises because we need to produce the trees for the
auxilary bundles in `ember-cli-build.js` to pass these trees as
argument to `app.toTree()`. In order to produce these trees, the
code internally need to set up babel, which deep-clones the addons'
babel configs.
When using `@embroider/macros`, the addon's babel config includes a
`MacrosConfig` object which is not supposed to be touched until the
configs are "finalized". In a classic build, the finalization step
happens when `app.toTree()` is called. In Embroider, this happens
somewhere deeper inside `CompatApp`.
We need to produce these auxilary bundle trees before we call
`app.toTree()` or before constructing `CompatApp` because they
need to be passed as arguments to these functions. So this poses a
tricky chicken-and-egg timing issue. It was difficult to find a
workaround for this that works for both the classic and Embroider
build pipeline.
Of all the internal addons that uses the auxilary bundle pattern,
this only affets `pretty-text` as it is (for now, at least) the
only addon that uses `@embroider/macros`.
Taking a step back, the only reason (for now, at least) it was
introduced was for the loader shim for the `xss` package. This
package is actually used inside the lazily loaded markdown-it
bundle. However, we didn't have a better way to include the dep
into the lazy bundle directly, so it ends up going into the main
addon tree, and, inturns, the discourse core bundle.
In core's main loader shim manifest, we already have an entry for
`xss`. This was perhaps a mistake at the time, but it doesn't make
a difference – as mentioned above, `xss` needs to be included into
the main bundle anyway.
So, for now, the simpliest solution is to avoid `@embroider/macros`
in these internal addons for the time being. Ideally we would soon
absorb these back into core as lazily loaded (`import()`-ed) code
managed by Webpack when we fully switch over to Embroider.
The new modal API removed the `#discourse-modal` id from the wrapper element, which meant that select-kit couldn't properly detect when it was inside a modal. This commit updates the detection to use `.fixed-modal` which will match both legacy and modern modals.
When we receive the stream parameter, we'll queue a job that periodically publishes partial updates, and after the summarization finishes, a final one with the completed version, plus metadata.
`summary-box` listens to these updates via MessageBus, and updates state accordingly.
The OpenComposer mixin comes from a time before we had a composer service. As well as being a general cleanup/refactor, this commit aims to removes interlinking between composer APIs and the discovery-related controllers which are being removed as part of #22622.
In summary, this commit:
- Removes OpenComposer mixin
- Adds and updates composer service APIs to support everything that `openComposer` did
- Updates consumers to call the composer service directly, instead of relying on the mixin (either directly, or via a route-action which bubbled up to some parent)
- Deprecates composer-related methods on `DiscourseRoute` and on the application route
Prior to this fix the user tip was rendered with panels and interfering with widget code. I suspect it was causing the widget node (revamped-hamburger-menu-wrapper) to not be removed, as a result clickOutside would be called two times, negating the effect of the click.
This fix is just rendering the tip in a different node, preventing the interference, it shouldn't impact behavior as the positioning is absolute.
Currently when we decide we're going to drop a column in the future we just mark it with a TODO comment and add it to ignored_columns. This makes it instantly unavailable, and we mostly forget about the TODO in the end. 😬
This change adds a HasDeprecatedColumns concern which offers a little bit more flexibility. We can still simulate the old behaviour by setting drop_from to the current version, but we can also set it to a future version, causing it to raise a deprecation warning until then if used.
This commit moves the calendar date and time picker shown in
the local dates modal into a core component that can be reused
in other places. Also add system specs to make sure there isn't
any breakages with this feature, and a section to the styleguide.
The original motivation for this change was to avoid mutating imported modules (by stubbing imported functions in tests)
Other than that it's a good practice to place code like this in services, especially (although not the case here) if it requires access to other services or controller.
The previous version of ember-on-resize-modifier depended on
ember-modifier@^3.2.7 while discourse had ember-modifier@^4.1.0.
As far as Yarn is concerned, it can accomplish this with:
node_modules
...
ember-modifier 4.1.0
...
ember-on-resize-modifier 1.1.0
...
ember-modifier 3.2.7
...
...
This does NOT work!
In a classic build everything is compiled down to AMD modules and
at runtime there can only be one uniquely named "ember-modifier"
module. When we have duplicates, depending on activation ordering,
one of them will randomly win.
In practice, it seems like ember-modifier 3.2.7 had "won" in the
current build, and we are shipping it to production, you can find
these modules in vendor.js like:
```js
;define("ember-modifier/-private/class/modifier", /* ... */, function(/* ... */) {
/* the 3.2.7 version with deprecations, etc */
})
/* ... */
;define("ember-modifier/index", /* ... */)
```
However, ember-auto-import also "found" the 4.1.0 version and in
one of the chunk.app.js:
```js
d('ember-modifier', /* ... */, function() { return __webpack_require__(/*! ember-modifier */ 227); });
```
...and in one of the chunk.vendors.js...
```js
/* 227 */
/*!****************************************************!*\
!*** ../node_modules/ember-modifier/dist/index.js ***!
\****************************************************/
/***/ ((__unused_webpack_module, __webpack_exports__, __webpack_require__) => {
"use strict";
/* ...the 4.1.0 version... */
}),
```
So, in practice:
* We are brining both copies into the production build
* The 3.2.7 modules are available in the AMD loader as "ember-modifier/..."
* But 4.1.0 modules are available in the AMD loader as "ember-modifier"
* Because mostly it's consumed as `import ... from "ember-modifier";`, the
latter end up actually winning
* Because the newer code is compatible enough, and the deprecated features
are unused, it seems to work ok..?
But in the Embroider build, ember-auto-import doesn't emit those shims
anymore. It does process most of the core modules through Webpack so the
imports get correctly wired up to the 4.1.0 as expected, as they no longer
go through/need the runtime AMD loader.js.
The older 3.2.7 copy is _still_ shipped in the vendor bundle and registered
the same, but not "stomped over" by the EAI shims anymore. Our manual shims
(#22703, merged yesterday) are more "polite" and check `require.has(...)`
before defining the module, and since `require.has(...)` check for the
`/index` alias and returns `true`, our shim does not stomp the 3.2.7 modules
either.
So then, when our "auxilary bundles" (admin, plugins, etc) tries to import
`"ember-modifier", they get the 3.2.7 version.
There is a case when developer would like to go to separated mode but not show switch panel buttons. We need additional functions to show/add buttons to support this case.
* REFACTOR: Glimerify topic summarization widgets.
Simplifies all the logic for generating/regenerating summaries and expanding/collapsing the summary box. It makes streaming easier to implement since now we can subscribe to message bus directly from the component.
* Update app/assets/javascripts/discourse/app/components/summary-box.hbs
Co-authored-by: David Taylor <david@taylorhq.com>
* Update app/assets/javascripts/discourse/app/components/summary-box.hbs
Co-authored-by: David Taylor <david@taylorhq.com>
* Update app/assets/javascripts/discourse/app/components/summary-box.hbs
Co-authored-by: David Taylor <david@taylorhq.com>
---------
Co-authored-by: David Taylor <david@taylorhq.com>
This adds a new `loaderShim()` function to ensure certain modules
are present in the `loader.js` registry and therefore runtime
`require()`-able.
Currently, the classic build pipeline puts a lot of things in the
runtime `loader.js` registry automatically. For example, all of
the ember-auto-import packages are in there.
Going forward, and especially as we switch to the Embroider build
pipeline, this will not be guarenteed. We need to keep an eye on
what modules (packages) our "external" bundles (admin, wizard,
markdown-it, plugins, etc) are expecting to be present and put
them into the registry proactively.
There is no decorateCooked equivalent for small action posts,
so we need to manually call decorateHashtags when there is a custom
message for small action posts in order for the hashtags to get
their coloured icon/square.
Bug when you click fast on the switch panel button. It is happening because we are not waiting for the transition to finish before update state.
In addition, unused currentPanel property was removed.
This commit removes any logic in the app and in specs around
enable_experimental_hashtag_autocomplete and deletes some
old category hashtag code that is no longer necessary.
It also adds a `slug_ref` category instance method, which
will generate a reference like `parent:child` for a category,
with an optional depth, which hashtags use. Also refactors
PostRevisor which was using CategoryHashtagDataSource directly
which is a no-no.
Deletes the old hashtag markdown rule as well.
This fixes:
- a regression from 30c152c, where navigating to a topic's last reply
via keyboard would lose track of the topic when returning to the topic
list
- an issue where if a topic's last post is a small post, navigating to it
via keyboard would not focus the post
Co-authored-by: David Taylor <david@taylorhq.com>
Adds a padding-bottom to the wrapper to avoid cutting the message on the mobile app and sets a max-width to align with the timeline on the desktop.
Fixes a bug on mobile where we updated the preference, but the user had a single list.
When Ember rendering fails for some reason, `this.header` will be undefined. This causes site-header to raise an error, which often gets printed to the console more obviously than the actual root cause.
This commit makes site-header fail more gracefully in this situation, to avoid it being a red-herring during development/debugging.
This function was previously expecting multiple services to be injected on any class that uses it. This kind of hidden requirement leads to some very difficult-to-debug situations, so this commit updates the function to lookup all its required services inline.
The filter-mode mixin was previously serving two distinct purposes via a complex arrangement of getters/setters:
1. To calculate a filterMode, given values for category, filterType and noSubcategories
2. To calculate a filterType, given a filterMode
This commit splits the mixin into two functions, and updates all call sites to use them instead of the mixin.
This makes more sense (and is likely faster) than redefining the entire route for every call to `buildTopicRoute`. Also moves the top-specific logic into the route rather than injecting it via an initializer.
Similar to d5107d1aba
This brings the theme development experience (via the discourse_theme cli) closer to the experience of making javascript changes in Discourse core/plugins via Ember CLI. Whenever a change is made to a non-css theme field, all clients will be instructed to immediately refresh via message-bus.
Why this change?
In production, this appeared as a small hotspot as where we're calling
`poster.name_and_description` twice which in turns makes a method call
to `I18n.t`. When we're rendering a topic list with many topics and each
topic has many posters, this repeated and unnecessary method call
quickly adds up.
In #20135 we prevented invalid inputs from being accepted in category setting form fields on the front-end. We didn't do anything on the back-end at that time, because we were still discussing which path we wanted to take. Eventually we decided we want to move this to a new CategorySetting model.
This PR moves the num_auto_bump_daily from custom fields to the new CategorySetting model.
In addition it sets the default value to 0, which exhibits the same behaviour as when the value is NULL.
This commit makes some visual tweaks to the admin panel plugin list, and introduces functional 'toggle switches' for admins to enable/disable plugins more easily.
Co-authored-by: Jordan Vidrine <jordan@jordanvidrine.com>
We need these Ember framework class overrides to be applied before anything attempts to extend them. An initializer is too late, because initializer files may `import` a module which defines classes which extend the framework classes.
In the past this rarely mattered because Ember's legacy `SomeObject.extend` is quite forgiving - it will respect changes made to `SomeObject` right up until the first `.create()` call. However, the native class syntax (`class extends SomeObject`) will 'freeze' `SomeObject` as soon as the class is defined.
Edit community section button is hidden in secondary/more section. However, when there are no secondary links, then more section is not shown. In that case, we should still display an edit button for admins, so they can edit the section.
This commit introduces the :push_notification event and deprecates :post_notification_alert.
The old :post_notification_alert event was not triggered when pushing chat notifications and did not respect when the user was in "do not disturb" mode.
The new event fixes these issues.
Should fix an iOS regression in f5e8e73. iOS does not pull up the keyboard if the `.focus()` call is delayed by a rendering timeout or an asynchronous ajax call. This PR adds earlier `.focus()` calls if the input element is present.
This was forgotten during the work in 22991bba44
This revealed two differences we were depending on: the merged `actions` hash (re-implemented on the service), and a couple of calls to `composer.send` (now removed)
* FIX Add 'Ignored' flags to Moderator Activity report
The Moderator Activity query didn’t include the number of deferred flags in the Flags Reviewed totals. As this number is designed to reflect how many flags a moderator has seen, reviewed, and made a judgement on, the Ignored ones should also be included.
* Apply suggestions from code review
---------
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
1. recent css regression related to modal upgrade
2. autofocus and on-enter regressions
3. array related linting issue (reliance on Ember's firstObject/lastObject)
This change prevents event bubbling for the Escape key on all modals. Currently when we close the modal using the Escape key, all other event listeners attached will also be triggered (such as closing the chat drawer if it's open).
When we use the escape key to exit lightbox for images within a chat channel, it also closes the chat drawer due to event bubbling (since both lightbox and chat use an event listener on the escape key).
This change prevents event bubbling when using the escape key within lightbox, which means that it will close the lightbox but won't close the chat drawer.
The escape key is used as a shortcut to escape the Discourse Lightbox. However, some browsers also use the escape key to exit fullscreen mode.
This change is to allow escaping the lightbox when browser is in fullscreen mode, while preventing any behavior associated with the Escape key (such as exiting fullscreen). This has to be done on the keydown event, as this means we can handle our logic and then preventDefault before the browser tries to exit fullscreen.
We only want to scroll to the top for successful transitions. If a transition is aborted (e.g. when clicking a chat link when chat is in drawer mode) then we should maintain the existing scroll location.
We were proxying all `/assets/*` requests through to the origin. In local development that was fine, because Rails was able to serve files from the `dist/` directory. But when proxying to a remote origin, we want the local ember-cli to serve its own JS assets
We never propagated the preference change because of the early return, meaning lists listening to it never got to decide if they had to remain hidden.
Also, we don't want to track the preference when there's a single list, as the user didn't choose to see it.
This PR updates how we display related and suggested topics on mobile and desktop. It adds a new `PluginOutlet` specifically designed for adding new topic lists, which automatically work if following the same conventions as the ones inside `<MoreTopics />`.
While we display lists side by side on desktop, we only display one in mobile. You can switch to another one by clicking on the nav pills, and we'll automatically save your preference for next time.
In e1d27400f5 we started running the splash-screen JS through terser, which removed the trailing newline from the `sourceMappingURL` line.
Adding a reliable end-to-end test for this isn't possible because our testing environment doesn't use terser.
FEATURE: Only approved flags for post counters
* Why was this change necessary?
The counters for flagged posts in the user's profile and user index from
the admin view include flags that were rejected, ignored or pending
review. This introduces unnecessary noise. Also the flagged posts
counter in the user's profile includes custom flags which add further
noise to this signal.
* How does it address the problem?
* Modifying User#flags_received_count to return posts with only approved
standard flags
* Refactoring User#number_of_flagged_posts to alias to
User#flags_received_count
* Updating the flagged post staff counter hyperlink to navigate to a
filtered view of that user's approved flagged posts to maintain
consistency with the counter
* Adding system tests for the profile page to cover the flagged posts
staff counter
