Adding a machine name to the administrator role leads to a change in the role id, but the according 'user_admin_role' variable doesn't get updated, which then causes some problems on the website.

CommentFileSizeAuthor
#8 1334030-role-export-admin-var.patch1.65 KBklausi
#3 1334030-role-export-admin-var.patch777 bytesklausi
#1 1334030-role-export-admin-var.patch826 bytesklausi
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Active » Needs review
FileSize
1334030-role-export-admin-var.patch826 bytes

Simple patch attached.

Steven Brown’s picture

Steven Brown CreditAttribution: Steven Brown commented

Klausi, please re-roll the patch. I committed the changes from issue #1333048 so the patch doesn't work against the current code base.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
FileSize
1334030-role-export-admin-var.patch777 bytes

Reroll.

mh86’s picture

mh86 CreditAttribution: mh86 commented

no, patch from #3 doesn't solve the problem.
This needs to be fixed in hook_user_role_insert as well, as in hook_user_role_update the check with $role->rid == variable_get('user_admin_role') will always fail, if the admin role has been inserted while the role export module was already enabled (then $role->rid has the generated id, admin role the original one).

mh86’s picture

mh86 CreditAttribution: mh86 commented
Status: Needs review » Needs work
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Needs work » Active

I don't understand - if a role is inserted and has not existed before, how can user_admin_role be already set to this role?

And of course you can fix your admin role manually on the user account settings page.

mh86’s picture

mh86 CreditAttribution: mh86 commented

If you do something like this (which happens in the standard install profile)

user_role_save($admin_role);
variable_set('user_admin_role', $admin_role->rid);

Then $admin_role->rid unfortunately still has the old role id.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Active » Needs review
FileSize
1334030-role-export-admin-var.patch1.65 KB

Ah, now I get it. We need to set the new id in the role object. And we should make sure that role_export hooks run first, so that other modules get the new role id. Patch attached.

mh86’s picture

mh86 CreditAttribution: mh86 commented
Status: Needs review » Reviewed & tested by the community

Patch from #8 works now. Thanks for taking a look at it.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Reviewed & tested by the community » Fixed

Committed, see http://drupalcode.org/project/role_export.git/commit/c8f70c5

Status: Fixed » Closed (fixed)

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