* FIX: participating users statistics...
... was (mis-)counting
- bots
- anonymous users
- suspended users
There's now a "valid_users" function that holds the AR query for valid users and which is used in all "users", "active_users", and "participating_users" queries.
Internal ref - t/138435
- Add concurrency when running on multisite clusters (default 10, configurable via THEME_UPDATE_CONCURRENCY env)
- Add a version cache for the duration of the rake task. This avoids duplicating work when many sites in the cluster have the same theme installed, and it is already up-to-date
- Updates output to be more concurrent friendly (all `puts`, no `print`)
* FEATURE: Create rake for db migration in plugins
before the dev-xp was clunky, we had to create a migration file in core and
move it to the plugin.
Now we automated this process, we still create the migration file in core
but the rake task will move it to the plugin.
the usage is:
```
rake plugin:generate_migration[plugin_name,migration_name,migration_args]
rake plugin:generate_migration[discourse-automation,add_group_id_to_automation_rule,"group_id:integer"]
```
* DEV: change rake to be a generator for plugin migrations
* DEV: trying to add extra class option to migration generator
* DEV: revert to have only `plugin_migration_generator`
* DEV: remove rake task for plugin migration creation
* DEV: remove migration_generator.rb
* DEV: remove if because options with `plugin_name` will always be true
We want to allow lightboxing of smaller images, even if they are below the minimum size for image thumbnail generation.
This change sets a minimum threshold of 100 x 100 pixels for triggering the lightbox.
---------
Co-authored-by: Régis Hanol <regis@hanol.fr>
This patch improves the custom `array` type available in contracts.
It’s now able to split strings on `|` on top of `,`, and to be more
consistent, it also tries to cast the resulting items to integers.
Theme modifiers can now be defined as theme settings, this allows for
site operators to override behavior of theme modifiers.
New syntax is:
```
{
...
"modifiers": {
"modifier_name": {
"type": "setting",
"value": "setting_name"
}
}
}
```
This also introduces a new theme modifier for serialize_post_user_badges. Name of badge must match the name of the badge in the badges table. The client-side is updated to load this new data from the post-stream serializer.
Co-authored-by: David Taylor <david@taylorhq.com>
Constants should always be only assigned once. The logical OR assignment
of a constant is a relic of the past before we used zeitwerk for
autoloading and had bugs where a file could be loaded twice resulting in
constant redefinition warnings.
- limits security key deletes to second factor keys
- also deletes backup codes (lingering backup codes break login flow entirely)
* Add spec for rake task to disable 2FA for a user
Currently, when the MessageFormat compiler fails on some translations,
we just have the raw output from the compiler in the logs and that’s not
always very helpful.
Now, when there is an error, we iterate over the translation keys and
try to compile them one by one. When we detect one that is failing, it’s
added to a list that is now outputted in the logs. That way, it’s easier
to know which keys are not properly translated, and the problems can be
addressed quicker.
---
The previous implementation of this patch had a bug: it wasn’t handling
locales with country/region code properly. So instead of iterating over
the problematic keys, it was raising an error.
If a plugin's JS fails to load for some reason, most commonly
ad blockers, the entire admin interface would break. This is because
we are adding links to the admin routes for plugins that define
them in the sidebar.
We have a fix for this already in the plugin list which shows a warning
to the admin. This fix just prevents the broken link from rendering
in the sidebar if the route is not valid.
* Add migrations to ensure password hash is synced across users & user_passwords
* Persist password-related data in user_passwords instead of users
* Merge User#expire_old_email_tokens with User#expire_tokens_if_password_changed
* Add post deploy migration to mark password-related columns from users table as read-only
* Refactored UserPassword#confirm_password? and changes required to accommodate hashing the password after validations
In our production environment, we have been seeing Sidekiq processes
getting stuck randomly when a USR1 signal is sent to the Unicorn master
process. We have not been able to identify the root cause of why the
Sidekiq process gets stuck. We however noticed that when the Unicorn
master process receives a USR1 signal, it will reopen the logs for the
Unicorn master process first before sending a USR1 signal for the
Unicorn worker processes to reopen the logs. We figured that we should
do the same for the Sidekiq process as well when a USR1 signal.
In this commit, we introduce an arbitrary delay of 1 second before we
the Sidekiq process reopens its log files so as to allow enough time for the Unicorn
master to finish reopening it logs first.
We also do not send reopen logs for the Sidekiq process if the `DISCOURSE_LOG_SIDEKIQ`
env is not present because there is no need to reopen any logs.
There have been too many flaky tests as a result of leaking state in
Redis so it is easier to resolve them by ensuring we flush Redis'
database.
Locally on my machine, calling `Discourse.redis.flushdb` takes around
0.1ms which means this change will have very little impact on test
runtimes.
While using `OpenStruct` is nice, it’s generally not a very good idea as
it usually leads to performance problems.
The `OpenStruct` source code even says basically to avoid it.
Since the context object is crucial in our services, this patch replaces
`OpenStruct` with a custom implementation instead.
When a post has some replies, and the user click on the button to show them, we would load ALL the replies. This could lead to DoS if there were a very large number of replies.
This adds support for pagination to these post replies.
Internal ref t/129773
FIX: Duplicated parent posts
DEV: Query refactor
XHR requests are handled differently by the application and the
responses do not have any preloaded data so the cache key needs to
differntiate between those requests.
Remove emoji.clear cache calls as data.js.es6.erb hasn't existed in a while.
Emoji data is now compiled separately via javascript rake tasks.
Skip db and redis precompilation when no db is present
Currently, when the MessageFormat compiler fails on some translations,
we just have the raw output from the compiler in the logs and that’s not
always very helpful.
Now, when there is an error, we iterate over the translation keys and
try to compile them one by one. When we detect one that is failing, it’s
added to a list that is now outputted in the logs. That way, it’s easier
to know which keys are not properly translated, and the problems can be
addressed quicker.
My theory is that there were nil entries (that we were filtering out) that then changed and we weren't resetting them properly.
(the failure no longer repro'd in 30 CI runs in this PR)
Currently in services, the `contract` step is only used to define where
the contract will be called in the execution flow. Then, a `Contract`
class has to be defined with validations in it.
This patch allows the `contract` step to take a block containing
validations, attributes, etc. directly. No need to then open a
`Contract` class later in the service.
It also has a nice side effect, as it’s now easy to define multiples
contracts inside the same service. Before, we had the `class_name:`
option, but it wasn’t really useful as you had to redefine a complete
new contract class.
Now, when using a name for the contract other than `default`, a new
contract will be created automatically using the provided name.
Example:
```ruby
contract(:user) do
attribute :user_id, :integer
validates :user_id, presence: true
end
```
This will create a `UserContract` class and use it, also putting the
resulting contract in `context[:user_contract]`.
Previously admins could still click on topics when `suppress_secured_categories_from_admin` was set
This change improves the block so admins without permission will not be allowed to click through till they add themselves to appropriate groups
Keep in mind this setting is a quality of life setting and not a SECURITY
setting, admins have an infinite way of bypassing visiblity limits
This commit introduces a feature that allows an admin to delete a user's
associated account. After deletion, a log will be recorded in staff
actions.
ref=t/136675
With the current implementation, a service step can be written as:
```ruby
def my_step(a_default_value: 2)
…
end
```
That’s a pattern we want to avoid as default values (if needed) should
be probably defined in a contract.
This patch makes a service raise an exception if a default value is
encountered.
Currently, when certain search terms are provided, this can lead to
`Search.need_segmenting?` raising an error because it makes `URI#path`
to return `nil` instead of a string.
This patch forces a cast to string so it won’t raise anymore.
At a point we split FileStore#download into two methods, one which returns nil (#download) and one which raises an exception (#download!) if there's a download error.
When we did this we added a deprecation warning to the #download method.
It has now been over a year and I have checked all relevant internal cases, so I think it's about time we remove this warning.
This will help to enforce a consistent pattern for creating service
actions.
This patch also namespaces actions and policies, making everything
related to a service available directly in
`app/services/<concept-name>`, making things more consistent at that
level too.
In TopicController, in addition to ensure_can_move_posts!, we also
checked if the topic is private message in this line:
```ruby
raise Discourse::InvalidAccess if params[:archetype] == "private_message" && !guardian.is_staff?
```
However, this was not present in `guardian.can_move_posts?`. As a result,
the frontend topic view got an incorrect serialized result, thinking
that TL4 could move the private message post. In fact, once they tried
to move it, they got the `InvalidAccess` error message.
This commit fixes that TL4 will no longer sees the "move to" option in
the "select post" panel for a private message.
The user directory (`/u`) excludes inactive and silenced users from the list, so for the sake parity, it makes sense to also exclude those users from the /about page stats.
Internal topic: t/70928.
- fetch models inside services
- validate `user_id` in contracts
- use policy objects
- extract more logic to actions
- write specs for services and action
The user option 'hide_profile_and_presence' is necessary to figure out
if the user status has to be displayed or not. In order to avoid N+1s
generated by `include_status?` method, both `user_status` and
`user_option` relations have to be included.
`track_sql_queries` only returned queries that were executed by
ActiveRecord. All queries executed through DB.exec, DB.query and others
were not returned.
We were running into errors running `ember build` on machines with high
CPU counts. It was then noted that `thread-loader`, which embroider uses, defaults to spinning
up x workers where x is number of physical CPU cores - 1. That is
probably too much so we set out to find out an optimial count to set for
the `JOBS` env which embroider will use to set the number of
`thread-loader` workers.
I first built an image using the following Dockerfile.
```
FROM discourse/base:release
RUN cd /var/www/discourse && sudo -EH -u discourse bundle exec rake plugin:install_all_official
RUN cd /var/www/discourse && sudo -EH -u discourse bundle exec rake assets:precompile:prereqs
```
I then ran the following command on my M3 Max Macbook Pro that has 14
phyisal CPU cores.
```
for j in 1 2 4 8 14; do echo "JOBS=$j"; time docker run --rm -it -e JOBS=$j test:latest /bin/bash -c "su discourse -c 'cd /var/www/discourse && bundle exec rake assets:precompile:build'"; done
```
These are the results I got:
```
JOBS=1 0.04s user 0.03s system 0% cpu 1:01.92 total
JOBS=2 0.04s user 0.02s system 0% cpu 42.605 total
JOBS=4 0.04s user 0.02s system 0% cpu 37.012 total
JOBS=8 0.04s user 0.02s system 0% cpu 35.199 total
JOBs=14 0.04s user 0.02s system 0% cpu 37.941 total
```
We think JOBS=2 is a good default when the `JOBS` env has not been set.
Anything above just consumes more resources for little benefit.
Previously we were opening the file handles, then putting them in a queue for upload. If that queue grows too large, we can hit a maximum open files limit.
This commit opens the file handle 'just in time', so the maximum number of open handles is equal to the upload concurrency (20).
