Handle extra field updates on report inspect page

We want to make sure the extra metadata stored on a problem is kept in sync with
the requirements of the category the report is in. Thus, if a report is moved to
a category that collects extra metadata then the appropriate input fields should
be shown so the inspector can enter the data.

This commit shows the category extra HTML elements for every selectable category
on the inspector page hiding those that don't apply to the selected category. JS
is used to show/hide the extra fields when the category dropdown is changed.

A consequence of the implementation is that any extra metadata from the original
category is lost when the category is changed. Some thought was given to storing
the old values elsewhere in the problem's extra field but I decided against this
for the time being.

For mysociety/fixmystreetforcouncils#45

6 files changed, 87 insertions, 43 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index e68b7fda0..355a3f2a9 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -309,6 +309,7 @@ sub inspect : Private {
     my $problem = $c->stash->{problem};
 
     $c->stash->{categories} = $c->forward('/admin/categories_for_point');
+    $c->stash->{report_meta} = { map { $_->{name} => $_ } @{ $c->stash->{problem}->get_extra_fields() } };
 
     if ( $c->get_param('save') ) {
         $c->forward('/auth/check_csrf_token');
@@ -359,6 +360,14 @@ sub inspect : Private {
 
         if ($permissions->{report_inspect} || $permissions->{report_edit_category}) {
             $c->forward( '/admin/report_edit_category', [ $problem ] );
+
+            # The new category might require extra metadata (e.g. pothole size), so
+            # we need to update the problem with the new values.
+            my $param_prefix = lc $problem->category;
+            $param_prefix =~ s/[^a-z]//g;
+            $param_prefix = "category_" . $param_prefix . "_";
+            my @contacts = grep { $_->category eq $problem->category } @{$c->stash->{contacts}};
+            $c->forward('/report/new/set_report_extras', [ \@contacts, $param_prefix ]);
         }
 
         if ($valid) {
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index f26120829..7766e08a1 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -849,33 +849,11 @@ sub process_report : Private {
             $report->bodies_missing($missing);
         }
 
-        my @extra;
-        foreach my $contact (@contacts) {
-            my $metas = $contact->get_metadata_for_input;
-            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}) || '',
-                };
-            }
-        }
+        $c->forward('set_report_extras', [ \@contacts ]);
 
         if ( $c->stash->{non_public_categories}->{ $report->category } ) {
             $report->non_public( 1 );
         }
-
-        $c->cobrand->process_open311_extras( $c, $contacts[0]->body_id, \@extra );
-
-        if ( @extra ) {
-            $c->stash->{report_meta} = { map { $_->{name} => $_ } @extra };
-            $report->set_extra_fields( @extra );
-        }
     } elsif ( @{ $c->stash->{bodies_to_list} } ) {
 
         # There was an area with categories, but we've not been given one. Bail.
@@ -927,6 +905,36 @@ sub contacts_to_bodies : Private {
     }
 }
 
+sub set_report_extras : Private {
+    my ($self, $c, $contacts, $param_prefix) = @_;
+
+    $param_prefix ||= "";
+    my @extra;
+    foreach my $contact (@$contacts) {
+        my $metas = $contact->get_metadata_for_input;
+        foreach my $field ( @$metas ) {
+            if ( lc( $field->{required} ) eq 'true' ) {
+                unless ( $c->get_param($param_prefix . $field->{code}) ) {
+                    $c->stash->{field_errors}->{ $field->{code} } = _('This information is required');
+                }
+            }
+            push @extra, {
+                name => $field->{code},
+                description => $field->{description},
+                value => $c->get_param($param_prefix . $field->{code}) || '',
+            };
+        }
+    }
+
+    $c->cobrand->process_open311_extras( $c, @$contacts[0]->body_id, \@extra )
+        if ( scalar @$contacts );
+
+    if ( @extra ) {
+        $c->stash->{report_meta} = { map { $_->{name} => $_ } @extra };
+        $c->stash->{report}->set_extra_fields( @extra );
+    }
+}
+
 =head2 check_for_errors
 
 Examine the user and the report for errors. If found put them on stash and
diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html
index c426b4d23..34a7ea96c 100644
--- a/templates/web/base/report/_inspect.html
+++ b/templates/web/base/report/_inspect.html
@@ -48,6 +48,20 @@
           </select>
         </p>
 
+        [% FOREACH category IN categories %]
+          [% cat_prefix = category | lower | replace('[^a-z]', '') %]
+          [% cat_prefix = "category_" _ cat_prefix _ "_" %]
+          [% IF category == problem.category %]
+            <p data-category="[% category | html %]">
+              [% INCLUDE 'report/new/category_extras_fields.html' %]
+            </p>
+          [% ELSIF category_extras.$category.size %]
+            <p data-category="[% category | html %]" class="hidden">
+              [% INCLUDE 'report/new/category_extras_fields.html' report_meta='' %]
+            </p>
+          [% END %]
+        [% END %]
+
         [% IF permissions.report_inspect %]
         <p>
           <label for="state">[% loc('State') %]</label>
