Redirect to confirmation message after POST when making report

[davea]

Fixes https://github.com/mysociety/societyworks/issues/2229

[codecov]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (085d1c2) 82.72% compared to head (fefada8) 80.36%.

❗ Current head fefada8 differs from pull request most recent head f104a16. Consider uploading reports for the commit f104a16 to get more accurate results

Files	Patch %	Lines
perllib/FixMyStreet/App/Controller/Report.pm	90.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4362      +/-   ##
==========================================
- Coverage   82.72%   80.36%   -2.36%     
==========================================
  Files         385      374      -11     
  Lines       30039    30113      +74     
  Branches     4732     4889     +157     
==========================================
- Hits        24849    24200     -649     
- Misses       3756     4473     +717     
- Partials     1434     1440       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dracos]

Good start, and I can see you've tried to deal with the non_public/unconfirmed issues, but I don't think this is going to work like this due to the leakage (actual and potential) of information. How about this, would this work - redirect to a /report/new/confirmation?token=[token] or /report/new/confirmation/[token] or whatever endpoint where token is generated as, I dunno, hmac_sha1_hex("$time-$id", secret); Then that endpoint will validate that hash and allow fetching of that report ID alone for 1 minute/30s since $time (similar to how Auth.pm does this for the CSRF token). And that can then show either template, wouldn't need to worry about state/non_public/etc, as it knows it's the report just created.

[davea]

Thanks for the suggestion of the token - that's made it much neater! I don't think any/all of the template changes are needed any more but the tests aren't happy if I just back them all out, so once I've got that sorted it'll be ready for another review.

[dracos]

Sorry, looks good, just some possible refactoring suggestions for perfection ;)

[dracos]

Sorry for not spotting the test failure before, but hopefully have diagnosed it

[dracos]

Sadly the same tests are still failing (only on coverage, so I still think it must be this part somehow? but your fix looks good. And in e.g. report_new.t surely it can't be taking a minute to create the report? But what else could it be) :-/ Guess we might need more debugging into if it's reaching that if statement, what URL it's constructed at that point, might have to push a yucky print STDERR "$cutoff $timestamp\n"; and things like that, I can't get it to happen locally at all.

[davea]

Looks like it was indeed just taking a long time...! I increased the token cutoff to 3 minutes and the coverage tests all passed that time.

[davea]

Phew, OK, think we're there now...

[dracos]

Thanks for all that :)