From a5b9a4fabdc2d4ab54f83254843f78d608698df7 Mon Sep 17 00:00:00 2001 From: Juan Pablo Novillo Date: Sat, 3 Sep 2011 02:24:30 +0200 Subject: [PATCH 1/4] Issue #1259892 Moved user fields that are not username and password to a new tab called "Personal". They are not validated nor processed yet. --- modules/system/system.module | 10 ++---- modules/user/user.module | 56 +++++++--------------------------- modules/user/user.pages.inc | 68 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 51 deletions(-) diff --git a/modules/system/system.module b/modules/system/system.module index 3737af1..eb7a1f9 100644 --- a/modules/system/system.module +++ b/modules/system/system.module @@ -1943,13 +1943,11 @@ function system_custom_theme() { /** * Implements hook_form_FORM_ID_alter(). */ -function system_form_user_profile_form_alter(&$form, &$form_state) { - if ($form['#user_category'] == 'account') { - if (variable_get('configurable_timezones', 1)) { - system_user_timezone($form, $form_state); - } - return $form; +function system_form_user_personal_form_alter(&$form, &$form_state) { + if (variable_get('configurable_timezones', 1)) { + system_user_timezone($form, $form_state); } + return $form; } /** diff --git a/modules/user/user.module b/modules/user/user.module index db0591f..b6d98f2 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -1119,50 +1119,6 @@ function user_account_form(&$form, &$form_state) { '#title' => t('Notify user of new account'), '#access' => $register && $admin, ); - - // Signature. - $form['signature_settings'] = array( - '#type' => 'fieldset', - '#title' => t('Signature settings'), - '#weight' => 1, - '#access' => (!$register && variable_get('user_signatures', 0)), - ); - - $form['signature_settings']['signature'] = array( - '#type' => 'text_format', - '#title' => t('Signature'), - '#default_value' => isset($account->signature) ? $account->signature : '', - '#description' => t('Your signature will be publicly displayed at the end of your comments.'), - '#format' => isset($account->signature_format) ? $account->signature_format : NULL, - ); - - // Picture/avatar. - $form['picture'] = array( - '#type' => 'fieldset', - '#title' => t('Picture'), - '#weight' => 1, - '#access' => (!$register && variable_get('user_pictures', 0)), - ); - $form['picture']['picture'] = array( - '#type' => 'value', - '#value' => isset($account->picture) ? $account->picture : NULL, - ); - $form['picture']['picture_current'] = array( - '#markup' => theme('user_picture', array('account' => $account)), - ); - $form['picture']['picture_delete'] = array( - '#type' => 'checkbox', - '#title' => t('Delete picture'), - '#access' => !empty($account->picture->fid), - '#description' => t('Check this box to delete your current picture.'), - ); - $form['picture']['picture_upload'] = array( - '#type' => 'file', - '#title' => t('Upload picture'), - '#size' => 48, - '#description' => t('Your virtual face or picture. Pictures larger than @dimensions pixels will be scaled down.', array('@dimensions' => variable_get('user_picture_dimensions', '85x85'))) . ' ' . filter_xss_admin(variable_get('user_picture_guidelines', '')), - ); - $form['#validate'][] = 'user_validate_picture'; } /** @@ -1754,7 +1710,7 @@ function user_menu() { ); $items['user/%user/edit'] = array( - 'title' => 'Edit', + 'title' => 'User account', 'page callback' => 'drupal_get_form', 'page arguments' => array('user_profile_form', 1), 'access callback' => 'user_edit_access', @@ -1763,6 +1719,16 @@ function user_menu() { 'file' => 'user.pages.inc', ); + $items['user/%user/personal'] = array( + 'title' => 'Personal', + 'page callback' => 'drupal_get_form', + 'page arguments' => array('user_personal_form', 1), + 'access callback' => 'user_edit_access', + 'access arguments' => array(1), + 'type' => MENU_LOCAL_TASK, + 'file' => 'user.pages.inc', + ); + $items['user/%user_category/edit/account'] = array( 'title' => 'Account', 'type' => MENU_DEFAULT_LOCAL_TASK, diff --git a/modules/user/user.pages.inc b/modules/user/user.pages.inc index 09bf33b..55d0500 100644 --- a/modules/user/user.pages.inc +++ b/modules/user/user.pages.inc @@ -327,6 +327,74 @@ function user_profile_form_submit($form, &$form_state) { } /** + * Form builder; Edit personal details + * + * @see user_personal_form_validate() + * @see user_personal_form_submit() + */ +function user_personal_form($form, &$form_state, $account) { + $form['#user'] = $account; + + // Signature. + $form['signature_settings'] = array( + '#type' => 'fieldset', + '#title' => t('Signature settings'), + '#weight' => 1, + '#access' => variable_get('user_signatures', 0), + ); + + $form['signature_settings']['signature'] = array( + '#type' => 'text_format', + '#title' => t('Signature'), + '#default_value' => isset($account->signature) ? $account->signature : '', + '#description' => t('Your signature will be publicly displayed at the end of your comments.'), + '#format' => isset($account->signature_format) ? $account->signature_format : NULL, + ); + + // Picture/avatar. + $form['picture'] = array( + '#type' => 'fieldset', + '#title' => t('Picture'), + '#weight' => 1, + '#access' => variable_get('user_pictures', 0), + ); + $form['picture']['picture'] = array( + '#type' => 'value', + '#value' => isset($account->picture) ? $account->picture : NULL, + ); + $form['picture']['picture_current'] = array( + '#markup' => theme('user_picture', array('account' => $account)), + ); + $form['picture']['picture_delete'] = array( + '#type' => 'checkbox', + '#title' => t('Delete picture'), + '#access' => !empty($account->picture->fid), + '#description' => t('Check this box to delete your current picture.'), + ); + $form['picture']['picture_upload'] = array( + '#type' => 'file', + '#title' => t('Upload picture'), + '#size' => 48, + '#description' => t('Your virtual face or picture. Pictures larger than @dimensions pixels will be scaled down.', array('@dimensions' => variable_get('user_picture_dimensions', '85x85'))) . ' ' . filter_xss_admin(variable_get('user_picture_guidelines', '')), + ); + $form['#validate'][] = 'user_validate_picture'; + + return $form; +} + +/** + * Validate personal details form + */ +function user_personal_form_validate($form, &$form_state) { + entity_form_field_validate('user', $form, $form_state); +} + +/** + * Process personal details form submission + */ +function user_personal_form_submit($form, &$form_state) { +} +/** * Submit function for the 'Cancel account' button on the user edit form. */ function user_edit_cancel_submit($form, &$form_state) { -- 1.7.4.1 From 17225ce5786bd54dfc84d2fb281189dbe96e9b46 Mon Sep 17 00:00:00 2001 From: Juan Pablo Novillo Date: Sat, 3 Sep 2011 03:30:37 +0200 Subject: [PATCH 2/4] Issue #1259892 Personal tab fields are now validated and saved correctly. --- modules/user/user.module | 14 ------------- modules/user/user.pages.inc | 45 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/modules/user/user.module b/modules/user/user.module index b6d98f2..0138fc7 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -1181,20 +1181,6 @@ function user_account_form_validate($form, &$form_state) { form_set_error('mail', t('The e-mail address %email is already registered. Have you forgotten your password?', array('%email' => $form_state['values']['mail'], '@password' => url('user/password')))); } } - - // Make sure the signature isn't longer than the size of the database field. - // Signatures are disabled by default, so make sure it exists first. - if (isset($form_state['values']['signature'])) { - // Move text format for user signature into 'signature_format'. - $form_state['values']['signature_format'] = $form_state['values']['signature']['format']; - // Move text value for user signature into 'signature'. - $form_state['values']['signature'] = $form_state['values']['signature']['value']; - - $user_schema = drupal_get_schema('users'); - if (drupal_strlen($form_state['values']['signature']) > $user_schema['fields']['signature']['length']) { - form_set_error('signature', t('The signature is too long: it must be %max characters or less.', array('%max' => $user_schema['fields']['signature']['length']))); - } - } } } diff --git a/modules/user/user.pages.inc b/modules/user/user.pages.inc index 55d0500..32ccbd5 100644 --- a/modules/user/user.pages.inc +++ b/modules/user/user.pages.inc @@ -377,7 +377,16 @@ function user_personal_form($form, &$form_state, $account) { '#size' => 48, '#description' => t('Your virtual face or picture. Pictures larger than @dimensions pixels will be scaled down.', array('@dimensions' => variable_get('user_picture_dimensions', '85x85'))) . ' ' . filter_xss_admin(variable_get('user_picture_guidelines', '')), ); + + $form['#validate'][] = 'user_personal_form_validate'; $form['#validate'][] = 'user_validate_picture'; + $form['#submit'][] = 'user_personal_form_submit'; + + $form['actions'] = array('#type' => 'actions'); + $form['actions']['submit'] = array( + '#type' => 'submit', + '#value' => t('Save'), + ); return $form; } @@ -387,12 +396,48 @@ function user_personal_form($form, &$form_state, $account) { */ function user_personal_form_validate($form, &$form_state) { entity_form_field_validate('user', $form, $form_state); + // Make sure the signature isn't longer than the size of the database field. + // Signatures are disabled by default, so make sure it exists first. + if (isset($form_state['values']['signature'])) { + // Move text format for user signature into 'signature_format'. + $form_state['values']['signature_format'] = $form_state['values']['signature']['format']; + // Move text value for user signature into 'signature'. + $form_state['values']['signature'] = $form_state['values']['signature']['value']; + + $user_schema = drupal_get_schema('users'); + if (drupal_strlen($form_state['values']['signature']) > $user_schema['fields']['signature']['length']) { + form_set_error('signature', t('The signature is too long: it must be %max characters or less.', array('%max' => $user_schema['fields']['signature']['length']))); + } + } } /** * Process personal details form submission */ function user_personal_form_submit($form, &$form_state) { + $account = $form_state['complete_form']['#user']; + + // Remove unneeded values. + form_state_values_clean($form_state); + + // Before updating the account entity, keep an unchanged copy for use with + // user_save() later. This is necessary for modules implementing the user + // hooks to be able to react on changes by comparing the values of $account + // and $edit. + $account_unchanged = clone $account; + + entity_form_submit_build_entity('user', $account, $form, $form_state); + + // Populate $edit with the properties of $account, which have been edited on + // this form by taking over all values, which appear in the form values too. + $edit = array_intersect_key((array) $account, $form_state['values']); + user_save($account_unchanged, $edit); + $form_state['values']['uid'] = $account->uid; + + // Clear the page cache because pages can contain usernames and/or profile information: + cache_clear_all(); + + drupal_set_message(t('The changes have been saved.')); } /** * Submit function for the 'Cancel account' button on the user edit form. -- 1.7.4.1 From 12c1c83524a5995a578b12f4f286a6d8c5d095e0 Mon Sep 17 00:00:00 2001 From: Juan Pablo Novillo Date: Sat, 3 Sep 2011 03:52:24 +0200 Subject: [PATCH 3/4] Issue #1259892 Updated user tests. Assertions related to fields that are not username or password go to /user/*/personal instead of /user/*/edit. --- modules/user/user.test | 16 +++------------- 1 files changed, 3 insertions(+), 13 deletions(-) diff --git a/modules/user/user.test b/modules/user/user.test index 6ecbfac..f3eaf97 100644 --- a/modules/user/user.test +++ b/modules/user/user.test @@ -1082,7 +1082,7 @@ class UserPictureTestCase extends DrupalWebTestCase { function saveUserPicture($image) { $edit = array('files[picture_upload]' => drupal_realpath($image->uri)); - $this->drupalPost('user/' . $this->user->uid . '/edit', $edit, t('Save')); + $this->drupalPost('user/' . $this->user->uid . '/personal', $edit, t('Save')); // Load actual user data from database. $account = user_load($this->user->uid, TRUE); @@ -1316,9 +1316,8 @@ class UserTimeZoneFunctionalTest extends DrupalWebTestCase { // Change user time zone to Santiago time. $edit = array(); - $edit['mail'] = $web_user->mail; $edit['timezone'] = 'America/Santiago'; - $this->drupalPost("user/$web_user->uid/edit", $edit, t('Save')); + $this->drupalPost("user/$web_user->uid/personal", $edit, t('Save')); $this->assertText(t('The changes have been saved.'), t('Time zone changed to Santiago time.')); // Confirm date format and time zone. @@ -1626,11 +1625,6 @@ class UserEditTestCase extends DrupalWebTestCase { $this->drupalPost("user/$user1->uid/edit", $edit, t('Save')); $this->assertRaw(t('The name %name is already taken.', array('%name' => $edit['name']))); - // Repeat the test with user pictures enabled, which modifies the form. - variable_set('user_pictures', 1); - $this->drupalPost("user/$user1->uid/edit", $edit, t('Save')); - $this->assertRaw(t('The name %name is already taken.', array('%name' => $edit['name']))); - // Check that filling out a single password field does not validate. $edit = array(); $edit['pass[pass1]'] = ''; @@ -1715,10 +1709,6 @@ class UserSignatureTestCase extends DrupalWebTestCase { // Create a new node with comments on. $node = $this->drupalCreateNode(array('comment' => COMMENT_NODE_OPEN)); - // Verify that user signature field is not displayed on registration form. - $this->drupalGet('user/register'); - $this->assertNoText(t('Signature')); - // Log in as a regular user and create a signature. $this->drupalLogin($this->web_user); $signature_text = "

