Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
first cut, this is just rearranging mostly.
motivation comes from lots of places, good discussion on the dev list here.
all tests pass. (as an aside, huge thanks to all who've worked on simpletest, definitely saved me a bunch of time when creating this patch.)
still needs heaps of code docs, and another cut to take advantage of the fact that we don't have to use the one-size-fits-all API. might be a separate issue or two though.
Comment | File | Size | Author |
---|---|---|---|
#28 | 310212.patch | 7.21 KB | RobLoach |
#27 | 310212.patch | 7.5 KB | RobLoach |
#24 | 310212_0.patch | 9.88 KB | dawehner |
#22 | 310212.patch | 9.17 KB | RobLoach |
#20 | user_op_comment_4.patch | 10.62 KB | dawehner |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedadded some phpdoc for new functions.
took out the
if ($hook == 'user')
hack inmodule.inc
(thanks chx).all tests pass except for 3 in 'Upload user picture', which are failing on a clean HEAD for me at the moment.
any opinions on whether this will make it into D7?
Comment #2
catchWe shouldn't release D7 with any $ops left in it IMO.
A few things I noticed -
statistics.module is missing phpdoc for Implementation of...
trigger.module has // Nasty hack... - surely // @TODO?
We should be passing $user as an object to all of these hooks, and probably using module_invoke_all for them too, but that could be a followup patch.
I looks like we should remove hook_user_form and just use hook_form_alter() instead unless I'm missing something - again - fine with that being a followup patch.
Comment #3
Anonymous (not verified) CreditAttribution: Anonymous commented@catch: thanks for quick review. i like the idea of passing in $user, but the intention of this patch is to just remove $op, then we can make the api better with follow up patches. trying to do as webchick suggests with simple patches...
added phpdoc for statistics.module.
added TODO for trigger.module.
Comment #4
Anonymous (not verified) CreditAttribution: Anonymous commentedwill update this patch once HEAD is fixed.
Comment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedupdated for HEAD, all tests pass.
Comment #6
catchSo there's plenty of followup work to do once this gets in - like use module_invoke_all instead of user_module_invoke, pass it a user object, get rid of the fapi stuff etc. etc. - but that can all be done in subsequent patches. I confirmed that all tests pass, so marking RTBC.
Comment #7
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD.
I'm marking this code needs work until the upgrade docs have been updated.
Thanks! Awesome patch.
Comment #8
RobLoachAdded the update module documentation for remove $op.
Comment #9
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.
Comment #10
catchapi docs haven't been updated yet: http://api.drupal.org/api/function/hook_user/7
Comment #11
RobLoachOn a rather unrelated topic of docs, #314870: UNSTABLE-4 blocker: Add api.php for every core module would really be nice....
Comment #12
catchJust noticed this still needs docs (now in user.api.php).
Comment #13
dawehnerthis patch rewrites the user.api.php to a removed op
TODO: examples for the hooks
Comment #14
RobLoachIt was missing hook_user_form. I also think that the implementation examples of the hooks should be added after this patch, due to the kittens. Nicely done!
Comment #15
dawehneroh damn i deleted it because, i missed it because i read something with hook_nodeapi and remove of $op = 'form'
but i don't trust myself to be able to set to RTBC, anyone else, reading the documentation?
Comment #16
Dries CreditAttribution: Dries commentedThere seems to be some inconsistent line-wrapping going on in the last version.
Comment #18
dawehnerthis is the new version, just copyed http://drupal.org/files/issues/310212.patch and resolved the reject by hand.
Comment #19
catchStill some line wrapping issues - i.e.:
Comment #20
dawehneri'm not sure about the load, login and logout hooks, see the old comment
it seems to be than the last line should be under load, so i did this
Comment #21
dawehnerComment #22
RobLoachIt seems like "and insert additional information into the user object." is the prefix of "The module may respond to this". This patch includes that fix.
Also, looking at other implementations of hook_*, we see that there is an empty line between the first line and the next line of the documentation:
This is also fixed in this patch.......... Documentation line margin set to 80.
Comment #24
dawehnerreroll
Comment #26
moshe weitzman CreditAttribution: moshe weitzman commentedAny chance we can get a reroll? This is a significant doc bug.
Comment #27
RobLoachComment #28
RobLoachWhoops, wrong patch.
Comment #29
moshe weitzman CreditAttribution: moshe weitzman commentedThanks.
Only the 'view' hook gets a $category. For example,
module_invoke_all('user_logout', NULL, $user);
.Instead of 'user object', say $edit.
Comment #30
BerdirPlease have a look at #491972: Fix horrifying mess of crap in user module hooks.
I created the other issue without knowing about this one. In my patch, I removed all unecessary arguments of those hooks. I haven't yet looked at this, but if this contains better docs, we should merge it and if not, close one of them as duplicate.
Edit: I've also removed those unecessary & from $account, as that is an object...
Comment #31
casey CreditAttribution: casey commentedThis issue seems to be fixed.
Comment #32
casey CreditAttribution: casey commented