We can find the booth through the booth assignment, so we don't need to
pass it in the URL.
Since the parameter is in the URL and not sent through a form, we can
also use `params[:poll_id]` directly, and so we can reuse the
`load_poll` method.
We were manually adding forgery protection to all our controllers, but
in Rails 5.2 there's an option (enabled by default for new applications)
which adds this protection to all controllers.
Rails 5.2 is raising a warning in some places:
DEPRECATION WARNING: Dangerous query method (method whose arguments are
used as raw SQL) called with non-attribute argument(s). Non-attribute
arguments will be disallowed in Rails 6.0. This method should not be
called with user-provided values, such as request parameters or model
attributes. Known-safe values can be passed by wrapping them in
Arel.sql().
IMHO this warning is simply wrong, since we're using known PostgreSQL
functions like LOWER() or RANDOM(). AFAIK this code works without warnings
in Rails 6.0 [1][2]
However, since the warning is annoying, we need to take measures so our
logs are clean.
[1] https://github.com/rails/rails/commit/6c82b6c99d
[2] https://github.com/rails/rails/commit/64d8c54e16
It was removed in commit 128a8164 because we hadn't reviewed it nor
tested it properly. We're now adding it again, fixing the issues we've
found while reviewing.
Sometimes we define URLs for POST requests which are not defined for GET
requests, such as "/residence", so redirecting to it after signing out
results in a routing error.
So instead of using the request referer, we're using the stored location
devise uses, and we're not storing locations in POST requests.
For reasons I'm not sure about, the homepage (and the welcome pages)
were an exception in our "redirect users to the same page they were"
policy.
I'm not sure about the welcome pages (no test was present indicating
they should behave in a special way), but in the case of the home page,
it was a bit annoying to be redirected to a different place after
signing in.
When signing in from a page containing GET params, like
`/budgets/1/investments?heading_id=4`, we were redirected to a URL
without those GET params; in this case, `/budgets/1/investments`.
Using the request fullpath, as recommended in the devise documentation,
keeps these parameters when redirecting.
Up until now, we were assuming the voter was valid, but were not raising
an exception if it wasn't. And in the user interface everything seemed
to be working properly.
We were having this issue when skipping verification, when there could
be voters without a document number, which would be considered invalid.
Raising an exception when failing to save the voter and making sure the
answer and the voter are saved inside a transaction solves the problem.
The heading is used with `find_by_slug_or_id`, which raises an exception
if it isn't found, so executing `@heading.group` after it does not need
the safe navigation operator.
Rails 5.2 crashes in the `db:create` task because it tries to run the
`after_initialize` block before the database is created.
The easiest way to solve it is to move the code out of the initializer
and calculate the API type definitions on demand. Note results are still
cached using a class instance variable (not to be confused with a class
variable), and so once definitions are obtained, they will remain
constant until the application is restarted, even in the development
environment.
I'm not sure why the code didn't work without this line, but it doesn't
seem to be necessary anymore (maybe after upgrading Ruby or Rails?).
I'm removing it now because... why not now? :) The Ruby interpreter is
raising a warning due to this line, and in Ruby 2.5 constant lookup has
changed slightly (although I don't think this line is affected by that
change).
Note about the change in the Setting model: Ruby actually ignores return
values in setter methods, so the line isn't necessary.
We were treating legislation proposals as if they were proposals,
omitting the "legislation" namespace, and so we were flagging/unflagging
proposals when we wanted to flag/unflag a legislation proposal.
The `respond_with` method is no longer part of Rails (it's now part of
the responders gem) and we barely use it. Using a template forced us to
use different criteria for different controllers.
This change will also make it easier to fix the flag/unflag actions for
legislation proposals. With the old code, we would have to add another
condition for the legislation/proposals controller.
We don't need to set this value. In commit f2ef27d3 I made a mistake
thinking `Globalize.locale` and `I18n.locale` should always be in sync,
but they're actually automatically in sync when `Globalize.locale` is
`nil`.
So the best way to avoid any issues is not to assign `Globalize.locale`,
and use `Globalize.with_locale` where necessary instead.
In the past, we couldn't use `polymorphic_path` in many places. For
instance, `polymorphic_path(budget, investment)` would return
`budget_budget_investment_path`, while in our routes we had defined
`budget_investment_path`.
With the `resolve` method, introduced in Rails 5.1, we can use symbols
to define we want it to use `investment` instead of `budget_investment`.
It also works with nested resources, so now we can write
`polymorphic_path(investment)`.
This makes the code for `resource_hierarchy_for` almost impossible to
understand. I reached this result after having a look at the internals
of the `resolve` method in order to get its results and then remove the
symbols we include.
Note using this method will not make admin routes compatible with
`polymorphic_path`. Quoting from the Rails documentation:
> This custom behavior only applies to simple polymorphic URLs where a
> single model instance is passed and not more complicated forms, e.g:
> [example showing admin routes won't work]
Also note that now the `admin_polymorphic_path` method will not work for
every model due to inconsistencies in our admin routes. For instance, we
define `groups` and `budget_investments`; we should either use the
`budget_` prefix in all places or remove it everywhere. Right now the
code only works for items with the prefix; it isn't a big deal because
we never call it with an item without the prefix.
Finally, for unknown reasons some routing tests fail if we use
`polymorphic_path`, so we need to redefine that method in those tests
and force the `only_path: true` option.
Using `render :nothing` was deprecated, but we never noticed it because
we didn't have a test for the action using it. In Rails 5.1, it raises
an exception.
Using `head :ok` and adding a test for this scenario solves the issue.
This filter was added in commit 4285ba4b, it was changed in commit
002d8688, and most of the code from the original commit has disappeared
without a trace (maybe due to a merge conflict?).
This filter could actually be useful if we started using it when users
click on a tag. Since we don't, I'm removing it. We might add it back if
we decide to actually use it.
Legislation::Proposal is not Globalize model but use CommentableActions and try
detect remote translations. Add new condition to discard Non Globalize models.
This fix is necessary since the following commit was included: c1f3a4ad.
These changes fix a bug that causes categories
of a legislation process to be wiped on update
of the process. It also adds a regression test
for this fix.
