#118345 by sun: Make user.module conform to standard...

#118345 by sun: Make user.module conform to standard presave/insert/update/form_alter hook paradigm.

includes/form.inc

@@ -33,8 +33,8 @@
  * form for modules automatically. For example:
  *
  * @code
- * // Display the user registration form.
- * $output = drupal_get_form('user_register');
+ *   // Display the user registration form.
+ *   $output = drupal_get_form('user_register_form');
  * @endcode
  *
  * Forms can also be built and submitted programmatically without any user input
@@ -389,7 +389,7 @@ function form_set_cache($form_build_id, $form, $form_state) {
  * $form_state['values']['mail'] = 'robouser@example.com';
  * $form_state['values']['pass'] = 'password';
  * $form_state['values']['op'] = t('Create new account');
- * drupal_form_submit('user_register', $form_state);
+ * drupal_form_submit('user_register_form', $form_state);
  *
  * // Create a new node
  * $form_state = array();

modules/block/block.module

@@ -425,42 +425,33 @@ function block_form_user_profile_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));
-    $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']) {
-        $return = TRUE;
-        $form['block'][$block->module][$block->delta] = array(
+        $blocks[$block->module][$block->delta] = array(
           '#type' => 'checkbox',
           '#title' => check_plain($data[$block->delta]['info']),
           '#default_value' => isset($account->block[$block->module][$block->delta]) ? $account->block[$block->module][$block->delta] : ($block->custom == 1),
         );
       }
     }
-
-    if (!isset($return)) {
-      $form['block']['#access'] = FALSE;
+    // 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;
     }
   }
 }
 
-/**
- * Implement hook_user_validate().
- */
-function block_user_validate(&$edit, $account, $category) {
-  if (empty($edit['block'])) {
-    $edit['block'] = array();
-  }
-  return $edit;
-}
-
 /**
  * Implement hook_form_FORM_ID_alter().
  */

modules/locale/locale.module

