[Merton][Echo] Amend Bulky collection on same date #5474
Conversation
When amending a bulky booking but keeping the same collection date, don't cancel the report. Use an update on the report to send the data to Open311 and do the update to same booking. mysociety/societyworks#4732
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5474 +/- ##
==========================================
+ Coverage 82.30% 82.40% +0.10%
==========================================
Files 428 428
Lines 34053 34861 +808
Branches 5511 5613 +102
==========================================
+ Hits 28026 28726 +700
- Misses 4396 4493 +97
- Partials 1631 1642 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
This all looks good, made a few small comments.
It is missing one thing however - the fact that if the current date of the booking isn't included in the list of slot dates returned by Echo then it isn't shown to the user and they can't keep the same date. We need to make sure the current chosen date is always included in the list of returned dates for an amendment, so they can choose not to change it.
| extra => $p->extra, | ||
| }); | ||
| $c->forward( '/admin/log_edit', [ $c->cobrand->body->comment_user->id, 'problem', 'edit' ] ); | ||
| $p->photo(''); |
There was a problem hiding this comment.
Is this line needed? Looks like amend_extra_data always sets $p->photo anyway.
| } | ||
| }; | ||
| $p->set_extra_fields(@fields); | ||
| $p->update; |
There was a problem hiding this comment.
Is this line needed, as you call $p->update soon after anyway after amend_extra_data
| sub update_report_for_same_day_amend { | ||
| my ($self, $p, $data) = @_; | ||
|
|
||
| my $collection_date = $p->get_extra_field_value('Collection_Date'); |
There was a problem hiding this comment.
Note that this can be different per cobrand - Kingston/Sutton it is now Collection_Date_-_Bulky_Items (might need to also default back to this for old reports though, sorry).
| category => $p->category, | ||
| extra => $p->extra, | ||
| }); | ||
| $c->forward( '/admin/log_edit', [ $c->cobrand->body->comment_user->id, 'problem', 'edit' ] ); |
There was a problem hiding this comment.
I don't think we need this line - the Peterborough else case a few lines below doesn't have it, and the moderation entry is logging the change. Unless you think Peterborough should have it as well?
| $p->add_to_comments( { | ||
| text => 'Amend ' . $p->category, | ||
| user => $c->cobrand->body->comment_user, | ||
| problem_state => $p->state, |
There was a problem hiding this comment.
I wonder if we want to set a flag on this rather than rely on precise comment text.
| $self->save_item_names_to_report($data); | ||
|
|
||
| my @fields = @{$p->get_extra_fields}; | ||
| for my $field (@fields) { |
There was a problem hiding this comment.
Rather than this way round, could this loop over all the fields perhaps be instead:
foreach (qw(Bulky_Collection_Bulky_Items Bulky_Collection_Notes
TEM_-_Bulky_Collection_Description TEM_-_Bulky_Collection_Item Exact_Location)) {
if ($data->{"extra_$_"}) {
$p->update_extra_field({ name => $_, value => $data->{"extra_$_"} });
}
}
Does that work okay?
There was a problem hiding this comment.
Could even put the cobrand specific fields in a cobrand function and then call it for the list here and in your open_311_bulky_update_same_date perhaps
There was a problem hiding this comment.
Oh, sorry, just thought of another thing, sigh - if someone amends a booking and adds items that brings it above the lower payment threshold, they have to pay more. The way the current cancellation works is it creates unconfirmed cancellation update, and new report, then awaits payment confirmation before confirming the cancellation and the payment on the new report.
With this new way, hmm. We can in theory create the unconfirmed comment the same way, so it won't update Echo until payment confirmed (and we'll have to update the payment amount then paid on the report as well for any further amendments); but that still leaves the changing of the actual report with the new information. Do we... store the new data on the comment and then if payment is confirmed (or no payment necessary) copy the new data to the existing report at that point? Not sure I can think of an easier way.
When amending a bulky booking but keeping the same collection date, don't cancel the report.
Use an update on the report to send the data to Open311 and do the update to same booking.
https://github.com/mysociety/societyworks/issues/4732