-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow Budget Group names to be edited #2504
Merged
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
…he budget is still inthe drafting phase
Backport on hold, this doesn't work on preproduction environment AyuntamientoMadrid#1319 |
Closing until madrid's fork gets it right AyuntamientoMadrid#1361 |
We were having a problem were some groups where not updating correctly. That was because it was finding the first group with that name. However we were looking for another group with the same name from another budget Apart from the group_id, we also get the budget_id in the params for updating a group. By finding groups within that budget we get the expected behaviour
To maintain order after editing a group’s name We should probably add timestamps to `groups` and `headings` and order by `created_at` instead
To maintain order after editing a heading’s name We should probably add timestamps to `groups` and `headings` and order by `created_at` instead Could have done it in a separate PR, but gotta keep moving to other priority issues. Creating issue for timestamps to do correctly and with tests 😌
Changing a group’s `to_param` to return the slug instead of the id, breaks many tests in the user facing interface We should use slugs in upstream soon, but it should be done in a separate PR, bringing the whole slug implementation from Madrid’s fork and the corresponding test coverage
No need for deprecation warnings in release notes, this is an attribute that was used temporarily but did not make it to a Release
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.
Where
What
This is a backport from Madrid's fork AyuntamientoMadrid#1265 that will close issue #2438
Deployment
As usual
Warnings
None