aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGELOG.md2
-rwxr-xr-xbin/send-comments163
-rwxr-xr-xperllib/Open311/PostServiceRequestUpdates.pm174
-rw-r--r--t/open311.t20
-rw-r--r--t/open311/post-service-request-updates.t102
5 files changed, 304 insertions, 157 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a0a3d2d5d..bc13c6dac 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,8 @@
- Clearer relocation options while you’re reporting a problem #2238
- Bugfixes:
- Add perl 5.26/5.28 support.
+ - Internal things:
+ - Move send-comments code to package for testing.
* v2.4.1 (2nd October 2018)
- New features:
diff --git a/bin/send-comments b/bin/send-comments
index 33da7379e..53afeb045 100755
--- a/bin/send-comments
+++ b/bin/send-comments
@@ -18,167 +18,18 @@ BEGIN {
}
use CronFns;
-
-use DateTime;
use FixMyStreet;
-use FixMyStreet::Cobrand;
-use FixMyStreet::DB;
-use Open311;
-
-# send_method config values found in by-area config data, for selecting to appropriate method
-use constant SEND_METHOD_EMAIL => 'email';
-use constant SEND_METHOD_OPEN311 => 'Open311';
+use Open311::PostServiceRequestUpdates;
-use constant COUNCIL_ID_OXFORDSHIRE => 2237;
-use constant COUNCIL_ID_BROMLEY => 2482;
-use constant COUNCIL_ID_LEWISHAM => 2492;
-use constant COUNCIL_ID_BUCKS => 2217;
+my ($verbose, $nomail) = CronFns::options();
# Set up site, language etc.
-my ($verbose, $nomail) = CronFns::options();
my $base_url = FixMyStreet->config('BASE_URL');
my $site = '';
$site = 'fixmystreet.com' if $base_url eq "https://www.fixmystreet.com";
-my $bodies = FixMyStreet::DB->resultset('Body')->search( {
- send_method => SEND_METHOD_OPEN311,
- send_comments => 1,
-} );
-
-while ( my $body = $bodies->next ) {
-
- # XXX Cobrand specific - see also list in Problem->updates_sent_to_body
- if ($site eq 'fixmystreet.com') {
- # Lewisham does not yet accept updates
- next if $body->areas->{+COUNCIL_ID_LEWISHAM};
- }
-
- my $use_extended = 0;
- my $comments = FixMyStreet::DB->resultset('Comment')->search( {
- 'me.whensent' => undef,
- 'me.external_id' => undef,
- 'me.state' => 'confirmed',
- 'me.confirmed' => { '!=' => undef },
- 'problem.whensent' => { '!=' => undef },
- 'problem.external_id' => { '!=' => undef },
- 'problem.bodies_str' => { -like => '%' . $body->id . '%' },
- 'problem.send_method_used' => 'Open311',
- },
- {
- join => 'problem',
- order_by => [ 'confirmed', 'id' ],
- }
- );
-
- if ( $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,
- );
-
- 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 $o = Open311->new( %open311_conf );
-
- if ( $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 );
- }
-
- while ( my $comment = $comments->next ) {
- my $cobrand = $body->get_cobrand_handler ||
- FixMyStreet::Cobrand->get_class_for_moniker($comment->cobrand)->new();
-
- # Some cobrands (e.g. Buckinghamshire) don't want to receive updates
- # from anyone except the original problem reporter.
- if ($cobrand->call_hook(should_skip_sending_update => $comment)) {
- unless (defined $comment->get_extra_metadata('cobrand_skipped_sending')) {
- $comment->set_extra_metadata(cobrand_skipped_sending => 1);
- $comment->update;
- }
- 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;
- }
-
- # TODO actually this should be OK for any devolved endpoint if original Open311->can_be_devolved, presumably
- if ( 0 ) { # Check can_be_devolved and do this properly if set
- my $sender = $cobrand->get_body_sender( $body, $comment->problem->category );
- my $config = $sender->{config};
- $o = Open311->new(
- endpoint => $config->endpoint,
- jurisdiction => $config->jurisdiction,
- api_key => $config->api_key,
- use_extended_updates => 1, # FMB uses extended updates
- );
- }
-
- next if !$verbose && $comment->send_fail_count && retry_timeout($comment);
-
- if ( $site eq 'fixmystreet.com' && $body->areas->{+COUNCIL_ID_BROMLEY} ) {
- my $extra = $comment->extra;
- if ( !$extra ) {
- $extra = {};
- }
-
- unless ( $extra->{title} ) {
- $extra->{title} = $comment->user->title;
- $comment->extra( $extra );
- }
- }
-
- my $id = $o->post_service_request_update( $comment );
-
- if ( $id ) {
- $comment->update( {
- external_id => $id,
- whensent => \'current_timestamp',
- } );
- } else {
- $comment->update( {
- send_fail_count => $comment->send_fail_count + 1,
- send_fail_timestamp => \'current_timestamp',
- send_fail_reason => "Failed to post over Open311\n\n" . $o->error,
- } );
-
- if ( $verbose && $o->error ) {
- warn $o->error;
- }
- }
- }
-}
-
-sub retry_timeout {
- my $row = shift;
-
- my $tz = FixMyStreet->local_time_zone;
- my $now = DateTime->now( time_zone => $tz );
- my $diff = $now - $row->send_fail_timestamp;
- if ( $diff->in_units( 'minutes' ) < 30 ) {
- return 1;
- }
-
- return 0;
-}
+my $updates = Open311::PostServiceRequestUpdates->new(
+ site => $site,
+ verbose => $verbose,
+);
+$updates->send;
diff --git a/perllib/Open311/PostServiceRequestUpdates.pm b/perllib/Open311/PostServiceRequestUpdates.pm
new file mode 100755
index 000000000..c2b8c2494
--- /dev/null
+++ b/perllib/Open311/PostServiceRequestUpdates.pm
@@ -0,0 +1,174 @@
+package Open311::PostServiceRequestUpdates;
+
+use strict;
+use warnings;
+use v5.14;
+
+use DateTime;
+use Moo;
+use FixMyStreet;
+use FixMyStreet::Cobrand;
+use FixMyStreet::DB;
+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;
+
+ my $bodies = FixMyStreet::DB->resultset('Body')->search( {
+ send_method => SEND_METHOD_OPEN311,
+ send_comments => 1,
+ } );
+
+ 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};
+ }
+
+ $self->process_body($body);
+ }
+}
+
+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,
+ );
+
+ 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;
+ }
+
+ return %open311_conf;
+}
+
+sub process_body {
+ my ($self, $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,
+ 'me.state' => 'confirmed',
+ 'me.confirmed' => { '!=' => undef },
+ 'problem.whensent' => { '!=' => undef },
+ 'problem.external_id' => { '!=' => undef },
+ 'problem.bodies_str' => { -like => '%' . $body->id . '%' },
+ 'problem.send_method_used' => 'Open311',
+ },
+ {
+ join => 'problem',
+ order_by => [ 'confirmed', 'id' ],
+ }
+ );
+
+ while ( my $comment = $comments->next ) {
+ my $cobrand = $body->get_cobrand_handler ||
+ FixMyStreet::Cobrand->get_class_for_moniker($comment->cobrand)->new();
+
+ # Some cobrands (e.g. Buckinghamshire) don't want to receive updates
+ # from anyone except the original problem reporter.
+ if ($cobrand->call_hook(should_skip_sending_update => $comment)) {
+ unless (defined $comment->get_extra_metadata('cobrand_skipped_sending')) {
+ $comment->set_extra_metadata(cobrand_skipped_sending => 1);
+ $comment->update;
+ }
+ 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);
+ }
+}
+
+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;
+
+ unless ( $extra->{title} ) {
+ $extra->{title} = $comment->user->title;
+ $comment->extra( $extra );
+ }
+ }
+
+ my $id = $o->post_service_request_update( $comment );
+
+ if ( $id ) {
+ $comment->update( {
+ external_id => $id,
+ whensent => \'current_timestamp',
+ } );
+ } else {
+ $comment->update( {
+ send_fail_count => $comment->send_fail_count + 1,
+ send_fail_timestamp => \'current_timestamp',
+ send_fail_reason => "Failed to post over Open311\n\n" . $o->error,
+ } );
+
+ if ( $self->verbose && $o->error ) {
+ warn $o->error;
+ }
+ }
+}
+
+sub retry_timeout {
+ my $row = shift;
+
+ my $tz = FixMyStreet->local_time_zone;
+ my $now = DateTime->now( time_zone => $tz );
+ my $diff = $now - $row->send_fail_timestamp;
+ if ( $diff->in_units( 'minutes' ) < 30 ) {
+ return 1;
+ }
+
+ return 0;
+}
+
+1;
diff --git a/t/open311.t b/t/open311.t
index e1ad578d7..ef52eb538 100644
--- a/t/open311.t
+++ b/t/open311.t
@@ -415,10 +415,26 @@ foreach my $test (
status => 'OPEN',
extended => 'IN_PROGRESS',
},
+ {
+ desc => 'comment that marks problem open sends OPEN if not mark_reopen',
+ state => 'confirmed',
+ status => 'OPEN',
+ extended => 'OPEN',
+ mark_open => 1,
+ },
+ {
+ desc => 'comment that marks problem open sends REOPEN if mark_reopen',
+ state => 'confirmed',
+ status => 'OPEN',
+ extended => 'REOPEN',
+ mark_open => 1,
+ mark_reopen => 1,
+ },
) {
subtest $test->{desc} => sub {
$comment->problem_state( $test->{state} );
$comment->problem->state( $test->{state} );
+ $comment->mark_open(1) if $test->{mark_open};
my $results = make_update_req( $comment, '<?xml version="1.0" encoding="utf-8"?><service_request_updates><request_update><update_id>248</update_id></request_update></service_request_updates>' );
@@ -426,7 +442,9 @@ foreach my $test (
is $c->param('status'), $test->{status}, 'correct status';
if ( $test->{extended} ) {
- my $results = make_update_req( $comment, '<?xml version="1.0" encoding="utf-8"?><service_request_updates><request_update><update_id>248</update_id></request_update></service_request_updates>', { extended_statuses => 1 } );
+ my $params = { extended_statuses => 1 };
+ $params->{mark_reopen} = 1 if $test->{mark_reopen};
+ my $results = make_update_req( $comment, '<?xml version="1.0" encoding="utf-8"?><service_request_updates><request_update><update_id>248</update_id></request_update></service_request_updates>', $params );
my $c = CGI::Simple->new( $results->{ req }->content );
is $c->param('status'), $test->{extended}, 'correct extended status';
}
diff --git a/t/open311/post-service-request-updates.t b/t/open311/post-service-request-updates.t
new file mode 100644
index 000000000..95013b951
--- /dev/null
+++ b/t/open311/post-service-request-updates.t
@@ -0,0 +1,102 @@
+#!/usr/bin/env perl
+
+use FixMyStreet::TestMech;
+
+my $mech = FixMyStreet::TestMech->new;
+
+use_ok( 'Open311::PostServiceRequestUpdates' );
+
+my $o = Open311::PostServiceRequestUpdates->new( site => 'fixmystreet.com' );
+
+my $params = {
+ send_method => 'Open311',
+ send_comments => 1,
+ api_key => 'KEY',
+ endpoint => 'endpoint',
+ jurisdiction => 'home',
+};
+my $bromley = $mech->create_body_ok(2482, 'Bromley', { %$params, send_extended_statuses => 1 });
+my $oxon = $mech->create_body_ok(2237, 'Oxfordshire', $params);
+my $bucks = $mech->create_body_ok(2217, 'Buckinghamshire', $params);
+my $lewisham = $mech->create_body_ok(2492, 'Lewisham', $params);
+
+subtest 'Check Open311 params' => sub {
+ my $result = {
+ endpoint => 'endpoint',
+ jurisdiction => 'home',
+ api_key => 'KEY',
+ use_extended_updates => 0,
+ };
+ my %conf = $o->open311_params($bromley);
+ is_deeply \%conf, {
+ %$result,
+ extended_statuses => 1,
+ use_extended_updates => 1,
+ }, 'Bromley params match';
+ %conf = $o->open311_params($oxon);
+ is_deeply \%conf, {
+ %$result,
+ use_customer_reference => 1
+ }, 'Oxfordshire params match';
+ %conf = $o->open311_params($bucks);
+ is_deeply \%conf, {
+ %$result,
+ mark_reopen => 1,
+ }, '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');
+
+sub c {
+ my ($p, $user) = @_;
+ my $c = $mech->create_comment_for_problem($p, $user || $p->user, 'Name', 'Update text', 'f', 'confirmed', 'confirmed', { confirmed => \'current_timestamp' });
+ return $c;
+}
+
+sub p_and_c {
+ my ($body, $user) = @_;
+
+ my $prob_params = { send_method_used => 'Open311', whensent => \'current_timestamp', external_id => 1 };
+ my ($p) = $mech->create_problems_for_body(1, $body->id, 'Title', $prob_params);
+ my $c = c($p, $user);
+ return ($p, $c);
+}
+
+my ($p1, $c1) = p_and_c($bromley, $other_user);
+my ($p2, $c2) = p_and_c($oxon);
+my ($p3, $c3a) = p_and_c($bucks);
+my $c3b = c($p3, $other_user);
+my ($p4, $c4) = p_and_c($lewisham);
+
+subtest 'Send comments' => sub {
+ FixMyStreet::override_config {
+ ALLOWED_COBRANDS => ['fixmystreet', 'bromley', 'buckinghamshire', 'lewisham', 'oxfordshire'],
+ }, sub {
+ $o->send;
+ $c3a->discard_changes;
+ is $c3a->extra, undef, 'Bucks update by owner was sent';
+ $c3b->discard_changes;
+ is $c3b->extra->{cobrand_skipped_sending}, 1, 'Bucks update by other was not';
+ $c1->discard_changes;
+ is $c1->extra->{title}, "MRS", 'Title set on Bromley update';
+ $c2->discard_changes;
+ is $c2->send_fail_count, 0, 'Oxfordshire update skipped entirely';
+ };
+};
+
+subtest 'Oxfordshire gets an ID' => sub {
+ FixMyStreet::override_config {
+ ALLOWED_COBRANDS => ['fixmystreet', 'bromley', 'buckinghamshire', 'lewisham', 'oxfordshire'],
+ }, sub {
+ $p2->set_extra_metadata(customer_reference => 'ABC');
+ $p2->update;
+ $o->send;
+ $c2->discard_changes;
+ is $c2->send_fail_count, 1, 'Oxfordshire update tried to send, failed';
+ };
+};
+
+
+done_testing();
generated by cgit v1.2.3 (git 2.25.1) at 2025-04-26 16:28:54 +0000