[UK] Prevent duplicate extra fields when blank.

If a report had an existing, but empty, extra field that was looked up
on the server when being sent by Open311, the looked up information
would be added as an additional extra field, rather than replacing the
blank one.

12 files changed, 20 insertions, 21 deletions

diff --git a/perllib/FixMyStreet/Cobrand/Bexley.pm b/perllib/FixMyStreet/Cobrand/Bexley.pm
index 3f52c42b0..215b9d2cb 100644
--- a/perllib/FixMyStreet/Cobrand/Bexley.pm
+++ b/perllib/FixMyStreet/Cobrand/Bexley.pm
@@ -85,7 +85,7 @@ sub open311_config {
 }
 
 sub open311_extra_data_include {
-    my ($self, $row, $h, $extra, $contact) = @_;
+    my ($self, $row, $h, $contact) = @_;
 
     my $open311_only;
     if ($contact->email =~ /^Confirm/) {
@@ -99,7 +99,7 @@ sub open311_extra_data_include {
 
         if (!$row->get_extra_field_value('site_code')) {
             if (my $ref = $self->lookup_site_code($row, 'NSG_REF')) {
-                push @$extra, { name => 'site_code', value => $ref, description => 'Site code' };
+                $row->update_extra_field({ name => 'site_code', value => $ref, description => 'Site code' });
             }
         }
     } elsif ($contact->email =~ /^Uniform/) {
@@ -108,7 +108,7 @@ sub open311_extra_data_include {
         # WFS service at the point we're sending the report over Open311.
         if (!$row->get_extra_field_value('uprn')) {
             if (my $ref = $self->lookup_site_code($row, 'UPRN')) {
-                push @$extra, { name => 'uprn', description => 'UPRN', value => $ref };
+                $row->update_extra_field({ name => 'uprn', description => 'UPRN', value => $ref });
             }
         }
     } else { # Symology
@@ -117,7 +117,7 @@ sub open311_extra_data_include {
         # WFS service at the point we're sending the report over Open311.
         if (!$row->get_extra_field_value('NSGRef')) {
             if (my $ref = $self->lookup_site_code($row, 'NSG_REF')) {
-                push @$extra, { name => 'NSGRef', description => 'NSG Ref', value => $ref };
+                $row->update_extra_field({ name => 'NSGRef', description => 'NSG Ref', value => $ref });
             }
         }
     }
diff --git a/perllib/FixMyStreet/Cobrand/Bromley.pm b/perllib/FixMyStreet/Cobrand/Bromley.pm
index dd8282fa2..c8da01226 100644
--- a/perllib/FixMyStreet/Cobrand/Bromley.pm
+++ b/perllib/FixMyStreet/Cobrand/Bromley.pm
@@ -172,10 +172,11 @@ sub open311_config {
 }
 
 sub open311_extra_data_include {
-    my ($self, $row, $h, $extra) = @_;
+    my ($self, $row, $h) = @_;
 
     my $title = $row->title;
 
+    my $extra = $row->get_extra_fields;
     foreach (@$extra) {
         next unless $_->{value};
         $title .= ' | ID: ' . $_->{value} if $_->{name} eq 'feature_id';
diff --git a/perllib/FixMyStreet/Cobrand/EastSussex.pm b/perllib/FixMyStreet/Cobrand/EastSussex.pm
index 04394f776..b2fd58dc1 100644
--- a/perllib/FixMyStreet/Cobrand/EastSussex.pm
+++ b/perllib/FixMyStreet/Cobrand/EastSussex.pm
@@ -7,7 +7,7 @@ use warnings;
 sub council_area_id { return 2224; }
 
 sub open311_extra_data {
-    my ($self, $row, $h, $extra, $contact) = @_;
+    my ($self, $row, $h, $contact) = @_;
 
     $h->{es_original_detail} = $row->detail;
 
diff --git a/perllib/FixMyStreet/Cobrand/Greenwich.pm b/perllib/FixMyStreet/Cobrand/Greenwich.pm
index de7dd5d96..4cc4e4163 100644
--- a/perllib/FixMyStreet/Cobrand/Greenwich.pm
+++ b/perllib/FixMyStreet/Cobrand/Greenwich.pm
@@ -45,7 +45,7 @@ sub reports_per_page { return 20; }
 sub admin_user_domain { 'royalgreenwich.gov.uk' }
 
 sub open311_extra_data_include {
-    my ($self, $row, $h, $extra) = @_;
+    my ($self, $row, $h) = @_;
 
     # Greenwich doesn't have category metadata to fill this
     return [
diff --git a/perllib/FixMyStreet/Cobrand/Hackney.pm b/perllib/FixMyStreet/Cobrand/Hackney.pm
index 4c035f926..b8f92f1ea 100644
--- a/perllib/FixMyStreet/Cobrand/Hackney.pm
+++ b/perllib/FixMyStreet/Cobrand/Hackney.pm
@@ -67,7 +67,7 @@ sub open311_config {
 }
 
 sub open311_extra_data {
-    my ($self, $row, $h, $extra, $contact) = @_;
+    my ($self, $row, $h, $contact) = @_;
 
     my $open311_only = [
         { name => 'report_url',
diff --git a/perllib/FixMyStreet/Cobrand/Northamptonshire.pm b/perllib/FixMyStreet/Cobrand/Northamptonshire.pm
index 791e2f83f..2543f701d 100644
--- a/perllib/FixMyStreet/Cobrand/Northamptonshire.pm
+++ b/perllib/FixMyStreet/Cobrand/Northamptonshire.pm
@@ -92,7 +92,7 @@ sub open311_config {
 }
 
 sub open311_extra_data_include {
-    my ($self, $row, $h, $extra) = @_;
+    my ($self, $row, $h) = @_;
 
     return [
         { name => 'report_url',
diff --git a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
index 63f114c27..0f34ff09f 100644
--- a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
+++ b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
@@ -123,7 +123,7 @@ sub open311_config {
 }
 
 sub open311_extra_data_include {
-    my ($self, $row, $h, $extra) = @_;
+    my ($self, $row, $h) = @_;
 
     return [
         { name => 'external_id', value => $row->id },
diff --git a/perllib/FixMyStreet/Cobrand/Peterborough.pm b/perllib/FixMyStreet/Cobrand/Peterborough.pm
index cb7da18af..9fe6dbbec 100644
--- a/perllib/FixMyStreet/Cobrand/Peterborough.pm
+++ b/perllib/FixMyStreet/Cobrand/Peterborough.pm
@@ -44,9 +44,9 @@ sub geocoder_munge_results {
 sub admin_user_domain { "peterborough.gov.uk" }
 
 around open311_extra_data_include => sub {
-    my ($orig, $self, $row, $h, $extra) = @_;
+    my ($orig, $self, $row, $h) = @_;
 
-    my $open311_only = $self->$orig($row, $h, $extra);
+    my $open311_only = $self->$orig($row, $h);
     foreach (@$open311_only) {
         if ($_->{name} eq 'description') {
             my ($ref) = grep { $_->{name} =~ /pcc-Skanska-csc-ref/i } @{$row->get_extra_fields};
diff --git a/perllib/FixMyStreet/Cobrand/Rutland.pm b/perllib/FixMyStreet/Cobrand/Rutland.pm
index 557cf83a9..bc8eff6d2 100644
--- a/perllib/FixMyStreet/Cobrand/Rutland.pm
+++ b/perllib/FixMyStreet/Cobrand/Rutland.pm
@@ -30,7 +30,7 @@ sub open311_config {
 }
 
 sub open311_extra_data_include {
-    my ($self, $row, $h, $extra) = @_;
+    my ($self, $row, $h) = @_;
 
     return [
         { name => 'external_id', value => $row->id },
diff --git a/perllib/FixMyStreet/Cobrand/Westminster.pm b/perllib/FixMyStreet/Cobrand/Westminster.pm
index 40ae9f6c6..e00a7c092 100644
--- a/perllib/FixMyStreet/Cobrand/Westminster.pm
+++ b/perllib/FixMyStreet/Cobrand/Westminster.pm
@@ -79,14 +79,14 @@ sub open311_config {
 }
 
 sub open311_extra_data_include {
-    my ($self, $row, $h, $extra) = @_;
+    my ($self, $row, $h) = @_;
 
     # Reports made via the app probably won't have a USRN because we don't
     # display the road layer. Instead we'll look up the closest asset from the
     # asset service at the point we're sending the report over Open311.
     if (!$row->get_extra_field_value('USRN')) {
         if (my $ref = $self->lookup_site_code($row, 'USRN')) {
-            push @$extra, { name => 'USRN', value => $ref };
+            $row->update_extra_field({ name => 'USRN', value => $ref });
         }
     }
 
@@ -96,7 +96,7 @@ sub open311_extra_data_include {
     my ($uprn_field) = grep { $_->{name} eq 'UPRN' } @$fields;
     if ( $uprn_field && !$uprn_field->{value} ) {
         if (my $ref = $self->lookup_site_code($row, 'UPRN')) {
-            push @$extra, { name => 'UPRN', value => $ref };
+            $row->update_extra_field({ name => 'UPRN', value => $ref });
         }
     }
 
diff --git a/perllib/FixMyStreet/Roles/ConfirmOpen311.pm b/perllib/FixMyStreet/Roles/ConfirmOpen311.pm
index 6787ba3f4..1663844a2 100644
--- a/perllib/FixMyStreet/Roles/ConfirmOpen311.pm
+++ b/perllib/FixMyStreet/Roles/ConfirmOpen311.pm
@@ -14,7 +14,7 @@ sub open311_config {
 }
 
 sub open311_extra_data_include {
-    my ($self, $row, $h, $extra) = @_;
+    my ($self, $row, $h) = @_;
 
     my $open311_only = [
         { name => 'report_url',
@@ -31,9 +31,7 @@ sub open311_extra_data_include {
     # service at the point we're sending the report over Open311.
     if (!$row->get_extra_field_value('site_code')) {
         if (my $site_code = $self->lookup_site_code($row)) {
-            push @$extra,
-                { name => 'site_code',
-                value => $site_code };
+            $row->update_extra_field({ name => 'site_code', value => $site_code });
         }
     }
 
diff --git a/perllib/FixMyStreet/SendReport/Open311.pm b/perllib/FixMyStreet/SendReport/Open311.pm
index 5c4407094..e51bd76c9 100644
--- a/perllib/FixMyStreet/SendReport/Open311.pm
+++ b/perllib/FixMyStreet/SendReport/Open311.pm
@@ -41,7 +41,7 @@ sub send {
 
         # Try and fill in some ones that we've been asked for, but not asked the user for
         my $extra = $row->get_extra_fields();
-        my ($include, $exclude) = $cobrand->call_hook(open311_extra_data => $row, $h, $extra, $contact);
+        my ($include, $exclude) = $cobrand->call_hook(open311_extra_data => $row, $h, $contact);
 
         my $original_extra = [ @$extra ];
         push @$extra, @$include if $include;