Filter contacts before constructing options.

If we wait until after to filter the contacts and the constructed
options, then we can end up in the situation where e.g. we've filtered
out all the TfL contacts but the category option is still the TfL one
(with its associated group).

5 files changed, 12 insertions, 20 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index cbbf8971a..ba37a25fa 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -691,6 +691,8 @@ sub setup_categories_and_bodies : Private {
       ->search( { 'me.body_id' => [ keys %bodies ] }, { prefetch => 'body' } );
     my @contacts = $c->cobrand->categories_restriction($contacts)->all_sorted;
 
+    $c->cobrand->call_hook(munge_report_new_contacts => \@contacts);
+
     # variables to populate
     my %bodies_to_list = ();       # Bodies with categories assigned
     my @category_options = ();       # categories to show
diff --git a/perllib/FixMyStreet/Cobrand/FixMyStreet.pm b/perllib/FixMyStreet/Cobrand/FixMyStreet.pm
index 6e0a0e2a5..fcbeebb48 100644
--- a/perllib/FixMyStreet/Cobrand/FixMyStreet.pm
+++ b/perllib/FixMyStreet/Cobrand/FixMyStreet.pm
@@ -83,8 +83,8 @@ sub munge_reports_categories_list {
     }
 }
 
-sub munge_report_new_category_list {
-    my ($self, $options, $contacts, $extras) = @_;
+sub munge_report_new_contacts {
+    my ($self, $contacts) = @_;
 
     my %bodies = map { $_->body->name => $_->body } @$contacts;
 
@@ -92,20 +92,16 @@ sub munge_report_new_category_list {
         my $user = $self->{c}->user;
         if ( $user && ( $user->is_superuser || $user->belongs_to_body( $bodies{'Isle of Wight Council'}->id ) ) ) {
             @$contacts = grep { !$_->send_method || $_->send_method ne 'Triage' } @$contacts;
-            my $seen = { map { $_->category => 1 } @$contacts };
-            @$options = grep { my $c = ($_->{category} || $_->category); $c =~ 'Pick a category' || $seen->{ $c } } @$options;
             return;
         }
 
         @$contacts = grep { $_->send_method && $_->send_method eq 'Triage' } @$contacts;
-        my $seen = { map { $_->category => 1 } @$contacts };
-        @$options = grep { my $c = ($_->{category} || $_->category); $c =~ 'Pick a category' || $seen->{ $c } } @$options;
     }
 
     if ( $bodies{'TfL'} ) {
         # Presented categories vary if we're on/off a red route
         my $tfl = FixMyStreet::Cobrand->get_class_for_moniker( 'tfl' )->new({ c => $self->{c} });
-        $tfl->munge_red_route_categories($options, $contacts);
+        $tfl->munge_red_route_categories($contacts);
     }
 
 }
diff --git a/perllib/FixMyStreet/Cobrand/IsleOfWight.pm b/perllib/FixMyStreet/Cobrand/IsleOfWight.pm
index ab79be832..a46b540ad 100644
--- a/perllib/FixMyStreet/Cobrand/IsleOfWight.pm
+++ b/perllib/FixMyStreet/Cobrand/IsleOfWight.pm
@@ -108,8 +108,8 @@ sub munge_reports_category_list {
     return @$categories;
 }
 
-sub munge_report_new_category_list {
-    my ($self, $options, $contacts, $extras) = @_;
+sub munge_report_new_contacts {
+    my ($self, $contacts) = @_;
 
     my $user = $self->{c}->user;
     my %bodies = map { $_->body->name => $_->body } @$contacts;
@@ -117,14 +117,10 @@ sub munge_report_new_category_list {
 
     if ( $user && ( $user->is_superuser || $user->belongs_to_body( $b->id ) ) ) {
         @$contacts = grep { !$_->send_method || $_->send_method ne 'Triage' } @$contacts;
-        my $seen = { map { $_->category => 1 } @$contacts };
-        @$options = grep { my $c = ($_->{category} || $_->category); $c =~ 'Pick a category' || $seen->{ $c } } @$options;
         return;
     }
 
     @$contacts = grep { $_->send_method && $_->send_method eq 'Triage' } @$contacts;
-    my $seen = { map { $_->category => 1 } @$contacts };
-    @$options = grep { my $c = ($_->{category} || $_->category); $c =~ 'Pick a category' || $seen->{ $c } } @$options;
 }
 
 sub munge_around_category_where {
diff --git a/perllib/FixMyStreet/Cobrand/TfL.pm b/perllib/FixMyStreet/Cobrand/TfL.pm
index 5183e00c9..281243832 100644
--- a/perllib/FixMyStreet/Cobrand/TfL.pm
+++ b/perllib/FixMyStreet/Cobrand/TfL.pm
@@ -434,10 +434,10 @@ sub report_new_is_on_tlrn {
     return scalar @$features ? 1 : 0;
 }
 
-sub munge_report_new_category_list { }
+sub munge_report_new_contacts { }
 
 sub munge_red_route_categories {
-    my ($self, $options, $contacts) = @_;
+    my ($self, $contacts) = @_;
     if ( $self->report_new_is_on_tlrn ) {
         # We're on a red route - only send TfL categories (except the disabled
         # one that directs the user to borough for street cleaning) and borough
@@ -455,8 +455,6 @@ sub munge_red_route_categories {
         $tlrn_cats{$self->_tfl_council_category} = 1;
         @$contacts = grep { !( $_->body->name eq 'TfL' && $tlrn_cats{$_->category } ) } @$contacts;
     }
-    my $seen = { map { $_->category => 1 } @$contacts };
-    @$options = grep { my $c = ($_->{category} || $_->category); $c =~ 'Pick a category' || $seen->{ $c } } @$options;
 }
 
 # Reports in these categories can only be made on a red route
diff --git a/perllib/FixMyStreet/Cobrand/UKCouncils.pm b/perllib/FixMyStreet/Cobrand/UKCouncils.pm
index 97c8b6c81..7336e60ca 100644
--- a/perllib/FixMyStreet/Cobrand/UKCouncils.pm
+++ b/perllib/FixMyStreet/Cobrand/UKCouncils.pm
@@ -289,14 +289,14 @@ sub prefill_report_fields_for_inspector { 1 }
 
 sub social_auth_disabled { 1 }
 
-sub munge_report_new_category_list {
-    my ($self, $options, $contacts, $extras) = @_;
+sub munge_report_new_contacts {
+    my ($self, $contacts) = @_;
 
     my %bodies = map { $_->body->name => $_->body } @$contacts;
     if ( $bodies{'TfL'} ) {
         # Presented categories vary if we're on/off a red route
         my $tfl = FixMyStreet::Cobrand->get_class_for_moniker( 'tfl' )->new({ c => $self->{c} });
-        $tfl->munge_red_route_categories($options, $contacts);
+        $tfl->munge_red_route_categories($contacts);
     }
 }