[Bexley] [Waste] WIP show existing subscription info #5346

davea · 2025-01-28T09:40:57Z

[skip changelog]

For https://github.com/mysociety/societyworks/issues/4666

codecov · 2025-01-28T09:48:07Z

Codecov Report

Attention: Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.

Project coverage is 82.44%. Comparing base (71b7645) to head (df0cd37).

Files with missing lines	Patch %	Lines
perllib/FixMyStreet/Cobrand/Bexley/Garden.pm	94.44%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@                   Coverage Diff                    @@
##           bexley-ww-ggw-cancel    #5346      +/-   ##
========================================================
+ Coverage                 82.41%   82.44%   +0.02%     
========================================================
  Files                       420      420              
  Lines                     33018    33042      +24     
  Branches                   5291     5294       +3     
========================================================
+ Hits                      27213    27241      +28     
+ Misses                     4246     4242       -4     
  Partials                   1559     1559

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

nephila-nacrea · 2025-02-04T23:17:43Z

perllib/FixMyStreet/Cobrand/Bexley/Garden.pm

+    return undef unless $results && $results->{Customers};
+    my $customer = $results->{Customers}[0];
+    return undef unless $customer && $customer->{ServiceContracts};
+    my $contract = $results->{ServiceContracts}[0];


I think this needs to be $customer->{ServiceContracts}[0].

nephila-nacrea · 2025-02-04T23:21:30Z

perllib/FixMyStreet/Cobrand/Bexley/Garden.pm

@@ -30,14 +35,27 @@ sub garden_current_subscription {
    my $uprn = $self->{c}->stash->{property}{uprn};
    return undef unless $uprn;

-    my $is_free = $self->agile->IsAddressFree($uprn);
-    return undef if $is_free->{IsFree} eq 'True';
+    my $results = $self->agile->CustomerSearch($uprn);


I think we mentioned doing a check against the current subscription that is in the DB and making sure the customer email & external_id match the Customer and ServiceContract returned from Agile.
I'm not sure if it would be better to do this match as part of your PR, as strictly speaking it's not necessary to get cancellation working (am also worried about me making too many changes on my PR that then mess with your work here). I think get_original_sub() in Controller/Waste.pm needs updating so it searches on Bexley's uprn field as well as property_id.

nephila-nacrea · 2025-02-04T23:22:11Z

perllib/Integrations/Agile.pm

@@ -65,4 +65,14 @@ sub IsAddressFree {
    );
 }

