Taxonomy term references and autocomplete fields should support duplicate term names [#2841128]

I'm currently working on a project where a taxonomy vocabulary contains duplicate term names.

In short, one master vocabulary contains terms which have term-reference fields referencing the terms of another sub-vocabulary.

If, for example, there are multiple terms within this referenced sub-vocabulary which share the same name (ex. "Accessories:[tid:111], Accessories[tid:222], Accessories[tid:333]"), any save operation using the 'taxonomy/term/[tid]/edit' page (and/or using the Taxonomy Manager contrib module), will result in a term reference to ONLY the last duplicated term in that vocabulary.

There are 2 issues with this bug:
1) Viewing the taxonomy term:

If one were simply viewing the 'taxonomy/term/[tid]' page, any term reference field contains ONLY the term name ("Accessories" in our example) with NO reference to a tid. From the looks of things, things do indeed look correct but that reference could be referencing any of our duplicated terms (Accessories:[tid:111], Accessories[tid:222], Accessories[tid:333]) and a user wouldn't know that the term referenced should actually reference tid 222 when, in fact, according to the database the term referenced is actually tid 333.

2) Editing the taxonomy term:

Using the autocomplete widget for a taxonomy term reference field, there is currently nothing being shown to users who have purposely created duplicate terms. If there are 3 "Accessories" terms, only one shows when choosing a term from the autocomplete field.

Furthermore, the only way to guarantee that a term reference to a duplicated term is indeed correct, one must use something like a feeds import which allows a hard-coded reference to a tid. That's fine until one uses the 'taxonomy/term/[tid]/edit' page and hits the save button (whether they changed a term reference field or not!)

For example, if you did a feeds import to save your term references but you wanted to edit something in the term's description field (again using the 'taxonomy/term/[tid]/edit' page) , you'd make your change to the description field and hit 'save'.

Now, however, because taxonomy_term_validate() uses taxonomy_term_load_multiple() which currently only matches on 'name' and 'vid' alone (in the $conditions array), the array_pop of that result will only use the last duplicated term in the list ("Accessories[tid:333]" in our example), without the user knowing that although they intended for the referenced term to be, say, the 2nd term in the list.

In short, the user edited some text in the description field but without knowing it, because there are duplicate terms in their referenced vocab, they've also, unknowingly, just edited the term reference field(s) as well.

Comment	File	Size	Author
#22	taxonomy-view-edit-vocabulary-reference-2841128-22.patch	1.17 KB	puspanjalim
#22	7.x: PHP 5.3 & MySQL 5.5 2,030 pass, 1 fail
#16	taxonomy-autocomplete-should-use-tid-2841128-16.patch	10.05 KB	mikeytown2
#16	7.x: PHP 5.3 & MySQL 5.5 2,030 pass
#11	taxonomy-autocomplete-should-use-tid-2841128-11.patch	10.04 KB	AaronELBorg
#11	7.x: PHP 5.3 & MySQL 5.5 2,030 pass
#8	taxonomy-autocomplete-should-use-tid-2841128-8.patch	5.62 KB	AaronELBorg
#8	7.x: PHP 5.3 & MySQL 5.5 2,028 pass, 2 fail PHP 5.4 & MySQL 5.5 2,028 pass, 2 fail PHP 7 & MySQL 5.5 2,028 pass, 2 fail
#6	taxonomy-autocomplete-should-use-tid-2841128-6.patch	4.12 KB	mikeytown2
#6	7.x: PHP 5.3 & MySQL 5.5 2,024 pass, 14 fail
#4	taxonomy-autocomplete-should-use-tid-2841128-4.patch	4.06 KB	AaronELBorg
#4	7.x: PHP 5.3 & MySQL 5.5 2,030 pass, 1 fail
#2	taxonomy-autocomplete-should-use-tid-2841128-1.patch	4.08 KB	AaronELBorg
#2	7.x: PHP 5.3 & MySQL 5.5 2,003 pass, 41 fail

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

4 January 2017 at 20:27

AaronELBorg created an issue. See original summary.

Comment #2

AaronELBorg CreditAttribution: AaronELBorg commented 4 January 2017 at 20:37

File	Size
taxonomy-autocomplete-should-use-tid-2841128-1.patch	4.08 KB
7.x: PHP 5.3 & MySQL 5.5 2,003 pass, 41 fail

Comment #3

4 January 2017 at 20:58

Status:

Needs review

» Needs work

The last submitted patch, 2: taxonomy-autocomplete-should-use-tid-2841128-1.patch, failed testing.

Comment #4

AaronELBorg CreditAttribution: AaronELBorg commented 4 January 2017 at 22:09

Status:

Needs work

» Needs review

