diff options
-rw-r--r-- | CHANGELOG.md | 3 | ||||
-rwxr-xr-x | bin/update-schema | 1 | ||||
-rw-r--r-- | db/downgrade_0061---0060.sql | 5 | ||||
-rw-r--r-- | db/schema.sql | 3 | ||||
-rw-r--r-- | db/schema_0061-add-extra-body.sql | 5 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin.pm | 23 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/BathNES.pm | 17 | ||||
-rw-r--r-- | perllib/FixMyStreet/DB/Result/Body.pm | 12 | ||||
-rw-r--r-- | perllib/Open311/GetServiceRequests.pm | 24 | ||||
-rw-r--r-- | t/app/controller/admin/bodies.t | 39 | ||||
-rw-r--r-- | t/app/controller/report_inspect.t | 25 | ||||
-rw-r--r-- | t/open311/getservicerequests.t | 41 | ||||
-rw-r--r-- | templates/web/base/admin/open311-form-fields.html | 126 | ||||
-rw-r--r-- | templates/web/base/report/_inspect.html | 3 | ||||
-rw-r--r-- | templates/web/bathnes/header_extra.html | 1 | ||||
-rw-r--r-- | templates/web/bathnes/tracking_code.html | 12 | ||||
-rw-r--r-- | web/cobrands/sass/_admin.scss | 5 |
17 files changed, 274 insertions, 71 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index cdf048a9e..bb6e0e94e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ * Unreleased - New features - - Fetch problems over Open311 #1986 + - Fetch problems over Open311 #1986 #2067 - Option to send multiple photos over Open311 #1986 - Allow Open311 service definitions to include automated attributes #1986 @@ -42,6 +42,7 @@ - Don't send sent-report emails to as-body/as-anonymous reports. - Show Open311 service code as tooltip on admin category checkboxes. #2049 - Bulk user import admin page. #2057 + - Add link to admin edit page for reports. #2071 - Development improvements: - Add HTML email previewer. - Add CORS header to Open311 output. #2022 diff --git a/bin/update-schema b/bin/update-schema index bbfd732f2..fb88c84a1 100755 --- a/bin/update-schema +++ b/bin/update-schema @@ -212,6 +212,7 @@ else { # (assuming schema change files are never half-applied, which should be the case) sub get_db_version { return 'EMPTY' if ! table_exists('problem'); + return '0061' if column_exists('body', 'extra'); return '0060' if column_exists('body', 'convert_latlong'); return '0059' if column_exists('response_templates', 'external_status_code'); return '0058' if column_exists('body', 'blank_updates_permitted'); diff --git a/db/downgrade_0061---0060.sql b/db/downgrade_0061---0060.sql new file mode 100644 index 000000000..d6934b9b0 --- /dev/null +++ b/db/downgrade_0061---0060.sql @@ -0,0 +1,5 @@ +BEGIN; + +ALTER TABLE body DROP extra; + +COMMIT; diff --git a/db/schema.sql b/db/schema.sql index d08ea675d..fa0bd07bd 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -57,7 +57,8 @@ create table body ( fetch_problems boolean not null default 'f', blank_updates_permitted boolean not null default 'f', convert_latlong boolean not null default 'f', - deleted boolean not null default 'f' + deleted boolean not null default 'f', + extra text ); create table body_areas ( diff --git a/db/schema_0061-add-extra-body.sql b/db/schema_0061-add-extra-body.sql new file mode 100644 index 000000000..125b171fb --- /dev/null +++ b/db/schema_0061-add-extra-body.sql @@ -0,0 +1,5 @@ +BEGIN; + +ALTER TABLE body ADD extra text; + +COMMIT; diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm index 6f7214906..33404cb82 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -218,9 +218,14 @@ sub bodies : Path('bodies') : Args(0) { $c->forward('check_for_super_user'); $c->forward('/auth/check_csrf_token'); - my $params = $c->forward('body_params'); + my $values = $c->forward('body_params'); unless ( keys %{$c->stash->{body_errors}} ) { - my $body = $c->model('DB::Body')->create( $params ); + my $body = $c->model('DB::Body')->create( $values->{params} ); + if ($values->{extras}) { + $body->set_extra_metadata( $_ => $values->{extras}->{$_} ) + for keys %{$values->{extras}}; + $body->update; + } my @area_ids = $c->get_param_list('area_ids'); foreach (@area_ids) { $c->model('DB::BodyArea')->create( { body => $body, area_id => $_ } ); @@ -396,9 +401,14 @@ sub update_contacts : Private { $c->forward('check_for_super_user'); $c->forward('/auth/check_csrf_token'); - my $params = $c->forward( 'body_params' ); + my $values = $c->forward( 'body_params' ); unless ( keys %{$c->stash->{body_errors}} ) { - $c->stash->{body}->update( $params ); + $c->stash->{body}->update( $values->{params} ); + if ($values->{extras}) { + $c->stash->{body}->set_extra_metadata( $_ => $values->{extras}->{$_} ) + for keys %{$values->{extras}}; + $c->stash->{body}->update; + } my @current = $c->stash->{body}->body_areas->all; my %current = map { $_->area_id => 1 } @current; my @area_ids = $c->get_param_list('area_ids'); @@ -465,7 +475,10 @@ sub body_params : Private { ); my %params = map { $_ => $c->get_param($_) || $defaults{$_} } keys %defaults; $c->forward('check_body_params', [ \%params ]); - return \%params; + my @extras = qw/fetch_all_problems/; + %defaults = map { $_ => '' } @extras; + my %extras = map { $_ => $c->get_param($_) || $defaults{$_} } @extras; + return { params => \%params, extras => \%extras }; } sub check_body_params : Private { diff --git a/perllib/FixMyStreet/Cobrand/BathNES.pm b/perllib/FixMyStreet/Cobrand/BathNES.pm index 02d491e0e..f83887469 100644 --- a/perllib/FixMyStreet/Cobrand/BathNES.pm +++ b/perllib/FixMyStreet/Cobrand/BathNES.pm @@ -190,5 +190,22 @@ sub enter_postcode_text { return 'Enter a location in ' . $self->council_area; } +sub categories_restriction { + my ($self, $rs) = @_; + # Categories covering BANES have a mixture of Open311 and Email + # send methods. BANES only want specific categories to be visible on their + # cobrand, not the email categories from FMS.com. + # The FMS.com categories have a devolved send_method set to Email, so we can + # filter these out. + # NB. BANES have a 'Street Light Fault' category that has its + # send_method set to 'Email::BathNES' (to use a custom template) which must + # be show on the cobrand. + return $rs->search( { -or => [ + 'me.send_method' => undef, # Open311 categories + 'me.send_method' => '', # Open311 categories that have been edited in the admin + 'me.send_method' => 'Email::BathNES', # Street Light Fault + ] } ); +} + 1; diff --git a/perllib/FixMyStreet/DB/Result/Body.pm b/perllib/FixMyStreet/DB/Result/Body.pm index 0b11f2771..74a38f225 100644 --- a/perllib/FixMyStreet/DB/Result/Body.pm +++ b/perllib/FixMyStreet/DB/Result/Body.pm @@ -50,6 +50,8 @@ __PACKAGE__->add_columns( { data_type => "boolean", default_value => \"false", is_nullable => 1 }, "convert_latlong", { data_type => "boolean", default_value => \"false", is_nullable => 0 }, + "extra", + { data_type => "text", is_nullable => 1 }, ); __PACKAGE__->set_primary_key("id"); __PACKAGE__->has_many( @@ -124,13 +126,17 @@ __PACKAGE__->has_many( ); -# Created by DBIx::Class::Schema::Loader v0.07035 @ 2018-03-15 12:38:36 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:rturOWpYmPRO0yE9yWHXjA +# Created by DBIx::Class::Schema::Loader v0.07035 @ 2018-04-05 14:29:33 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:HV8IM2C1ErrpvXoRTZ1B1Q + +__PACKAGE__->load_components("+FixMyStreet::DB::RABXColumn"); +__PACKAGE__->rabx_column('extra'); use Moo; use namespace::clean; -with 'FixMyStreet::Roles::Translatable'; +with 'FixMyStreet::Roles::Translatable', + 'FixMyStreet::Roles::Extra'; sub url { my ( $self, $c, $args ) = @_; diff --git a/perllib/Open311/GetServiceRequests.pm b/perllib/Open311/GetServiceRequests.pm index 78e9647fa..48e35acab 100644 --- a/perllib/Open311/GetServiceRequests.pm +++ b/perllib/Open311/GetServiceRequests.pm @@ -9,6 +9,7 @@ use DateTime::Format::W3CDTF; has system_user => ( is => 'rw' ); has start_date => ( is => 'ro', default => sub { undef } ); has end_date => ( is => 'ro', default => sub { undef } ); +has fetch_all => ( is => 'rw', default => 0 ); has verbose => ( is => 'ro', default => 0 ); has schema => ( is =>'ro', lazy => 1, default => sub { FixMyStreet::DB->schema->connect } ); has convert_latlong => ( is => 'rw', default => 0 ); @@ -26,19 +27,28 @@ sub fetch { ); while ( my $body = $bodies->next ) { - - my $o = Open311->new( - endpoint => $body->endpoint, - api_key => $body->api_key, - jurisdiction => $body->jurisdiction, - ); + my $o = $self->create_open311_object( $body ); $self->system_user( $body->comment_user ); $self->convert_latlong( $body->convert_latlong ); + $self->fetch_all( $body->get_extra_metadata('fetch_all_problems') ); $self->create_problems( $o, $body ); } } +# this is so we can test +sub create_open311_object { + my ($self, $body) = @_; + + my $o = Open311->new( + endpoint => $body->endpoint, + api_key => $body->api_key, + jurisdiction => $body->jurisdiction, + ); + + return $o; +} + sub create_problems { my ( $self, $open311, $body ) = @_; @@ -49,7 +59,7 @@ sub create_problems { $args->{start_date} = DateTime::Format::W3CDTF->format_datetime( $self->start_date ); $args->{end_date} = DateTime::Format::W3CDTF->format_datetime( $self->end_date ); - } else { + } elsif ( !$self->fetch_all ) { my $end_dt = DateTime->now(); my $start_dt = $end_dt->clone; $start_dt->add( hours => -1 ); diff --git a/t/app/controller/admin/bodies.t b/t/app/controller/admin/bodies.t index 9bdf8fb9a..a485d286d 100644 --- a/t/app/controller/admin/bodies.t +++ b/t/app/controller/admin/bodies.t @@ -150,6 +150,45 @@ subtest 'check open311 configuring' => sub { is $conf->endpoint, 'http://example.org/open311', 'endpoint updated'; is $conf->api_key, 'new api key', 'api key updated'; is $conf->jurisdiction, 'open311', 'jurisdiction configures'; + ok !$conf->get_extra_metadata('fetch_all_problems'), 'fetch all problems unset'; + + $mech->form_number(3); + $mech->submit_form_ok( + { + with_fields => { + api_key => 'new api key', + endpoint => 'http://example.org/open311', + jurisdiction => 'open311', + send_comments => 0, + send_method => 'Open311', + fetch_all_problems => 1, + } + } + ); + + $mech->content_contains('Values updated'); + + $conf = FixMyStreet::App->model('DB::Body')->find( $body->id ); + ok $conf->get_extra_metadata('fetch_all_problems'), 'fetch all problems set'; + + $mech->form_number(3); + $mech->submit_form_ok( + { + with_fields => { + api_key => 'new api key', + endpoint => 'http://example.org/open311', + jurisdiction => 'open311', + send_comments => 0, + send_method => 'Open311', + fetch_all_problems => 0, + } + } + ); + + $mech->content_contains('Values updated'); + + $conf = FixMyStreet::App->model('DB::Body')->find( $body->id ); + ok !$conf->get_extra_metadata('fetch_all_problems'), 'fetch all problems unset'; }; subtest 'check text output' => sub { diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t index 5447c744e..f2b300e11 100644 --- a/t/app/controller/report_inspect.t +++ b/t/app/controller/report_inspect.t @@ -50,18 +50,36 @@ FixMyStreet::override_config { $mech->content_lacks('Save changes'); $mech->content_lacks('Priority'); $mech->content_lacks('Traffic management'); + $mech->content_lacks('/admin/report_edit/'.$report_id.'">admin</a>)'); $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_edit_priority' }); $mech->get_ok("/report/$report_id"); $mech->content_contains('Save changes'); $mech->content_contains('Priority'); $mech->content_lacks('Traffic management'); + $mech->content_lacks('/admin/report_edit/'.$report_id.'">admin</a>)'); $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_inspect' }); $mech->get_ok("/report/$report_id"); $mech->content_contains('Save changes'); $mech->content_contains('Priority'); $mech->content_contains('Traffic management'); + $mech->content_lacks('/admin/report_edit/'.$report_id.'">admin</a>)'); + }; + + subtest "council staff can't see admin report edit link on FMS.com" => sub { + my $report_edit_permission = $user->user_body_permissions->create({ + body => $oxon, permission_type => 'report_edit' }); + $mech->get_ok("/report/$report_id"); + $mech->content_lacks('/admin/report_edit/'.$report_id.'">admin</a>)'); + $report_edit_permission->delete; + }; + + subtest "superusers can see admin report edit link on FMS.com" => sub { + $user->update({is_superuser => 1}); + $mech->get_ok("/report/$report_id"); + $mech->content_contains('/admin/report_edit/'.$report_id.'">admin</a>)'); + $user->update({is_superuser => 0}); }; subtest "test basic inspect submission" => sub { @@ -539,6 +557,13 @@ FixMyStreet::override_config { is $report->get_extra_metadata('traffic_information'), 'Signs and Cones', 'report data changed'; }; + subtest "admin link present on inspect page on cobrand" => sub { + my $report_edit_permission = $user->user_body_permissions->create({ + body => $oxon, permission_type => 'report_edit' }); + $mech->get_ok("/report/$report_id"); + $mech->content_contains('/admin/report_edit/'.$report_id.'">admin</a>)'); + $report_edit_permission->delete; + }; }; FixMyStreet::override_config { diff --git a/t/open311/getservicerequests.t b/t/open311/getservicerequests.t index c59e5cc41..57f112e2f 100644 --- a/t/open311/getservicerequests.t +++ b/t/open311/getservicerequests.t @@ -273,6 +273,47 @@ for my $test ( }; } +subtest 'check fetch_all body setting ignores date errors' => sub { + my $xml = prepare_xml({ id => '12345678' }); + + $body->update( { + send_method => 'Open311', + fetch_problems => 1, + comment_user_id => $user->id, + endpoint => 'http://open311.localhost/', + api_key => 'KEY', + jurisdiction => 'test', + } ); + $body->set_extra_metadata( fetch_all_problems => 1 ); + $body->update(); + + my $update = Open311::GetServiceRequests->new( + verbose => 1, + system_user => $user, + ); + $update = Test::MockObject::Extends->new($update); + + $update->mock('create_open311_object', sub { + return Open311->new( + jurisdiction => 'mysociety', + endpoint => 'http://example.com', + test_mode => 1, + test_get_returns => { 'requests.xml' => $xml} + ); + } ); + + my $count = FixMyStreet::DB->resultset('Problem')->count; + FixMyStreet::override_config { + MAPIT_URL => 'http://mapit.uk/', + }, sub { + $update->fetch; + }; + + my $after = FixMyStreet::DB->resultset('Problem')->count; + + is $after, $count + 1, 'problem created'; +}; + for my $test ( { desc => 'convert easting/northing to lat/long', diff --git a/templates/web/base/admin/open311-form-fields.html b/templates/web/base/admin/open311-form-fields.html index 954c38b08..b716cf175 100644 --- a/templates/web/base/admin/open311-form-fields.html +++ b/templates/web/base/admin/open311-form-fields.html @@ -76,6 +76,52 @@ <label for="send_comments" class="inline">[% loc('Use Open311 update-sending extension') %]</label> </p> + <div class="admin-open311-section"> + <div class="admin-hint"> + <p> + [% loc( + "If you've enabled Open311 update-sending above, you must identify which + FixMyStreet <strong>user</strong> will be attributed as the creator of those updates + when they are shown on the site. Enter the ID (number) of that user." + ) %] + </p> + </div> + <p> + <label for"comment_user_id">[% loc('User ID to attribute fetched comments to') %]</label> + <input type="text" class="form-control" name="comment_user_id" value="[% object.comment_user_id %]"> + [% IF object.comment_user_id %] + <a href="[% c.uri_for('user_edit', object.comment_user_id) %]">[% loc('edit user') %]</a> + [% END %] + </p> + + <div class="admin-hint"> + <p> + [% loc( + "If you've enabled Open311 update-sending above, enable <strong>suppression of alerts</strong> + if you do <strong>not</strong> want that user to be notified whenever these updates are created." + ) %] + </p> + </div> + <p> + <input type="checkbox" id="suppress_alerts" name="suppress_alerts"[% ' checked' IF object.suppress_alerts %]> + <label for="suppress_alerts" class="inline">[% loc('Do not send email alerts on fetched comments to problem creator') %]</label> + </p> + + <div class="admin-hint"> + <p> + [% loc( + "If you've enabled Open311 update-sending above, Open311 usually only accepts OPEN or CLOSED status in + its updates. Enable <strong>extended Open311 stauses</strong> if you want to allow extra states to be passed. + Check that your cobrand supports this feature before switching it on." + ) %] + </p> + </div> + <p> + <input type="checkbox" id="send_extended_statuses" name="send_extended_statuses"[% ' checked' IF object.send_extended_statuses %]> + <label for="send_extended_statuses" class="inline">[% loc('Send extended Open311 statuses with service request updates') %]</label> + </p> + </div> + <div class="admin-hint"> <p> [% loc( @@ -91,61 +137,33 @@ <label for="fetch_problems" class="inline">[% loc('Use Open311 problem fetching') %]</label> </p> - <div class="admin-hint"> - <p> - [% loc( - "If you've enabled Open311 update-sending above, you must identify which - FixMyStreet <strong>user</strong> will be attributed as the creator of those updates - when they are shown on the site. Enter the ID (number) of that user." - ) %] - </p> - </div> - <p> - <label for"comment_user_id">[% loc('User ID to attribute fetched comments to') %]</label> - <input type="text" class="form-control" name="comment_user_id" value="[% object.comment_user_id %]"> - [% IF object.comment_user_id %] - <a href="[% c.uri_for('user_edit', object.comment_user_id) %]">[% loc('edit user') %]</a> - [% END %] - </p> + <div class="admin-open311-section"> + <div class="admin-hint"> + <p> + [% loc( + "Enable <strong>Convert location from Easting/Northing</strong> if you've enabled Open311 problem-fetching above + and problems fetching from the endpoint have the location in Easting/Northings and not Latitude/Longitude." + ) %] + </p> + </div> + <p> + <input type="checkbox" id="convert_latlong" name="convert_latlong"[% ' checked' IF object.convert_latlong %]> + <label for="convert_latlong" class="inline">[% loc('Convert location from Easting/Northing') %]</label> + </p> - <div class="admin-hint"> - <p> - [% loc( - "If you've enabled Open311 update-sending above, enable <strong>suppression of alerts</strong> - if you do <strong>not</strong> want that user to be notified whenever these updates are created." - ) %] - </p> + <div class="admin-hint"> + <p> + [% loc( + "Enable <strong>Always fetch all problems</strong> if you've enabled Open311 problem-fetching above + and the endpoint always returns a list of all problems. This will suppress error messages about + bad dates in the problems fetched." + ) %] + </p> + </div> + <p> + <input type="checkbox" id="fetch_all_problems" name="fetch_all_problems"[% ' checked' IF object.get_extra_metadata('fetch_all_problems') %]> + <label for="fetch_all_problems" class="inline">[% loc('Always fetch all problems') %]</label> + </p> </div> - <p> - <input type="checkbox" id="suppress_alerts" name="suppress_alerts"[% ' checked' IF object.suppress_alerts %]> - <label for="suppress_alerts" class="inline">[% loc('Do not send email alerts on fetched comments to problem creator') %]</label> - </p> - - <div class="admin-hint"> - <p> - [% loc( - "If you've enabled Open311 update-sending above, Open311 usually only accepts OPEN or CLOSED status in - its updates. Enable <strong>extended Open311 stauses</strong> if you want to allow extra states to be passed. - Check that your cobrand supports this feature before switching it on." - ) %] - </p> - </div> - <p> - <input type="checkbox" id="send_extended_statuses" name="send_extended_statuses"[% ' checked' IF object.send_extended_statuses %]> - <label for="send_extended_statuses" class="inline">[% loc('Send extended Open311 statuses with service request updates') %]</label> - </p> - - <div class="admin-hint"> - <p> - [% loc( - "Enable <strong>Convert location from Easting/Northing</strong> if you've enabled Open311 problem-fetching above - and problems fetching from the endpoint have the location in Easting/Northings and not Latitude/Longitude." - ) %] - </p> - </div> - <p> - <input type="checkbox" id="convert_latlong" name="convert_latlong"[% ' checked' IF object.convert_latlong %]> - <label for="convert_latlong" class="inline">[% loc('Convert location from Easting/Northing') %]</label> - </p> [% END %] </div> diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html index 4c285e330..8708b08de 100644 --- a/templates/web/base/report/_inspect.html +++ b/templates/web/base/report/_inspect.html @@ -16,6 +16,9 @@ <p> <strong>[% loc('Report ID:') %]</strong> <span class="js-report-id">[% problem.id %]</span> + [% IF c.user_exists AND c.cobrand.admin_allow_user(c.user) AND c.user.has_permission_to('report_edit', problem.bodies_str_ids) %] + (<a href="[% c.uri_for_action( "admin/report_edit", problem.id ) %]">[% loc('admin') %]</a>) + [% END %] </p> [% IF permissions.report_inspect AND problem.user.phone %] <p> diff --git a/templates/web/bathnes/header_extra.html b/templates/web/bathnes/header_extra.html new file mode 100644 index 000000000..8a977495f --- /dev/null +++ b/templates/web/bathnes/header_extra.html @@ -0,0 +1 @@ +[% INCLUDE 'tracking_code.html' %] diff --git a/templates/web/bathnes/tracking_code.html b/templates/web/bathnes/tracking_code.html new file mode 100644 index 000000000..e6f20b90f --- /dev/null +++ b/templates/web/bathnes/tracking_code.html @@ -0,0 +1,12 @@ +[% IF c.config.BASE_URL == "https://www.fixmystreet.com" %] +<script async src="https://www.googletagmanager.com/gtag/js?id=UA-418184-9"></script> +<script> + window.dataLayer = window.dataLayer || []; + function gtag(){dataLayer.push(arguments);} + gtag('js', new Date()); + + gtag('config', 'UA-418184-9'); +</script> +[% ELSE %] +<!-- Tracking code not inserted as "[% c.config.BASE_URL %]" not "https://www.fixmystreet.com" --> +[% END %] diff --git a/web/cobrands/sass/_admin.scss b/web/cobrands/sass/_admin.scss index 11536882b..bf96dd08f 100644 --- a/web/cobrands/sass/_admin.scss +++ b/web/cobrands/sass/_admin.scss @@ -111,6 +111,11 @@ $button_bg_col: #a1a1a1; // also search bar (tables) margin: 1em 0; } +.admin-open311-section { + padding-left: 1em; + border-left: 1px solid #ccc; +} + .admin-hint { font-size: 80%; // little question marks are small cursor: pointer; |