[Open311] Move send-comments cobrand specific code

7 files changed, 67 insertions, 61 deletions

diff --git a/bin/send-comments b/bin/send-comments
index 53afeb045..665f377bc 100755
--- a/bin/send-comments
+++ b/bin/send-comments
@@ -23,13 +23,7 @@ use Open311::PostServiceRequestUpdates;
 
 my ($verbose, $nomail) = CronFns::options();
 
-# Set up site, language etc.
-my $base_url = FixMyStreet->config('BASE_URL');
-my $site = '';
-$site = 'fixmystreet.com' if $base_url eq "https://www.fixmystreet.com";
-
 my $updates = Open311::PostServiceRequestUpdates->new(
-    site => $site,
     verbose => $verbose,
 );
 $updates->send;
diff --git a/perllib/FixMyStreet/Cobrand/Bromley.pm b/perllib/FixMyStreet/Cobrand/Bromley.pm
index a6d58504c..e1021eda9 100644
--- a/perllib/FixMyStreet/Cobrand/Bromley.pm
+++ b/perllib/FixMyStreet/Cobrand/Bromley.pm
@@ -165,5 +165,24 @@ sub open311_config {
     $params->{extended_description} = 0;
 }
 
+sub open311_config_updates {
+    my ($self, $params) = @_;
+    $params->{use_extended_updates} = 1;
+    $params->{endpoints} = {
+        service_request_updates => 'update.xml',
+        update => 'update.xml'
+    };
+}
+
+sub open311_pre_send {
+    my ($self, $row, $open311) = @_;
+
+    my $extra = $row->extra || {};
+    unless ( $extra->{title} ) {
+        $extra->{title} = $row->user->title;
+        $row->extra( $extra );
+    }
+}
+
 1;
 
diff --git a/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm b/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm
index 783a565f1..fb074da85 100644
--- a/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm
+++ b/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm
@@ -100,6 +100,11 @@ sub open311_post_send {
     $sender->send($row, $h);
 }
 
+sub open311_config_updates {
+    my ($self, $params) = @_;
+    $params->{mark_reopen} = 1;
+}
+
 sub map_type { 'Buckinghamshire' }
 
 sub default_map_zoom { 3 }
diff --git a/perllib/FixMyStreet/Cobrand/Lewisham.pm b/perllib/FixMyStreet/Cobrand/Lewisham.pm
new file mode 100644
index 000000000..325f6e833
--- /dev/null
+++ b/perllib/FixMyStreet/Cobrand/Lewisham.pm
@@ -0,0 +1,15 @@
+package FixMyStreet::Cobrand::Lewisham;
+use base 'FixMyStreet::Cobrand::UK';
+
+use strict;
+use warnings;
+
+sub council_area_id { 2492 }
+
+sub open311_post_update_skip {
+    my ($self) = @_;
+    return 1;
+}
+
+1;
+
diff --git a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
index cac79a347..dafd6e069 100644
--- a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
+++ b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
@@ -155,6 +155,19 @@ sub open311_config {
     $params->{extended_description} = 'oxfordshire';
 }
 
+sub open311_config_updates {
+    my ($self, $params) = @_;
+    $params->{use_customer_reference} = 1;
+}
+
+sub should_skip_sending_update {
+    my ($self, $update ) = @_;
+
+    # Oxfordshire stores the external id of the problem as a customer reference
+    # in metadata
+    return 1 if !$update->problem->get_extra_metadata('customer_reference');
+}
+
 sub on_map_default_status { return 'open'; }
 
 sub contact_email {
diff --git a/perllib/Open311/PostServiceRequestUpdates.pm b/perllib/Open311/PostServiceRequestUpdates.pm
index dccd26184..63e149ece 100755
--- a/perllib/Open311/PostServiceRequestUpdates.pm
+++ b/perllib/Open311/PostServiceRequestUpdates.pm
@@ -13,13 +13,7 @@ use Open311;
 
 use constant SEND_METHOD_OPEN311 => 'Open311';
 
-use constant COUNCIL_ID_OXFORDSHIRE => 2237;
-use constant COUNCIL_ID_BROMLEY => 2482;
-use constant COUNCIL_ID_LEWISHAM => 2492;
-use constant COUNCIL_ID_BUCKS => 2217;
-
 has verbose => ( is => 'ro', default => 0 );
-has site => ( is => 'ro', default => '' );
 
 sub send {
     my $self = shift;
@@ -30,12 +24,8 @@ sub send {
     } );
 
     while ( my $body = $bodies->next ) {
-        # XXX Cobrand specific - see also list in Problem->updates_sent_to_body
-        if ($self->site eq 'fixmystreet.com') {
-            # Lewisham does not yet accept updates
-            next if $body->areas->{+COUNCIL_ID_LEWISHAM};
-        }
-
+        my $cobrand = $body->get_cobrand_handler;
+        next if $cobrand && $cobrand->call_hook('open311_post_update_skip');
         $self->process_body($body);
     }
 }
