Opened 15 years ago

Closed 6 months ago

#10941 closed New feature (fixed)

Add a templatetag to generate querystrings

Reported by: Ben Spaulding Owned by: Tom Carrick
Component: Template system Version: dev
Severity: Normal Keywords: pagination
Cc: dan.fairs@…, Ning, Thomas Capricelli, Carsten Fuchs Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Working with pagination and query strings within a template can be painful. Personally, I have never had a situation when I was paginating using a GET parameter where there were not other parameters that needed to be preserved through out the various pages.

Take search, for example. There may be parameters for searching within one or more models, for a particular author and sorting by date. Maintaining all of these parameters within the pagination links takes some serious template logic.

{# Linebreaks added for readability. In real life this would need to be one, long line. #}
<a href="?{% for key, values in request.GET.iterlists %}
  {% ifnotequal key "page" %}
    {% for value in values %}
      {{ key }}={{ value }}&amp;
    {% endfor %}
  {% endifnotequal %}
{% endfor %}page={{ page.next_page_number }}">Next page</a>

That kind of logic shouldn’t be in a template. I have created a patch that would allow for something much simpler, like so:

<a href="?{{ page.next_page_querystring }}">Next page</a>

Though there has been much talk of creating template tags which would produce all-out pagination bars, I believe this particular functionality should be an actual method on the page object for two reasons:

  1. This is basic functionality whose end result is hard to dispute (as opposed to a full pagination bar where markup and features could be disputed eternally),
  2. This does not require the request context processor to be installed.

Note that this patch includes documentation. Tests are still needed. I am not married to the exact implementation, but I and others I have discussed this and feel that this simplicity and fuctionality belong in Django’s pagination.

Attachments (1)

querystring-methods.diff (3.8 KB ) - added by Ben Spaulding 15 years ago.
Methods for handling pagination query strings, documentation.

Download all attachments as: .zip

Change History (43)

by Ben Spaulding, 15 years ago

Attachment: querystring-methods.diff added

Methods for handling pagination query strings, documentation.

comment:1 by Greg Brown, 15 years ago

I'm in favour of this proposal

comment:3 by Chris Beaven, 14 years ago

Needs tests: set
Triage Stage: UnreviewedDesign decision needed

Patch seems like a good idea - should be brought up in the django-dev google group.

in reply to:  3 comment:4 by Ben Spaulding, 14 years ago

Replying to SmileyChris:

Patch seems like a good idea - should be brought up in the django-dev google group.

Thanks for the advice. I have started a discussion on django-dev: Add querystring helper methods to Page class.

comment:5 by Julien Phalip, 13 years ago

Severity: Normal
Type: New feature

comment:6 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:7 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:8 by Dan Fairs, 12 years ago

Cc: dan.fairs@… added

comment:9 by Florian Apolloner, 11 years ago

Triage Stage: Design decision neededAccepted

Oh yeah, I run into this all the time. I think that we should allow customization of the querystring name, eg 'p' instead of 'page'. To that end a simple filter and/or tag might be better.

in reply to:  9 comment:10 by Ben Spaulding, 11 years ago

Replying to apollo13:

Oh yeah, I run into this all the time. I think that we should allow customization of the querystring name, eg 'p' instead of 'page'. To that end a simple filter and/or tag might be better.

Just to be clear, drop the helper methods and go with a filter or tag, requiring use of the django.core.context_processors.request? (Sounds fine to me, I just want to be clear on the recommended route so this doesn’t bog down in implementation.) What template tag library would that live in? There doesn’t seem to be a current contrib app it would fit in, and I am not sure it belongs in built-ins if it depends on a context processor that is not installed by default.

comment:11 by Florian Demmer, 9 years ago

i really miss this functionality and like the proposed solution. what is missing from the patch to get his merged?

comment:12 by Tim Graham, 9 years ago

As indicated by the flags on this ticket "Needs tests".

comment:13 by Florian Demmer, 9 years ago

Cc: fdemmer@… added

sorry, i should have seen that. there is too much nice green in the theme ;)