+sub CustomerSearch {


Thanks for this 🙏 I've copied it over to my cancellation PR.

nephila-nacrea · 2025-02-04T23:27:49Z

perllib/FixMyStreet/Cobrand/Bexley/Garden.pm


    # Agile says there is a subscription; now get service data from
    # Whitespace
    my $services = $self->{c}->stash->{services};
-    map { my $srv = $services->{$_}; return $srv if $srv }
-        @{ $self->garden_service_ids };
+    for my $id (@{ $self->garden_service_ids }) {


Not sure if it's something that needs solving right now, but I am not sure of the best way to handle the Whitespace/Agile split; e.g. if we have Agile data for the contract but not Whitespace. Perhaps how we're doing it now will suffice 🤔

In my cancellation PR I have extended the agile_only bit below to include a couple more fields like customer_external_ref and end_date (I think this can be used as the 'DueDate' required by the cancel call, but I'll need to check that with Luis). I guess I should add those to the Whitespace $srv data here as well.

nephila-nacrea · 2025-02-11T21:23:04Z

perllib/FixMyStreet/Cobrand/Bexley/Garden.pm


-    my $uprn = $self->{c}->stash->{property}{uprn};
-    return undef unless $uprn;
+        email => undef,


Do you ever populate email?

nephila-nacrea · 2025-02-11T21:35:09Z

perllib/FixMyStreet/Cobrand/Bexley/Garden.pm

+    $sub->{bins_count} = $contract->{WasteContainerQuantity};
+
+    my $c = $self->{c};
+    my $p = $c->model('DB::Problem')->search({


I think we may need to change get_original_sub in Controller/Waste as well (or special-case it so it calls this subroutine for Bexley, to get the 'row' bit).
The current Waste code for other cobrands makes use of both get_original_sub and garden_current_subscription, looks like the former is for DB lookup and the latter for getting the garden service from the list of services...? I don't know if we should be keeping this distinction here.

nephila-nacrea · 2025-02-11T23:38:16Z

perllib/FixMyStreet/Cobrand/Bexley/Garden.pm

+            $srv->{end_date} = $sub->{end_date};
+            $srv->{garden_bins} = $sub->{bins_count};
+            $srv->{garden_cost} = $sub->{cost};
+            $self->{c}->stash->{orig_sub} = $sub->{row};


Should this be set in lookup_subscription_for_uprn, at the $sub->{row} = $p if $p; bit?

nephila-nacrea · 2025-02-12T12:40:52Z

perllib/FixMyStreet/Cobrand/Bexley/Garden.pm

-                = $customer->{CustomerExternalReference};
-            $srv->{end_date} = $end_date;
+        if ( my $srv = $service_ids->{$_} ) {
+            my $sub = $self->lookup_subscription_for_uprn($uprn);


You can pull this out of the loop and then you don't need to call it a second time below.

nephila-nacrea · 2025-02-12T12:46:35Z

perllib/FixMyStreet/Cobrand/Bexley/Garden.pm

    };
+    push @$services, $service;
+    $self->{c}->stash->{property}{garden_current_subscription} = $service;


I am not sure you need this line if it is already set Cobrand/Bexley/Waste.pm below. (Or, perhaps better to get rid of it there and keep it here? But you may still need to call garden_current_subscription, but not assign it to anything.)

nephila-nacrea force-pushed the bexley-ww-ggw-cancel branch from d0cee9b to e8dfeed Compare February 2, 2025 18:40

davea force-pushed the bexley-ww-ggw-show-existing branch from 004985b to 4e5eb23 Compare February 4, 2025 12:13

nephila-nacrea reviewed Feb 4, 2025

View reviewed changes

nephila-nacrea force-pushed the bexley-ww-ggw-cancel branch 5 times, most recently from 38ec8f8 to 6029fd1 Compare February 6, 2025 11:14

davea force-pushed the bexley-ww-ggw-show-existing branch from 4e5eb23 to af8001d Compare February 9, 2025 21:21

nephila-nacrea reviewed Feb 11, 2025

View reviewed changes

nephila-nacrea requested changes Feb 11, 2025

View reviewed changes

nephila-nacrea requested changes Feb 14, 2025

View reviewed changes

davea added 5 commits February 14, 2025 10:45

[Bexley] WIP show existing GGW subscription

46cebf5

[Bexley] Show GGW cancel link to all

e4bb6c9

[Bexley] Show current payment on GGW subscription

c3566d6

[Bexley] Add lookup for subscription from database

755f1f9

[Bexley] Show existing GGW sub from Agile if it’s not in Whitespace

df0cd37

davea force-pushed the bexley-ww-ggw-show-existing branch from 353ed2f to df0cd37 Compare February 14, 2025 10:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bexley] [Waste] WIP show existing subscription info #5346

[Bexley] [Waste] WIP show existing subscription info #5346

davea commented Jan 28, 2025

codecov bot commented Jan 28, 2025 •

edited

Loading

nephila-nacrea Feb 4, 2025

nephila-nacrea Feb 4, 2025

nephila-nacrea Feb 4, 2025

nephila-nacrea Feb 4, 2025

nephila-nacrea Feb 11, 2025

nephila-nacrea Feb 11, 2025

nephila-nacrea Feb 11, 2025

nephila-nacrea Feb 12, 2025

nephila-nacrea Feb 12, 2025

[Bexley] [Waste] WIP show existing subscription info #5346

Are you sure you want to change the base?

[Bexley] [Waste] WIP show existing subscription info #5346

Conversation

davea commented Jan 28, 2025

codecov bot commented Jan 28, 2025 • edited Loading

Codecov Report

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

codecov bot commented Jan 28, 2025 •

edited

Loading