Opened 13 years ago

Closed 2 years ago

#16406 closed Cleanup/optimization (fixed)

Allow separate access to matches from urlpatterns and extra_context args

Reported by: Florian Apolloner Owned by: Alokik Roy
Component: Core (URLs) Version: dev
Severity: Normal Keywords: resolvers, reverse
Cc: Alexandre Prieto Pantoja, Alokik Roy Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Currently Django's ResolverMatch (eg the one returned by urlresolvers.resolve) gives access to args and kwargs which are ready to be passed to the func being resolved. This does eliminate the possibility of an interesting use-case:

Let's assume I have a page written in english and german. I configured my urls using the new urlpatterns like this:

urlpatterns = i18n_patterns('',
    url(_(r'^about/$'), 'about.view', {'some_extra': 'data'}, name='about'),
    url(_(r'^news/$'), 'home.view', {'some_extra': 'data'}, name='home'),
)

I now would like to give my visitors an easy way to access the content in german. So my /about should have a link to /über. To do that, one could write a templatetag which does the following:

match = urlresolvers.resolve('current-url')
links = []
for lang in settings.LANGUAGES:
  translation.activate(lang):
  links.append(reverse(match.url_name, args=match.args, kwargs=match.kwargs))
# Further logic to display the links in the template

This doesn't work since the ResolverMatch combined the kwargs captured from the url-regex with the dictionary supplied for the extra data.

I think it would be a great if the ResolverMatch would give access to kwargs and extra_context separately. There might be more usecases, but that's the only one I can come up with for now. From a quick view at the code it should be possible to do that backwards-compat (eg ResolverMatch.__getitem__ would still return the current result but internally it would use two dictionaries).

Attachments (3)

t16406.diff (6.4 KB ) - added by Florian Apolloner 13 years ago.
simple patch demonstrating the feature
t16406_new.diff (6.5 KB ) - added by Florian Apolloner 12 years ago.
valueslisterrormessage3.diff (8.1 KB ) - added by antoviaque 12 years ago.

Download all attachments as: .zip

Change History (28)

by Florian Apolloner, 13 years ago

Attachment: t16406.diff added

simple patch demonstrating the feature

comment:1 by Aymeric Augustin, 13 years ago

Has patch: set
Triage Stage: UnreviewedDesign decision needed
Type: UncategorizedCleanup/optimization

comment:2 by Jacob, 13 years ago

Triage Stage: Design decision neededAccepted

Good idea!

comment:3 by antoviaque, 12 years ago

Owner: changed from nobody to antoviaque

by Florian Apolloner, 12 years ago

Attachment: t16406_new.diff added

comment:4 by Florian Apolloner, 12 years ago

New and cleaner patch, but it does change behavior (see changes of the doc file)

by antoviaque, 12 years ago

comment:5 by antoviaque, 12 years ago

Owner: changed from antoviaque to nobody

Looks like you beat me to the update apollo : )

I've added a test specific to the behavior change though, you may want to integrate it in your patch.

comment:6 by anonymous, 12 years ago

If your patch includes my changes that should be fine…

comment:7 by Aymeric Augustin, 11 years ago

Component: Core (Other)Core (URLs)

comment:8 by Tim Graham, 10 years ago

Patch needs improvement: set

Patch no longer applies cleanly.

comment:9 by David Smith, 3 years ago

Easy pickings: set

comment:10 by Alexandre Prieto Pantoja, 3 years ago

Owner: changed from nobody to Alexandre Prieto Pantoja
Status: newassigned

comment:11 by Alexandre Prieto Pantoja, 3 years ago

Hello,

I'am trying to figure out the context of this ticket but I am rather stuck on the resolution until now.

I reproduced the use case with the information in the ticket. My code is now the following:

views.py file:

from django.shortcuts import render
from django.urls import resolve, reverse
from django.utils import translation
from django.utils.translation import gettext as _

def homepage(request, *args, **kwargs):
    links = []
    langs = settings.LANGUAGES
    match = resolve(request.path) 
    
    for lang in langs:         
        with translation.override(lang[0]):                  
            links.append(reverse(match.url_name, args=match.args, kwargs=match.kwargs))
     
    message = _('Welcome to our site!')
   
    return render(request, 'ticket_16406/homepage.html', {'message': message, 'links':links})

url.py file:

from django.conf.urls.i18n import i18n_patterns
from django.contrib import admin
from django.urls import path, re_path
from django.utils.translation import gettext_lazy as _
from ticket_16406 import views


urlpatterns = [
    path(_('admin/'), admin.site.urls),
]
urlpatterns += i18n_patterns(    
    re_path(_(r'^home/(?P<slug>[\w-]+)/$'), views.homepage, {'extra': True}, name="homepage"),
)

template

    {% for link in links %}
        <a href={{ link }}>{{ link }}</a>
    {% endfor %}

I have access to content in different languages in my template.

I don't find the problem I have to tackle here.

Could you give me  more information/details on  "why the patch no longer applies cleanly"?

It would be a great help. Thanks!

Last edited 3 years ago by Alexandre Prieto Pantoja (previous) (diff)

comment:12 by Alexandre Prieto Pantoja, 3 years ago

Cc: Alexandre Prieto Pantoja added

comment:13 by Anvesh Mishra, 3 years ago

Owner: changed from Alexandre Prieto Pantoja to Anvesh Mishra

in reply to:  9 comment:14 by Pedro Schlickmann Mendes, 2 years ago

Replying to David Smith:
Can you give us more details on why the patch no longer applies cleanly?

comment:15 by Alokik Roy, 2 years ago

Owner: changed from Anvesh Mishra to Alokik Roy

comment:17 by Alokik Roy, 2 years ago

Patch needs improvement: unset

comment:18 by Tim Graham, 2 years ago

Patch needs improvement: set

The patch is missing tests (all code changes require tests). "Needs tests" is checked if there is a patch without tests, not an indication that tests are required. Also the documentation is incomplete. Please review the patch review checklist for all requirements.

comment:19 by Alokik Roy, 2 years ago

Patch needs improvement: unset

Added the tests and documentation

Last edited 2 years ago by Alokik Roy (previous) (diff)

comment:20 by Alokik Roy, 2 years ago

Cc: Alokik Roy added

comment:21 by Mariusz Felisiak, 2 years ago

Needs tests: set

comment:22 by Alokik Roy, 2 years ago

Needs tests: unset

Requested Tests added.

comment:23 by Mariusz Felisiak, 2 years ago

Needs tests: set
Patch needs improvement: set

comment:24 by Mariusz Felisiak, 2 years ago

Needs tests: unset
Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:25 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In baf9604:

Fixed #16406 -- Added ResolveMatch.captured_kwargs and extra_kwargs.

Thanks Florian Apolloner for the review and implementation idea.

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