### Why?
Before, all flags were static. Therefore, they were stored in class variables and serialized by SiteSerializer. Recently, we added an option for admins to add their own flags or disable existing flags. Therefore, the class variable had to be dropped because it was unsafe for a multisite environment. However, it started causing performance problems.
### Solution
When a new Flag system is used, instead of using PostActionType, we can serialize Flags and use fragment cache for performance reasons.
At the same time, we are still supporting deprecated `replace_flags` API call. When it is used, we fall back to the old solution and the admin cannot add custom flags. In a couple of months, we will be able to drop that API function and clean that code properly. However, because it may still be used, redis cache was introduced to improve performance.
To test backward compatibility you can add this code to any plugin
```ruby
replace_flags do |flag_settings|
flag_settings.add(
4,
:inappropriate,
topic_type: true,
notify_type: true,
auto_action_type: true,
)
flag_settings.add(1001, :trolling, topic_type: true, notify_type: true, auto_action_type: true)
end
```
When upgrading to Rails 7.1, we had some problems because we were using
several tagged loggers at the same time. They were all added to the main
broadcast logger shipped with Rails, but the Rails 7.1 codebase contains
a bug making a request being run as many times as there are tagged loggers.
The fix was to use the code from the Rails 7.2 codebase.
This patch adds a small spec to ensure the behavior will stay the proper
one in the future.
This change ensures native push notifications respect the site setting for push_notification_time_window_mins. Previously only web push notifications would account for the delay, now we can bring more consistency between Discourse in browser vs Hub, by applying the same delay strategy to both forms of push notifications.
### Why?
Before, all flags were static. Therefore, they were stored in class variables and serialized by SiteSerializer. Recently, we added an option for admins to add their own flags or disable existing flags. Therefore, the class variable had to be dropped because it was unsafe for a multisite environment. However, it started causing performance problems.
### Solution
When a new Flag system is used, instead of using PostActionType, we can serialize Flags and use fragment cache for performance reasons.
At the same time, we are still supporting deprecated `replace_flags` API call. When it is used, we fall back to the old solution and the admin cannot add custom flags. In a couple of months, we will be able to drop that API function and clean that code properly. However, because it may still be used, redis cache was introduced to improve performance.
To test backward compatibility you can add this code to any plugin
```ruby
replace_flags do |flag_settings|
flag_settings.add(
4,
:inappropriate,
topic_type: true,
notify_type: true,
auto_action_type: true,
)
flag_settings.add(1001, :trolling, topic_type: true, notify_type: true, auto_action_type: true)
end
```
Adds a new statistics (hidden from the UI, but available via the API) that tracks daily participating users.
A user is considered as "participating" if they have
- Reacted to a post
- Replied to a topic
- Created a new topic
- Created a new PM
- Sent a chat message
- Reacted to a chat message
Internal ref - t/131013
This commit continues on work laid out by 6039b513fe to redesign the /about page. In this commit, we add sections for showing the site admins and moderators.
The lists of admins and moderators display the 10 most recently seen admins/moderators, with a button to display the rest of admins or moderators. Admins or moderators that have not logged in to the site in the last year will not be shown. Clicking on an admin's or moderator's name/avatar will show their user card.
In development, Ember raises an error when previously-used values are updated during a render. This is to avoid 'backtracking', where parts of templates have to be re-rendered multiple times. In general, this kind of pattern should be avoided, and Ember's warning helps us do that.
However, for the deprecation warning banner, it is quite reasonable for some rendering to trigger a deprecation, and thereby require the global-notice to be re-rendered. We can use our `DeferredTrackedSet` to achieve that. Its `.add` method will delay adding an item to the Set until after the current render has completed.
Very similar to move up/down flag problem fixed here - https://github.com/discourse/discourse/pull/28272
Those are the steps to toggle the flag:
1. click toggle - `saving` CSS class is added;
2. request to backend;
3. `saving` CSS class is removed.
And check if the flag was toggle was:
```ruby
def has_saved_flag?(key)
has_css?(".admin-flag-item.#{key}.saving")
has_no_css?(".admin-flag-item.#{key}.saving")
end
```
If the save action is very fast, then the saving class is removed before the first check.
Therefore I decided to invert it, and once action is finished add `saved` CSS class.
Then we can have a quick positive check:
```ruby
def has_saved_flag?(key)
has_css?(".admin-flag-item.#{key}.saved")
end
```
Currently, in the list controller, when encountering an unsafe redirect
error, a 404 is rendered. The problem is that it’s done in a way that it
also logs a fatal error (because a `Discourse::NotFound` exception was
raised inside a `rescue_from` block).
This patch addresses that issue by simply rendering a 404 without
raising any error.
This commit adds a `MiniSchedulerLongRunningJobLogger` class which will
poll every 60 seconds for mini_scheduler jobs which are stuck. When it
detects that a job is stuck, it will log a warning message with the
current backtrace of the thread that is executing the job.
Note that for scheduled jobs which are executed at a frequency of less
than 30 minutes, we will log when the job has been executing for 30
minutes.
For scheduled jobs executed at a frequency of less than 2 hours, we will
log when the job has been executing for a duration greater than its
specified frequency.
For scheduled jobs executed at a frequency greater than 2 hours, we will
log as long as the job has been executing for more than 2 hours.
Those are the steps to move the flag:
1. open menu;
2. click move up - `saving` CSS class is added;
3. request to backend;
4. `saving` CSS class is removed.
To check if the action was finished we are using this method:
```
def move_up(key)
open_flag_menu(key)
find(".admin-flag-item__move-up").click
has_saved_flag?(key)
self
end
def has_saved_flag?(key)
has_css?(".admin-flag-item.#{key}.saving")
has_no_css?(".admin-flag-item.#{key}.saving")
end
```
However, sometimes specs were failing with `expected to find CSS ".admin-flag-item.spam.saving" but there were no matches`
I think that the problem is with those 2 lines:
```
find(".admin-flag-item__move-up").click
has_closed_flag_menu?
```
If the save action is very fast, then the `saving` class is removed before the first check.
Therefore, to determine that the move action is finished, I am checking if the menu is closed.
Currently, when a badly named category slug is provided, it can lead to
an infinite redirect.
This patch addresses the issue by properly unescaping `request.fullpath`
so the path is successfully rewritten and the redirect happens as
expected.
Currently to handle stub topics after merging, there are only options to (1) never delete a stub topic and (2) delete a stub topic after X amount of days. This adds the option to immediately delete a stub topic upon merge.
---------
Co-authored-by: Mark VanLandingham <markvanlan@gmail.com>
Co-authored-by: Renato Atilio <renato@discourse.org>
This commit continues on work laid out by 6039b513fe to redesign the /about page. In this commit, we add the site age and a section on the right hand side to show site activities/statistics such as topics, posts, sign-ups, likes etc.
Following a recent refactor, some methods from `FlagSettings` have been
renamed (`custom_types` -> `additional_message_types`). The
`PostActionType` model was using `custom_types` but when the renaming
was done, it was renamed to `with_additional_message` instead of
`additional_message_types`, which under the right circumstances will
raise an error.
Admin can create up to 50 custom flags. It is limited for performance reasons.
When the limit is reached "Add button" is disabled and backend is protected by guardian.
This commit patches `Net::HTTP` to reduce the default timeouts of 60
seconds when we are processing a request. There are certain routes in
Discourse which makes external requests and if the proper timeouts are
not set, we risk having the Unicorn master process force restarting the
Unicorn workers once the `30` seconds timeout is reached. This can
potentially become a vector for DoS attacks and this commit is aimed at
reducing the risk here.
We support a low-level construct called "inline checks", which you can use to register a problem ad-hoc from within application code.
Problems registered by inline checks never show up in the admin dashboard, this is because when loading the dashboard, we run all realtime checks and look for problems. Because of an oversight, we considered inline checks to be "realtime", causing them to be run and clear their problem status.
To fix this, we don't consider inline checks to be realtime, to prevent them from running when loading the admin dashboard.
This change is mainly a refactor of the desktop notifications service to improve readability and have standardised values for tracking state for current user in regards to the Notification API and Push API.
Also improves readability when handling push notification jobs, especially in scenarios where the push_notification_time_window_mins site setting is set to 0, which will allow sending push notifications instantly.
We were writing theme-transpiler JS files to the filesystem on a per-process basis, and then immediately reading them back in. Plus, there was no cleanup mechanism, so the tmp directory would grow indefinitely.
This commit refactors things so that the `build.js` script outputs the theme-transpiler source to stdout. That way, we can read it directly into the process, and then into mini-racer, without needing to go via the filesystem. No cleanup required!
In production, the theme-transpiler is still cached in a file during `assets:precompile`
In the formkit conversion in 2ca06ba236
we missed setting a type for the UppyImageUploader for badges. Also,
we were not passing down the `image_url` as form data, so when we used
`data.image` for that field the badge was not updating in the UI after
page loads and the image URL was not loading for preview.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Before this commit, running `rspec --seed 22953 --format documentation spec/requests/admin/site_texts_controller_spec.rb:191 spec/lib/freedom_patches/translate_accelerator_spec.rb:109` will fail.
Setting `I18n.config.available_locales` is equivalent to hard coding the
locales for the entire process. It should not be set so that `I18n` will
fallback to `backend.locales`.
Sometimes the backtrace is quite big for failing specs, this env var
(RSPEC_EXCLUDE_NOISE_IN_BACKTRACE) can be set to
1 to remove backtrace from anything but spec or application code in
rspec. This makes it easier to see where the actual failure is
coming from, most of the time all the gem paths are noise.
When creating a shared draft, we're recording topic view stats on the draft and then pass those on when the draft is published, conflating the actual view count.
This fixes that by not registering topic views if the topic is a shared draft.
When `SiteSetting.review_every_post` is true and the category `require_topic_approval` system creates two reviewable items.
1. Firstly, because the category needs approval, the `ReviewableQueuePost` record` is created - at this stage, no topic is created.
2. Admin is approving the review. The topic and first post are created.
3. Because `review_every_post` is true `queue_for_review_if_possible` callback is evaluated and `ReviewablePost` is created.
4. Then `ReviewableQueuePost` is linked to the newly generated topic and post.
At the beginning, we were thinking about hooking to those guards:
```
def self.queue_for_review_if_possible(post, created_or_edited_by)
return unless SiteSetting.review_every_post
return if post.post_type != Post.types[:regular] || post.topic.private_message?
return if Reviewable.pending.where(target: post).exists?
...
```
And add something like
```
return if Reviewable.approved.where(target: post).exists?
```
However, because the callback happens in point 3. before the `ReviewableQueuePost` is linked to the `Topic`, it was not possible.
Therefore, when `ReviewableQueuePost` is creating a `Topic`, a new option called `:reviewed_queued_post` is passed to `PostCreator` to avoid creating a second `Reviewable`.
Currently, descriptions for flag types aren’t interpolated, returning
`%{base_path}` in their string, for example. This breaks the navigation
on the sites.
The behavior changed probably because of an upgrade of Ruby, as two
hashes were passed to `I18n.t` (`vars` and `default`) without using the
splat operator.
When using `Discourse.cache.fetch` with an expiry, there's a potential for a race condition due to how we read the data from redis.
The code used to be
```ruby
raw = redis.get(key) if !force
entry = read_entry(key) if raw
return entry if raw && !(entry == :__corrupt_cache__)
```
with `read_entry` defined as follow
```ruby
def read_entry(key)
if data = redis.get(key)
Marshal.load(data)
end
rescue => e
:__corrupt_cache__
end
```
If the value at "key" expired in redis between `raw = redis.get` and `entry = read_entry`, the `entry` variable would be `nil` despite `raw` having a value.
We would then proceed to return `entry` (which is `nil`) thinking it had a value, when it didn't.
The first `redis.get` can be skipped altogether and we can rely only on `read_entry` to read the data from redis. Thus avoiding the race condition and removing the double read operations.
Internal ref - t/132507
* SECURITY: Update default allowed iframes list
Change the default iframe url list to all include 3 slashes.
* SECURITY: limit group tag's name length
Limit the size of a group tag's name to 100 characters.
Internal ref - t/130059
* SECURITY: Improve sanitization of SVGs in Onebox
---------
Co-authored-by: Blake Erickson <o.blakeerickson@gmail.com>
Co-authored-by: Régis Hanol <regis@hanol.fr>
Co-authored-by: David Taylor <david@taylorhq.com>
Followup 4aea12fdcb
In certain config areas (like About) we want to be able
to fetch specific site settings by name. In this case,
sometimes we need to be able to fetch hidden settings,
in cases where a config area is still experimental.
Splitting out a different endpoint for this purpose
allows us to be stricter with what we return for config
areas without affecting the main site settings UI, revealing
hidden settings before they are ready.