i have created a pr based on the old patch, but slightly changed the implementation, updated MultipleObjectMixin to use the new feature, added a test, updated two existing tests and the documentation.

rfc: https://github.com/django/django/pull/4581

comment:14 by Tim Graham, 9 years ago

Needs tests: unset

comment:15 by Tim Graham, 9 years ago

Patch needs improvement: set

comment:16 by Ben Spaulding, 9 years ago

Owner: changed from nobody to Ben Spaulding
Status: newassigned

comment:17 by Ben Spaulding, 9 years ago

Owner: Ben Spaulding removed
Status: assignednew

comment:18 by Florian Demmer, 9 years ago

Patch needs improvement: unset

fixed documentation and added another test; please review updated pr...

comment:19 by Tim Graham, 9 years ago

After thinking about this some more, I wonder if something like https://djangosnippets.org/snippets/2428/ would be a more flexible, generic solution (allowing query strings to be generated in other areas besides pagination, for example). I'm not in love with adding request details to the paginator (seems like a poor separation of concerns).

comment:20 by Florian Demmer, 9 years ago

Cc: fdemmer@… removed

Two years ago apollo13 set this approach from "design decision needed" to "accepted", then Ben and I worked on getting the old patch up to date and came back fixing all kinds of details and now you think the alternative approach with a template tag is better.

This is your right of course, but please make a final decision and close this issue regarding "Add querystring helper methods to Page class" as wontfix if you decide in favour of a template tag solution, so that nobody else's time is wasted.

I am out.

Version 0, edited 9 years ago by Florian Demmer (next)

comment:21 by Tim Graham, 9 years ago

We got rid of the "design decision needed" triage stage, so many "accepted" tickets need discussion about implementation details. Often it's difficult to make such decisions without seeing an actual patch. I hoped to start a discussion about the best approach with that comment, not offend you. Of course, I am not trying to waste anyone's time; just trying to craft the best possible APIs for Django.

comment:22 by Tim Graham, 9 years ago

From apollo13 on IRC: "for me a tag ala {% query_string page=3 %} which would take the existing qs and rewrite it would be perfect".

comment:23 by Tim Graham, 9 years ago

Patch needs improvement: set

comment:24 by Tim Graham, 9 years ago

Component: Core (Other)Template system
Has patch: unset
Patch needs improvement: unset
Summary: Add querystring helper methods to Page classAdd a templatetag to generate querystrings

comment:25 by Luke Plant, 8 years ago

I think this whole area is solved by django-spurl - https://github.com/j4mie/django-spurl

It builds on urlobject which can be used for Python code: https://github.com/zacharyvoase/urlobject/

There is also furl for Python code: https://github.com/gruns/furl

I don't think we need to duplicate any of these within Django. Having a template tag to do 1/10 of what spurl does is just an invitation to include more and more of spurl's functionality. It might be useful to have some links to spurl in the documentation.

comment:26 by Curtis Maloney, 8 years ago

Personally I think it wouldn't hurt to have a helper for the specific case of altering the page number _only_.

