Merge branch 'rabx-utf8-encode'

3 files changed, 61 insertions, 4 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9b67c782f..595768272 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -28,6 +28,7 @@
         - Add space below "map page" contents on narrow screens.
         - Use relative report links where possible. #1995
         - Improve inline checkbox spacing. #2411
+        - Prevent duplicate contact history creation with Unicode data.
     - Development improvements:
         - Make front page cache time configurable.
         - Better working of /fakemapit/ under https.
diff --git a/perllib/FixMyStreet/DB/RABXColumn.pm b/perllib/FixMyStreet/DB/RABXColumn.pm
index 5f1583018..d14b48dc8 100644
--- a/perllib/FixMyStreet/DB/RABXColumn.pm
+++ b/perllib/FixMyStreet/DB/RABXColumn.pm
@@ -59,7 +59,6 @@ sub rabx_column {
                 my $self = shift;
                 my $ser  = shift;
                 return undef unless defined $ser;
-                utf8::encode($ser) if utf8::is_utf8($ser);
                 my $h = new IO::String($ser);
                 return RABX::wire_rd($h);
             },
@@ -78,18 +77,23 @@ sub rabx_column {
     $RABX_COLUMNS{ _get_class_identifier($class) }{$col} = 1;
 }
 
+# The underlying column should always be UTF-8 encoded bytes.
+sub get_column {
+    my ($self, $col) = @_;
+    my $res = $self->next::method ($col);
+    utf8::encode($res) if $RABX_COLUMNS{_get_class_identifier($self)}{$col} && utf8::is_utf8($res);
+    return $res;
+}
 
 sub set_filtered_column {
     my ($self, $col, $val) = @_;
 
-    my $class = ref $self;
-
     # because filtered objects may be expensive to marshall for storage there
     # is a cache that attempts to detect if they have changed or not. For us
     # this cache breaks things and our marshalling is cheap, so clear it when
     # trying set a column.
     delete $self->{_filtered_column}{$col}
-        if $RABX_COLUMNS{ _get_class_identifier($class) }{$col};
+        if $RABX_COLUMNS{ _get_class_identifier($self) }{$col};
 
     return $self->next::method($col, $val);
 }
diff --git a/t/open311/populate-service-list.t b/t/open311/populate-service-list.t
index 9747dfd2f..4d70dfebc 100644
--- a/t/open311/populate-service-list.t
+++ b/t/open311/populate-service-list.t
@@ -19,6 +19,7 @@ package main;
 
 use FixMyStreet::Test;
 use FixMyStreet::DB;
+use utf8;
 
 use_ok( 'Open311::PopulateServiceList' );
 use_ok( 'Open311' );
@@ -297,6 +298,47 @@ for my $test (
         ',
     },
     {
+        desc => 'check meta data unchanging',
+        has_meta => 1,
+        has_no_history => 1,
+        orig_meta => [ {
+            variable => 'true',
+            code => 'type',
+            datatype => 'string',
+            required => 'true',
+            datatype_description => 'Colour of bin',
+            order => 1,
+            description => 'Cólour of bin'
+
+        } ],
+        end_meta => [ {
+            variable => 'true',
+            code => 'type',
+            datatype => 'string',
+            required => 'true',
+            datatype_description => 'Colour of bin',
+            order => 1,
+            description => 'Cólour of bin'
+
+        } ],
+        meta_xml => '<?xml version="1.0" encoding="utf-8"?>
+    <service_definition>
+        <service_code>100</service_code>
+        <attributes>
+            <attribute>
+                <variable>true</variable>
+                <code>type</code>
+                <datatype>string</datatype>
+                <required>true</required>
+                <datatype_description>Colour of bin</datatype_description>
+                <order>1</order>
+                <description>Cólour of bin</description>
+            </attribute>
+        </attributes>
+    </service_definition>
+        ',
+    },
+    {
         desc => 'check meta data removed',
         has_meta => 0,
         end_meta => [],
@@ -389,11 +431,21 @@ for my $test (
         $processor->_current_open311( $o );
         $processor->_current_body( $body );
 
+        my $count = FixMyStreet::DB->resultset('ContactsHistory')->search({
+            contact_id => $contact->id,
+        })->count;
+
         $processor->process_services( $service_list );
 
         $contact->discard_changes;
 
         is_deeply $contact->get_extra_fields, $test->{end_meta}, 'meta data saved';
+
+        if ($test->{has_no_history}) {
+            is +FixMyStreet::DB->resultset('ContactsHistory')->search({
+                contact_id => $contact->id,
+            })->count, $count, 'No new history';
+        }
     };
 }