diff options
-rw-r--r-- | CHANGELOG.md | 5 | ||||
-rw-r--r-- | cpanfile | 1 | ||||
-rw-r--r-- | cpanfile.snapshot | 9 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Location.pm | 25 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/Update.pm | 7 | ||||
-rw-r--r-- | perllib/FixMyStreet/Script/Alerts.pm | 4 | ||||
-rw-r--r-- | perllib/FixMyStreet/Script/Inactive.pm | 6 | ||||
-rw-r--r-- | perllib/Open311/PopulateServiceList.pm | 6 | ||||
-rw-r--r-- | t/Mock/Nominatim.pm | 5 | ||||
-rw-r--r-- | t/app/controller/alert_new.t | 2 | ||||
-rw-r--r-- | t/app/controller/around.t | 120 | ||||
-rw-r--r-- | t/open311/populate-service-list.t | 64 | ||||
-rwxr-xr-x | templates/web/fixmystreet.com/about/privacy.html | 6 |
13 files changed, 170 insertions, 90 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 6830bfbb5..5c25f3590 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,15 @@ ## Releases * Unreleased + - New features: + - Add Open Location Codes support to search box. #3047 + - Changes: + - Mark user as active when sent an email alert. - Bugfixes: - Fix issue with dashboard report CSV export. #3026 - bin/update-schema PostgreSQL 12 compatibility. #3043 - Make sure category shown in all its groups when reporting. + - Do not remove any devolved contacts. - Admin improvements: - Display user name/email for contributed as reports. #2990 - Interface for enabling anonymous reports for certain categories. #2989 @@ -80,6 +80,7 @@ requires 'Error'; requires 'FCGI'; # Required by e.g. Plack::Handler::FCGI requires 'File::Find'; requires 'File::Path'; +requires 'Geo::OLC'; requires 'Geography::NationalGrid', mirror => 'https://cpan.metacpan.org/'; requires 'Getopt::Long::Descriptive', '0.105'; diff --git a/cpanfile.snapshot b/cpanfile.snapshot index 53ce1f120..edc125960 100644 --- a/cpanfile.snapshot +++ b/cpanfile.snapshot @@ -3116,6 +3116,15 @@ DISTRIBUTIONS perl 5.014000 strict 0 warnings 0 + Geo-OLC-1 + pathname: J/JG/JGREELY/Geo-OLC-1.tar.gz + provides: + Geo::OLC 1 + requirements: + ExtUtils::MakeMaker 0 + List::Util 1 + Test::More 0 + perl 5.010001 Geography-NationalGrid-1.6 pathname: P/PK/PKENT/Geography-NationalGrid-1.6.tar.gz provides: diff --git a/perllib/FixMyStreet/App/Controller/Location.pm b/perllib/FixMyStreet/App/Controller/Location.pm index 416fb942a..3869ef8d3 100644 --- a/perllib/FixMyStreet/App/Controller/Location.pm +++ b/perllib/FixMyStreet/App/Controller/Location.pm @@ -6,6 +6,7 @@ BEGIN {extends 'Catalyst::Controller'; } use Encode; use FixMyStreet::Geocode; +use Geo::OLC; use Try::Tiny; use Utils; @@ -74,6 +75,30 @@ sub determine_location_from_pc : Private { map { Utils::truncate_coordinate($_) } ($1, $2); return $c->forward( 'check_location' ); } + + if (Geo::OLC::is_full($pc)) { + my $ref = Geo::OLC::decode($pc); + ($c->stash->{latitude}, $c->stash->{longitude}) = + map { Utils::truncate_coordinate($_) } @{$ref->{center}}; + return $c->forward( 'check_location' ); + } + + if ($pc =~ /^\s*([2-9CFGHJMPQRVWX]{4,6}\+[2-9CFGHJMPQRVWX]{2,3})\s+(.*)$/i) { + my ($code, $rest) = ($1, $2); + my ($lat, $lon, $error) = FixMyStreet::Geocode::lookup($rest, $c); + if (ref($error) eq 'ARRAY') { # Just take the first result + $lat = $error->[0]{latitude}; + $lon = $error->[0]{longitude}; + } + if (defined $lat && defined $lon) { + $code = Geo::OLC::recover_nearest($code, $lat, $lon); + my $ref = Geo::OLC::decode($code); + ($c->stash->{latitude}, $c->stash->{longitude}) = + map { Utils::truncate_coordinate($_) } @{$ref->{center}}; + return $c->forward( 'check_location' ); + } + } + if ( $c->cobrand->country eq 'GB' && $pc =~ /^([A-Z])([A-Z])([\d\s]{4,})$/i) { if (my $convert = gridref_to_latlon( $1, $2, $3 )) { ($c->stash->{latitude}, $c->stash->{longitude}) = diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm index c5d20a5da..8ffba3dcf 100644 --- a/perllib/FixMyStreet/App/Controller/Report/Update.pm +++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm @@ -484,6 +484,13 @@ sub save_update : Private { $update->confirm(); } elsif ($c->stash->{contributing_as_anonymous_user}) { $update->set_extra_metadata( contributed_as => 'anonymous_user' ); + if ( $c->user_exists && $c->user->from_body ) { + # If a staff user has clicked the 'report anonymously' button then + # there would be no record of who that staff member was as we've + # used the cobrand's anonymous_account for the report. In this case + # record the staff user ID in the report metadata. + $update->set_extra_metadata( contributed_by => $c->user->id ); + } $update->confirm(); } elsif ( !$update->user->in_storage ) { # User does not exist. diff --git a/perllib/FixMyStreet/Script/Alerts.pm b/perllib/FixMyStreet/Script/Alerts.pm index cb1f022fa..d07728092 100644 --- a/perllib/FixMyStreet/Script/Alerts.pm +++ b/perllib/FixMyStreet/Script/Alerts.pm @@ -307,6 +307,10 @@ sub _send_aggregated_alert_email(%) { # Ignore phone-only users return unless $data{alert_user}->email_verified; + # Mark user as active as they're being sent an alert + $data{alert_user}->set_last_active; + $data{alert_user}->update; + my $email = $data{alert_user}->email; my ($domain) = $email =~ m{ @ (.*) \z }x; return if $data{schema}->resultset('Abuse')->search( { diff --git a/perllib/FixMyStreet/Script/Inactive.pm b/perllib/FixMyStreet/Script/Inactive.pm index 8dd524ce1..4d28057d4 100644 --- a/perllib/FixMyStreet/Script/Inactive.pm +++ b/perllib/FixMyStreet/Script/Inactive.pm @@ -158,8 +158,14 @@ sub delete_reports { sub anonymize_users { my $self = shift; + my $body_users = FixMyStreet::DB->resultset("Body")->search({ + comment_user_id => { '!=' => undef }, + }, { + columns => 'comment_user_id', + }); my $users = FixMyStreet::DB->resultset("User")->search({ last_active => { '<', interval($self->anonymize) }, + id => { -not_in => $body_users->as_query }, email => { -not_like => 'removed-%@' . FixMyStreet->config('EMAIL_DOMAIN') }, }); diff --git a/perllib/Open311/PopulateServiceList.pm b/perllib/Open311/PopulateServiceList.pm index 9be17946e..20fca90b3 100644 --- a/perllib/Open311/PopulateServiceList.pm +++ b/perllib/Open311/PopulateServiceList.pm @@ -350,10 +350,10 @@ sub _delete_contacts_not_in_service_list { ); if ($self->_current_body->can_be_devolved) { - # If the body has can_be_devolved switched on, it's most likely a - # combination of Open311/email, so ignore any email addresses. + # If the body has can_be_devolved switched on, ignore any + # contact with its own send method $found_contacts = $found_contacts->search( - { email => { -not_like => '%@%' } } + { send_method => [ "", undef ] }, ); } diff --git a/t/Mock/Nominatim.pm b/t/Mock/Nominatim.pm index 806ebbfd3..d108256f5 100644 --- a/t/Mock/Nominatim.pm +++ b/t/Mock/Nominatim.pm @@ -1,6 +1,7 @@ package t::Mock::Nominatim; use JSON::MaybeXS; +use LWP::Protocol::PSGI; use Web::Simple; has json => ( @@ -35,6 +36,10 @@ sub query { {"osm_type"=>"way","osm_id"=>"4684282","lat"=>"55.9504009","lon"=>"-3.1858425","display_name"=>"High Street, Old Town, City of Ed\x{ed}nburgh, Scotland, EH1 1SP, United Kingdom","class"=>"highway","type"=>"tertiary","importance"=>0.55892577838734}, {"osm_type"=>"node","osm_id"=>"27424410","lat"=>"55.8596449","lon"=>"-4.240377","display_name"=>"High Street, Collegelands, Merchant City, Glasgow, Glasgow City, Scotland, G, United Kingdom","class"=>"railway","type"=>"station","importance"=>0.53074299592768} ]; + } elsif ($q eq 'edinburgh') { + return [ + {"osm_type"=>"node","osm_id"=>"17898859","lat"=>"55.9533456","lon"=>"-3.1883749","display_name"=>"Edinburgh","class"=>"place","type"=>"place:city","importance"=>0.676704} + ]; } return []; } diff --git a/t/app/controller/alert_new.t b/t/app/controller/alert_new.t index 7eba90530..d968b56b1 100644 --- a/t/app/controller/alert_new.t +++ b/t/app/controller/alert_new.t @@ -523,6 +523,8 @@ subtest "Test alerts are not sent for no-text updates" => sub { }; $mech->email_count_is(1); + $user2->discard_changes; + isnt $user2->last_active, undef, 'Last active has been set'; $mech->delete_user($user1); $mech->delete_user($user2); diff --git a/t/app/controller/around.t b/t/app/controller/around.t index 6e49c6f29..784801517 100644 --- a/t/app/controller/around.t +++ b/t/app/controller/around.t @@ -9,6 +9,7 @@ use constant MIN_ZOOM_LEVEL => 88; package main; use Test::MockModule; +use t::Mock::Nominatim; use FixMyStreet::TestMech; my $mech = FixMyStreet::TestMech->new; @@ -53,6 +54,11 @@ foreach my $test ( }; } +FixMyStreet::override_config { + ALLOWED_COBRANDS => 'fixmystreet', + MAPIT_URL => 'http://mapit.uk/', +}, sub { + # check that exact queries result in the correct lat,lng foreach my $test ( { @@ -69,19 +75,45 @@ foreach my $test ( { subtest "check lat/lng for '$test->{pc}'" => sub { $mech->get_ok('/'); - FixMyStreet::override_config { - ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ], - MAPIT_URL => 'http://mapit.uk/', - }, sub { - $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } }, - "good location" ); - }; + $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } }, + "good location" ); is_deeply $mech->page_errors, [], "no errors for pc '$test->{pc}'"; is_deeply $mech->extract_location, $test, "got expected location for pc '$test->{pc}'"; + $mech->get_ok('/'); + my $pc = "$test->{latitude},$test->{longitude}"; + $mech->submit_form_ok( { with_fields => { pc => $pc } }, + "good location" ); + is_deeply $mech->page_errors, [], "no errors for pc '$pc'"; + is_deeply $mech->extract_location, { %$test, pc => $pc }, + "got expected location for pc '$pc'"; }; } +subtest "check lat/lng for full plus code" => sub { + $mech->get_ok('/'); + $mech->submit_form_ok( { with_fields => { pc => "9C7RXR26+R5" } } ); + is_deeply $mech->page_errors, [], "no errors for plus code"; + is_deeply $mech->extract_location, { + pc => "9C7RXR26+R5", + latitude => 55.952063, + longitude => -3.189562, + }, + "got expected location for full plus code"; +}; + +subtest "check lat/lng for short plus code" => sub { + $mech->get_ok('/'); + $mech->submit_form_ok( { with_fields => { pc => "XR26+R5 Edinburgh" } } ); + is_deeply $mech->page_errors, [], "no errors for plus code"; + is_deeply $mech->extract_location, { + pc => "XR26+R5 Edinburgh", + latitude => 55.952063, + longitude => -3.189562, + }, + "got expected location for short plus code"; +}; + my $body_edin_id = $mech->create_body_ok(2651, 'City of Edinburgh Council')->id; my $body_west_id = $mech->create_body_ok(2504, 'Westminster City Council')->id; @@ -93,10 +125,10 @@ my @edinburgh_problems = $mech->create_problems_for_body( 5, $body_edin_id, 'Aro subtest 'check lookup by reference' => sub { $mech->get_ok('/'); - $mech->submit_form_ok( { with_fields => { pc => 'ref:12345' } }, 'bad ref'); + $mech->submit_form_ok( { with_fields => { pc => '12345' } }, 'bad ref'); $mech->content_contains('Searching found no reports'); my $id = $edinburgh_problems[0]->id; - $mech->submit_form_ok( { with_fields => { pc => "ref:$id" } }, 'good ref'); + $mech->submit_form_ok( { with_fields => { pc => $id } }, 'good ref'); is $mech->uri->path, "/report/$id", "redirected to report page"; }; @@ -106,19 +138,14 @@ subtest 'check lookup by reference does not show non_public reports' => sub { }); my $id = $edinburgh_problems[0]->id; $mech->get_ok('/'); - $mech->submit_form_ok( { with_fields => { pc => "ref:$id" } }, 'non_public ref'); + $mech->submit_form_ok( { with_fields => { pc => $id } }, 'non_public ref'); $mech->content_contains('Searching found no reports'); }; subtest 'check non public reports are not displayed on around page' => sub { $mech->get_ok('/'); - FixMyStreet::override_config { - ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ], - MAPIT_URL => 'http://mapit.uk/', - }, sub { - $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, - "good location" ); - }; + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, + "good location" ); $mech->content_contains( "Around page Test 3 for $body_edin_id", 'problem to be marked non public visible' ); @@ -126,26 +153,16 @@ subtest 'check non public reports are not displayed on around page' => sub { ok $private->update( { non_public => 1 } ), 'problem marked non public'; $mech->get_ok('/'); - FixMyStreet::override_config { - ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ], - MAPIT_URL => 'http://mapit.uk/', - }, sub { - $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, - "good location" ); - }; + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, + "good location" ); $mech->content_lacks( "Around page Test 3 for $body_edin_id", 'problem marked non public is not visible' ); }; subtest 'check missing body message not shown when it does not need to be' => sub { $mech->get_ok('/'); - FixMyStreet::override_config { - ALLOWED_COBRANDS => 'fixmystreet', - MAPIT_URL => 'http://mapit.uk/', - }, sub { - $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, - "good location" ); - }; + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, + "good location" ); $mech->content_lacks('yet have details for the other councils that cover this location'); }; @@ -162,24 +179,14 @@ for my $permission ( qw/ report_inspect report_mark_private/ ) { }); $mech->get_ok('/'); - FixMyStreet::override_config { - ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ], - MAPIT_URL => 'http://mapit.uk/', - }, sub { - $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, - "good location" ); - }; + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, + "good location" ); $mech->content_contains( "Around page Test 3 for $body_edin_id", 'problem marked non public is visible' ); $mech->content_contains( "Around page Test 2 for $body_edin_id", 'problem marked public is visible' ); - FixMyStreet::override_config { - ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ], - MAPIT_URL => 'http://mapit.uk/', - }, sub { - $mech->get_ok('/around?pc=EH1+1BB&status=non_public'); - }; + $mech->get_ok('/around?pc=EH1+1BB&status=non_public'); $mech->content_contains( "Around page Test 3 for $body_edin_id", 'problem marked non public is visible' ); $mech->content_lacks( "Around page Test 2 for $body_edin_id", @@ -193,24 +200,14 @@ for my $permission ( qw/ report_inspect report_mark_private/ ) { }); $mech->get_ok('/'); - FixMyStreet::override_config { - ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ], - MAPIT_URL => 'http://mapit.uk/', - }, sub { - $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, - "good location" ); - }; + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, + "good location" ); $mech->content_lacks( "Around page Test 3 for $body_edin_id", 'problem marked non public is not visible' ); $mech->content_contains( "Around page Test 2 for $body_edin_id", 'problem marked public is visible' ); - FixMyStreet::override_config { - ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ], - MAPIT_URL => 'http://mapit.uk/', - }, sub { - $mech->get_ok('/around?pc=EH1+1BB&status=non_public'); - }; + $mech->get_ok('/around?pc=EH1+1BB&status=non_public'); $mech->content_lacks( "Around page Test 3 for $body_edin_id", 'problem marked non public is not visible' ); $mech->content_lacks( "Around page Test 2 for $body_edin_id", @@ -230,17 +227,14 @@ subtest 'check assigned-only list items do not display shortlist buttons' => sub $user->update({ from_body => $body }); $user->user_body_permissions->find_or_create({ body => $body, permission_type => 'planned_reports' }); - FixMyStreet::override_config { - ALLOWED_COBRANDS => 'fixmystreet', - MAPIT_URL => 'http://mapit.uk/', - }, sub { - $mech->get_ok('/around?pc=EH1+1BB'); - }; + $mech->get_ok('/around?pc=EH1+1BB'); $mech->content_contains('shortlist-add-' . $edinburgh_problems[4]->id); $mech->content_lacks('shortlist-add-' . $edinburgh_problems[3]->id); $mech->content_lacks('shortlist-add-' . $edinburgh_problems[1]->id); }; +}; # End big override_config + my $body = $mech->create_body_ok(2237, "Oxfordshire"); subtest 'check category, status and extra filtering works on /around' => sub { diff --git a/t/open311/populate-service-list.t b/t/open311/populate-service-list.t index bd837f203..20f092da4 100644 --- a/t/open311/populate-service-list.t +++ b/t/open311/populate-service-list.t @@ -175,33 +175,53 @@ subtest "set multiple groups with groups element" => sub { is_deeply $contact->get_extra->{group}, ['sanitation & cleaning','street'], "groups set correctly"; }; -subtest 'check non open311 contacts marked as deleted' => sub { - FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id } )->delete(); +$body->update({ can_be_devolved => 1 }); +for my $test ( + { + test => 'check non open311 contacts marked as deleted', + contact_params => { + email => 'contact@example.com', + }, + deleted => 1, + }, + { + test => 'check devolved non open311 contacts not marked as deleted', + contact_params => { + email => 'contact', + send_method => 'Open311', + }, + deleted => 0, + }, +) { + subtest $test->{test} => sub { + FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id } )->delete(); - my $contact = FixMyStreet::DB->resultset('Contact')->create( - { - body_id => $body->id, - email => 'contact@example.com', - category => 'An old category', - state => 'confirmed', - editor => $0, - whenedited => \'current_timestamp', - note => 'test contact', - } - ); + my $contact = FixMyStreet::DB->resultset('Contact')->create( + { + body_id => $body->id, + category => 'An old category', + state => 'confirmed', + editor => $0, + whenedited => \'current_timestamp', + note => 'test contact', + %{$test->{contact_params}}, + } + ); - my $service_list = get_xml_simple_object( get_standard_xml() ); + my $service_list = get_xml_simple_object( get_standard_xml() ); - my $processor = Open311::PopulateServiceList->new(); - $processor->_current_body( $body ); - $processor->process_services( $service_list ); + my $processor = Open311::PopulateServiceList->new(); + $processor->_current_body( $body ); + $processor->process_services( $service_list ); - my $contact_count = FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id } )->count(); - is $contact_count, 4, 'correct number of contacts'; + my $contact_count = FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id } )->count(); + is $contact_count, 4, 'correct number of contacts'; - $contact_count = FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id, state => 'deleted' } )->count(); - is $contact_count, 1, 'correct number of deleted contacts'; -}; + $contact_count = FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id, state => 'deleted' } )->count(); + is $contact_count, $test->{deleted}, 'correct number of deleted contacts'; + }; +} +$body->update({ can_be_devolved => 0 }); subtest 'check email changed if matching category' => sub { FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id } )->delete(); diff --git a/templates/web/fixmystreet.com/about/privacy.html b/templates/web/fixmystreet.com/about/privacy.html index 8ed953cc9..665b0968b 100755 --- a/templates/web/fixmystreet.com/about/privacy.html +++ b/templates/web/fixmystreet.com/about/privacy.html @@ -287,8 +287,10 @@ When you make a report </p> <p> + Personal details will automatically be removed from our database after two + years of inactivity of the associated account. Please <a href="/contact">contact us</a> if you would like your details to be removed from our admin - database. + database sooner than that. </p> <h3> @@ -297,7 +299,7 @@ When you make a report <p> If you contact FixMyStreet via our support email address we keep your message for two - years at which point they will be automatically deleted.. This is to aid continuity + years at which point it will be automatically deleted. This is to aid continuity and so that we can view any historic context which may have bearing on subsequent support mail, even if members of the support staff change. Support staff adhere to internal privacy policies which may be viewed on request. |