Hello,

Thank you for this module very useful.

I will upload a patch to allow to have certain role that see the 403 page.

CommentFileSizeAuthor
#7 interdiff-2929461-7-5.txt1.11 KBgrimreaper
#7 m4032404-allow_bypass-2929461-7.patch6.02 KBgrimreaper
#5 m4032404-allow_bypass-2929461-5.patch5.99 KBgrimreaper
#2 m4032404-allow_bypass-2929461-2.patch3.41 KBgrimreaper

Comments

Grimreaper created an issue. See original summary.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » Unassigned
Status: Active » Needs review
StatusFileSize
new m4032404-allow_bypass-2929461-2.patch3.41 KB

And here is the patch.

Thanks for the review.

Status: Needs review » Needs work

The last submitted patch, 2: m4032404-allow_bypass-2929461-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: Unassigned » grimreaper
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » Unassigned
Status: Needs work » Needs review
StatusFileSize
new m4032404-allow_bypass-2929461-5.patch5.99 KB

Fixing tests.

manuel garcia’s picture

manuel garcia commented
Status: Needs review » Needs work

Thanks for the patch @Grimreaper - I suppose this could be useful in certain scenarios.

+++ b/m4032404.permissions.yml
@@ -0,0 +1,3 @@
+display_403_page:
+  title: 'Display 403 page'
+  description: 'Bypass the redirection and Display the 403 page.'

Permissions should not include underscores as far as I know.

Also, I think a better permission name would be access 403 page. If you agree then the title and description should be updated accordingly :)

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Needs work » Needs review
StatusFileSize
new m4032404-allow_bypass-2929461-7.patch6.02 KB
new interdiff-2929461-7-5.txt1.11 KB

Hello,

Thanks @Manuel Garcia for your review.

Here is a new patch with an interdiff.

Yes, permission may not have underscores such as in D7, but I prefer on my projects to have underscores, I think it highlights the fact that it is a "machine name" and not the label of the permission.

Anyway, as you have requested I changed the permission machione name to remove the underscores.

Thanks for the new review.

manuel garcia’s picture

manuel garcia commented
Status: Needs review » Reviewed & tested by the community

Thanks @Grimreaper, changes look good - I've got nothing else to complain about, so RTBC :)

skwashd’s picture

skwashd commented
Status: Reviewed & tested by the community » Fixed

Thanks for the patch. This will be included in the next release of the module.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.