diff options
author | Matthew Somerville <matthew-github@dracos.co.uk> | 2018-06-26 18:18:56 +0100 |
---|---|---|
committer | Matthew Somerville <matthew@mysociety.org> | 2018-06-28 13:35:23 +0100 |
commit | dadddc42f5e5c4b3a02db9171a485885b1e365a7 (patch) | |
tree | 603a745179c5b141211ccb6355fcbeaf7ecaeb95 | |
parent | 53f3af1fd561d852346902a06ef27c110d0bd2c1 (diff) |
[UK] Fix issue when body ID not equal to MapIt ID.
Hitherto when creating a body or ward alert on a UK site,
the MapIt area ID has been stored instead of the body ID.
This is okay for www.fixmystreet.com which for historical
reasons does have body IDs that match MapIt area IDs, but
other UK-based sites may well not. The alert lookup looks
for body ID, meaning those alerts will not work. Save the
body ID instead, plus fix some tests that were making the
same assumption.
-rw-r--r-- | CHANGELOG.md | 1 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/FiksGataMi.pm | 11 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/UK.pm | 13 | ||||
-rw-r--r-- | perllib/FixMyStreet/Script/Alerts.pm | 5 | ||||
-rw-r--r-- | perllib/FixMyStreet/TestMech.pm | 7 | ||||
-rw-r--r-- | t/Mock/MapIt.pm | 3 | ||||
-rw-r--r-- | t/app/controller/alert.t | 17 | ||||
-rw-r--r-- | t/app/controller/alert_new.t | 24 | ||||
-rw-r--r-- | t/app/model/alert_type.t | 8 | ||||
-rw-r--r-- | t/cobrand/fiksgatami.t | 30 |
10 files changed, 88 insertions, 31 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index e5e142212..7a6784c63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - Fix issue with category filter when category contains comma #2166 - Inspectors can unset priority. #2171 - Defect type is recorded if category change made. #2172 + - [UK] Store body ID on council/ward alerts. #2175 - Admin improvements: - Mandatory defect type selection if defect raised. - Send login email button on user edit page #2041 diff --git a/perllib/FixMyStreet/Cobrand/FiksGataMi.pm b/perllib/FixMyStreet/Cobrand/FiksGataMi.pm index 306ad358a..74af9cb7d 100644 --- a/perllib/FixMyStreet/Cobrand/FiksGataMi.pm +++ b/perllib/FixMyStreet/Cobrand/FiksGataMi.pm @@ -125,6 +125,9 @@ sub council_rss_alert_options { } } + my $body_kommune = FixMyStreet::DB->resultset('Body')->for_areas($kommune->{id})->first; + my $body_fylke = FixMyStreet::DB->resultset('Body')->for_areas($fylke->{id})->first; + if ( $fylke->{id} == 3 ) { # Oslo my $short_name = $self->short_name($fylke, $all_councils); ( my $id_name = $short_name ) =~ tr/+/_/; @@ -132,7 +135,7 @@ sub council_rss_alert_options { push @options, { type => 'council', - id => sprintf( 'council:%s:%s', $fylke->{id}, $id_name ), + id => sprintf( 'council:%s:%s', $body_fylke->id, $id_name ), rss_text => sprintf( _('RSS feed of problems within %s'), $fylke->{name} ), text => sprintf( _('Problems within %s'), $fylke->{name} ), @@ -167,7 +170,7 @@ sub council_rss_alert_options { push @reported_to_options, { type => 'council', - id => sprintf( 'council:%s:%s', $kommune->{id}, $id_kommune_name ), + id => sprintf( 'council:%s:%s', $body_kommune->id, $id_kommune_name ), rss_text => sprintf( _('RSS feed of %s'), $kommune->{name} ), text => $kommune->{name}, @@ -175,11 +178,11 @@ sub council_rss_alert_options { }, { type => 'council', - id => sprintf( 'council:%s:%s', $fylke->{id}, $id_fylke_name ), + id => sprintf( 'council:%s:%s', $body_fylke->id, $id_fylke_name ), rss_text => sprintf( _('RSS feed of %s'), $fylke->{name} ), text => $fylke->{name}, - uri => $c->uri_for( '/rss/reports/', $short_fylke_name ), + uri => $c->uri_for( '/rss/reports', $short_fylke_name ), }; } diff --git a/perllib/FixMyStreet/Cobrand/UK.pm b/perllib/FixMyStreet/Cobrand/UK.pm index f7b71b0ae..ebedf8711 100644 --- a/perllib/FixMyStreet/Cobrand/UK.pm +++ b/perllib/FixMyStreet/Cobrand/UK.pm @@ -190,9 +190,11 @@ sub council_rss_alert_options { my ( @options, @reported_to_options ); if ( $num_councils == 1 or $num_councils == 2 ) { my ($council, $ward); + my $body = FixMyStreet::DB->resultset('Body')->active->search({ name => { '!=' => 'TfL' } })->for_areas(keys %$all_areas)->first; foreach (values %$all_areas) { if ($councils{$_->{type}}) { $council = $_; + $council->{id} = $body->id; # Want to use body ID, not MapIt area ID $council->{short_name} = $self->short_name( $council ); ( $council->{id_name} = $council->{short_name} ) =~ tr/+/_/; } else { @@ -247,6 +249,9 @@ sub council_rss_alert_options { my $county_name = $county->{name}; my $c_ward_name = $c_ward->{name}; + my $body_dis = FixMyStreet::DB->resultset('Body')->active->for_areas($district->{id})->first; + my $body_cty = FixMyStreet::DB->resultset('Body')->active->for_areas($county->{id})->first; + push @options, { type => 'area', id => sprintf( 'area:%s:%s', $district->{id}, $district->{id_name} ), @@ -275,25 +280,25 @@ sub council_rss_alert_options { push @reported_to_options, { type => 'council', - id => sprintf( 'council:%s:%s', $district->{id}, $district->{id_name} ), + id => sprintf( 'council:%s:%s', $body_dis->id, $district->{id_name} ), text => sprintf( _('Reports sent to %s'), $district->{name} ), rss_text => sprintf( _('RSS feed of %s'), $district->{name}), uri => $c->uri_for( '/rss/reports/' . $district->{short_name} ), }, { type => 'ward', - id => sprintf( 'ward:%s:%s:%s:%s', $district->{id}, $d_ward->{id}, $district->{id_name}, $d_ward->{id_name} ), + id => sprintf( 'ward:%s:%s:%s:%s', $body_dis->id, $d_ward->{id}, $district->{id_name}, $d_ward->{id_name} ), rss_text => sprintf( _('RSS feed of %s, within %s ward'), $district->{name}, $d_ward->{name}), text => sprintf( _('Reports sent to %s, within %s ward'), $district->{name}, $d_ward->{name}), uri => $c->uri_for( '/rss/reports/' . $district->{short_name} . '/' . $d_ward->{short_name} ), }, { type => 'council', - id => sprintf( 'council:%s:%s', $county->{id}, $county->{id_name} ), + id => sprintf( 'council:%s:%s', $body_cty->id, $county->{id_name} ), text => sprintf( _('Reports sent to %s'), $county->{name} ), rss_text => sprintf( _('RSS feed of %s'), $county->{name}), uri => $c->uri_for( '/rss/reports/' . $county->{short_name} ), }, { type => 'ward', - id => sprintf( 'ward:%s:%s:%s:%s', $county->{id}, $c_ward->{id}, $county->{id_name}, $c_ward->{id_name} ), + id => sprintf( 'ward:%s:%s:%s:%s', $body_cty->id, $c_ward->{id}, $county->{id_name}, $c_ward->{id_name} ), rss_text => sprintf( _('RSS feed of %s, within %s ward'), $county->{name}, $c_ward->{name}), text => sprintf( _('Reports sent to %s, within %s ward'), $county->{name}, $c_ward->{name}), uri => $c->uri_for( '/rss/reports/' . $county->{short_name} . '/' . $c_ward->{short_name} ), diff --git a/perllib/FixMyStreet/Script/Alerts.pm b/perllib/FixMyStreet/Script/Alerts.pm index 4b5641f9e..a0c25a8fc 100644 --- a/perllib/FixMyStreet/Script/Alerts.pm +++ b/perllib/FixMyStreet/Script/Alerts.pm @@ -185,9 +185,12 @@ sub send() { # Get a report object for its photo and static map $data{report} = $schema->resultset('Problem')->find({ id => $row->{id} }); } - if ($ref eq 'area_problems' || $ref eq 'council_problems' || $ref eq 'ward_problems') { + if ($ref eq 'area_problems') { my $va_info = mySociety::MaPit::call('area', $row->{alert_parameter}); $data{area_name} = $va_info->{name}; + } elsif ($ref eq 'council_problems' || $ref eq 'ward_problems') { + my $body = FixMyStreet::DB->resultset('Body')->find({ id => $row->{alert_parameter} }); + $data{area_name} = $body->name; } if ($ref eq 'ward_problems') { my $va_info = mySociety::MaPit::call('area', $row->{alert_parameter2}); diff --git a/perllib/FixMyStreet/TestMech.pm b/perllib/FixMyStreet/TestMech.pm index c5b72a7cf..e7fc573c0 100644 --- a/perllib/FixMyStreet/TestMech.pm +++ b/perllib/FixMyStreet/TestMech.pm @@ -10,6 +10,7 @@ sub import { Test::More->export_to_level(1); } +use Encode; use Test::WWW::Mechanize::Catalyst 'FixMyStreet::App'; use t::Mock::MapIt; use Test::More; @@ -743,4 +744,10 @@ sub create_comment_for_problem { FixMyStreet::App->model('DB::Comment')->create($params); } + +sub encoded_content { + my $self = shift; + return encode_utf8($self->content); +} + 1; diff --git a/t/Mock/MapIt.pm b/t/Mock/MapIt.pm index c57f7e0ed..8cb5d777f 100644 --- a/t/Mock/MapIt.pm +++ b/t/Mock/MapIt.pm @@ -37,6 +37,9 @@ my @PLACES = ( [ 'WS1 4NH', 52.563074, -1.991032, 2535, 'Sandwell Borough Council', 'MTD' ], [ 'OX28 4DS', 51.784721, -1.494453 ], [ 'E14 2DN', 51.508536, '0.000001' ], + # Norway + [ '3290', 59, 10, 709, 'Larvik', 'NKO', 7, 'Vestfold', 'NFY' ], + [ '0045', "59.9", "10.9", 301, 'Oslo', 'NKO', 3, 'Oslo', 'NFY' ], ); sub dispatch_request { diff --git a/t/app/controller/alert.t b/t/app/controller/alert.t index 57e73e5ec..41aee5bbc 100644 --- a/t/app/controller/alert.t +++ b/t/app/controller/alert.t @@ -9,6 +9,11 @@ $mech->title_like(qr/^Local RSS feeds and email alerts/); $mech->content_contains('Local RSS feeds and email alerts'); $mech->content_contains('html class="no-js" lang="en-gb"'); +my $body = $mech->create_body_ok(2651, 'Edinburgh'); +$mech->create_body_ok(2504, 'Birmingham City Council'); +$mech->create_body_ok(2226, 'Gloucestershire County Council'); +$mech->create_body_ok(2326, 'Cheltenham Borough Council'); + # check that we can get list page FixMyStreet::override_config { ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ], @@ -33,8 +38,8 @@ FixMyStreet::override_config { $mech->content_contains('Problems within City Centre ward'); $mech->content_contains('/rss/reports/Edinburgh'); $mech->content_contains('/rss/reports/Edinburgh/City+Centre'); - $mech->content_contains('council:2651:Edinburgh'); - $mech->content_contains('ward:2651:20728:Edinburgh:City_Centre'); + $mech->content_contains('council:' . $body->id . ':Edinburgh'); + $mech->content_contains('ward:' . $body->id . ':20728:Edinburgh:City_Centre'); subtest "Test Nominatim lookup" => sub { $mech->get_ok('/alert/list?pc=High Street'); @@ -49,16 +54,12 @@ FixMyStreet::override_config { $mech->content_contains('Problems in an area'); $mech->content_contains('Reports by destination'); - $mech->get_ok('/alert/subscribe?rss=1&type=local&pc=ky16+8yg&rss=Give+me+an+RSS+feed&rznvy=' ); + $mech->get_ok('/alert/subscribe?rss=1&type=local&pc=EH1+1BB&rss=Give+me+an+RSS+feed&rznvy=' ); $mech->content_contains('Please select the feed you want'); - $mech->get_ok('/alert/subscribe?rss=1&feed=invalid:1000:A_Locationtype=local&pc=ky16+8yg&rss=Give+me+an+RSS+feed&rznvy='); + $mech->get_ok('/alert/subscribe?rss=1&feed=invalid:1000:A_Locationtype=local&pc=EH1+1BB&rss=Give+me+an+RSS+feed&rznvy='); $mech->content_contains('Illegal feed selection'); - $mech->create_body_ok(2504, 'Birmingham City Council'); - $mech->create_body_ok(2226, 'Gloucestershire County Council'); - $mech->create_body_ok(2326, 'Cheltenham Borough Council'); - $mech->get_ok('/alert/subscribe?rss=1&feed=area:1000:Birmingham'); is $mech->uri->path, '/rss/reports/Birmingham'; diff --git a/t/app/controller/alert_new.t b/t/app/controller/alert_new.t index 27371e4a9..49efc4efa 100644 --- a/t/app/controller/alert_new.t +++ b/t/app/controller/alert_new.t @@ -184,13 +184,15 @@ foreach my $test ( }; } +my $body = $mech->create_body_ok(2651, 'Edinburgh Council'); + foreach my $test ( { desc => 'logged in user signing up', email => 'test-sign-in@example.com', type => 'council', - param1 => 2651, - param2 => 2651, + param1 => $body->id, + param2 => $body->id, confirmed => 1, } ) @@ -207,9 +209,9 @@ foreach my $test ( ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ], MAPIT_URL => 'http://mapit.uk/', }, sub { - $mech->get_ok('/alert/list?pc=EH991SP'); + $mech->get_ok('/alert/list?pc=EH11BB'); }; - $mech->set_visible( [ radio => 'council:2651:City_of_Edinburgh' ] ); + $mech->set_visible( [ radio => 'council:' . $body->id . ':City_of_Edinburgh' ] ); $mech->click('alert'); my $alert = FixMyStreet::App->model('DB::Alert')->find( @@ -789,7 +791,7 @@ subtest "Test signature template is used from cobrand" => sub { my $report_time = '2011-03-01 12:00:00'; my $report = FixMyStreet::App->model('DB::Problem')->find_or_create( { postcode => 'EH1 1BB', - bodies_str => '2651', + bodies_str => $body->id, areas => ',11808,135007,14419,134935,2651,20728,', category => 'Street lighting', title => 'Testing', @@ -884,15 +886,15 @@ for my $test ( desc => 'check non public reports are not included in council problems alerts', alert_params => { alert_type => 'council_problems', - parameter => '2651', - parameter2 => '2651', + parameter => $body->id, + parameter2 => $body->id, } }, { desc => 'check non public reports are not included in ward problems alerts', alert_params => { alert_type => 'ward_problems', - parameter => '2651', + parameter => $body->id, parameter2 => '20728', } }, @@ -936,7 +938,7 @@ for my $test ( my $report = FixMyStreet::App->model('DB::Problem')->find_or_create( { postcode => 'EH1 1BB', - bodies_str => '2651', + bodies_str => $body->id, areas => ',11808,135007,14419,134935,2651,20728,', category => 'Street lighting', title => 'Alert test for non public reports', @@ -998,7 +1000,7 @@ subtest 'check new updates alerts for non public reports only go to report owner my $report = FixMyStreet::App->model('DB::Problem')->find_or_create( { postcode => 'EH1 1BB', - bodies_str => '2651', + bodies_str => $body->id, areas => ',11808,135007,14419,134935,2651,20728,', category => 'Street lighting', title => 'Alert test for non public reports', @@ -1091,7 +1093,7 @@ subtest 'check setting inlude dates in new updates cobrand option' => sub { my $report = FixMyStreet::App->model('DB::Problem')->find_or_create( { postcode => 'EH1 1BB', - bodies_str => '2651', + bodies_str => $body->id, areas => ',11808,135007,14419,134935,2651,20728,', category => 'Street lighting', title => 'Alert test for non public reports', diff --git a/t/app/model/alert_type.t b/t/app/model/alert_type.t index c978b5ccf..124e838ee 100644 --- a/t/app/model/alert_type.t +++ b/t/app/model/alert_type.t @@ -22,6 +22,8 @@ my $user3 = ->find_or_create( { email => 'bystander@example.com', name => 'Bystander' } ); ok $user3, "created bystander"; +my $body = $mech->create_body_ok(2504, 'Westminster'); + my $dt = DateTime->new( year => 2011, month => 04, @@ -34,7 +36,7 @@ my $dt = DateTime->new( my $report = FixMyStreet::DB->resultset('Problem')->find_or_create( { postcode => 'SW1A 1AA', - bodies_str => '2504', + bodies_str => $body->id, areas => ',105255,11806,11828,2247,2504,', category => 'Other', title => 'Test 2', @@ -165,8 +167,8 @@ $report->update(); my $council_alert = FixMyStreet::DB->resultset('Alert')->find_or_create( { user => $user2, - parameter => 2504, - parameter2 => 2504, + parameter => $body->id, + parameter2 => $body->id, alert_type => 'council_problems', whensubscribed => $dt->ymd . ' ' . $dt->hms, confirmed => 1, diff --git a/t/cobrand/fiksgatami.t b/t/cobrand/fiksgatami.t new file mode 100644 index 000000000..6510f5ebe --- /dev/null +++ b/t/cobrand/fiksgatami.t @@ -0,0 +1,30 @@ +use FixMyStreet::TestMech; +my $mech = FixMyStreet::TestMech->new; + +my $oslo = $mech->create_body_ok(3, 'Oslo'); +my $vestfold = $mech->create_body_ok(7, 'Vestfold'); +my $larvik = $mech->create_body_ok(709, 'Larvik'); + +FixMyStreet::override_config { + ALLOWED_COBRANDS => 'fiksgatami', + MAPIT_URL => 'http://mapit.uk/', + GEOCODER => '', +}, sub { + $mech->get_ok('/alert/list?pc=0045'); + $mech->content_contains('rss/l/59.9,10.9/2'); + $mech->content_contains('/rss/reports/Oslo'); + $mech->content_contains('council:' . $oslo->id . ':Oslo'); + + $mech->get_ok('/alert/list?pc=3290'); + $mech->content_contains('rss/l/59,10/5'); + $mech->content_contains('/rss/area/Larvik'); + $mech->content_contains('/rss/area/Vestfold'); + $mech->content_contains('/rss/reports/Larvik'); + $mech->content_contains('/rss/reports/Vestfold'); + $mech->content_contains('area:7:Vestfold'); + $mech->content_contains('area:709:Larvik'); + $mech->content_contains('council:' . $vestfold->id . ':Vestfold'); + $mech->content_contains('council:' . $larvik->id . ':Larvik'); +}; + +done_testing(); |