Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Because people with this permission administer both the site-wide and personal contact forms, we should rename this permission to 'administer contact forms' to make it less specific.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 597784-followup-D7.patch | 1.04 KB | gpk |
| #7 | 597784-followup-D7.patch | 1.04 KB | Dave Reid |
| #4 | 597784-contact-rename-admin-perm-D7.patch | 7.84 KB | Dave Reid |
| #1 | 597784-contact-rename-admin-perm-D7.patch | 7.62 KB | Dave Reid |
Comments
Comment #1
Dave ReidPatch attached for review.
Comment #2
gpk CreditAttribution: gpk commentedA patch :) yay! :)
Marked #573510: Usability: contact module permissions need clarifying as duplicate.
On reflection, now that the contact settings page has gone (#570142: Get rid of the admin/structure/contact/settings page), this means that you need 'administer users' permission to get to the contact_default_status checkbox for users' contact forms. So actually, the current 'administer site-wide contact form' is accurate I think? [Update: depends on whether this permission is also used to turn off flood control checking on users' personal contact forms.. see bottom para below.]
Also: per the other issue, the description for the 'access site-wide contact form' permission is a bit wide of the mark. Currently it says
I'd suggest
Finally, just to note that 'administer users' permission lets you control the contact_default_status checkbox and access a user's personal contact form whether or not they have enabled it; however you need 'administer site-wide contact form' permission to be freed of the flood control limit for the form. I'm in two minds as to whether this should also be 'administer users' permission.
Comment #3
Dave Reid@gpk Yeah sorry, I should have posted in that existing issue. You make some good points and I pretty much agree. I'll re-roll with some changes today.
Comment #4
Dave ReidYeah because users with the adminster contact forms permission would be able to access a flood-controlled contact form (including the personal contact forms), we should rename the permission. Plus this helps future-proof the permission incase we add more settings at some point in the future.
Summary of revised patch:
1. Rename 'administer site-wide contact form' permission to 'administer contact forms'.
2. Improved the descriptions for all current contact permissions
Comment #5
Dries CreditAttribution: Dries commentedLooks good to me. Committed to CVS HEAD.
Comment #6
gpk CreditAttribution: gpk commentedMinor comment cleanup/followup: we have a (great) double typo:
Also I reckon that admins with 'administer users' permission but without 'administer contact forms' permission should not be subject to the flood constraints, since administering users is precisely the sort of activity that might require sending messages to users via their contact form.
Comment #7
Dave Reid@gpk: Good find. Let's make a new issue for the administer users to pass flood control plz since it will add some tests. Marking this as RTBC since it just fixes spelling errors. I double checked and nothing else is misspelled in contact.module.
Comment #8
gpk CreditAttribution: gpk commentedTry this one ;) ;)
Comment #9
Dave ReidJeez. Yep, looks good.
Comment #10
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.