Refactor SendReport::Open311 to use cobrand hooks.

Avoid doing cobrand-specific logic within SendReport::Open311.
The hooks defined are:
    open311_config: set extra fields
    open311_pre_send: set endpoints and munge data

These are tested in the new t/app/sendreport/open311.t, which does
not send any actual reports, but just checks that all the data is
munged into the expected format.  (tests under t/open311* should
already test the actual sending)

Remove unused Open311 special cases.

9 files changed, 124 insertions, 80 deletions

diff --git a/perllib/FixMyStreet/Cobrand.pm b/perllib/FixMyStreet/Cobrand.pm
index 9f61635d8..4b9f2bd0b 100644
--- a/perllib/FixMyStreet/Cobrand.pm
+++ b/perllib/FixMyStreet/Cobrand.pm
@@ -153,4 +153,14 @@ sub exists {
     return 0;
 }
 
+sub body_handler {
+    my ($class, $areas) = @_;
+
+    foreach my $avail ( $class->available_cobrand_classes ) {
+        my $cobrand = $class->get_class_for_moniker($avail->{moniker})->new({});
+        next unless $cobrand->can('council_id');
+        return $cobrand if $areas->{$cobrand->council_id};
+    }
+}
+
 1;
diff --git a/perllib/FixMyStreet/Cobrand/Bromley.pm b/perllib/FixMyStreet/Cobrand/Bromley.pm
index 2d0cb86f1..169175947 100644
--- a/perllib/FixMyStreet/Cobrand/Bromley.pm
+++ b/perllib/FixMyStreet/Cobrand/Bromley.pm
@@ -3,6 +3,7 @@ use parent 'FixMyStreet::Cobrand::UKCouncils';
 
 use strict;
 use warnings;
+use DateTime::Format::W3CDTF;
 
 sub council_id { return 2482; }
 sub council_area { return 'Bromley'; }
@@ -111,5 +112,37 @@ sub title_list {
     return ["MR", "MISS", "MRS", "MS", "DR"];
 }
 
+sub open311_config {
+    my ($self, $row, $h, $params) = @_;
+
+    my $extra = $row->get_extra_fields;
+    push @$extra,
+        { name => 'report_url',
+          value => $h->{url} },
+        { name => 'report_title',
+          value => $row->title },
+        { name => 'public_anonymity_required',
+          value => $row->anonymous ? 'TRUE' : 'FALSE' },
+        { name => 'email_alerts_requested',
+          value => 'FALSE' }, # always false as can never request them
+        { name => 'requested_datetime',
+          value => DateTime::Format::W3CDTF->format_datetime($row->confirmed->set_nanosecond(0)) },
+        { name => 'email',
+          value => $row->user->email };
+
+    # make sure we have last_name attribute present in row's extra, so
+    # it is passed correctly to Bromley as attribute[]
+    if ( $row->cobrand ne 'bromley' ) {
+        my ( $firstname, $lastname ) = ( $row->name =~ /(\w+)\.?\s+(.+)/ );
+        push @$extra, { name => 'last_name', value => $lastname };
+    }
+
+    $row->set_extra_fields(@$extra);
+
+    $params->{always_send_latlong} = 0;
+    $params->{send_notpinpointed} = 1;
+    $params->{extended_description} = 0;
+}
+
 1;
 
diff --git a/perllib/FixMyStreet/Cobrand/Greenwich.pm b/perllib/FixMyStreet/Cobrand/Greenwich.pm
index 7d6058145..700a12782 100644
--- a/perllib/FixMyStreet/Cobrand/Greenwich.pm
+++ b/perllib/FixMyStreet/Cobrand/Greenwich.pm
@@ -61,4 +61,13 @@ sub on_map_default_max_pin_age {
     return '21 days';
 }
 
+sub open311_config {
+    my ($self, $row, $h, $params) = @_;
+
+    my $extra = $row->get_extra_fields;
+    # Greenwich doesn't have category metadata to fill this
+    push @$extra, { name => 'external_id', value => $row->id };
+    $row->set_extra_fields( @$extra );
+}
+
 1;
diff --git a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
index 2820719b9..d585a5328 100644
--- a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
+++ b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
@@ -112,6 +112,29 @@ sub pin_colour {
     return 'yellow';
 }
 
