diff options
| -rw-r--r-- | perllib/FixMyStreet/Cobrand.pm | 10 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Cobrand/Bromley.pm | 33 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Cobrand/Greenwich.pm | 9 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Cobrand/Oxfordshire.pm | 23 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Cobrand/UK.pm | 10 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Cobrand/WestBerkshire.pm | 16 | ||||
| -rw-r--r-- | perllib/FixMyStreet/DB/Result/Body.pm | 15 | ||||
| -rw-r--r-- | perllib/FixMyStreet/DB/Result/Problem.pm | 4 | ||||
| -rw-r--r-- | perllib/FixMyStreet/SendReport/Open311.pm | 84 | ||||
| -rw-r--r-- | t/app/sendreport/open311.t | 267 |
10 files changed, 391 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) { diff --git a/t/app/sendreport/open311.t b/t/app/sendreport/open311.t new file mode 100644 index 000000000..c4c17577c --- /dev/null +++ b/t/app/sendreport/open311.t @@ -0,0 +1,267 @@ +use strict; +use warnings; + +use Test::More; +use Test::Deep; + +use Open311; +use FixMyStreet::SendReport::Open311; +use FixMyStreet::DB; + +use Data::Dumper; + +package main; +sub test_overrides; # defined below + +use constant TEST_USER_EMAIL => 'fred@example.com'; + +my %standard_open311_parameters = ( + 'use_extended_updates' => 0, + 'send_notpinpointed' => 0, + 'extended_description' => 1, + 'use_service_as_deviceid' => 0, + 'extended_statuses' => 0, + 'always_send_latlong' => 1, + 'debug' => 0, + 'error' => '', + 'endpoints' => { + 'requests' => 'requests.xml', + 'service_request_updates' => 'servicerequestupdates.xml', + 'services' => 'services.xml', + 'update' => 'servicerequestupdates.xml', + }, +); + +test_overrides oxfordshire => + { + body_name => 'Oxfordshire', + body_id => 2237, + row_data => { + postcode => 'OX1 1AA', + }, + extra => { + northing => 100, + easting => 100, + closest_address => '49 St Giles', + }, + }, + superhashof({ + handler => isa('FixMyStreet::Cobrand::Oxfordshire'), + discard_changes => 1, + 'open311' => noclass(superhashof({ + %standard_open311_parameters, + 'extended_description' => 'oxfordshire', + 'endpoints' => { + 'requests' => 'open311_service_request.cgi' + }, + })), + problem_extra => bag( + { name => 'northing', value => 100 }, + { name => 'easting', value => 100 }, + { name => 'closest_address' => value => '49 St Giles' }, + { name => 'external_id', value => re('[0-9]+') }, + ), + }); + +my $bromley_check = + superhashof({ + handler => isa('FixMyStreet::Cobrand::Bromley'), + discard_changes => 1, + 'open311' => noclass(superhashof({ + %standard_open311_parameters, + 'send_notpinpointed' => 1, + 'extended_description' => 0, + 'use_service_as_deviceid' => 0, + 'always_send_latlong' => 0, + })), + problem_extra => bag( + { name => 'report_url' => value => 'http://example.com/1234' }, + { name => 'report_title', value => 'Problem' }, + { name => 'public_anonymity_required', value => 'TRUE' }, + { name => 'email_alerts_requested', value => 'FALSE' }, + { name => 'requested_datetime', value => re(qr/^(\d{4})-(\d\d)-(\d\d)T(\d\d):(\d\d):(\d\d)/) }, + { name => 'email', value => TEST_USER_EMAIL }, + { name => 'last_name', value => 'Bloggs' }, + ), + }); + +test_overrides bromley => + { + body_name => 'Bromley', + body_id => 2482, + row_data => { + postcode => 'BR1 1AA', + extra => [ { name => 'last_name', value => 'Bloggs' } ], + }, + extra => { + northing => 100, + easting => 100, + url => 'http://example.com/1234', + }, + }, + $bromley_check; + +test_overrides fixmystreet => + { + body_name => 'Bromley', + body_id => 2482, + row_data => { + postcode => 'BR1 1AA', + # NB: we don't pass last_name here, as main cobrand doesn't know to do this! + }, + extra => { + northing => 100, + easting => 100, + url => 'http://example.com/1234', + }, + }, + $bromley_check; + +test_overrides greenwich => + { + body_name => 'Greenwich', + body_id => 2493, + }, + superhashof({ + handler => isa('FixMyStreet::Cobrand::Greenwich'), + 'open311' => noclass(superhashof({ + %standard_open311_parameters, + })), + problem_extra => bag( + { name => 'external_id', value => re('[0-9]+') }, + ), + }); + +test_overrides fixmystreet => + { + body_name => 'West Berkshire', + body_id => 2619, + row_data => { + postcode => 'RG1 1AA', + }, + }, + superhashof({ + handler => isa('FixMyStreet::Cobrand::WestBerkshire'), + 'open311' => noclass(superhashof({ + %standard_open311_parameters, + 'endpoints' => { + 'requests' => 'Requests', + 'services' => 'Services', + }, + })), + }); + +sub test_overrides { + # NB: Open311 and ::SendReport::Open311 are mocked below in BEGIN { ... } + my ($cobrand, $input, $expected_data) = @_; + subtest "$cobrand ($input->{body_name}) overrides" => sub { + + FixMyStreet::override_config { + ALLOWED_COBRANDS => ['fixmystreet', 'oxfordshire', 'bromley', 'westberkshire', 'greenwich'], + }, sub { + my $db = FixMyStreet::DB->storage->schema; + $db->txn_begin; + + my $params = { id => $input->{body_id}, name => $input->{body_name} }; + my $body = $db->resultset('Body')->find_or_create($params); + $body->body_areas->create({ area_id => $input->{body_id} }); + ok $body, "found/created body " . $input->{body_name}; + $body->update({ can_be_devolved => 1 }); + + my $contact = $body->contacts->find_or_create( + confirmed => 1, + email => 'ZZ', + category => 'ZZ', + deleted => 0, + editor => 'test suite', + note => '', + whenedited => DateTime->now, + jurisdiction => '1234', + api_key => 'SEEKRIT', + body_id => $input->{body_id}, + ); + $contact->update({ send_method => 'Open311', endpoint => 'http://example.com/open311' }); + + my $user = $db->resultset('User')->create( { + name => 'Fred Bloggs', + email => TEST_USER_EMAIL, + password => 'dummy', + }); + + my $row = $db->resultset('Problem')->create( { + title => 'Problem', + detail => 'A big problem', + used_map => 1, + name => 'Fred Bloggs', + anonymous => 1, + state => 'unconfirmed', + bodies_str => $input->{body_id}, + areas => (sprintf ',%d,', $input->{body_id}), + category => 'ZZ', + cobrand => $cobrand, + user => $user, + postcode => 'ZZ1 1AA', + latitude => 100, + longitude => 100, + confirmed => DateTime->now(), + %{ $input->{row_data} || {} }, + } ); + + my $sr = FixMyStreet::SendReport::Open311->new; + $sr->add_body($body, $contact); + $sr->send( $row, $input->{extra} || {} ); + + cmp_deeply (Open311->_get_test_data, $expected_data, 'Data as expected') + or diag Dumper( Open311->_get_test_data ); + + Open311->_reset_test_data(); + $db->txn_rollback; + }; + } +} + +BEGIN { + # Prepare the %data variable to write data from Open311 calls to... + my %data; + package Open311; + use Class::Method::Modifiers; + around new => sub { + my $orig = shift; + my ($class, %params) = @_; + my $self = $class->$orig(%params); + $data{open311} = $self; + $self; + }; + around send_service_request => sub { + my $orig = shift; + my ($self, $problem, $extra, $service_code) = @_; + $data{problem} = { $problem->get_columns }; + $data{extra} = $extra; + $data{problem_extra} = $problem->get_extra_fields; + $data{problem_user} = { $problem->user->get_columns }; + $data{service_code} = $service_code; + # don't actually send the service request! + }; + + sub _get_test_data { return +{ %data } } + sub _reset_test_data { %data = () } + + package FixMyStreet::DB::Result::Problem; + use Class::Method::Modifiers; # is marked as immutable by Moose + sub discard_changes { + $data{discard_changes}++; + # no need to actually discard, as we're in transaction anyway + }; + + package FixMyStreet::DB::Result::Body; + use Class::Method::Modifiers; # is marked as immutable by Moose + around get_cobrand_handler => sub { + my $orig = shift; + my ($self) = @_; + my $handler = $self->$orig(); + $data{handler} = $handler; + $handler; + }; +} + +done_testing(); |