nodeapi hook is wrong
Ilya1st - November 23, 2007 - 22:35
| Project: | Excerpt |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
I thin the most correct variant of hook function is:
function excerpt_nodeapi(&$node, $op, $teaser) {
switch ($op) {
case 'submit':
case 'update':
case 'validate':
if (trim($node->teaser) == '') {
$node->teaser = node_teaser($node->body, $node->format);
}
break;
case 'view':
$node->readmore = (strlen($node->teaser) < strlen($node->body) );
break;
break;
}
}То store all to the database and not to make all without filterformatting.
E.g. do not use filter is wrong. I use safehtml for most correct html input cause I don't want slowdown of the htmlcorrector module.(reformat html everytime on high load is stupid ;-) )
#1
Two issues with above code: hook_nodeapi($op = update) is run after the node has been saved to the database, $op = validate is called with a copy of the submitted values, ie. in both cases the changes won't be reflected in the node itself.
However, generating the teaser in hook_nodeapi($op = submit) seems correct, and is much more efficient than generating the teaser in $op = load, where it's currently being generated (although the node_teaser() call should verify that both the body and format fields actually exist, see node_submit() for reference).
The readmore condition, however, is wrong: since teaser and body can be of equal length but can have completely different contents, we have to compare the contents of these fields. Drupal core just checks whether the length of the teaser is less than the body length. With excerpts however, teasers can be even longer than the body field, which is why we need to overwrite that property with our own condition.
The attached patch takes all of above into consideration. Additionally, it only processes a node if the configuration allows it to, and has hook_help() removed (which was referencing old D4.7.x paths).
Note that the patch is against CVS HEAD, not the 5.x snapshot available on the project page (syncing both branches would be a fine idea, though).
#2
you're right :)
later Iwill publish my additional changes after I include your hook there
#3
Ilya1st, I've just applied my patch and some additional changes to CVS HEAD as well as DRUPAL-5. The devel snapshot on the project page should update within the next 12 hours or so. Feel free to open a new issue for any of your other changes.
#4
Automatically closed -- issue fixed for two weeks with no activity.