This rule was added in rubocop-performance 1.24.
We currently don't have code where it'd be relevant, but it's a nice
rule to have if we ever write `something.map { |element| [element] }`.
We're changing the `.rubocop.yml` file because we were getting a
warning:
```
rubocop-factory_bot extension supports plugin, specify `plugins:
rubocop-factory_bot` instead of `require: rubocop-factory_bot` in
.rubocop.yml. For more information, see
https://docs.rubocop.org/rubocop/plugin_migration_guide.html
```
---
updated-dependencies:
- dependency-name: rubocop-factory_bot
dependency-version: 2.27.1
dependency-type: direct:development
update-type: version-update:semver-minor
...
Signed-off-by: dependabot[bot] <support@github.com>
Our original interface to vote in a poll had a few issues:
* Since there was no button to send the form, it wasn't clear that
selecting an option would automatically store it in the database.
* The interface was almost identical for single-choice questions and
multiple-choice questions, which made it hard to know which type of
question we were answering.
* Adding other type of questions, like open answers, was hard since we
would have to add a different submit button for each answer.
So we're now using radio buttons for single-choice questions and
checkboxes for multiple-choice questions, which are the native controls
designed for these purposes, and a button to send the whole form.
Since we don't have a database table for poll ballots like we have for
budget ballots, we're adding a new `Poll::WebVote` model to manage poll
ballots. We're using WebVote instead of Ballot or Vote because they
could be mistaken with other vote classes.
Note that browsers don't allow removing answers with radio buttons, so
once somebody has voted in a single-choice question, they can't remove
the vote unless they manually edit their HTML. This is the same behavior
we had before commit 7df0e9a96.
As mentioned in c2010f975, we're now adding the `ChangeByZero` rubocop
rule, since we've removed the test that used `and change`.
This rule was introduced in RuboCop 1.76.0 to ensure methods ending
in '?' return boolean.
This commit applies suggested renames and code cleanup:
- Renames 'is_active?' to 'active_class' since it returns a string
- Renames 'parsed_value' to 'in_favor?' and 'is_request_active' to end with '?'
for boolean semantics
- Skips false positives like 'save', 'auto_labels' or 'save_requiring_finish_signup',
which are not predicate methods.
This is an error we've only been able to reproduce on one specific
machine and only when using the development environment.
It looks like Rails 7.1.5.1 uses `ActiveStorage::PreviewImageJob` when
we attach a PDF. However, that raises an IntegrityError because we're
removing the metadata from PDFs. That is, the final PDF is no longer the
same PDF that was uploaded.
This issue wasn't present in the original Rails 7.1.0 release, but was
introduced in Rails 7.1.4 [1] and has already been fixed in Rails 7.2.0
[2].
So we're applying the same solution that was applied in Rails 7.2.0:
disabling automatic previews for PDFs when no variants require them by
changing a method in `ActiveStorage::Attachment`.
[1] See commit 6f729dd39 in https://github.com/rails/rails/
[2] See pull request 51351 in https://github.com/rails/rails/
We're disabling `action_controller.raise_on_missing_callback_actions`
because sometimes we include `before_action :something, only: actions`
in concerns, and we include these concerns in controllers that don't
have all these actions.
Note that Rails 7.1 logs to STDOUT by default [1]; we're re-adding the
condition `if ENV["RAILS_LOG_TO_STDOUT"].present?` because we're still
using files and we're rotating the logs to avoid running out of space.
Also note that the GraphQL controller test (which is actually a request
test, since it's got `type: :request`) that was raising an exception no
longer does it thanks to the new default value for the
`config.action_dispatch.show_exceptions` configuration option. So we're
updating the test accordingly. This option doesn't affect regular
controller tests (without the `type: :request` option), so in other
tests we're still checking exceptions.
[1] Pull request 47138 in https://github.com/rails/rails
This code was added in commit b3f570512 in order to rotate existing
cookies used by Consul Democracy 2.1 and earlier. Since the code was
included in Consul Democracy 2.2, existing installation using Consul
Democracy 2.2 will have already rotated the old cookies, which means we
don't need the cookie rotator anymore.
Make `path`, `fill_resource_method_name`, `submit_button`, and
`imageable_success_notice` dynamic based on the factory.
Also adjusted the user. The proposals no longer require the user to be an
administrator but do require them to be a level 2 user.
Note that we are adding the Style/CaseLikeIf rubocop rule.
In rubocop-rails 2.26.0, the Rails/CompactBlank rule was modified to handle
cases where select(&:present?) is used. After identifying three occurrences
in our code, we've decided to apply this rule as it encourages the use of the
more efficient and clearer method, compact_blank.
By using compact_blank, we improve code clarity and performance, as this method performs the same operation but in a more optimized way.
In rubocop-rails 2.26.0, support was added for Rails 7 syntax in the
Rails/EnumHash rule. We took this opportunity to ensure consistency
by converting all enums to hash with integer values. This format minimizes
the risk of data consistency issues in the database when adding new values.
This rule was added in rubocop-rails 2.26.0. Applying it allows
us to anticipate the deprecation of the current enum syntax
using keyword arguments, which is set to be removed in Rails
8.0, as mentioned in the rule's own documentation:
https://docs.rubocop.org/rubocop-rails/cops_rails.html#railsenumsyntax
This rule was introduced in RuboCop 0.67.2, but now after seeing a fix in version 1.65.1,
we have decided to add it. The reason for adding it is to ensure consistency in how we
reference exceptions throughout the project, by following a standard naming convention
for exception variables.
This rule was introduced in RuboCop 1.53.0. After adding the
Style/RedundantRegexpCharacterClass rule in the previous commit,
RuboCop started detecting redundant regular expression arguments.
Therefore, we apply this rule to remove them and prevent future
occurrences.
This rule was introduced in RuboCop 0.93.0, but now after seeing a fix in version 1.65,
we have decided to add it. The reason for adding it is to simplify our regular
expressions. This enforcement will help us maintain better regular expression
practices across the project.
This rule was introduced in rubocop-rspec 3.0.0. We were already
following it. It might be helpful because during development I've made
typos prevented by this rule a couple of times.
This rule was added in rubocop-rspec 2.29.0. While we never use the
`output` matcher, it might actually be a good idea to use it in tests of
take tasks, to check the logger output.
This rule was added in rubocop 1.64.0.
For clarity, in order to make it obvious that we're modifying the object
we received, we're excluding the Ahoy initializer, whose code was copied
from the Ahoy documentation.
We're also changing the `Types::BaseObject` class so we don't use a
variable with the same name as the parameter and we don't get a false
positive for this rule.
This rule was added in rubocop-rails 2.25.0. Applying it allows us to
simplify the code a little bit. For example, now there's no need to
specify the `proposals` table in proposal scopes, which was actually
causing a bug in the `Legislation::Proposal` model, which was using the
`proposals` table instead of the `legislation_proposals` table (but,
since we don't use this scope, it didn't affect the application).
The main reason for this change is that Pronto doesn't run in pull
requests opened by external contributors, and we haven't found how to
fix this issue.
Another reason is that Pronto only detects issues in the lines where the
changes are done, but sometimes we introduce issues related to other
lines and those aren't detected by Pronto. Also, when adding or changing
Rubocop rules, or when we update Rubocop, Pronto doesn't check whether
those rules are applied in every Ruby and ERB file, and we sometimes
forget to do so (particularly in ERB files).
So, from now, the linters will check all the code in every pull request.
Note we're now excluding the `vendor` folder in our linters because the
`setup-ruby` action actions generates a `vendor/bundle/` folder with all
our gem dependencies, and we don't want to check those files.
Note that, currently, we take these settings from the database but we
don't provide a way to edit them through the admin interface, so the
locales must be manually introduced through a Rails console.
While we did consider using a comma-separated list, we're using spaces
in order to be consistent with the way we store the allowed content
types settings.
The `enabled_locales` nomenclature, which contrasts with
`available_locales`, is probably subconsciously based on similar
patterns like the one Nginx uses to enable sites.
Note that we aren't using `Setting.enabled_locales` in the globalize
initializer when setting the fallbacks. This means the following test
(which we could add to the shared globalizable examples) would fail:
```
it "Falls back to an enabled locale if the fallback is not enabled" do
Setting["locales.default"] = "en"
Setting["locales.enabled"] = "fr en"
allow(I18n.fallbacks).to receive(:[]).and_return([:fr, :es])
Globalize.set_fallbacks_to_all_available_locales
I18n.with_locale(:fr) do
expect(record.send(attribute)).to eq "In English"
end
end
```
The reason is that the code making this test pass could be:
```
def Globalize.set_fallbacks_to_all_available_locales
Globalize.fallbacks = I18n.available_locales.index_with do |locale|
((I18n.fallbacks[locale] & Setting.enabled_locales) + Setting.enabled_locales).uniq
end
end
```
However, this would make it impossible to run `rake db:migrate` on new
applications because the initializer would try to load the `Setting`
model but the `settings` table wouldn't exist at that point.
Besides, this is a really rare case that IMHO we don't need to support.
For this scenario, an installation would have to enable a locale, create
records with contents in that locale, then disable that locale and have
that locale as a fallback for a language where content for that record
wasn't created. If that happened, it would be solved by creating content
for that record in every enabled language.
As mentioned in Ahoy's README [1]:
> Ahoy provides a number of options to help with GDPR compliance.
> Update config/initializers/ahoy.rb with:
>
> class Ahoy::Store < Ahoy::DatabaseStore
> def authenticate(data)
> # disables automatic linking of visits and users
> end
> end
>
> Ahoy.mask_ips = true
> Ahoy.cookies = :none
As also mentioned in the README:
> If Ahoy was installed before v5, add an index before making this
> change.
> (...)
> For Active Record, create a migration with:
> add_index :ahoy_visits, [:visitor_token, :started_at]
However, the `visitor_token` doesn't exist in our table, since we
generated the `visits` table when Ahoy used the `visitor_id` column. So
we're using this column for the index.
Note we also need to change the `visit` method, since otherwise we get
an exception [2]. As mentioned on the issue reporting the exception:
> you'll need to copy the latest version of that method and adapt it to
> your model. I believe you'll want to replace:
>
> where(visit_token: ahoy.visit_token) with
> where(id: ensure_uuid(ahoy.visit_token))
>
> where(visitor_token: ahoy.visitor_token) with
> where(visitor_id: ensure_uuid(ahoy.visitor_token))
So we're copying the latest version of that method and changing it
accordingly.
[1] https://github.com/ankane/ahoy/blob/v5.0.2/README.md
[2] Issue 549 in https://github.com/ankane/ahoy
Note that enabling this options means all encrypted messages and cookies
generated the application become invalid, so we're adding a cookie
rotator in order to keep sessions from expiring when upgrading the
application, as recommended in the "Upgrading Ruby on Rails" guideline
[1].
Since we haven't seen any Consul Democracy applications using encrypted
messages and these messages become invalid with this change, we're also
removing the pre-Rails 5.2 encryption to authenticate messages
(AES-256-CBC) and switching to the default one since Rails 5.2
(AES-256-GCM). Since the configured encryption is used by the cookie
rotator initializer (through the ActiveSupport::MessageEncryptor.key_len
method), at first I thought this might affect the cookie rotator, but it
doesn't: upgrading works as expected, and existing sessions are still
active.
I'm adding a comment to remove the initializer once all cookies have
been migrated. I've added "Rails 7.1" in the comment because we usually
check for these comments when upgrading Rails, but we rarely check for
them when after releasing new versions of Consul Democracy.
[1] https://guides.rubyonrails.org/v7.0/upgrading_ruby_on_rails.html#key-generator-digest-class-changing-to-use-sha256
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
The purpose of the lib folder is to have code that doesn't necessary
belong in the application but can be shared with other applications.
However, we don't have other applications and, if we did, the way to
share code between them would be using a gem or even a git submodule.
So having both the `app/` and the `lib/` folders is confusing IMHO, and
it causes unnecessary problems with autoloading.
So we're moving the `lib/` folder to `app/lib/`. Originally, some of
these files were in the `app/services/` folder and then they were moved
to the `lib/` folder. We're using `app/lib/` instead of `app/services/`
so the upgrade is less confusing.
There's an exception, though. The `OmniAuth::Strategies::Wordpress`
class needs to be available in the Devise initializer. Since this is an
initializer and trying to autoload a class here will be problematic when
switching to Zeitwerk, we'll keep the `require` clause on top of the
Devise initializer in order to load the file and so it will be loaded
even if it isn't in the autoload paths anymore.
We were using generic names like `args` and `options` which don't really
add anything to `*` or `**` because Ruby required us to.
That's no longer the case in Ruby 3.2, so we can simplify the code a
bit.
