diff options
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/New.pm | 124 | ||||
| -rw-r--r-- | t/app/controller/report_import.t | 4 | ||||
| -rw-r--r-- | t/app/controller/report_new.t | 13 | ||||
| -rw-r--r-- | templates/web/default/report/new/all_councils_text.html | 13 | ||||
| -rw-r--r-- | templates/web/default/report/new/fill_in_details.html | 11 | ||||
| -rw-r--r-- | templates/web/default/report/new/some_councils_text.html | 12 |
6 files changed, 112 insertions, 65 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index dbefe9066..671454272 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -448,17 +448,18 @@ Look up categories for this council or councils sub setup_categories_and_councils : Private { my ( $self, $c ) = @_; - my @all_council_ids = keys %{ $c->stash->{all_councils} }; + my $all_councils = $c->stash->{all_councils}; + my $first_council = ( values %$all_councils )[0]; my @contacts # = $c # ->model('DB::Contact') # ->not_deleted # - ->search( { area_id => \@all_council_ids } ) # + ->search( { area_id => [ keys %$all_councils ] } ) # ->all; # variables to populate - my @area_ids_to_list = (); # Areas with categories assigned + my %area_ids_to_list = (); # Areas with categories assigned my @category_options = (); # categories to show my $category_label = undef; # what to call them @@ -467,7 +468,7 @@ sub setup_categories_and_councils : Private { # add all areas found to the list foreach (@contacts) { - push @area_ids_to_list, $_->area_id; + $area_ids_to_list{ $_->area_id } = 1; } # set our own categories @@ -481,14 +482,23 @@ sub setup_categories_and_councils : Private { _('Empty public building - school, hospital, etc.') ); $category_label = _('Property type:'); - } - else { - @contacts = keysort { $_->category } @contacts; + } elsif ($first_council->{type} eq 'LBO') { + + $area_ids_to_list{ $first_council->{id} } = 1; + @category_options = ( + _('-- Pick a category --'), + sort keys %{ Utils::london_categories() } + ); + $category_label = _('Category:'); + + } else { + + @contacts = keysort { $_->category } @contacts; # TODO Old code used strcoll, check if this no longer needed my %seen; foreach my $contact (@contacts) { - push @area_ids_to_list, $contact->area_id; + $area_ids_to_list{ $contact->area_id } = 1; next # TODO - move this to the cobrand if $c->cobrand->moniker eq 'southampton' @@ -509,24 +519,20 @@ sub setup_categories_and_councils : Private { } # put results onto stash for display - $c->stash->{area_ids_to_list} = \@area_ids_to_list; + $c->stash->{area_ids_to_list} = [ keys %area_ids_to_list ]; $c->stash->{category_label} = $category_label; $c->stash->{category_options} = \@category_options; - # add some conveniant things to the stash - my $all_councils = $c->stash->{all_councils}; - my %area_ids_to_list_hash = map { $_ => 1 } @area_ids_to_list; - my @missing_details_councils = - grep { !$area_ids_to_list_hash{$_} } # + grep { !$area_ids_to_list{$_} } # keys %$all_councils; my @missing_details_council_names = map { $all_councils->{$_}->{name} } # @missing_details_councils; - $c->stash->{missing_details_councils} = @missing_details_councils; - $c->stash->{missing_details_council_names} = @missing_details_council_names; + $c->stash->{missing_details_councils} = \@missing_details_councils; + $c->stash->{missing_details_council_names} = \@missing_details_council_names; } =head2 check_form_submitted @@ -607,7 +613,7 @@ sub process_report : Private { $report->latitude( $c->stash->{latitude} ); $report->longitude( $c->stash->{longitude} ); - # Capture whether the may was used + # Capture whether the map was used $report->used_map( $params{skipped} ? 0 : 1 ); # Short circuit unless the form has been submitted @@ -625,64 +631,73 @@ sub process_report : Private { $report->name( Utils::trim_text( $params{name} ) ); $report->category( _ $params{category} ); + # FIXME: This is more inefficient than old FixMyStreet code, + # with 2 MaPit point calls per submission now rather than just one. my $mapit_query = sprintf( "4326/%s,%s", $report->longitude, $report->latitude ); my $areas = mySociety::MaPit::call( 'point', $mapit_query ); $report->areas( ',' . join( ',', sort keys %$areas ) . ',' ); - # determine the area_types that this cobrand is interested in - my @area_types = $c->cobrand->area_types(); - my %area_types_lookup = map { $_ => 1 } @area_types; + # From earlier in the process. + my $councils = $c->stash->{all_councils}; + my $first_council = ( values %$councils )[0]; - # get all the councils that are of these types and cover this area - my %councils = - map { $_ => 1 } # - grep { $area_types_lookup{ $areas->{$_}->{type} } } # - keys %$areas; + if ( $c->cobrand->moniker eq 'emptyhomes' ) { - # partition the councils onto these two arrays - my @councils_with_category = (); - my @councils_without_category = (); + # all councils have all categories for emptyhomes + $report->council( join( ',', keys %$councils) ); - # all councils have all categories for emptyhomes - if ( $c->cobrand->moniker eq 'emptyhomes' ) { - @councils_with_category = keys %councils; - } - else { + } elsif ( $first_council->{type} eq 'LBO') { + unless ( Utils::london_categories()->{ $report->category } ) { + # TODO Perfect world, this wouldn't short-circuit, other errors would + # be included as well. + $c->stash->{field_errors} = { category => _('Please choose a category') }; + return; + } + $report->council( $first_council->{id} ); + + } elsif ( $report->category ) { + + # FIXME All contacts were fetched in setup_categories_and_councils, + # so can this DB call also be avoided? my @contacts = $c-> # model('DB::Contact') # ->not_deleted # ->search( { - area_id => [ keys %councils ], # + area_id => [ keys %$councils ], category => $report->category } )->all; - # clear category if it is not in db for possible councils - $report->category(undef) unless @contacts; + unless ( @contacts ) { + $c->stash->{field_errors} = { category => _('Please choose a category') }; + return; + } - my %councils_with_contact_for_category = - map { $_->area_id => 1 } @contacts; + # construct the council string: + # 'x,x' - x are council IDs that have this category + # 'x,x|y,y' - x are council IDs that have this category, y council IDs with *no* contact + my $council_string = join( ',', map { $_->area_id } @contacts ); + $council_string .= + '|' . join( ',', @{ $c->stash->{missing_details_councils} } ) + if $council_string && @{ $c->stash->{missing_details_councils} }; + $report->council($council_string); - foreach my $council_key ( keys %councils ) { - $councils_with_contact_for_category{$council_key} - ? push( @councils_with_category, $council_key ) - : push( @councils_without_category, $council_key ); - } + } elsif ( @{ $c->stash->{area_ids_to_list} } ) { - } + # There was an area with categories, but we've not been given one. Bail. + $c->stash->{field_errors} = { category => _('Please choose a category') }; + return; - # construct the council string: - # 'x,x' - x are councils_ids that have this category - # 'x,x|y,y' - x are councils_ids that have this category, y don't - my $council_string = join '|', grep { $_ } # - ( - join( ',', @councils_with_category ), - join( ',', @councils_without_category ) - ); - $report->council($council_string); + } else { + + # If we're here, we've been submitted somewhere + # where we have no contact information at all. + $report->council( -1 ); + + } # set defaults that make sense $report->state('unconfirmed'); @@ -855,6 +870,9 @@ sub save_user_and_report : Private { # Set a default if possible $report->category( _('Other') ) unless $report->category; + # Set unknown to DB unknown + $report->council( undef ) if $report->council == -1; + # save the report; $report->in_storage ? $report->update : $report->insert(); diff --git a/t/app/controller/report_import.t b/t/app/controller/report_import.t index 5bd0b21b8..4f225fc5a 100644 --- a/t/app/controller/report_import.t +++ b/t/app/controller/report_import.t @@ -125,6 +125,7 @@ subtest "Submit a correct entry" => sub { photo => '', phone => '', may_show_name => '1', + category => '-- Pick a category --', }, "check imported fields are shown"; @@ -141,6 +142,7 @@ subtest "Submit a correct entry" => sub { detail => 'This is a test report', phone => '01234 567 890', may_show_name => '1', + category => 'Street lighting', } }, "Update details and save" @@ -208,6 +210,7 @@ subtest "Submit a correct entry (with location)" => sub { photo => '', phone => '', may_show_name => '1', + category => '-- Pick a category --', }, "check imported fields are shown"; @@ -224,6 +227,7 @@ subtest "Submit a correct entry (with location)" => sub { detail => 'This is a test report ll', phone => '01234 567 890', may_show_name => '1', + category => 'Street lighting', } }, "Update details and save" diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t index 16e8e6445..160d1a156 100644 --- a/t/app/controller/report_new.t +++ b/t/app/controller/report_new.t @@ -34,6 +34,7 @@ foreach my $test ( may_show_name => '1', email => '', phone => '', + category => 'Street lighting', }, changes => {}, errors => [ @@ -54,6 +55,7 @@ foreach my $test ( may_show_name => undef, email => '', phone => '', + category => 'Street lighting', }, changes => { may_show_name => '1' }, errors => [ @@ -74,6 +76,7 @@ foreach my $test ( may_show_name => undef, email => '', phone => '', + category => 'Street lighting', }, changes => {}, errors => [ @@ -93,6 +96,7 @@ foreach my $test ( may_show_name => '1', email => '', phone => '', + category => 'Street lighting', }, changes => {}, errors => [ @@ -112,6 +116,7 @@ foreach my $test ( may_show_name => '1', email => '', phone => '', + category => 'Street lighting', }, changes => { title => 'Dog poo on walls', @@ -131,6 +136,7 @@ foreach my $test ( may_show_name => '1', email => '', phone => '', + category => 'Street lighting', }, changes => {}, errors => [ @@ -149,6 +155,7 @@ foreach my $test ( may_show_name => '1', email => '', phone => '', + category => 'Street lighting', }, changes => {}, errors => [ @@ -167,6 +174,7 @@ foreach my $test ( may_show_name => '1', email => 'not an email', phone => '', + category => 'Street lighting', }, changes => { email => 'notanemail', }, errors => [ 'Please enter a valid email', ], @@ -182,6 +190,7 @@ foreach my $test ( may_show_name => '1', email => '', phone => '', + category => 'Street lighting', }, changes => { title => 'Test title', @@ -200,6 +209,7 @@ foreach my $test ( may_show_name => '1', email => ' BOB @ExAmplE.COM ', phone => '', + category => 'Street lighting', }, changes => { name => 'Bob Jones', @@ -266,6 +276,7 @@ subtest "test report creation for a user who does not have an account" => sub { may_show_name => '1', email => 'test-1@example.com', phone => '07903 123 456', + category => 'Street lighting', } }, "submit good details" @@ -358,6 +369,7 @@ subtest "test report creation for a user who is logged in" => sub { name => 'Test User', phone => '01234 567 890', photo => '', + category => '-- Pick a category --', }, "user's details prefilled" ); @@ -375,6 +387,7 @@ subtest "test report creation for a user who is logged in" => sub { name => 'Joe Bloggs', may_show_name => '1', phone => '07903 123 456', + category => 'Street lighting', } }, "submit good details" diff --git a/templates/web/default/report/new/all_councils_text.html b/templates/web/default/report/new/all_councils_text.html index 2cd90b213..8514e0b0a 100644 --- a/templates/web/default/report/new/all_councils_text.html +++ b/templates/web/default/report/new/all_councils_text.html @@ -1,8 +1,19 @@ <p> +[% IF all_councils.${area_ids_to_list.0}.type == 'LBO' %] [% tprintf( - loc('All the information you provide here will be sent to <strong>%s</strong>. The subject and details of the problem will be public, plus your name if you give us permission.'), + loc('All the information you provide here will be sent to <strong>%s</strong> or a relevant local body such as <strong>TfL</strong>, via the London Report-It system.'), all_council_names.join( '</strong>' _ loc(' or ') _ '<strong>' ) ); %] +[% ELSE %] +[% + tprintf( + loc('All the information you provide here will be sent to <strong>%s</strong>.'), + all_council_names.join( '</strong>' _ loc(' or ') _ '<strong>' ) + ); +%] +[% END %] + +[% loc('The subject and details of the problem will be public, plus your name if you give us permission.') %] </p> diff --git a/templates/web/default/report/new/fill_in_details.html b/templates/web/default/report/new/fill_in_details.html index 00055c21b..63f8e41e8 100644 --- a/templates/web/default/report/new/fill_in_details.html +++ b/templates/web/default/report/new/fill_in_details.html @@ -29,11 +29,9 @@ <h1>[% loc('Reporting a problem') %]</h1> -[% - IF report.used_map; - loc('You have located the problem at the point marked with a purple pin on the map. If this is not the correct location, simply click on the map again. '); - END; -%] +[% IF report.used_map %] +<p>[% loc('You have located the problem at the point marked with a purple pin on the map. If this is not the correct location, simply click on the map again. ') %]</p> +[% END %] [% IF area_ids_to_list.size == 0 %] [% INCLUDE 'report/new/no_councils_text.html' %] @@ -43,12 +41,13 @@ [% INCLUDE 'report/new/some_councils_text.html' %] [% END %] - +<p> [% IF skipped %] [% loc('Please fill in the form below with details of the problem, and describe the location as precisely as possible in the details box.') %] [% ELSE %] [% INCLUDE 'report/new/fill_in_details_text.html' %] [% END %] +</p> [% INCLUDE 'errors.html' %] diff --git a/templates/web/default/report/new/some_councils_text.html b/templates/web/default/report/new/some_councils_text.html index 7de7c3f45..5019becc8 100644 --- a/templates/web/default/report/new/some_councils_text.html +++ b/templates/web/default/report/new/some_councils_text.html @@ -1,21 +1,23 @@ <p> [% loc('All the information you provide here will be sent to') %] -[% FOREACH council IN all_council_names %] +[% FOREACH council_id IN area_ids_to_list %] [% loc( ' or ') IF ! loop.first %] - <strong>[% council %]</strong> + <strong>[% all_councils.$council_id.name %]</strong> [%- '.' IF loop.last %] [% END %] [% loc('The subject and details of the problem will be public, plus your name if you give us permission.'); - +%] +[% nget( 'We do <strong>not</strong> yet have details for the other council that covers this location.', 'We do <strong>not</strong> yet have details for the other councils that cover this location.', missing_details_councils.size ); - +%] +[% tprintf( loc("You can help us by finding a contact email address for local problems for %s and emailing it to us at <a href='mailto:%s'>%s</a>."), missing_details_council_names.join( loc(' or ') ), @@ -23,4 +25,4 @@ c.cobrand.contact_email ); %] - +</p> |