File	Size
taxonomy-autocomplete-should-use-tid-2841128-4.patch	4.06 KB
7.x: PHP 5.3 & MySQL 5.5 2,030 pass, 1 fail

Hmmm... trailing whitespace?

Let's try this.

Comment #5

4 January 2017 at 22:20

Status:

Needs review

» Needs work

The last submitted patch, 4: taxonomy-autocomplete-should-use-tid-2841128-4.patch, failed testing.

Comment #6

mikeytown2 CreditAttribution: mikeytown2 commented 4 January 2017 at 23:32

Status:

Needs work

» Needs review

File	Size
taxonomy-autocomplete-should-use-tid-2841128-6.patch	4.12 KB
7.x: PHP 5.3 & MySQL 5.5 2,024 pass, 14 fail

Comment #7

4 January 2017 at 23:44

Status:

Needs review

» Needs work

The last submitted patch, 6: taxonomy-autocomplete-should-use-tid-2841128-6.patch, failed testing.

Comment #8

AaronELBorg CreditAttribution: AaronELBorg commented 9 January 2017 at 20:14

File	Size
taxonomy-autocomplete-should-use-tid-2841128-8.patch	5.62 KB
7.x: PHP 5.3 & MySQL 5.5 2,028 pass, 2 fail PHP 5.4 & MySQL 5.5 2,028 pass, 2 fail PHP 7 & MySQL 5.5 2,028 pass, 2 fail

Upon further testing, there were problems with terms that contained double-quotes as well as commas.

I also familiarized myself with the tests included in core. It should now pass the Menu.MenuBreadcrumbTestCase.

Comment #9

AaronELBorg CreditAttribution: AaronELBorg commented 9 January 2017 at 20:16

Status:

Needs work

» Needs review

Comment #10

9 January 2017 at 22:26

Status:

Needs review

» Needs work

The last submitted patch, 8: taxonomy-autocomplete-should-use-tid-2841128-8.patch, failed testing.

Comment #11

AaronELBorg CreditAttribution: AaronELBorg commented 10 January 2017 at 22:21

Status:

Needs work

» Needs review

File	Size
taxonomy-autocomplete-should-use-tid-2841128-11.patch	10.04 KB
7.x: PHP 5.3 & MySQL 5.5 2,030 pass

From looking at taxonomy.test, my changes will make it fail every time as the returned data is no longer limited to simply:

$term_objects['term2']->name

They'll now look like

$term_objects['term2']->name . '[term-id:' . $term_objects['term2']->tid . ']';

