A taxonomy term synonym clears after re-ordering using drag-and-drop feature

Losing data this easily sounds like a critical issue to me.

Same problem, synonyms got deleted after reordering terms with ajax.

Line "353" in "taxonomy.module", inside the function "taxonomy_save_term"

db_query('DELETE FROM {term_synonym} WHERE tid = %d', $form_values['tid']);

runs also on reordering so synonyms get deleted. As a temporary patch I added an "if" to prevent deletion:

and seems to work, but that's just a trick

Devis Lucato

Same problem here, in 6.6.

I have over 150 terms in one of my given vocabularies, and I tend to lose all but 25 synonyms when re-ordering using drag and drop.
It seems as though few people seem to use synonyms, so this issue may go unnoticed, as it seems to have for more than half a year :P

If it's still killing me as much as it does, I'm going to look into making a patch, though i hope someone with a little more experience can take a crack at it.

Confirmed that the same bug is present in Drupal 7. And not just synonyms, related terms too.

We'll need to fix it in the development version first then backport.

OK it's pretty obvious why this is happening, but not necessarily easy to fix in a clean way.

The issue is that when re-ordering the taxonomy tree, the terms are loaded via taxonomy_get_tree() - this does a direct query on the term_data tables, which doesn't contain synonyms or related terms. taxonomy_get_term() in 6 or taxonomy_term_load() in Drupal 7 also don't load synonyms into the term object so aren't any use here either. So when these terms are saved, they're incomplete, and taxonomy_term_save() deletes the synonyms and related terms - since they're not a part of the term that's passed in.

For Drupal 7, I think we should add an explicit flag to vocabularies for whether they support synonyms or related terms, then load these into the term object in hook_taxonomy_term_load() - and ensure that's used to get the term objects consistently. That would make the object returned by taxonomy_term_load and saved by taxonomy_term_save actually the same object.

For Drupal 6, we either need to query the synonym and related terms tables directly before saving the term, or we could use a direct db_query to update the parent information instead of calling taxonomy_term_save(). The second option would be more performant I think.

Here's a working patch for Drupal 6 - re-ordering works without data-loss.

Since the changes necessary to do this cleanly are quite a long way off, I'm tempted to re-roll this for Drupal 7 with a test and suggest we commit it there too.

Re-roll for Drupal 7. No test yet.

The last submitted patch failed testing.

hi all,
i think this patch works not correctlyand it should be fixed in taxonomy_save_term() (for D6)...
georg

to delete synonyms this patch has to be executed too :-) (I forgotten)

gbuske - that doesn't deal with the fact we don't have full term objects here in the first place - so other fields like related terms could equally be destroyed.

Rerolling patch from #8. Patch passed the test locally for me that it failed on testbed.

This is really minor, but according to Crell (#394484-3: DBTNG: Node module) should "single-value fields calls" be on a single line, no need to split them up.

Ok. Fixed the whitespace.

is this most recent patch posted for D7 or D6? it's not too clear whether people are working on the current version and backporting, or just working in D6 from comments...

D7, as it says at the top

In LA codesprint right now - I submitted the last patch for re-testing because when I applied the patch to a fresh d7 install, my manual testing caused the following error when I tried to move the taxonomy term up:

• Warning: include(/Users/rain/Documents/SUNRAIN/sites-in-progress/drupalhead/codesprint/includes/maintenance-page.tpl.php): failed to open stream: No such file or directory in theme_render_template() (line 1145 of /Users/rain/Documents/SUNRAIN/sites-in-progress/drupalhead/codesprint/includes/theme.inc).
• Warning: include(): Failed opening '/Users/rain/Documents/SUNRAIN/sites-in-progress/drupalhead/codesprint/includes/maintenance-page.tpl.php' for inclusion (include_path='.:/Applications/MAMP/bin/php5/lib/php') in theme_render_template() (line 1145 of /Users/rain/Documents/SUNRAIN/sites-in-progress/drupalhead/codesprint/includes/theme.inc).

This error did not show up when I reversed the patch.

I suspected that perhaps with the patches I've already applied today my Drupal install may have had something going on, adding to the problem, so I tried a completely fresh update from Drupal Head and an entirely fresh database (so, 100% clean install). The error still occurred and reversed exactly as it had on my previous test.

I also requested a retest on the patch in #17.

Those last comments seem irrelevant, so this still needs review.

The comment #20 was relevant but incomplete.

This patch no longer work because the tables were renamed from term_data to taxonomy_term_data and from term_hierarchy to taxonomy_term_hierarchy. I will reroll the patch with the new table names.
Because the table names were changed, Drupal tried to show the maintenance page but instead showed a blank page with some warning in watchdog.

There seem to be another problem related to the maintenance page which should not be addressed here.

Here is the rerolled patch which takes into consideration the table name changes.

oh man, that was me that changed those table names... I can test.

this should be backported after commit to HEAD.

Needs to be backported after commit.

I can't reproduce the original book in HEAD now. (yay?)

roychri, could you reproduce the bug? If not, this issue should go back to 6.x.

@Jody Lynn - Yes, I can reproduce the problem.

At first I could not reproduce the problem. But I found a way.

So here is how I reproduce the problem:
1- Installed Drupal7 fresh.
2- Added a new vocabulary with options left as default (tags: off; required: off; multiple select: off)
3- Added 4 terms to that vocabulary
4- Added a synonym to the second term in the middle of the list.
5- Click on "list terms" for that vocabulary.
6- Re-order the second term that I added using drag&drop to the position #3 (was #2).
7- Click "Save".

This should do it.

I think I had trouble with only 2 terms because it would update the other term's weight, not the one with the synonym, even if that was the one I swapped.

Thanks, yes, my problem was I only had 2 terms, seems like you need 3 or more to reproduce.

The patch fixes the problem and code looks good.

fyi, another patch is rtbc that removes synonyms from core.

This patch will still be valid for any module which uses hook_taxonomy_term_*() to maintain it's own data - we should never call taxonomy_term_save() with a fake/incomplete term object.

FYI, #567572: Remove taxonomy synonyms since Field API is better was committed to CVS HEAD.

Retesting the last patch. Let's see if this works.

Still works, still needed.

Committed to HEAD. Thanks!

There's a 6.x equivalent of this patch up there somewhere. Didn't test it, but if you do, mark RTBC.

Hm. And also, we could use some tests on this, come to think of it...

marking "needs work" for tests

I looked into writing a test for this, but now that taxonomy synonyms have been removed, there's nothing left to test for (there's no bug that occurs before or after the patch anymore)

In Drupal 6.14 the synonyms also disappear when using the Reset to aplhabetical button.

This needs to stay at 6.x-dev as that's where it will have to be fixed.

There is also a patch for 6.x per #36 that should be tested per #36.

and please don't alter the title.