[DNM] Edit multiple measures #724
Conversation
Codecov ReportBase: 93.52% // Head: 93.50% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #724 +/- ##
==========================================
- Coverage 93.52% 93.50% -0.02%
==========================================
Files 345 349 +4
Lines 23095 23366 +271
Branches 1787 1819 +32
==========================================
+ Hits 21600 21849 +249
- Misses 1160 1177 +17
- Partials 335 340 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| return constants.CONDITIONS in cleaned_data.get("fields_to_edit") | ||
|
|
||
|
|
||
| def show_step_footnotes(wizard): |
There was a problem hiding this comment.
Could these be refactored into one generic method?
There was a problem hiding this comment.
After refactoring it might be worth considering whether these belong in their own file or whether they should sit on/above a class somewhere
There was a problem hiding this comment.
Agree it'd be nice to be able to parametrise these with the different step names but due to how formtools is expecting them to be passed to the view (as a dictionary in urls.py) I don't think this is an easy fix. Maybe a future tech debt ticket?
There was a problem hiding this comment.
Ah yeah that's a bit awkward. I feel like there's a way to do it with lambdas, but might be worth creating ticket, as you say
| from measures.util import diff_components | ||
|
|
||
|
|
||
| def update_measure(instance, transaction, current_workbasket, cleaned_data, defaults): |
There was a problem hiding this comment.
I'm wondering if these methods should live on a class, perhaps a MeasureUpdatePattern or MeasureEditPattern, in patterns.py. I think they're all part of a similar process
There was a problem hiding this comment.
Yeah that would be good. I'll see if I can get that done before I go on leave at the end of the week
eadpearce
force-pushed
the
TP2000-553-edit-multiple-measures
branch
from
7ad54e3
to
6042d51
Compare
| ] | ||
| conditions_data = cleaned_data.get("formset-conditions", []) | ||
| for condition in conditions_data: | ||
| condition["update_type"] = UpdateType.CREATE |
There was a problem hiding this comment.
What happens when we want to update existing conditions on the measure?
There was a problem hiding this comment.
This form currently only supports adding additional conditions.
There was a problem hiding this comment.
I've been thinking about this as a separate ticket - and we need to support editing/removing geo area exclusions - I wonder if a small interim addition would be a "Remove" button to remove Geo Areas and Measure Conditions for now. And then (in the same form) to re-enter the correct information
There was a problem hiding this comment.
I'm not sure that the ability to add additional conditions is useful without the ability to remove or reorder the existing ones
| "component_measure", | ||
| ) | ||
|
|
||
| return instance.new_version(current_workbasket) |
There was a problem hiding this comment.
You need to pass the the fields you want to update to this method. i.e. instance.new_version(current_workbasket, **cleaned_data)
There was a problem hiding this comment.
You'll see:
TypeError: Measure() got an unexpected keyword argument 'fields_to_edit'
and then I imagine it will complain about M2M fields like conditions and duties, so you'll need to find a way to filter those out before passing the dict
| # 1 condition | ||
| # 1 footnote | ||
| assert workbasket.tracked_models.count() == 6 | ||
|
|
There was a problem hiding this comment.
I know this test is already long, but it would be good if you could loop over both measures and make assertions about individual fields (e.g. valid_between.upper)
TP-2000-553: Edit multiple measures
Why
What
Checklist