Merge remote-tracking branch 'origin/1291-extra_fields'

8 files changed, 114 insertions, 23 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index 2f03bf13b..6aaa4531f 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -854,12 +854,6 @@ sub process_report : Private {
         $bodies = join( ',', @{ $c->stash->{bodies_to_list} } ) || -1;
         $report->bodies_str( $bodies );
 
-        my %extra;
-        $c->cobrand->process_extras( $c, undef, \%extra );
-        if ( %extra ) {
-            $report->extra( \%extra );
-        }
-
     } elsif ( $report->category ) {
 
         # FIXME All contacts were fetched in setup_categories_and_bodies,
@@ -931,7 +925,7 @@ sub process_report : Private {
             $report->non_public( 1 );
         }
 
-        $c->cobrand->process_extras( $c, $contacts[0]->body_id, \@extra );
+        $c->cobrand->process_open311_extras( $c, $contacts[0]->body_id, \@extra );
 
         if ( @extra ) {
             $c->stash->{report_meta} = { map { $_->{name} => $_ } @extra };
@@ -950,6 +944,15 @@ sub process_report : Private {
 
     }
 
+    # Get a list of custom form fields we want and store them in extra metadata
+    foreach my $field ($c->cobrand->report_form_extras) {
+        my $form_name = $field->{name};
+        my $value = $c->get_param($form_name) || '';
+        $c->stash->{field_errors}->{$form_name} = _('This information is required')
+            if $field->{required} && !$value;
+        $report->set_extra_metadata( $form_name => $value );
+    }
+
     # set defaults that make sense
     $report->state('unconfirmed');
 
diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm
index 45d924335..8d6bc2019 100644
--- a/perllib/FixMyStreet/App/Controller/Report/Update.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm
@@ -286,9 +286,9 @@ sub process_update : Private {
 
 
     my @extra; # Next function fills this, but we don't need it here.
-    # This is just so that the error checkign for these extra fields runs.
+    # This is just so that the error checking for these extra fields runs.
     # TODO Use extra here as it is used on reports.
-    $c->cobrand->process_extras( $c, $update->problem->bodies_str, \@extra );
+    $c->cobrand->process_open311_extras( $c, $update->problem->bodies_str, \@extra );
 
     if ( $c->get_param('fms_extra_title') ) {
         my %extras = ();
diff --git a/perllib/FixMyStreet/Cobrand/Bromley.pm b/perllib/FixMyStreet/Cobrand/Bromley.pm
index 687843a2a..c9f9a98be 100644
--- a/perllib/FixMyStreet/Cobrand/Bromley.pm
+++ b/perllib/FixMyStreet/Cobrand/Bromley.pm
@@ -81,9 +81,9 @@ sub ask_ever_reported {
     return 0;
 }
 
-sub process_extras {
+sub process_open311_extras {
     my $self = shift;
-    $self->SUPER::process_extras( @_, [ 'first_name', 'last_name' ] );
+    $self->SUPER::process_open311_extras( @_, [ 'first_name', 'last_name' ] );
 }
 
 sub contact_email {
diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm
index e7a4dad72..b3b830f06 100644
--- a/perllib/FixMyStreet/Cobrand/Default.pm
+++ b/perllib/FixMyStreet/Cobrand/Default.pm
@@ -754,7 +754,18 @@ If set to an arrayref, will plot those area ID(s) from mapit on all the /around
 
 sub areas_on_around { []; }
 
-sub process_extras {}
+=head2
+
+A list of extra fields we wish to save to the database in the 'extra' column of
+problems based on variables passed in by the form. Return a list of hashrefs
+of values we wish to save, e.g.
+( { name => 'address', required => 1 }, { name => 'passport', required => 0 } )
+
+=cut
+
+sub report_form_extras {}
+
+sub process_open311_extras {}
 
 =head 2 pin_colour
 
diff --git a/perllib/FixMyStreet/Cobrand/EmptyHomes.pm b/perllib/FixMyStreet/Cobrand/EmptyHomes.pm
index 0b02f90c4..995c39c85 100644
--- a/perllib/FixMyStreet/Cobrand/EmptyHomes.pm
+++ b/perllib/FixMyStreet/Cobrand/EmptyHomes.pm
@@ -132,16 +132,8 @@ sub council_rss_alert_options {
     return ( \@options, @reported_to_options ? \@reported_to_options : undef );
 }
 
-sub process_extras {
-    my $self    = shift;
-    my $ctx     = shift;
-    my $body_id = shift;
-    my $extra   = shift;
-
-    my $value = $ctx->get_param('address') || '';
-    $ctx->stash->{field_errors}->{address} = _('This information is required')
-        unless $value;
-    $extra->{address} = $value;
+sub report_form_extras {
+    ( { name => 'address', required => 1 } )
 }
 
 sub front_stats_data {
diff --git a/perllib/FixMyStreet/Cobrand/UK.pm b/perllib/FixMyStreet/Cobrand/UK.pm
index 824590d38..c33b39aac 100644
--- a/perllib/FixMyStreet/Cobrand/UK.pm
+++ b/perllib/FixMyStreet/Cobrand/UK.pm
@@ -27,7 +27,7 @@ sub disambiguate_location {
     };
 }
 
-sub process_extras {
+sub process_open311_extras {
     my $self    = shift;
     my $ctx     = shift;
     my $body_id = shift;
diff --git a/t/cobrand/form_extras.t b/t/cobrand/form_extras.t
new file mode 100644
index 000000000..9c20b7ad4
--- /dev/null
+++ b/t/cobrand/form_extras.t
@@ -0,0 +1,73 @@
+use strict;
+use warnings;
+
+package FixMyStreet::Cobrand::Tester;
+use parent 'FixMyStreet::Cobrand::FixMyStreet';
+
+sub report_form_extras {
+    ( { name => 'address', required => 1 }, { name => 'passport', required => 0 } )
+}
+
+# To allow a testing template override
+sub path_to_web_templates {
+    my $self = shift;
+    return [
+        FixMyStreet->path_to( 't/cobrand/form_extras/templates' )->stringify,
+        FixMyStreet->path_to( 'templates/web/fixmystreet' )->stringify
+    ];
+}
+
+package main;
+
+use Test::More;
+use FixMyStreet::TestMech;
+
+# disable info logs for this test run
+FixMyStreet::App->log->disable('info');
+END { FixMyStreet::App->log->enable('info'); }
+
+my $mech = FixMyStreet::TestMech->new;
+
+FixMyStreet::override_config {
+    ALLOWED_COBRANDS => [ { tester => '.' } ],
+    MAPIT_URL => 'http://mapit.mysociety.org/',
+}, sub {
+    $mech->get_ok('/around');
+    $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB', } }, "submit location" );
+    $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" );
+    $mech->submit_form_ok( {
+            button      => 'submit_register',
+            with_fields => {
+                title => 'Test Report',
+                detail => 'Test report details.',
+                name => 'Joe Bloggs',
+                may_show_name => '1',
+                email => 'test-1@example.com',
+                passport => '123456',
+                password_register => '',
+            }
+        },
+        "submit details without address, with passport",
+    );
+    $mech->content_like(qr{<label for="form_address">Address</label>\s*<p class='form-error'>This information is required</p>}, 'Address is required');
+    $mech->content_contains('value="123456" name="passport"', 'Passport number reshown');
+
+    $mech->submit_form_ok( {
+            button      => 'submit_register',
+            with_fields => {
+                address => 'My address',
+            }
+        },
+        "submit details, now with address",
+    );
+    $mech->content_contains('Now check your email');
+
+    my $problem = FixMyStreet::DB->resultset('Problem')->search({}, { order_by => '-id' })->first;
+    is $problem->get_extra_metadata('address'), 'My address', 'Address is stored';
+    is $problem->get_extra_metadata('passport'), '123456', 'Passport number is stored';
+};
+
+END {
+    $mech->delete_problems_for_body(undef);
+    done_testing();
+}
diff --git a/t/cobrand/form_extras/templates/report/new/after_photo.html b/t/cobrand/form_extras/templates/report/new/after_photo.html
new file mode 100644
index 000000000..b0c08ba20
--- /dev/null
+++ b/t/cobrand/form_extras/templates/report/new/after_photo.html
@@ -0,0 +1,12 @@
+<label for="form_address">Address</label>
+[% IF field_errors.address %]
+<p class='form-error'>[% field_errors.address %]</p>
+[% END %]
+<input type="text" value="[% report.get_extra_metadata('address') | html %]" name="address" id="form_address" required>
+
+<label for="form_passport">Passport number (optional)</label>
+[% IF field_errors.passport %]
+<p class='form-error'>[% field_errors.passport %]</p>
+[% END %]
+<input type="text" value="[% report.get_extra_metadata('passport') | html %]" name="passport" id="form_passport">
+