I will supply a patch that implements a plugin for the og_migrate module to migrate permissions from system-wide table for legacy drupal6 OGUR roles to newly created og_roles. This makes some assumptions:

1. In drupal6, the roles were system wide roles, so the permissions will be applied across all og bundles.
2. In drupal7 there are two hard coded og roles ADMINISTRATOR MEMBER and MEMBER that will replace any "configured" roles from drupal6.
3. This plugin depends on og_7200_ogur plugin.

Should this functionality reside in the og_7200_ogur plugin instead? According to, http://drupal.org/node/967686 , there was a change in $op where edit was changed to update for permissions. Not sure if my approach is best to accomodate this.

I'll upload my patch once the issue is created and I have my numbers. Thanks.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pgillis’s picture

Status: Active » Needs review
FileSize
4.43 KB
amitaibu’s picture

Status: Needs review » Needs work
+  'access callback' => 'og_migrate_7200_access',

Lets add an access callback for OGUR that checks if those tables even exist.

+++ b/plugins/og_migrate/7200/og_7200_ogur_permissions.incundefined
@@ -0,0 +1,115 @@
+      ->orderBy('name', 'ASC')

Do we really need the order?

+++ b/plugins/og_migrate/7200/og_7200_ogur_permissions.incundefined
@@ -0,0 +1,115 @@
+      } elseif ($name == 'og_user_roles_default_role') {
+        $context['sandbox']['legacy_roles']['default'] = $value;
+      } else {

elseif and else should be in a new line.

+++ b/plugins/og_migrate/7200/og_7200_ogur_permissions.incundefined
@@ -0,0 +1,115 @@
+        $pos = strpos($name, 'og_user_roles_roles_');
+        if ($pos == 0) {

This can be in oneline strpos(...) === 0

+++ b/plugins/og_migrate/7200/og_7200_ogur_permissions.incundefined
@@ -0,0 +1,115 @@
+    $count_query = db_select('role_permission', 'rp')
+      ->condition('rid', $context['sandbox']['legacy_roles'], 'IN');
+    $context['sandbox']['max'] = $count_query->countQuery()->execute()->fetchField();

Let's chain it in separate lines.

->countQuery()
->execute()
...

+++ b/plugins/og_migrate/7200/og_7200_ogur_permissions.incundefined
@@ -0,0 +1,115 @@
+      $context['sandbox']['role_map'][$role->r_rid] = $role->rid;

what is this $role->r_rid?

+++ b/plugins/og_migrate/7200/og_7200_ogur_permissions.incundefined
@@ -0,0 +1,115 @@
+  foreach ($og_roles as $og) {

better call it $roles, not $og.

+++ b/plugins/og_migrate/7200/og_7200_ogur_permissions.incundefined
@@ -0,0 +1,115 @@
+    } elseif ($og->name == OG_AUTHENTICATED_ROLE) {

elseif should be line below.

+++ b/plugins/og_migrate/7200/og_7200_ogur_permissions.incundefined
@@ -0,0 +1,115 @@
+    if (strpos($permission, 'edit') !== false) {

false => FALSE

pgillis’s picture

Status: Needs work » Needs review
FileSize
5.04 KB

I believe I have addressed all your concerns from #2. I've also finally found the drupal code validator for eclipse, so hopefully no more formatting issues!

what is this $role->r_rid?

I have renamed the column legacy_rid. Does that make sense? Thanks!

Status: Needs review » Needs work

The last submitted patch, og-ogur_permissions_migration-1611834-3.patch, failed testing.

pgillis’s picture

Status: Needs work » Needs review
FileSize
5.03 KB

I need to get my git configured better!

amitaibu’s picture

Status: Needs review » Needs work

Getting closer :)

+++ b/og_migrate/og_migrate.moduleundefined
@@ -284,6 +284,13 @@ function og_migrate_7200_access() {
+  return db_table_exists('og_role') && db_table_exists('role') && db_table_exists('role_permission');

Enough to do return db_table_exists('d6_og_users_roles');

Let's also add this access callback to og_7200_ogur

+++ b/plugins/og_migrate/7200/og_7200_ogur_permissions.incundefined
@@ -0,0 +1,121 @@
+        $key = substr($name, 20);

Please add docs.

+++ b/plugins/og_migrate/7200/og_7200_ogur_permissions.incundefined
@@ -0,0 +1,121 @@
+    $count_query = db_select('role_permission', 'rp')
+      ->condition('rid', $context['sandbox']['legacy_roles'], 'IN');
+    $context['sandbox']['max'] = $count_query

No need for another variable, just populate $context['sandbox']['max'] directly

+++ b/plugins/og_migrate/7200/og_7200_ogur_permissions.incundefined
@@ -0,0 +1,121 @@
+      $permission = 'update' . substr($permission, 4);

I think you are missing a space here, after update?

pgillis’s picture

I have incorporated your comments from #6.

Please add docs.

I have added documentation for the code that populates list of legacy roles, I think what I had previously was going to potentially miss some roles.

I think you are missing a space here, after update?

I don't need a space because it is already in the existing permissions. If you think it would be clearer I could add the space and pass 5 to substr.

pgillis’s picture

Status: Needs work » Needs review
amitaibu’s picture

Status: Needs review » Fixed

I've cleaned a bit (weird line breaks), and committed -- thanks!

pgillis’s picture

Status: Fixed » Closed (fixed)