aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGELOG.md1
-rw-r--r--perllib/FixMyStreet/App/Controller/Report.pm11
-rw-r--r--perllib/FixMyStreet/App/Controller/Tokens.pm4
-rw-r--r--perllib/FixMyStreet/DB/Result/Problem.pm25
-rw-r--r--t/app/controller/report_display.t32
-rw-r--r--t/app/controller/report_non_public.t85
-rw-r--r--templates/email/buckinghamshire/other-reported.html2
-rw-r--r--templates/email/buckinghamshire/other-reported.txt2
-rw-r--r--templates/email/default/other-reported.html2
-rw-r--r--templates/email/default/other-reported.txt2
-rw-r--r--templates/email/fixamingata/other-reported.html2
-rw-r--r--templates/email/fixamingata/other-reported.txt2
-rw-r--r--templates/email/hounslow/other-reported.html2
-rw-r--r--templates/email/hounslow/other-reported.txt2
-rw-r--r--templates/email/tfl/other-reported.html2
-rw-r--r--templates/email/tfl/other-reported.txt2
16 files changed, 132 insertions, 46 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 12ae1e0d1..c4f7bde7a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -28,6 +28,7 @@
- Use category groups whenever category lists are shown. #2702
- Display map inline with duplicate suggestions on mobile. #2668
- Improved try again process on mobile. #2863
+ - Improve messaging/display of private reports.
- Admin improvements:
- Add new roles system, to group permissions and apply to users. #2483
- Contact form emails now include user admin links.
diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index 7168e8379..72f96013a 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -160,7 +160,16 @@ sub load_problem_or_display_error : Private {
$c->stash->{problem} = $problem;
my $permissions = $c->stash->{_permissions} = $c->forward( 'check_has_permission_to',
[ qw/report_inspect report_edit_category report_edit_priority report_mark_private / ] );
- if ( !$c->user || ($c->user->id != $problem->user->id && !($permissions->{report_inspect} || $permissions->{report_mark_private})) ) {
+
+ # If someone has clicked a unique token link in an email to them
+ my $from_email = $c->sessionid && $c->flash->{alert_to_reporter} && $c->flash->{alert_to_reporter} == $problem->id;
+
+ my $allowed = 0;
+ $allowed = 1 if $from_email;
+ $allowed = 1 if $c->user_exists && $c->user->id == $problem->user->id;
+ $allowed = 1 if $permissions->{report_inspect} || $permissions->{report_mark_private};
+
+ unless ($allowed) {
my $url = '/auth?r=report/' . $problem->id;
$c->detach(
'/page_error_403_access_denied',
diff --git a/perllib/FixMyStreet/App/Controller/Tokens.pm b/perllib/FixMyStreet/App/Controller/Tokens.pm
index 659d763de..c4e601a85 100644
--- a/perllib/FixMyStreet/App/Controller/Tokens.pm
+++ b/perllib/FixMyStreet/App/Controller/Tokens.pm
@@ -185,9 +185,7 @@ sub alert_to_reporter : Path('/R') {
my $problem = $c->model('DB::Problem')->find( { id => $problem_id } )
|| $c->detach('token_error');
- $c->detach('token_too_old') if $auth_token->created < DateTime->now->subtract( months => 1 );
-
- $c->flash->{alert_to_reporter} = 1;
+ $c->flash->{alert_to_reporter} = $problem->id;
my $report_uri = $c->cobrand->base_url_for_report( $problem ) . $problem->url;
$c->res->redirect($report_uri);
}
diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm
index f10f1f7ec..b68c228b9 100644
--- a/perllib/FixMyStreet/DB/Result/Problem.pm
+++ b/perllib/FixMyStreet/DB/Result/Problem.pm
@@ -525,6 +525,31 @@ sub tokenised_url {
return "/M/". $token->token;
}
+has view_token => (
+ is => 'ro',
+ lazy => 1,
+ default => sub {
+ my $self = shift;
+ my $token = FixMyStreet::App->model('DB::Token')->create({
+ scope => 'alert_to_reporter',
+ data => { id => $self->id }
+ });
+ },
+);
+
+=head2 view_url
+
+Return a url for this problem report that will always show it
+(even if e.g. a private report) but does not log the user in.
+
+=cut
+
+sub view_url {
+ my $self = shift;
+ return $self->url unless $self->non_public;
+ return "/R/" . $self->view_token->token;
+}
+
=head2 is_hidden
Returns 1 if the problem is in an hidden state otherwise 0.
diff --git a/t/app/controller/report_display.t b/t/app/controller/report_display.t
index 48a827a63..4bd0fc991 100644
--- a/t/app/controller/report_display.t
+++ b/t/app/controller/report_display.t
@@ -73,38 +73,6 @@ subtest "change report to hidden and check for 410 status" => sub {
ok $report->update( { state => 'confirmed' } ), 'confirm report again';
};
-subtest "change report to non_public and check for 403 status" => sub {
- ok $report->update( { non_public => 1 } ), 'make report non public';
- ok $mech->get("/report/$report_id"), "get '/report/$report_id'";
- is $mech->res->code, 403, "access denied";
- is $mech->uri->path, "/report/$report_id", "at /report/$report_id";
- $mech->content_contains('permission to do that. If you are the problem reporter');
- $mech->content_lacks('Report another problem here');
- $mech->content_lacks($report->latitude);
- $mech->content_lacks($report->longitude);
- ok $report->update( { non_public => 0 } ), 'make report public';
-};
-
-subtest "check owner of report can view non public reports" => sub {
- ok $report->update( { non_public => 1 } ), 'make report non public';
- $mech->log_in_ok( $report->user->email );
- ok $mech->get("/report/$report_id"), "get '/report/$report_id'";
- is $mech->res->code, 200, "report can be viewed";
- is $mech->uri->path, "/report/$report_id", "at /report/$report_id";
- $mech->log_out_ok;
-
- $mech->log_in_ok( $user2->email );
- ok $mech->get("/report/$report_id"), "get '/report/$report_id'";
- is $mech->res->code, 403, "access denied to user who is not report creator";
- is $mech->uri->path, "/report/$report_id", "at /report/$report_id";
- $mech->content_contains('permission to do that. If you are the problem reporter');
- $mech->content_lacks('Report another problem here');
- $mech->content_lacks($report->latitude);
- $mech->content_lacks($report->longitude);
- $mech->log_out_ok;
- ok $report->update( { non_public => 0 } ), 'make report public';
-};
-
subtest "duplicate reports are signposted correctly" => sub {
$report2->set_extra_metadata(duplicate_of => $report->id);
$report2->state('duplicate');
diff --git a/t/app/controller/report_non_public.t b/t/app/controller/report_non_public.t
new file mode 100644
index 000000000..6d52647a8
--- /dev/null
+++ b/t/app/controller/report_non_public.t
@@ -0,0 +1,85 @@
+use FixMyStreet::TestMech;
+
+# disable info logs for this test run
+FixMyStreet::App->log->disable('info');
+END { FixMyStreet::App->log->enable('info'); }
+
+my $mech = FixMyStreet::TestMech->new;
+
+my $body = $mech->create_body_ok(2237, 'Oxfordshire County Council');
+$mech->create_contact_ok( body_id => $body->id, category => 'Potholes', email => 'potholes@example.com' );
+
+my $staffuser = $mech->create_user_ok('body-user@example.net', name => 'Body User', from_body => $body->id);
+$staffuser->user_body_permissions->create({ body => $body, permission_type => 'contribute_as_another_user' });
+$staffuser->user_body_permissions->create({ body => $body, permission_type => 'report_mark_private' });
+
+my $user = $mech->create_user_ok('test@example.com', name => 'Test User');
+my $user2 = $mech->create_user_ok('test2@example.com', name => 'Other User');
+
+my ($report) = $mech->create_problems_for_body(1, $body->id, "Example", {
+ user => $user,
+ non_public => 1,
+});
+my $report_id = $report->id;
+
+subtest "check cannot view non_public report by default" => sub {
+ ok $mech->get("/report/$report_id"), "get '/report/$report_id'";
+ is $mech->res->code, 403, "access denied";
+ is $mech->uri->path, "/report/$report_id", "at /report/$report_id";
+ $mech->content_contains('permission to do that. If you are the problem reporter');
+ $mech->content_lacks('Report another problem here');
+ $mech->content_lacks($report->latitude);
+ $mech->content_lacks($report->longitude);
+};
+
+subtest "check owner of report can view non public reports" => sub {
+ $mech->log_in_ok( $report->user->email );
+ ok $mech->get("/report/$report_id"), "get '/report/$report_id'";
+ is $mech->res->code, 200, "report can be viewed";
+ is $mech->uri->path, "/report/$report_id", "at /report/$report_id";
+ $mech->log_out_ok;
+
+ $mech->log_in_ok( $user2->email );
+ ok $mech->get("/report/$report_id"), "get '/report/$report_id'";
+ is $mech->res->code, 403, "access denied to user who is not report creator";
+ is $mech->uri->path, "/report/$report_id", "at /report/$report_id";
+ $mech->content_contains('permission to do that. If you are the problem reporter');
+ $mech->content_lacks('Report another problem here');
+ $mech->content_lacks($report->latitude);
+ $mech->content_lacks($report->longitude);
+ $mech->log_out_ok;
+};
+
+subtest "Logged email working on private report" => sub {
+ FixMyStreet::override_config {
+ ALLOWED_COBRANDS => 'fixmystreet',
+ MAPIT_URL => 'http://mapit.uk/',
+ }, sub {
+ $mech->log_in_ok($staffuser->email);
+ $mech->get_ok('/report/new?latitude=51.7549262252&longitude=-1.25617899435');
+ $mech->submit_form_ok({
+ with_fields => {
+ form_as => 'another_user',
+ title => "Test Report",
+ detail => 'Test report details.',
+ category => 'Potholes',
+ name => 'Another User',
+ username => 'another@example.net',
+ non_public => 1,
+ }
+ }, "submit details");
+ };
+ $mech->content_contains('Thank you for reporting this issue');
+ my $report = FixMyStreet::DB->resultset("Problem")->search(undef, { order_by => { -desc => 'id' } })->first;
+ ok $report, "Found the report";
+ is $report->state, 'confirmed', "report is now confirmed";
+ is $report->non_public, 1;
+
+ my $email = $mech->get_email;
+ my $body = $mech->get_text_body_from_email($email);
+ my $url = $mech->get_link_from_email($email);
+ like $body, qr/Your report to Oxfordshire County Council has been logged/;
+ $mech->get_ok($url);
+};
+
+done_testing();
diff --git a/templates/email/buckinghamshire/other-reported.html b/templates/email/buckinghamshire/other-reported.html
index 36d3c5746..341df8120 100644
--- a/templates/email/buckinghamshire/other-reported.html
+++ b/templates/email/buckinghamshire/other-reported.html
@@ -23,7 +23,7 @@ using the email address associated with the report.</p>
[% TRY %][% INCLUDE '_council_reference.html' problem=report %][% CATCH file %][% END %]
[% END %]
<p style="margin: 20px auto; text-align: center">
- <a style="[% button_style %]" href="[% cobrand.base_url_for_report(report) %][% report.url %]">View my report</a>
+ <a style="[% button_style %]" href="[% cobrand.base_url_for_report(report) %][% report.view_url %]">View my report</a>
</p>
[% end_padded_box | safe %]
</th>
diff --git a/templates/email/buckinghamshire/other-reported.txt b/templates/email/buckinghamshire/other-reported.txt
index 3d633f0c2..4d354d3ed 100644
--- a/templates/email/buckinghamshire/other-reported.txt
+++ b/templates/email/buckinghamshire/other-reported.txt
@@ -17,7 +17,7 @@ associated with the report.
It is available to view at:
-[% cobrand.base_url_for_report(report) %][% report.url %]
+[% cobrand.base_url_for_report(report) %][% report.view_url %]
Your report is at the following location:
diff --git a/templates/email/default/other-reported.html b/templates/email/default/other-reported.html
index 10bfe94e3..c22bee231 100644
--- a/templates/email/default/other-reported.html
+++ b/templates/email/default/other-reported.html
@@ -27,7 +27,7 @@ of report, so it will instead be sent to [% report.body %].</p>
[% END %]
<p style="margin: 20px auto; text-align: center">
- <a style="[% button_style %]" href="[% cobrand.base_url_for_report(report) %][% report.url %]">View my report</a>
+ <a style="[% button_style %]" href="[% cobrand.base_url_for_report(report) %][% report.view_url %]">View my report</a>
</p>
[% end_padded_box | safe %]
</th>
diff --git a/templates/email/default/other-reported.txt b/templates/email/default/other-reported.txt
index 7e7aefa39..522a89b50 100644
--- a/templates/email/default/other-reported.txt
+++ b/templates/email/default/other-reported.txt
@@ -20,7 +20,7 @@ of report, so it will instead be sent to [% report.body %].
It is available to view at:
-[% cobrand.base_url_for_report(report) %][% report.url %]
+[% cobrand.base_url_for_report(report) %][% report.view_url %]
Your report has the title:
diff --git a/templates/email/fixamingata/other-reported.html b/templates/email/fixamingata/other-reported.html
index 09fe0582a..751da0cae 100644
--- a/templates/email/fixamingata/other-reported.html
+++ b/templates/email/fixamingata/other-reported.html
@@ -25,7 +25,7 @@ rapporter, så kommer rapporten istället att skickas till [% report.body %].
[% END %]
</p>
<p style="margin: 20px auto; text-align: center">
- <a style="[% button_style %]" href="[% cobrand.base_url_for_report(report) %][% report.url %]">Visa min rapport</a>
+ <a style="[% button_style %]" href="[% cobrand.base_url_for_report(report) %][% report.view_url %]">Visa min rapport</a>
</p>
[% end_padded_box | safe %]
</th>
diff --git a/templates/email/fixamingata/other-reported.txt b/templates/email/fixamingata/other-reported.txt
index 408cc4186..e9ed542d3 100644
--- a/templates/email/fixamingata/other-reported.txt
+++ b/templates/email/fixamingata/other-reported.txt
@@ -18,7 +18,7 @@ rapporter, så kommer rapporten istället att skickas till [% report.body %].
Du kan se din rapport på:
-[% cobrand.base_url_for_report(report) %][% report.url %]
+[% cobrand.base_url_for_report(report) %][% report.view_url %]
Din rapport har titeln:
diff --git a/templates/email/hounslow/other-reported.html b/templates/email/hounslow/other-reported.html
index 41b4eeb7e..b4b4faa99 100644
--- a/templates/email/hounslow/other-reported.html
+++ b/templates/email/hounslow/other-reported.html
@@ -26,7 +26,7 @@ of report, so it will instead be sent to [% report.body %].</p>
[% TRY %][% INCLUDE '_council_reference.html' problem=report %][% CATCH file %][% END %]
[% END %]
<p style="margin: 20px auto; text-align: center">
- <a style="[% button_style %]" href="[% cobrand.base_url_for_report(report) %][% report.url %]">View my report</a>
+ <a style="[% button_style %]" href="[% cobrand.base_url_for_report(report) %][% report.view_url %]">View my report</a>
</p>
[% end_padded_box | safe %]
</th>
diff --git a/templates/email/hounslow/other-reported.txt b/templates/email/hounslow/other-reported.txt
index e718ddb83..a3b7e37c8 100644
--- a/templates/email/hounslow/other-reported.txt
+++ b/templates/email/hounslow/other-reported.txt
@@ -20,7 +20,7 @@ of report, so it will instead be sent to [% report.body %].
It is available to view at:
-[% cobrand.base_url_for_report(report) %][% report.url %]
+[% cobrand.base_url_for_report(report) %][% report.view_url %]
Your report has the title:
diff --git a/templates/email/tfl/other-reported.html b/templates/email/tfl/other-reported.html
index d3a3b9e1a..89e76b303 100644
--- a/templates/email/tfl/other-reported.html
+++ b/templates/email/tfl/other-reported.html
@@ -21,7 +21,7 @@ using the email address associated with the report.</p>
[% TRY %][% INCLUDE '_council_reference.html' problem=report %][% CATCH file %][% END %]
<p style="margin: 20px auto; text-align: center">
- <a style="[% button_style %]" href="[% cobrand.base_url_for_report(report) %][% report.url %]">View my report</a>
+ <a style="[% button_style %]" href="[% cobrand.base_url_for_report(report) %][% report.view_url %]">View my report</a>
</p>
[% end_padded_box | safe %]
</th>
diff --git a/templates/email/tfl/other-reported.txt b/templates/email/tfl/other-reported.txt
index 7271991ee..e70c4eb1c 100644
--- a/templates/email/tfl/other-reported.txt
+++ b/templates/email/tfl/other-reported.txt
@@ -15,7 +15,7 @@ associated with the report.
It is available to view at:
-[% cobrand.base_url_for_report(report) %][% report.url %]
+[% cobrand.base_url_for_report(report) %][% report.view_url %]
Your report has the title:
generated by cgit v1.2.3 (git 2.25.1) at 2025-04-26 17:37:20 +0000