diff --git a/templates/web/base/report/new/category_extras.html b/templates/web/base/report/new/category_extras.html
index 6b01f93e0..084dd2d93 100644
--- a/templates/web/base/report/new/category_extras.html
+++ b/templates/web/base/report/new/category_extras.html
@@ -15,25 +15,7 @@
         list_of_names.join( '</strong>' _ loc(' or ') _ '<strong>' )
       ); %]
     </p>
-      [%- FOR meta IN category_extras.$category %]
-        [%- meta_name = meta.code -%]
-
-        <label for="form_[% meta_name %]">[% meta.description %]</label>
-        [% IF field_errors.$meta_name %]
-        <p class='form-error'>[% field_errors.$meta_name %]</p>
-        [% END -%]
-        [% IF meta.variable != 'false' %]
-          [% IF meta.exists('values') %]
-            <select class="form-control" name="[% meta_name %]" id="form_[% meta_name %]"[% meta.required == 'true' ? ' required' : '' %]>
-              [% FOR option IN meta.values %]
-                <option value="[% option.key %]"[% IF option.key == report_meta.$meta_name.value %] selected[% END %]>[% option.name %]</option>
-              [% END %]
-            </select>
-          [% ELSE %]
-            <input class="form-control" type="text" value="[% report_meta.$meta_name.value | html %]" name="[% meta_name %]" id="form_[% meta_name %]"[% meta.required == 'true' ? ' required' : '' %]>
-          [% END %]
-        [% END %]
-      [%- END %]
+    [% INCLUDE 'report/new/category_extras_fields.html' %]
     </div>
   [%- END %]
 </div>
diff --git a/templates/web/base/report/new/category_extras_fields.html b/templates/web/base/report/new/category_extras_fields.html
new file mode 100644
index 000000000..ec72ee7f3
--- /dev/null
+++ b/templates/web/base/report/new/category_extras_fields.html
@@ -0,0 +1,19 @@
+[%- FOR meta IN category_extras.$category %]
+  [%- meta_name = meta.code -%]
+
+  <label for="[% cat_prefix %]form_[% meta_name %]">[% meta.description %]</label>
+  [% IF field_errors.$meta_name %]
+  <p class='form-error'>[% field_errors.$meta_name %]</p>
+  [% END -%]
+  [% IF meta.variable != 'false' %]
+    [% IF meta.exists('values') %]
+      <select class="form-control" name="[% cat_prefix %][% meta_name %]" id="[% cat_prefix %]form_[% meta_name %]"[% meta.required == 'true' ? ' required' : '' %]>
+        [% FOR option IN meta.values %]
+          <option value="[% option.key %]"[% IF option.key == report_meta.$meta_name.value %] selected[% END %]>[% option.name %]</option>
+        [% END %]
+      </select>
+    [% ELSE %]
+      <input class="form-control" type="text" value="[% report_meta.$meta_name.value | html %]" name="[% cat_prefix %][% meta_name %]" id="[% cat_prefix %]form_[% meta_name %]"[% meta.required == 'true' ? ' required' : '' %]>
+    [% END %]
+  [% END %]
+[%- END %]
diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js
index 2443b333d..4d222dc24 100644
--- a/web/cobrands/fixmystreet/fixmystreet.js
+++ b/web/cobrands/fixmystreet/fixmystreet.js
@@ -370,7 +370,9 @@ $.extend(fixmystreet.set_up, {
   },
 
   category_change: function() {
-    // Deal with changes to category by asking for details from the server.
+    // Deal with changes to report category.
+
+    // On the new report form, does this by asking for details from the server.
     // Delegation is necessary because #form_category may be replaced during the lifetime of the page
     $("#problem_form").on("change.category", "select#form_category", function(){
         var args = {
@@ -395,8 +397,18 @@ $.extend(fixmystreet.set_up, {
             }
         });
     });
+
+    // On the manage/inspect report form, we already have all the extra inputs
+    // in the DOM, we just need to hide/show them as appropriate.
+    $('form#report_inspect_form [name=category]').change(function() {
+        var category = $(this).val();
+        var selector = "[data-category='"+category+"']";
+        $("form#report_inspect_form [data-category]:not("+selector+")").addClass("hidden");
+        $("form#report_inspect_form "+selector).removeClass("hidden");
+    });
   },
 
+
   contribute_as: function() {
     $('.content').on('change', '.js-contribute-as', function(){
         var opt = this.options[this.selectedIndex],