If a selected term or any of its parents has multiple parents, taxiselect incorrectly lists all the parents. For instance given the vocabulary below (forgive the ascii art):

    A
   / \
  B   C 
   \ /
    D

If D is searched for within the vocabulary, taxiselect returns D >> B >> C >> A. It includes both B and C as a single selection instead of D >> B >> A and D >> C >> A as two separate selections. The cause is the call to taxonomy_get_parents_all() which returns all parents. The attached patch against 6.x-2.0-rc1 fixes this and will suggest both paths. There are a couple quirks with the patch:

  1. Although both options will appear in the suggestion list, only one can be added to a multiple select list. If the other path is already in the list the add is essentially a no-op. Since the list is a list of terms, and it is the same term, just displayed differently, I don't see this as a problem.
  2. Along the same lines, when the term is added, it is arbitrary which path is actually displayed. So, if "D >> B >> A" is selected, it is possible that "D >> C >> A" may be displayed. This is a problem, but more minor in nature than the current issue.
CommentFileSizeAuthor
#5 Original-Bug.png2.48 KBmrchuckles
#5 Mrchuckles-Patch-1.png2.51 KBmrchuckles
#5 Commited-Patch-3.png2.43 KBmrchuckles
#2 taxiselect-single-parental-path-657846-3.patch2.06 KBalan d.
#1 taxiselect-single-parental-path-657846-2.patch1.76 KBalan d.
mutliple-parents.diff5.81 KBmrchuckles

Comments

alan d.’s picture

alan d. commented
StatusFileSize
new taxiselect-single-parental-path-657846-2.patch1.76 KB

Nice work on your first Drupal issue.

The function seemed to complex for the task at hand, so I have rewritten it to parse the parental path directly via SQL.

alan d.’s picture

alan d. commented
StatusFileSize
new taxiselect-single-parental-path-657846-3.patch2.06 KB

This one should take into account the parental path that the user entered. Try this one instead of the above patch.

Without recording the exact user entered path per term added, it will be impossible to always use this path on the final rendition of the term on the node, so this is a big "Won't fix"!!

I haven't got a decent testing environment (for this use case), so all feedback is appreciated.

mrchuckles’s picture

mrchuckles commented

Thanks for the kind words and the quick response.

taxiselect-single-parental-path-657846-3.patch works for my scenarios, although per design it suggests only a single path. Continuing with the original example:

If "D" is typed, it will suggest a single path D >> C >> A.
If "D, B" is typed (ie/the path prefix for the path not suggested), the prefix is honored and it correctly suggests D >> B >> A. If that path is added to a multi-select box it will revert to D >> C >> A since only the single term, not the whole path, is saved. I understand the "Won't fix!" there.

It comes down to a question of how you want to handle multiple paths ... suggest all or just one. For my use case, users should recognize the final term via any path so a single suggestion is fine, but suggesting all feels "more correct".

alan d.’s picture

alan d. commented

In regards to suggest all or just one, I'll stick to all. I think that this feature is going to be dropped in Drupal 7, so I don't want to invest to much time into it. ( Just thinking that a simple distinct in the query could do this. Happy to follow through if you want to try. )

I've pushed the second patch into the next dev release; these take up to 24 hours to come through

Cheers

mrchuckles’s picture

mrchuckles commented
StatusFileSize
new Commited-Patch-3.png2.43 KB
new Mrchuckles-Patch-1.png2.51 KB
new Original-Bug.png2.48 KB

You said "In regards to suggest all or just one, I'll stick to all.", but your patch suggests just one. I fear I may not have explained the error well enough since I did not give detailed instructions to reproduce in my original post. So here we go:

1. Create the A -> B, C -> D vocabulary I described in my original post and associate it with a content type.
2. In Content Management -> Taxonomy -> TaxiSelect Settings
* Change "Taxiselect trigger length" to 1, so the lookup occurs after entering the first character.
* Associate the widget with the same content type you associated the vocabulary.
* Settings for the vocabulary set both of the following:
- Use TaxiSelect for the selected content types
- Use TaxiSelect for administrating this vocabulary
3. Create a new content item
4. When selecting the term for the new vocabulary, enter only "D" and Taxiselct will suggest matching taxonomy terms/paths. This is the behavior with the various patches:

* If no patch is applied, the single suggestion is D >> B >> C >> A, which is not a valid path through the hierarchy. This is the original bug. See "Original-Bug.png".
* If my original patch is applied, both paths are suggested D >> B >> A and D >> C >> A. This is what I meant by "suggesting all paths". See "Mrchuckles-Patch-1.png"
* Using your committed patch, only D >> B >> A is suggested. So only one of the possible paths is presented. See "Commited-Patch-3.png"

As I said, for my use case presenting a single path is sufficient as the final term will be recognized via any path, but it doesn't feel like the right solution.

alan d.’s picture

alan d. commented

Sorry, a moment of dyslexia there, I think that I meant just the one.

Reason: The path is used for both the lookup and presentation, so displaying the one path by default would be mean more probability of consistency here. Displaying both, and the user selects the second, would result in the system showing the first, confusing the user even more.

mrchuckles’s picture

mrchuckles commented

Fair enough. Thanks for the fix.

alan d.’s picture

alan d. commented
Version: 6.x-2.0-rc1 » 6.x-2.x-dev
Status: Needs review » Fixed

Thxs for the feedback too :)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.