Add Extra role to ease use of {extra} field.

Historically, the extra field has been used in two different ways by
different cobrands, both as a list (e.g. Open311 category fields) and a
hash (e.g. the Zurich cobrand).

This commit consolidates usage, adding an API to make use of the field
easier and always returning a hash for the code to use. Fixes #1018.

4 files changed, 151 insertions, 3 deletions

diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t
index 25fa01a1f..3e1446068 100644
--- a/t/app/controller/report_new.t
+++ b/t/app/controller/report_new.t
@@ -999,7 +999,7 @@ for my $test (
         host  => 'www.fixmystreet.com',
         postcode => 'EH99 1SP',
         fms_extra_title => '',
-        extra => undef,
+        extra => [],
         user_title => undef,
     },
     {
@@ -1127,7 +1127,7 @@ for my $test (
 
         my $report = $user->problems->first;
         ok $report, "Found the report";
-        my $extras = $report->extra;
+        my $extras = $report->get_extra_fields;
         is $user->title, $test->{'user_title'}, 'user title correct';
         is_deeply $extras, $test->{extra}, 'extra contains correct values';
 
diff --git a/t/app/controller/report_new_open311.t b/t/app/controller/report_new_open311.t
index 15ccef493..53b84b92d 100644
--- a/t/app/controller/report_new_open311.t
+++ b/t/app/controller/report_new_open311.t
@@ -158,7 +158,7 @@ foreach my $test (
         my $prob = $user->problems->first;
         ok $prob, 'problem created';
 
-        is_deeply $prob->extra, $test->{extra}, 'extra open311 data added to problem';
+        is_deeply $prob->get_extra_fields, $test->{extra}, 'extra open311 data added to problem';
 
         $user->problems->delete;
         $user->delete;
diff --git a/t/app/controller/token.t b/t/app/controller/token.t
new file mode 100644
index 000000000..9ca8b905d
--- /dev/null
+++ b/t/app/controller/token.t
@@ -0,0 +1,37 @@
+use strict;
+use warnings;
+use Test::More;
+use utf8;
+
+use FixMyStreet::TestMech;
+use FixMyStreet::App;
+
+my $user = FixMyStreet::App->model('DB::User')->find_or_create({
+        name => 'Bob', email => 'bob@example.com',
+    });
+my $mech = FixMyStreet::TestMech->new;
+
+subtest 'Zurich special case for C::Tokens->problem_confirm' => sub {
+    FixMyStreet::override_config {
+        ALLOWED_COBRANDS => ['zurich'],
+    }, sub {
+        my $c = FixMyStreet::App->new;
+        my $zurich = $mech->create_body_ok( 1, 'Zurich' );
+        my ($report) = $mech->create_problems_for_body( 
+            1, $zurich->id,
+            {
+                state     => 'unconfirmed',
+                confirmed => undef,
+                cobrand   => 'zurich',
+            });
+        
+        is $report->get_extra_metadata('email_confirmed'), undef, 'email_confirmed not yet set (sanity)';
+        my $token = $c->model('DB::Token')->create({ scope => 'problem', data => $report->id });
+
+        $mech->get_ok('/P/' . $token->token);
+        $report->discard_changes;
+        is $report->get_extra_metadata('email_confirmed'), 1, 'email_confirmed set by Zurich special case'; 
+    };
+};
+
+done_testing;
diff --git a/t/app/model/extra.t b/t/app/model/extra.t
new file mode 100644
index 000000000..21c37336e
--- /dev/null
+++ b/t/app/model/extra.t
@@ -0,0 +1,111 @@
+use strict;
+use warnings;
+use Test::More;
+use utf8;
+
+use FixMyStreet::App;
+use Data::Dumper;
+use DateTime;
+
+my $c = FixMyStreet::App->new;
+
+my $db = FixMyStreet::App->model('DB')->schema;
+$db->txn_begin;
+
+my $body = $db->resultset('Body')->create({ name => 'ExtraTestingBody' });
+
+my $serial = 1;
+sub get_test_contact {
+    my $extra = shift;
+    my $contact = $db->resultset('Contact')->create({
+        category => "Testing ${serial}",
+        body => $body,
+        email => 'test@example.com',
+        confirmed => 1,
+        deleted => 0,
+        editor => 'test script',
+        note => 'test script',
+        whenedited => DateTime->now(),
+        $extra ? ( extra => $extra ) : (),
+    });
+    $serial++;
+    return $contact;
+}
+
+subtest 'Old list layout transparently upgraded' => sub {
+
+    subtest 'layout' => sub {
+        my $contact = get_test_contact([]);
+
+        is_deeply $contact->get_extra(), { _fields => [] }, 'transparently upgraded to a hash';
+    };
+
+    subtest 'extra fields' => sub {
+        my $contact = get_test_contact([]);
+
+        is_deeply $contact->get_extra_fields(), [], 'No extra fields';
+
+        my @fields = ( { a => 1 }, { b => 2 } );
+        $contact->set_extra_fields(@fields);
+        is_deeply $contact->extra, { _fields => \@fields }, 'extra fields set...';
+        $contact->update;
+        $contact->discard_changes;
+        is_deeply $contact->extra, { _fields => \@fields }, '...and retrieved';
+        is_deeply $contact->get_extra_fields(), \@fields, 'extra fields returned';
+    };
+
+    subtest 'metadata' => sub {
+        my $contact = get_test_contact([]);
+        is_deeply $contact->get_extra_metadata_as_hashref(), {}, 'No extra metadata';
+
+        $contact->set_extra_metadata('foo' => 'bar');
+        is $contact->get_extra_metadata('foo'), 'bar', 'extra metadata set...';
+        $contact->update;
+        $contact->discard_changes;
+        is $contact->get_extra_metadata('foo'), 'bar', '... and retrieved';
+        is_deeply $contact->get_extra_metadata_as_hashref(), { foo => 'bar' }, 'No extra metadata';
+    };
+};
+
+subtest 'Default hash layout' => sub {
+    subtest 'layout' => sub {
+        my $contact = get_test_contact();
+
+        is_deeply $contact->get_extra(), {}, 'default layout is hash';
+    };
+
+    subtest 'extra fields' => sub {
+        my $contact = get_test_contact();
+
+        is_deeply $contact->get_extra_fields(), [], 'No extra fields';
+
+        my @fields = ( { a => 1 }, { b => 2 } );
+        $contact->set_extra_fields(@fields);
+        is_deeply $contact->get_extra_fields, \@fields, 'extra fields set...';
+        $contact->update;
+        $contact->discard_changes;
+        is_deeply $contact->get_extra_fields(), \@fields, '... and returned';
+        is_deeply $contact->extra, { _fields => \@fields }, '(sanity check layout)';
+    };
+
+    subtest 'metadata' => sub {
+        my $contact = get_test_contact();
+        is_deeply $contact->get_extra_metadata_as_hashref(), {}, 'No extra metadata';
+
+        $contact->set_extra_metadata('foo' => 'bar');
+        is $contact->get_extra_metadata('foo'), 'bar', 'extra metadata set...';
+        $contact->update;
+        $contact->discard_changes;
+        is $contact->get_extra_metadata( 'foo'), 'bar', '... and retrieved';
+        is_deeply $contact->get_extra_metadata_as_hashref(), { foo => 'bar' }, 'No extra metadata';
+
+        $contact->unset_extra_metadata('foo');
+        is $contact->get_extra_metadata('foo'), undef, 'extra metadata now unset';
+        $contact->update;
+        $contact->discard_changes;
+        is $contact->get_extra_metadata('foo'), undef, '... after retrieval';
+    };
+};
+
+$db->txn_rollback;
+done_testing();