" . $this->randomName() . "

"; @@ -1726,7 +1716,7 @@ class UserSignatureTestCase extends DrupalWebTestCase { 'signature[value]' => $signature_text, 'signature[format]' => $this->plain_text_format->format, ); - $this->drupalPost('user/' . $this->web_user->uid . '/edit', $edit, t('Save')); + $this->drupalPost('user/' . $this->web_user->uid . '/personal', $edit, t('Save')); // Verify that values were stored. $this->assertFieldByName('signature[value]', $edit['signature[value]'], 'Submitted signature text found.'); -- 1.7.4.1 From b13ab36cb49ff67ab697c6ffa31591596685c755 Mon Sep 17 00:00:00 2001 From: Juan Pablo Novillo Date: Sat, 3 Sep 2011 04:25:50 +0200 Subject: [PATCH 4/4] Issue #1259892 Other tests reviewed and updated. These were adding fields to the user edit form instead of the new user personal form. --- modules/block/block.module | 52 +++++++++++++++++++-------------------- modules/block/block.test | 4 +- modules/contact/contact.module | 30 ++++++++++------------ modules/overlay/overlay.module | 32 +++++++++++------------- modules/user/user.module | 4 ++- 5 files changed, 59 insertions(+), 63 deletions(-) diff --git a/modules/block/block.module b/modules/block/block.module index 40fc646..d34344c 100644 --- a/modules/block/block.module +++ b/modules/block/block.module @@ -546,36 +546,34 @@ function block_custom_block_save($edit, $delta) { /** * Implements hook_form_FORM_ID_alter(). */ -function block_form_user_profile_form_alter(&$form, &$form_state) { - if ($form['#user_category'] == 'account') { - $account = $form['#user']; - $rids = array_keys($account->roles); - $result = db_query("SELECT DISTINCT b.* FROM {block} b LEFT JOIN {block_role} r ON b.module = r.module AND b.delta = r.delta WHERE b.status = 1 AND b.custom <> 0 AND (r.rid IN (:rids) OR r.rid IS NULL) ORDER BY b.weight, b.module", array(':rids' => $rids)); +function block_form_user_personal_form_alter(&$form, &$form_state) { + $account = $form['#user']; + $rids = array_keys($account->roles); + $result = db_query("SELECT DISTINCT b.* FROM {block} b LEFT JOIN {block_role} r ON b.module = r.module AND b.delta = r.delta WHERE b.status = 1 AND b.custom <> 0 AND (r.rid IN (:rids) OR r.rid IS NULL) ORDER BY b.weight, b.module", array(':rids' => $rids)); - $blocks = array(); - foreach ($result as $block) { - $data = module_invoke($block->module, 'block_info'); - if ($data[$block->delta]['info']) { - $blocks[$block->module][$block->delta] = array( - '#type' => 'checkbox', - '#title' => check_plain($data[$block->delta]['info']), - '#default_value' => isset($account->data['block'][$block->module][$block->delta]) ? $account->data['block'][$block->module][$block->delta] : ($block->custom == 1), - ); - } - } - // Only display the fieldset if there are any personalizable blocks. - if ($blocks) { - $form['block'] = array( - '#type' => 'fieldset', - '#title' => t('Personalize blocks'), - '#description' => t('Blocks consist of content or information that complements the main content of the page. Enable or disable optional blocks using the checkboxes below.'), - '#weight' => 3, - '#collapsible' => TRUE, - '#tree' => TRUE, + $blocks = array(); + foreach ($result as $block) { + $data = module_invoke($block->module, 'block_info'); + if ($data[$block->delta]['info']) { + $blocks[$block->module][$block->delta] = array( + '#type' => 'checkbox', + '#title' => check_plain($data[$block->delta]['info']), + '#default_value' => isset($account->data['block'][$block->module][$block->delta]) ? $account->data['block'][$block->module][$block->delta] : ($block->custom == 1), ); - $form['block'] += $blocks; } } + // Only display the fieldset if there are any personalizable blocks. + if ($blocks) { + $form['block'] = array( + '#type' => 'fieldset', + '#title' => t('Personalize blocks'), + '#description' => t('Blocks consist of content or information that complements the main content of the page. Enable or disable optional blocks using the checkboxes below.'), + '#weight' => 3, + '#collapsible' => TRUE, + '#tree' => TRUE, + ); + $form['block'] += $blocks; + } } /** diff --git a/modules/block/block.test b/modules/block/block.test index 03f3048..0cba60f 100644 --- a/modules/block/block.test +++ b/modules/block/block.test @@ -267,7 +267,7 @@ class BlockTestCase extends DrupalWebTestCase { // Disable block visibility for the admin user. $edit = array(); $edit['block[' . $block['module'] . '][' . $block['delta'] . ']'] = FALSE; - $this->drupalPost('user/' . $this->admin_user->uid . '/edit', $edit, t('Save')); + $this->drupalPost('user/' . $this->admin_user->uid . '/personal', $edit, t('Save')); $this->drupalGet(''); $this->assertNoText($block['title'], t('Block was not displayed according to per user block visibility setting.')); @@ -280,7 +280,7 @@ class BlockTestCase extends DrupalWebTestCase { // Enable block visibility for the admin user. $edit = array(); $edit['block[' . $block['module'] . '][' . $block['delta'] . ']'] = TRUE; - $this->drupalPost('user/' . $this->admin_user->uid . '/edit', $edit, t('Save')); + $this->drupalPost('user/' . $this->admin_user->uid . '/personal', $edit, t('Save')); $this->drupalGet(''); $this->assertText($block['title'], t('Block was displayed according to per user block visibility setting.')); diff --git a/modules/contact/contact.module b/modules/contact/contact.module index eaae9c6..9a66768 100644 --- a/modules/contact/contact.module +++ b/modules/contact/contact.module @@ -212,22 +212,20 @@ function contact_mail($key, &$message, $params) { * * Add the enable personal contact form to an individual user's account page. */ -function contact_form_user_profile_form_alter(&$form, &$form_state) { - if ($form['#user_category'] == 'account') { - $account = $form['#user']; - $form['contact'] = array( - '#type' => 'fieldset', - '#title' => t('Contact settings'), - '#weight' => 5, - '#collapsible' => TRUE, - ); - $form['contact']['contact'] = array( - '#type' => 'checkbox', - '#title' => t('Personal contact form'), - '#default_value' => !empty($account->data['contact']) ? $account->data['contact'] : FALSE, - '#description' => t('Allow other users to contact you via a personal contact form which keeps your e-mail address hidden. Note that some privileged users such as site administrators are still able to contact you even if you choose to disable this feature.', array('@url' => url("user/$account->uid/contact"))), - ); - } +function contact_form_user_personal_form_alter(&$form, &$form_state) { + $account = $form['#user']; + $form['contact'] = array( + '#type' => 'fieldset', + '#title' => t('Contact settings'), + '#weight' => 5, + '#collapsible' => TRUE, + ); + $form['contact']['contact'] = array( + '#type' => 'checkbox', + '#title' => t('Personal contact form'), + '#default_value' => !empty($account->data['contact']) ? $account->data['contact'] : FALSE, + '#description' => t('Allow other users to contact you via a personal contact form which keeps your e-mail address hidden. Note that some privileged users such as site administrators are still able to contact you even if you choose to disable this feature.', array('@url' => url("user/$account->uid/contact"))), + ); } /** diff --git a/modules/overlay/overlay.module b/modules/overlay/overlay.module index 9af18e1..6391327 100644 --- a/modules/overlay/overlay.module +++ b/modules/overlay/overlay.module @@ -81,24 +81,22 @@ function overlay_theme() { /** * Implements hook_form_FORM_ID_alter(). */ -function overlay_form_user_profile_form_alter(&$form, &$form_state) { - if ($form['#user_category'] == 'account') { - $account = $form['#user']; - if (user_access('access overlay', $account)) { - $form['overlay_control'] = array( - '#type' => 'fieldset', - '#title' => t('Administrative overlay'), - '#weight' => 4, - '#collapsible' => TRUE, - ); - - $form['overlay_control']['overlay'] = array( - '#type' => 'checkbox', - '#title' => t('Use the overlay for administrative pages.'), - '#description' => t('Show administrative pages on top of the page you started from.'), - '#default_value' => isset($account->data['overlay']) ? $account->data['overlay'] : 1, - ); - } +function overlay_form_user_personal_form_alter(&$form, &$form_state) { + $account = $form['#user']; + if (user_access('access overlay', $account)) { + $form['overlay_control'] = array( + '#type' => 'fieldset', + '#title' => t('Administrative overlay'), + '#weight' => 4, + '#collapsible' => TRUE, + ); + + $form['overlay_control']['overlay'] = array( + '#type' => 'checkbox', + '#title' => t('Use the overlay for administrative pages.'), + '#description' => t('Show administrative pages on top of the page you started from.'), + '#default_value' => isset($account->data['overlay']) ? $account->data['overlay'] : 1, + ); } } diff --git a/modules/user/user.module b/modules/user/user.module index 0138fc7..1076256 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -1696,7 +1696,7 @@ function user_menu() { ); $items['user/%user/edit'] = array( - 'title' => 'User account', + 'title' => 'Account', 'page callback' => 'drupal_get_form', 'page arguments' => array('user_profile_form', 1), 'access callback' => 'user_edit_access', @@ -1823,6 +1823,8 @@ function user_admin_paths() { 'user/*/cancel' => TRUE, 'user/*/edit' => TRUE, 'user/*/edit/*' => TRUE, + 'user/*/personal' => TRUE, + 'user/*/personal/*' => TRUE, ); return $paths; } -- 1.7.4.1