diff --git a/core/includes/bootstrap.inc b/core/includes/bootstrap.inc index 1f4b97e..ca2e935 100644 --- a/core/includes/bootstrap.inc +++ b/core/includes/bootstrap.inc @@ -149,12 +149,12 @@ const DRUPAL_BOOTSTRAP_FULL = 7; /** * Role ID for anonymous users; should match what's in the "role" table. */ -const DRUPAL_ANONYMOUS_RID = 1; +const DRUPAL_ANONYMOUS_RID = 'anonymous'; /** * Role ID for authenticated users; should match what's in the "role" table. */ -const DRUPAL_AUTHENTICATED_RID = 2; +const DRUPAL_AUTHENTICATED_RID = 'authenticated'; /** * The number of bytes in a kilobyte. diff --git a/core/modules/block/block.install b/core/modules/block/block.install index 632e106..0ae303d 100644 --- a/core/modules/block/block.install +++ b/core/modules/block/block.install @@ -118,8 +118,8 @@ function block_schema() { 'description' => "The block's unique delta within module, from {block}.delta.", ), 'rid' => array( - 'type' => 'int', - 'unsigned' => TRUE, + 'type' => 'varchar', + 'length' => 64, 'not null' => TRUE, 'description' => "The user's role ID from {users_roles}.rid.", ), @@ -128,6 +128,12 @@ function block_schema() { 'indexes' => array( 'rid' => array('rid'), ), + 'foreign keys' => array( + 'role' => array( + 'table' => 'role', + 'columns' => array('rid' => 'rid'), + ), + ), ); $schema['block_custom'] = array( @@ -223,6 +229,17 @@ function block_install() { */ /** + * Implements hook_update_dependencies(). + */ +function block_update_dependencies() { + // Convert role IDs after User module converted {role}. + $dependencies['block'][8002] = array( + 'user' => 8002, + ); + return $dependencies; +} + +/** * Block cache is always enabled in 8.x. */ function block_update_8000() { @@ -267,6 +284,32 @@ function block_update_8001() { } /** + * Replace serial role IDs with machine name strings. + * + * @see user_update_8002() + */ +function block_update_8002() { + // Change serial rid column into string. + $column = array( + 'type' => 'varchar', + 'length' => 64, + 'not null' => TRUE, + 'description' => "The user's role ID from {users_roles}.rid.", + ); + db_change_field('block_role', 'rid', 'rid', $column); + + // Rename the built-in serial role IDs into the hardcoded machine names. + db_update('block_role') + ->fields(array('rid' => DRUPAL_ANONYMOUS_RID)) + ->condition('rid', 1) + ->execute(); + db_update('block_role') + ->fields(array('rid' => DRUPAL_AUTHENTICATED_RID)) + ->condition('rid', 2) + ->execute(); +} + +/** * @} End of "addtogroup updates-7.x-to-8.x". * The next series of updates should start at 9000. */ diff --git a/core/modules/field/modules/text/text.test b/core/modules/field/modules/text/text.test index a41e8cd..530aa95 100644 --- a/core/modules/field/modules/text/text.test +++ b/core/modules/field/modules/text/text.test @@ -215,7 +215,9 @@ class TextFieldTestCase extends WebTestBase { $format = filter_format_load($edit['format']); $format_id = $format->format; $permission = filter_permission_name($format); - $rid = max(array_keys($this->web_user->roles)); + $roles = $this->web_user->roles; + unset($roles[DRUPAL_AUTHENTICATED_RID]); + $rid = key($roles); user_role_grant_permissions($rid, array($permission)); $this->drupalLogin($this->web_user); diff --git a/core/modules/filter/filter.test b/core/modules/filter/filter.test index d021332..7b043ae 100644 --- a/core/modules/filter/filter.test +++ b/core/modules/filter/filter.test @@ -515,8 +515,10 @@ class FilterFormatAccessTestCase extends WebTestBase { } function testFormatRoles() { - // Get the role ID assigned to the regular user; it must be the maximum. - $rid = max(array_keys($this->web_user->roles)); + // Get the role ID assigned to the regular user. + $roles = $this->web_user->roles; + unset($roles[DRUPAL_AUTHENTICATED_RID]); + $rid = key($roles); // Check that this role appears in the list of roles that have access to an // allowed text format, but does not appear in the list of roles that have diff --git a/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php index db0f09f..4434909 100644 --- a/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php +++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php @@ -400,17 +400,24 @@ abstract class WebTestBase extends TestBase { /** * Internal helper function; Create a role with specified permissions. * - * @param $permissions + * @param array $permissions * Array of permission names to assign to role. - * @param $name - * (optional) String for the name of the role. Defaults to a random string. - * @return + * @param string $rid + * (optional) The role ID (machine name). Defaults to a random name. + * @param string $name + * (optional) The label for the role. Defaults to a random string. + * + * @return string * Role ID of newly created role, or FALSE if role creation failed. */ - protected function drupalCreateRole(array $permissions, $name = NULL) { - // Generate random name if it was not passed. - if (!$name) { - $name = $this->randomName(); + protected function drupalCreateRole(array $permissions, $rid = NULL, $name = NULL) { + // Generate a random, lowercase machine name if none was passed. + if (!isset($rid)) { + $rid = strtolower($this->randomName(8)); + } + // Generate a random label. + if (!isset($name)) { + $name = $this->randomString(8); } // Check the all the permissions strings are valid. @@ -420,14 +427,29 @@ abstract class WebTestBase extends TestBase { // Create new role. $role = new stdClass(); + $role->rid = $rid; $role->name = $name; - user_role_save($role); - user_role_grant_permissions($role->rid, $permissions); - - $this->assertTrue(isset($role->rid), t('Created role of name: @name, id: @rid', array('@name' => $name, '@rid' => (isset($role->rid) ? $role->rid : t('-n/a-')))), t('Role')); - if ($role && !empty($role->rid)) { - $count = db_query('SELECT COUNT(*) FROM {role_permission} WHERE rid = :rid', array(':rid' => $role->rid))->fetchField(); - $this->assertTrue($count == count($permissions), t('Created permissions: @perms', array('@perms' => implode(', ', $permissions))), t('Role')); + $result = user_role_save($role); + + $this->assertIdentical($result, SAVED_NEW, t('Created role ID @rid with name @name.', array( + '@name' => var_export($role->name, TRUE), + '@rid' => var_export($role->rid, TRUE), + )), t('Role')); + + if ($result === SAVED_NEW) { + // Grant the specified permissions to the role, if any. + if (!empty($permissions)) { + user_role_grant_permissions($role->rid, $permissions); + + $assigned_permissions = db_query('SELECT permission FROM {role_permission} WHERE rid = :rid', array(':rid' => $role->rid))->fetchCol(); + $missing_permissions = array_diff($permissions, $assigned_permissions); + if (!$missing_permissions) { + $this->pass(t('Created permissions: @perms', array('@perms' => implode(', ', $permissions))), t('Role')); + } + else { + $this->fail(t('Failed to create permissions: @perms', array('@perms' => implode(', ', $missing_permissions))), t('Role')); + } + } return $role->rid; } else { diff --git a/core/modules/system/system.info b/core/modules/system/system.info index 8a11985..2d89ffe 100644 --- a/core/modules/system/system.info +++ b/core/modules/system/system.info @@ -41,3 +41,4 @@ files[] = tests/uuid.test files[] = tests/xmlrpc.test files[] = tests/upgrade/upgrade.test files[] = tests/upgrade/upgrade.language.test +files[] = tests/upgrade/upgrade.roles.test diff --git a/core/modules/system/tests/upgrade/drupal-7.roles.database.php b/core/modules/system/tests/upgrade/drupal-7.roles.database.php new file mode 100644 index 0000000..a0ffe0a --- /dev/null +++ b/core/modules/system/tests/upgrade/drupal-7.roles.database.php @@ -0,0 +1,63 @@ +fields(array( + 'rid', + 'name', + 'weight', +)) +// Adds a role with an umlaut in it. +->values(array( + 'rid' => '4', + 'name' => 'gärtner', + 'weight' => '3', +)) +// Adds a very long role name. +->values(array( + 'rid' => '5', + 'name' => 'very long role name that has exactly sixty-four characters in it', + 'weight' => '4', +)) +// Adds a very similar role name to test edge cases. +->values(array( + 'rid' => '6', + 'name' => 'very_long role name that has exactly sixty-four characters in it', + 'weight' => '5', +)) +->execute(); + +// Add the "Edit own comments" permission to the gärtner test role. +db_insert('role_permission')->fields(array( + 'rid', + 'permission', + 'module', +)) +->values(array( + 'rid' => '4', + 'permission' => 'edit own comments', + 'module' => 'comment', +)) +->execute(); + +// Adds some role visibility settings on the who's online block for the long +// role. +db_insert('block_role')->fields(array( + 'module', + 'delta', + 'rid', +)) +->values(array( + 'module' => 'user', + 'delta' => 'online', + 'rid' => '5', +)) +->execute(); diff --git a/core/modules/system/tests/upgrade/upgrade.roles.test b/core/modules/system/tests/upgrade/upgrade.roles.test new file mode 100644 index 0000000..d51cacc --- /dev/null +++ b/core/modules/system/tests/upgrade/upgrade.roles.test @@ -0,0 +1,65 @@ + 'Role upgrade test', + 'description' => 'Upgrade tests with role data.', + 'group' => 'Upgrade path', + ); + } + + public function setUp() { + $this->databaseDumpFiles = array( + drupal_get_path('module', 'system') . '/tests/upgrade/drupal-7.bare.standard_all.database.php.gz', + drupal_get_path('module', 'system') . '/tests/upgrade/drupal-7.roles.database.php', + ); + parent::setUp(); + } + + /** + * Tests expected role ID conversions after a successful upgrade. + */ + public function testRoleUpgrade() { + $this->assertTrue($this->performUpgrade(), 'The upgrade was completed successfully.'); + + // Check that "gärtner" has been converted to "4" and that the role + // edit page for it exists. + $this->drupalGet('admin/people/permissions/roles/edit/4'); + $this->assertResponse(200, 'Role edit page for "gärtner" was found.'); + + // Check that the anonymous user role ID has been converted from "1" to + // "anonymous". + $this->drupalGet('admin/people/permissions/' . DRUPAL_ANONYMOUS_RID); + $this->assertResponse(200, 'Permission edit page for "anonymous" was found.'); + + // Check that the authenticated user role ID has been converted from "2" to + // "authenticated". + $this->drupalGet('admin/people/permissions/' . DRUPAL_AUTHENTICATED_RID); + $this->assertResponse(200, 'Permission edit page for "authenticated" was found.'); + + // Check that the permission for "gärtner" still exists. + $this->drupalGet('admin/people/permissions/4'); + $this->assertFieldChecked('edit-4-edit-own-comments', 'Edit own comments permission for "gärtner" is set correctly.'); + + // Check that the role visibility setting for the who's online block still + // exists. + $this->drupalGet('admin/structure/block/manage/user/online/configure'); + $this->assertFieldChecked('edit-roles-5', "Who's online block visibility setting is correctly set for the long role name."); + // Check that the role name is still displayed as expected. + $this->assertText('gärtner', 'Role name is displayed on block visibility settings.'); + $this->assertText('very long role name that has exactly sixty-four characters in it', 'Role name is displayed on block visibility settings.'); + $this->assertText('very_long role name that has exactly sixty-four characters in it', 'Role name is displayed on block visibility settings.'); + } +} diff --git a/core/modules/user/user-rtl.css b/core/modules/user/user-rtl.css index 642c943..5a1442c 100644 --- a/core/modules/user/user-rtl.css +++ b/core/modules/user/user-rtl.css @@ -4,12 +4,6 @@ padding-right: 1.5em; } -#user-admin-roles .form-item-name { - float: right; - margin-left: 1em; - margin-right: 0; -} - /** * Password strength indicator. */ diff --git a/core/modules/user/user.admin.inc b/core/modules/user/user.admin.inc index c65883a..37dcbc7 100644 --- a/core/modules/user/user.admin.inc +++ b/core/modules/user/user.admin.inc @@ -651,7 +651,7 @@ function user_admin_permissions($form, $form_state, $rid = NULL) { // Retrieve role names for columns. $role_names = user_roles(); - if (is_numeric($rid)) { + if (isset($rid)) { $role_names = array($rid => $role_names[$rid]); } // Fetch permissions for all roles or the one selected role. @@ -820,47 +820,62 @@ function theme_user_permission_description($variables) { * @see theme_user_admin_roles() */ function user_admin_roles($form, $form_state) { - $roles = user_roles(); + $roles = db_select('role', 'r') + ->addTag('translatable') + ->fields('r') + ->orderBy('weight') + ->orderBy('name') + ->execute(); $form['roles'] = array( '#tree' => TRUE, ); - $order = 0; - foreach ($roles as $rid => $name) { - $form['roles'][$rid]['#role'] = (object) array( - 'rid' => $rid, - 'name' => $name, - 'weight' => $order, + $max_weight = 0; + foreach ($roles as $role) { + $max_weight = max($max_weight, $role->weight); + $form['roles'][$role->rid]['#role'] = $role; + $form['roles'][$role->rid]['#weight'] = $role->weight; + $form['roles'][$role->rid]['name'] = array( + '#markup' => check_plain($role->name), ); - $form['roles'][$rid]['#weight'] = $order; - $form['roles'][$rid]['weight'] = array( + $form['roles'][$role->rid]['weight'] = array( '#type' => 'textfield', - '#title' => t('Weight for @title', array('@title' => $name)), + '#title' => t('Weight for @title', array('@title' => $role->name)), '#title_display' => 'invisible', '#size' => 4, - '#default_value' => $order, + '#default_value' => $role->weight, '#attributes' => array('class' => array('role-weight')), ); - $order++; + $form['roles'][$role->rid]['edit'] = array( + '#type' => 'link', + '#title' => t('edit role'), + '#href' => 'admin/people/permissions/roles/edit/' . $role->rid, + ); + $form['roles'][$role->rid]['permissions'] = array( + '#type' => 'link', + '#title' => t('edit permissions'), + '#href' => 'admin/people/permissions/' . $role->rid, + ); } - $form['name'] = array( - '#type' => 'textfield', - '#title' => t('Name'), - '#title_display' => 'invisible', - '#size' => 32, - '#maxlength' => 64, + // Embed the role add form. + $add_role = (object) array( + 'rid' => NULL, + 'name' => NULL, + 'weight' => $max_weight + 1, ); - $form['add'] = array( - '#type' => 'submit', - '#value' => t('Add role'), - '#validate' => array('user_admin_role_validate'), - '#submit' => array('user_admin_role_submit'), - ); - $form['actions'] = array('#type' => 'actions'); + $add_form = user_admin_role(array(), $form_state, $add_role); + $add_form['actions']['submit']['#submit'] = array('user_admin_role_submit'); + $add_form['role']['actions'] = $add_form['actions']; + unset($add_form['actions']); + $form += $add_form; + + $form['actions']['#type'] = 'actions'; $form['actions']['submit'] = array( '#type' => 'submit', '#value' => t('Save order'), + // Do not validate the add form when saving the order. + '#limit_validation_errors' => array(array('roles')), '#submit' => array('user_admin_roles_order_submit'), ); @@ -893,23 +908,27 @@ function theme_user_admin_roles($variables) { $header = array(t('Name'), t('Weight'), array('data' => t('Operations'), 'colspan' => 2)); foreach (element_children($form['roles']) as $rid) { - $name = $form['roles'][$rid]['#role']->name; $row = array(); - if (in_array($rid, array(DRUPAL_ANONYMOUS_RID, DRUPAL_AUTHENTICATED_RID))) { - $row[] = t('@name (locked)', array('@name' => $name)); - $row[] = drupal_render($form['roles'][$rid]['weight']); - $row[] = ''; - $row[] = l(t('edit permissions'), 'admin/people/permissions/' . $rid); - } - else { - $row[] = check_plain($name); - $row[] = drupal_render($form['roles'][$rid]['weight']); - $row[] = l(t('edit role'), 'admin/people/permissions/roles/edit/' . $rid); - $row[] = l(t('edit permissions'), 'admin/people/permissions/' . $rid); + foreach (element_children($form['roles'][$rid]) as $column) { + $row[] = drupal_render($form['roles'][$rid][$column]); } $rows[] = array('data' => $row, 'class' => array('draggable')); } - $rows[] = array(array('data' => drupal_render($form['name']) . drupal_render($form['add']), 'colspan' => 4, 'class' => 'edit-name')); + + // Distribute the role add form into table columns. + $form['role']['name']['#title_display'] = 'invisible'; + unset($form['role']['name']['#description']); + unset($form['role']['rid']['#description']); + + $actions = $form['role']['actions']; + unset($form['role']['actions']); + unset($form['role']['weight']); + $row = array(); + $row[] = drupal_render($form['role']); + // Empty placeholder for the weight column. + $row[] = ''; + $row[] = array('data' => drupal_render($actions), 'colspan' => 2); + $rows[] = array('data' => $row); drupal_add_tabledrag('user-roles', 'order', 'sibling', 'role-weight'); @@ -923,40 +942,48 @@ function theme_user_admin_roles($variables) { * Form to configure a single role. * * @ingroup forms - * @see user_admin_role_validate() * @see user_admin_role_submit() */ function user_admin_role($form, $form_state, $role) { - if ($role->rid == DRUPAL_ANONYMOUS_RID || $role->rid == DRUPAL_AUTHENTICATED_RID) { - drupal_goto('admin/people/permissions/roles'); - } + $form['role'] = array( + '#tree' => TRUE, + '#parents' => array('role'), + ); - // Display the edit role form. - $form['name'] = array( + $form['role']['name'] = array( '#type' => 'textfield', '#title' => t('Role name'), '#default_value' => $role->name, '#size' => 30, '#required' => TRUE, '#maxlength' => 64, - '#description' => t('The name for this role. Example: "moderator", "editorial board", "site architect".'), + '#description' => t('The name for this role. Example: "Moderator", "Editorial board", "Site architect".'), ); - $form['rid'] = array( - '#type' => 'value', - '#value' => $role->rid, + $form['role']['rid'] = array( + '#type' => 'machine_name', + '#default_value' => $role->rid, + '#required' => TRUE, + '#disabled' => !empty($role->rid), + '#size' => 30, + '#maxlength' => 64, + '#machine_name' => array( + 'exists' => 'user_role_load', + 'source' => array('role', 'name'), + ), ); - $form['weight'] = array( + $form['role']['weight'] = array( '#type' => 'value', '#value' => $role->weight, ); $form['actions'] = array('#type' => 'actions'); $form['actions']['submit'] = array( '#type' => 'submit', - '#value' => t('Save role'), + '#value' => !empty($role->rid) ? t('Save role') : t('Add role'), ); $form['actions']['delete'] = array( '#type' => 'submit', '#value' => t('Delete role'), + '#access' => !empty($role->rid) && !in_array($role->rid, array(DRUPAL_ANONYMOUS_RID, DRUPAL_AUTHENTICATED_RID)), '#submit' => array('user_admin_role_delete_submit'), ); @@ -964,49 +991,25 @@ function user_admin_role($form, $form_state, $role) { } /** - * Form validation handler for the user_admin_role() form. - */ -function user_admin_role_validate($form, &$form_state) { - if (!empty($form_state['values']['name'])) { - if ($form_state['values']['op'] == t('Save role')) { - $role = user_role_load_by_name($form_state['values']['name']); - if ($role && $role->rid != $form_state['values']['rid']) { - form_set_error('name', t('The role name %name already exists. Choose another role name.', array('%name' => $form_state['values']['name']))); - } - } - elseif ($form_state['values']['op'] == t('Add role')) { - if (user_role_load_by_name($form_state['values']['name'])) { - form_set_error('name', t('The role name %name already exists. Choose another role name.', array('%name' => $form_state['values']['name']))); - } - } - } - else { - form_set_error('name', t('You must specify a valid role name.')); - } -} - -/** * Form submit handler for the user_admin_role() form. */ function user_admin_role_submit($form, &$form_state) { - $role = (object) $form_state['values']; - if ($form_state['values']['op'] == t('Save role')) { - user_role_save($role); + $role = (object) $form_state['values']['role']; + $status = user_role_save($role); + if ($status === SAVED_UPDATED) { drupal_set_message(t('The role has been renamed.')); } - elseif ($form_state['values']['op'] == t('Add role')) { - user_role_save($role); + else { drupal_set_message(t('The role has been added.')); } $form_state['redirect'] = 'admin/people/permissions/roles'; - return; } /** * Form submit handler for the user_admin_role() form. */ function user_admin_role_delete_submit($form, &$form_state) { - $form_state['redirect'] = 'admin/people/permissions/roles/delete/' . $form_state['values']['rid']; + $form_state['redirect'] = 'admin/people/permissions/roles/delete/' . $form_state['values']['role']['rid']; } /** @@ -1024,7 +1027,7 @@ function user_admin_role_delete_confirm($form, &$form_state, $role) { * Form submit handler for user_admin_role_delete_confirm(). */ function user_admin_role_delete_confirm_submit($form, &$form_state) { - user_role_delete((int) $form_state['values']['rid']); + user_role_delete($form_state['values']['rid']); drupal_set_message(t('The role has been deleted.')); $form_state['redirect'] = 'admin/people/permissions/roles'; } diff --git a/core/modules/user/user.css b/core/modules/user/user.css index bbfeb83..866ee40 100644 --- a/core/modules/user/user.css +++ b/core/modules/user/user.css @@ -15,18 +15,6 @@ } /** - * Override default textfield float to put the "Add role" button next to - * the input textfield. - */ -#user-admin-roles td.edit-name { - clear: both; -} -#user-admin-roles .form-item-name { - float: left; /* LTR */ - margin-right: 1em; /* LTR */ -} - -/** * Password strength indicator. */ .password-strength { diff --git a/core/modules/user/user.install b/core/modules/user/user.install index b1ff272..5405442 100644 --- a/core/modules/user/user.install +++ b/core/modules/user/user.install @@ -55,8 +55,8 @@ function user_schema() { 'description' => 'Stores the permissions assigned to user roles.', 'fields' => array( 'rid' => array( - 'type' => 'int', - 'unsigned' => TRUE, + 'type' => 'varchar', + 'length' => 64, 'not null' => TRUE, 'description' => 'Foreign Key: {role}.rid.', ), @@ -81,7 +81,7 @@ function user_schema() { ), 'foreign keys' => array( 'role' => array( - 'table' => 'roles', + 'table' => 'role', 'columns' => array('rid' => 'rid'), ), ), @@ -91,17 +91,20 @@ function user_schema() { 'description' => 'Stores user roles.', 'fields' => array( 'rid' => array( - 'type' => 'serial', - 'unsigned' => TRUE, + 'type' => 'varchar', + // The role ID is often used as part of a compound index; at least MySQL + // has a maximum index length of 1000 characters (333 on utf8), so we + // limit the maximum length. + 'length' => 64, 'not null' => TRUE, 'description' => 'Primary Key: Unique role ID.', ), 'name' => array( 'type' => 'varchar', - 'length' => 64, + 'length' => 255, 'not null' => TRUE, 'default' => '', - 'description' => 'Unique role name.', + 'description' => 'Role label.', 'translatable' => TRUE, ), 'weight' => array( @@ -111,9 +114,6 @@ function user_schema() { 'description' => 'The weight of this role in listings and the user interface.', ), ), - 'unique keys' => array( - 'name' => array('name'), - ), 'primary key' => array('rid'), 'indexes' => array( 'name_weight' => array('name', 'weight'), @@ -268,10 +268,9 @@ function user_schema() { 'description' => 'Primary Key: {users}.uid for user.', ), 'rid' => array( - 'type' => 'int', - 'unsigned' => TRUE, + 'type' => 'varchar', + 'length' => 64, 'not null' => TRUE, - 'default' => 0, 'description' => 'Primary Key: {role}.rid for role.', ), ), @@ -285,7 +284,7 @@ function user_schema() { 'columns' => array('uid' => 'uid'), ), 'role' => array( - 'table' => 'roles', + 'table' => 'role', 'columns' => array('rid' => 'rid'), ), ), @@ -321,29 +320,12 @@ function user_install() { )) ->execute(); - // Built-in roles. - $rid_anonymous = db_insert('role') - ->fields(array('name' => 'anonymous user', 'weight' => 0)) + // Insert built-in roles. + db_insert('role') + ->fields(array('rid', 'name', 'weight')) + ->values(array(DRUPAL_ANONYMOUS_RID, 'anonymous user', 0)) + ->values(array(DRUPAL_AUTHENTICATED_RID, 'authenticated user', 1)) ->execute(); - $rid_authenticated = db_insert('role') - ->fields(array('name' => 'authenticated user', 'weight' => 1)) - ->execute(); - - // Sanity check to ensure the anonymous and authenticated role IDs are the - // same as the drupal defined constants. In certain situations, this will - // not be true. - if ($rid_anonymous != DRUPAL_ANONYMOUS_RID) { - db_update('role') - ->fields(array('rid' => DRUPAL_ANONYMOUS_RID)) - ->condition('rid', $rid_anonymous) - ->execute(); - } - if ($rid_authenticated != DRUPAL_AUTHENTICATED_RID) { - db_update('role') - ->fields(array('rid' => DRUPAL_AUTHENTICATED_RID)) - ->condition('rid', $rid_authenticated) - ->execute(); - } } /** @@ -399,5 +381,66 @@ function user_update_8001() { } /** + * Replace serial role IDs with machine name strings. + */ +function user_update_8002() { + // Change serial rid columns into strings. + $column = array( + 'type' => 'varchar', + 'length' => 64, + 'not null' => TRUE, + 'description' => 'Primary Key: Unique role ID.', + ); + db_change_field('role', 'rid', 'rid', $column); + + $column['description'] = 'Foreign Key: {role}.rid.'; + db_change_field('role_permission', 'rid', 'rid', $column); + + $column['description'] = 'Primary Key: {role}.rid for role.'; + db_change_field('users_roles', 'rid', 'rid', $column); + + // Enlarge the role name (label) column. + $column = array( + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + 'description' => 'Role label.', + 'translatable' => TRUE, + ); + db_change_field('role', 'name', 'name', $column); + // Remove unique index. + db_drop_unique_key('role', 'name'); + + // Rename the built-in serial role IDs into the hardcoded machine names. + db_update('role') + ->fields(array('rid' => DRUPAL_ANONYMOUS_RID)) + ->condition('rid', 1) + ->execute(); + db_update('role') + ->fields(array('rid' => DRUPAL_AUTHENTICATED_RID)) + ->condition('rid', 2) + ->execute(); + + db_update('role_permission') + ->fields(array('rid' => DRUPAL_ANONYMOUS_RID)) + ->condition('rid', 1) + ->execute(); + db_update('role_permission') + ->fields(array('rid' => DRUPAL_AUTHENTICATED_RID)) + ->condition('rid', 2) + ->execute(); + + db_update('users_roles') + ->fields(array('rid' => DRUPAL_ANONYMOUS_RID)) + ->condition('rid', 1) + ->execute(); + db_update('users_roles') + ->fields(array('rid' => DRUPAL_AUTHENTICATED_RID)) + ->condition('rid', 2) + ->execute(); +} + +/** * @} End of "addtogroup updates-7.x-to-8.x". */ diff --git a/core/modules/user/user.module b/core/modules/user/user.module index cfde270..7e1794a 100644 --- a/core/modules/user/user.module +++ b/core/modules/user/user.module @@ -1566,14 +1566,13 @@ function user_menu() { $items['admin/people/permissions/roles/edit/%user_role'] = array( 'title' => 'Edit role', 'page arguments' => array('user_admin_role', 5), - 'access callback' => 'user_role_edit_access', - 'access arguments' => array(5), + 'access arguments' => array('administer permissions'), ); $items['admin/people/permissions/roles/delete/%user_role'] = array( 'title' => 'Delete role', 'page callback' => 'drupal_get_form', 'page arguments' => array('user_admin_role_delete_confirm', 5), - 'access callback' => 'user_role_edit_access', + 'access callback' => 'user_role_delete_access', 'access arguments' => array(5), 'file' => 'user.admin.inc', ); @@ -2650,24 +2649,10 @@ function user_roles($membersonly = FALSE, $permission = NULL) { $query->innerJoin('role_permission', 'p', 'r.rid = p.rid'); $query->condition('p.permission', $permission); } - $result = $query->execute(); - - $roles = array(); - foreach ($result as $role) { - switch ($role->rid) { - // We only translate the built in role names - case DRUPAL_ANONYMOUS_RID: - if (!$membersonly) { - $roles[$role->rid] = t($role->name); - } - break; - case DRUPAL_AUTHENTICATED_RID: - $roles[$role->rid] = t($role->name); - break; - default: - $roles[$role->rid] = $role->name; - } + if ($membersonly) { + $query->condition('r.rid', DRUPAL_ANONYMOUS_RID, '!='); } + $roles = $query->execute()->fetchAllKeyed(); if (empty($permission)) { $user_roles[$cid] = $roles; @@ -2681,13 +2666,11 @@ function user_roles($membersonly = FALSE, $permission = NULL) { * Fetches a user role by role ID. * * @param $rid - * An integer representing the role ID. + * A string representing the role ID. * * @return * A fully-loaded role object if a role with the given ID exists, or FALSE * otherwise. - * - * @see user_role_load_by_name() */ function user_role_load($rid) { return db_select('role', 'r') @@ -2698,34 +2681,14 @@ function user_role_load($rid) { } /** - * Fetches a user role by role name. - * - * @param $role_name - * A string representing the role name. - * - * @return - * A fully-loaded role object if a role with the given name exists, or FALSE - * otherwise. - * - * @see user_role_load() - */ -function user_role_load_by_name($role_name) { - return db_select('role', 'r') - ->fields('r') - ->condition('name', $role_name) - ->execute() - ->fetchObject(); -} - -/** * Save a user role to the database. * * @param $role - * A role object to modify or add. If $role->rid is not specified, a new - * role will be created. + * A role object to modify or add. + * * @return * Status constant indicating if role was created or updated. - * Failure to write the user role record will return FALSE. Otherwise. + * Failure to write the user role record will return FALSE. Otherwise * SAVED_NEW or SAVED_UPDATED is returned depending on the operation * performed. */ @@ -2744,14 +2707,20 @@ function user_role_save($role) { // Let modules modify the user role before it is saved to the database. module_invoke_all('user_role_presave', $role); - if (!empty($role->rid) && $role->name) { - $status = drupal_write_record('role', $role, 'rid'); - module_invoke_all('user_role_update', $role); - } - else { + $exists = db_select('role', 'r') + ->fields('r', array('rid')) + ->condition('rid', $role->rid) + ->execute() + ->fetchAll(); + + if (empty($exists)) { $status = drupal_write_record('role', $role); module_invoke_all('user_role_insert', $role); } + else { + $status = drupal_write_record('role', $role, 'rid'); + module_invoke_all('user_role_update', $role); + } // Clear the user access cache. drupal_static_reset('user_access'); @@ -2764,15 +2733,10 @@ function user_role_save($role) { * Delete a user role from database. * * @param $role - * A string with the role name, or an integer with the role ID. + * A string with the role ID. */ function user_role_delete($role) { - if (is_int($role)) { - $role = user_role_load($role); - } - else { - $role = user_role_load_by_name($role); - } + $role = user_role_load($role); db_delete('role') ->condition('rid', $role->rid) @@ -2793,10 +2757,10 @@ function user_role_delete($role) { } /** - * Menu access callback for user role editing. + * Menu access callback for user role deletion. */ -function user_role_edit_access($role) { - // Prevent the system-defined roles from being altered or removed. +function user_role_delete_access($role) { + // Prevent the system-defined roles from being removed. if ($role->rid == DRUPAL_ANONYMOUS_RID || $role->rid == DRUPAL_AUTHENTICATED_RID) { return FALSE; } diff --git a/core/modules/user/user.permissions.js b/core/modules/user/user.permissions.js index 4b6d4c9..2707512 100644 --- a/core/modules/user/user.permissions.js +++ b/core/modules/user/user.permissions.js @@ -32,12 +32,12 @@ Drupal.behaviors.permissions = { .attr('title', Drupal.t("This permission is inherited from the authenticated user role.")) .hide(); - $table.find('input[type=checkbox]').not('.rid-2, .rid-1').addClass('real-checkbox').each(function () { + $table.find('input[type=checkbox]').not('.rid-anonymous, .rid-authenticated').addClass('real-checkbox').each(function () { $dummy.clone().insertAfter(this); }); // Initialize the authenticated user checkbox. - $table.find('input[type=checkbox].rid-2') + $table.find('input[type=checkbox].rid-authenticated') .bind('click.permissions', self.toggle) // .triggerHandler() cannot be used here, as it only affects the first // element. diff --git a/core/modules/user/user.test b/core/modules/user/user.test index f8a1d3a..4888ce2 100644 --- a/core/modules/user/user.test +++ b/core/modules/user/user.test @@ -1247,10 +1247,10 @@ class UserPermissionsTestCase extends WebTestBase { $this->admin_user = $this->drupalCreateUser(array('administer permissions', 'access user profiles', 'administer site configuration', 'administer modules', 'administer users')); - // Find the new role ID - it must be the maximum. - $all_rids = array_keys($this->admin_user->roles); - sort($all_rids); - $this->rid = array_pop($all_rids); + // Find the new role ID. + $all_rids = $this->admin_user->roles; + unset($all_rids[DRUPAL_AUTHENTICATED_RID]); + $this->rid = key($all_rids); } /** @@ -1374,7 +1374,9 @@ class UserAdminTestCase extends WebTestBase { $this->assertText($user_c->name, t('Found user C on filtered by perm admin users page')); // Filter the users by role. Grab the system-generated role name for User C. - $edit['role'] = max(array_flip($user_c->roles)); + $roles = $user_c->roles; + unset($roles[DRUPAL_AUTHENTICATED_RID]); + $edit['role'] = key($roles); $this->drupalPost('admin/people', $edit, t('Refine')); // Check if the correct users show up when filtered by role. @@ -2006,7 +2008,7 @@ class UserRoleAdminTestCase extends WebTestBase { public static function getInfo() { return array( - 'name' => 'User role administration', + 'name' => 'Role administration', 'description' => 'Test adding, editing and deleting user roles and changing role weights.', 'group' => 'User', ); @@ -2027,38 +2029,40 @@ class UserRoleAdminTestCase extends WebTestBase { // correspond to an integer, to test that the role administration pages // correctly distinguish between role names and IDs.) $role_name = '123'; - $edit = array('name' => $role_name); + $edit = array('role[name]' => $role_name, 'role[rid]' => $role_name); $this->drupalPost('admin/people/permissions/roles', $edit, t('Add role')); $this->assertText(t('The role has been added.'), t('The role has been added.')); - $role = user_role_load_by_name($role_name); + $role = user_role_load($role_name); $this->assertTrue(is_object($role), t('The role was successfully retrieved from the database.')); // Try adding a duplicate role. $this->drupalPost(NULL, $edit, t('Add role')); - $this->assertRaw(t('The role name %name already exists. Choose another role name.', array('%name' => $role_name)), t('Duplicate role warning displayed.')); + $this->assertRaw(t('The machine-readable name is already in use. It must be unique.'), t('Duplicate role warning displayed.')); // Test renaming a role. $old_name = $role_name; $role_name = '456'; - $edit = array('name' => $role_name); + $edit = array('role[name]' => $role_name); $this->drupalPost("admin/people/permissions/roles/edit/{$role->rid}", $edit, t('Save role')); $this->assertText(t('The role has been renamed.'), t('The role has been renamed.')); - $this->assertFalse(user_role_load_by_name($old_name), t('The role can no longer be retrieved from the database using its old name.')); - $this->assertTrue(is_object(user_role_load_by_name($role_name)), t('The role can be retrieved from the database using its new name.')); + $new_role = user_role_load($old_name); + $this->assertEqual($new_role->name, $role_name, 'The role name has been successfully changed.'); // Test deleting a role. $this->drupalPost("admin/people/permissions/roles/edit/{$role->rid}", NULL, t('Delete role')); $this->drupalPost(NULL, NULL, t('Delete')); $this->assertText(t('The role has been deleted.'), t('The role has been deleted')); $this->assertNoLinkByHref("admin/people/permissions/roles/edit/{$role->rid}", t('Role edit link removed.')); - $this->assertFalse(user_role_load_by_name($role_name), t('A deleted role can no longer be loaded.')); + $this->assertFalse(user_role_load($role_name), t('A deleted role can no longer be loaded.')); - // Make sure that the system-defined roles cannot be edited via the user + // Make sure that the system-defined roles can be edited via the user // interface. $this->drupalGet('admin/people/permissions/roles/edit/' . DRUPAL_ANONYMOUS_RID); - $this->assertResponse(403, t('Access denied when trying to edit the built-in anonymous role.')); + $this->assertResponse(200, 'Access granted when trying to edit the built-in anonymous role.'); + $this->assertNoText(t('Delete role'), 'Delete button for the anonymous role is not present.'); $this->drupalGet('admin/people/permissions/roles/edit/' . DRUPAL_AUTHENTICATED_RID); - $this->assertResponse(403, t('Access denied when trying to edit the built-in authenticated role.')); + $this->assertResponse(200, 'Access granted when trying to edit the built-in authenticated role.'); + $this->assertNoText(t('Delete role'), 'Delete button for the authenticated role is not present.'); } /** diff --git a/profiles/standard/standard.install b/profiles/standard/standard.install index eef8d0c..fdc5428 100644 --- a/profiles/standard/standard.install +++ b/profiles/standard/standard.install @@ -409,6 +409,8 @@ function standard_install() { // Create a default role for site administrators, with all available permissions assigned. $admin_role = new stdClass(); + $admin_role->rid = 'administrator'; + // @todo Proper human-readable label (at least ucfirst()). $admin_role->name = 'administrator'; $admin_role->weight = 2; user_role_save($admin_role);