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 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.
Followup to eea74e0e32. Site settings
which are a list without a list_type should also have the _map
extension added which returns an array based on split("|").
For example:
```
SiteSetting.post_menu_map
=> ["read", "like"]
```
Tries to fix the composer upload spec by making the upload
slow enough to allow clicking the Cancel button, and improves
generally the API for CDP network changes.
When navigating around, we make ajax requests with a parameter like `?filter=latest`. This results in the TopicQuery being set up with `filter: "latest"` as a string. The logic introduced in fd9a5bc0 checks for equality with `:latest` and `:unseen` symbols, which didn't work correctly in this situation
This commit makes the logic detect both strings and symbols, and adds a spec for the behaviour.
This adds support for oneboxing WEBP and AVIF images in posts and fixing
oneboxing fixes download remote images for those formats too.
Reported in https://meta.discourse.org/t/-/276433?u=falco
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>
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 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
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.
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.
What is the problem here?
The `selenium-webdriver` gem is responsible for downloading the
right version of the `chromedriver` binary and it downloads it into the
`~/.cache/selenium` folder. THe problem here is that when a user runs `bin/turbo_rspec spec/system`
for the first time, all of the processes will try to download the
`chromedriver` binary to the same path at the same time and will lead
to concurrency errors.
What is the fix here?
Before running any RSpec suite, we first check if the `.cache/selenium`
folder is present. If it is not present, we use a file system lock to
download the `chromedriver` binary such that other processes that runs
after will not need to install the `chromedriver` binary.
The long term fix here is to get `selenium-manager` to download the `chromedriver` binary to a unique path for each
process but the `--cache-path` option for `selenium-manager` is currently not supported in `selenium-webdriver`.
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.
We can no long user Webdriver - SeleniumHQ/selenium#11066. Bumping selenium-webdriver did the trick, as well as manually setting the user_agent for mobile system specs. Unsure what changed to make this necessary, but it is necessary to get the app to boot in mobile view.
* 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>
The category feature that automatically closes topics does it silently
This amends it so `rake topics:apply_autoclose` which does retroactive
closing will also do so silently.
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.
