So, i mark this as critical because i suspect a more serious problem...

Create an article node.
Edit the article and delete it.
Click the "Recent Content" link (/tracker)
I get multiple tracker notices:

# Notice: Undefined property: stdClass::$comment_count in tracker_page() (line 53 of /home/goargoli/domains/el-drupal7.dyndns.org/public_html/modules/tracker/tracker.pages.inc).
# Notice: Undefined property: stdClass::$type in tracker_page() (line 63 of /home/goargoli/domains/el-drupal7.dyndns.org/public_html/modules/tracker/tracker.pages.inc).
.
.
.
.

See: http://el-drupal7.dyndns.org/en/tracker

The strange here is that my content was updated "40 years 10 months ago" see the tracker's table last coloumn...

note: This is a drupal 7 beta2 upgraded from beta1.

Comments

FanisTsiros’s picture

Priority: Critical » Normal

Ok this is not critical.
Investigating the tracker db table there are deleted nodes there. Node table is ok.

bmateus’s picture

I've installed drupal 7 beta2 directly, and this happens also to me.

Looks like somethings still referencing those deleted nodes. The date is weird, though.

ericduran’s picture

Version: 7.0-beta2 » 7.x-dev

Ok, so this is an actual bug.

Here are my finding, I might be completely wrong :-p

I was looking at the tracker module and this is the problem.

The way the tracker module deletes is record, is that it queries the node table to to see if the node exists and if it doesn't exists it deletes the record. Now this should work except that the node isn't actually delete until after the hook_node_delete is called.

Here's the node delete code :

<?php
    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');
      }
    }

    // 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();
?>

As you can see it does the delete after the module_invoke_all.

Also this is a documentation bug, because according to http://api.drupal.org/api/drupal/modules--node--node.api.php/function/ho...
"This hook is invoked from node_delete_multiple() after the node has been removed from the node table in the database," which isn't the case.

Hmm, so now for the solution.
Not really a solution but a question. Is this a bug in the tracker or in node_delete_multiple. If we look at the comment_delete_multiple, comments are deleted from the database before calling the hooks. Also do modules need to query the database to act on the node object? I mean it is being pass on the hook.

Anyways, someone with more knowledge on this issue will need to provide guidance as to which direction to go. I'll be happy to provide a patch after that. :-)

ericduran’s picture

Ok, so I found the patch that switch the order in which node_delete_multiple runs. #890790: deleting nodes does not delete their comments.. It seems that this was causing and issue with comments not being deleted.

dave reid’s picture

Status: Active » Needs review
StatusFileSize
new1.91 KB

Tracker just needs to not be stupid on hook_node_delete(). There's no need to call its API function. Just delete the dang records.

ericduran’s picture

StatusFileSize
new2.09 KB

Cool,

Above patch doesn't work, the variable $nid doesn't exists.

This is a new patch using the correct variable $node->nid, and I also moved the test to the testTrackerAll function as it makes more sense there since it's where the other test related to the tracker listing is happening.

ericduran’s picture

StatusFileSize
new2.46 KB

Minor cosmetic fixes.

Eolmornedhel’s picture

Not sure whether to post this here, but I too had an issue with undefined property errors. I found I could get rid of them by simply surrounding the properties concerned with isset() as in : 'title' => array('data' => l(isset($node->title), 'node/' . $node->nid) . ' ' . theme('mark', array('type' => node_mark($node->nid, $node->changed)))).

This however results in a title of "1" for each item of tracked content. I also have the problem of "Last Updated" displaying 40 years 11 months ago. This is after applying #7.

Running D7 Beta3.

PS. This occurs only when viewing tracking information, not when performing an action on a node.

ericduran’s picture

@Eolmornedhel in order to test this patch correctly you need to apply it before deleting the nodes. This patch doesn't fix the errors you had before applying the patch. This is because the old data is still in the table and there's no function that's removing all that orphan data. Can you test this path on a new site? then create and delete nodes and see if you still get the error?

Also you can't change the l function to take isset the reason why those notices are showing up is because the $node object is empty (which shouldn't be the case).

@Dave Reid : Do we need to provide support for this? In other words does this patch need an update hook to remove all the orphan nodes from the tracker table?

Eolmornedhel’s picture

Hi Eric, I only recently noticed this issue and it is not to do with the act of "deleting" nodes. I get the errors when viewing the tracking info for a user, all existing content shows correctly. Any nodes that don't actually exist (and that I can't find in the database) display as "Author" : "Anonymous", have no "Type", a "Title" of "New", and a date of 40 years 11 months ago.

I've never received any errors when deleting nodes. However, just checked tracking info for a couple more users (mine is just a site for learning Drupal, so I know who has and who hasn't ever deleted a node), any user who has at some stage deleted a node gets the anomalous information described above displayed presumably for the deleted nodes. A user who has never deleted a node gets the correct info displayed and no errors.

After applying #7 I can safely create and delete nodes without producing any further erroneous tracking data, so the problem for me is solved apart from locating the erroneous data in the database, do you know where it is stored? as I can't find it anywhere.

Regards

Eolmornedhel’s picture

Sorted, just needed to delete the relevant nids from the tracker_node and tracker_users tables.

All is now working as expected.

ericduran’s picture

@Eolmornedhel cool. Maybe someone can switch this to RTBC.

catch’s picture

Priority: Normal » Major
Status: Needs review » Reviewed & tested by the community
joachim’s picture

Status: Reviewed & tested by the community » Needs work
+++ modules/tracker/tracker.module	11 Nov 2010 22:59:16 -0000
@@ -164,24 +164,29 @@ function _tracker_user_access($account) 
- * Implements hook_nodeapi_insert().
+ * Implements hook_node_insert().

AFAIK these clean-up changes have already been committed -- at the very least there is already a RTBC patch on a separate issue. Kittens! ;)

Powered by Dreditor.

catch’s picture

Status: Needs work » Needs review

Retesting to see if this still applies or not.

joachim’s picture

catch’s picture

StatusFileSize
new2.05 KB

Re-rolled.

joachim’s picture

Status: Needs review » Reviewed & tested by the community

Same patch as before, with code comments handled in that other issue removed. Looks like RTBC to me :)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)

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