If you remove or update a nodereference value it will update the count on any new node being referenced, but it does not decrement the count on the old node being referenced.

CommentFileSizeAuthor
#16 nodereference_count_delete-984428-16.patch2.94 KBgilgabar
#4 nodereference_count-6-984428.patch1.43 KBgilgabar
#4 nodereference_count-7-984428.patch2.02 KBgilgabar
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fourmi4x’s picture

fourmi4x CreditAttribution: fourmi4x commented

Many thanks for taking care of this issue !
EDIT : Let me know if there is anything I can do to help !

Khalor’s picture

Khalor CreditAttribution: Khalor commented

Also experiencing the same issue

gilgabar’s picture

gilgabar CreditAttribution: gilgabar commented
Version: 6.x-1.x-dev » 7.x-1.x-dev

This problem exists for the 7.x version as well. It should be easier to solve there, so I'm going to start with that.

The issue is that in hook_nodeapi the node object only contains the updated data. So if you've updated a node reference, then you only have access to the new nid. The nid that was previously referenced is no longer available, so there is no way to tell it to recalculate its count.

In the 7.x hook_nodeapi equivalent functions the node object includes a copy of the previous node data, so it should be a straightforward task to check if it has changed and then if it has, notify both the new node and the old node that they should update their counts.

Backporting that solution to the 6.x version will require finding a way to get a copy of the original node object into hook_nodeapi similar to what is happening in 7.x.

gilgabar’s picture

gilgabar CreditAttribution: gilgabar commented
Status: Active » Needs review
FileSize
nodereference_count-7-984428.patch2.02 KB
nodereference_count-6-984428.patch1.43 KB

Patches are attached for both 6 and 7. The respective dev releases should also be updated shortly. Please review and confirm that the changes fix the issue and don't cause new ones.

sheldon.nelson’s picture

sheldon.nelson CreditAttribution: sheldon.nelson commented
Priority: Normal » Major
  1. when adding nodes the increment works correctly, but after i remove all the nodes referencing that node the count stops at 1.
  2. if there are no nodes referencing the counter is blank(not zero/0), has no value
gilgabar’s picture

gilgabar CreditAttribution: gilgabar commented

Thanks for the report, which version are you testing?

sheldon.nelson’s picture

sheldon.nelson CreditAttribution: sheldon.nelson commented

version 7.x-1.x-dev

Andrew Schulman’s picture

Andrew Schulman CreditAttribution: Andrew Schulman commented
Status: Needs review » Needs work

Subscribing

gilgabar’s picture

gilgabar CreditAttribution: gilgabar commented

It looks like the first problem is that hook_node_delete() is called before anything is actually deleted, so when it triggers an update of the count all the same stuff is still there, so the number stays the same. We'll need to come up with an alternate mechanism so that updates are triggered after the references are actually deleted.

The second problem appears to be a bug with drupal_render(). A short term fix is to append a space to the count, so that it will be rendered even if it is zero.

Adam S’s picture

Adam S CreditAttribution: Adam S commented

Oh, so the problem is that it stops at 1? But, will work correct otherwise?

gilgabar’s picture

gilgabar CreditAttribution: gilgabar commented

No, the description in #5 is inaccurate. It is calculating counts before references are deleted, so if you delete one reference the count will be off by one. If you subsequently resave the node it will get the correct count because at that point the reference has actually been deleted.

Also the drupal_render() bug appears to be fixed as of the current release of Drupal 7.

goldlilys’s picture

goldlilys CreditAttribution: goldlilys commented

Subscribing

borort’s picture

borort CreditAttribution: borort commented

I have the same problem as #5 "when adding nodes the increment works correctly, but after i remove all the nodes referencing that node the count stops at 1." (version 7.x-1.x-dev). Anybody find solutions yet?

gilgabar’s picture

gilgabar CreditAttribution: gilgabar commented

In D6 hook_nodeapi() delete is called after the delete query has been executed, so it is possible to just get a count at that point that accurately reflects the current number of node references. In D7 hook_node_delete() is called before the delete query has been executed, so when you get a count of the node references it reflects the number that existed immediately prior to the delete, rather than immediately after which is what we would want.

I haven't had a lot of time or motivation to look into a solution or workaround yet. If anyone has any ideas about ways to deal with this, please chime in. Obviously a hook_node_post_delete() would be ideal, but that isn't going to happen in D7.

borort’s picture

borort CreditAttribution: borort commented

Thanks gilgabar. I have tried to modify the core module "node.module" of D7 to allow call of hook_node_delete() after the delete query executed (I know this is not a good idea to modify the core) and it works. But I am not sure if this will affect other things. I am also not sure about the reason why in D7, hook_node_delete() is called before the delete query executed which is different from in D6. Otherwise, the option is to look into how Node Reference Count module could be implemented to adjust to this D7 issue. Looking for more solutions.

function node_delete_multiple($nids) {
  $transaction = db_transaction();
  if (!empty($nids)) {
    $nodes = node_load_multiple($nids, array());

    try {
      

      // Delete after calling hooks so that they can query node tables as needed.
      db_delete('node')
        ->condition('nid', $nids, 'IN')
        ->execute();
      db_delete('node_revision')
        ->condition('nid', $nids, 'IN')
        ->execute();
      db_delete('history')
        ->condition('nid', $nids, 'IN')
        ->execute();
      db_delete('node_access')
       ->condition('nid', $nids, 'IN')
       ->execute();
	   
	   foreach ($nodes as $nid => $node) {
        // Call the node-specific callback (if any):
        node_invoke($node, 'delete');
        module_invoke_all('node_delete', $node);
        module_invoke_all('entity_delete', $node, 'node');
        field_attach_delete('node', $node);

        // Remove this node from the search index if needed.
        // This code is implemented in node module rather than in search module,
        // because node module is implementing search module's API, not the other
        // way around.
        if (module_exists('search')) {
          search_reindex($nid, 'node');
        }
      }
	   
	   
    }
    catch (Exception $e) {
      $transaction->rollback();
      watchdog_exception('node', $e);
      throw $e;
    }

    // Clear the page and block and node_load_multiple caches.
    entity_get_controller('node')->resetCache();
  }
}
gilgabar’s picture

gilgabar CreditAttribution: gilgabar commented
Status: Needs work » Needs review
FileSize
nodereference_count_delete-984428-16.patch2.94 KB

I think I have a reasonable workaround for this issue that doesn't involve hacking core. It's not ideal, but it appears to work. Please test the attached patch and make sure it fixes the problem for you, doesn't create new ones, etc.

borort’s picture

borort CreditAttribution: borort commented

Thanks gilgabar. This works perfectly.

gilgabar’s picture

gilgabar CreditAttribution: gilgabar commented
Status: Needs review » Fixed

Great, it has been committed and should show up in a dev release shortly. If there are no further issues related to this over the next couple of weeks I think it should be safe to roll a 7.x-1.0 release.

Status: Fixed » Closed (fixed)

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