diff options
-rw-r--r-- | .gitignore | 1 | ||||
-rw-r--r-- | CHANGELOG.md | 1 | ||||
-rwxr-xr-x | perllib/CronFns.pm | 1 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin.pm | 2 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/New.pm | 17 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Reports.pm | 1 | ||||
-rwxr-xr-x | perllib/FixMyStreet/Script/UpdateAllReports.pm | 42 | ||||
-rw-r--r-- | t/Mock/MapIt.pm | 1 | ||||
-rw-r--r-- | t/app/controller/report_new.t | 1 | ||||
-rw-r--r-- | t/app/controller/reports.t | 3 | ||||
-rw-r--r-- | t/cobrand/form_extras.t | 35 | ||||
-rw-r--r-- | templates/web/base/around/index.html | 2 | ||||
-rw-r--r-- | templates/web/base/report/new/category_extras.html | 3 | ||||
-rw-r--r-- | templates/web/base/report/new/councils_text_all.html | 3 | ||||
-rw-r--r-- | templates/web/base/report/new/form_report.html | 11 | ||||
-rw-r--r-- | templates/web/base/report/new/form_title.html | 7 | ||||
-rw-r--r-- | templates/web/base/report/update/form_state_checkbox.html | 17 | ||||
-rw-r--r-- | templates/web/base/report/update/form_update.html | 18 | ||||
-rw-r--r-- | templates/web/fixamingata/report/new/top_message_none.html | 6 |
19 files changed, 115 insertions, 57 deletions
diff --git a/.gitignore b/.gitignore index cf29316ee..b4e5679a1 100644 --- a/.gitignore +++ b/.gitignore @@ -52,3 +52,4 @@ gh_fixmycommunity # Commercial /fixmystreet-commercial *[Gg]round[Cc]ontrol* +[Ss]midsy* diff --git a/CHANGELOG.md b/CHANGELOG.md index f8a0ef7e5..66a1b69c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ - Add CORS header to Open311 output. #2022 - Add some Cypress browser-based testing. - Upgrade Vagrantfile to use Ubuntu Xenial. #2093 + - Add validation to cobrand-specific custom reporting fields. * v2.3.1 (12th February 2018) - Front end improvements: diff --git a/perllib/CronFns.pm b/perllib/CronFns.pm index 888817e05..58237d322 100755 --- a/perllib/CronFns.pm +++ b/perllib/CronFns.pm @@ -27,6 +27,7 @@ sub site { my $base_url = shift; my $site = 'fixmystreet'; $site = 'zurich' if $base_url =~ /zurich|zueri/; + $site = 'smidsy' if $base_url =~ /smidsy|collideosco/; return $site; } diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm index 33404cb82..2cd34b7e2 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -1042,7 +1042,7 @@ sub report_edit_location : Private { $c->forward('/council/load_and_check_areas', []); $c->forward('/report/new/setup_categories_and_bodies'); my %allowed_bodies = map { $_ => 1 } @{$problem->bodies_str_ids}; - my @new_bodies = @{$c->stash->{bodies_to_list}}; + my @new_bodies = keys %{$c->stash->{bodies_to_list}}; my $bodies_match = grep { exists( $allowed_bodies{$_} ) } @new_bodies; $c->stash($safe_stash); return unless $bodies_match; diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index 312268f65..8c6c1b244 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -695,9 +695,7 @@ sub setup_categories_and_bodies : Private { # put results onto stash for display $c->stash->{bodies} = \%bodies; $c->stash->{contacts} = \@contacts; - $c->stash->{bodies_to_list} = [ keys %bodies_to_list ]; - $c->stash->{bodies_to_list_names} = [ map { $_->name } values %bodies_to_list ]; - $c->stash->{bodies_to_list_urls} = [ map { $_->external_url } values %bodies_to_list ]; + $c->stash->{bodies_to_list} = \%bodies_to_list; $c->stash->{category_options} = \@category_options; $c->stash->{category_extras} = \%category_extras; $c->stash->{category_extras_hidden} = \%category_extras_hidden; @@ -920,6 +918,7 @@ sub process_report : Private { # set these straight from the params $report->category( _ $params{category} ) if $params{category}; + $c->cobrand->call_hook(report_new_munge_category => $report); $report->subcategory( $params{subcategory} ); my $areas = $c->stash->{all_areas_mapit}; @@ -948,7 +947,7 @@ sub process_report : Private { if ( $c->stash->{non_public_categories}->{ $report->category } ) { $report->non_public( 1 ); } - } elsif ( @{ $c->stash->{bodies_to_list} } ) { + } elsif ( %{ $c->stash->{bodies_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'); @@ -967,6 +966,14 @@ sub process_report : Private { my $value = $c->get_param($form_name) || ''; $c->stash->{field_errors}->{$form_name} = _('This information is required') if $field->{required} && !$value; + if ($field->{validator}) { + eval { + $value = $field->{validator}->($value); + }; + if ($@) { + $c->stash->{field_errors}->{$form_name} = $@; + } + } $report->set_extra_metadata( $form_name => $value ); } @@ -1353,6 +1360,8 @@ sub save_user_and_report : Private { $c->log->info($report->user->id . ' exists, but is not logged in for this report'); } + $c->cobrand->call_hook(report_new_munge_before_insert => $report); + $report->update_or_insert; # tidy up diff --git a/perllib/FixMyStreet/App/Controller/Reports.pm b/perllib/FixMyStreet/App/Controller/Reports.pm index 037458538..20381bb85 100644 --- a/perllib/FixMyStreet/App/Controller/Reports.pm +++ b/perllib/FixMyStreet/App/Controller/Reports.pm @@ -93,6 +93,7 @@ sub index : Path : Args(0) { } } else { my @bodies = $c->model('DB::Body')->active->translated->with_area_count->all_sorted; + @bodies = @{$c->cobrand->call_hook('reports_hook_restrict_bodies_list', \@bodies) || \@bodies }; $c->stash->{bodies} = \@bodies; } diff --git a/perllib/FixMyStreet/Script/UpdateAllReports.pm b/perllib/FixMyStreet/Script/UpdateAllReports.pm index d6f3eb64b..21d8d28a0 100755 --- a/perllib/FixMyStreet/Script/UpdateAllReports.pm +++ b/perllib/FixMyStreet/Script/UpdateAllReports.pm @@ -6,23 +6,24 @@ use warnings; use FixMyStreet; use FixMyStreet::Cobrand; use FixMyStreet::DB; +use CronFns; use List::MoreUtils qw(zip); use List::Util qw(sum); +my $site = CronFns::site(FixMyStreet->config('BASE_URL')); + my $fourweeks = 4*7*24*60*60; # Age problems from when they're confirmed, except on Zurich # where they appear as soon as they're created. my $age_column = 'confirmed'; -if ( FixMyStreet->config('BASE_URL') =~ /zurich|zueri/ ) { - $age_column = 'created'; -} +$age_column = 'created' if $site eq 'zurich'; my $dtf = FixMyStreet::DB->schema->storage->datetime_parser; -my $cobrand = FixMyStreet::Cobrand->get_class_for_moniker('default')->new; -FixMyStreet::DB->schema->cobrand($cobrand); +my $cobrand_cls = FixMyStreet::Cobrand->get_class_for_moniker($site)->new; +FixMyStreet::DB->schema->cobrand($cobrand_cls); sub generate { my $include_areas = shift; @@ -34,7 +35,7 @@ sub generate { }, { columns => [ - 'id', 'bodies_str', 'state', 'areas', 'cobrand', + 'id', 'bodies_str', 'state', 'areas', 'cobrand', 'category', { duration => { extract => "epoch from current_timestamp-lastupdate" } }, { age => { extract => "epoch from current_timestamp-$age_column" } }, ] @@ -43,13 +44,26 @@ sub generate { $problems = $problems->cursor; # Raw DB cursor for speed my ( %fixed, %open ); - my @cols = ( 'id', 'bodies_str', 'state', 'areas', 'cobrand', 'duration', 'age' ); + my %stats = ( + fixed => \%fixed, + open => \%open, + ); + my @cols = ( 'id', 'bodies_str', 'state', 'areas', 'cobrand', 'category', 'duration', 'age' ); while ( my @problem = $problems->next ) { my %problem = zip @cols, @problem; - my @bodies; - my @areas; + my @bodies = split( /,/, $problem{bodies_str} ); my $cobrand = $problem{cobrand}; + + if (my $type = $cobrand_cls->call_hook(dashboard_categorize_problem => \%problem)) { + foreach my $body ( @bodies ) { + $stats{$type}{$body}++; + $stats{$cobrand}{$type}{$body}++; + } + next; + } + my $duration_str = ( $problem{duration} > 2 * $fourweeks ) ? 'old' : 'new'; + my $type = ( $problem{duration} > 2 * $fourweeks ) ? 'unknown' : ($problem{age} > $fourweeks ? 'older' : 'new'); @@ -57,9 +71,6 @@ sub generate { FixMyStreet::DB::Result::Problem->fixed_states()->{$problem{state}} || FixMyStreet::DB::Result::Problem->closed_states()->{$problem{state}}; - # Add to bodies it was sent to - @bodies = split( /,/, $problem{bodies_str} ); - foreach my $body ( @bodies ) { if ( $problem_fixed ) { # Fixed problems are either old or new @@ -73,7 +84,7 @@ sub generate { } if ( $include_areas ) { - @areas = grep { $_ } split( /,/, $problem{areas} ); + my @areas = grep { $_ } split( /,/, $problem{areas} ); foreach my $area ( @areas ) { if ( $problem_fixed ) { $fixed{areas}{$area}{$duration_str}++; @@ -84,10 +95,7 @@ sub generate { } } - return { - fixed => \%fixed, - open => \%open, - }; + return \%stats; } sub end_period { diff --git a/t/Mock/MapIt.pm b/t/Mock/MapIt.pm index 1e94bb3e6..c57f7e0ed 100644 --- a/t/Mock/MapIt.pm +++ b/t/Mock/MapIt.pm @@ -23,6 +23,7 @@ sub output { } my @PLACES = ( + [ '?', 53.387402, -2.943997, 2527, 'Liverpool City Council', 'MTD' ], [ 'EH1 1BB', 55.952055, -3.189579, 2651, 'Edinburgh City Council', 'UTA', 20728, 'City Centre', 'UTE' ], [ 'BS10 5EE', 51.494885, -2.602237, 2561, 'Bristol City Council', 'UTA', 148646, 'Bedminster', 'UTW' ], [ 'SW1A 1AA', 51.501009, -0.141588, 2504, 'Westminster City Council', 'LBO' ], diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t index 0e7547a85..e12391804 100644 --- a/t/app/controller/report_new.t +++ b/t/app/controller/report_new.t @@ -1169,6 +1169,7 @@ FixMyStreet::override_config { $extra_details = $mech->get_ok_json( '/report/new/ajax?latitude=' . $saved_lat . '&longitude=' . $saved_lon ); }; $mech->content_contains( "Pothol\xc3\xa9s" ); +like $extra_details->{councils_text}, qr/<strong>Cheltenham/; ok !$extra_details->{titles_list}, 'Non Bromley does not send back list of titles'; FixMyStreet::override_config { diff --git a/t/app/controller/reports.t b/t/app/controller/reports.t index 76c920562..8cdfddd1b 100644 --- a/t/app/controller/reports.t +++ b/t/app/controller/reports.t @@ -95,6 +95,9 @@ $fife_problems[10]->update( { state => 'hidden', }); +# Run the cron script old-data (for the table no longer used by default) +FixMyStreet::Script::UpdateAllReports::generate(1); + # Run the cron script that makes the data for /reports so we don't get an error. my $data = FixMyStreet::Script::UpdateAllReports::generate_dashboard(); diff --git a/t/cobrand/form_extras.t b/t/cobrand/form_extras.t index 84ded5bc1..df76ccbe1 100644 --- a/t/cobrand/form_extras.t +++ b/t/cobrand/form_extras.t @@ -2,7 +2,10 @@ package FixMyStreet::Cobrand::Tester; use parent 'FixMyStreet::Cobrand::FixMyStreet'; sub report_form_extras { - ( { name => 'address', required => 1 }, { name => 'passport', required => 0 } ) + ( + { name => 'address', required => 1 }, + { name => 'passport', required => 0, validator => sub { die "Invalid number\n" if $_[0] && $_[0] !~ /^P/; return $_[0] } }, + ) } # To allow a testing template override @@ -30,6 +33,7 @@ FixMyStreet::override_config { $mech->get_ok('/around'); $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB', } }, "submit location" ); $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" ); + $mech->submit_form_ok( { button => 'submit_register', with_fields => { @@ -38,13 +42,24 @@ FixMyStreet::override_config { name => 'Joe Bloggs', may_show_name => '1', username => 'test-1@example.com', - passport => '123456', password_register => '', } }, - "submit details without address, with passport", + "submit details without address or passport", ); $mech->content_like(qr{<label for="form_address">Address</label>\s*<p class='form-error'>This information is required</p>}, 'Address is required'); + $mech->content_lacks("<p class='form-error'>Invalid number", 'Passport is optional'); + + $mech->submit_form_ok( { + button => 'submit_register', + with_fields => { + passport => '123456', + } + }, + "submit details with bad passport", + ); + $mech->content_like(qr{<label for="form_address">Address</label>\s*<p class='form-error'>This information is required</p>}, 'Address is required'); + $mech->content_like(qr{<p class='form-error'>Invalid number}, 'Passport format wrong'); $mech->content_contains('value="123456" name="passport"', 'Passport number reshown'); $mech->submit_form_ok( { @@ -55,11 +70,23 @@ FixMyStreet::override_config { }, "submit details, now with address", ); + $mech->content_lacks('This information is required', 'Address is present'); + $mech->content_like(qr{<p class='form-error'>Invalid number}, 'Passport format wrong'); + $mech->content_contains('value="123456" name="passport"', 'Passport number reshown'); + + $mech->submit_form_ok( { + button => 'submit_register', + with_fields => { + passport => 'P123456', + } + }, + "submit details with correct passport", + ); $mech->content_contains('Now check your email'); my $problem = FixMyStreet::DB->resultset('Problem')->search({}, { order_by => '-id' })->first; is $problem->get_extra_metadata('address'), 'My address', 'Address is stored'; - is $problem->get_extra_metadata('passport'), '123456', 'Passport number is stored'; + is $problem->get_extra_metadata('passport'), 'P123456', 'Passport number is stored'; }; END { diff --git a/templates/web/base/around/index.html b/templates/web/base/around/index.html index 8f6af6225..df49bff8d 100644 --- a/templates/web/base/around/index.html +++ b/templates/web/base/around/index.html @@ -1,5 +1,5 @@ [% pre_container_extra = INCLUDE 'around/postcode_form.html' %] -[% SET bodyclass = 'frontpage fullwidthpage' ~%] +[% SET bodyclass = 'frontpage fullwidthpage aroundpage' ~%] [% INCLUDE 'header.html', title = loc('Reporting a problem') %] [% diff --git a/templates/web/base/report/new/category_extras.html b/templates/web/base/report/new/category_extras.html index e2926901c..f787b9c52 100644 --- a/templates/web/base/report/new/category_extras.html +++ b/templates/web/base/report/new/category_extras.html @@ -1,4 +1,5 @@ -[% DEFAULT list_of_names = bodies_to_list_names %] +[% SET default_list = [] %][% FOR b IN bodies_to_list.values %][% default_list.push(b.name) %][% END %] +[% DEFAULT list_of_names = default_list %] <div id="category_meta"> [%- IF unresponsive.$category %] diff --git a/templates/web/base/report/new/councils_text_all.html b/templates/web/base/report/new/councils_text_all.html index 9a11eaae6..3ea641cbf 100644 --- a/templates/web/base/report/new/councils_text_all.html +++ b/templates/web/base/report/new/councils_text_all.html @@ -1,4 +1,5 @@ -[% DEFAULT list_of_names = bodies_to_list_names %] +[% SET default_list = [] %][% FOR b IN bodies_to_list.values %][% default_list.push(b.name) %][% END %] +[% DEFAULT list_of_names = default_list %] <p> [% diff --git a/templates/web/base/report/new/form_report.html b/templates/web/base/report/new/form_report.html index 3c7012348..d6efac423 100644 --- a/templates/web/base/report/new/form_report.html +++ b/templates/web/base/report/new/form_report.html @@ -17,13 +17,7 @@ [% END %] </div> - [% DEFAULT form_title = loc('Summarise the problem') %] - <label for="form_title">[% form_title %]</label> -[% IF field_errors.title %] - <p class='form-error'>[% field_errors.title %]</p> -[% END %] - [% DEFAULT form_title_placeholder = loc('10 inch pothole on Example St, near post box') %] - <input class="form-control" type="text" value="[% report.title | html %]" name="title" id="form_title" placeholder="[% form_title_placeholder %]" required> + [% INCLUDE 'report/new/form_title.html' %] [% TRY %][% PROCESS 'report/new/after_title.html' %][% CATCH file %][% END %] @@ -54,7 +48,8 @@ [% TRY %][% PROCESS 'report/new/after_photo.html' %][% CATCH file %][% END %] - <label for="form_detail">[% loc('Explain what’s wrong') %]</label> + [% DEFAULT form_detail_label = loc('Explain what’s wrong') %] + <label for="form_detail">[% form_detail_label %]</label> [% IF field_errors.detail %] <p class='form-error'>[% field_errors.detail %]</p> [% END %] diff --git a/templates/web/base/report/new/form_title.html b/templates/web/base/report/new/form_title.html new file mode 100644 index 000000000..88996fd3f --- /dev/null +++ b/templates/web/base/report/new/form_title.html @@ -0,0 +1,7 @@ +[% DEFAULT form_title = loc('Summarise the problem') %] +<label for="form_title">[% form_title %]</label> +[% IF field_errors.title %] + <p class='form-error'>[% field_errors.title %]</p> +[% END %] +[% DEFAULT form_title_placeholder = loc('10 inch pothole on Example St, near post box') %] +<input class="form-control" type="text" value="[% report.title | html %]" name="title" id="form_title" placeholder="[% form_title_placeholder %]" required> diff --git a/templates/web/base/report/update/form_state_checkbox.html b/templates/web/base/report/update/form_state_checkbox.html new file mode 100644 index 000000000..5316affb9 --- /dev/null +++ b/templates/web/base/report/update/form_state_checkbox.html @@ -0,0 +1,17 @@ +[% IF (problem.is_fixed OR problem.is_closed) AND ((c.user_exists AND c.user.id == problem.user_id) OR alert_to_reporter) %] + + <input type="checkbox" name="reopen" id="form_reopen" value="1"[% ' checked' IF (update.mark_open || c.req.params.reopen) %]> + [% IF problem.is_closed %] + <label class="inline" for="form_reopen">[% loc('This problem is still ongoing') %]</label> + [% ELSE %] + <label class="inline" for="form_reopen">[% loc('This problem has not been fixed') %]</label> + [% END %] + +[% ELSIF !problem.is_fixed AND has_fixed_state %] + + <div class="checkbox-group"> + <input type="checkbox" name="fixed" id="form_fixed" value="1"[% ' checked' IF update.mark_fixed %]> + <label class="inline" for="form_fixed">[% loc('This problem has been fixed') %]</label> + </div> + +[% END %] diff --git a/templates/web/base/report/update/form_update.html b/templates/web/base/report/update/form_update.html index 5a1b3b602..06104c9e9 100644 --- a/templates/web/base/report/update/form_update.html +++ b/templates/web/base/report/update/form_update.html @@ -39,21 +39,5 @@ <label for="state">[% loc( 'State' ) %]</label> [% INCLUDE 'report/inspect/state_groups_select.html' %] [% ELSE %] - [% IF (problem.is_fixed OR problem.is_closed) AND ((c.user_exists AND c.user.id == problem.user_id) OR alert_to_reporter) %] - - <input type="checkbox" name="reopen" id="form_reopen" value="1"[% ' checked' IF (update.mark_open || c.req.params.reopen) %]> - [% IF problem.is_closed %] - <label class="inline" for="form_reopen">[% loc('This problem is still ongoing') %]</label> - [% ELSE %] - <label class="inline" for="form_reopen">[% loc('This problem has not been fixed') %]</label> - [% END %] - - [% ELSIF !problem.is_fixed AND has_fixed_state %] - - <div class="checkbox-group"> - <input type="checkbox" name="fixed" id="form_fixed" value="1"[% ' checked' IF update.mark_fixed %]> - <label class="inline" for="form_fixed">[% loc('This problem has been fixed') %]</label> - </div> - - [% END %] + [% INCLUDE report/update/form_state_checkbox.html %] [% END %] diff --git a/templates/web/fixamingata/report/new/top_message_none.html b/templates/web/fixamingata/report/new/top_message_none.html index 9a9141b33..641c08f47 100644 --- a/templates/web/fixamingata/report/new/top_message_none.html +++ b/templates/web/fixamingata/report/new/top_message_none.html @@ -1,15 +1,15 @@ <p> -[% IF bodies_to_list_names.size == 1 %] +[% IF bodies_to_list.size == 1 %] [% tprintf( "%s har valt att inte ta emot rapporter från FixaMinGata, utan hänvisar fel- & synpunktsrapportering till <a href='%s'>kommunens egen webbplats</a>.", - bodies_to_list_names.first, bodies_to_list_urls.first); + bodies_to_list.values.first.name, bodies_to_list.values.first.external_url); %] [% END %] [% loc("If you submit a problem here the problem will <strong>not</strong> be reported to the council."); %] -[% IF bodies_to_list_names.size != 1 %] +[% IF bodies_to_list.size != 1 %] [% 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>."), |