Opened 3 years ago

Closed 10 months ago

Last modified 8 months ago

#17551 closed Cleanup/optimization (invalid)

Namespaces declared in different places

Reported by: Kronuz Owned by: gnosek
Component: Documentation Version: 1.4-alpha-1
Severity: Normal Keywords:
Cc: Kronuz Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently, if the same namespace is declared for different included urls, only one included urls module gets the namespace.

In this scenario:

urlpatterns = patterns('',
    url(r'^first/', include('first.urls')),
    url(r'^first_default/', include('first.urls', app_name='first', namespace='first')),
    url(r'^first_in_a/', include('first.urls', app_name='first', namespace='A')),
    url(r'^first_in_b/', include('first.urls', app_name='first', namespace='B')),
    url(r'^second/', include('second.urls')),
    url(r'^second_default/', include('second.urls', app_name='second', namespace='second')),
    url(r'^second_in_a/', include('second.urls', app_name='second', namespace='A')),
    url(r'^second_in_b/', include('second.urls', app_name='second', namespace='B')),
)

The expected output is:

reverse('first') -> /first/
reverse('second') -> /second/
reverse('A:first') -> /first_in_a/
reverse('A:second') -> /second_in_a/
reverse('B:first') -> /first_in_b/
reverse('B:second') -> /second_in_b/

But instead is:

reverse('first') -> /first/
reverse('second') -> /second/
reverse('A:first') -> /first_in_a/
reverse('A:second') -> NoReverseMatch
reverse('B:first') -> /first_in_b/
reverse('B:second') -> NoReverseMatch

Attachments (3)

#17551-multiple_namespaces.tar.bz2 (4.1 KB) - added by Kronuz 3 years ago.
Example which fails
#17551-multiple_namespaces.diff (7.4 KB) - added by Kronuz 3 years ago.
Added tests (makes just two of them fail)
17551.diff (6.4 KB) - added by gnosek 3 years ago.

Download all attachments as: .zip

Change History (16)

Changed 3 years ago by Kronuz

Example which fails

comment:1 Changed 3 years ago by Kronuz

  • Cc Kronuz added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Changed 3 years ago by Kronuz

Added tests (makes just two of them fail)

comment:2 Changed 3 years ago by gnosek

  • Component changed from Uncategorized to Core (URLs)
  • Has patch set
  • Owner changed from nobody to gnosek
  • Patch needs improvement set
  • Status changed from new to assigned

Changed 3 years ago by gnosek

comment:3 Changed 3 years ago by gnosek

  • Patch needs improvement unset

comment:4 Changed 3 years ago by jezdez

  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

comment:5 Changed 3 years ago by oinopion

  • Triage Stage changed from Accepted to Ready for checkin

Good for me.

comment:6 Changed 3 years ago by oinopion

  • Version changed from 1.3 to 1.4-alpha-1

comment:7 Changed 3 years ago by PaulM

  • Severity changed from Normal to Release blocker

Marking as a release blocker

comment:8 Changed 3 years ago by aaugustin

  • Severity changed from Release blocker to Normal

After further discussion, this isn't really a release blocker, since it isn't a regression or a major bug in a new feature.

However, the patch looks correct and stands a good chance to be committed before 1.4.

I've spend a few hours on it, but I don't feel sufficiently familiar with the URL namespacing feature to commit it. I asked Russell (the original author) if he could take a look.

comment:9 Changed 3 years ago by aaugustin

  • Triage Stage changed from Ready for checkin to Accepted

After discussion with Malcolm (the other author), this rather looks like a misuse of app_name.

This could be resolved by improving the documentation rather than changing the code.

comment:10 Changed 2 years ago by Kronuz

I really wish this could be checked in since, even of it's a "missuse" given the original design, I believe it to be a good addition which just further increases the url resolvers code isometry and flexibly allowing the use of the same namespace name across apps... *plus* it doesn't really break anything, just covers a legit and intuitive use case for namespaces.

Any other thoughts about this ticket?

Version 1, edited 2 years ago by Kronuz (previous) (next) (diff)

comment:11 Changed 10 months ago by timo

  • Component changed from Core (URLs) to Documentation
  • Has patch unset
  • Type changed from Bug to Cleanup/optimization

Moving to documentation based on Aymeric's comment 22. If someone wants to lobby for a design change, please start a thread on the DevelopersMailingList.

comment:12 Changed 10 months ago by timo

  • Resolution set to duplicate
  • Status changed from assigned to closed

I think we can roll this documentation into #17707 - "Docs for URL namespaces should explain the motivation and use cases".

comment:13 Changed 8 months ago by timgraham

  • Resolution changed from duplicate to invalid

It's documented that "Instance namespaces should be unique across your entire project." That seems to be the issue here.

Note: See TracTickets for help on using tickets.
Back to Top