[Peterborough][WW] Add text messaging option for bulky waste #4887

MorayMySoc · 2024-03-15T11:47:52Z

Adds text box to opt into text updates for bulky waste bookings.

Allows multiple govuk_notify templates/keys to be added in array

https://github.com/mysociety/societyworks/issues/3782

[skip changelog]

codecov · 2024-03-15T11:52:35Z

Codecov Report

Attention: Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.61%. Comparing base (399a997) to head (fdf0144).
Report is 546 commits behind head on master.

Files with missing lines	Patch %	Lines
perllib/FixMyStreet/Cobrand/UKCouncils.pm	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4887      +/-   ##
==========================================
+ Coverage   82.57%   82.61%   +0.04%     
==========================================
  Files         393      393              
  Lines       30697    30726      +29     
  Branches     4861     4869       +8     
==========================================
+ Hits        25347    25385      +38     
+ Misses       3898     3892       -6     
+ Partials     1452     1449       -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

davea

Functionally this is nearly there, but I've made a few comments about ways the code could be improved/tidied up a bit.

davea · 2024-03-18T15:57:53Z

perllib/FixMyStreet/Cobrand/Peterborough.pm

+    sprintf("%s\n\n
+        Date: %s
+        Items: %d
+        %s
+        View more details or cancel: %s",
+        $title,
+        $self->bulky_nice_collection_date($report->get_extra_field_value('DATE')),
+        scalar grep ({ $_->{name} =~ /^ITEM/ && $_->{value} } @{$report->get_extra_fields}),
+        $address,
+        $h->{url});


Please indent this.

davea · 2024-03-18T15:59:48Z

perllib/FixMyStreet/Queue/Item/Report.pm

