diff --git a/perllib/FixMyStreet/App/Controller/Alert.pm b/perllib/FixMyStreet/App/Controller/Alert.pm index 0e1b205a128..d7db6e40330 100644 --- a/perllib/FixMyStreet/App/Controller/Alert.pm +++ b/perllib/FixMyStreet/App/Controller/Alert.pm @@ -88,9 +88,8 @@ sub rss : Private { } my $url; - if ( $feed =~ /^area:(?:\d+:)+(.*)$/ ) { - ( my $id = $1 ) =~ tr{:_}{/+}; - $url = $c->cobrand->base_url() . '/rss/area/' . $id; + if ( $feed =~ /^area:(\d+)/ ) { + $url = $c->cobrand->base_url() . '/rss/area/' . $1; $c->res->redirect($url); } elsif ( $feed =~ /^(?:council|ward):(?:\d+:)+(.*)$/ ) { @@ -246,7 +245,7 @@ sub set_local_alert_options : Private { my $feed = $c->get_param('feed'); my ( $type, @params, $alert ); - if ( $feed =~ /^area:(?:\d+:)?(\d+)/ ) { + if ( $feed =~ /^area:(\d+)/ ) { $type = 'area_problems'; push @params, $1; } diff --git a/perllib/FixMyStreet/App/Controller/Reports.pm b/perllib/FixMyStreet/App/Controller/Reports.pm index 4bc103fb8a3..c4ed5316f4e 100644 --- a/perllib/FixMyStreet/App/Controller/Reports.pm +++ b/perllib/FixMyStreet/App/Controller/Reports.pm @@ -254,11 +254,21 @@ sub rss_area_ward : Path('/rss/area') : Args(2) { $c->stash->{rss} = 1; - # area_check - $area =~ s/\+/ /g; $area =~ s/\.html//; + if ($area =~ /^[0-9]+$/) { + my $area = FixMyStreet::MapIt::call( 'area', $area ); + $c->detach( 'redirect_index' ) unless $area; + + $c->stash->{qs} = ""; + $c->stash->{type} = 'area_problems'; + $c->stash->{title_params} = { NAME => $area->{name} }; + $c->stash->{db_params} = [ $area->{id} ]; + + $c->detach( '/rss/output' ); + } + # XXX Currently body/area overlaps here are a bit muddy. # We're checking an area here, but this function is currently doing that. return if $c->cobrand->reports_body_check( $c, $area ); diff --git a/perllib/FixMyStreet/App/Controller/Rss.pm b/perllib/FixMyStreet/App/Controller/Rss.pm index 2eb95d63801..0b10aa68a07 100755 --- a/perllib/FixMyStreet/App/Controller/Rss.pm +++ b/perllib/FixMyStreet/App/Controller/Rss.pm @@ -50,29 +50,6 @@ sub new_problems : Path('problems') : Args(0) { $c->forward('output'); } -# FIXME I don't think this is used - check -#sub reports_to_council : Private { -# my ( $self, $c ) = @_; -# -# my $id = $c->stash->{id}; -# $c->stash->{type} = 'council_problems'; -# $c->stash->{qs} = '/' . $id; -# $c->stash->{db_params} = [ $id ]; -# $c->forward('output'); -#} - -sub reports_in_area : LocalRegex('^area/(\d+)$') { - my ( $self, $c ) = @_; - - my $id = $c->req->captures->[0]; - my $area = FixMyStreet::MapIt::call('area', $id); - $c->stash->{type} = 'area_problems'; - $c->stash->{qs} = '/' . $id; - $c->stash->{db_params} = [ $id ]; - $c->stash->{title_params} = { NAME => $area->{name} }; - $c->forward('output'); -} - sub all_problems : Private { my ( $self, $c ) = @_; diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm index d2ca6508b4c..7503f108a93 100644 --- a/perllib/FixMyStreet/Cobrand/Default.pm +++ b/perllib/FixMyStreet/Cobrand/Default.pm @@ -945,14 +945,12 @@ sub council_rss_alert_options { my ( @options, @reported_to_options ); foreach (values %$all_areas) { - $_->{short_name} = $self->short_name( $_ ); - ( $_->{id_name} = $_->{short_name} ) =~ tr/+/_/; push @options, { type => 'council', - id => sprintf( 'area:%s:%s', $_->{id}, $_->{id_name} ), + id => sprintf( 'area:%s', $_->{id} ), text => sprintf( _('Problems within %s'), $_->{name}), rss_text => sprintf( _('RSS feed of problems within %s'), $_->{name}), - uri => $c->uri_for( '/rss/area/' . $_->{short_name} ), + uri => $c->uri_for( '/rss/area/' . $_->{id} ), }; } diff --git a/perllib/FixMyStreet/Cobrand/FiksGataMi.pm b/perllib/FixMyStreet/Cobrand/FiksGataMi.pm index 73e98ffdb3a..3b658261128 100644 --- a/perllib/FixMyStreet/Cobrand/FiksGataMi.pm +++ b/perllib/FixMyStreet/Cobrand/FiksGataMi.pm @@ -152,19 +152,19 @@ sub council_rss_alert_options { push @options, { type => 'area', - id => sprintf( 'area:%s:%s', $kommune->{id}, $id_kommune_name ), + id => sprintf( 'area:%s', $kommune->{id} ), rss_text => sprintf( _('RSS feed of %s'), $kommune->{name} ), text => $kommune->{name}, - uri => $c->uri_for( '/rss/area', $short_kommune_name ), + uri => $c->uri_for( '/rss/area', $kommune->{id} ), }, { type => 'area', - id => sprintf( 'area:%s:%s', $fylke->{id}, $id_fylke_name ), + id => sprintf( 'area:%s', $fylke->{id} ), rss_text => sprintf( _('RSS feed of %s'), $fylke->{name} ), text => $fylke->{name}, - uri => $c->uri_for( '/rss/area', $short_fylke_name ), + uri => $c->uri_for( '/rss/area', $fylke->{id} ), }; push @reported_to_options, diff --git a/perllib/FixMyStreet/Cobrand/UK.pm b/perllib/FixMyStreet/Cobrand/UK.pm index f8b304cce0e..1f7ddc00331 100644 --- a/perllib/FixMyStreet/Cobrand/UK.pm +++ b/perllib/FixMyStreet/Cobrand/UK.pm @@ -262,17 +262,17 @@ sub council_rss_alert_options { push @options, { type => 'area', - id => sprintf( 'area:%s:%s', $council->{id}, $council->{id_name} ), + id => sprintf( 'area:%s', $council->{id} ), text => $council_text, rss_text => sprintf( _('RSS feed of problems within %s'), $council->{name}), - uri => $c->uri_for( '/rss/area/' . $council->{short_name} ), + uri => $c->uri_for( '/rss/area/' . $council->{id} ), }; push @options, { type => 'area', - id => sprintf( 'area:%s:%s:%s:%s', $council->{id}, $ward->{id}, $council->{id_name}, $ward->{id_name} ), + id => sprintf( 'area:%s', $ward->{id} ), rss_text => sprintf( _('RSS feed of problems within %s ward'), $ward->{name}), text => sprintf( _('All reports within %s ward'), $ward->{name}), - uri => $c->uri_for( '/rss/area/' . $council->{short_name} . '/' . $ward->{short_name} ), + uri => $c->uri_for( '/rss/area/' . $ward->{id} ), } if $ward; } elsif ( $num_councils == 4 ) { @@ -300,31 +300,31 @@ sub council_rss_alert_options { push @options, { title => $district_name, type => 'area', - id => sprintf( 'area:%s:%s', $district->{id}, $district->{id_name} ), + id => sprintf( 'area:%s', $district->{id} ), text => sprintf( _('All reports within %s'), $district_name ), rss_text => sprintf( _('RSS feed for %s'), $district_name ), - uri => $c->uri_for( '/rss/area/' . $district->{short_name} ) + uri => $c->uri_for( '/rss/area/' . $district->{id} ) }, { title => "$d_ward_name, $district_name", type => 'area', - id => sprintf( 'area:%s:%s:%s:%s', $district->{id}, $d_ward->{id}, $district->{id_name}, $d_ward->{id_name} ), + id => sprintf( 'area:%s', $d_ward->{id} ), text => sprintf( _('All reports within %s ward, %s'), $d_ward_name, $district_name ), rss_text => sprintf( _('RSS feed for %s ward, %s'), $d_ward_name, $district_name ), - uri => $c->uri_for( '/rss/area/' . $district->{short_name} . '/' . $d_ward->{short_name} ) + uri => $c->uri_for( '/rss/area/' . $d_ward->{id} ) }, { title => $county_name, type => 'area', - id => sprintf( 'area:%s:%s', $county->{id}, $county->{id_name} ), + id => sprintf( 'area:%s', $county->{id} ), text => sprintf( _('All reports within %s'), $county_name ), rss_text => sprintf( _('RSS feed for %s'), $county_name ), - uri => $c->uri_for( '/rss/area/' . $county->{short_name} ) + uri => $c->uri_for( '/rss/area/' . $county->{id} ) }, { title => "$c_ward_name, $county_name", type => 'area', - id => sprintf( 'area:%s:%s:%s:%s', $county->{id}, $c_ward->{id}, $county->{id_name}, $c_ward->{id_name} ), + id => sprintf( 'area:%s', $c_ward->{id} ), text => sprintf( _('All reports within %s ward, %s'), $c_ward_name, $county_name ), rss_text => sprintf( _('RSS feed for %s ward, %s'), $c_ward_name, $county_name ), - uri => $c->uri_for( '/rss/area/' . $county->{short_name} . '/' . $c_ward->{short_name} ) + uri => $c->uri_for( '/rss/area/' . $c_ward->{id} ) }; push @reported_to_options, { diff --git a/t/app/controller/alert.t b/t/app/controller/alert.t index 0f13dfe7e09..700725147ed 100644 --- a/t/app/controller/alert.t +++ b/t/app/controller/alert.t @@ -35,10 +35,10 @@ FixMyStreet::override_config { $mech->content_contains('rss/pc/EH11BB'); $mech->content_contains('All reports within Edinburgh City'); $mech->content_contains('All reports within City Centre ward'); - $mech->content_contains('/rss/area/Edinburgh'); - $mech->content_contains('/rss/area/Edinburgh/City+Centre'); - $mech->content_contains('area:2651:Edinburgh', 'Council feed contains Edinburgh id and details'); - $mech->content_contains('area:2651:20728:Edinburgh:City_Centre', 'Ward feed contains Edinburgh id and details'); + $mech->content_contains('/rss/area/2651'); + $mech->content_contains('/rss/area/20728'); + $mech->content_contains('area:2651', 'Council feed contains Edinburgh id and details'); + $mech->content_contains('area:20728', 'Ward feed contains Edinburgh id and details'); subtest "Test Nominatim lookup" => sub { $mech->get_ok('/alert/list?pc=High Street'); @@ -58,11 +58,11 @@ FixMyStreet::override_config { $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->get_ok('/alert/subscribe?rss=1&feed=area:1000:Birmingham'); - is $mech->uri->path, '/rss/reports/Birmingham'; + $mech->get_ok('/alert/subscribe?rss=1&feed=area:1000'); + is $mech->uri->path, '/rss/area/1000'; - $mech->get_ok('/alert/subscribe?rss=1&feed=area:1000:1001:Cheltenham:Lansdown'); - is $mech->uri->path, '/rss/area/Cheltenham/Lansdown'; + $mech->get_ok('/alert/subscribe?rss=1&feed=area:1001'); + is $mech->uri->path, '/rss/area/1001'; $mech->get_ok('/alert/subscribe?rss=1&feed=council:1000:Gloucestershire'); is $mech->uri->path, '/rss/reports/Gloucestershire'; diff --git a/t/app/controller/alert_new.t b/t/app/controller/alert_new.t index 6f19a13ef5a..031a0e2131f 100644 --- a/t/app/controller/alert_new.t +++ b/t/app/controller/alert_new.t @@ -383,12 +383,10 @@ subtest "Test two-tier council alerts" => sub { for my $alert ( { feed => "local:51.896269:-2.093063", result => '/rss/l/51.896269,-2.093063' }, { feed => "local:51.896269:-2.093063", result => '/rss/l/51.896269,-2.093063/4.5', distance => 4.5 }, - { feed => "area:2326:Cheltenham", result => '/rss/area/Cheltenham' }, - { feed => "area:2326:4544:Cheltenham:Lansdown", result => '/rss/area/Cheltenham/Lansdown' }, - { feed => "area:2226:Gloucestershire", result => '/rss/area/Gloucestershire' }, - { feed => "area:2226:14949:Gloucestershire:Lansdown_and_Park", - result => '/rss/area/Gloucestershire/Lansdown+and+Park' - }, + { feed => "area:2326", result => '/rss/area/2326' }, + { feed => "area:4544", result => '/rss/area/4544' }, + { feed => "area:2226", result => '/rss/area/2226' }, + { feed => "area:14949", result => '/rss/area/14949' }, { feed => "council:2326:Cheltenham", result => '/rss/reports/Cheltenham' }, { feed => "ward:2326:4544:Cheltenham:Lansdown", result => '/rss/reports/Cheltenham/Lansdown' }, { feed => "council:2226:Gloucestershire", result => '/rss/reports/Gloucestershire' }, diff --git a/t/cobrand/fiksgatami.t b/t/cobrand/fiksgatami.t index 7c006636bc4..1f0efc36b1f 100644 --- a/t/cobrand/fiksgatami.t +++ b/t/cobrand/fiksgatami.t @@ -17,12 +17,12 @@ FixMyStreet::override_config { $mech->get_ok('/alert/list?pc=3290'); $mech->content_contains('rss/l/59,10'); - $mech->content_contains('/rss/area/Larvik'); - $mech->content_contains('/rss/area/Vestfold'); + $mech->content_contains('/rss/area/709"'); + $mech->content_contains('/rss/area/7"'); $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('area:7"'); + $mech->content_contains('area:709"'); $mech->content_contains('council:' . $vestfold->id . ':Vestfold'); $mech->content_contains('council:' . $larvik->id . ':Larvik'); };