Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
The example of hook_prepare()
has numerous isses:
- It uses functions that do not exist since Drupal 6.
- It uses the wrong parameters for functions that do exist (
file_save_upload()
). - Most of all, it is not a good example of a typical use of
hook_prepare()
. (That code would usually be better in a form-validator callback.)
Note that #2222497 made some changes to improve this, but left or introduced many problems: #2222497: hook_prepare() example uses a D6 only function
Solution
Copy the existing implementation from the function body of comment_prepare() to this function body.
Comment | File | Size | Author |
---|---|---|---|
#16 | 0001-2325055-Improving-hook_preapare-example.patch | 1.07 KB | darol100 |
#14 | 0001-2325055-Improving-hook_preapare-example.patch | 1.16 KB | darol100 |
#6 | D7-hook-prepare-2325055-6.patch | 790 bytes | joshi.rohit100 |
Comments
Comment #1
jhodgdonWhat is the "Newbie" tag? I'm not familiar with this. If you mean "A new contributor could patch this", then the correct tag is "Novice". If you mean "I'm a newbie and I need this information", then the correct tag is "DX (Developer Experience)". I'm assuming the latter... but this is just a regular documentation issue with some hook documentation, so I think it doesn't need a special DX tag, since that is for more pervasive issues.
Anyway, thanks for the issue! Patch welcome...
Comment #2
Bevan CreditAttribution: Bevan commentedI meant "Novice".
Comment #3
jhodgdonAs stated, "come up with a better example and completely rewrite the body of the function", without any further guidance, is probably not a great "Novice" issue... Any thoughts on what a better example would be?
Comment #4
dcam CreditAttribution: dcam commentedIf we take the example from one of the existing implementations there are two likely candidates: comment_node_prepare() and book_node_prepare(). Both functions deal with setting of additional node properties that are required by the respective modules. Comment's is the simpler of the two, only setting the one parameter indicating whether comments are open/closed. Book's is more involved, dealing with all the stuff relating to the book hierarchies. It may be a little complex, but may have some additional benefit because it demonstrates checking permissions before adding the properties.
I don't know if it's customary to take the API examples from existing implementations, but I know that there are a lot of places in core where that was done in the past.
Comment #5
jhodgdonYes -- then let's take it from Comment.
Now it's probably a good Novice task. Thanks!
Comment #6
joshi.rohit100Uploaded the patch. Please review now.
Comment #7
jhodgdonActually, sorry, no... the above information was not good.
This is talking about hook_prepare(), which is only invoked on the module that defines the node type. So for instance, book_prepare() would be invoked for book nodes. comment_node_prepare() is actually an implementation of hook_node_prepare(), not hook_prepare().
So we'll have to figure out a better function body to put in this hook.
Probably it should just be something simple, like adding a new field value to the node object. How about something like:
Comment #8
dcam CreditAttribution: dcam commentedOh no. I'm sorry. =( The list of implementations on hook_prepare()'s API page mistakenly includes all of the hook_node_prepare() implementations. I didn't even notice the problem.
Comment #9
jhodgdonYes, see the note above that list "generated by pattern matching"... that's just how it is.
Comment #10
Bevan CreditAttribution: Bevan commentedFor future reference, api.drupal.org does a pretty good job of linking function invocations and hook implementations from the respective documentation pages.
E.g. The page about
hook_prepare()
lists "6 functions implement hook_prepare()" and links to the "full list".Comment #11
jhodgdonYes and in this case it was misleading. That list is generated through pattern matching, and it finds all functions that end in _prepare, so it found a bunch of functions, none of which is actually an implementation of hook_prepare. See previous comments...
Comment #12
darol100 CreditAttribution: darol100 as a volunteer and commented@jhodgdon, Something like this ?
Comment #13
jhodgdonYes, that looks fine, although in the issset line it should be $node->mymodule_value presumably?
Comment #14
darol100 CreditAttribution: darol100 as a volunteer and commentedHere is a patch.
Comment #16
darol100 CreditAttribution: darol100 as a volunteer and commentedI believe it automatic test fail because of :
function hook_node_prepare($node) {
and phpstorm it was also complaining about it.I change it to this now:
Here is new patch.
Comment #17
jhodgdonThis looks fine to me. Thanks!
Comment #18
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedCommitted to 7.x - thanks!