@@ -344,6 +344,9 @@ sub _add_confirmed_update {
 sub _send_report_sent_email {
    my $self = shift;

+    if ($self->report->get_extra_field_value('bulky_text_updates')) {


Could this instead live in Peterborough::post_report_sent or Peterborough::open311_post_send perhaps? It'd be nice to keep cobrand-specific bits out of here if possible.

davea · 2024-03-19T10:18:08Z

perllib/FixMyStreet/App/Form/Waste/Bulky.pm

    title => 'About you',
    next => 'choose_date_earlier',
+    update_field_list => sub {
+        my $form = shift;
+        if ($form->c->cobrand->moniker eq 'peterborough') {


It would be better to have a COBRAND_FEATURES flag for the new feature rather than hardcoding the peterborough check everywhere. This would make it easier to roll out to other cobrands later and also means we can turn it on/off for Peterborough with a redeploy rather than code changes.

davea · 2024-03-19T10:19:46Z

perllib/FixMyStreet/Roles/CobrandBulkyWaste.pm

@@ -552,10 +552,14 @@ sub bulky_reminders {
 sub _bulky_send_reminder_email {
    my ($self, $report, $h, $params) = @_;

-    return unless $report->user->email;
+    return unless $report->user->email || $report->get_extra_field_value('bulky_text_updates');


This is inside a function called _bulky_send_reminder_email so shouldn't have anything to do with sending texts - can you factor this (and the below call_hook lines) into a new _bulky_send_reminder_text function which gets called independently?

davea · 2024-03-19T10:22:19Z

t/app/controller/waste_peterborough_bulky.t

@@ -64,6 +66,7 @@ create_contact(
    { code => 'payment' },
    { code => 'payment_method' },
    { code => 'property_id' },
+    { code => 'bulky_text_updates' },


This is probably me not paying close enough attention but it took me ages of wondering why the feature wasn't working before I spotted this line and realised I also needed to add a bulky_text_updates hidden field to my existing Bulky collection category. Always helpful to please include notes for the reviewer when adding code that needs configuration/changes beyond checking out the branch.

davea · 2024-03-19T10:26:50Z

perllib/FixMyStreet/App/Form/Waste.pm

-        unless $email->is_inactive || $email->value || ($is_staff_user && !$staff_provide_email);
-
+        unless $email->is_inactive || $email->value || ($is_staff_user && !$staff_provide_email)
+        || (FixMyStreet::SMS->parse_username($phone->value)->{may_be_mobile} && $cobrand eq 'peterborough');


This line seems to make the email address optional on the "About you" page if I provide a mobile number, but I can't complete a booking without an email address:

Fix to this in ba1e015 May need more discussion...

Don't think that fix has had the desired effect - if bulky_offer_text_updates isn't present in waste_features this still allows the user to omit their email address on the "about you" page and they'll still see the above error when trying to go to the payment screen.

davea · 2024-03-19T10:31:28Z

perllib/FixMyStreet/App/Form/Waste/AboutYou.pm

@@ -62,6 +62,14 @@ has_field email => (
    },
 );

+has_field extra_bulky_text_updates => (


The wording here isn't very clear, what about something like:

davea · 2024-03-25T13:53:16Z

perllib/FixMyStreet/App/Form/Waste/AboutYou.pm

@@ -62,6 +62,14 @@ has_field email => (
    },
 );

+has_field extra_bulky_text_updates => (
+    type => 'Checkbox',
+    label => 'Bulky text updates',


I still think the inclusion of 'updates' here is misleading as the message aren't really updates - they're reminders.

davea · 2024-03-25T13:57:45Z

perllib/FixMyStreet/App/Form/Waste.pm

-        unless $email->is_inactive || $email->value || ($is_staff_user && !$staff_provide_email);
-
+        unless $email->is_inactive || $email->value || ($is_staff_user && !$staff_provide_email)
+        || (FixMyStreet::SMS->parse_username($phone->value)->{may_be_mobile} && $cobrand eq 'peterborough');


Don't think that fix has had the desired effect - if bulky_offer_text_updates isn't present in waste_features this still allows the user to omit their email address on the "about you" page and they'll still see the above error when trying to go to the payment screen.

Adds text message request checkout to bulky waste details page that is active only for Peterborough mysociety/societyworks#3782

Checks if govuk_notify is an array which will contain hashes that now include type. If the config is not an array continues as previously, otherwise will choose which config to use based on data assigned to notify_choice Related to mysociety/societyworks#3782

If the user chose to receive updates by phone when making their bulky waste booking, send text updates for 3 and 1 day reminders. Continue to send emails if there is an email address. mysociety/societyworks#3782

If the user has elected to receive optional texts for updates to their booking, they will be sent a text when the bulky booking is confirmed. Normal email confirmation also takes place. mysociety/societyworks#3782

Adds tests for optional text messaging for Bulky waste bookings mysociety/societyworks#3782

…o Bulky

Renaming to be clearer about what it's for which is for sending reminders about bulky waste collections which have been ordered.

MorayMySoc requested a review from davea March 15, 2024 11:48

davea requested changes Mar 19, 2024

View reviewed changes

MorayMySoc requested a review from davea March 20, 2024 14:37

davea requested changes Mar 25, 2024

View reviewed changes

MorayMySoc requested a review from davea April 5, 2024 14:18

davea approved these changes Apr 9, 2024

View reviewed changes

MorayMySoc added 13 commits May 17, 2024 11:01

[Peterborough][WW] Adds text message request checkbox to Bulky

35c1e3c

Adds text message request checkout to bulky waste details page that is active only for Peterborough mysociety/societyworks#3782

[Peterborough][WW] Optionally send bulky waste text updates

a958b5a

If the user chose to receive updates by phone when making their bulky waste booking, send text updates for 3 and 1 day reminders. Continue to send emails if there is an email address. mysociety/societyworks#3782

[Peterborough][WW] Send optional text on bulky waste confirmation

9ee526d

If the user has elected to receive optional texts for updates to their booking, they will be sent a text when the bulky booking is confirmed. Normal email confirmation also takes place. mysociety/societyworks#3782

[Peterborough][WW] Add tests for text sending

adb120d

Adds tests for optional text messaging for Bulky waste bookings mysociety/societyworks#3782

fixup! [Peterborough][WW] Adds text message request checkbox to Bulky

ef21d07

fixup! [Peterborough][WW] Adds text message request checkbox to Bulky

fb888cb

fixup! [Peterborough][WW] Optionally send bulky waste text updates

95a5710

fixup! [Peterborough][WW] Send optional text on bulky waste confirmation

7c08e4b

fixup! [Peterborough][WW] Add tests for text sending

46750be

fixup! fixup! [Peterborough][WW] Adds text message request checkbox t…

16e2ff3

…o Bulky

[Peterborough][WW] Rename extra_bulky_text_updates field

fc79667

Renaming to be clearer about what it's for which is for sending reminders about bulky waste collections which have been ordered.

fixup! [Peterborough][WW] Adds text message request checkbox to Bulky

cea77c2

mysociety-pusher force-pushed the 3782-peterborough-bulky-text-messaging branch from 2932a18 to cea77c2 Compare May 17, 2024 10:18

dracos added the Awaiting approval label Jun 14, 2024

[Peterborough] Bulky text updates/staff only.

fdf0144

dracos force-pushed the master branch from d011e81 to e84e4a6 Compare November 20, 2024 09:36

dracos force-pushed the master branch from 5bf556d to bcce64b Compare December 4, 2024 18:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Peterborough][WW] Add text messaging option for bulky waste #4887

[Peterborough][WW] Add text messaging option for bulky waste #4887

MorayMySoc commented Mar 15, 2024 •

edited

Loading

codecov bot commented Mar 15, 2024 •

edited

Loading

davea left a comment

davea Mar 18, 2024

davea Mar 18, 2024

davea Mar 19, 2024

davea Mar 19, 2024

davea Mar 19, 2024

davea Mar 19, 2024

MorayMySoc Mar 20, 2024

davea Mar 25, 2024

davea Mar 19, 2024

davea Mar 25, 2024

davea Mar 25, 2024

[Peterborough][WW] Add text messaging option for bulky waste #4887

Are you sure you want to change the base?

[Peterborough][WW] Add text messaging option for bulky waste #4887

Conversation

MorayMySoc commented Mar 15, 2024 • edited Loading

codecov bot commented Mar 15, 2024 • edited Loading

Codecov Report

davea left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MorayMySoc commented Mar 15, 2024 •

edited

Loading

codecov bot commented Mar 15, 2024 •

edited

Loading