Redirect to comments index when deleting it from its own show page#9031
Redirect to comments index when deleting it from its own show page#9031Fivell wants to merge 1 commit into
Conversation
…page Closes #7770. Previously, deleting a comment from `/admin/comments/:id` redirected back to that same URL (via `redirect_back`), which then 404'd because the comment had just been destroyed. The existing `redirect_back` is the right behavior when the comment is deleted from the commentable resource's page (e.g. `/admin/posts/5`) -- the user wants to return there. Only the self-referencing case is broken. Check the referer against the deleted comment's own path and redirect to the comments index when they match; otherwise keep `redirect_back`. A new cucumber scenario exercises the broken path (delete from the comment show page) and asserts the user lands on the comments index.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9031 +/- ##
=======================================
Coverage 99.08% 99.08%
=======================================
Files 139 139
Lines 4040 4042 +2
=======================================
+ Hits 4003 4005 +2
Misses 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # to the comments index instead -- otherwise redirect_back | ||
| # would return to a now-404 page. Deleting from a | ||
| # commentable resource still redirects back to that resource. | ||
| if request.referer && URI(request.referer).path == admin_comment_path(id: params[:id]) |
There was a problem hiding this comment.
I would personally avoid dealing with the Referer header, even if that means living with the bug for now.
There was a problem hiding this comment.
@tagliala yes, I agree. We should avoid relying on that header. @Fivell I've been aware of this bug but didn't seem clear how to reliably resolve it. Since the issue is from the admin/comments page which is implemented internally, could we set a parameter in the request to then change the redirect behavior here only for that page?
There was a problem hiding this comment.
@javierjulio @tagliala thanks for your comments, I'll try alternative implementation this friday and let you know
Closes #7770.
Previously, deleting a comment from
/admin/comments/:idredirected back to that same URL (viaredirect_back), which then 404'd because the comment had just been destroyed.The existing
redirect_backis the right behavior when the comment is deleted from the commentable resource's page (e.g./admin/posts/5) -- the user wants to return there. Only the self-referencing case is broken.Check the referer against the deleted comment's own path and redirect to the comments index when they match; otherwise keep
redirect_back.A new cucumber scenario exercises the broken path (delete from the comment show page) and asserts the user lands on the comments index.