idlesign / django-sitetree

Reusable application for Django introducing site tree, menu and breadcrumbs navigation elements.
http://github.com/idlesign/django-sitetree
BSD 3-Clause "New" or "Revised" License
348 stars 130 forks source link

Matching this when multiple URL matches fails #287

Open dwasyl opened 4 years ago

dwasyl commented 4 years ago

In continuing to setup this package I stumbled across a bug in how sitetree matches urls. When there are multiple matches to the same URL, sitetree only tries to match against the first possible url match, not the entire list.

With the code setup below, if I was on a page say: /books/1/ the page would print the side menu setup below just fine, however, if I was on /library/books/1/ the sitetree_menu include for this-parent-siblings doesn't find a match for this (and so displays nothing). When I comment out the first matching URL, the /library/books/1/ matches this just fine.

As a fix, I think sitetree should attempting matching this against all possible urls, not just the first one returned.

Here's the setup: An url structure something like this:

re_path(r'^books/(?P<pk>\d+)/$', views.book_view, name='book_view'),
re_path(r'^library/books/(?P<pk>\d+)/$', views.book_view, name='book_view'),

A dynamic menu setup like this:

item('Library', 'library_home', children=[
    item("Add Book", "book_add"),
    item("Book List", "book_list"),
    item("{{ obj }}", "book_view obj.pk"),
])

Sidebar menus are display with the menu templatetag: {% sitetree_menu from "main-tree" include "this-parent-siblings" %}

idlesign commented 4 years ago

Sorry for the delay. Actually such behaviour is expected.

As a fix, I think sitetree should attempting matching this against all possible urls, not just the first one returned.

You've already mentioned that there are multiple matches, so how should we define and detect the "best" match in that case? I'm afraid there's no universal heuristic for that. Do you have one in mind?

dwasyl commented 4 years ago

@idlesign It's okay, I think it's a bit different than the intended behaviour with multiple matches.

Using the setup I posted above if a user accesses http://site.com/books/1/, sitetree would display the menu as specified based on this being equal to item("{{ obj }}", "book_view obj.pk") this menu item.

However, if a user accesses http://site.com/library/books/1/, sitetree would have no matching value for this and then the sitetree_menu command as specified wouldn't display anything. If you were to comment out the first urls.py path then accessing it via the second url would match to this.

So I don't think this is an issue where there needs to be a "best" match, but since either path meets the specified item then either should be matched with this.

At the same time, when generating a link for that menu item, sitetree just picks the first matching url which is the current and likely best solution.