Code

Opened 2 years ago

Last modified 18 months ago

#17551 assigned Bug

Namespaces declared in different places

Reported by: Kronuz Owned by: gnosek
Component: Core (URLs) Version: 1.4-alpha-1
Severity: Normal Keywords:
Cc: Kronuz Triage Stage: Accepted
Has patch: yes 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 2 years ago.
Example which fails
#17551-multiple_namespaces.diff (7.4 KB) - added by Kronuz 2 years ago.
Added tests (makes just two of them fail)
17551.diff (6.4 KB) - added by gnosek 2 years ago.

Download all attachments as: .zip

Change History (13)

Changed 2 years ago by Kronuz

Example which fails

comment:1 Changed 2 years ago by Kronuz

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

Changed 2 years ago by Kronuz

Added tests (makes just two of them fail)

comment:2 Changed 2 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 2 years ago by gnosek

comment:3 Changed 2 years ago by gnosek

  • Patch needs improvement unset

comment:4 Changed 2 years ago by jezdez

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

comment:5 Changed 2 years ago by oinopion

  • Triage Stage changed from Accepted to Ready for checkin

Good for me.

comment:6 Changed 2 years ago by oinopion

  • Version changed from 1.3 to 1.4-alpha-1

comment:7 Changed 2 years ago by PaulM

  • Severity changed from Normal to Release blocker

Marking as a release blocker

comment:8 Changed 2 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 20 months 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 18 months 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 18 months ago by Kronuz (previous) (next) (diff)

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as assigned
The owner will be changed from gnosek to anonymous. Next status will be 'assigned'
The ticket will be disowned. Next status will be 'new'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.