@@ -43,29 +33,16 @@ sub send {
 sub open311_params {
     my ($self, $body) = @_;
 
-    my $use_extended = 0;
-    if ( $self->site eq 'fixmystreet.com' && $body->areas->{+COUNCIL_ID_BROMLEY} ) {
-        $use_extended = 1;
-    }
-
     my %open311_conf = (
         endpoint => $body->endpoint,
         jurisdiction => $body->jurisdiction,
         api_key => $body->api_key,
-        use_extended_updates => $use_extended,
+        extended_statuses => $body->send_extended_statuses,
     );
 
-    if ( $body->areas->{+COUNCIL_ID_OXFORDSHIRE} ) {
-        $open311_conf{use_customer_reference} = 1;
-    }
-
-    if ( $body->areas->{+COUNCIL_ID_BUCKS} ) {
-        $open311_conf{mark_reopen} = 1;
-    }
-
-    if ( $body->send_extended_statuses ) {
-        $open311_conf{extended_statuses} = 1;
-    }
+    my $cobrand = $body->get_cobrand_handler;
+    $cobrand->call_hook(open311_config_updates => \%open311_conf)
+        if $cobrand;
 
     return %open311_conf;
 }
@@ -75,13 +52,6 @@ sub process_body {
 
     my $o = Open311->new( $self->open311_params($body) );
 
-    if ( $self->site eq 'fixmystreet.com' && $body->areas->{+COUNCIL_ID_BROMLEY} ) {
-        my $endpoints = $o->endpoints;
-        $endpoints->{update} = 'update.xml';
-        $endpoints->{service_request_updates} = 'update.xml';
-        $o->endpoints( $endpoints );
-    }
-
     my $comments = FixMyStreet::DB->resultset('Comment')->search( {
             'me.whensent' => undef,
             'me.external_id' => undef,
@@ -111,31 +81,16 @@ sub process_body {
             next;
         }
 
-        # Oxfordshire stores the external id of the problem as a customer reference
-        # in metadata
-        if ($body->areas->{+COUNCIL_ID_OXFORDSHIRE} &&
-            !$comment->problem->get_extra_metadata('customer_reference') ) {
-            next;
-        }
-
         next if !$self->verbose && $comment->send_fail_count && retry_timeout($comment);
 
-        $self->process_update($body, $o, $comment);
+        $self->process_update($body, $o, $comment, $cobrand);
     }
 }
 
 sub process_update {
-    my ($self, $body, $o, $comment) = @_;
-
-    if ( $self->site eq 'fixmystreet.com' && $body->areas->{+COUNCIL_ID_BROMLEY} ) {
-        my $extra = $comment->extra;
-        $extra = {} if !$extra;
+    my ($self, $body, $o, $comment, $cobrand) = @_;
 
-        unless ( $extra->{title} ) {
-            $extra->{title} = $comment->user->title;
-            $comment->extra( $extra );
-        }
-    }
+    $cobrand->call_hook(open311_pre_send => $comment, $o);
 
     my $id = $o->post_service_request_update( $comment );
 
diff --git a/t/open311/post-service-request-updates.t b/t/open311/post-service-request-updates.t
index 95013b951..5c8211bbf 100644
--- a/t/open311/post-service-request-updates.t
+++ b/t/open311/post-service-request-updates.t
@@ -21,17 +21,21 @@ my $bucks = $mech->create_body_ok(2217, 'Buckinghamshire', $params);
 my $lewisham = $mech->create_body_ok(2492, 'Lewisham', $params);
 
 subtest 'Check Open311 params' => sub {
+  FixMyStreet::override_config {
+    ALLOWED_COBRANDS => ['fixmystreet', 'bromley', 'buckinghamshire', 'lewisham', 'oxfordshire'],
+  }, sub {
     my $result = {
         endpoint => 'endpoint',
         jurisdiction => 'home',
         api_key => 'KEY',
-        use_extended_updates => 0,
+        extended_statuses => undef,
     };
     my %conf = $o->open311_params($bromley);
     is_deeply \%conf, {
         %$result,
         extended_statuses => 1,
         use_extended_updates => 1,
+        endpoints => { service_request_updates => 'update.xml', update => 'update.xml' },
     }, 'Bromley params match';
     %conf = $o->open311_params($oxon);
     is_deeply \%conf, {
@@ -45,6 +49,7 @@ subtest 'Check Open311 params' => sub {
     }, 'Bucks params match';
     %conf = $o->open311_params($lewisham);
     is_deeply \%conf, $result, 'Lewisham params match';
+  };
 };
 
 my $other_user = $mech->create_user_ok('test2@example.com', title => 'MRS');