These controller tests are passing locally and in CI, but are failing the build when run in parallel.
I managed to recreate the failures by running the entire suite with turbo_spec and the right seed locally. After these changes, the parallel suite passes locally as well. 🤞
`new_in_category` was using `first` instead of `limit`
This meant it gets an array and that means that you can not operate on it easily in a modifier.
This ensures we always give the modifier a relation, with the notable exception of suggested topics.
We're changing the default of hide_email_address_taken to true. This is a trade-off we want to make, as it prevents account enumeration with minimal impact on legitimate users. If you forget you have an account and try to sign up again with the same e-mail you'll receive an e-mail letting you know.
Add flag reason filter and improve handling of deleted content in review queue
This commit enhances the review queue with several key improvements:
1. Adds a new "Reason" filter to allow filtering flags by their score type
2. Improves UI for deleted content by:
- Adding visual indication for deleted posts (red background)
- Properly handling deleted content visibility for staff (category mods can not see deleted content)
3. Refactors reviewable score type handling for better code organization
4. Adds tests for trashed topics/posts visibility
This change will help moderators more efficiently manage the review queue by
being able to focus on specific types of flags and better identify deleted
content.
This commit introduces <NotificationsTracking /> which is a wrapper component around <DMenu /> which replaces the select-kit component <TopicNotificationsButton />.
Each tracking case has its dedicated component:
- topic -> `<TopicNotificationsTracking />`
- group -> `<GroupNotificationsTracking />`
- tag -> `<TagNotificationsTracking />`
- category -> `<CategoryNotificationsTracking />`
- chat thread -> `<ThreadNotificationsTracking />`
Previously, theme hbr files were compiled to an IIFE, which would be executed before the app is booted. That is causing silenced deprecations to be printed, because the deprecation-workflow isn't set up when the IIFE is run.
This commit updates the theme compiler so that it matches the ember-cli-based raw-hbs compiler. Templates are output to normal modules, which will then be loaded by the existing `eager-load-raw-templates` initializer. This runs after the app has started booting.
* DEV: add db consistency check for UserEmail
* DEV: add db consistency check for UserAvatar
* DEV: ignore inconsistent data related to user avatars when deciding whether to rebake old posts
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
---------
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Currently, there are two ways (kind of) for accessing `params` inside a
service:
- when there is no contract or it hasn’t been reached yet, `params` is
just the hash that was provided to the service. To access a key, you
have to use the bracket notation `params[:my_key]`.
- when there is a contract and it has been executed successfully,
`params` now references the contract and the attributes are accessible
using methods (`params.my_key`).
This patch unifies how `params` exposes its attributes. Now, even if
there is no contract at all in a service, `params` will expose its
attributes through methods, that way things are more consistent.
This patch also makes sure there is always a `params` object available
even when no `params` key is provided to the service (this allows a
contract to fail because its attributes are blank instead of having the
service raising an error because it doesn’t find `params` in its context).
This commit adds a new column full_move to the moved_posts table. This is useful to look back at history and determine if a whole topic was moved or partial.
This commit also adds an apply_modifier to skip the creation of the moved posts small action.
This patch aims to improve the steps inspector output:
- The service class name is displayed at the top.
- Next to each step is displayed the time it took to run said step.
- Steps that didn’t run are hidden.
- `#inspect` automatically outputs the error when it is present.
It doesn't make much sense to have the content of a `<details>` in an excerpt so I replaced them with "▶ summary" instead.
That way, they can't be (ab)used in user cards for example.
Reference - https://meta.discourse.org/t/335094
The new name may be too long for the bookmarks.name column and raise an
exception. This changes allows the remapper to truncate the new value to
fit (truncates to 100 characters).
The test was flaky and failing with the following errors:
```
Failure/Error:
klass
.connection
.select_raw(relation.arel) do |result, _|
result.type_map = DB.type_map
result.nfields == 1 ? result.column_values(0) : result.values
end
NoMethodError:
undefined method `select_raw' for nil
./lib/freedom_patches/fast_pluck.rb:60:in `pluck'
./vendor/bundle/ruby/3.3.0/gems/activerecord-7.2.2.1/lib/active_record/relation/calculations.rb:354:in `pick'
./app/models/web_crawler_request.rb:27:in `request_id'
./app/models/web_crawler_request.rb:31:in `rescue in request_id'
./app/models/web_crawler_request.rb:26:in `request_id'
./app/models/web_crawler_request.rb:19:in `write_cache!'
./app/models/concerns/cached_counting.rb:135:in `block (3 levels) in flush_to_db'
./vendor/bundle/ruby/3.3.0/gems/rails_multisite-6.1.0/lib/rails_multisite/connection_management/null_instance.rb:49:in `with_connection'
./vendor/bundle/ruby/3.3.0/gems/rails_multisite-6.1.0/lib/rails_multisite/connection_management.rb:21:in `with_connection'
./app/models/concerns/cached_counting.rb:134:in `block (2 levels) in flush_to_db'
./app/models/concerns/cached_counting.rb:124:in `each'
./app/models/concerns/cached_counting.rb:124:in `block in flush_to_db'
./lib/distributed_mutex.rb:53:in `block in synchronize'
./lib/distributed_mutex.rb:49:in `synchronize'
./lib/distributed_mutex.rb:49:in `synchronize'
./lib/distributed_mutex.rb:34:in `synchronize'
./app/models/concerns/cached_counting.rb:120:in `flush_to_db'
./app/models/concerns/cached_counting.rb:187:in `perform_increment!'
./app/models/web_crawler_request.rb:15:in `increment!'
./lib/middleware/request_tracker.rb:74:in `log_request'
./lib/middleware/request_tracker.rb:409:in `block in log_later'
./lib/scheduler/defer.rb:125:in `block in do_work'
./vendor/bundle/ruby/3.3.0/gems/rails_multisite-6.1.0/lib/rails_multisite/connection_management/null_instance.rb:49:in `with_connection'
./vendor/bundle/ruby/3.3.0/gems/rails_multisite-6.1.0/lib/rails_multisite/connection_management.rb:21:in `with_connection'
./lib/scheduler/defer.rb:119:in `do_work'
./lib/scheduler/defer.rb:105:in `block (2 levels) in start_thread'
```
This was due to running the defer thread in an async manner which is
actually no representative of the production environment. It also
revealed a spot in our code base where writes are happening in a GET
request which can cause requests to fail if ActiveRecord is in readonly
mode.
The directory items controller specs that have a search param were not
matching how things worked in production. In a non-test environment the
UserSearch class depends on the `user_search_data` table being
populated, so the tests I corrected now use this table as well to match
reality.
Also added a new test to match the 20 user limit for search results that
currently exists. This 20 user limit is on the line between a bug and a
feature but it is how it is currently working so we should document
that. We have plans to increase this limit and it has been documented
here: https://meta.discourse.org/t/296485
This PR is a no-op and only changes the tests.
Co-authored-by: brrusselburg <25828824+brrusselburg@users.noreply.github.com>
When a post containing an apostrophe (') is being cooked, the apostrophe is being converted to the "typographic" version (’) (because we enable markdown-it's **typographer** mode by default in Discourse)
When you select text that contains such apostrophe and then try to save your fast edit, it fails miserably without any error.
That's because when you select text from the DOM, it uses the cooked version which has the typographic apostrophe.
When you save your fast edit, we fetch the raw version of the post, which has the "regular" apostrophe. Thus doing `raw.replace(selectedText, editedText)` doesn't work because `raw` has the regular apostrophe but `selectedText` has the typographic apostrophe.
Since it's somewhat complicated to handle all typographic characters, we would basically have to reverse the process done in `custom-typographer-replacements.js`, we instead bail out and show the composer when we detect such character in the selection.
Internal ref - t/143836
This reverts commit 766ff723f8.
Ensure that we create the sidekiq log file first before opening it for
logging. This avoids any issue of the log file not being present when we
initialize an instance of the `Logger`.
Some pages like new/edit item should not display admin header. New attribute called `@shouldDisplay` was added.
As a proof of concept, the flags page was updated.
In 806e37aaec, I improved the conflict handling when editing a post to account for title and tags.
This fixes an edge cases when a topic has a hidden tag the current editor can't see. When they submit their edit, we automatically add the hidden tags before checking with the tags stored in the database.
Reported in https://meta.discourse.org/t/341375
We've seen in some communities abuse of user profile where bios and other fields are used in malicious ways, such as malware distribution. A common pattern between all the abuse cases we've seen is that the malicious actors tend to have 0 posts and have a low trust level.
To eliminate this abuse vector, or at least make it much less effective, we're making the following changes to user profiles:
1. Anonymous, TL0 and TL1 users cannot see any user profiles for users with 0 posts except for staff users
2. Anonymous and TL0 users can only see profiles of TL1 users and above
Users can always see their own profile, and they can still hide their profiles via the "Hide my public profile" preference. Staff can always see any user's profile.
Internal topic: t/142853.
Currently when copy an OP to another topic, the link is to the topic that wasn't moved. The notification should instead be to the new topic the OP was moved to -- we have duplicate logic already for this but first post creation get special treatment, and this applies the same treatment.
Follow-up from this commit - 9b8af0ea9f
Adds helpful data into MovedPost records for later lookup. ALSO fixes notifications for freeze_original to point to the newly created post, not the moved post.
PostMover has a new option called freeze_original implemented in this commit. It was previously unexposed in the controller. This PR permits the param in the controller, and passes it into PostMover.
Also, this applies a value transformer for move/merge payload options. In addition a plugin outlet in the move post modal. This allows plugins to add content to the modal, which can modify the payload (and use the freeze_original argument for example)
Followup c7e471d35a
It is currently possible to add a bundle (which is a collection
of actions used for a dropdown on the client) for a reviewable
via actions.add_bundle and then never add any actions to it.
This causes the client to explode, as seen in the referenced
commit, because of the way our store expects to resolve objects
referenced by ID that are passed down by the serializer, which
then causes Ember to have an unrecoverable render error.
Fixing this on the serializer level is not really possible because
of all the ActiveModel::Serializer magic that serializes
objects by ID reference when doing things like has_many.
`Reviewable#actions_for` is a better place to do this anyway,
because this is the main location where the bundles and actions
are built for every action via the serializer.
Followup aca6c462a6
Remove the warning message if DISCOURSE_INCLUDE_GEMS_IN_RSPEC_BACKTRACE
is not set for now while we decide whether we want to include
this or not, it's a little in-your-face.
Currently only system flags are translated. When we send message to the user that their post was deleted because of custom flag, we should default to custom flag name.
Previously when attempting to edit a globally shadowed setting, the
error message was not very helpful, it said "You are not allowed to
change hidden settings". This commit changes the error message to
reflect the actual problem, which is that the setting is shadowed by
a global setting via ENV var.
Followup:
* https://github.com/discourse/discourse/pull/28160
* https://github.com/discourse/discourse/pull/25921
In the previous PRs we added 2 environent variables
to control backtrace output for errors in rspec,
`RSPEC_EXCLUDE_NOISE_IN_BACKTRACE`, and
`RSPEC_EXCLUDE_GEMS_IN_BACKTRACE`
These largely do the same thing, and we want to enable
that behaviour by default.
This commit consolidates them into one env var,
`DISCOURSE_INCLUDE_GEMS_IN_RSPEC_BACKTRACE`, which is
disabled by default, meaning gem backtraces will not
be shown in rspec backtraces by default.
Also for the request spec use case with `RspecErrorTracker`,
we now show an indicator of how many lines were hidden from
the backtrace e.g. "...(21 framework line(s) excluded)",
and for this and the normal rspec backtrace exclusion we
show a warning if `DISCOURSE_INCLUDE_GEMS_IN_RSPEC_BACKTRACE`
is not enabled.
Ted Johansson
Sam
Natalie Tay
Krzysztof Kotlarek
Joffrey JAFFEUX
David Taylor
Kelv
Mark VanLandingham
Renato Atilio
Loïc Guitaut
Régis Hanol
Bianca Nenciu
Sérgio Saquetim
Gary Pendergast
Alan Guo Xiang Tan
Isaac Janzen
Blake Erickson
Osama Sayegh
Martin Brennan
Juan David Martínez Cubillos