When customizing CONSUL, one of the most common actions is adding a new
field to a form.
This requires modifying the permitted/allowed parameters. However, in
most cases, the method returning these parameters returned an instance
of `ActionController::Parameters`, so adding more parameters to it
wasn't easy.
So customizing the code required copying the method returning those
parameters and adding the new ones. For example:
```
def something_params
params.require(:something).permit(
:one_consul_attribute,
:another_consul_attribute,
:my_custom_attribute
)
end
```
This meant that, if the `something_params` method changed in CONSUL, the
customization of this method had to be updated as well.
So we're extracting the logic returning the parameters to a method which
returns an array. Now this code can be customized without copying the
original method:
```
alias_method :consul_allowed_params, :allowed_params
def allowed_params
consul_allowed_params + [:my_custom_attribute]
end
```
Having to wait for a whole page refresh after updating each setting was
painful when modifying several settings.
Even though the navigation is updated immediately to reflect which
sections have been enabled/disabled, there's one gotcha. Changing the
"SDG" setting will not update the user menu (which contains a link to
SDG content) nor the "SDG Configuration" tab; refreshing the page will
be necessary to check these changes. The same happens with the map and
remote census tabs. So in these cases we're making an exception and
sending the form. We might find a better solution in the future.
For this reason, we aren't using the `switch` ARIA role. Some users
might not expect a switch control to refresh the page, just like they
usually don't expect checkboxes to refresh the page. Furthermore, screen
reader support for the `switch` role seems to be inconsistent. For
instance, NVDA with Chrome announces the control as a checkbox instead
of a switch.
Note AJAX is only used for feature settings. Other settings are still
updated with regular HTTP requests.
Since we're now using AJAX requests, we have to make sure to add an
expectation in the homepage tests in order to make sure the request has
finished before starting a new one.
We were inconsistent on this one. I consider it particularly useful when
a method starts with a `return` statement.
In other cases, we probably shouldn't have a guard rule in the middle of
a method in any case, but that's a different refactoring.
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.
Currently after each update of any Settings is redirected to the first
tab by default.
As this new tab remote_census_configuation has a lot of fields to fill
in it is a bit uncomfortable to have to go back to the tab after each
update.
- Add hidden field :tag to set current tag value
- After update add tag value to request.referer
- To avoid errors when partial call has not param :tag, add the "define?"
method on hidden_field value.
- 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.
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