+sub open311_config {
+    my ($self, $row, $h, $params) = @_;
+
+    my $extra = $row->get_extra_fields;
+    push @$extra, { name => 'external_id', value => $row->id };
+
+    if ($h->{closest_address}) {
+        push @$extra, { name => 'closest_address', value => $h->{closest_address} }
+    }
+    if ( $row->used_map || ( !$row->used_map && !$row->postcode ) ) {
+        push @$extra, { name => 'northing', value => $h->{northing} };
+        push @$extra, { name => 'easting', value => $h->{easting} };
+    }
+    $row->set_extra_fields( @$extra );
+
+    $params->{extended_description} = 'oxfordshire';
+}
+
+sub open311_pre_send {
+    my ($self, $row, $open311) = @_;
+    $open311->endpoints( { requests => 'open311_service_request.cgi' } );
+}
+
 sub on_map_default_status { return 'open'; }
 
 sub contact_email {
diff --git a/perllib/FixMyStreet/Cobrand/UK.pm b/perllib/FixMyStreet/Cobrand/UK.pm
index 08ecf0b7d..945af48f8 100644
--- a/perllib/FixMyStreet/Cobrand/UK.pm
+++ b/perllib/FixMyStreet/Cobrand/UK.pm
@@ -1,5 +1,6 @@
 package FixMyStreet::Cobrand::UK;
 use base 'FixMyStreet::Cobrand::Default';
+use strict;
 
 use JSON::MaybeXS;
 use mySociety::MaPit;
@@ -354,13 +355,8 @@ sub get_body_handler_for_problem {
     my @bodies = values %{$row->bodies};
     my %areas = map { %{$_->areas} } @bodies;
 
-    foreach my $avail ( FixMyStreet::Cobrand->available_cobrand_classes ) {
-        my $class = FixMyStreet::Cobrand->get_class_for_moniker($avail->{moniker});
-        my $cobrand = $class->new({});
-        next unless $cobrand->can('council_id');
-        return $cobrand if $areas{$cobrand->council_id};
-    }
-
+    my $cobrand = FixMyStreet::Cobrand->body_handler(\%areas);
+    return $cobrand if $cobrand;
     return ref $self ? $self : $self->new;
 }
 
diff --git a/perllib/FixMyStreet/Cobrand/WestBerkshire.pm b/perllib/FixMyStreet/Cobrand/WestBerkshire.pm
new file mode 100644
index 000000000..1ffdf0286
--- /dev/null
+++ b/perllib/FixMyStreet/Cobrand/WestBerkshire.pm
@@ -0,0 +1,16 @@
+package FixMyStreet::Cobrand::WestBerkshire;
+use base 'FixMyStreet::Cobrand::UKCouncils';
+
+use strict;
+use warnings;
+
+sub council_id { 2619 }
+
+# non standard west berks end points
+sub open311_pre_send {
+    my ($self, $row, $open311) = @_;
+    $open311->endpoints( { services => 'Services', requests => 'Requests' } );
+}
+
+1;
+
diff --git a/perllib/FixMyStreet/DB/Result/Body.pm b/perllib/FixMyStreet/DB/Result/Body.pm
index 037b69352..6dac8821c 100644
--- a/perllib/FixMyStreet/DB/Result/Body.pm
+++ b/perllib/FixMyStreet/DB/Result/Body.pm
@@ -127,4 +127,19 @@ sub areas {
     return \%ids;
 }
 
+=head2 get_cobrand_handler
+
+Get a cobrand object for this body, if there is one.
+
+e.g.
+    * if the problem was sent to Bromley it will return ::Bromley
+    * if the problem was sent to Camden it will return nothing
+
+=cut
+
+sub get_cobrand_handler {
+    my $self = shift;
+    return FixMyStreet::Cobrand->body_handler($self->areas);
+}
+
 1;
diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm
index 4ccad3690..dcd5ecc71 100644
--- a/perllib/FixMyStreet/DB/Result/Problem.pm
+++ b/perllib/FixMyStreet/DB/Result/Problem.pm
@@ -1107,9 +1107,7 @@ has traffic_management_options => (
     default => sub {
         my $self = shift;
         my $cobrand = $self->get_cobrand_logged;
-        if ( $cobrand->can('get_body_handler_for_problem') ) {
-            $cobrand = $cobrand->get_body_handler_for_problem( $self );
-        }
+        $cobrand = $cobrand->call_hook(get_body_handler_for_problem => $self) || $cobrand;
         return $cobrand->traffic_management_options;
     },
 );
diff --git a/perllib/FixMyStreet/SendReport/Open311.pm b/perllib/FixMyStreet/SendReport/Open311.pm
index 9c55683ed..059690612 100644
--- a/perllib/FixMyStreet/SendReport/Open311.pm
+++ b/perllib/FixMyStreet/SendReport/Open311.pm
@@ -5,14 +5,7 @@ use namespace::autoclean;
 
 BEGIN { extends 'FixMyStreet::SendReport'; }
 
-use DateTime::Format::W3CDTF;
 use Open311;
-use Readonly;
-
-Readonly::Scalar my $COUNCIL_ID_OXFORDSHIRE => 2237;
-Readonly::Scalar my $COUNCIL_ID_WARWICKSHIRE => 2243;
-Readonly::Scalar my $COUNCIL_ID_GREENWICH => 2493;
-Readonly::Scalar my $COUNCIL_ID_BROMLEY => 2482;
 
 has open311_test_req_used => (
     is => 'rw',
@@ -27,47 +20,18 @@ sub send {
     foreach my $body ( @{ $self->bodies } ) {
         my $conf = $self->body_config->{ $body->id };
 
-        my $always_send_latlong = 1;
-        my $send_notpinpointed  = 0;
-        my $use_service_as_deviceid = 0;
-
-        my $extended_desc = 1;
-
-        my $extra = $row->get_extra_fields();
+        my %open311_params = (
+            jurisdiction            => $conf->jurisdiction,
+            endpoint                => $conf->endpoint,
+            api_key                 => $conf->api_key,
+            always_send_latlong     => 1,
+            send_notpinpointed      => 0,
+            use_service_as_deviceid => 0,
+            extended_description    => 1,
+        );
 
-        # Extra bromley fields
-        if ( $row->bodies_str eq $COUNCIL_ID_BROMLEY ) {
-            push @$extra, { name => 'report_url', value => $h->{url} };
-            push @$extra, { name => 'report_title', value => $row->title };
-            push @$extra, { name => 'public_anonymity_required', value => $row->anonymous ? 'TRUE' : 'FALSE' };
-            push @$extra, { name => 'email_alerts_requested', value => 'FALSE' }; # always false as can never request them
-            push @$extra, { name => 'requested_datetime', value => DateTime::Format::W3CDTF->format_datetime($row->confirmed->set_nanosecond(0)) };
-            push @$extra, { name => 'email', value => $row->user->email };
-            # make sure we have last_name attribute present in row's extra, so
-            # it is passed correctly to Bromley as attribute[]
-            if ( $row->cobrand ne 'bromley' ) {
-                my ( $firstname, $lastname ) = ( $row->name =~ /(\w+)\.?\s+(.+)/ );
-                push @$extra, { name => 'last_name', value => $lastname };
-            }
-            $always_send_latlong = 0;
-            $send_notpinpointed = 1;
-            $extended_desc = 0;
-        } elsif ( $row->bodies_str =~ /\b$COUNCIL_ID_OXFORDSHIRE\b/ ) {
-            # Oxfordshire doesn't have category metadata to fill these
-            $extended_desc = 'oxfordshire';
-            push @$extra, { name => 'external_id', value => $row->id };
-            push @$extra, { name => 'closest_address', value => $h->{closest_address} } if $h->{closest_address};
-            if ( $row->used_map || ( !$row->used_map && !$row->postcode ) ) {
-                push @$extra, { name => 'northing', value => $h->{northing} };
-                push @$extra, { name => 'easting', value => $h->{easting} };
-            }
-        } elsif ( $row->bodies_str =~ /\b$COUNCIL_ID_WARWICKSHIRE\b/ ) {
-            $extended_desc = 'warwickshire';
-            push @$extra, { name => 'closest_address', value => $h->{closest_address} } if $h->{closest_address};
-        } elsif ( $row->bodies_str == $COUNCIL_ID_GREENWICH ) {
-            # Greenwich doesn't have category metadata to fill this
-            push @$extra, { name => 'external_id', value => $row->id };
-        }
+        my $cobrand = $body->get_cobrand_handler || $row->get_cobrand_logged;
+        $cobrand->call_hook(open311_config => $row, $h, \%open311_params);
 
         # Try and fill in some ones that we've been asked for, but not asked the user for
 
@@ -77,6 +41,8 @@ sub send {
             category => $row->category
         } );
 
+        my $extra = $row->get_extra_fields();
+
         my $id_field = $contact->id_field;
         foreach (@{$contact->get_extra_fields}) {
             if ($_->{code} eq $id_field) {
@@ -92,15 +58,6 @@ sub send {
 
         $row->set_extra_fields( @$extra ) if @$extra;
 
-        my %open311_params = (
-            jurisdiction            => $conf->jurisdiction,
-            endpoint                => $conf->endpoint,
-            api_key                 => $conf->api_key,
-            always_send_latlong     => $always_send_latlong,
-            send_notpinpointed      => $send_notpinpointed,
-            use_service_as_deviceid => $use_service_as_deviceid,
-            extended_description    => $extended_desc,
-        );
         if (FixMyStreet->test_mode) {
             my $test_res = HTTP::Response->new();
             $test_res->code(200);
@@ -112,20 +69,7 @@ sub send {
 
         my $open311 = Open311->new( %open311_params );
 
-        # non standard west berks end points
-        if ( $row->bodies_str =~ /2619/ ) {
-            $open311->endpoints( { services => 'Services', requests => 'Requests' } );
-        }
-
-        # non-standard Oxfordshire endpoint (because it's just a script, not a full Open311 service)
-        if ( $row->bodies_str =~ /$COUNCIL_ID_OXFORDSHIRE/ ) {
-            $open311->endpoints( { requests => 'open311_service_request.cgi' } );
-        }
-
-        # required to get round issues with CRM constraints
-        if ( $row->bodies_str =~ /2218/ ) {
-            $row->user->name( $row->user->id . ' ' . $row->user->name );
-        }
+        $cobrand->call_hook(open311_pre_send => $row, $open311);
 
         my $resp = $open311->send_service_request( $row, $h, $contact->email );
         if (FixMyStreet->test_mode) {