I tried to override text field in comments and i figured out:

Text shows "field nme: current state >> new state" instead of "field nme: old state >> new state"

so, all comments look like changing curent value to their own, like that:
field one: bar >> foo
field one: bar >> bar
field one: bar >> foo
field one: bar >> buzz

where last comment sets field to "bar" again and initial value is not shown at all.

I suppose
field one: test >> foo
field one: foo >> bar
field one: bar >> foo
field one: foo >> buzz

wil be much more useful.

Comments

fasdalf@fasdalf.ru’s picture

I figured out that function comment_cck_comment(&$comment, $op) has a line #120

 db_query('INSERT INTO {comment_cck_revisions} (cid, vid, previous_vid) VALUES (%d, %d, %d)', $comment['cid'], $node->vid, $previous_vid);

But $previous_vid variable was not initialised in this function before, so previous revision ID becomes zero. This makes drupal to load current revision.

fasdalf@fasdalf.ru’s picture

Component: User interface » Code
Assigned: Unassigned » fasdalf@fasdalf.ru
Status: Active » Patch (to be ported)

i've added a line

$previous_vid = db_result(db_query('SELECT vid FROM {node_revisions} WHERE nid = %d AND vid < %d ORDER BY vid DESC LIMIT 1', $node->nid, $node->vid));

just before line 120 and now module works well for new comments.

fasdalf@fasdalf.ru’s picture

Assigned: fasdalf@fasdalf.ru » Unassigned

I'm not sure if i should assign it to myself since i can't apply changes to CVS

aren cambre’s picture

Status: Patch (to be ported) » Needs review
Hiroaki’s picture

thanks for the help, im adding the line, this module seems to be bugged a lot :P

Hiroaki’s picture

Status: Needs review » Reviewed & tested by the community

This works like magic.
It even fixed the status history showing up on wrong comments. (the one right above!!!)

Azol’s picture

Before adding this SELECT to the module I'd advise you to have a look at stable module version (1.0 beta2). It seems to me the author (or the maintainer rather) has rewritten a part of the module without paying much attention to the original text of this function:

  switch ($op) {
    case 'update':
      $original_node = node_load($comment['nid']);
      if ($fields = variable_get('comment_cck_fields_'. $original_node->type, array())) {
        // Ungroup the fields in this comment, if necessary.
        $comment_fields = _comment_cck_ungroup_fields($comment['comment_cck'], $fields, $original_node->type);
        // Merge the updated fields in this comment with the original node.
        $node = (object) array_merge((array) $original_node, $comment_fields);
        // We don't want a node revision, since we're updating an old comment.
        // @TODO: Do we?
        $node->revision = 0;
        $node->vid = db_result(db_query('SELECT vid FROM {comment_cck_revisions} WHERE cid = %d', $comment['cid']));
        // Save the node with the updated field data.
        node_save($node);
      }
      break;

    case 'insert':
      $original_node = node_load($comment['nid']);
      if ($fields = variable_get('comment_cck_fields_'. $original_node->type, array())) {
        // Ungroup the fields in this comment, if necessary.
        $comment_fields = _comment_cck_ungroup_fields($comment['comment_cck'], $fields, $original_node->type);
        // Merge the updated fields in this comment with the original node.
        $node = (object) array_merge((array) $original_node, $comment_fields);
        $previous_vid = $node->vid;
        // We do want a node revision, since this is a new comment.
        $node->revision = 1;
        // Save the node with the updated field data.
        node_save($node);
        // Record that this comment added a node revision.
        db_query('INSERT INTO {comment_cck_revisions} (cid, vid, previous_vid) VALUES (%d, %d, %d)', $comment['cid'], $node->vid, $previous_vid);
        // Update the comment revision id.
        // @TODO: What is this needed for?
        $comment['revision_id'] = $node->vid;

Basically what happened is $previous_vid = $node->vid; was completely dropped from newer version, breaking it functionality.

Alternative solution would be just reverting this change (using the snippet I have provided). And we're keeping DB engine happy!

Azol’s picture

Status: Reviewed & tested by the community » Needs work

Changing this back to Needs work, because I still believe just adding another SELECT is not the best way to fix this problem (esp. performance-wise).

This module is very useful, but I am not sure if original author is watching it. Any undertakers?

I can assemble a new version of this module, incorporating some of the fixes, but I do not think I will have time to check it into CVS or maintain it.

aren cambre’s picture

Azol’s picture

Status: Needs work » Fixed

applied to dev. branch

Status: Fixed » Closed (fixed)

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