In a prior project we ended up needing 4 different querystring manipulators [and I'm sure there could have been more].

One would output the original, less any listed keys, plus any specified keys _overridden_ in value.
One would just add new values.
One would return the original with only the listed keys.
A final just updated the page number.

This still doesn't cover, for instance, when you want to add a second value to an existing key... or.... well, given it's a multi-dict, the options are endless.

So, to reiterate: A _simple_ helper for helping with pagination, which is a problem almost everyone encounters, would be good.

A general purpose querystring manipulator? Leave that to 3rd parties.

comment:27 by Jimmy Merrild Krag, 7 years ago

It's been some time since something happened on this one. i have just stumpled into needing to change query strings in templates, so I thought I'd add my findings.

I have created two rather simple filter which allows you ro edit the query string. django-spurl already does some of this, however not very elegantly. I think the following three filters would be a great addition to Django that will get you "there" much of the time:

https://pastebin.com/vJ3feNvY

Usage is e.g. <a href="?{% set_query_values page=page.next_page_number %}">&raquo;</a> or <a href="?{% append_query_values selected=item.id %}">Select {{ item.name }}</a>
Note: They all support iterables as values.

I think the only ones probably missing is one for removing a key and one for removing a specific value from a key.

I'm also thinking of merging the set and append (also adding remove) by prepending each key with the action and letting None clear the list when using set_. I imagine something like {% modify_query set_level=7 set_nextlevel=None append_consumedlevels=processed_levels_list remove_availablelevels=7 %}.

What do you think? Any thoughts?

Last edited 7 years ago by Jimmy Merrild Krag (previous) (diff)

comment:28 by Jimmy Merrild Krag, 7 years ago

I made an implementation of the above mentioned modify_query.

https://pastebin.com/nGmVdeks

Thoughts and comments are most welcome.

Last edited 7 years ago by Jimmy Merrild Krag (previous) (diff)

comment:29 by Ning, 6 years ago

Cc: Ning added

comment:30 by Thomas Capricelli, 6 years ago

Cc: Thomas Capricelli added

comment:31 by Federico Jaramillo Martínez, 4 years ago

Is this ticket still valid?

Would a PR with a templatetag be accepted?
If so, I could be working on this.

If anyone can confirm I would appreciate.

Thanks

comment:32 by Claude Paroz, 4 years ago

Sure, it's still valid. Read attentively the existing comments.

Maybe the greatest challenge of this ticket is to find a compromise between functionality and simplicity, addressing the more common use cases while accepting it won't address all possible needs.

comment:33 by Carsten Fuchs, 2 years ago

Cc: Carsten Fuchs added

I too had reinvented this wheel a couple of years ago, unaware of this ticket. Other implementations above are more powerful, but this one is short:

@register.simple_tag
def query_string(*args, **kwargs):
    """
    Combines dictionaries of query parameters and individual query parameters
    and builds an encoded URL query string from the result.
    """
    query_dict = QueryDict(mutable=True)

    for a in args:
        query_dict.update(a)

    remove_keys = []

    for k, v in kwargs.items():
        if v is None:
            remove_keys.append(k)
        elif isinstance(v, list):
            query_dict.setlist(k, v)
        else:
            query_dict[k] = v

    for k in remove_keys:
        if k in query_dict:
            del query_dict[k]

    qs = query_dict.urlencode()
    if not qs:
        return ""
    return "?" + qs

It cannot do everything, but covers all my use cases: Keep most of the existing source parameters, but delete, add or change some. Examples:

{# Just repeat the parameters: #}
{% query_string request.GET %}

{# Add a parameter: #}
{% query_string request.GET format='pdf' %}

{# Change a parameter: #}
{% query_string request.GET page=next_page_number %}

{# Overwrite month and year with precomputed values, e.g. with next_month_year = {'month': 1, 'year': 2022}, and clear the day: #}
{% query_string request.GET next_month_year day=None %}

I'm aware that posting yet another implementation does not achieve true progress and that the more elaborate implementations above may be more appropriate for inclusion in Django.
Still, I wanted to mention this because in my opinion it keeps the balance between features and simplicity.

comment:34 by Leonardo Cavallucci, 11 months ago

I would really love to have this: I could work on this, if it is decided how this should work

comment:35 by Tom Carrick, 6 months ago

Owner: set to Tom Carrick
Status: newassigned

comment:36 by Tom Carrick, 6 months ago

Has patch: set

comment:37 by Mariusz Felisiak, 6 months ago

Patch needs improvement: set

comment:38 by Tom Carrick, 6 months ago

Patch needs improvement: unset

comment:39 by Mariusz Felisiak, 6 months ago

Patch needs improvement: set

comment:40 by Tom Carrick, 6 months ago

Patch needs improvement: unset

comment:41 by Mariusz Felisiak, 6 months ago

Triage Stage: AcceptedReady for checkin

comment:42 by Mariusz Felisiak <felisiak.mariusz@…>, 6 months ago

Resolution: fixed
Status: assignedclosed

In e67d3580:

Fixed #10941 -- Added {% query_string %} template tag.

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