[Bexley] Always use $service->{last} for recent collection checks

There were a couple of places that were trying to get a recent_collection date using a round_schedule directly, but this doesn't work because some properties have multiple comma-separated schedules in the RoundSchedule field. The fix is to always use $service->{last}, which is calculated based on all of the rounds mentioned in the RoundSchedule field.
mysociety · Jun 12, 2024 · 05f371a · 05f371a
1 parent fb17323
commit 05f371a
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 45 deletions.
diff --git a/perllib/FixMyStreet/Cobrand/Bexley/Waste.pm b/perllib/FixMyStreet/Cobrand/Bexley/Waste.pm
@@ -262,6 +262,27 @@ sub bin_services_for_address {
             next if $now_dt > $to_dt;
         }
 
+        # Get the last collection date from recent collections.
+        #
+        # Some services may have two collections a week; these are concatenated
+        # together in $service->{RoundSchedule}. We need to split them so we
+        # can look them up individually in $property->{recent_collections}.
+        my @round_schedules = split /, /, $service->{RoundSchedule};
+
+        my $last_dt;
+        for (@round_schedules) {
+            my $dt_to_check = $property->{recent_collections}{$_};
+
+            if (
+                $dt_to_check
+                && (  !$last_dt
+                    || $dt_to_check > $last_dt )
+                )
+            {
+                $last_dt = $dt_to_check;
+            }
+        }
+
         my ($round) = split / /, $service->{RoundSchedule};
 
         # 'Next collection date' could be today; successful collection logs
@@ -275,11 +296,10 @@ sub bin_services_for_address {
         my $scheduled_for_today = $now_dt->ymd eq $next_dt->ymd;
 
         my $min_dt = $self->_subtract_working_days(WORKING_DAYS_WINDOW);
-        my $last_expected_collection_dt = $property->{recent_collections}{$service->{RoundSchedule}};
         # If the last expected collection was more than three days ago and
         # it's not scheduled for collection today, then we don't consider
         # it to have been collected today.
-        if ($last_expected_collection_dt < $min_dt && !$scheduled_for_today) {
+        if ($last_dt && $last_dt < $min_dt && !$scheduled_for_today) {
             $collected_today = 0;
         }
 
@@ -300,6 +320,13 @@ sub bin_services_for_address {
             assisted_collection => $service->{ServiceName} && $service->{ServiceName} eq 'Assisted Collection' ? 1 : 0,
         };
 
+        if ($last_dt) {
+            $filtered_service->{last} = {
+                date    => $last_dt,
+                ordinal => ordinal( $last_dt->day ),
+            };
+        }
+
         # Set some flags on property as well; these are used for missed
         # collection location options
         $property->{has_assisted} = 1
@@ -308,34 +335,6 @@ sub bin_services_for_address {
         $property->{above_shop} = 1
             if $filtered_service->{service_id} eq 'MDR-SACK';
 
-        # Get the last collection date from recent collections.
-        #
-        # Some services may have two collections a week; these are concatenated
-        # together in $service->{RoundSchedule}. We need to split them so we
-        # can look them up individually in $property->{recent_collections}.
-        my @round_schedules = split /, /, $service->{RoundSchedule};
-
-        my $last_dt;
-        for (@round_schedules) {
-            my $dt_to_check = $property->{recent_collections}{$_};
-
-            if (
-                $dt_to_check
-                && (  !$last_dt
-                    || $dt_to_check > $last_dt )
-                )
-            {
-                $last_dt = $dt_to_check;
-            }
-        }
-
-        if ($last_dt) {
-            $filtered_service->{last} = {
-                date    => $last_dt,
-                ordinal => ordinal( $last_dt->day ),
-            };
-        }
-
         # Frequency of collection
         if ( @round_schedules > 1 ) {
             $filtered_service->{schedule} = 'Twice Weekly';
@@ -577,10 +576,9 @@ sub can_report_missed {
 
     # Needs to be within 3 working days of the last completed round, today
     # not included.
-    # NOTE recent_collections shows the 'ideal'/expected collection date,
-    # not actual. So we need to check cab logs for actual collection date.
-    my $last_expected_collection_dt
-        = $property->{recent_collections}{ $service->{round_schedule} };
+    # NOTE last expected collection is the 'ideal'/expected collection date,
+    # not actual. So we need to check cab logs below for actual collection date.
+    my $last_expected_collection_dt = $service->{last} && $service->{last}{date};
 
     if ($last_expected_collection_dt) {
         # TODO We can probably get successful collections directly off the

diff --git a/t/app/controller/waste_bexley.t b/t/app/controller/waste_bexley.t
@@ -812,6 +812,9 @@ FixMyStreet::override_config {
                 next => {
                     is_today => 1,
                 },
+                last => {
+                    date => DateTime->today,
+                }
             },
             # Had a collection earlier today
             'FO-140' => {
@@ -821,34 +824,52 @@ FixMyStreet::override_config {
                 next => {
                     is_today => 1,
                 },
+                last => {
+                    date => DateTime->today,
+                },
             },
             # Collection due last working day but it did not happen
             'RES-180' => {
                 service_id => 'RES-180',
                 round => 'RES-R2',
                 round_schedule => 'RES-R2 Fri',
+                last => {
+                    date => DateTime->today->subtract( days => 3 ),
+                },
             },
             # Collections due last working day and they happened
             'RES-240' => {
                 service_id => 'RES-240',
                 round => 'RES-R3',
                 round_schedule => 'RES-R3 Fri',
+                last => {
+                    date => DateTime->today->subtract( days => 3 ),
+                },
             },
             'RES-660' => {
                 service_id => 'RES-660',
                 round => 'RES-R4',
                 round_schedule => 'RES-R4 Fri',
+                last => {
+                    date => DateTime->today->subtract( days => 3 ),
+                },
             },
             # Collection too old
             'GA-240' => {
                 service_id => 'GA-240',
                 round => 'GDN-R1',
                 round_schedule => 'GDN-R1 Tue',
+                last => {
+                    date => DateTime->today->subtract( days => 6 ),
+                },
             },
             'PG-240' => {
                 service_id => 'PG-240',
                 round => 'RCY-R2',
                 round_schedule => 'RCY-R2 Mon PG Wk 2',
+                last => {
+                    date => DateTime->today->subtract( days => 7 ),
+                },
             },
         );
 
@@ -857,17 +878,6 @@ FixMyStreet::override_config {
             missed_collection_reports => {
                 'RES-SACK' => 1,
             },
-            recent_collections => {
-                'RCY-R1 Mon' => DateTime->today, # FO-23
-                'RCY-R2 Mon' => DateTime->today, # FO-140
-                'RES-R2 Fri' => DateTime->today->subtract( days => 3 ), # RES-180
-                'RES-R3 Fri' => DateTime->today->subtract( days => 3 ), # RES-240
-                'RES-R4 Fri' => DateTime->today->subtract( days => 3 ), # RES-240
-                'GDN-R1 Tue' => DateTime->today->subtract( days => 6 ), # GA-240
-                'RCY-R2 Mon PG Wk 2' => DateTime->today->subtract( days => 7 ), # PG-240
-                'RCY-R1 Mon' => DateTime->today->subtract( days => 14 ), # FO-23
-                'RCY-R2 Mon' => DateTime->today->subtract( days => 14 ), # FO-140
-            },
         };
 
         my $cobrand = FixMyStreet::Cobrand::Bexley->new;