We were getting a warning after upgrading to Rails 6:
DEPRECATION WARNING: ActionView::Base instances should be constructed
with a lookup context, assignments, and a controller.
We didn't upgrade web-console earlier because version 4.x isn't
compatible with Rails 5.
It's now used by default to handle image variants. We were getting a
warning:
DEPRECATION WARNING: Generating image variants will require the
image_processing gem in Rails 6.1. Please add `gem 'image_processing',
'~> 1.2'` to your Gemfile.
Note `mini_magick` is required in order to use the `analyze` method [1].
Since we use it in our image (and site customization image) validations,
we're still keeping the explicit dependency in our Gemfile.
[1] https://guides.rubyonrails.org/v6.0/active_storage_overview.html#analyzing-files
We were getting a warning because it won't be included by default in
Rails 6.1:
DEPRECATION WARNING: Using I18n fallbacks with an empty `defaults` sets
the defaults to include the `default_locale`. This behavior will change
in Rails 6.1 . If you desire the default locale to be included in the
defaults, please explicitly configure it with
`config.i18n.fallbacks.defaults = [I18n.default_locale]` or
`config.i18n.fallbacks = [I18n.default_locale, {...}]`. If you want to
opt-in to the new behavior, use `config.i18n.fallbacks.defaults = [nil,
{...}] `.
We were getting some deprecation warnings:
DEPRECATION WARNING: `Module#parent` has been renamed to
`module_parent`. `parent` is deprecated and will be removed in Rails
6.1.
DEPRECATION WARNING: `Module#parents` has been renamed to
`module_parents`. `parents` is deprecated and will be removed in Rails
6.1.
We can remove the `new_framework_defaults_6_0` file by using Rails 6.0
default options and overwriting the ones we haven't enabled.
We're still using the classic autoloader because we still haven't
checked how switching to zeitwerk will affect the way CONSUL
installations customize their code.
And we're using the default queues for Active Storage because we were
already using them and that will be the default option in Rails 6.1.
Similar to the way we enabled cache versioning in commit e01a94d7b. This
only affects caching `ActiveRecord::Relation` objects. I'm not even sure
we cache these objects, though, so we're just enabling the option
because it's the default one in Rails 6.0.
The default delivery job class in Rails 5.2 (ActionMailer::DeliveryJob)
is deprecated.
This option wasn't already enabled in order to ease the upgrade, since
after upgrading with Rails 6 `MailDeliveryJob`, it won't be possible to
downgrade to Rails 5.2 without risking some crashes in background jobs.
In Active Storage 5.2 there was an unexpected behavior: assigning a
collection appended records to the existing collection, instead of
replacing them as it's done in Active Record associations.
It doesn't really affect us, though, since we don't use
`has_many_attached` anywhere.
In order to prevent a warning:
```
Rails 6.1 will return false when the enqueuing is aborted. Make sure
your code doesn't depend on it returning the instance of the job and set
`config.active_job.return_false_on_aborted_enqueue = true` to remove the
deprecations.
```
This is the default encryption for cookies in Rails 6.0 applications.
The reason it isn't enabled automatically for existing applications is
these cookies are not compatible with running the application with Rails
5. Since this isn't our case, and existing cookies are still read
correctly, we can safely enable it.
This feature was kept in Rails mainly for Internet Explorer 8 and
earlier. But those browsers are now used by less than 0.1% of the
population, and we already display an alert for people using that
browser, warning we don't support it.
All the code in the `bin/` and the `config/` folders has been generated
running `rake app:update`. The only exception is the code in
`config/application.rb` where we've excluded the engines that Rails 6.0
has added, since we don't use them.
There are a few changes in Active Storage which aren't compatible with
the code we were using until now.
Since the method to assign an attachment in ActiveStorage has changed
and is incompatible with the hack we used to allow assigning `nil`
attachments, and since ActiveStorage now supports assigning `nil`
attachments, we're removing the mentioned hack. This makes the
HasAttachment module redundant, so we're removing it.
Another change in ActiveStorage is files are no longer saved before
saving the `ActiveStorage::Attachment` record. This means we need to
manually upload the file when using direct uploads. We also have to
change the width and height validations we used for images; however,
doing so results in very complex code, and we currently have to write
that code for both images and site customization images.
So, for now, we're just uploading the file before checking its
dimensions. Not ideal, though. We might use active_storage_validations
in the future to fix this issue (when they support a proc/lambda, as
mentioned in commit 600f5c35e).
We also need to update a couple of tests due to a small change in
response headers. Now the content disposition returns something like:
```
attachment; filename="budget_investments.csv"; filename*=UTF-8''budget_investments.csv
```
So we're updating regular expression we use to check the filename.
Finally, Rails 6.0.1 changed the way the host is set in integration
tests [1] and so both `Capybara.app_host` and `Capybara.default_host`
were ignored when generating URLs in the relationable examples. The only
way I've found to make it work is to explicitely assign the host to the
integration session. Rails 6.1 will change this setup again, so maybe
then we can remove this hack.
[1] https://github.com/rails/rails/pull/36283/commits/fe00711e9
While this bug was already present in the general admin search, the
combination of both search and filters was very uncommon. I've only
found this combinations in the users section, where you've got the
"erased" filter, but in this case searching for erased users doesn't
really make sense since their username and email have been deleted and
so there's nothing to find.
So the hidden content seemed to be the only affected section. However,
we're adding the field to every section so we don't have to make sure we
add it when we need it (like we did in the SDGManagement section).
We introduced this bug in commit 55d339572, since we didn't take hidden
records into consideration.
I've tried to use `update_column` to simplify the code, but got a syntax
error `unnamed portal parameter` and didn't find how to fix it.
Without this change, we were getting errors when manually creating an
annotation using symbols in the ranges hash (that is, `ranges: [{ start:
"/p[1] }]` instead of `ranges: [{ "start" => "/p[1] }]`.
Using symbols isn't really supported and it might cause strange issues
with code like: `@draft_version.annotations.find_by(range_start: ...)`.
However, this might be an extreme case and using symbols might work
properly most of the time (if not all the time), and we've been using
them in our demo for years.
So I'm not writing a test for this case because I'm not sure we should
support it, but I'm still allowing symbols so we don't get exceptions in
cases where everything might be working fine. Rails already adds Symbol
by default to column permitted classes [1], so it's safe to enable it.
[1] https://github.com/rails/rails/pull/45584/commits/c2b96e3e8
We were ignoring the draft version param when loading an annotation,
which could result in a strange situation where we load an annotation
and a draft version different than the one it belongs to.
Thanks to this change, we can simplify the code a little bit. IMHO the
`comments` and `new_comment` routes should have been added on member
instead of on collection, which would further simplify the code. I'm
leaving the routes untouched just in case changing the URL has side
effects on existing installations.
Apply new structure in the section that shows the comments icon together
with the number of comments so that it is easier to unify them into one
component.
Please note that we updated the comment-number class to comments-count
in order to simplify the css in the new component in the next commit.
This was the only place in the application where the comments icon was
included without the span with the class "icon-comments".
We unified with the rest of the application and removed the
"comments-count" class which is no longer needed.
Remove unnecessary span class "debate-comments".
We take advantage of this commit to also unify the format of the date
that appears next to the comments with the rest of the application. The
format that we removed is being used on the same page in the
"Participation phases" tab (I guess that was the reason for putting it
the same) but I think it makes more sense to use the date format that is
used in this kind section in the rest of the application.
Apply new structure in the section that shows the comments icon together
with the number of comments so that it is easier to unify them into one
component.
In this case we make only the text clickable and not the icon as in the
rest of the application. We're keeping the color and text-decoration so
it looks the same way it has looked until now, but we might change it
in the future.
We forgot to change the line rendering the image in commits 3574bf867c
and 810bdae37a, and so the custom image was being ignored.
Note that, in the test, we're stubbing a constant instead of adding a
new image. The main reason is that, if we add a new image, forks would
have to change the image when they change the `VALID_IMAGES` constant;
otherwise the tests would fail.
Apparently the dashboard branch wasn't updated after we extracted a
common head for all layouts in commit 6e4f697ce, so when said branch was
merged we reintroduced the duplication. Furthermore, we forgot to add to
the dashboard layout the changes we were applying to the common head
partial.
The Annotator library included all the dependencies it had at the moment
it was released, and so it was using jQuery 1.11.2 instead of the
version we use in the rest of the application.
Since including jQuery 1.11.2 raises a few warnings, we're removing it
so Annotator uses the same version of jQuery that is used in the rest of
the application.
We expected we needed to change some code in order to make it compatible
with jQuery 3.x. So far, everything seems to work fine, but we might
face errors in the future due to this change.