Fix assigning staff members to budgets #3807
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
javierm
force-pushed
the
fix_budget_staff
branch
from
249cf3f
to
4b30692
Compare
javierm
force-pushed
the
fix_budget_staff
branch
from
4b30692
to
11b16f8
Compare
javierm
force-pushed
the
fix_budget_staff
branch
3 times, most recently
from
32ea7b7
to
7c514dd
Compare
javierm
force-pushed
the
fix_budget_staff
branch
from
7c514dd
to
3ccccaf
Compare
houndci-bot
reviewed
Oct 31, 2019
javierm
force-pushed
the
fix_budget_staff
branch
from
09c40b8
to
8b24b86
Compare
javierm
changed the title
javierm
force-pushed
the
fix_budget_staff
branch
2 times, most recently
from
8dc32f1
to
e2a0533
Compare
houndci-bot
reviewed
Oct 31, 2019
javierm
force-pushed
the
fix_budget_staff
branch
4 times, most recently
from
6b74a13
to
649bc36
Compare
They weren't caught by `i18n-tasks` because there are places were we use
`t("budgets.edit.#{variable}"`, which marks as used all translations
starting with `budgets.edit`
We need to add a hidden field for each group of check boxes, so if we don't check anything, the hidden field is sent to the server, indicating nothing was selected. Without the hidden field, the server will not know anything has been done to the check boxes. The easiest way to do it is using `collection_check_boxes`, which also adds labels to every check box.
Same result, with less code.
javierm
force-pushed
the
fix_budget_staff
branch
from
649bc36
to
e21bfff
Compare
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.
Since budgets now have milestone tags, the name of this method was confusing and will conflict with the name generated by acts_as_taggable. Note the new name could be improved too.
We were adding columns to the budgets table instead of using the same logic we use everywhere else.
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.
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.
If we didn't run this task, investments for existing budgets wouldn't show their administrator/valuators as an option when we're editing them, leading to data loss.
javierm
force-pushed
the
fix_budget_staff
branch
from
e21bfff
to
9b511ed
Compare
smarques
pushed a commit
to venetochevogliamo/consul
that referenced
this pull request
Apr 29, 2020
Fix assigning staff members to budgets
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Objectives
BudgetRolAssignmentmodelNotes
Before removing tags from budgets, I made them work the way we usually use tags in the application. So if we want to add them back in the future, we can use this implementation as a reference.