-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Implementation/64524 recalculate when a score of a scored list is changed within the administration #20511
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
base: implementation/64523-allow-usage-of-custom-fields-of-type-hierarchy-with-a-value-within-a-calculated-values-formula
Are you sure you want to change the base?
Conversation
a69cd9f
to
eeb3263
Compare
Rendering "#{id} not found" does not make a lot of sense.
|
||
return unless custom_field.field_format_scored_list? | ||
|
||
custom_field.projects.each do |project| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This explicitly binds to projects maybe better to do it like in CustomFields::UpdateService
.
Or maybe even go through custom values of this field that have value set to the deleted/updated item id
item = cached_typed_value | ||
|
||
item.score.presence || item | ||
item&.score.presence || item |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be in #20454 :)
|
||
if item.nil? | ||
"#{value} #{I18n.t(:label_not_found)}" | ||
"" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed?
Ticket
https://community.openproject.org/wp/64524
Important
This branch is based on #20454. Either merge that one first - or merge this branch into it instead of
dev
.What are you trying to accomplish?
When the score of a scored list is changed within the administration, all Calculated Values that are using the scored list should be recalculated.
Notes
In case I have to take my parental leave in the near future, here are some notes on the current state of this PR:
hierarchy_strategy.rb
). But I did not yet double check if that broke the UI somewhere else. Please be aware that the strategy is not only used by Scored Lists, but also by Hierarchies. If Hierarchies need the error string, we could simply include a type check and render a different formatted value.hierarchical_item_service_spec.rb
) - but I am not happy with them. They utilize the database quite a lot. I feel like it should be possible without doing so - but then, I had to mock too much for my taste. Maybe you have a better idea of how to do this.scored_lists_and_calculated_values_spec.rb
TL;DR: Everything should work. Specs ugly. Please double-check if there are other cases in which recalculation might be needed for Scored Lists. Thank you!
Screenshots
scored.lists.update.when.score.changes.mov
Merge checklist