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.
The interface was a bit confusing, since after clicking on "See
unfeasible investments" (or similar), we were on a page where no
investments were shown.
Besides, since commit 7e3dd47d5, the group page is only linked from the
"my ballot" page, through a link inviting the user to vote in that
group, and it's only possible to vote selected investments (which is the
default filter during the final voting phase).
The only reason we had these links here was these links weren't present
in the investments page. But they're present there since commit
04605d5d5, so we don't need them in the group page anymore.
There was a big difference between the current budget and a specific
budget landing page. This didn't really make too much sense. Also, it
was not possible to know how a draft participatory budget will look
before it was published.
By unifying those two views now they will look quite similar and it
will be possible for administrators to preview any draft budget and to
know how the budget will look like before actually publishing it.
Now it's easier to change the investments filter. Previously we had to
go back to the budget index page, change the filter there, and then
select one heading.
Now the links to change the current filter in the budget index page
aren't needed anymore.
Previously the draft mode was a phase of the PB, but that had some
limitations.
Now the phase drafting disappears and therefore the PB can have the
status published or not published (in draft mode).
That will give more flexibility in order to navigate through the
different phases and see how it looks for administrators before
publishing the PB and everybody can see.
By default, the PB is always created in draft mode, so it gives you
the flexibility to adjust and modify anything before publishing it.
We had helper methods all over the place which were only used in one
view. Since we're going to change some of them to use the budget as a
parameter, they don't belong in those helpers anymore.
Note the method `budget_heading_select_options` is used in more places,
so we're keeping it where it was.
Tags and help links can be edited, but aren't used anywhere. Since we
don't know what the intended behavior was, I'm removing them for now.
My best guess is tags were supposed to be used so investments for a
budget can only be assigned tags present in the budget. Achieving that
behavior wouldn't be a trivial task.
We were ordering one group of tags alphabetically and then adding
another group of tags which wasn't ordered alphabetically, which didn't
make much sense.
We were manually doing the same thing, generating inconsistent results,
since the method `valuation_tag_list` was using the `valuation` context,
when actually the expected behavior would be to use the `valuation_tag`
context.
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.
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
DEPRECATION WARNING: Method symbolize_keys is deprecated and will be
removed in Rails 5.1, as `ActionController::Parameters` no longer
inherits from hash. Using this deprecated behavior exposes potential
security problems. If you continue to use this method you may be
creating a security vulnerability in your app that can be exploited.
Instead, consider using one of these documented methods which are not
deprecated:
http://api.rubyonrails.org/v5.0.4/classes/ActionController/Parameters.ht
ml (called from csv_params at
/home/travis/build/consul/consul/app/helpers/budgets_helper.rb:15)
Move the button so that it appears in the Winners tab of the
budget_investments index.
When the budget is in "Balloting projects", "Reviewing Ballots"
or "Finished budget" phases, the button will appear as a button
(clickable). If there are no winners calculated yet, the text
will be "Calculate winner investments"; if there are, the text
will be "Recalculate winner investments".
If the budget is in another phase, the button will be
disabled and a message will appear: 'The budget has to stay on
phase "Balloting projects", "Reviewing Ballots" or
"Finished budget" in order to calculate winners projects'
When there are no budgets we were seeing an exception in the budgets’
index
There are two parts to take into account here:
1) Making sure there is a current_budget present, otherwise we display
the “no budgets” message
2) The map helper is called from the controller, so we need to make
sure current_budget is present there too
Note: We could have added a bunch of `try` statements in the budgets’s
index, instead of using a conditional, however there are quite a few
`current_budget` calls so it seems more appropriate to use a conditional
