when you delete a project node, we attempt to delete all traces of the project: the node, releases, subscriptions, issues, etc. unfortunately, we delete the wrong thing to try to delete all the comments. :(

currently, we just do this:

  db_query('DELETE FROM {project_comments} WHERE nid = %d', $node->nid);

sadly, the nid in the {project_comments} table is the nid of the issue the comment belongs to, not the project. in fact, {project_comments} has no reference to the project nid at all.

so, we need to use a subselect (or equivalent), and delete these first, *before* deleting from {project_issues}.

perhaps this is best cleaned up by the move to using real comments for issues... (http://drupal.org/node/18920).

this also means that project_update_3() is wrong. :(

CommentFileSizeAuthor
#11 project_comments_delete47_2.patch1.9 KBhunmonk
#9 project_comments_delete47_1.patch1.32 KBhunmonk
#8 project_comments_delete_1.patch1.78 KBhunmonk
#5 project_comments_delete47_0.patch1.17 KBhunmonk
#4 project_comments_delete_0.patch1.17 KBhunmonk
#3 project_comments_delete47.patch1.08 KBhunmonk
#2 project_comments_delete.patch1.07 KBhunmonk

Comments

hunmonk’s picture

hunmonk commented
Version: 4.7.x-1.x-dev » 5.x-1.x-dev
Status: Active » Needs review

tested on HEAD, and works perfectly

hunmonk’s picture

hunmonk commented
StatusFileSize
new project_comments_delete.patch1.07 KB

...and the patch

hunmonk’s picture

hunmonk commented
Assigned: Unassigned » hunmonk
StatusFileSize
new project_comments_delete47.patch1.08 KB

patch for 4.7 -- untested.

hunmonk’s picture

hunmonk commented
StatusFileSize
new project_comments_delete_0.patch1.17 KB

bah. subselects won't work in 4.7 and 5 :(

attached is for HEAD, tested and working.

hunmonk’s picture

hunmonk commented
StatusFileSize
new project_comments_delete47_0.patch1.17 KB

for 4.7, untested

dww’s picture

dww
we/he/they
commented
Status: Needs review » Needs work

Wow, all kinds of bugs related to this code. ;) The {project*} tables are in fact getting cleaned out, but we never actually delete the issue nodes themselves. :( So, you end up with a weird skeleton of the issue nodes for that project. Technically, the patch fixes the specific problem of the comments being left behind, but we should probably fix really deleting the issues, too, while we're at it.

hunmonk’s picture

hunmonk commented

i think the best approach might be to:

  1. project_issue_issue_nodeapi() should have a delete op that properly deals with all cleanup related to deletion of an issue node
  2. project_issue_project_nodeapi()'s delete op should pull all associated issues for the project, then call node_delete() on them

sound like a plan?

hunmonk’s picture

hunmonk commented
Status: Needs work » Needs review
StatusFileSize
new project_comments_delete_1.patch1.78 KB

ok, let's try this again...

patch for HEAD

hunmonk’s picture

hunmonk commented
StatusFileSize
new project_comments_delete47_1.patch1.32 KB

... and 4.7

dww’s picture

dww
we/he/they
commented
Status: Needs review » Needs work

tested #9 on a 4.7.x-2.* site, works like a charm. code looks good.

the one lingering problem is that we don't delete the files in the filesystem that are attached to the comments we're deleting. :(

hunmonk’s picture

hunmonk commented
StatusFileSize
new project_comments_delete47_2.patch1.9 KB

attached should handle file deletions for comments as well. untested. also, this patch should apply to 5.x-1.x, and should probably be tested there as well.

dww’s picture

dww
we/he/they
commented
Status: Needs work » Reviewed & tested by the community

excellent. tested #11 on 4.7.x-2.* and 5.x-1.* -- works like a charm. code is good. #8 is still good for HEAD, since comment_upload() is cleaning up the files for us in that case. RTBC!

hunmonk’s picture

hunmonk commented
Status: Reviewed & tested by the community » Fixed

fixed in 4.7.x-1.x, 4.7.x-2.*, 5.x-1.*, HEAD. phew!

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)