nodeapi hook is wrong [#194862]

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 ;-) )

Comment	File	Size	Author
#1	excerpt.patch	1.5 KB	smk-ka

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

smk-ka CreditAttribution: smk-ka commented 18 March 2008 at 19:40

Version:	5.x-1.2	» 6.x-1.x-dev
Status:	Active	» Needs review

File	Size
excerpt.patch	1.5 KB

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

Comment #2

Ilya1st CreditAttribution: Ilya1st commented 19 March 2008 at 00:44

you're right :)
later Iwill publish my additional changes after I include your hook there

Comment #3

smk-ka CreditAttribution: smk-ka commented 19 March 2008 at 02:22

Status:

Needs review

» Fixed

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.