The example of hook_prepare() has numerous isses:

  1. It uses functions that do not exist since Drupal 6.
  2. It uses the wrong parameters for functions that do exist (file_save_upload()).
  3. 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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Issue tags: -Newbie

What 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...

Bevan’s picture

Issue tags: +Novice

I meant "Novice".

jhodgdon’s picture

As 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?

dcam’s picture

If 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.

jhodgdon’s picture

Issue summary: View changes

Yes -- then let's take it from Comment.

Now it's probably a good Novice task. Thanks!

joshi.rohit100’s picture

Status: Needs work » Needs review
FileSize
790 bytes

Uploaded the patch. Please review now.

jhodgdon’s picture

Status: Needs review » Needs work

Actually, 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:

// Set a value that would be used in hook_form().
$node->mymodule_value = 'foo';
dcam’s picture

Oh 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.

jhodgdon’s picture

Yes, see the note above that list "generated by pattern matching"... that's just how it is.

Bevan’s picture

For 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".

jhodgdon’s picture

Yes 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...

darol100’s picture

@jhodgdon, Something like this ?

function hook_node_prepare($node) {
  if (!isset($node->mymodule)) {
    $node->mymodule_value = 'foo';
  }
}
 
jhodgdon’s picture

Yes, that looks fine, although in the issset line it should be $node->mymodule_value presumably?

darol100’s picture

Status: Needs work » Needs review
FileSize
1.16 KB

Here is a patch.

Status: Needs review » Needs work

The last submitted patch, 14: 0001-2325055-Improving-hook_preapare-example.patch, failed testing.

darol100’s picture

Status: Needs work » Needs review
FileSize
1.07 KB

I 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:

function hook_prepare($node) {
  if (!isset($node->mymodule_value)) {
    $node->mymodule_value = 'foo';
  }
}

Here is new patch.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

This looks fine to me. Thanks!

David_Rothstein’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x - thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.