Before this change, two important things depend on the format of each key,
where to render it in the administration panel and which kind of interface
to use for each setting. Following this strategy led us to a very complex
code, very difficult to maintain or modify. So, we do not want to depend
on the setting key structure anymore to decide how or where to render each
setting.
With this commit, we get rid of the key format-based rules. Now we render
each setting explicitly passing to it the type and the tab where it belongs.
This rule was added in rubocop-rspec 2.9.0.
We were using `be_nil` 50% of the time, and `be nil` the rest of the
time. No strong preference for either one, but IMHO we don't lose
anything be being consistent.
Creating a schema takes about 3-4 seconds on my machine, so omitting
the callbacks makes tests much faster.
To do so, we're using the `insert!` method added in Rails 6.0, which
inserts a record without executing callbacks or validations. To make the
tests look consistent, we're adding a FactoryBot strategy which uses
`insert!` instead of `create!`.
Note this strategy is useless in most cases because it doesn't work when
models have translatable attributes or associations. However, IMHO it's
worth it even if we only use it for tenants.
We could also use `Tenant.insert!` instead, but then we would have to
add all the mandatory attributes, and in this case the code is clearer
if we only add the attributes we need for the test.
This way we'll avoid having all tenant organizations named "CONSUL" by
default, and we'll also use different email senders per tenant, which
will reduce the change of the emails being marked as spam.
We were using `consul.dev` as default, which might be fine for the
development environment, but it's something that definitely needs to be
changed on production.
Now we're providing a better default setting: using the same domain
which is used to generate URLs in emails. This also makes it easier to
configure new tenants, since the setting will default to whatever host
the new tenant is using.
The `|| "consul.dev"` part might not be needed; I'm keeping it because
I'm not sure production environents would not experience any new issues
if we don't add it, but we might remove it in the future.
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
Having exceptions is better than having silent bugs.
There are a few methods I've kept the same way they were.
The `RelatedContentScore#score_with_opposite` method is a bit peculiar:
it creates scores for both itself and the opposite related content,
which means the opposite related content will try to create the same
scores as well.
We've already got a test to check `Budget::Ballot#add_investment` when
creating a line fails ("Edge case voting a non-elegible investment").
Finally, the method `User#send_oauth_confirmation_instructions` doesn't
update the record when the email address isn't already present, leading
to the test "Try to register with the email of an already existing user,
when an unconfirmed email was provided by oauth" fo fail if we raise an
exception for an invalid user. That's because updating a user's email
doesn't update the database automatically, but instead a confirmation
email is sent.
There are also a few false positives for classes which don't have bang
methods (like the GraphQL classes) or destroying attachments.
For these reasons, I'm adding the rule with a "Refactor" severity,
meaning it's a rule we can break if necessary.
- Add :date_of_birth and :postal_code
- Only display new fields when aplication has configured the
custom census API and contains alias values for fields. Add 2
class Setting methods to check this feature:
- force_presence_date_of_birth?
- force_presence_postal_code?
- Render remote census configuration content on settings index.
- Update type method from Setting
On Admin::SettingsController#index we are using 'all_settings' to
group all settings by 'type' method.
'type' method return the first part of key when split by '.'
To allow use by example: all_settings["remote_census.general"]
and recover only settings related with this key we have added new
'elsif' on 'type' method.
