The "by_authors" scope was the last remaining name from the removed
family of `by_author` scopes. It no longer reflects its purpose: it
simply loads users by IDs.
The "on_budget_investments" scope in Activity has never been used
anywhere in the codebase. It was introduced in commit d9d38482b3
("extends Activity to include Investment valuations") but no references
were ever added.
Instead of removing it, we make use of the scope by adding the missing
"Budget investments" filter to the admin Activity section. This aligns
it with the rest of the activity filters and gives the scope the purpose
it was originally intended for.
The "by_official_level" scope in Proposal is no longer used anywhere in
the code. Its last use was removed in commit 9f1f912d84 ("Remove
official level filter from advanced search").
The "sort_by_most_commented" scope in Debate is no longer used anywhere in
the code. Its last use was removed in commit b89f39bfef ("Removes
unused orders from debates controller")
The "created_by" scope in Proposal is no longer used anywhere in the code.
It was introduced in 77dd604 and its last usage was dropped in commit 64258baf97
("Refactor getting the public activity information").
The "for_render", "sort_by_hot_score" and "sort_by_most_commented"
scopes in Legislation::Proposal are no longer used
anywhere in the code. They were all introduced in commit 335399e571
("Created Legislation Proposals model") and have never been
referenced since.
The "by_geozone_id" scope in Poll is no longer used anywhere in the code.
It was first introduced in commit 20cb044015 ("adds search and filter
for poll questions") and later moved to the Poll model in commit
d024505960 ("moves geozones from poll question to poll in models"),
but has never been referenced since.
The "by_email" scope in VerifiedUser is no longer used anywhere in the
code. Its last occurrence was removed in commit 76daee1fb0 ("removes
unmasked emails and phones in forms").
According to W3C documentation [1]:
> For compatibility with legacy content, the word-break property also
> supports a deprecated break-word keyword. When specified, this has the
> same effect as word-break: normal and overflow-wrap: anywhere,
> regardless of the actual value of the overflow-wrap property.
This is currently *not* detected by the `property-no-deprecated`
stylelint rule. However, since this rule detects other places where we
should use `overflow-wrap`, we're changing it as part of the same pull
request.
Note that, as of November 2025, 98.22% of the browsers support
`overflow-wrap: anywhere`. We're still making this change because we
only use this property in two specific places and it isn't very likely
that long words will really be a problem with the remaining 1.78%.
By the way, I'm not sure why we use this property in the budget
invesment show paragraphs and not in other similar places. It was
introduced in commit e3878ff43, alongside other changes which don't seem
to be related. When in doubt, we leave things as they were, so we're
doing the same thing here.
[1] https://www.w3.org/TR/css-text-3/#word-break-property
[2] https://caniuse.com/mdn-css_properties_overflow-wrap_anywhere
This way we make it easier to customize this component. In this specific
case, ideally we'd remove some of the CSS as well. For now we're leaving
it as it was for compatibility reasons.
This way it's easier to know that the styles for the `break` HTML class
and the JavaScript for sortable elements (which we shouldn't use, by the
way, because of its accessibility issues) are only used here.
This rule was introduced in stylelint 16.23. Out of the deprecated
properties it detects, we were only using `word-wrap`.
According to the CSS Working Group [1]:
> For legacy reasons, UAs must treat word-wrap as a legacy name alias of
> the overflow-wrap property.
Since there's universal browser support for `overflow-wrap: break-word`
[2], we can safely replace `word-wrap` with `overflow-wrap`.
[1] https://drafts.csswg.org/css-text/#overflow-wrap-property
[2] https://caniuse.com/mdn-css_properties_overflow-wrap_break-word
The button flashed briefly when loading the page. This was even more
obvious when using devices with slow internet connections.
Using CSS instead of JavaScript to hide the menu solves the issue, since
styles are loaded before scripts.
This method was returning a boolean value and caused a
`Naming/PredicateMethod` when upgrading rubocop.
So, instead, we're returning the created line when it was successfully
created, and `nil` when it wasn't.
Having said that, I'm not sure why we added the `.persisted?` back in
commit 3eb22ab7b since as far as I can tell we don't use the return
value for anything. The test added in commit da43e9e2e for this change
passes if we simply return `lines.create(investment: investment)`.
For now I'm leaving the `persisted?` check just in case, but removing it
might be fine.
This method was calling `check_availability`, which returned a boolean
value and caused a `Naming/PredicateMethod` when upgrading rubocop.
So we're changing the logic a little bit to remove the
`check_availability` method and merge the tests of `check_availability`
and `notifiable_available?` (which were almost identical) together.
This way it's more obvious that the method is supposed to return a
boolean. When upgrading rubocop, we get a `Naming/PredicateMethod` error
due to `.find` returning a boolean.
This rule was added in rubocop-rails 2.33.
At first, I wasn't very fond of this rule. It made the code less
readable even if it improved performace in some cases.
Then I realized that in the `Admin::MachineLearning::SettingComponent`
we were using `find_by` when we should be using `find_by!` instead, and
we detected that thanks to this rule.
So, only for that reason, I'm adding this rule, but I'm fine if we
remove it.
We introduced this issue in commit f8faabf7d.
Since this component didn't have any tests (there are system tests for
it, though), we're also adding tests that check that only the right
buttons are rendered when accessing as administrator.
The "by_author" scope in Poll::Recount is no longer used anywhere in the
code. It was introduced in commit 6c297ae789 ("Add Poll Recount model,
factory and spec") but has never been referenced since.
The "by_author" scope in Poll::PartialResult is no longer used anywhere in
the code. Its usage was replaced by Poll::Answer.by_author in commit
6bc4f5b307 ("adds Poll::Answer model for web users").
The "by_author" scope in Poll::Answer is no longer used anywhere in the
code. Its last occurrence was removed in commit 69eaf66b93 ("Remove
redundant max_votes validation from Poll::Answer")
This meta tag was added years ago as a workaround for
foundation-sites issue 12167, in commit 791b2e12 from PR 4753
("Fix invalid HTML in application layout").
Since we now use foundation-sites >= 6.8.1 (fixed in 6.7.1),
the workaround is unnecessary.
We were getting a warning on staging and production environments:
```
app/models/local_census_records/import.rb:1: warning: csv was loaded
from the standard library, but will no longer be part of the default
gems starting from Ruby 3.4.0.
You can add csv to your Gemfile or gemspec to silence this warning
```
The reason we weren't getting this warning during development is that we
do have `csv` in our `Gemfile.lock`, but only in development
environments, since it's an indirect dependency of pronto. On production
environments, we don't install pronto or its dependencies, though.
We can reproduce the warning locally by temporarily removing the pronto
gems from the Gemfile, running `bundle install` and starting a rails
console.
We were calling `parse_remote_to_hash` in the Devise initializer, which
runs when the application starts.
That meant that, if we got an exception when calling that method, the
application wouldn't start. We got exceptions if the single sign-on
(SSO) URL isn't available or we aren't providing the right credentials.
So we're moving the call to `parse_remote_to_hash` to
`OmniauthTenantSetup`, which is only called when actually trying to sign
in with SAML.
Since we're moving the code there, we're also unifying the code so SAML
settings are configured the same way for the main tenant and other
tenants, like we did for OpenID Connect in commit c3b523290.
In order to keep the existing behavior, we're caching the result of
`parse_remote_to_hash` in an instance variable. Not sure about the
advantages and disadvantages of doing so over parsing the remote URL
metadata on every SAML-related request.
Note that the SAML tests in `OmniauthTenantSetup` use the `stub_secrets`
method. But this method is called after the application has started,
meaning it doesn't stub calls to `Rails.application.secrets` in
`config/initializers/`. So, before this commit, the code that parsed the
IDP metadata URL wasn't executed in the tests. Since now we've moved the
code but we don't want to depend on external URLs when running the
tests, we need to stub the call to the external URL. Since we're now
stubbing the call, we're adding expectations in the tests to check that
we correctly use the settings returned in that call.
The `issuer` setting was renamed to `sp_entity_id` in omniauth-saml [1],
and it's been deprecated in ruby-saml since version 1.11.0, released on
July 24, 2019 [2].
The ruby-saml code currently uses:
```
def sp_entity_id
@sp_entity_id || @issuer
end
```
So setting `issuer` to the same value as `sp_entity_id` if
`sp_entity_id` is present, as we were doing, has no effect.
On the other hand, neither omniauth-saml nor ruby-saml use the
`idp_metadata_url` and `idp_metadata` settings.
[1] https://github.com/omniauth/omniauth-saml/commit/74ed8dfb3aed
[2] https://github.com/SAML-Toolkits/ruby-saml/releases/tag/v1.11.0
When we first added OIDC support, we were configuring the redirect URI
in the devise initializer, just like we did for other providers.
Thanks to the changes in the previous commit, that code is no longer in
the devise initializer, which means we can use `url_helpers` to get the
redirect URI.
This means we no longer need to define this URI in the secrets. This is
particularly useful for multitenancy; previously, we had to define the
redirect URI for every tenant because different tenants use different
domains or different subdomains.
We were following the same pattern as we used for other providers like
twitter or facebook, but for OIDC we aren't passing the key and the
secret as separate attributes but only a hash of options. This means we
don't need to duplicate the same logic in the devise initializer and the
`OmniauthTenantSetup` class.
Thanks to these changes, we'll be able to introduce dynamic redirect
URLs for both the default tenant and the other tenants (see next commit).
Note that we could probably apply similar changes for the SAML provider.
We might do so in the future. For other providers, removing the
references to `Rails.application.secrets` broke their configuration when
we tested it back in 2022 as part of the multitenancy feature. We might
check whether that's no longer the case (or whether we made a mistake
during our tests in 2022) in the future.
We were using the `client_options` hash for the default tenant, defined
in the Devise initializer, but we forgot to include that key in the
multitenant code. This means OIDC wasn't working when different tenants
used different configurations.
Note that we are not including Poll::PartialResults for open-ended
questions resutls. The reason is that we do not contemplate the
possibility of there being open questions in booths. Manually
counting and introducing the votes in the system is not feasible.
Running tests at the component level is faster than at the system level,
so we move tests from system/polls/results_spec.rb to the component.
Note that moving these tests removes vote_for_poll_via_web and the visit
to results_poll_path, but both are already covered in other tests. We
also take the opportunity to reuse the method in another test where
it makes sense.
Additionally, the spec title has been reverted from "Results for polls
with questions but without options" to "renders results for polls with
questions but without answers", as it was before commit 8997ed316c.
