Opened 6 years ago

Closed 6 years ago

#15070 closed Bug (fixed)

url tag does not work within inclusion_tags when current_app is needed

Reported by: raony araújo Owned by: Gary Reynolds
Component: Template system Version: master
Severity: Normal Keywords: easy-pickings
Cc: botondus@…, gary@…, jleidel@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX:

Description

suppose we deployed the same app two times (e.g., django-admin), each include in urls.py has tha namespace and app_name set.

now we create a custom tag (with the inclusion_tag function of Library) that renders a specific template which has an url tag trying to resolve an url from the mentioned app.

then we create a view which set current_app on the RequestContext object and renders a template with the custom tag; the reverse inside the url_tag at the included template fails to resolve to the right app as if the current_app was not set.

Attachments (4)

15070.diff (790 bytes) - added by raony araújo 6 years ago.
15070_with_tests.patch (2.7 KB) - added by Béres Botond 6 years ago.
15070.2.diff (2.8 KB) - added by Matthias Kestenholz 6 years ago.
15070_goodtune.diff (3.9 KB) - added by Gary Reynolds 6 years ago.

Download all attachments as: .zip

Change History (15)

Changed 6 years ago by raony araújo

Attachment: 15070.diff added

comment:1 Changed 6 years ago by Łukasz Rekucki

Keywords: easy-pickings added
Needs tests: set
Triage Stage: UnreviewedAccepted

A regression test is also required. tests/regressiontests/templates/custom.py is most likely the right place.

comment:2 Changed 6 years ago by Béres Botond

Owner: changed from nobody to Béres Botond

Changed 6 years ago by Béres Botond

Attachment: 15070_with_tests.patch added

comment:3 Changed 6 years ago by Béres Botond

Cc: botondus@… added
Needs tests: unset
Status: newassigned

comment:4 Changed 6 years ago by Matthias Kestenholz

Refreshed patch for current SVN trunk.

Only change: Made sure it applies correctly and added strip() calls so that the trailing newline in the newly added template does not cause test failures.

Changed 6 years ago by Matthias Kestenholz

Attachment: 15070.2.diff added

comment:5 Changed 6 years ago by Gary Reynolds

Cc: gary@… added
Patch needs improvement: set
Severity: Normal
Type: Uncategorized

I encountered this myself just recently and had started working on my own patch.

When fixing this we should also provide coverage for the use_l10n attribute as well.

comment:6 Changed 6 years ago by James Addison

Type: UncategorizedBug

Changed 6 years ago by Gary Reynolds

Attachment: 15070_goodtune.diff added

comment:7 Changed 6 years ago by Gary Reynolds

Owner: changed from Béres Botond to Gary Reynolds
Patch needs improvement: unset
Status: assignednew

I've added a new patch in attachment:15070_goodtune.diff which also deals with the use_l10n case I mentioned in comment:5.

The tests follow the same convention to the previous suggestions. If there is a better approach for writing tests for inclusion tags then please let me know and I'll review.

comment:8 Changed 6 years ago by Jacob

Easy pickings: set

comment:9 Changed 6 years ago by Jannis Leidel

Triage Stage: AcceptedReady for checkin

comment:10 Changed 6 years ago by Jannis Leidel

Cc: jleidel@… added

comment:11 Changed 6 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

In [16117]:

Fixed #15070 -- Also pass current_app and use_l10n in inclusion_tags. Thanks, raony, mk and goodtune.

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