Merge branch '400-editing-category'

5 files changed, 65 insertions, 41 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index 50b4360eb..bcf66f36f 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -681,6 +681,8 @@ sub report_edit : Path('report_edit') : Args(1) {
         }
     }
 
+    $c->stash->{categories} = $c->forward('categories_for_point');
+
     if ( $c->cobrand->moniker eq 'zurich' ) {
         my $done = $c->cobrand->admin_report_edit();
         return if $done;
@@ -729,12 +731,18 @@ sub report_edit : Path('report_edit') : Args(1) {
             flagged => $c->get_param('flagged') ? 1 : 0,
             non_public => $c->get_param('non_public') ? 1 : 0,
         );
-        $columns{bodies_str} = $c->get_param('body') if $c->get_param('body');
         foreach (qw/state anonymous title detail name external_id external_body external_team/) {
             $columns{$_} = $c->get_param($_);
         }
         $problem->set_inflated_columns(\%columns);
 
+        if ((my $category = $c->get_param('category')) ne $problem->category) {
+            $problem->category($category);
+            my @contacts = grep { $_->category eq $problem->category } @{$c->stash->{contacts}};
+            my $bs = join( ',', map { $_->body_id } @contacts );
+            $problem->bodies_str($bs);
+        }
+
         if ( $c->get_param('email') ne $problem->user->email ) {
             my $user = $c->model('DB::User')->find_or_create(
                 { email => $c->get_param('email') }
@@ -777,6 +785,24 @@ sub report_edit : Path('report_edit') : Args(1) {
     return 1;
 }
 
+sub categories_for_point : Private {
+    my ($self, $c) = @_;
+
+    $c->stash->{report} = $c->stash->{problem};
+    # We have a report, stash its location
+    $c->forward('/report/new/determine_location_from_report');
+    # Look up the areas for this location
+    $c->stash->{prefetched_all_areas} = [ grep { $_ } split ',', $c->stash->{report}->areas ];
+    $c->forward('/around/check_location_is_acceptable');
+    # As with a new report, fetch the bodies/categories
+    $c->forward('/report/new/setup_categories_and_bodies');
+
+    # Remove the "Pick a category" option
+    shift @{$c->stash->{category_options}} if @{$c->stash->{category_options}};
+
+    return $c->stash->{category_options};
+}
+
 sub templates : Path('templates') : Args(0) {
     my ( $self, $c ) = @_;
 
diff --git a/perllib/FixMyStreet/App/Controller/Council.pm b/perllib/FixMyStreet/App/Controller/Council.pm
index a5915aa46..06a23aec9 100644
--- a/perllib/FixMyStreet/App/Controller/Council.pm
+++ b/perllib/FixMyStreet/App/Controller/Council.pm
@@ -55,7 +55,12 @@ sub load_and_check_areas : Private {
     $params{generation} = $c->config->{MAPIT_GENERATION}
         if $c->config->{MAPIT_GENERATION};
 
-    if ( $c->stash->{fetch_all_areas} ) {
+    if ($c->stash->{prefetched_all_areas}) {
+        $all_areas = {
+            map { $_ => { id => $_ } }
+            @{$c->stash->{prefetched_all_areas}}
+        };
+    } elsif ( $c->stash->{fetch_all_areas} ) {
         my %area_types = map { $_ => 1 } @$area_types;
         $all_areas =
           mySociety::MaPit::call( 'point',
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index e81dc719f..bbd27c666 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -661,6 +661,7 @@ sub setup_categories_and_bodies : Private {
 
     # put results onto stash for display
     $c->stash->{bodies} = \%bodies;
+    $c->stash->{contacts} = \@contacts;
     $c->stash->{all_body_names} = [ map { $_->name } values %bodies ];
     $c->stash->{all_body_urls} = [ map { $_->external_url } values %bodies ];
     $c->stash->{bodies_to_list} = [ keys %bodies_to_list ];
@@ -777,8 +778,7 @@ sub process_report : Private {
       map { $_ => $c->get_param($_) }
       (
         'title', 'detail', 'pc',                 #
-        'detail_size', 'detail_depth',
-        'detail_offensive',
+        'detail_size',
         'may_show_name',                         #
         'category',                              #
         'subcategory',                              #
@@ -802,9 +802,8 @@ sub process_report : Private {
     $report->title( Utils::cleanup_text( $params{title} ) );
 
     my $detail = Utils::cleanup_text( $params{detail}, { allow_multiline => 1 } );
-    for my $w ('depth', 'size', 'offensive') {
+    for my $w ('size') {
         next unless $params{"detail_$w"};
-        next if $params{"detail_$w"} eq '-- Please select --';
         $detail .= "\n\n\u$w: " . $params{"detail_$w"};
     }
     $report->detail( $detail );
@@ -814,32 +813,13 @@ sub process_report : Private {
 
     # set these straight from the params
     $report->category( _ $params{category} ) if $params{category};
-
     $report->subcategory( $params{subcategory} );
 
     my $areas = $c->stash->{all_areas_mapit};
     $report->areas( ',' . join( ',', sort keys %$areas ) . ',' );
 
-    # From earlier in the process.
-    $areas = $c->stash->{all_areas};
-    my $bodies = $c->stash->{bodies};
-    my $first_area = ( values %$areas )[0];
-    my $first_body = ( values %$bodies )[0];
-
     if ( $report->category ) {
-
-        # FIXME All contacts were fetched in setup_categories_and_bodies,
-        # so can this DB call also be avoided?
-        my @contacts = $c->       #
-          model('DB::Contact')    #
-          ->not_deleted           #
-          ->search(
-            {
-                body_id => [ keys %$bodies ],
-                category => $report->category
-            }
-          )->all;
-
+        my @contacts = grep { $_->category eq $report->category } @{$c->stash->{contacts}};
         unless ( @contacts ) {
             $c->stash->{field_errors}->{category} = _('Please choose a category');
             $report->bodies_str( -1 );
@@ -877,20 +857,20 @@ sub process_report : Private {
         }
 
         my @extra;
-        # NB: we are only checking extras for the *first* retrieved contact.
-        my $metas = $contacts[0]->get_extra_fields();
-
-        foreach my $field ( @$metas ) {
-            if ( lc( $field->{required} ) eq 'true' ) {
-                unless ( $c->get_param($field->{code}) ) {
-                    $c->stash->{field_errors}->{ $field->{code} } = _('This information is required');
+        foreach my $contact (@contacts) {
+            my $metas = $contact->get_extra_fields;
+            foreach my $field ( @$metas ) {
+                if ( lc( $field->{required} ) eq 'true' ) {
+                    unless ( $c->get_param($field->{code}) ) {
+                        $c->stash->{field_errors}->{ $field->{code} } = _('This information is required');
+                    }
                 }
+                push @extra, {
+                    name => $field->{code},
+                    description => $field->{description},
+                    value => $c->get_param($field->{code}) || '',
+                };
             }
-            push @extra, {
-                name => $field->{code},
-                description => $field->{description},
-                value => $c->get_param($field->{code}) || '',
-            };
         }
 
         if ( $c->stash->{non_public_categories}->{ $report->category } ) {
diff --git a/t/app/controller/admin.t b/t/app/controller/admin.t
index 007948299..d7fcb30e6 100644
--- a/t/app/controller/admin.t
+++ b/t/app/controller/admin.t
@@ -531,7 +531,7 @@ foreach my $test (
         $log_entries->reset;
         $mech->get_ok("/admin/report_edit/$report_id");
 
-        @{$test->{fields}}{'external_id', 'external_body', 'external_team'} = (13, "", "");
+        @{$test->{fields}}{'external_id', 'external_body', 'external_team', 'category'} = (13, "", "", "Other");
         is_deeply( $mech->visible_form_values(), $test->{fields}, 'initial form values' );
 
         my $new_fields = {
@@ -582,6 +582,7 @@ subtest 'change email to new user' => sub {
         state  => $report->state,
         name   => $report->name,
         email  => $report->user->email,
+        category => 'Other',
         anonymous => 1,
         flagged => 'on',
         non_public => 'on',
diff --git a/templates/web/base/admin/report_edit.html b/templates/web/base/admin/report_edit.html
index 351cf86f1..48a02593b 100644
--- a/templates/web/base/admin/report_edit.html
+++ b/templates/web/base/admin/report_edit.html
@@ -123,8 +123,20 @@ class="admin-offsite-link">[% problem.latitude %], [% problem.longitude %]</a>
 [% END %]
 </select></li>
 <li><label class="inline-text" for="category">[% loc('Category:') %]</label>
-    [% problem.category | html %]
-</li>
+<select name="category" id="category">
+  [% IF NOT problem.category OR NOT categories.grep(problem.category).size %]
+  <optgroup label="[% loc('Existing category') %]">
+    <option selected value="[% problem.category | html %]">[% (problem.category OR '-') | html %]</option>
+  </optgroup>
+  [% END %]
+  [% IF categories.size %]
+    <optgroup label="[% loc('Available categories') %]">
+      [% FOREACH cat IN categories %]
+        <option[% ' selected' IF problem.category == cat %]>[% cat | html %]</option>
+      [% END %]
+    </optgroup>
+  [% END %]
+</select></li>
 <li><label class="inline-text" for="anonymous">[% loc('Anonymous:') %]</label>
 <select name="anonymous"  id="anonymous">
 <option [% 'selected ' IF problem.anonymous %]value="1">[% loc('Yes') %]</option>