There was only one different line between these two lines, and we really
want these two files to be as similar as possible, so testing on a
staging server we get the same results we'll get on production.
This duplication made it easy to forget to update the staging.rb file
when updating the production.rb, particularly when upgrading to a new
version of Rails, which automatically changes the production.rb file but
not the staging.rb one.
So, now that we're getting ready to upgrade to Rails 7.0, we're basing
the staging configuration on the production one, just like we based the
preproduction configuration on the staging one in commit 7b91adb16.
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`, ...
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.
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.
Now that we've got rid of all the warnings we had, we can enable them so
we'll notice new warnings when we introduce them.
This was the default option until Ruby 2.7.2 was released [1]. These
warnings were turned off by default because pretty much every Ruby gem
had dozens of warnings with Ruby 2.7 due to the changes in the way Ruby
handles keyword arguments.
[1] https://www.ruby-lang.org/en/news/2020/10/02/ruby-2-7-2-released/
Since records in different tenants can have the same ID, they can share
the same `cache_key`, and so we need a namespace to differentiate them.
Without them, records from one tenant could expire the cache of a record
from another tenant.
The subdomain elevator we were using, which is included in apartment,
didn't work on hosts already including a subdomain (like
demo.consul.dev, for instance). In those cases, we would manually add
the subdomain to the list of excluded subdomains. Since these subdomains
will be different for different CONSUL installations, it meant each
installation had to customize the code. Furthermore, existing
installations using subdomains would stop working.
So we're using a custom method to find the current tenant, based on the
host defined in `default_url_options`.
In order to avoid any side-effects on single-tenant applications, we're
adding a new configuration option to enable multitenancy
We're enabling two ways to handle this configuration option:
a) Change the application_custom.rb file, which is under version control
b) Change the secrets.yml file, which is not under version control
This way people prefering to handle configuration options through
version control can do so, while people who prefer handling
configuration options through te secrets.yml file can do so as well.
We're also disabling the super-annoying warnings mentioning there are no
tenants which we got every time we run migrations on single-tenant
applications. These messages will only be enabled when the multitenancy
feature is enabled too. For this reason, we're also disabling the
multitenancy feature in the development environment by default.
Rails 6.0 introduced a `hosts` option which, in the development
environment, defaults to all IP addresses and the `localhost` domain.
However, we can't work with subdomains using `localhost`. For that
purpose, the `lvh.me` domain was created [1].
So we're allowing this domain and its subdomains so we can use them
while working with multitenancy in the development environment.
[1] http://railscasts.com/episodes/123-subdomains-revised
We were duplicating the asset host and the URL host in all environments,
but we can make it so the asset host uses the URL host unless we
specifically set it.
Note that, inside the `ApplicationMailer`, the `root_url` method already
uses `default_url_options` to generate the URL.
In the rare case of CONSUL installations who have changed the asset
host, this change has no effect since they'll get a conflict in the
environment files when upgrading and they'll choose to use their own
asset host.
Until now, when editing a specific environment, other CONSUL
installations had to edit the original file, which made it harder to
upgrade.
Now it's possible to change the default CONSUL settings using custom
files, making it easier to upgrade to versions of CONSUL which change
the original environment files (which is very common when upgrading
versions of Rails).
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
We changed the structure of the production environment file in commit
eb36b7e2e, but forgot to do so for staging and preproduction.
We're also changing the comments in the production environment file so
they use double quotes, like we do everywhere else.
After these changes, the staging and production environments are mostly
identical, and we could simply require the production environment
configuration in the `staging.rb` file. We aren't doing so because we've
just done changes affecting the preproduction environment and, since
Consul installations might have customized these files, it might be hard
for them to deal with all these changes at once.
We were already doing so by duplicating the code; now we're doing it
explicitly.
This is going to help because we're going to do a few changes in the
environment files, and this way we don't have to do everything twice.
Note that, after we're finished with these changes, the staging and
production environments will be mostly identical and we could simply
require the production environment configuration in the `staging.rb`
file. We aren't doing so because some Consul installations might have
customized these files and it'd be hard for them to deal with all these
changes at once. So, for now, they'll only have to deal with the
possible differences between their staging and preproduction
environments.
Changes to the preproduction environment that won't be applied to the
staging environment can be added like this:
require Rails.root.join("config", "environments", "staging")
Rails.application.configure do
config.log_level = :warn
end
We're not adding the rule because it would apply the current line length
rule of 110 characters per line. We still haven't decided whether we'll
keep that rule or make lines shorter so they're easier to read,
particularly when vertically splitting the editor window.
So, for now, I'm applying the rule to lines which are about 90
characters long.
`dalli_store` is deprecated since dalli 2.7.11.
We can now enable cache_versioning. We didn't enable it when upgrading
to Rails 5.2 because of possible incompatibility with `dalli_store` [1],
even though apparently some the issues were fixed in dalli 2.7.9 and
dalli 2.7.10 [2].
Since using cache versioning makes cache expiration more efficient, and
I'm not sure whether the options we were passing to the dalli store are
valid with memcache store (documentation here is a bit lacking), I'm
just removing the option we used to double the default cache size on
production.
[1] https://www.schneems.com/2018/10/17/cache-invalidation-complexity-rails-52-and-dalli-cache-store
[2] https://github.com/petergoldstein/dalli/blob/master/History.md
While debugging JavaScript is certainly useful, enabling it generates
about 100 extra HTTP requests because we include about 100 JavaScript
files (including external dependencies and files written by us).
Depending on the browser configuration, this might make the browser take
a very long time processing these requests.
On my machine, with these changes, refreshing a page on Firefox takes
about 1 second, while previously it took about 6-8 seconds. With
Chromium, there doesn't seem to be much difference.
Developers are encouraged to temporarily turn debugging while debugging
JavaScript (which is a task I personally do about once a month) if that
makes debugging easier for them.
This change doesn't affect our CSS files, since for CSS we use Sass
instead of the asset pipeline. Sass already compiles all CSS files into
one in the development environment.
All the code in the `bin/` and the `config/` folder has been generated
running `rake app:update`, except the `escape_javascript_fix` file,
which we've removed since the code there is already included in Rails
5.2.
This option was added by Rails 4 new application generator. However, the
`assets.digest` option is set to true by default, and recent Rails
versions don't even add this option to the environment files.
In commit 574133a5 we configured the development to use Dalli to cache
pages. However, cache is usually disabled in the development
environment.
When we upgraded to Rails 5 in commit eb36b7e2, we configured the
development environment to enable caching (using a memory store) when a
certain file is present, and to disable it when it's not. This
configuration makes more sense IMHO, and it was being overwritten by the
one previously mentioned.
After this change, using memcached is no longer required in the
development environment and the `DalliError: No server available` error
message is gone.
this is usually configured in the production.rb file (which is under
version control), the natural place to configure it is the secrets.yml
file.
Until now we were using the capistrano shared folder, but that's a bit
inconvenient since changes we've done to the production.rb file (like
changing eager_load_paths when we upgraded to Rails 5) won't take effect
after a deployment.
Since SMTP passwords should not be in a file under version control, and
they're usually configured in the production.rb file (which is under
version control), the natural place to configure it is the secrets.yml
file.
Until now we were using the capistrano shared folder, but that's a bit
inconvenient since changes we've done to the production.rb file (like
changing eager_load_paths when we upgraded to Rails 5) won't take effect
after a deployment.
We were very inconsistent regarding these rules.
Personally I prefer no empty lines around blocks, clases, etc... as
recommended by the Ruby style guide [1], and they're the default values
in rubocop, so those are the settings I'm applying.
The exception is the `private` access modifier, since we were leaving
empty lines around it most of the time. That's the default rubocop rule
as well. Personally I don't have a strong preference about this one.
[1] https://rubystyle.guide/#empty-lines-around-bodies
Metrics/LineLength: Line is too long.
RSpec/InstanceVariable: Use let instead of an instance variable.
Layout/TrailingBlankLines: Final newline missing.
Style/StringLiterals: Prefer double-quoted strings.
Keep a blank line before and after private
Keep a blank line before and after protected
Remove extra empty line at class body end
Remove extra blank line
Add final newline
Use 2 (not 3) spaces for indentation
Use 2 (not 4) spaces for indentation
Remove space before comma
Add space after comma
Remove trailing whitespaces
Remove unnecessary spacing
Use snake_case for variable names
Do not use then for multi-line if
Remove unused block argument - i
Use the new Ruby 1.9 hash syntax
Remove unused assignment to variable
Indent when as deep as case
Align attributes
Align end with def
