aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--notes/location_related_flow.txt23
-rw-r--r--perllib/FixMyStreet/App/Controller/Around.pm136
-rw-r--r--perllib/FixMyStreet/App/Controller/Report/New.pm107
-rw-r--r--perllib/FixMyStreet/TestMech.pm18
-rw-r--r--t/app/controller/around.t74
-rw-r--r--t/app/controller/report_import.t8
-rw-r--r--t/app/controller/report_new.t73
-rw-r--r--templates/web/default/around/around_index.html (renamed from templates/web/default/report/new/report_new.html)20
8 files changed, 266 insertions, 193 deletions
diff --git a/notes/location_related_flow.txt b/notes/location_related_flow.txt
new file mode 100644
index 000000000..5af3c88f0
--- /dev/null
+++ b/notes/location_related_flow.txt
@@ -0,0 +1,23 @@
+--- '/' ---
+
+ Homepage has a search form that submits to /around. There is no location
+ processing done on the home page. Any queries sent to the homepage are
+ redirected to /around.
+
+--- '/around' ---
+
+ SEARCH: allows user to search for a location using postcode or other text.
+ If nothing matched error shown. If multiple match show alternatives.
+
+ LIST: If a search could be resolved to a lat/lon, or a lot/lon was in query
+ show matches near that location.
+
+ PARTIAL: If there is a partial token show a message when searching. When a
+ match is found redirect to '/report/new' for the partial to be completed.
+
+--- '/report/new' ---
+
+ Requires a lat/lng, or a tile click, or a partial report with a location
+ stored. If no location can be deteremined redirects back to '/around'. All
+ form information is lost but the partial token is preserved.
+
diff --git a/perllib/FixMyStreet/App/Controller/Around.pm b/perllib/FixMyStreet/App/Controller/Around.pm
index 8df7b880f..6a6e82d02 100644
--- a/perllib/FixMyStreet/App/Controller/Around.pm
+++ b/perllib/FixMyStreet/App/Controller/Around.pm
@@ -6,6 +6,7 @@ BEGIN { extends 'Catalyst::Controller'; }
use FixMyStreet::Map;
use List::MoreUtils qw(any);
+use Encode;
=head1 NAME
@@ -40,14 +41,139 @@ sub around_index : Path : Args(0) {
return;
}
- # if there was no search then redirect to the homepage
- if ( !any { $c->req->param($_) } qw(pc lat lon) ) {
- return $c->res->redirect( $c->uri_for('/') );
- }
-
+ # Try to create a location for whatever we have
+ return
+ unless $c->forward('determine_location_from_coords')
+ || $c->forward('determine_location_from_pc');
+
+ # Check to see if the spot is covered by a council - if not show an error.
+ return unless $c->forward('check_location_is_acceptable');
+
+ # If we have a partial - redirect to /report/new so that it can be
+ # completed.
+ warn "FIXME - implement";
+
+ # Show the nearby reports
+ die "show nearby reports";
}
+=head2 determine_location_from_coords
+
+Use latitude and longitude if provided in parameters.
+
+=cut
+
+sub determine_location_from_coords : Private {
+ my ( $self, $c ) = @_;
+
+ my $latitude = $c->req->param('latitude');
+ my $longitude = $c->req->param('longitude');
+
+ if ( defined $latitude && defined $longitude ) {
+ $c->stash->{latitude} = $latitude;
+ $c->stash->{longitude} = $longitude;
+
+ # Also save the pc if there is one
+ if ( my $pc = $c->req->param('pc') ) {
+ $c->stash->{pc} = $pc;
+ }
+
+ return 1;
+ }
+
+ return;
+}
+
+=head2 determine_location_from_pc
+
+User has searched for a location - try to find it for them.
+
+If one match is found returns true and lat/lng is set.
+
+If several possible matches are found puts an array onto stash so that user can be prompted to pick one and returns false.
+
+If no matches are found returns false.
+
+=cut
+
+sub determine_location_from_pc : Private {
+ my ( $self, $c ) = @_;
+
+ # check for something to search
+ my $pc = $c->req->param('pc') || return;
+ $c->stash->{pc} = $pc; # for template
+
+ my ( $latitude, $longitude, $error ) =
+ eval { FixMyStreet::Geocode::lookup( $pc, $c->req ) };
+
+ # Check that nothing blew up
+ if ($@) {
+ warn "Error: $@";
+ return;
+ }
+
+ # If we got a lat/lng set to stash and return true
+ if ( defined $latitude && defined $longitude ) {
+ $c->stash->{latitude} = $latitude;
+ $c->stash->{longitude} = $longitude;
+ return 1;
+ }
+
+ # $error doubles up to return multiple choices by being an array
+ if ( ref($error) eq 'ARRAY' ) {
+ @$error = map {
+ decode_utf8($_);
+ s/, United Kingdom//;
+ s/, UK//;
+ $_;
+ } @$error;
+ $c->stash->{possible_location_matches} = $error;
+ return;
+ }
+
+ # pass errors back to the template
+ $c->stash->{pc_error} = $error;
+ return;
+}
+
+=head2 check_location_is_acceptable
+
+Find the lat and lon in stash and check that they are acceptable to the council,
+and that they are in UK (if we are in UK).
+
+=cut
+
+sub check_location_is_acceptable : Private {
+ my ( $self, $c ) = @_;
+
+ # These should be set now
+ my $lat = $c->stash->{latitude};
+ my $lon = $c->stash->{longitude};
+
+ # Check this location is okay to be displayed for the cobrand
+ my ( $success, $error_msg ) = $c->cobrand->council_check( #
+ { lat => $lat, lon => $lon },
+ 'submit_problem'
+ );
+
+ # If in UK and we have a lat,lon coocdinate check it is in UK
+ if ( !$error_msg && $lat && $c->config->{COUNTRY} eq 'GB' ) {
+ eval { Utils::convert_latlon_to_en( $lat, $lon ); };
+ $error_msg =
+ _( "We had a problem with the supplied co-ordinates - outside the UK?"
+ ) if $@;
+ }
+
+ # all good
+ return 1 if !$error_msg;
+
+ # show error
+ $c->stash->{pc_error} = $error_msg;
+ return;
+
+}
+
__PACKAGE__->meta->make_immutable;
1;
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index 4e077f32b..2dbec4235 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -375,34 +375,12 @@ sub determine_location : Private {
return
unless $c->forward('determine_location_from_tile_click')
- || $c->forward('determine_location_from_coords')
- || $c->forward('determine_location_from_pc')
+ || $c->forward('/around/determine_location_from_coords')
+ || $c->forward('/around/determine_location_from_pc')
|| $c->forward('determine_location_from_report');
- # These should be set now
- my $lat = $c->stash->{latitude};
- my $lon = $c->stash->{longitude};
- # Check this location is okay to be displayed for the cobrand
- my ( $success, $error_msg ) = $c->cobrand->council_check( #
- { lat => $lat, lon => $lon },
- 'submit_problem'
- );
-
- # If in UK and we have a lat,lon coocdinate check it is in UK
- if ( !$error_msg && $lat && $c->config->{COUNTRY} eq 'GB' ) {
- eval { Utils::convert_latlon_to_en( $lat, $lon ); };
- $error_msg =
- _( "We had a problem with the supplied co-ordinates - outside the UK?"
- ) if $@;
- }
-
- # all good
- return 1 if !$error_msg;
-
- # show error
- $c->stash->{pc_error} = $error_msg;
- return;
+ return $c->forward('/around/check_location_is_acceptable');
}
=head2 determine_location_from_tile_click
@@ -456,85 +434,6 @@ sub determine_location_from_tile_click : Private {
return 1;
}
-=head2 determine_location_from_coords
-
-Use latitude and longitude if provided in parameters.
-
-=cut
-
-sub determine_location_from_coords : Private {
- my ( $self, $c ) = @_;
-
- my $latitude = $c->req->param('latitude');
- my $longitude = $c->req->param('longitude');
-
- if ( defined $latitude && defined $longitude ) {
- $c->stash->{latitude} = $latitude;
- $c->stash->{longitude} = $longitude;
-
- # Also save the pc if there is one
- if ( my $pc = $c->req->param('pc') ) {
- $c->stash->{pc} = $pc;
- }
-
- return 1;
- }
-
- return;
-}
-
-=head2 determine_location_from_pc
-
-User has searched for a location - try to find it for them.
-
-If one match is found returns true and lat/lng is set.
-
-If several possible matches are found puts an array onto stash so that user can be prompted to pick one and returns false.
-
-If no matches are found returns false.
-
-=cut
-
-sub determine_location_from_pc : Private {
- my ( $self, $c ) = @_;
-
- # check for something to search
- my $pc = $c->req->param('pc') || return;
- $c->stash->{pc} = $pc; # for template
-
- my ( $latitude, $longitude, $error ) =
- eval { FixMyStreet::Geocode::lookup( $pc, $c->req ) };
-
- # Check that nothing blew up
- if ($@) {
- warn "Error: $@";
- return;
- }
-
- # If we got a lat/lng set to stash and return true
- if ( defined $latitude && defined $longitude ) {
- $c->stash->{latitude} = $latitude;
- $c->stash->{longitude} = $longitude;
- return 1;
- }
-
- # $error doubles up to return multiple choices by being an array
- if ( ref($error) eq 'ARRAY' ) {
- @$error = map {
- decode_utf8($_);
- s/, United Kingdom//;
- s/, UK//;
- $_;
- } @$error;
- $c->stash->{possible_location_matches} = $error;
- return;
- }
-
- # pass errors back to the template
- $c->stash->{pc_error} = $error;
- return;
-}
-
=head2 determine_location_from_report
Use latitude and longitude stored in the report - this is probably result of a
diff --git a/perllib/FixMyStreet/TestMech.pm b/perllib/FixMyStreet/TestMech.pm
index 26738556d..6a4382c3d 100644
--- a/perllib/FixMyStreet/TestMech.pm
+++ b/perllib/FixMyStreet/TestMech.pm
@@ -204,6 +204,24 @@ sub form_errors {
return $result->{errors} || [];
}
+=head2 page_errors
+
+ my $arrayref = $mech->page_errors;
+
+Find all the form errors on the current page and return them in page order as an
+arrayref of TEXTs. If none found return empty arrayref.
+
+=cut
+
+sub page_errors {
+ my $mech = shift;
+ my $result = scraper {
+ process 'p.error', 'errors[]', 'TEXT';
+ }
+ ->scrape( $mech->response );
+ return $result->{errors} || [];
+}
+
=head2 import_errors
my $arrayref = $mech->import_errors;
diff --git a/t/app/controller/around.t b/t/app/controller/around.t
index 021cf8f7d..069ec605a 100644
--- a/t/app/controller/around.t
+++ b/t/app/controller/around.t
@@ -7,7 +7,7 @@ my $mech = FixMyStreet::TestMech->new;
subtest "check that if no query we get sent back to the homepage" => sub {
$mech->get_ok('/around');
- is $mech->uri->path, '/', "sent back to '/'";
+ is $mech->uri->path, '/around', "still at '/around'";
};
# x,y requests were generated by the old map code. We keep the behavior for
@@ -28,4 +28,76 @@ subtest "redirect x,y requests to lat/lon (301 - permanent)" => sub {
};
+# test various locations on inital search box
+foreach my $test (
+ {
+ pc => '', #
+ errors => [],
+ pc_alternatives => [],
+ },
+ {
+ pc => 'xxxxxxxxxxxxxxxxxxxxxxxxxxx',
+ errors => ['Sorry, we could not find that location.'],
+ pc_alternatives => [],
+ },
+ {
+ pc => 'glenthorpe',
+ errors => [],
+ pc_alternatives => [ # TODO - should filter out these non-UK addresses
+ 'Glenthorpe Crescent, Leeds LS9 7',
+ 'Glenthorpe Rd, Merton, Greater London SM4 4',
+ 'Glenthorpe Ln, Katy, TX 77494, USA',
+ 'Glenthorpe Dr, Walnut, CA 91789, USA',
+ 'Glenthorpe Ave, Leeds LS9 7',
+ 'Glenthorpe Ct, Katy, TX 77494, USA',
+ ],
+ },
+ {
+ pc => 'Glenthorpe Ct, Katy, TX 77494, USA',
+ errors =>
+ ['We had a problem with the supplied co-ordinates - outside the UK?'],
+ pc_alternatives => [],
+ },
+ )
+{
+ subtest "test bad pc value '$test->{pc}'" => sub {
+ $mech->get_ok('/');
+ $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } },
+ "bad location" );
+ is_deeply $mech->page_errors, $test->{errors},
+ "expected errors for pc '$test->{pc}'";
+ is_deeply $mech->pc_alternatives, $test->{pc_alternatives},
+ "expected alternatives for pc '$test->{pc}'";
+ };
+}
+
+# check that exact queries result in the correct lat,lng
+foreach my $test (
+ {
+ pc => 'SW1A 1AA',
+ latitude => '51.5010096115539',
+ longitude => '-0.141587067110009',
+ },
+ {
+ pc => 'Manchester',
+ latitude => '53.4807125',
+ longitude => '-2.2343765',
+ },
+ {
+ pc => 'Glenthorpe Rd, Merton, Greater London SM4 4, UK',
+ latitude => '51.3937997',
+ longitude => '-0.2209596',
+ },
+ )
+{
+ subtest "check lat/lng for '$test->{pc}'" => sub {
+ $mech->get_ok('/');
+ $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } },
+ "good location" );
+ is_deeply $mech->form_errors, [], "no errors for pc '$test->{pc}'";
+ is_deeply $mech->extract_location, $test,
+ "got expected location for pc '$test->{pc}'";
+ };
+}
+
done_testing();
diff --git a/t/app/controller/report_import.t b/t/app/controller/report_import.t
index 5c16324d3..764357b57 100644
--- a/t/app/controller/report_import.t
+++ b/t/app/controller/report_import.t
@@ -105,12 +105,16 @@ subtest "Submit a correct entry" => sub {
is_deeply $mech->visible_form_values, { pc => '' },
"check only pc field is shown";
+ is $mech->uri->path, '/around', "sent to report page";
+
$mech->submit_form_ok( #
{ with_fields => { pc => 'SW1A 1AA' } },
"fill in postcode"
);
- # check that we are not shown anything as we don't have a location yet
+ is $mech->uri->path, '/report/new', "sent to report page";
+
+ # check that fields are prefilled for us
is_deeply $mech->visible_form_values,
{
name => 'Test User',
@@ -124,7 +128,7 @@ subtest "Submit a correct entry" => sub {
"check imported fields are shown";
TODO: {
- local $TODO = "'/report/123' urls not srved by catalyst yet";
+ local $TODO = "'/report/123' urls not served by catalyst yet";
# change the details
$mech->submit_form_ok( #
diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t
index dca86db77..1310a3e35 100644
--- a/t/app/controller/report_new.t
+++ b/t/app/controller/report_new.t
@@ -8,77 +8,10 @@ use Web::Scraper;
my $mech = FixMyStreet::TestMech->new;
$mech->get_ok('/report/new');
-# test various locations on inital search box
-foreach my $test (
- {
- pc => '', #
- errors => [],
- pc_alternatives => [],
- },
- {
- pc => 'xxxxxxxxxxxxxxxxxxxxxxxxxxx',
- errors => ['Sorry, we could not find that location.'],
- pc_alternatives => [],
- },
- {
- pc => 'glenthorpe',
- errors => [],
- pc_alternatives => [ # TODO - should filter out these non-UK addresses
- 'Glenthorpe Crescent, Leeds LS9 7, UK',
- 'Glenthorpe Rd, Merton, Greater London SM4 4, UK',
- 'Glenthorpe Ln, Katy, TX 77494, USA',
- 'Glenthorpe Dr, Walnut, CA 91789, USA',
- 'Glenthorpe Ave, Leeds LS9 7, UK',
- 'Glenthorpe Ct, Katy, TX 77494, USA',
- ],
- },
- {
- pc => 'Glenthorpe Ct, Katy, TX 77494, USA',
- errors =>
- ['We had a problem with the supplied co-ordinates - outside the UK?'],
- pc_alternatives => [],
- },
- )
-{
- subtest "test bad pc value '$test->{pc}'" => sub {
- $mech->get_ok('/report/new');
- $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } },
- "bad location" );
- is_deeply $mech->form_errors, $test->{errors},
- "expected errors for pc '$test->{pc}'";
- is_deeply $mech->pc_alternatives, $test->{pc_alternatives},
- "expected alternatives for pc '$test->{pc}'";
- };
-}
+fail "test that lat,lon not covered by council goes to '/around'";
+
+fail "test that partial code is transferred";
-# check that exact queries result in the correct lat,lng
-foreach my $test (
- {
- pc => 'SW1A 1AA',
- latitude => '51.5010096115539',
- longitude => '-0.141587067110009',
- },
- {
- pc => 'Manchester',
- latitude => '53.4807125',
- longitude => '-2.2343765',
- },
- {
- pc => 'Glenthorpe Rd, Merton, Greater London SM4 4, UK',
- latitude => '51.3937997',
- longitude => '-0.2209596',
- },
- )
-{
- subtest "check lat/lng for '$test->{pc}'" => sub {
- $mech->get_ok('/report/new');
- $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } },
- "good location" );
- is_deeply $mech->form_errors, [], "no errors for pc '$test->{pc}'";
- is_deeply $mech->extract_location, $test,
- "got expected location for pc '$test->{pc}'";
- };
-}
# test that the various bit of form get filled in and errors correctly
# generated.
diff --git a/templates/web/default/report/new/report_new.html b/templates/web/default/around/around_index.html
index 434da9c6b..07b3cee4d 100644
--- a/templates/web/default/report/new/report_new.html
+++ b/templates/web/default/around/around_index.html
@@ -8,8 +8,16 @@
%]
-<h1>[% loc('Reporting a problem') %]</h1>
+<form action="[% partial_token ? '/report/new' : '/' %]" method="get" name="postcodeForm" id="postcodeForm"><label for="pc">[% loc("Enter a nearby GB postcode, or street name and area") %]:</label>
+&nbsp;<input type="text" name="pc" value="[% pc | html %]" id="pc" size="10" maxlength="200">
+&nbsp;<input type="submit" value="[% loc('Go') %]" id="submit">
+[% c.cobrand.form_elements %]
+
+[% IF partial_token %]
+ <input type="hidden" name="partial" value="[% partial_token.token %]">
+[% END %]
+</form>
[% IF location_error %]
<div class="error">[% location_error %]</div>
[% END %]
@@ -34,15 +42,5 @@
<p class="error">[% pc_error %]</p>
[% END %]
-<form action="[% partial_token ? '/report/new' : '/' %]" method="get" name="postcodeForm" id="postcodeForm"><label for="pc">[% loc("Enter a nearby GB postcode, or street name and area") %]:</label>
-&nbsp;<input type="text" name="pc" value="[% pc | html %]" id="pc" size="10" maxlength="200">
-&nbsp;<input type="submit" value="[% loc('Go') %]" id="submit">
-[% c.cobrand.form_elements %]
-
-[% IF partial_token %]
- <input type="hidden" name="partial" value="[% partial_token.token %]">
-[% END %]
-
-</form>
[% INCLUDE 'footer.html' %]
generated by cgit v1.2.3 (git 2.25.1) at 2025-04-26 16:47:02 +0000