diff options
author | Matthew Somerville <matthew-github@dracos.co.uk> | 2019-03-01 17:04:50 +0000 |
---|---|---|
committer | Matthew Somerville <matthew-github@dracos.co.uk> | 2019-03-05 13:08:51 +0000 |
commit | c029ec15c481cfe153670c735a1d4ca35b3153f9 (patch) | |
tree | 774ec135781366875045eb1f218ae9f03310cc14 | |
parent | 86827c23436fef52b6b38d3fbc357fb0bf20f0c6 (diff) |
Use relative report links where possible.
On some UK council cobrands, some reports listed might not have been sent
to that council, so links to those must go to the national site. However,
using absolute URLs for all these reports means that sometimes you change
domain when you don't need to (eg. if you’re on osm.fixmystreet.com or an
aliased version of the site), which can cause confusion. State when we’re
happy to use a relative link (ie. web-facing report links, not emails, or
share links) and do that when we can.
-rw-r--r-- | CHANGELOG.md | 1 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/Default.pm | 12 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/UKCouncils.pm | 5 | ||||
-rw-r--r-- | t/app/controller/report_new.t | 4 | ||||
-rw-r--r-- | t/cobrand/bromley.t | 2 | ||||
-rw-r--r-- | templates/web/base/maps/pin.html | 2 | ||||
-rw-r--r-- | templates/web/base/report/_item.html | 2 | ||||
-rw-r--r-- | templates/web/base/report/_item_expandable.html | 4 | ||||
-rw-r--r-- | templates/web/base/tokens/confirm_problem.html | 4 | ||||
-rw-r--r-- | templates/web/zurich/tokens/confirm_problem.html | 2 |
10 files changed, 28 insertions, 10 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index abe507e60..e96252cd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ - 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 + - Use relative report links where possible. #1995 - Development improvements: - Make front page cache time configurable. - Better working of /fakemapit/ under https. diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm index 4c5d29ee5..e54d85aa7 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 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/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/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/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/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 ); %] |