--

CommentFileSizeAuthor
#8 679500-administer-perm.patch13.67 KBboombatower
#6 679500-administer-perm.patch13.66 KBboombatower
#2 679500-administer-perm.patch2.44 KBboombatower

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

To be consistent with other perms in Drupal, shouldn't it be "Administer PIFR"?

boombatower’s picture

boombatower commented
Title: Change 'pifr administer' permission to 'pifr administrator' » Change 'pifr administer' permission to 'administer pifr'
Assigned: Unassigned » boombatower
Status: Active » Needs review
StatusFileSize
new 679500-administer-perm.patch2.44 KB

Yea, that makes sense.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

It looks like some of the other permissions in that module are more similar to your previous permission name?

From the patch:

       'pifr add test client',
       'pifr manage environments',
       'pifr manage clients',
-      'pifr administer',
+      'administer pifr',
       'access administration pages',

Hmmm. I guess those are in a different module?

boombatower’s picture

boombatower commented

That is in the test "pifr_server.test".

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Yes, but presumably those are other permmissions that the test is using, that come from PIFR or a related module? I'm just saying that those other permissions are using the format "pifr whatever" rather than "whatever pifr". Also, I think most permissions don't abbreviate the module name, so it would be even better to be "administer project issue file review".

But it's your module. :)

boombatower’s picture

boombatower commented
StatusFileSize
new 679500-administer-perm.patch13.66 KB

Update the rest of the permissions the same way, and provided update path.

boombatower’s picture

boombatower commented

I think for now I'll stick with abbreviation like code user pifr.module, pifr_server.module. At somepoint I'm thinking about renaming.

boombatower’s picture

boombatower commented
StatusFileSize
new 679500-administer-perm.patch13.67 KB

Copy-paste fail, pifr_update_6204 in pifr_server.install should be pifr_server_update_6204().

boombatower’s picture

boombatower commented
Title: Change 'pifr administer' permission to 'administer pifr' » Change 'pifr administer' permission to 'administer pifr' and related
Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

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