Step by step, we are fixing the user module hooks. Yesterday, sun remove hook_user_form(). Today, we remove hook_user_validate(). This is a pre-FAPI hook, that makes strictly no sense since about Drupal 5.0-RC1.

Comments

damien tournoud’s picture

Status: Active » Needs review
StatusFileSize
new9.63 KB

A first stab at this.

Status: Needs review » Needs work

The last submitted patch failed testing.

damien tournoud’s picture

Status: Needs work » Needs review
StatusFileSize
new9.67 KB

Tiny parse error in the previous patch ;)

dave reid’s picture

Awesome.

Status: Needs review » Needs work

The last submitted patch failed testing.

dave reid’s picture

Awesomely...failed. :)

damien tournoud’s picture

Status: Needs work » Needs review
StatusFileSize
new14.35 KB

This one should already be a little bit better.

Status: Needs review » Needs work

The last submitted patch failed testing.

sun’s picture

+++ modules/block/block.module
@@ -404,17 +404,21 @@ function block_form_user_profile_form_alter(&$form, &$form_state) {
 /**
- * Implement hook_user_validate().
+ * Form validation callback for the user profile form.
+ *
+ * @see block_form_user_profile_form_alter().
  */
-function block_user_validate(&$edit, $account, $category) {
-  if (empty($edit['block'])) {
-    $edit['block'] = array();
+function block_user_profile_form_validate($form, &$form_state) {
+  if (empty($form_state['values']['block'])) {
+    $form_state['values']['block'] = array();
   }
-  return $edit;
 }

This part we should really discuss in #118345: Revamp hook_user_form/_register/_validate/_submit/_insert/_update first.

The entire validation here makes no sense, and as outlined over there, the purpose of this function should be to not store or to remove any value in {users}.data in case there are no user customizable blocks on the user form.

+++ modules/user/user.module
@@ -1794,7 +1798,28 @@ function user_pass_rehash($password, $timestamp, $login) {
+function user_form_account_fields(&$form, &$form_state) {
@@ -1921,9 +1952,11 @@ function user_edit_form(&$form, &$form_state) {
+  $form['#validate'][] = 'user_form_account_fields_validate';

Can we please rename these functions to

user_account_form()
user_account_form_validate()

Or, if you don't like "form" for whatever reason (I like it), then at least "elements", but not "fields", because "fields" means something completely different now. ;)

Especially the order of terms in those function names looks strange to me... user-form-account-fields -- normally, we put "form" last...

uhm, maybe we can skip the entire naming question and just use a single user_form_alter()!

+++ modules/user/user.module
@@ -1880,6 +1904,13 @@ function user_edit_form(&$form, &$form_state) {
+  if (user_access('administer users')) {
+    $form['account']['notify'] = array(
+     '#type' => 'checkbox',
+     '#title' => t('Notify user of new account'),
+    );
+  }

This additionally needs the $register condition.

+++ modules/user/user.pages.inc
@@ -258,14 +254,6 @@ function user_profile_form($form, &$form_state, $account, $category = 'account')
-  if ((!user_access('administer users') && array_intersect(array_keys($edit), array('uid', 'init', 'session'))) || (!user_access('administer permissions') && isset($form_state['values']['roles']))) {

oh, thanks for those! :) I also discovered that pre-FAPI stuff in #588550: Allow the user edit form to only ask for the current password when necessary (in a followup confirmation step)...

This review is powered by Dreditor.

sun’s picture

Issue tags: +API clean-up

Tagging.

sun’s picture

Status: Needs work » Closed (duplicate)

Let's continue over in #118345: Revamp hook_user_form/_register/_validate/_submit/_insert/_update -- as outlined/questioned above, we need to account for the bigger picture.