diff options
22 files changed, 152 insertions, 72 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index f2cc00dae..9b67c782f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ while reporting, to discourage duplicate reports. #2386 - Front end improvements: - Track map state in URL to make sharing links easier. #2242 + - Default to unchecked for show name checkbox. #347 - Admin improvements: - Include moderation history in report updates. #2379 - Allow moderation to potentially change state. #2381 @@ -23,10 +24,18 @@ - Fix issue with Open311 codes starting with ‘_’. #2391 - Add parameter to URL when “Show older” clicked. #2397 - Don't ask for email on alert signup if logged in. #2402 + - Filter out hidden reports from top 5 list. #1957 + - Add space below "map page" contents on narrow screens. + - Use relative report links where possible. #1995 - Improve inline checkbox spacing. #2411 - Development improvements: - Make front page cache time configurable. - Better working of /fakemapit/ under https. + - Backwards incompatible changes: + - If you wish the default for the showname checkbox to be checked, + add `sub default_show_name { 1 }` to your cobrand file. + - The admin body and user sections have been refactored – if you have + custom templates/code, you may need to update links to those. * v2.5 (21st December 2018) - Front end improvements: diff --git a/perllib/FixMyStreet/App/Controller/Admin/ExorDefects.pm b/perllib/FixMyStreet/App/Controller/Admin/ExorDefects.pm index d965dd8f2..0026acb9c 100644 --- a/perllib/FixMyStreet/App/Controller/Admin/ExorDefects.pm +++ b/perllib/FixMyStreet/App/Controller/Admin/ExorDefects.pm @@ -5,6 +5,7 @@ use namespace::autoclean; use DateTime; use Try::Tiny; use FixMyStreet::Integrations::ExorRDI; +use FixMyStreet::DateRange; BEGIN { extends 'Catalyst::Controller'; } @@ -43,15 +44,16 @@ sub download : Path('download') : Args(0) { $c->detach( '/page_error_404_not_found', [] ); } - my $parser = DateTime::Format::Strptime->new( pattern => '%Y-%m-%d' ); - my $start_date = $parser-> parse_datetime ( $c->get_param('start_date') ); - my $end_date = $parser-> parse_datetime ( $c->get_param('end_date') ) ; - my $one_day = DateTime::Duration->new( days => 1 ); + my $range = FixMyStreet::DateRange->new( + start_date => $c->get_param('start_date'), + end_date => $c->get_param('end_date'), + parser => DateTime::Format::Strptime->new( pattern => '%Y-%m-%d' ), + ); my $params = { - start_date => $start_date, - inspection_date => $start_date, - end_date => $end_date + $one_day, + start_date => $range->start, + inspection_date => $range->start, + end_date => $range->end, user => $c->get_param('user_id'), mark_as_processed => 0, }; diff --git a/perllib/FixMyStreet/App/Controller/Dashboard.pm b/perllib/FixMyStreet/App/Controller/Dashboard.pm index 5bc82444d..a7de23ce3 100644 --- a/perllib/FixMyStreet/App/Controller/Dashboard.pm +++ b/perllib/FixMyStreet/App/Controller/Dashboard.pm @@ -7,6 +7,7 @@ use JSON::MaybeXS; use Path::Tiny; use Text::CSV; use Time::Piece; +use FixMyStreet::DateRange; BEGIN { extends 'Catalyst::Controller'; } @@ -161,16 +162,16 @@ sub construct_rs_filter : Private { $where{"$table_name.state"} = [ FixMyStreet::DB::Result::Problem->visible_states() ]; } - my $dtf = $c->model('DB')->storage->datetime_parser; - - my $start_date = $dtf->parse_datetime($c->stash->{start_date}); - $where{"$table_name.confirmed"} = { '>=', $dtf->format_datetime($start_date) }; + my $days30 = DateTime->now(time_zone => FixMyStreet->time_zone || FixMyStreet->local_time_zone)->subtract(days => 30); + $days30->truncate( to => 'day' ); - if (my $end_date = $c->stash->{end_date}) { - my $one_day = DateTime::Duration->new( days => 1 ); - $end_date = $dtf->parse_datetime($end_date) + $one_day; - $where{"$table_name.confirmed"} = [ -and => $where{"$table_name.confirmed"}, { '<', $dtf->format_datetime($end_date) } ]; - } + my $range = FixMyStreet::DateRange->new( + start_date => $c->stash->{start_date}, + start_default => $days30, + end_date => $c->stash->{end_date}, + formatter => $c->model('DB')->storage->datetime_parser, + ); + $where{"$table_name.confirmed"} = $range->sql; $c->stash->{params} = \%where; my $rs = $updates ? $c->cobrand->updates : $c->cobrand->problems; diff --git a/perllib/FixMyStreet/App/Controller/JSON.pm b/perllib/FixMyStreet/App/Controller/JSON.pm index 762e3c115..e1e135054 100644 --- a/perllib/FixMyStreet/App/Controller/JSON.pm +++ b/perllib/FixMyStreet/App/Controller/JSON.pm @@ -8,6 +8,7 @@ use JSON::MaybeXS; use DateTime; use DateTime::Format::ISO8601; use List::MoreUtils 'uniq'; +use FixMyStreet::DateRange; =head1 NAME @@ -50,16 +51,19 @@ sub problems : Local { } # convert the dates to datetimes and trap errors - my $iso8601 = DateTime::Format::ISO8601->new; - my $start_dt = eval { $iso8601->parse_datetime($start_date); }; - my $end_dt = eval { $iso8601->parse_datetime($end_date); }; - unless ( $start_dt && $end_dt ) { + my $range = FixMyStreet::DateRange->new( + start_date => $start_date, + end_date => $end_date, + parser => DateTime::Format::ISO8601->new, + formatter => $c->model('DB')->schema->storage->datetime_parser, + ); + unless ($range->start && $range->end) { $c->stash->{error} = 'Invalid dates supplied'; return; } # check that the dates are sane - if ( $start_dt > $end_dt ) { + if ($range->start >= $range->end) { $c->stash->{error} = 'Start date after end date'; return; } @@ -80,14 +84,8 @@ sub problems : Local { $date_col = 'lastupdate'; } - my $dt_parser = $c->model('DB')->schema->storage->datetime_parser; - - my $one_day = DateTime::Duration->new( days => 1 ); my $query = { - $date_col => { - '>=' => $dt_parser->format_datetime($start_dt), - '<=' => $dt_parser->format_datetime($end_dt + $one_day), - }, + $date_col => $range->sql, state => [ @state ], }; $query->{category} = $category if $category; diff --git a/perllib/FixMyStreet/Cobrand/BathNES.pm b/perllib/FixMyStreet/Cobrand/BathNES.pm index 800ca88fa..e3ae6763f 100644 --- a/perllib/FixMyStreet/Cobrand/BathNES.pm +++ b/perllib/FixMyStreet/Cobrand/BathNES.pm @@ -91,8 +91,6 @@ sub send_questionnaires { 0 } sub enable_category_groups { 1 } -sub default_show_name { 0 } - sub default_map_zoom { 3 } sub map_js_extra { diff --git a/perllib/FixMyStreet/Cobrand/Bromley.pm b/perllib/FixMyStreet/Cobrand/Bromley.pm index 6def2b2b1..950431e85 100644 --- a/perllib/FixMyStreet/Cobrand/Bromley.pm +++ b/perllib/FixMyStreet/Cobrand/Bromley.pm @@ -7,6 +7,7 @@ use utf8; use DateTime::Format::W3CDTF; use DateTime::Format::Flexible; use Try::Tiny; +use FixMyStreet::DateRange; sub council_area_id { return 2482; } sub council_area { return 'Bromley'; } @@ -45,8 +46,6 @@ sub problems_on_map_restriction { return $rs->to_body($tfl ? [ $self->body->id, $tfl->id ] : $self->body); } -sub default_show_name { 0 } - sub disambiguate_location { my $self = shift; my $string = shift; @@ -387,27 +386,17 @@ sub munge_load_and_group_problems { } # Date range - my $dtf = $c->model('DB')->storage->datetime_parser; - my $dtp = DateTime::Format::Flexible->new; my $start_default = DateTime->today(time_zone => FixMyStreet->time_zone || FixMyStreet->local_time_zone)->subtract(months => 3); $c->stash->{start_date} = $c->get_param('start_date') || $start_default->strftime('%Y-%m-%d'); $c->stash->{end_date} = $c->get_param('end_date'); - my $start_date = try { - $dtp->parse_datetime($c->stash->{start_date}, european => 1); - } catch { - $start_default; - }; - $where->{'me.confirmed'} = { '>=', $dtf->format_datetime($start_date) }; - - my $end_date = try { - $dtp->parse_datetime($c->stash->{end_date}, european => 1); - }; - if ($end_date) { - my $one_day = DateTime::Duration->new( days => 1 ); - $end_date += $one_day; - $where->{'me.confirmed'} = [ -and => $where->{'me.confirmed'}, { '<', $dtf->format_datetime($end_date) } ]; - } + my $range = FixMyStreet::DateRange->new( + start_date => $c->stash->{start_date}, + start_default => $start_default, + end_date => $c->stash->{end_date}, + formatter => $c->model('DB')->storage->datetime_parser, + ); + $where->{'me.confirmed'} = $range->sql; delete $filter->{rows}; diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm index 4c5d29ee5..53d25cec2 100644 --- a/perllib/FixMyStreet/Cobrand/Default.pm +++ b/perllib/FixMyStreet/Cobrand/Default.pm @@ -237,6 +237,18 @@ sub base_url_for_report { return $self->base_url_with_lang; } +=item relative_url_for_report + +Returns the relative base url for a report (might be different in a two-tier +county, but normally blank). Report may be an object, or a hashref. + +=cut + +sub relative_url_for_report { + my ( $self, $report ) = @_; + return ""; +} + =item base_host Return the base host for the cobranded version of the site @@ -1000,9 +1012,7 @@ Returns true if the show name checkbox should be ticked by default. =cut -sub default_show_name { - 1; -} +sub default_show_name { 0 } =item report_check_for_errors diff --git a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm index b8dc49f72..5def2bb61 100644 --- a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm +++ b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm @@ -73,8 +73,6 @@ sub default_map_zoom { return 3; } # let staff hide OCC reports sub users_can_hide { return 1; } -sub default_show_name { 0 } - sub lookup_by_ref_regex { return qr/^\s*((?:ENQ)?\d+)\s*$/; } diff --git a/perllib/FixMyStreet/Cobrand/UKCouncils.pm b/perllib/FixMyStreet/Cobrand/UKCouncils.pm index ab3d8f91b..1beafef73 100644 --- a/perllib/FixMyStreet/Cobrand/UKCouncils.pm +++ b/perllib/FixMyStreet/Cobrand/UKCouncils.pm @@ -204,6 +204,11 @@ sub base_url_for_report { } } +sub relative_url_for_report { + my ( $self, $report ) = @_; + return $self->owns_problem($report) ? "" : FixMyStreet->config('BASE_URL'); +} + sub admin_allow_user { my ( $self, $user ) = @_; return 1 if $user->is_superuser; diff --git a/perllib/FixMyStreet/DateRange.pm b/perllib/FixMyStreet/DateRange.pm new file mode 100644 index 000000000..bc4f4e1af --- /dev/null +++ b/perllib/FixMyStreet/DateRange.pm @@ -0,0 +1,72 @@ +package FixMyStreet::DateRange; + +use DateTime; +use DateTime::Format::Flexible; +use Moo; +use Try::Tiny; + +my $one_day = DateTime::Duration->new( days => 1 ); + +has start_date => ( is => 'ro' ); + +has start_default => ( is => 'ro' ); + +has end_date => ( is => 'ro' ); + +has parser => ( + is => 'ro', + default => sub { DateTime::Format::Flexible->new } +); + +has formatter => ( + is => 'lazy', + default => sub { + my $self = shift; + return $self->parser; + } +); + +sub _dt { + my ($self, $date) = @_; + my %params; + $params{european} = 1 if $self->parser->isa('DateTime::Format::Flexible'); + my $d = try { + $self->parser->parse_datetime($date, %params) + }; + return $d; +} + +sub start { + my $self = shift; + $self->_dt($self->start_date) || $self->start_default +} + +sub end { + my $self = shift; + my $d = $self->_dt($self->end_date); + $d += $one_day if $d; + return $d; +} + +sub _formatted { + my ($self, $dt) = @_; + return unless $dt; + $self->formatter->format_datetime($dt); +} + +sub start_formatted { $_[0]->_formatted($_[0]->start) } +sub end_formatted { $_[0]->_formatted($_[0]->end) } + +sub sql { + my ($self, $default) = @_; + my $sql = {}; + if (my $start = $self->start_formatted) { + $sql->{'>='} = $start; + } + if (my $end = $self->end_formatted) { + $sql->{'<'} = $end; + } + return $sql; +} + +1; diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t index d50a682cf..d9fae5fbc 100644 --- a/t/app/controller/report_new.t +++ b/t/app/controller/report_new.t @@ -1719,7 +1719,7 @@ subtest "test Hart" => sub { if ( $test->{confirm} ) { is $mech->uri->path, "/report/new"; my $base = 'www.fixmystreet.com'; - $base = "hart.fixmystreet.com" unless $test->{national}; + $base = '"' unless $test->{national}; $mech->content_contains("$base/report/" . $report->id, "links to correct site"); } else { # receive token @@ -1746,7 +1746,7 @@ subtest "test Hart" => sub { }; my $base = 'www.fixmystreet.com'; - $base = 'hart.fixmystreet.com' unless $test->{national}; + $base = '"' unless $test->{national}; $mech->content_contains( $base . '/report/' . $report->id, 'confirm page links to correct site' ); diff --git a/t/app/controller/report_new_text.t b/t/app/controller/report_new_text.t index e6f0a9017..fad7fb6ab 100644 --- a/t/app/controller/report_new_text.t +++ b/t/app/controller/report_new_text.t @@ -324,7 +324,7 @@ subtest "test report creation for a user who is logged in" => sub { { title => '', detail => '', - may_show_name => '1', + may_show_name => undef, name => 'Joe Bloggs', email => 'joe@example.net', photo1 => '', diff --git a/t/app/controller/report_updates.t b/t/app/controller/report_updates.t index 76594a74a..8ff5b4d24 100644 --- a/t/app/controller/report_updates.t +++ b/t/app/controller/report_updates.t @@ -368,7 +368,7 @@ for my $test ( initial_values => { name => '', username => '', - may_show_name => 1, + may_show_name => undef, add_alert => 1, photo1 => '', photo2 => '', @@ -393,7 +393,7 @@ for my $test ( initial_values => { name => '', username => '', - may_show_name => 1, + may_show_name => undef, add_alert => 1, photo1 => '', photo2 => '', @@ -496,7 +496,7 @@ for my $test ( initial_values => { name => '', username => '', - may_show_name => 1, + may_show_name => undef, add_alert => 1, photo1 => '', photo2 => '', diff --git a/t/cobrand/bromley.t b/t/cobrand/bromley.t index a9f9fb144..6750d3183 100644 --- a/t/cobrand/bromley.t +++ b/t/cobrand/bromley.t @@ -192,7 +192,7 @@ subtest 'check display of TfL reports' => sub { $mech->follow_link_ok({ text_regex => qr/Back to all reports/i }); }; $mech->content_like(qr{<a title="TfL Test[^>]*www.example.org[^>]*><img[^>]*grey}); - $mech->content_like(qr{<a title="Test Test[^>]*bromley.example.org[^>]*><img[^>]*yellow}); + $mech->content_like(qr{<a title="Test Test[^>]*href="/[^>]*><img[^>]*yellow}); }; subtest 'check geolocation overrides' => sub { diff --git a/templates/web/base/maps/pin.html b/templates/web/base/maps/pin.html index e2d1c0021..3a3fb4cf0 100644 --- a/templates/web/base/maps/pin.html +++ b/templates/web/base/maps/pin.html @@ -1,6 +1,6 @@ [% DEFAULT pin_style = 'top:' _ (pin.py - 64) _ 'px; left:' _ (pin.px - 24) _ 'px; position: absolute;' -%] [% IF pin.id %] -<a title="[% pin.title | html %]" href="[% c.cobrand.base_url_for_report( pin.problem ) %][% pin.problem.url %]"> +<a title="[% pin.title | html %]" href="[% c.cobrand.relative_url_for_report( pin.problem ) %][% pin.problem.url %]"> [%- END -%] <img border="0" src="[% start %][% c.cobrand.path_to_pin_icons _ 'pin-' _ pin.colour _ '.png' %]" [% IF js -%] diff --git a/templates/web/base/report/_item.html b/templates/web/base/report/_item.html index 863b87817..200c690a6 100644 --- a/templates/web/base/report/_item.html +++ b/templates/web/base/report/_item.html @@ -30,7 +30,7 @@ <li class="item-list__item item-list--reports__item [% item_extra_class %]" data-report-id="[% problem.id | html %]" data-lastupdate="[% problem.lastupdate %]" id="report-[% problem.id | html %]"> -<a href="[% c.cobrand.base_url_for_report( problem ) %][% problem.url %]"> +<a href="[% c.cobrand.relative_url_for_report( problem ) %][% problem.url %]"> [% IF problem.photo %] <img class="img" height="60" width="90" src="[% problem.photos.first.url_fp %]" alt=""> [% END %] diff --git a/templates/web/base/report/_item_expandable.html b/templates/web/base/report/_item_expandable.html index 6a4fe7191..27682012b 100644 --- a/templates/web/base/report/_item_expandable.html +++ b/templates/web/base/report/_item_expandable.html @@ -18,7 +18,7 @@ id="report-[% problem.id | html %]"> [% IF problem.photo %] - <a href="[% c.cobrand.base_url_for_report( problem ) %][% problem.url %]" class="item-list__item--expandable__hide-when-expanded"> + <a href="[% c.cobrand.relative_url_for_report( problem ) %][% problem.url %]" class="item-list__item--expandable__hide-when-expanded"> <img class="img" height="60" width="90" src="[% problem.photos.first.url_fp %]" alt=""> </a> [% END %] @@ -27,7 +27,7 @@ [% PROCESS 'report/_item_heading.html' %] [% CATCH file %] <h3> - <a href="[% c.cobrand.base_url_for_report( problem ) %][% problem.url %]"> + <a href="[% c.cobrand.relative_url_for_report( problem ) %][% problem.url %]"> [% problem.title | html %] </a> </h3> diff --git a/templates/web/base/report/_show_name_label.html b/templates/web/base/report/_show_name_label.html index 1e62b5fc0..f57ba4295 100644 --- a/templates/web/base/report/_show_name_label.html +++ b/templates/web/base/report/_show_name_label.html @@ -1,4 +1,3 @@ -[%# if there is nothing in the name field then set check box as default on form %] <div class="checkbox-group"> <input type="checkbox" name="may_show_name" id="form_may_show_name" value="1"[% ' checked' IF name_public %]> <label class="inline" for="form_may_show_name">[% loc('Show my name publicly') %]</label> diff --git a/templates/web/base/tokens/confirm_problem.html b/templates/web/base/tokens/confirm_problem.html index f74517c8a..62053b1bc 100644 --- a/templates/web/base/tokens/confirm_problem.html +++ b/templates/web/base/tokens/confirm_problem.html @@ -2,7 +2,7 @@ <div class="confirmation-header"> - <h1><a href="[% c.cobrand.base_url_for_report( report ) %][% report.url %]">[% report.title %]</a></h1> + <h1><a href="[% c.cobrand.relative_url_for_report( report ) %][% report.url %]">[% report.title %]</a></h1> [% IF c.cobrand.is_council %] [% IF c.cobrand.owns_problem( report ) %] @@ -21,7 +21,7 @@ <b>[% report.body %]</b> </p> <p> - You can follow this problem on <a href="[% c.cobrand.base_url_for_report( report ) %][% report.url %]">FixMyStreet.com</a>. + You can follow this problem on <a href="[% c.cobrand.relative_url_for_report( report ) %][% report.url %]">FixMyStreet.com</a>. </p> [% END %] diff --git a/templates/web/base/user/_anonymity.html b/templates/web/base/user/_anonymity.html index cc3630f16..ee8882dd8 100644 --- a/templates/web/base/user/_anonymity.html +++ b/templates/web/base/user/_anonymity.html @@ -1,9 +1,9 @@ [% - IF c.cobrand.default_show_name AND anonymous==''; + IF anonymous==''; IF c.user_exists; SET name_public = NOT c.user.latest_anonymity; ELSE; - SET name_public = 1; + SET name_public = c.cobrand.default_show_name; END; ELSE; SET name_public = anonymous==0; diff --git a/templates/web/bathnes/report/_show_name_label.html b/templates/web/bathnes/report/_show_name_label.html index 8e58f816a..20f28f099 100644 --- a/templates/web/bathnes/report/_show_name_label.html +++ b/templates/web/bathnes/report/_show_name_label.html @@ -1,4 +1,3 @@ -[%# if there is nothing in the name field then set check box as default on form %] <div class="checkbox-group"> <input type="checkbox" name="may_show_name" id="form_may_show_name" value="1"[% ' checked' IF name_public %]> <label class="inline" for="form_may_show_name">Tick here to show my name publicly</label> diff --git a/templates/web/zurich/tokens/confirm_problem.html b/templates/web/zurich/tokens/confirm_problem.html index d2025f124..52376adce 100644 --- a/templates/web/zurich/tokens/confirm_problem.html +++ b/templates/web/zurich/tokens/confirm_problem.html @@ -6,7 +6,7 @@ loc('You have successfully confirmed your email address.'); tprintf( loc( 'You can <a href="%s%s">view the problem on this site</a>.' ), - c.cobrand.base_url_for_report( report ), + c.cobrand.relative_url_for_report( report ), report.url ); %] |