As noticed by hefox in this issue: #1920342-8: Feature implementation of roles/permissions is broken we should not write roles to features code without checking whether they still exist in the database.
The current code builds the exported code solely from the data it receives from features, but this data might not correspond with the current state of the database.

Note: Please note that you will need to use this patch together with the patch from #1265168-38: Rebuild the file list properly when a feature is enabled or disabled in order to make Features work correctly for OG.

CommentFileSizeAuthor
#1 og-Do_not_export_nonexisting_roles-2021673-1.patch1.68 KBxatoo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xatoo’s picture

xatoo CreditAttribution: xatoo commented
Assigned: xatoo » Unassigned
Status: Active » Needs review
FileSize
og-Do_not_export_nonexisting_roles-2021673-1.patch1.68 KB

Status: Needs review » Needs work

The last submitted patch, og-Do_not_export_nonexisting_roles-2021673-1.patch, failed testing.

xatoo’s picture

xatoo CreditAttribution: xatoo commented
Status: Needs work » Needs review

#1: og-Do_not_export_nonexisting_roles-2021673-1.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, og-Do_not_export_nonexisting_roles-2021673-1.patch, failed testing.

jlapp’s picture

jlapp CreditAttribution: jlapp commented
Status: Needs work » Needs review

#1: og-Do_not_export_nonexisting_roles-2021673-1.patch queued for re-testing.

jlapp’s picture

jlapp CreditAttribution: jlapp commented

The patch in #1 worked perfectly for me. The issue I was experiencing was:

  • Create OG roles.
  • Export the roles to a feature.
  • Install the feature on a fresh site/database.
  • The roles were not created, although features showed that the OG roles were in the "Default" state.

I see that the patch is passing all tests now. Is there any reason this should not be committed?

xatoo’s picture

xatoo CreditAttribution: xatoo commented

Well, one reason for not committing: Somebody needs to RTCB it. :)

gbaudoin’s picture

gbaudoin CreditAttribution: gbaudoin commented
Title: [Featues] Do not export non-existing roles. » [Features] Do not export non-existing roles.
Status: Needs review » Reviewed & tested by the community

This also fixed my issue.

gbaudoin’s picture

gbaudoin CreditAttribution: gbaudoin commented
Issue summary: View changes

Added warning about the need for an extra patch

guillaumev’s picture

guillaumev CreditAttribution: guillaumev commented
Issue summary: View changes

This also fixed my issue.

aurelianzaha’s picture

aurelianzaha CreditAttribution: aurelianzaha at jobiqo - job board technology commented

Hi,

Strange, a very old patch and still not committed. the patch works for me as well..

izmeez’s picture

izmeez CreditAttribution: izmeez commented
Related issues: +#987568: Sortable roles by weight

The patch in this issue includes a hunk that is also changed in #987568: Sortable roles by weight thus they conflict with each other. Once committed the other will need to be re-rolled.