Deleting forums leaves stale data
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | forum.module |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | active |
While setting up a view today, I suddenly discovered 200 forum posts from a forum I deleted over year ago (on a 5.x site). This is because forum.module doesn't delete posts when you delete a forum, even though it says it does, leaving potentially thousands of orphaned nodes in the database (and otherwise difficult to find - none of the filter settings in admin/content/content will find them for example because they're not in a term to be filtered by).
To reproduce. Create one forum. Post one forum topic in it. Delete the forum in admin/content/forum/edit/forum/1 and see this message:
Deleting a forum or container will also delete its sub-forums and posts, if any. This action cannot be undone.
After you delete, go back to admin/content/node and your post will still be there.
http://api.drupal.org/api/function/forum_confirm_delete_submit/6 doesn't make any attempt to delete any nodes at all, and hasn't since it was introduced.
If this is by design, then the message shouldn't say "delete [...] posts" because it's not doing that. But really, if you delete a forum, you ought to be able to delete all the posts at the same time rather than go separately to admin/content/node beforehand. The main issue though, is you can't go back and delete those forum posts all together once you've deleted the forum not possible afterwards because you can no longer filter by that term.
At this stage in the release cycle, and especially now there's multiple node types in forums, I'm tempted to just submit a patch for the string and revisit in D7. Since this has led to 200 stale nodes languishing somewhere in the depths of my db, and only a custom view to get rid of them, I'm marking as critical though. It'll be a trivial fix to the string, probably not straightforward to sort out a delete action.
#1
#2
OK here's a patch to just change the string.
t('Are you sure you want to delete the forum %name?', array('%name' => $term->name)), 'admin/content/forum', t('Deleting a forum or container will also delete its sub-forums if any. To delete posts in this forum, visit <a href="@content_admin">content administration</a> first. This action cannot be undone.', array('@content-admin' => 'url(admin/content/node)')),For D7, rather than changing the function to actually delete the posts (since a forum could include all kinds of content types now), it might be better to force the user to reassign posts to another forum if there's any nodes associated with the one they're deleting - this is an option with a couple of different forum packages I think. That's a feature request though.
#3
I agree. The delete would be quite difficult esp that if there is a shadow copy then it does not need to be deleted... Also it's not entirely impossible that there are other navigation means leading to said nodes (you can tag them for eg) so deleting might not be desired at all.
#4
I can't roll a patch right now, unfortunately, but there is a double space between sentences in this string (and is it missing a comma near "if any")?
#5
Thanks Keith, both fixed in this one.
#6
Agreed that we'd rather document this good now, and revisit this in D7. Even if this is a string change for translators, oh well. Committed this one.
#7
Thanks Gabor, glad my 3am bug report made some sense.
Since this still leaves admins with no (easy) way to find all their 'deleted' forum topics even with the string change, I'm changing the status back to active and bumping to D7 for continuity.
#8
#9
Patch to modify message and watchdog strings to remove verbiage stating that posts were deleted as well, since well, they're not.
#10
Damn, missed that one. Since it only removes words from a string, any chance this could be backported to 6.x?
edit: and the string changes look fine to me.
#11
I'm willing to write a patch that will give the option to a) delete posts from a deleted forum or b) move the posts to another forum if others desire this behavior. One or two +1s should give be convincing enough for me :)
#12
@boydjd - a) is going to be tricky now we have multiple content types in forums, and some of them may also be represented outside forums as well. It's going to be very hard to tell what should be considered 'inside' a forum. b) is what I currently do direct in the database when deleting forums, and it's also a feature of a couple different forum packages, so +2 ;)
However, we should get #9 RTBCed and committed first
#13
I committed the help text fixes to D7. I'm not sure this is critical so lowering the priority to 'normal'.
There is no good or wrong here -- the solution will be non-trivial if we want to take into account all the use cases. Posts can belong to multiple forums (or multiple taxonomy terms), different node types can be in a forum, etc. We can investigate this more, but I'm tempted to suggest that we should focus our efforts on the mass node administration pages. They already allow complex filtering and mass-deletion. There is room for improvement there that might make this particular issue easier to deal with as well. Food for thought, huh.
#14
I'm pretty sure posts can't belong to multiple forums - that ought to be pretty much impossible given forum_form_alter apart from shadow forum topics no?
The way I see forum deletion working would be you go to delete a forum, and if there's any posts in it, you get a required select form with all forums in it (apart from the one being deleted) and some help text explaining that all forum topics will be moved to the new forum, alongside a pointer to admin/content/node.
When you submit the form it does UPDATE {forum} SET tid = $tid WHERE tid = $old_tid; then the same in {term_node} - with probably some special handling for shadow forum topics to avoid duplicates. As noted - this is what I currently do with deleted forums by hand to avoid orphaned topics. Once that's done, taxonomy_del_term() handles the rest as now.
That way - actual deletion is down to the administrator in admin/content/node, but we prevent the orphaning of forum topics.
Also, as far as I know
taxonomy_del_term()doesn't touch records in the {forum} table, and I can't see anything to correct this elsewhere in forum module, which means a whole bunch of stale data in there. So marking back to critical on that basis, and active, since there's no patch here with the string corrections committed.#15
Retitling since we' no longer dealing with the actual deletion of posts in this issue.
#16
@catch What would be the suggested handling for deleting a forum when no other forum exists?
I think we should develop some use cases/scenarios here for forum deletion. Off the top of my head:
1) Delete a forum, with no posts
2) Delete a forum, with posts, no other forums exist
3) Delete a forum, with posts, other forums exist
#17
Related issue: http://drupal.org/node/90202
#18
I don't understand, #0 claims that deleting a forum does not delete the associated nodes.
#90202: Deleting forum deletes moved forum topics and http://api.drupal.org/api/function/forum_taxonomy indicate otherwise!
#19
I just manually created a forum and forum topic, then deleted the forum, and the forum topic still shows up at admin/content/node. So this is still valid, but I'm not sure why that's the case..
#20
wait a minute... moved topics get deleted... and non-moved topics doesn't get deleted... does someone sees a pattern there???