casetracker_comment() directly modifies node via sql rather than using nodeapi
| Project: | Case Tracker |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
Jump to:
Casetracker's hook_comment() is directly modifying node tables, rather than using the nodeapi. This causes problems for modules which are relying on all node updates happening via the nodeapi, for example if some action should be triggered when a case node is updated.
I noticed the problem due to another side effect: it becomes quite difficult to determine the date/time that a case was closed. Normally this could be determined by getting the timestamp from the node_revisions table for the first closed version of a case. But because casetracker updates the node directly, this timetamp isn't updated so it's necessary to write some complex logic looking both here and in the comments table for the correct timestamp to use.
The problematic code is in this excerpt:
function casetracker_comment(&$comment, $op) {
...
db_query("UPDATE {node} SET title = '%s' WHERE nid = %d AND vid = %d", $case_data['new']->case_title, $comment['nid'], $comment['revision_id']);
db_query("UPDATE {node_revisions} SET title = '%s' WHERE nid = %d AND vid = %d", $case_data['new']->case_title, $comment['nid'], $comment['revision_id']);
db_query("UPDATE {casetracker_case} SET assign_to = %d, case_status_id = %d, case_priority_id = %d, case_type_id = %d, pid = %d WHERE nid = %d AND vid = %d ", $case_data['new']->assign_to, $case_data['new']->case_status_id, $case_data['new']->case_priority_id, $case_data['new']->case_type_id, $case_data['new']->prid, $comment['nid'], $comment['revision_id']);
...
}I'd suggest this should instead be using node_submit() and node_save() to create a new revision of the node and fire off any nodeapi hooks.
#1
i subscribe hoping this issue helps solving of this: http://drupal.org/node/230692
#2
It's unclear to me the best way to address this issue, this problem code specifically has been removed. Comment submission no longer update these tables directly, but this is at the expense of the ability to change the title of the node in a comment.
I would like to have this feature back, and make sure that it will play nice with modules that take meaningful action on node updates (like notifications module) and not trigger the same action twice, ie on comment submission and node update.
#3
The ability to modify the node title through comments is a critical feature of this module that should be preserved at all costs — especially since it is a legacy feature that users of previous versions of this module have come to rely on.
Removing this code may have helped casetracker play nicer with other modules, but only at the expense of one of its most critical features.
I move that this functionality should be restored until a there is proper fix that can preserve it.
#4
Hmm, my intention in filing this issue wasn't to strip out the functionality. I simply wanted a way to find the date/time that a case was closed. And creating a new revision of the node would make for an easy way to query this timestamp.
#5
The problem is not the feature, but how the feature was implemented; any changes made to a node should be made in
hook_nodeapi().#6
I completely agree that it should use
hook_nodeapi(). But I don't agree with leaving a module hobbled like this.Either provide a fix that uses
hook_nodeapi()properly, or leave it alone until someone can.#7
Hi folks, I'm happy to see attention around this issue. However, I'm not going to have the time to re-examine how node and views modules both utilize the title field in the node and node_revision tables and come up with a solution for at little while.
***I'd very much appreciate a deeper analysis than just "use hook_nodeapi() properly" which could serve as the basis for a solution.***