Even if pretty much nobody uses a browser with JavaScript disabled when
navigating our sites, there might be times where JavaScript isn't loaded
for reasons like a slow internet connections not getting the JavaScript
files or a technical issue.
So we're making it possible to still use the like/unlike buttons in
these cases.
Other packages depend on jQuery, so that's why these are the first one
we move from the Gemfile to the package.json file.
This way we can also test whether dependabot correctly opens pull
requests to update Node packages.
I've tried several configuration options for the asset pipeline in order
to be able to include images referenced in jQuery UI CSS files. So far,
adding the `node_modules/jquery-ui/themes/base` folder to the assets
paths is the only way I've found to make it work. Hopefully we can find
a better solution in the future so we don't have to study the internals
of every Node package in order to integrate it with the assets pipeline.
Note that, even if we're excluding the `node_modules/` folder from
version control, we aren't adding it to Capistrano's shared folders
because, when `node_modules` is a symbolic link, NPM removes it when
running `npm install`.
This code is based on what the rvm1-capistrano and capistrano-nvm gems
do, but simplified a bit to take advantage of the `fnm exec` command.
Since ExecJS will check for a JavaScript runtime every time the
application is started, we need to include commands like `bundle` (used
when running `bin/delayed_job`), `puma` and `rake`, so Node.js is found
when running these commands. I'm not sure whether `pumactl` is also
necessary, but I've added it for consistency.
We're also including the default commands gems like capistrano-nvm use
because we will add the `npm` command in the near future.
Note that, in order to setup FNM, we need to enter the actual release
folder (using `within release_path` isn't enough). So we have to run
this task after the actual release is created; otherwise many commands
would run in the previous release's folder.
We use FNM instead of NVM. Reason: the setup seems easier with just
`eval "$(fnm env)"`.
So now, we try to install Node.js; if the command fails, it could be
because FNM isn't installed (and we need to install it) or because the
version of Node.js cannot be installed with the current version of FNM
(and we need to update FNM). After installing/updating FNM, we try to
install Node.js again.
Note we're using `fnm env` in the middle of the `fnm_setup_command`.
That way, the command will raise a `SSHKit::Command::Failed` exception
if `fnm` isn't installed, and it will give us an indicator that we need
to actually install it.
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 added this option in commit d17b2523c, but Bundler now keeps
descriptors by default. While this behavior was backported to Ruby
3.0.x, we're changing it now because, we've only noticed it now that
we're upgrading to Ruby 3.1.x, since it was first developed for that
version [1].
[1] https://github.com/rubygems/rubygems/pull/4812/commits/88b7a3e7e2
Note we updated the `mail` gem in commit 103742847, which is necesary
for Ruby 3.1 because it adds the net-smtp dependency. The net-smtp
library was removed from Ruby in Ruby 3.1, and if we don't include it,
we get an error:
```
cannot load such file -- net/smtp (LoadError)
```
We're also updating the Bundler version in the Gemfile.lock so it's the
one included in Ruby 3.1. Without updating it, we get a warning:
```
Calling `DidYouMean::SPELL_CHECKERS.merge!(error_name => spell_checker)'
has been deprecated. Please call `DidYouMean.correct_error(error_nam e,
spell_checker)' instead.
```
Finally, in order to make Capistrano work, we need to add a couple more
changes:
* Make the net-ssh gem compatible with SSL 3.0; done in commit b2eec088b
* Explicitly allow aliases in the `deploy-secrets.yml` file because
Psych 4.x (included in Ruby 3.1) doesn't load aliases without this
option
We can remove the `new_framework_defaults_6_1` file by using Rails 6.1
default options and overwriting the one we haven't enabled.
We've experienced problems while running the tests (probably the same
would happen on production) when enabling the `has_many_inversing`
option. For example, after creating a legislation answer for a question
with no answers, calling `question.answers_count` would then return `2`
instead of `1`.
So we aren't enabling this option.
This is the default setting in Rails 6.1, and generates an extra tag in
the HTML which tells the browser to download and cache these files as
soon as possible, even before they're needed.
This might not be that relevant in our application, since on most pages
we only generate one CSS and one JS file. But it might make it easier to
move the `javascript_include_tag` statement to the bottom of the page in
the future if we detect that doing so increases performance.
Since we aren't using the old way to handle multiple databases (because
we don't use multiple databases), we can safely enable this option
without breaking anything.
This way user agents will know that the redirection from HTTP to HTTPS
is permanent and not temporary, which is the case if we activate the
`force_ssl` option (which we do by default).
These measures increase protection against CSRF ataks. The only reason
Rails provides them as a configuration option is there are complex
applications that run one version of the code in some servers while
running an old version of the code in other servers might run into
issues because the the old version won't handle the tokens or cookies
generated by the new version.
Since most Consul applications use just one server and the ones with
more servers would only face this issue for a few seconds (while
upgrading to a new version of Consul Democracy), we can safely enable
these configuration options.
Not sure this configuration option does anything, though, since it's
been removed in Rails 7.0 because it was not halting the callbacks.
But, if it does nothing, it's the same as disabling it, which is what we
were doing until now, so in the end using the Rails 6.1 default value
does no harm.
This mostly benefit people using external services, as now there's no
need to query the service to check whether a variant exists.
For most Consul Democracy installations, this will probably not be
relevant, so we're sticking wih the default value.
This patch was added in commit baefc249f because both ViewComponent and
Wicked PDF monkey-patched the `render` method and so they were
incompatible.
However, Rails 6.1 includes the patch used by ViewComponent, meaning
ViewComponent doesn't monkey-patch the `render` method anymore, and so
it's compatible with Wicked PDF.
Note that `Capybara.app_host` now returns `nil` by default and that
breaks tests using `lvh.me` or our custom `app_host` method, so we're
setting `Capybara.app_host` to the value it had in earlier versions of
Rails. I also haven't found a way to remove the code to set the
integration session host in relationable tests which I mentioned in
commit ffc14e499.
Also note that we now filter more parameters, and that they match
regular expressions, so filtering `:passw` means we're filtering
`passwd`, `password`, ...
Internet Explorer 8 was released in 2009 and people using it already
know that most web pages look broken on it, so we don't need to warn
them.
Removing it makes our application layout file much easier to read and
modify.
The code based on the logger Rails uses by default; as mentioned in the
Rails configuration guide:
> [the logger] defaults to an instance of ActiveSupport::TaggedLogging
> that wraps an instance of ActiveSupport::Logger which outputs a log to
> the log/ directory. You can supply a custom logger, to get full
> compatibility you must follow these guidelines:
>
> * To support a formatter, you must manually assign a formatter from
> the config.log_formatter value to the logger.
> * To support tagged logs, the log instance must be wrapped with
> ActiveSupport::TaggedLogging.
> * To support silencing, the logger must include
> ActiveSupport::LoggerSilence module. The ActiveSupport::Logger class
> already includes these modules.
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.
We were already applying these rules in most cases.
Note we aren't enabling the `MultilineArrayLineBreaks` rule because
we've got places with many elements whire it isn't clear whether
having one element per line would make the code more readable.
Code based on the logger Rails uses by default; as mentioned in the
Rails configuration guide:
> [the logger] defaults to an instance of ActiveSupport::TaggedLogging
> that wraps an instance of ActiveSupport::Logger which outputs a log to
> the log/ directory. You can supply a custom logger, to get full
> compatibility you must follow these guidelines:
>
> * To support a formatter, you must manually assign a formatter from
> the config.log_formatter value to the logger.
> * To support tagged logs, the log instance must be wrapped with
> ActiveSupport::TaggedLogging.
> * To support silencing, the logger must include
> ActiveSupport::LoggerSilence module. The ActiveSupport::Logger class
> already includes these modules.
Just like the documentation mentions, we're enabling log rotation using
"1" as the number of old files to keep and then the maximum size of the
log file.
