While testing #484624: Fix all broken category/container previews, and add category_display defaults, I discovered a problem with hiearchy selection on categories, where distant parents are allowed. Later I confirmed, that exactly the same happens on fresh 6.x-2.0-beta3 code too, so this is a separate issue.

How to reproduce the problem:
- Have a site with, say, 2 already populated containers.
- Create a new container, select one of the existing containers as allowed parent.
- Create a new category (via the "add child category" link on new container)
- Play with the hiearchy selector:
* By default, the new container is correctly pre-selected
* But however, the selector below shows the hiearchy of categories inside the old container, that you've selected as allowed distant parent.
* Changing the top selector, you'll notice that you can select just *any* container on the entire site, not just the local/allowed distant parent.
* The bottom selector shows the hiearchies in these containers correctly, except that whenever you choose the local (new, real parent) container, you see the hiearchy of the allowed distant parent container instead.
* The selectors currently allow you to select a hiearchy anywhere on the site, not limited to local/allowed distant parents. If you select such disallowed hiearchy, the category still saves fine, no validation error shows.

I doubt I'll have enough time and motivation to look into this one (or to do so soon, at least), as I never quite used the distant parents feature. I don't even know, how exactly is it supposed to work. So, just recording the issue here.

CommentFileSizeAuthor
#2 parent-select.patch1.86 KBJirkaRybka
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JirkaRybka’s picture

Version: 6.x-2.0-beta3 » 6.x-2.0-rc1

I need to research on this, to understand how distant parents are supposed to work in the first place. Now I see that I *should* be able to put a new category to any container available. That bit is OK.

The problem is, that whenever I select a container that have allowed distant parenting, the sub-list of categories only shows categories in the distant container - not the distant container itself (not sure if that's a problem), and not anything from the local [selected] container (definitely a bug). This means, that I currently cannot create a category below a container with distant parenting enabled, at all.

JirkaRybka’s picture

Title: Distant parent selection on categories broken » Distant parenting of categories is flawed
Status: Active » Needs review
FileSize
1.86 KB

So, I finally found a bit of time for this...

Actually, this is more of a flaw (rather than a bug), introduced by the D6 simplification. Both D5 and D6 versions of Category module behave the same, if a container is configured to allow only just one (i.e. distant) container as a parent for categories. The problem is, that D5 allowed to choose multiple allowed containers for parenting, so that you can have the actual/local container AND a distant container available for categories parents. D6 got simplified to only just one allowed container, which in fact means that once a distant container is chosen, categories can't have the local container/categories as parents anymore. That looks pretty useless to me, because if a category can only have ONE container for parents, then I can put that category right into that container, and never care about distant parenting (which in fact only just alters the parent container setting now).

The change was probably more or less intentional, per code comment in _category_parent_select(), where the whole thing lies:

// If the container of the current node has an allowed distant parent, then
// generate the list based on the distant container instead of based on the
// actual container. Also exclude the distant container itself from the
// list.

But this still doesn't acknowledge that _removal_ of local parents options was the real intention, and is contradicted by the text shown on UI below allowed parent selector:
Specifies the container whose categories the categories in this container can have as parents. Leave as 'none' if you only want categories in this container to have local parents. A category can only have distant categories as its parents, not distant containers. However, a category can have this container, and other categories in this container, as local parents.
It sounds like a category can have local parents unconditionally (which makes a lot of sense), and also the selector itself, having "none" option for local parents only, looks like a selection of an *additional* parenting option, not of the *only* possibility.
And on the overall D6 issue #158598: Category port for Drupal 6.x, Jaza mentioned the change like this:
Distant parents has been made less ambitious, and also less buggy. You can now only define one other container as an 'allowed distant parent' for each container. Categories can only have distant categories as their parents, not distant containers.
All in all, none of these mentions say anything about a category being now *not* allowed to have local parents, although it actually works that way now.

I'm a bit unsure what the possible use cases for distant parenting are, but unless we're going to use distant parenting only just to apply other per-container settings to a set of categories (rather than really building a cross-container hierarchy), local parents are quite necessary.

So, my proposal is: Always offer local parents as options, and if an allowed distant container is set, *add* the distant options to the selector (so that both local and distant parents are available).

Attached patch does that. I tested briefly, and all seems to be working fine.

Jaza’s picture

Status: Needs review » Needs work

@JirkaRybka: the current behaviour is actually exactly what I intended. That is (in the Drupal 6 version), if a category can have distant parents, then it cannot have local parents.

Think about the most common example of distant parents that I generally use: countries and regions and cities. If 'region' has 'country' set as its distant parent, then does it make sense that a region can have another region as its parent? Probably not - I'd say you would want to force a region to have a country as its parent. Similar thing for 'city' and 'region' - surely a city cannot have another city as its parent.

If you think it really would be useful, then I'm willing to commit this patch, if you have "Always allow local parents" (or something worded similarly to that) as a per-container setting, and if local parents are then only possible for categories in distant-parent containers if this option is checked. But I'm not going to commit a patch that means local parents are always allowed, or that allows them in distant-parent situations by default.

JirkaRybka’s picture

Status: Needs work » Closed (works as designed)

@Jaza: Thanks for the clarification! Actually, I never really used this feature, this is just something I came across while testing other things. I did my best to guess how is this supposed to work, but it seems like I got it wrong. This is exactly the kind of stuff, where further input is necessary, so thanks a lot for yours. I always thought of distant parenting as something additional to regular hierarchy (but didn't have a real use-case), which led me to the conclusions above.

Based on your explanation of supposed use-case, I'm marking my own issue as 'By Design'.