Float-kit elements (menus/tooltips) are positioned where they should be by setting an inline `left` property in JavaScript when they're rendered. For some reasons, we also set `left: 0` on float-kit elements here:
25d9927785/app/assets/stylesheets/common/float-kit/d-menu.scss (L11-L15)
This property is overridden by the inline property that the library sets in JavaScript. However, in RTL mode, all of our scss files are flipped where everything left becomes right and vice versa. In this case, the `left: 0` property in the scss file above becomes `right: 0`.
This results in a conflict specific to RTL mode where both the `left` and `right` properties are defined on the same absolute-positioned element; the `right` property will always be set to 0 because it comes from the (flipped) scss file above, and the inline `left` property will be set to some px amount determined in JavaScript.
The `right` property will take precedence over the inline `left` property due to the page being right-to-left (source: https://developer.mozilla.org/en-US/docs/Web/CSS/right#description) and this causes float-kit elements to incorrectly always stick to the right.
This commit removes the `left: 0` property altogether for float-kit elements from our scss files. It's not clear from git history why the property was added, and removing it doesn't seem to cause any issues.
Meta topic: https://meta.discourse.org/t/positioning-issues-with-rtl-locales-after-recent-updates/280220?u=osama
Before this commit, this output is possible:
```
Rewriting all occurrences of STRING1 to STRING2
THIS TASK WILL REWRITE DATA, ARE YOU SURE (type YES)
WILL RUN ON ALL 1 DBS
```
Which, when run from a script, might lead one to believe that YES was
automatically inserted into STDIN and the script is continuing.
Turns out this isn't the case so the obvious expectation is broken.
This commit swaps the order of those last lines to make it clear that
the script is blocked on input.
Why this change?
The `tests` workflow runs many jobs. Each job when ran is given a unique
id. Since a job can be re-run, we do not want the test reports to
override each other so we differentiate it further by the `job_id` given
by `${{ github.job }}`.
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the min_trust_level_to_allow_user_card_background site setting to user_card_background_allowed_groups.
Nothing of note here. This is used in exactly one place, and there's no fallout.
This validator is used for site settings where one or more groups are to be input.
At the moment this validator just checks that the value isn't blank. This PR adds a validation for the existence of the groups passed in.
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the tl4_delete_posts_and_topics site setting to delete_all_posts_and_topics_allowed_groups.
This one is a bit different from previous ones, as it's a boolean flag, and the default should be no group. Pay special attention to the migration during review.
Why this change?
Pull requests can introduce flaky tests into the mix and we do not want
to be hiing that during the pull request process. While this does mean
builds for PR will be less stable than the `main` branch without
retries, we do not foresee this to be a problem long term since the
monitoring of flaky tests on the `main` branch will mean that the number
of flaky tests will eventually be reduced.
What does this change do?
1. Introduce the `DISCOURSE_TURBO_RSPEC_RETRY_AND_LOG_FLAKY_TESTS` env
variable which will initialize `TurboTest::Runner` with the `retry_and_log_flaky_tests`
kwarg set to true when set.
2. Change the tests workflow run to set `DISCOURSE_TURBO_RSPEC_RETRY_AND_LOG_FLAKY_TESTS` only when
the build type is `backend` or `system` and the `github.ref_name` is
`main`.
* FEATURE: core code, tests for feature to allow backups to removed based on a time window
* FEATURE: getting tests working for time-based backup
* FEATURE: getting tests running
* FEATURE: linting
It's very unlikely that something will be introduced which works under Ember 5 and not Ember 3. To reduce GitHub actions costs, flakiness, and visual noise, let's cut down the matrix so we're only using Ember 3 for the 'core frontend' job. All others can run under Ember 5.
This commit adds an additional toggle to our safe-mode system. When enabled, it will cause all deprecation messages to become exceptions. This gives admins a way to test their themes/plugins against upcoming Discourse changes without needing to use the browser developer tools.
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the min_trust_to_flag_posts site setting to flag_post_allowed_groups.
Note: In the original setting, "posts" is plural. I have changed this to "post" singular in the new setting to match others.
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the min_trust_to_edit_post site setting to edit_post_allowed_groups.
The old implementation will co-exist for a short period while I update any references in plugins and themes.
This change converts the min_trust_to_create_topic site setting to
create_topic_allowed_groups.
See: https://meta.discourse.org/t/283408
- Hides the old setting
- Adds the new site setting
- Add a deprecation warning
- Updates to use the new setting
- Adds a migration to fill in the new setting if the old setting was
changed
- Adds an entry to the site_setting.keywords section
- Updates tests to account for the new change
- After a couple of months, we will remove the min_trust_to_create_topicsetting entirely.
Internal ref: /t/117248
This change converts the allow_uploaded_avatars site setting to uploaded_avatars_allowed_groups.
See: https://meta.discourse.org/t/283408
Hides the old setting
Adds the new site setting
Adds a deprecation warning
Updates to use the new setting
Adds a migration to fill in the new setting if the old setting was changed
Adds an entry to the site_setting.keywords section
Updates tests to account for the new change
After a couple of months, we will remove the allow_uploaded_avatars setting entirely.
Internal ref: /t/117248
What motivated this change?
Our builds on Github actions have been extremely flaky mostly due to system tests. This has led to a drop in confidence
in our test suite where our developers tend to assume that a failed job is due to a flaky system test. As a result, we
have had occurrences where changes that resulted in legitimate test failures are merged into the `main` branch because developers
assumed it was a flaky test.
What does this change do?
This change seeks to reduce the flakiness of our builds on Github Actions by automatically re-running RSpec tests once when
they fail. If a failed test passes subsequently in the re-run, we mark the test as flaky by logging it into a file on disk
which is then uploaded as an artifact of the Github workflow run. We understand that automatically re-runs will lead to
lower accuracy of our tests but we accept this as an acceptable trade-off since a fragile build has a much greater impact
on our developers' time. Internally, the Discourse development team will be running a service to fetch the flaky tests
which have been logged for internal monitoring.
How is the change implemented?
1. A `--retry-and-log-flaky-tests` CLI flag is added to the `bin/turbo_rspec` CLI which will then initialize `TurboTests::Runner`
with the `retry_and_log_flaky_tests` kwarg set to `true`.
2. When the `retry_and_log_flaky_tests` kwarg is set to `true` for `TurboTests::Runner`, we will register an additional
formatter `Flaky::FailuresLoggerFormatter` to the `TurboTests::Reporter` in the `TurboTests::Runner#run` method.
The `Flaky::FailuresLoggerFormatter` has a simple job of logging all failed examples to a file on disk when running all the
tests. The details of the failed example which are logged can be found in `TurboTests::Flaky::FailedExample.to_h`.
3. Once all the tests have been run once, we check the result for any failed examples and if there are, we read the file on
disk to fetch the `location_rerun_location` of the failed examples which is then used to run the tests in a new RSpec process.
In the rerun, we configure a `TurboTests::Flaky::FlakyDetectorFormatter` with RSpec which removes all failed examples from the log file on disk since those examples are not flaky tests. Note that if there are too many failed examples on the first run, we will deem the failures to likely not be due to flaky tests and not re-run the test failures. As of writing, the threshold of failed examples is set to 10. If there are more than 10 failed examples, we will not re-run the failures.
Ability to automatically generate migration when site setting name is changed.
Example usage: `rails generate site_setting_rename_migration site_description contact_email`
Now that we're using native `import()`, our main JS bundles might not even be parse-able by older browsers. In that case, `I18n` will never be defined, and so we need to account for that situation in the browser-update code.
