From 0d526e9912f9974c0fd8fa3e9a8e7ba9d7d69785 Mon Sep 17 00:00:00 2001 From: Nathaniel Catchpole Date: Fri, 21 Oct 2016 16:19:27 +0100 Subject: [PATCH] Issue #2716133 by quietone, kevinquillen, phenaproxima, mikeryan, Charlotte17: Roles are duplicated instead of updated existing roles --- .../migrate_drupal/tests/fixtures/drupal6.php | 16 +- .../src/Tests/d7/MigrateUpgrade7Test.php | 4 +- .../user/migration_templates/d6_user_role.yml | 23 ++- .../user/migration_templates/d7_user_role.yml | 23 ++- .../src/Plugin/migrate/source/d6/Role.php | 10 +- .../Kernel/Migrate/d6/MigrateUserRoleTest.php | 152 ++++++++++++++---- .../Kernel/Migrate/d7/MigrateUserRoleTest.php | 44 +++++ 7 files changed, 198 insertions(+), 74 deletions(-) diff --git a/core/modules/migrate_drupal/tests/fixtures/drupal6.php b/core/modules/migrate_drupal/tests/fixtures/drupal6.php index 347b2f2721b..9debae00868 100644 --- a/core/modules/migrate_drupal/tests/fixtures/drupal6.php +++ b/core/modules/migrate_drupal/tests/fixtures/drupal6.php @@ -42136,13 +42136,13 @@ $connection->insert('permission') ->values(array( 'pid' => '1', 'rid' => '1', - 'perm' => 'migrate test anonymous permission', + 'perm' => 'access content, migrate test anonymous permission', 'tid' => '0', )) ->values(array( 'pid' => '2', 'rid' => '2', - 'perm' => 'migrate test authenticated permission', + 'perm' => 'access comments, access content, post comments, post comments without approval, migrate test authenticated permission', 'tid' => '0', )) ->values(array( @@ -42157,18 +42157,6 @@ $connection->insert('permission') 'perm' => 'migrate test role 2 test permission, use PHP for settings, administer contact forms, skip comment approval, edit own blog content, edit any blog content, delete own blog content, delete any blog content, create forum content, delete any forum content, delete own forum content, edit any forum content, edit own forum content, administer nodes', 'tid' => '0', )) -->values(array( - 'pid' => '5', - 'rid' => '1', - 'perm' => 'access content', - 'tid' => '0', -)) -->values(array( - 'pid' => '6', - 'rid' => '2', - 'perm' => 'access comments, access content, post comments, post comments without approval', - 'tid' => '0', -)) ->execute(); $connection->schema()->createTable('profile_fields', array( diff --git a/core/modules/migrate_drupal_ui/src/Tests/d7/MigrateUpgrade7Test.php b/core/modules/migrate_drupal_ui/src/Tests/d7/MigrateUpgrade7Test.php index eef7b908267..8e299c3c4c6 100644 --- a/core/modules/migrate_drupal_ui/src/Tests/d7/MigrateUpgrade7Test.php +++ b/core/modules/migrate_drupal_ui/src/Tests/d7/MigrateUpgrade7Test.php @@ -56,13 +56,13 @@ class MigrateUpgrade7Test extends MigrateUpgradeTestBase { 'search_page' => 2, 'shortcut' => 6, 'shortcut_set' => 2, - 'action' => 18, + 'action' => 16, 'menu' => 10, 'taxonomy_term' => 18, 'taxonomy_vocabulary' => 3, 'tour' => 4, 'user' => 4, - 'user_role' => 4, + 'user_role' => 3, 'menu_link_content' => 9, 'view' => 12, 'date_format' => 11, diff --git a/core/modules/user/migration_templates/d6_user_role.yml b/core/modules/user/migration_templates/d6_user_role.yml index a97d5194fd0..1d224f48974 100644 --- a/core/modules/user/migration_templates/d6_user_role.yml +++ b/core/modules/user/migration_templates/d6_user_role.yml @@ -9,11 +9,6 @@ process: - plugin: machine_name source: name - - - plugin: dedupe_entity - entity_type: user_role - field: id - length: 32 - plugin: user_update_8002 label: name @@ -26,15 +21,15 @@ process: 'use PHP for block visibility': 'use PHP for settings' 'administer site-wide contact form': 'administer contact forms' 'post comments without approval': 'skip comment approval' - 'edit own blog entries' : 'edit own blog content' - 'edit any blog entry' : 'edit any blog content' - 'delete own blog entries' : 'delete own blog content' - 'delete any blog entry' : 'delete any blog content' - 'create forum topics' : 'create forum content' - 'delete any forum topic' : 'delete any forum content' - 'delete own forum topics' : 'delete own forum content' - 'edit any forum topic' : 'edit any forum content' - 'edit own forum topics' : 'edit own forum content' + 'edit own blog entries': 'edit own blog content' + 'edit any blog entry': 'edit any blog content' + 'delete own blog entries': 'delete own blog content' + 'delete any blog entry': 'delete any blog content' + 'create forum topics': 'create forum content' + 'delete any forum topic': 'delete any forum content' + 'delete own forum topics': 'delete own forum content' + 'edit any forum topic': 'edit any forum content' + 'edit own forum topics': 'edit own forum content' - plugin: system_update_7000 - plugin: node_update_7008 - plugin: flatten diff --git a/core/modules/user/migration_templates/d7_user_role.yml b/core/modules/user/migration_templates/d7_user_role.yml index 68c0d1168e0..5f887574e81 100644 --- a/core/modules/user/migration_templates/d7_user_role.yml +++ b/core/modules/user/migration_templates/d7_user_role.yml @@ -9,11 +9,6 @@ process: - plugin: machine_name source: name - - - plugin: dedupe_entity - entity_type: user_role - field: id - length: 32 - plugin: user_update_8002 label: name @@ -26,15 +21,15 @@ process: 'use PHP for block visibility': 'use PHP for settings' 'administer site-wide contact form': 'administer contact forms' 'post comments without approval': 'skip comment approval' - 'edit own blog entries' : 'edit own blog content' - 'edit any blog entry' : 'edit any blog content' - 'delete own blog entries' : 'delete own blog content' - 'delete any blog entry' : 'delete any blog content' - 'create forum topics' : 'create forum content' - 'delete any forum topic' : 'delete any forum content' - 'delete own forum topics' : 'delete own forum content' - 'edit any forum topic' : 'edit any forum content' - 'edit own forum topics' : 'edit own forum content' + 'edit own blog entries': 'edit own blog content' + 'edit any blog entry': 'edit any blog content' + 'delete own blog entries': 'delete own blog content' + 'delete any blog entry': 'delete any blog content' + 'create forum topics': 'create forum content' + 'delete any forum topic': 'delete any forum content' + 'delete own forum topics': 'delete own forum content' + 'edit any forum topic': 'edit any forum content' + 'edit own forum topics': 'edit own forum content' - plugin: flatten weight: weight destination: diff --git a/core/modules/user/src/Plugin/migrate/source/d6/Role.php b/core/modules/user/src/Plugin/migrate/source/d6/Role.php index f7bac99c2e1..2357c5c860e 100644 --- a/core/modules/user/src/Plugin/migrate/source/d6/Role.php +++ b/core/modules/user/src/Plugin/migrate/source/d6/Role.php @@ -69,7 +69,15 @@ class Role extends DrupalSqlBase { ->condition('rid', $rid) ->execute() ->fetchField(); - $row->setSourceProperty('permissions', explode(', ', $permissions)); + + // If a role has no permissions then set to an empty array. The role will + // be migrated and given the default D8 permissions. + if ($permissions) { + $row->setSourceProperty('permissions', explode(', ', $permissions)); + } + else { + $row->setSourceProperty('permissions', []); + } if (isset($this->filterPermissions[$rid])) { $row->setSourceProperty("filter_permissions:$rid", $this->filterPermissions[$rid]); } diff --git a/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserRoleTest.php b/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserRoleTest.php index d3f49ccfc41..a52b6a77111 100644 --- a/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserRoleTest.php +++ b/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserRoleTest.php @@ -3,7 +3,9 @@ namespace Drupal\Tests\user\Kernel\Migrate\d6; use Drupal\user\Entity\Role; +use Drupal\user\RoleInterface; use Drupal\Tests\migrate_drupal\Kernel\d6\MigrateDrupal6TestBase; +use Drupal\migrate\Plugin\MigrateIdMapInterface; /** * Upgrade user roles to user.role.*.yml. @@ -21,29 +23,69 @@ class MigrateUserRoleTest extends MigrateDrupal6TestBase { } /** - * Tests user role migration. + * Helper function to perform assertions on a user role. + * + * @param string $id + * The role ID. + * @param string[] $permissions + * An array of user permissions. + * @param int $lookupId + * The original numeric ID of the role in the source database. + * @param \Drupal\migrate\Plugin\MigrateIdMapInterface $id_map + * The map table plugin. */ - public function testUserRole() { - /** @var \Drupal\migrate\Plugin\MigrationInterface $migration */ - $id_map = $this->getMigration('d6_user_role')->getIdMap(); - $rid = 'anonymous'; - $anonymous = Role::load($rid); - $this->assertSame($rid, $anonymous->id()); - $this->assertSame(array('migrate test anonymous permission', 'use text format filtered_html'), $anonymous->getPermissions()); - $this->assertSame(array($rid), $id_map->lookupDestinationId(array(1))); - $rid = 'authenticated'; - $authenticated = Role::load($rid); - $this->assertSame($rid, $authenticated->id()); - $this->assertSame(array('migrate test authenticated permission', 'use text format filtered_html'), $authenticated->getPermissions()); - $this->assertSame(array($rid), $id_map->lookupDestinationId(array(2))); - $rid = 'migrate_test_role_1'; - $migrate_test_role_1 = Role::load($rid); - $this->assertSame($rid, $migrate_test_role_1->id()); - $this->assertSame(array('migrate test role 1 test permission', 'use text format full_html', 'use text format php_code'), $migrate_test_role_1->getPermissions()); - $this->assertSame(array($rid), $id_map->lookupDestinationId(array(3))); - $rid = 'migrate_test_role_2'; - $migrate_test_role_2 = Role::load($rid); - $this->assertSame(array( + protected function assertRole($id, array $permissions, $lookupId, MigrateIdMapInterface $id_map) { + /** @var \Drupal\user\RoleInterface $role */ + $role = Role::load($id); + $this->assertInstanceOf(RoleInterface::class, $role); + $this->assertSame($permissions, $role->getPermissions()); + $this->assertSame([[$id]], $id_map->lookupDestinationIds(['rid' => $lookupId])); + } + + /** + * Helper function to test the migration of the user roles. The user roles + * will be re-imported and the tests here will be repeated. + * + * @param \Drupal\migrate\Plugin\MigrateIdMapInterface $id_map + * The map table plugin. + */ + protected function assertRoles(MigrateIdMapInterface $id_map) { + + // The permissions for each role are found in the two tables in the Drupal 6 + // source database. One is the permission table and the other is the + // filter_format table. + $permissions = [ + // From permission table. + 'access content', + 'migrate test anonymous permission', + // From filter_format tables. + 'use text format filtered_html' + ]; + $this->assertRole('anonymous', $permissions, 1, $id_map); + + $permissions = [ + // From permission table. + 'access comments', + 'access content', + 'post comments', + 'skip comment approval', + 'migrate test authenticated permission', + // From filter_format. + 'use text format filtered_html', + ]; + $this->assertRole('authenticated', $permissions, 2, $id_map); + + $permissions = [ + // From permission table. + 'migrate test role 1 test permission', + // From filter format. + 'use text format full_html', + 'use text format php_code' + ]; + $this->assertRole('migrate_test_role_1', $permissions, 3, $id_map); + + $permissions = [ + // From permission table. 'migrate test role 2 test permission', 'use PHP for settings', 'administer contact forms', @@ -59,14 +101,66 @@ class MigrateUserRoleTest extends MigrateDrupal6TestBase { 'edit own forum content', 'administer nodes', 'access content overview', + // From filter format. 'use text format php_code', - ), $migrate_test_role_2->getPermissions()); - $this->assertSame($rid, $migrate_test_role_2->id()); - $this->assertSame(array($rid), $id_map->lookupDestinationId(array(4))); - $rid = 'migrate_test_role_3_that_is_long'; - $migrate_test_role_3 = Role::load($rid); - $this->assertSame($rid, $migrate_test_role_3->id()); - $this->assertSame(array($rid), $id_map->lookupDestinationId(array(5))); + ]; + $this->assertRole('migrate_test_role_2', $permissions, 4, $id_map); + + // The only permission for this role is a filter format. + $permissions = ['use text format php_code']; + $this->assertRole('migrate_test_role_3_that_is_longer_than_thirty_two_characters', $permissions, 5, $id_map); + } + + /** + * Tests user role migration. + */ + public function testUserRole() { + $id_map = $this->getMigration('d6_user_role')->getIdMap(); + $this->assertRoles($id_map); + + // Test there are no duplicated roles. + $roles = [ + 'anonymous1', + 'authenticated1', + 'administrator1', + 'migrate_test_role_11', + 'migrate_test_role_21', + 'migrate_test_role_3_that_is_longer_than_thirty_two_characters1' + ]; + $this->assertEmpty(Role::loadMultiple($roles)); + + // Remove the map row for the migrate_test_role_1 role and rerun the + // migration. This will re-import the migrate_test_role_1 role migration + // again. + $this->sourceDatabase->insert('role') + ->fields([ + 'rid' => 6, + 'name' => 'migrate test role 4', + ]) + ->execute(); + $this->sourceDatabase->insert('permission') + ->fields([ + 'pid' => 7, + 'rid' => 6, + 'perm' => 'access content', + 'tid' => 0, + ]) + ->execute(); + + $id_map->delete(['rid' => 3]); + + $this->executeMigration('d6_user_role'); + + // Test there are no duplicated roles. + $roles[] = 'migrate_test_role_41'; + $this->assertEmpty(Role::loadMultiple($roles)); + + // Test that the existing roles have not changed. + $this->assertRoles($id_map); + + // Test the migration of the new role, migrate_test_role_4. + $permissions = ['access content']; + $this->assertRole('migrate_test_role_4', $permissions, 6, $id_map); } } diff --git a/core/modules/user/tests/src/Kernel/Migrate/d7/MigrateUserRoleTest.php b/core/modules/user/tests/src/Kernel/Migrate/d7/MigrateUserRoleTest.php index de0d0b96a69..edd5e3c0331 100644 --- a/core/modules/user/tests/src/Kernel/Migrate/d7/MigrateUserRoleTest.php +++ b/core/modules/user/tests/src/Kernel/Migrate/d7/MigrateUserRoleTest.php @@ -56,6 +56,50 @@ class MigrateUserRoleTest extends MigrateDrupal7TestBase { $this->assertEntity('anonymous', 'anonymous user', 1); $this->assertEntity('authenticated', 'authenticated user', 2); $this->assertEntity('administrator', 'administrator', 3); + // Test there are no duplicated roles. + $roles = [ + 'anonymous1', + 'authenticated1', + 'administrator1', + ]; + $this->assertEmpty(Role::loadMultiple($roles)); + + // Remove the map row for the administrator role and rerun the migration. + // This will re-import the administrator role again. + $id_map = $this->getMigration('d7_user_role')->getIdMap(); + $id_map->delete(['rid' => 3]); + + $this->sourceDatabase->insert('role') + ->fields([ + 'rid' => 4, + 'name' => 'test role', + 'weight' => 10, + ]) + ->execute(); + $this->sourceDatabase->insert('role_permission') + ->fields([ + 'rid' => 4, + 'permission' => 'access content', + 'module' => 'node', + ]) + ->execute(); + $this->executeMigration('d7_user_role'); + + // Test there are no duplicated roles. + $roles = [ + 'anonymous1', + 'authenticated1', + 'administrator1', + ]; + $this->assertEmpty(Role::loadMultiple($roles)); + + // Test that the existing roles have not changed. + $this->assertEntity('administrator', 'administrator', 3); + $this->assertEntity('anonymous', 'anonymous user', 1); + $this->assertEntity('authenticated', 'authenticated user', 2); + + // Test the migration of the new role, test role. + $this->assertEntity('test_role', 'test role', 4); } }