@@ -338,7 +338,7 @@ function locale_form_alter(&$form, &$form_state, $form_id) {
   if (variable_get('language_count', 1) > 1) {
     // Display language selector when either creating a user on the admin
     // interface or editing a user account.
-    if (($form_id == 'user_register' && user_access('administer users')) || ($form_id == 'user_profile_form' && $form['#user_category'] == 'account')) {
+    if (($form_id == 'user_register_form' && user_access('administer users')) || ($form_id == 'user_profile_form' && $form['#user_category'] == 'account')) {
       locale_language_selector_form($form, $form_state, $form['#user']);
     }
   }

modules/openid/openid.module

@@ -129,7 +129,7 @@ function _openid_user_login_form_alter(&$form, &$form_state) {
  *
  * Adds OpenID login to the login forms.
  */
-function openid_form_user_register_alter(&$form, &$form_state) {
+function openid_form_user_register_form_alter(&$form, &$form_state) {
   if (isset($_SESSION['openid']['values'])) {
     // We were unable to auto-register a new user. Prefill the registration
     // form with the values we have.
@@ -437,9 +437,9 @@ function openid_authentication($response) {
     $form_state['values']['pass']  = user_password();
     $form_state['values']['status'] = variable_get('user_register', 1) == 1;
     $form_state['values']['response'] = $response;
-    $form = drupal_retrieve_form('user_register', $form_state);
-    drupal_prepare_form('user_register', $form, $form_state);
-    drupal_validate_form('user_register', $form, $form_state);
+    $form = drupal_retrieve_form('user_register_form', $form_state);
+    drupal_prepare_form('user_register_form', $form, $form_state);
+    drupal_validate_form('user_register_form', $form, $form_state);
     if (form_get_errors()) {
       // We were unable to register a valid new user, redirect to standard
       // user/register and prefill with the values we received.
@@ -452,7 +452,7 @@ function openid_authentication($response) {
     }
     else {
       unset($form_state['values']['response']);
-      $account = user_save('', $form_state['values']);
+      $account = user_save(drupal_anonymous_user(), $form_state['values']);
       // Terminate if an error occurred during user_save().
       if (!$account) {
         drupal_set_message(t("Error saving user account."), 'error');

modules/profile/profile.module

@@ -210,17 +210,19 @@ function profile_block_view($delta = '') {
 }
 
 /**
- * Implement hook_user_update().
+ * Implement hook_user_presave().
  */
-function profile_user_update(&$edit, $account, $category) {
-  return profile_save_profile($edit, $account, $category);
+function profile_user_presave(&$edit, $account, $category) {
+  if ($account->uid) {
+    profile_save_profile($edit, $account, $category);
+  }
 }
 
 /**
  * Implement hook_user_insert().
  */
 function profile_user_insert(&$edit, $account, $category) {
-  return profile_save_profile($edit, $account, $category, TRUE);
+  profile_save_profile($edit, $account, $category, TRUE);
 }
 
 /**
@@ -368,25 +370,23 @@ function _profile_form_explanation($field) {
  * Implement hook_form_alter().
  */
 function profile_form_alter(&$form, &$form_state, $form_id) {
-  if ($form_id == 'user_register' || $form_id == 'user_profile_form') {
-    $register = ($form['#user']->uid > 0 ? FALSE : TRUE);
-    $form = array_merge($form, profile_form_profile($form['#user'], $form['#user_category'], $register));
+  if (!($form_id == 'user_register_form' || $form_id == 'user_profile_form')) {
+    return;
   }
-}
-
-function profile_form_profile($account, $category, $register = FALSE) {
-  $result = _profile_get_fields($category, $register);
+  $form['#validate'][] = 'profile_user_form_validate';
+  $account = $form['#user'];
+  $result = _profile_get_fields($form['#user_category'], $form['#user_category'] == 'register');
   $weight = 1;
-  $fields = array();
   foreach ($result as $field) {
     $category = $field->category;
-    if (!isset($fields[$category])) {
-      $fields[$category] = array('#type' => 'fieldset', '#title' => check_plain($category), '#weight' => $weight++);
+    if (!isset($form[$category])) {
+      $form[$category] = array('#type' => 'fieldset', '#title' => check_plain($category), '#weight' => $weight++);
     }
     switch ($field->type) {
       case 'textfield':
       case 'url':
-        $fields[$category][$field->name] = array('#type' => 'textfield',
+        $form[$category][$field->name] = array(
+          '#type' => 'textfield',
           '#title' => check_plain($field->title),
           '#default_value' => isset($account->{$field->name}) ? $account->{$field->name} : '',
           '#maxlength' => 255,
@@ -394,33 +394,40 @@ function profile_form_profile($account, $category, $register = FALSE) {
           '#required' => $field->required,
         );
         if ($field->autocomplete) {
-          $fields[$category][$field->name]['#autocomplete_path'] = "profile/autocomplete/" . $field->fid;
+          $form[$category][$field->name]['#autocomplete_path'] = "profile/autocomplete/" . $field->fid;
         }
         break;
+
       case 'textarea':
-        $fields[$category][$field->name] = array('#type' => 'textarea',
+        $form[$category][$field->name] = array(
+          '#type' => 'textarea',
           '#title' => check_plain($field->title),
           '#default_value' => isset($account->{$field->name}) ? $account->{$field->name} : '',
           '#description' => _profile_form_explanation($field),
           '#required' => $field->required,
         );
         break;
+
       case 'list':
-        $fields[$category][$field->name] = array('#type' => 'textarea',
+        $form[$category][$field->name] = array(
+          '#type' => 'textarea',
           '#title' => check_plain($field->title),
           '#default_value' => isset($account->{$field->name}) ? $account->{$field->name} : '',
           '#description' => _profile_form_explanation($field),
           '#required' => $field->required,
         );
         break;
+
       case 'checkbox':
-        $fields[$category][$field->name] = array('#type' => 'checkbox',
+        $form[$category][$field->name] = array(
+          '#type' => 'checkbox',
           '#title' => check_plain($field->title),
           '#default_value' => isset($account->{$field->name}) ? $account->{$field->name} : '',
           '#description' => _profile_form_explanation($field),
           '#required' => $field->required,
         );
         break;
+
       case 'selection':
         $options = $field->required ? array() : array('--');
         $lines = preg_split("/[\n\r]/", $field->options);
@@ -429,7 +436,8 @@ function profile_form_profile($account, $category, $register = FALSE) {
             $options[$line] = $line;
           }
         }
-        $fields[$category][$field->name] = array('#type' => 'select',
+        $form[$category][$field->name] = array(
+          '#type' => 'select',
           '#title' => check_plain($field->title),
           '#default_value' => isset($account->{$field->name}) ? $account->{$field->name} : '',
           '#options' => $options,
@@ -437,8 +445,10 @@ function profile_form_profile($account, $category, $register = FALSE) {
           '#required' => $field->required,
         );
         break;
+
       case 'date':
-        $fields[$category][$field->name] = array('#type' => 'date',
+        $form[$category][$field->name] = array(
+          '#type' => 'date',
           '#title' => check_plain($field->title),
           '#default_value' => isset($account->{$field->name}) ? $account->{$field->name} : '',
           '#description' => _profile_form_explanation($field),
@@ -447,7 +457,6 @@ function profile_form_profile($account, $category, $register = FALSE) {
         break;
     }
   }
-  return $fields;
 }
 
 /**
@@ -461,25 +470,24 @@ function _profile_update_user_fields($fields, $account) {
 }
 
 /**
- * Implement hook_user_validate().
+ * Form validation handler for the user register/profile form.
+ *
+ * @see profile_form_alter()
  */
-function profile_user_validate(&$edit, $account, $category) {
-  $result = _profile_get_fields($category);
+function profile_user_form_validate($form, &$form_state) {
+  $result = _profile_get_fields($form['#user_category'], $form['#user_category'] == 'register');
   foreach ($result as $field) {
-    if ($edit[$field->name]) {
-      if ($field->type == 'url') {
-        if (!valid_url($edit[$field->name], TRUE)) {
-          form_set_error($field->name, t('The value provided for %field is not a valid URL.', array('%field' => $field->title)));
-        }
+    if (!empty($form_state['values'][$field->name])) {
+      if ($field->type == 'url' && !valid_url($form_state['values'][$field->name], TRUE)) {
+        form_set_error($field->name, t('The value provided for %field is not a valid URL.', array('%field' => $field->title)));
       }
     }
     elseif ($field->required && !user_access('administer users')) {
       form_set_error($field->name, t('The field %field is required.', array('%field' => $field->title)));
     }
   }
-
-  return $edit;
 }
+
 /**
  * Implement hook_user_categories().
  */

modules/simpletest/drupal_web_test_case.php

@@ -866,7 +866,7 @@ protected function drupalCreateUser($permissions = array('access comments', 'acc
     $edit['pass']   = user_password();
     $edit['status'] = 1;
 
-    $account = user_save('', $edit);
+    $account = user_save(drupal_anonymous_user(), $edit);
 
     $this->assertTrue(!empty($account->uid), t('User created with name %name and pass %pass', array('%name' => $edit['name'], '%pass' => $edit['pass'])), t('User login'));
     if (empty($account->uid)) {

modules/system/system.api.php

@@ -555,7 +555,7 @@ function hook_form_alter(&$form, &$form_state, $form_id) {
  */
 function hook_form_FORM_ID_alter(&$form, &$form_state) {
   // Modification for the form with the given form ID goes here. For example, if
-  // FORM_ID is "user_register" this code would run only on the user
+  // FORM_ID is "user_register_form" this code would run only on the user
   // registration form.
 
   // Add a checkbox to registration form about agreeing to terms of use.

modules/system/system.module

@@ -1492,7 +1492,7 @@ function system_form_user_profile_form_alter(&$form, &$form_state) {
 /**
  * Implement hook_form_FORM_ID_alter().
  */
-function system_form_user_register_alter(&$form, &$form_state) {
+function system_form_user_register_form_alter(&$form, &$form_state) {
   if (variable_get('configurable_timezones', 1)) {
     if (variable_get('user_default_timezone', DRUPAL_USER_TIMEZONE_DEFAULT) == DRUPAL_USER_TIMEZONE_SELECT) {
       system_user_timezone($form, $form_state);

modules/trigger/tests/trigger_test.module

@@ -35,6 +35,7 @@ function trigger_test_action_info() {
         'comment_insert',
         'comment_update',
         'comment_delete',
+        'user_presave',
         'user_insert',
         'user_update',
         'user_delete',

modules/trigger/trigger.module

@@ -136,11 +136,14 @@ function trigger_trigger_info() {
        ),
      ),
      'user' => array(
+       'user_presave' => array(
+         'label' => t('When either creating a new user account or updating an existing'),
+       ),
        'user_insert' => array(
-         'label' => t('After a user account has been created'),
+         'label' => t('After creating a new user account'),
        ),
        'user_update' => array(
-         'label' => t("After a user's profile has been updated"),
+         'label' => t('After updating a user account'),
        ),
        'user_delete' => array(
          'label' => t('After a user has been deleted'),
@@ -469,6 +472,13 @@ function trigger_user_logout($account) {
   _trigger_user('user_logout', $edit = NULL, $account);
 }
 
+/**
+ * Implement hook_user_presave().
+ */
+function trigger_user_presave(&$edit, $account, $category) {
+  _trigger_user('user_presave', $edit, $account, $category);
+}
+
 /**
  * Implement hook_user_insert().
  */

modules/user/user.admin.inc

@@ -12,7 +12,7 @@ function user_admin($callback_arg = '') {
   switch ($op) {
     case t('Create new account'):
     case 'create':
-      $build['user_register'] = drupal_get_form('user_register');
+      $build['user_register'] = drupal_get_form('user_register_form');
       break;
     default:
       if (!empty($_POST['accounts']) && isset($_POST['operation']) && ($_POST['operation'] == 'cancel')) {

modules/user/user.api.php

@@ -178,28 +178,6 @@ function hook_user_operations() {
   return $operations;
 }
 
-/**
- * The user object has been updated and changed.
- *
- * Use this if (probably along with 'insert') if you want to reuse some
- * information from the user object.
- *
- * @param &$edit
- *   The array of form values submitted by the user.
- * @param $account
- *   The user object on which the operation is performed.
- * @param $category
- *   The active category of user information being edited.
- */
-function hook_user_after_update(&$edit, $account, $category) {
-  db_insert('user_changes')
-    ->fields(array(
-      'uid' => $account->uid,
-      'changed' => time(),
-    ))
-    ->execute();
-}
-
 /**
  * Retrieve a list of all user setting/information categories.
  *
@@ -218,7 +196,36 @@ function hook_user_categories() {
 }
 
 /**
- * The user account is being added.
+ * A user account is about to be created or updated.
+ *
+ * This hook is primarily intended for modules that want to store properties in
+ * the serialized {users}.data column, which is automatically loaded whenever a
+ * user account object is loaded, and the module needs to prepare the stored
+ * data in some way.
+ * The module should save its custom additions to the user object into the
+ * database and set the saved fields to NULL in $edit.
+ *
+ * @param &$edit
+ *   The array of form values submitted by the user.
+ * @param $account
+ *   The user object on which the operation is performed.
+ * @param $category
+ *   The active category of user information being edited.
+ *
+ * @see hook_user_insert()
+ * @see hook_user_update()
+ */
+function hook_user_presave(&$edit, $account, $category) {
+  // Make sure that our form value 'mymodule_foo' is stored as 'mymodule_bar'.
+  if (isset($edit['mymodule_foo'])) {
+    $edit['mymodule_bar'] = $edit['mymodule_foo'];
+    // Inform user_save() to ignore the value of our property.
+    $edit['mymodule_foo'] = NULL;
+  }
+}
+
+/**
+ * A user account was created.
  *
  * The module should save its custom additions to the user object into the
  * database and set the saved fields to NULL in $edit.
@@ -229,6 +236,9 @@ function hook_user_categories() {
  *   The user object on which the operation is being performed.
  * @param $category
  *   The active category of user information being edited.
+ *
+ * @see hook_user_presave()
+ * @see hook_user_update()
  */
 function hook_user_insert(&$edit, $account, $category) {
   db_insert('mytable')
@@ -237,9 +247,35 @@ function hook_user_insert(&$edit, $account, $category) {
       'uid' => $account->uid,
     ))
     ->execute();
+  // Inform user_save() to ignore the value of our property.
   $edit['myfield'] = NULL;
 }
 
+/**
+ * A user account was updated.
+ *
+ * Modules may use this hook to update their user data in a custom storage
+ * after a user account has been updated.
+ *
+ * @param &$edit
+ *   The array of form values submitted by the user.
+ * @param $account
+ *   The user object on which the operation is performed.
+ * @param $category
+ *   The active category of user information being edited.
+ *
+ * @see hook_user_presave()
+ * @see hook_user_insert()
+ */
+function hook_user_update(&$edit, $account, $category) {
+  db_insert('user_changes')
+    ->fields(array(
+      'uid' => $account->uid,
+      'changed' => time(),
+    ))
+    ->execute();
+}
+
 /**
  * The user just logged in.
  *
@@ -270,75 +306,6 @@ function hook_user_logout($account) {
     ->execute();
 }
 
-/**
- * Modify the account before it gets saved.
- *
- * @param &$edit
- *   The array of form values submitted by the user.
- * @param $account
- *   The user object on which the operation is performed.
- * @param $category
- *   The active category of user information being edited.
- */
-function hook_user_submit(&$edit, $account, $category) {
-  if ($category == 'account') {
-    if (!empty($edit['picture_upload'])) {
-      $edit['picture'] = $edit['picture_upload'];
-    }
-    // Delete picture if requested, and if no replacement picture was given.
-    elseif (!empty($edit['picture_delete'])) {
-      $edit['picture'] = NULL;
-    }
-    // Remove these values so they don't end up serialized in the data field.
-    $edit['picture_upload'] = NULL;
-    $edit['picture_delete'] = NULL;
-
-    if (isset($edit['roles'])) {
-      $edit['roles'] = array_filter($edit['roles']);
-    }
-  }
-}
-
-/**
- * The user account is being changed.
- *
- * The module should save its custom additions to the user object into the
- * database and set the saved fields to NULL in $edit.
- *
- * @param &$edit
- *   The array of form values submitted by the user.
- * @param $account
- *   The user object on which the operation is performed.
- * @param $category
- *   The active category of user information being edited.
- */
-function hook_user_update(&$edit, $account, $category) {
-  db_update('mytable')
-    ->fields(array('myfield' => $edit['myfield']))
-    ->condition('uid', $account->uid)
-    ->execute();
-  $edit['myfield'] = NULL;
-}
-
-/**
- * The user account is about to be modified.
- *
- * The module should validate its custom additions to the user object,
- * registering errors as necessary.
- *
- * @param &$edit
- *   The array of form values submitted by the user.
- * @param $account
- *   The user object on which the operation is being performed.
- * @param $category
- *   The active category of user information being edited.
- */
-function hook_user_validate(&$edit, $account, $category) {
-  if ($category == 'mymodule' && empty($edit['myfield'])) {
-    form_set_error('myfield', t('Myfield is required.'));
-  }
-}
-
 /**
  * The user's account information is being displayed.
  *

modules/user/user.pages.inc

@@ -209,18 +209,13 @@ function template_preprocess_user_profile_category(&$variables) {
   }
 }
 
-/**
- * Menu callback; Present the form to edit a given user or profile category.
- */
-function user_edit($account, $category = 'account') {
-  drupal_set_title($account->name);
-  return drupal_get_form('user_profile_form', $account, $category);
-}
-
 /**
  * Form builder; edit a user account or one of their profile categories.
  *
  * @ingroup forms
+ * @see user_account_form()
+ * @see user_account_form_validate()
+ * @see user_account_form_submit()
  * @see user_profile_form_validate()
  * @see user_profile_form_submit()
  * @see user_cancel_confirm_form_submit()
@@ -232,23 +227,31 @@ function user_profile_form($form, &$form_state, $account, $category = 'account')
   $form['#user_category'] = $category;
 
   if ($category == 'account') {
-    user_edit_form($form, $form_state);
+    user_account_form($form, $form_state);
   }
 
   // Attach field widgets.
   field_attach_form('user', $account, $form, $form_state);
 
-  $form['submit'] = array('#type' => 'submit', '#value' => t('Save'), '#weight' => 30);
-
-  if (($account->uid == $user->uid && user_access('cancel account')) || user_access('administer users')) {
+  $form['submit'] = array(
+    '#type' => 'submit',
+    '#value' => t('Save'),
+    '#weight' => 30,
+  );
+  if ($category == 'account') {
     $form['cancel'] = array(
       '#type' => 'submit',
       '#value' => t('Cancel account'),
       '#weight' => 31,
       '#submit' => array('user_edit_cancel_submit'),
+      '#access' => ($account->uid == $user->uid && user_access('cancel account')) || user_access('administer users'),
     );
   }
 
+  $form['#validate'][] = 'user_profile_form_validate';
+  // Add the final user profile form submit handler.
+  $form['#submit'][] = 'user_profile_form_submit';
+
   return $form;
 }
 
@@ -258,14 +261,6 @@ function user_profile_form($form, &$form_state, $account, $category = 'account')
 function user_profile_form_validate($form, &$form_state) {
   $edit = (object)$form_state['values'];
   field_attach_form_validate('user', $edit, $form, $form_state);
-  $edit = (array)$edit;
-  user_module_invoke('validate', $edit, $form['#user'], $form['#user_category']);
-  // Validate input to ensure that non-privileged users can't alter protected data.
-  if ((!user_access('administer users') && array_intersect(array_keys($edit), array('uid', 'init', 'session'))) || (!user_access('administer permissions') && isset($form_state['values']['roles']))) {
-    watchdog('security', 'Detected malicious attempt to alter protected user fields.', array(), WATCHDOG_WARNING);
-    // set this to a value type field
-    form_set_error('category', t('Detected malicious attempt to alter protected user fields.'));
-  }
 }
 
 /**
@@ -279,7 +274,7 @@ function user_profile_form_submit($form, &$form_state) {
   $edit = (object)$form_state['values'];
   field_attach_submit('user', $edit, $form, $form_state);
   $edit = (array)$edit;
-  user_module_invoke('submit', $edit, $account, $category);
+
   user_save($account, $edit, $category);
 
   // Clear the page cache because pages can contain usernames and/or profile information:

modules/user/user.test

@@ -1224,7 +1224,7 @@ class UserSaveTestCase extends DrupalWebTestCase {
       'pass' => user_password(),
       'status' => 1,
     );
-    $user_by_return = user_save('', $user);
+    $user_by_return = user_save(drupal_anonymous_user(), $user);
     $this->assertTrue($user_by_return, t('Loading user by return of user_save().'));
 
     // Test if created user exists.