The config.file_watcher option still exists but it's no longer included
in the default environtment file. Since we don't use it, we're removing
it.
The config.assets.assets.debug option is no longer true by default [1],
so it isn't included anymore.
The config.active_support.deprecation option is now omitted on
production in favor of config.active_support.report_deprecations, which
is false by default. I think it's OK to keep it this way, since we check
deprecations in the development and test environments but never on
production environments.
As mentioned in the Rails upgrade guide, sprockets-rails is no longer a
rails dependency and we need to explicitly include it in our Gemfile.
The behavior of queries trying to find an invalid enum value has changed
[2], so we're updating the tests accordingly.
The `favicon_link_tag` method has removed the deprecated `shortcut`
link type [3], so we're updating the tests accordingly.
The method `raw_filter` in ActiveSupport callbacks has been renamed to
`filter` [4], so we're updating the code accordingly.
[1] https://github.com/rails/rails/commit/adec7e7ba87e3
[2] https://github.com/rails/rails/commit/b68f0954
[3] Pull request 43850 in https://github.com/rails/rails
[4] Pull request 41598 in https://github.com/rails/rails
Byebug hasn't been maintained for years, and it isn't fully compatible
with Zeitwerk [1]. On the other hand, Ruby includes the debug gem since
version 3.1.0. We tried to start using at after commit e74eff217, but
couldn't do so because our CI was hanging forever in a test related to
machine learning, with the message:
> DEBUGGER: Attaching after process X fork to child process Y
(Note this message appeared with debug 1.6.3 but not with the version
we're currently using.)
So we're changing the debug gem fork mode in the test so it doesn't hang
anymore when running our CI. We tried to change the test so it wouldn't
call `Process.fork`, but this required changing the code, and since
there are no tests checking machine learning behavior with real scripts,
we aren't sure whether these script would keep working after changing
the code.
[1] Issue 564 in https://github.com/deivid-rodriguez/byebug
Note that the `budget` parameter was added to the `delete_path` method
so it works in the tests; on production, it worked because this
component is only rendered on pages which already have the `budget`
parameter.
Co-authored-by: Javi Martín <javim@elretirao.net>
Note that we keep :created_at order as complement to new :order field.
We do this so that current installations will not notice any change in the
sorting of their cards when upgrading, as the default "order" field will always
be 1, so it will continue to sort by the "created_at".
Before this change, two important things depend on the format of each key,
where to render it in the administration panel and which kind of interface
to use for each setting. Following this strategy led us to a very complex
code, very difficult to maintain or modify. So, we do not want to depend
on the setting key structure anymore to decide how or where to render each
setting.
With this commit, we get rid of the key format-based rules. Now we render
each setting explicitly passing to it the type and the tab where it belongs.
This way only verified users will be able to access this page, which
shows the username of the receiver of the direct message. With this,
it's no longer possible for unverified users to browse direct message
URLs in order to collect usernames from every user.
We were already opening them in the same window because we were
accidentall sanitizing the `target` attribute, but now we're making the
point more explicit.
As far as possible I think the code is clearer if we use CRUD actions
rather than custom actions. This will make it easier to add the action
to remove votes in the next commit.
Note that we are adding this line as we need to validate it that a vote
can be created on a comment by the current user:
```authorize! :create, Vote.new(voter: current_user, votable: @comment)```
We have done it this way and not with the following code as you might
expect, as this way two votes are created instead of one.
```load_and_authorize_resource through: :comment, through_association: :votes_for```
This line tries to load the resource @comment and through the association
"votes_for" it tries to create a new vote associated to that debate.
Therefore a vote is created when trying to authorise the resource and
then another one in the create action, when calling @comment.vote.
As far as possible I think the code is clearer if we use CRUD actions
rather than custom actions. This will make it easier to add the action
to remove votes in the next commit.
Note that we are adding this line as we need to validate it that a vote
can be created on a debate by the current user:
```authorize! :create, Vote.new(voter: current_user, votable: @debate)```
We have done it this way and not with the following code as you might
expect, as this way two votes are created instead of one.
```load_and_authorize_resource through: :debate, through_association: :votes_for```
This line tries to load the resource @debate and through the association
"votes_for" it tries to create a new vote associated to that debate.
Therefore a vote is created when trying to authorise the resource and
then another one in the create action, when calling @debate.vote_by (which
is called by @debate.register_vote).
We originally added the `cached_votes_up > 0` in commit 4ce95e273
because back then `cached_votes_up` was used in the denominator. That's
no longer the case, and it doesn't make sense to mark a debate with 1
vote and 10 flags as conflictive but not doing it when the debate has no
votes and 1000 flags.
We're fixing the bug right now because we're about to change the
affected line in order to apply a new rubocop rule.
This rule was added in rubocop-rspec 2.11.0. We aren't adding it
because, out of 3 offenses, this cop can only correct 2 automatically.
Not sure how to correct the other one since it uses `.and change`.
This rule was added in rubocop-rspec 2.9.0.
We were using `be_nil` 50% of the time, and `be nil` the rest of the
time. No strong preference for either one, but IMHO we don't lose
anything be being consistent.
Note we're excluding a few files:
* Configuration files that weren't generated by us
* Migration files that weren't generated by us
* The Gemfile, since it includes an important comment that must be on
the same line as the gem declaration
* The Budget::Stats class, since the heading statistics are a mess and
having shorter lines would require a lot of refactoring
For the HashAlignment rule, we're using the default `key` style (keys
are aligned and values aren't) instead of the `table` style (both keys
and values are aligned) because, even if we used both in the
application, we used the `key` style a lot more. Furthermore, the
`table` style looks strange in places where there are both very long and
very short keys and sometimes we weren't even consistent with the
`table` style, aligning some keys without aligning other keys.
Ideally we could align hashes to "either key or table", so developers
can decide whether keeping the symmetry of the code is worth it in a
case-per-case basis, but Rubocop doesn't allow this option.
