Django

Code

Ticket #2606 (closed: fixed)

Opened 2 years ago

Last modified 1 year ago

[patch] Tag for URL reverse lookup

Reported by: Ivan Sagalaev <Maniac@SoftwareManiacs.Org> Assigned to: adrian
Milestone: Component: Template system
Version: Keywords: reverselookups
Cc: Maniac@SoftwareManiacs.Org, gary.wilson@gmail.com, rushman@mail.ru Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 1
Needs tests: 0 Patch needs improvement: 0

Description

A template tag for doing reverse URL lookup. Supports only positional arguments for now. Patch follows.

Attachments

2606.diff (2.4 kB) - added by Ivan Sagalaev <Maniac@SoftwareManiacs.Org> on 08/26/06 12:28:09.
Patch
2606.2.diff (2.8 kB) - added by Ivan Sagalaev <Maniac@SoftwareManiacs.Org> on 09/17/06 16:45:05.
Support for kwargs
2606.3.diff (3.0 kB) - added by Ivan Sagalaev <Maniac@SoftwareManiacs.Org> on 09/28/06 17:38:47.
Support for referring views outside project
2606.4.diff (6.2 kB) - added by Ivan Sagalaev <Maniac@SoftwareManiacs.Org> on 11/07/06 05:55:04.
Patch + docs + tests
views.py (159 bytes) - added by Ivan Sagalaev <Maniac@SoftwareManiacs.Org> on 11/07/06 05:56:17.
tests/regressiontests/templates/views.py
urls.py (290 bytes) - added by Ivan Sagalaev <Maniac@SoftwareManiacs.Org> on 11/07/06 05:57:46.
tests/regressiontests/templates/urls.py

Change History

08/26/06 12:28:09 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

  • attachment 2606.diff added.

Patch

08/26/06 14:26:34 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

  • cc set to Maniac@SoftwareManiacs.Org.
  • summary changed from Tag for URL reverse lookup to [patch] Tag for URL reverse lookup.

09/17/06 16:45:05 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

  • attachment 2606.2.diff added.

Support for kwargs

09/17/06 16:51:02 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

Added support for keyword arguments in {% url %}, docstring updated.

However I have a hard time figuring out how to write tests for this thing. The tag uses current urlconf from settings to do its job and I'm not sure if it would be correct to replace it with special testing urls.py and views.py... So for now there are no tests for {% url %} in the patch.

09/28/06 06:29:49 changed by jeroen_at_lbvd.nl

Very usefull tag. I would propose a small change though. In the render function you prepend the project_name to the viewname (return reverse(project_name + '.' + self.view_name, args=args, kwargs=kwargs)).

However, this assumes that the application is part of the project (in the python sense). If the app is in a python package that is NOT the current project this aproach fails. This could be the case if you 'borrow' an application from another project (without copying it).

For example, I have an 'library project' (magicstrings) that contains various apps, used by different projects. The actual projects are little more than configuration and some very project-specific code. Thus the python path to a view in one of the apps part of Magic Strings is in my case (regardless of the project they are actually used in) magicstrings.app_name.views.view_name. And this patch puts, wrongly, the name of the actual project in front of that.

Another example is contrib. If you try to reverse map django.contrib.admin.views.main.index, this approach would try to reverse map current_project.django.contrib.admin.views.main.index, which would obviously fail.

There are imo two solutions: a) let people always specify the projectname or b) perhaps it is an idea to change the tag so that you can add the projectname in front of the path, separated by e.g. a colon. Option a) is the simplest, but perhaps I miss the reason behind the current behavior and is b) the only option.

{% url my_project:app_name.views.view_name %}

Would result in reverse('my_project.app_name.views.view_name'), and if projectname is ommited, fallback to the current one:

{% url app_name.views.view_name %}

Would result in reverse('current_project.app_name.views.view_name')

Rgds, Jeroen

09/28/06 12:18:20 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

First of all thank you for a very useful comment!

You wrote: "perhaps I miss the reason behind the current behavior".

The only reason is that I indeed wrongly had in mind only project's local views.

Looking at your proposals I can think of a third way that can be both short for in-project views and suitable for out-of-project views. It'll be like this:

  1. try to import specified path without modifications
  2. if that fails, add the project name and try again

I'll make an updated patch shortly.

09/28/06 17:38:47 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

  • attachment 2606.3.diff added.

Support for referring views outside project

09/30/06 10:38:48 changed by jeroen_at_lbvd.nl

Good solution. Thanks!

11/06/06 22:48:13 changed by jacob

This is very nice. Can you please add unit tests and documentation so we can check this in?

11/07/06 03:52:39 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

I have a problem making a unit test for it. Every other template tag can be tested on its own but {% url %} actually needs urls.py and views.py... I can't think of a good way to fake this environment.

As for the docs, I'll post the patch shortly. Thanks!

11/07/06 05:54:10 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

I've wrapped my head around testing and made a new patch (2606.4.diff) with tests and documentation.

The patch requires 2 new files (attached). I tried to include them in the patch itself but couldn't apply it then, so I decided to attach them as is.

11/07/06 05:55:04 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

  • attachment 2606.4.diff added.

Patch + docs + tests

11/07/06 05:56:17 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

  • attachment views.py added.

tests/regressiontests/templates/views.py

11/07/06 05:57:46 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

  • attachment urls.py added.

tests/regressiontests/templates/urls.py

01/11/07 18:23:10 changed by Gary Wilson <gary.wilson@gmail.com>

  • cc changed from Maniac@SoftwareManiacs.Org to Maniac@SoftwareManiacs.Org, gary.wilson@gmail.com.

01/19/07 06:10:53 changed by Simon G. <dev@simon.net.nz>

  • keywords set to reverselookups.
  • needs_docs set to 1.
  • stage changed from Unreviewed to Accepted.

I think this is just awaiting some brief documentation, but looks good to go.

01/19/07 06:16:33 changed by Ivan Sagalaev <Maniac@SoftwareManiacs.Org>

It does have docs right in the patch :-). (the last one -- 2606.4.diff)

And to get you on course: two attached files views.py and urls.py are for tests. I didn't include them in the patch because I beleive they wouldn't be created by the 'patch' tool itself. But may be I'm mistaken :-)

01/19/07 06:31:01 changed by Simon G. <dev@simon.net.nz>

  • stage changed from Accepted to Ready for checkin.

Ahh. Didn't scroll down that far. Now that I know what this does, I like it very much.

01/22/07 16:26:10 changed by Sergey Kirillov <rushman@mail.ru>

  • cc changed from Maniac@SoftwareManiacs.Org, gary.wilson@gmail.com to Maniac@SoftwareManiacs.Org, gary.wilson@gmail.com, rushman@mail.ru.

02/12/07 22:24:58 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

(In [4494]) Fixed #2606 -- Added tag for working out the URL of a particular view function. All work done by Ivan Sagalaev.


Add/Change #2606 ([patch] Tag for URL reverse lookup)




Change Properties
Action