Beta breaker: Forum: Leave shadow copy doesn't leave shadow copy.

Yuck. I think forum_nodeapi wins the stupidity contest. Note for example how line 258 adds term_id as a value and yet the foreach on line 249 called for term_ids in keys...

On freshly CVS updated Drupal 6 install, I tried to enable forum module, create two fora ("Test 1" and "Test 2") and then created a forum topic ("Test: Shadow post from forum Test 1 to forum Test 2"). I then went to edit and moved it from "Test 1" to "Test 2" and checked the box saying "Leave shadow copy". This resulted in the forum topic appearing under "Test 2" and not under "Test 1".

I then fetched and (cleanly) applied the patch. Then I created the forum topic "Test #2: Shadow post from forum Test 1 to forum Test 2", again editing after saving it, moving it from "Test 1" to "Test 2" while checking "Leave shadow copy". This time, the entry appears under both "Test 1" and "Test 2".

As far as I can tell, this patch fixes the bug.

Alas. This topic has been moved does not appear in the topic list. So , CNW.

Oh dear. The old forum table held valuable information about the forum topics -- their actual tid. http://drupal.org/node/20295 does not even mention shadow, just dropped the table. Which also means that this drop is totally, absolutely wrong because shadow copy information will be lost. You can't deduce from term_node which is the current topic where the forum node lives.

Aside from fixing this terrible mess I enrolled a modified fgm and webchick patch from http://drupal.org/node/172633 as those changes are indeed badly needed. I fixed all occurences of tn.nid = n.nid I could find as I went inc. node_search .

OK, I removed the node search fix, submitting another issue.

Final note before collapsing back to bed: the forum.schema is from an earlier HEAD so it's quite OK.

Not sure what are the conditions to test this, but I installed D6, enabled Forum module, added some containers and forums, then created a forum topic and moved it to another forum. Succesfully moved, this forum topic has "This topic has been moved" in old forum.

Given meba's comments and my observation that shadow only is displayed when you are leaving behind a shadow, not otherwise, we are fine here.

I agree, it works as expected.

No. Why it would? forum table is just nid,vid,tid. And it's in nodeapi.

Might be a silly question, but why not only store the "shadow" threads in {forum} - that would save bringing back the additional insert that the node types patch removed nearly all the time wouldn't it?

It would be great to add some code comments before this statement:
+ if ($topic->forum_tid != $variables['tid']) {

I have not written that part it's already in core and yet the patch is held back over a comment to that? :(

chx: that's correct. There is nothing wrong with holding a patch back, and taking the opportunity to improve the overall quality of core. That's what we are here for not? :-)

Yeah, I know. But, I have written a three line comment at the place you asked for, so what holds it now?

This is very, very, very minor, but thought I might as well mention it: [...] means the topic appears in two topics, [...] (in the newly added comment) - The topic appears in two topics? I believe this should be "the topic appear in two forums" and the attached patch changes that, and nothing else, in Károly's latest patch.

Ah yes. I mix'd up those, thanks.

Committed to CVS HEAD. Thanks.

A piece of this fix is still missing — see #422218: $node->tid can get the wrong tid if a forum content type has additional vocabularies