Included in the attached patch (#11) are changes to taxonomy.test. Using this patched version, my changes to taxonomy.module and taxonomy.pages.inc should pass with no failures.

How to get drupal.org to use my newly patched taxonomy.test is another issue, however.

If anyone has suggestions/recommendations, please let me know.

Comment #12

10 January 2017 at 22:33

Status:

Needs review

» Needs work

The last submitted patch, 11: taxonomy-autocomplete-should-use-tid-2841128-11.patch, failed testing.

Comment #13

AaronELBorg CreditAttribution: AaronELBorg commented 10 January 2017 at 23:27

What am I missing???

aaron@frank:/var/www/html/drupal7/modules/taxonomy$ git apply -v taxonomy-autocomplete-should-use-tid-2841128-11.patch
Checking patch modules/taxonomy/taxonomy.module...
Checking patch modules/taxonomy/taxonomy.pages.inc...
Checking patch modules/taxonomy/taxonomy.test...
Applied patch modules/taxonomy/taxonomy.module cleanly.
Applied patch modules/taxonomy/taxonomy.pages.inc cleanly.
Applied patch modules/taxonomy/taxonomy.test cleanly.

Applied patch modules/taxonomy/taxonomy.module cleanly.
Applied patch modules/taxonomy/taxonomy.pages.inc cleanly.
Applied patch modules/taxonomy/taxonomy.test cleanly.

Comment #14

AaronELBorg CreditAttribution: AaronELBorg commented 10 January 2017 at 22:52

Status:

Needs work

» Needs review

Comment #15

10 January 2017 at 23:07

Status:

Needs review

» Needs work

The last submitted patch, 11: taxonomy-autocomplete-should-use-tid-2841128-11.patch, failed testing.

Comment #16

mikeytown2 CreditAttribution: mikeytown2 commented 10 January 2017 at 23:17

Status:

Needs work

» Needs review

File	Size
taxonomy-autocomplete-should-use-tid-2841128-16.patch	10.05 KB
7.x: PHP 5.3 & MySQL 5.5 2,030 pass

Try again

Comment #17

10 January 2017 at 23:30

Status:

Needs review

» Needs work

The last submitted patch, 16: taxonomy-autocomplete-should-use-tid-2841128-16.patch, failed testing.

Comment #18

puspanjalim CreditAttribution: puspanjalim at HyTechPro.com commented 18 January 2017 at 11:38

taxonomy term references with autocomplete feature is designed in such a way that new tags can be created directly from the suggestion box. I think we should not encourage for duplicate taxonomy terms as these are basically meant for tags for content.

Rather than we can create different vocabulary for same named terms.

For viewing and editing taxonomy page I have created patch.

Please confirm my profile.

Comment #19

AaronELBorg CreditAttribution: AaronELBorg commented 18 January 2017 at 18:26

taxonomy term references with autocomplete feature is designed in such a way that new tags can be created directly from the suggestion box

This is still 100% possible with my patch. (Which is now passing?? That's strange as it was failing with a "patch does not apply message" a couple of days ago. What changed?)

So even with my patch, if you type a term into the autocomplete field that doesn't exist yet, a new one will be created. This is also the current default behavior.

I think we should not encourage for duplicate taxonomy terms as these are basically meant for tags for content.

I agree that duplicate term names are not desirable but since it's entirely possible to create duplicate term names (and there are, arguably a few "good" reasons to do so), matching on tid seems like a better idea than simply matching on name. By matching on name alone, one might not even be aware that there are duplicate term names and even then data changes without the user knowing. That's very bad, IMO.

Rather than we can create different vocabulary for same named terms.

This seems more convoluted and confusing on an end-user level than the current situation. If your Content Type has a term reference field, currently, the out-of-the-box, default behavior is that the term can only come from a single vocabulary at a time.

If you created a new vocab with the addition of the new term (due to it being a duplicate), how would you know you created the new vocabulary? There would have to be some sort of messaging involved.

If that messaging is indeed in place, the user would then have to go back into the Manage Fields interface and now add the newly created vocab to the current term reference field? More messaging, more work...

This seems much more complicated than simply displaying [term-id:111] next to each term in the autocomplete field. The existence of this string ("[term-id:111]") makes it very apparent if and when a duplicate term has been created.

It also changes very little about the current autocomplete situation. It all works just like it always has yet functions correctly if there are duplicated terms.

As it stands, if a user has a "good" reason to use duplicated term names, the current autocomplete functionality is 100% broken. It will only match on the last duplicated term with nothing to notify the user that this has happened. They'll simply be left wondering how and why their data is incorrectly tagged.

Comment #20

AaronELBorg CreditAttribution: AaronELBorg commented 18 January 2017 at 18:27

Status:

Needs work

» Needs review

Comment #21

puspanjalim CreditAttribution: puspanjalim at HyTechPro.com commented 19 January 2017 at 06:30

Hi AaronELBorg,

You are right if you see as admin who create/manage content types and fields from Drupal admin UI.
But [term-id:111] will be confusing and less readable for a simple Editor who prepare content and post. For an editor tags/taxonomy terms are simple/meaningful words(terms) which are related to that content.

About the duplicate term we can use multi select widget for term reference where all duplicate terms are visible.

There are different approaches to use a core feature. Rather than modifying existing autocomplete feature if using the patch code a new feature can be developed for specific use than that will be better idea.

Comment #22

puspanjalim CreditAttribution: puspanjalim at HyTechPro.com commented 20 January 2017 at 08:40

File	Size
taxonomy-view-edit-vocabulary-reference-2841128-22.patch	1.17 KB
7.x: PHP 5.3 & MySQL 5.5 2,030 pass, 1 fail

Vocabulary reference to view and edit page of a taxonomy term. Vocabulary name as simple label has been added to to both pages.

Comment #23

20 January 2017 at 08:51

Status:

Needs review

» Needs work

The last submitted patch, 22: taxonomy-view-edit-vocabulary-reference-2841128-22.patch, failed testing.

Comment #24

amit0212 CreditAttribution: amit0212 as a volunteer and commented 25 January 2017 at 09:56

Here's the code:

function MODULE_NAME_taxonomy_term_presave($term) {
	// Change existing term? Nothing to do!
	if ((isset($term->tid)) && ($term->tid > 0)) {
	  return;
	  }
	
	// Lookup the term in the given vocabulary
	$query = new EntityFieldQuery;
	$result = $query
	  ->entityCondition('entity_type', 'taxonomy_term')
	  ->propertyCondition('name', $term->name)
	  ->propertyCondition('vid', $term->vid)
	  ->execute();
	
	// Term doesn't exist: handle it the normal way
	if (empty($result['taxonomy_term'])) {
	  return;
	  }
	
	// Term exists already: update it
	$termslist = array_keys($result['taxonomy_term']);
	$tid = array_shift($termslist);
	$term->tid = $tid;
	$original = taxonomy_term_load($tid);
	$term->original = $original;
	}