Error in user.api.php [#1483664]

Comment	File	Size	Author
#27	hook_user_presave-1483664-25.patch	679 bytes	nmudgal

#17	hook_user_presave-d7-1483664-14.patch	679 bytes	nmudgal

#15	hook_user_presave-d8-1483664-14.patch	688 bytes	nmudgal

#10	hook_user_presave-D8-1483664.patch	690 bytes	nmudgal

#9	hook_user_presave-D7-1483664.patch	681 bytes	nmudgal

#5	hook_user_presave-D7-1483664.patch	525 bytes	chertzog

#4	hook_user_presave-D8-1483664.patch	534 bytes	chertzog

Comment #1

rafamd CreditAttribution: rafamd commented 15 March 2012 at 13:28

Issue tags:

+Novice

Added correct code sample and Novice tag.

Log in or register to post comments

Comment #2

rafamd CreditAttribution: rafamd commented 18 March 2012 at 12:11

Version:	7.x-dev	» 8.x-dev
Component:	documentation	» user.module
Priority:	Minor	» Normal
Status:	Needs work	» Active

This is also wrong in D8, see: http://api.drupal.org/api/drupal/core!modules!user!user.api.php/function...

Maybe the correction should be the opposite ? I mean:
$edit['data']['mymodule_foo'] = $edit['mymodule_bar'];

~~ps: unrelated, but where did $category param go in D8 version of hook_user_presave ?~~ already found out... all the "category" mechanism was taken away.

Log in or register to post comments

Comment #4

chertzog

he/him

English

Harrisburg, PA, USA

CreditAttribution: chertzog commented 18 March 2012 at 02:47

Status:

Active

» Needs review

File	Size
hook_user_presave-D8-1483664.patch	534 bytes

Patch attached.

Log in or register to post comments

Comment #5

chertzog

he/him

English

Harrisburg, PA, USA

CreditAttribution: chertzog commented 18 March 2012 at 02:47

Version:

8.x-dev

» 7.x-dev

File	Size
hook_user_presave-D7-1483664.patch	525 bytes

D7 Patch.

Log in or register to post comments

Comment #6

rafamd CreditAttribution: rafamd commented 18 March 2012 at 11:33

Status:	Needs review	» Needs work
Issue tags:	-Novice

Thanks chertzog!

Looking at your patches, seems the same line is removed and introduced both times. Shouldn't the correct new line be:
+ $edit['data']['mymodule_foo'] = $edit['mymodule_bar']; ??

Log in or register to post comments

Comment #7

rafamd CreditAttribution: rafamd commented 18 March 2012 at 11:36

Issue tags:

+Novice

restore tag

Log in or register to post comments

Comment #8

rafamd CreditAttribution: rafamd commented 18 March 2012 at 12:08

hummm ... taking another look at the example, I think I was a confused in #2 and #6 and would like to stick to the recommended solution in the original post (and improve the inline comment to make it more informative).

Something like this:

<?php
function hook_user_presave(&$edit, $account, $category) {
  // Make sure that the form value 'mymodule_foo' is also stored as
  // 'mymodule_bar' in the 'data' (serialized) column.
  if (isset($edit['mymodule_foo'])) {
    $edit['data']['mymodule_bar'] = $edit['mymodule_foo'];
  }
}
?>

Log in or register to post comments

Comment #9

nmudgal CreditAttribution: nmudgal commented 18 March 2012 at 13:44

Version:	8.x-dev	» 7.x-dev
Component:	user.module	» documentation
Priority:	Normal	» Minor
Status:	Active	» Needs review

File	Size
hook_user_presave-D7-1483664.patch	681 bytes

Patch attached after updating as per comment.

Log in or register to post comments

Comment #10

nmudgal CreditAttribution: nmudgal commented 18 March 2012 at 13:46

Version:

7.x-dev

» 8.x-dev

File	Size
hook_user_presave-D8-1483664.patch	690 bytes

8.x -> Patch attached after updating as per comment.

Log in or register to post comments

Comment #11

chertzog

he/him

English

Harrisburg, PA, USA

CreditAttribution: chertzog commented 18 March 2012 at 14:43

nice catch. At first glance all that i saw as the problem was the "my_module" change. I didnt notice the "_foo" to "_bar" change.

Log in or register to post comments

Comment #12

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 18 March 2012 at 15:58

Status:

Needs review

» Reviewed & tested by the community

Thanks! Patch looks good; I'll get it committed soon.

Log in or register to post comments

Comment #13

nmudgal CreditAttribution: nmudgal commented 18 March 2012 at 16:01

@jhodgdon
It would my first commit to core then :-)
Thanks.

Log in or register to post comments

Comment #14

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 18 March 2012 at 23:28

Status:

Reviewed & tested by the community

» Needs work

Uh oh. Actually I just noticed that the comment doesn't seem to match the patch (mymodule_foo vs. my_module_foo, and same for bar).

Log in or register to post comments

Comment #15

nmudgal CreditAttribution: nmudgal commented 19 March 2012 at 02:16

Status:

Needs work

» Needs review

File	Size
hook_user_presave-d8-1483664-14.patch	688 bytes

oops here is the updated patch.

Thanks

Log in or register to post comments

Comment #16

rafamd CreditAttribution: rafamd commented 19 March 2012 at 05:19

Version:	8.x-dev	» 7.x-dev
Status:	Needs review	» Needs work

Thanks nmudgal, looks better now. Care to re-roll the 7.x patch also ?

Log in or register to post comments

Comment #17

nmudgal CreditAttribution: nmudgal commented 19 March 2012 at 05:22

Status:

Needs work

» Needs review

File	Size
hook_user_presave-d7-1483664-14.patch	679 bytes

here is one, just was waiting for review of 8.x.

Thanks

Log in or register to post comments

Comment #18

rafamd CreditAttribution: rafamd commented 19 March 2012 at 05:39

Status:

Needs review

» Reviewed & tested by the community

Thanks !

Log in or register to post comments

Comment #19

19 March 2012 at 05:57

Status:	Reviewed & tested by the community	» Needs work
Issue tags:	-Novice

The last submitted patch, hook_user_presave-d7-1483664-14.patch, failed testing.

Log in or register to post comments

Comment #20

boombatower CreditAttribution: boombatower commented 19 March 2012 at 06:19

Status:	Needs work	» Needs review
Issue tags:		+Novice

#17: hook_user_presave-d7-1483664-14.patch queued for re-testing.

Log in or register to post comments

Comment #21

nmudgal CreditAttribution: nmudgal commented 19 March 2012 at 06:20

Couldn't understand why it failed ?

Log in or register to post comments

Comment #22

boombatower CreditAttribution: boombatower commented 19 March 2012 at 06:46

Seems unrelated and I don't know why someone would be using hook documentation in a test.

Log in or register to post comments

Comment #23

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 19 March 2012 at 13:14

Version:

7.x-dev

» 8.x-dev

Please leave this at 8.x until 8.x is done -- read http://drupal.org/node/1488414#comment-5748422

Log in or register to post comments

Comment #24

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 19 March 2012 at 13:16

Status:

Needs review

» Reviewed & tested by the community

Patch in #15 is RTBC for 8.x.

Log in or register to post comments

Comment #25

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 19 March 2012 at 13:58

Version:	8.x-dev	» 7.x-dev
Status:	Reviewed & tested by the community	» Patch (to be ported)

The patch in #15 has been committed to Drupal 8. Can we have a patch for Drupal 7 that puts the same code/doc in there too? The patch above for D7 is missing some of the comments.