Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect Routing for Taxonomy Branches and Leaf Nodes with Absolute Paths #15456

Open
jooni91 opened this issue Mar 5, 2024 · 1 comment
Open
Labels
Milestone

Comments

@jooni91
Copy link

jooni91 commented Mar 5, 2024

Consider the following example of a taxonomy tree:

  • root
    • branch
      • leaf
    • branch-2 (with absolute path)
      • leaf

Here root refers to the taxonomy itself, and branch/leaf are the terms of that taxonomy.

I would expect the following paths to the leaf nodes in the tree:

/root/branch/leaf
/branch-2/leaf

However, what I get instead is:

/root/branch/leaf
/root/branch-2/leaf

Note: The AutoroutePart property RouteContainedItems is enabled for both the root and the branches.

Upon examining the code, it appears that AutoroutePartHandler.PopulateContainedContentItemRoutesAsync(...) is responsible for the unexpected routing behavior observed. The basePath parameter is consistently used to create the itemBasePath for contained items, which may not be ideal. It might be more appropriate to use the direct parent's path instead (which might be absolute).

One additional consideration, which is related to this issue, is how scenarios should be handled where the route for a parent/branch is disabled. In such cases, would it be more appropriate to continue using the basePath parameter, or should routing for all child items be disabled as well?

I'd appreciate your thoughts on this. As I've already dug into the code and the solution seems pretty straightforward, I'm up for tackling this one, assuming there aren't any objections.

@sebastienros
Copy link
Member

I am not sure "with absolute path" is something that was considered. Have you seen that used somewhere or is that something you tries and had expectations it would work?

If you can make it work it's all good, but it might break other parts of the code that assume the autoroute part is hierarchical.

@sebastienros sebastienros added this to the 1.x milestone Mar 7, 2024
jooni91 added a commit to jooni91/OrchardCore that referenced this issue Mar 13, 2024
… routes.

- In case that the parent route is disabled and the child rout is not absolute, we should skip routing of that child item.
Fix OrchardCMS#15456
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants