Needs review
Project:
Directory
Version:
5.x-1.x-dev
Component:
Code
Priority:
Minor
Category:
Task
Assigned:
Reporter:
Created:
18 May 2007 at 15:03 UTC
Updated:
20 May 2007 at 12:23 UTC
Jump to comment: Most recent file
This is strictly speaking a Drupal core bug because taxonomy_term_count_nodes returns wrong count in some circumstances.
See core issue for details: http://drupal.org/node/144969.
However, we'd be lucky is the fix I posted for review is committed for Drupal-6, and it is already certain that the proposed fix won't be accepted for Drupal-5.
I am therefore proposing to lift the function from taxonomy.module, and create our own, for own own use.
I have created the function directory_taxonomy_term_count_nodes() which is basically taxonomy_term_count_nodes() + my patch.
This function allows us not only to fix the bug, but also to add some functionality.
Now, instead of having the term with the number of nodes (i.e. Cheetah (5)), we have two numbers after the term:
1) the number of the term's own nodes,
2) the number of nodes of the term's children (excluding duplicates).
i.e. Cheetah (1 + 3))
This is a fairly large patch. I'll polish it further tomorrow.
If you have no objection, I'll commit it tomorrow.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | taxonomy.node_.count_.replacement3.patch | 7.68 KB | beginner |
| #2 | taxonomy.node_.count_.replacement2.patch | 7.35 KB | beginner |
| #1 | taxonomy.node_.count_.replacement.patch | 6.19 KB | beginner |
Comments
Comment #1
beginner commentedHere is the patch.
It is not that large, after all.
It is 3 things:
* the new replacement function (at the bottom)
* change the name of the function called from elsewhere in the code.
* since the return value is different (an array instead of an integer), change the handling of the return value.
Comment #2
beginner commentedI have improved the patch with a database caching mechanism for the result of taxonomy_node_count(), for better performance.
In order for me to understand how to use the caching mechanism, I looked around, asked a few questions ... and wrote this handbook page : http://drupal.org/node/145279.
I'll review my own patch tomorrow, when I am less tired, and commit it then.
Comment #3
beginner commentedAfter some more time spent debugging the new function, we have a spiffy new feature that works efficiently.
In order to avoid overheads, the minimum cache duration is set to 1 hour: i.e. the data returned by the function is accurate at worst as of one hour ago.
Attached is the patch that I am finally committing.
Watch the core function!
Don't close this issue, yet!
I am committing this as part of the directory.module, because my fix wouldn't be accepted for Drupal 5.
However, it remains at its base a Drupal core bug.
Leave this issue open until the core issue is resolved: http://drupal.org/node/144969.
When the fix has been committed into core, we can remove the duplicate function from this module and start using the core function again.