Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add 'page_kwarg' attribute to `MultipleObjectMixin`, removing hardcoded "page" #467

Merged
merged 4 commits into from Nov 17, 2012

Conversation

Projects
None yet
5 participants
Member

tomchristie commented Oct 25, 2012

At the moment it's awkward to modify the generic views to use a custom name for the 'page' query parameter.

Includes documentation.

Member

mjtamlyn commented Oct 25, 2012

+1, needs a test as well though

Owner

apollo13 commented Oct 25, 2012

We don't accept pull requests without the associated ticket beeing accepted. Is there a ticket? If yes please link it, if no, open one. Thx!

Member

mjtamlyn commented Oct 25, 2012

Marked as RFC!

Member

tomchristie commented Nov 8, 2012

Any chance of this making it in before the beta freeze?
Don't think there's anything contentious here, and the patch, tests and docs should cover everything.

@apollo13 apollo13 commented on the diff Nov 9, 2012

docs/ref/class-based-views/mixins-multiple-object.txt
@@ -69,8 +69,15 @@ MultipleObjectMixin
An integer specifying how many objects should be displayed per page. If
this is given, the view will paginate objects with
:attr:`MultipleObjectMixin.paginate_by` objects per page. The view will
- expect either a ``page`` query string parameter (via ``GET``) or a
- ``page`` variable specified in the URLconf.
+ expect either a ``page`` query string parameter (via ``request.GET``)
+ or a ``page`` variable specified in the URLconf.
+
+ .. attribute:: page_kwarg
@apollo13

apollo13 Nov 9, 2012

Owner

Misses a "versionadded 1.5" directive.

@apollo13 apollo13 commented on an outdated diff Nov 9, 2012

docs/ref/class-based-views/mixins-multiple-object.txt
@@ -69,8 +69,15 @@ MultipleObjectMixin
An integer specifying how many objects should be displayed per page. If
this is given, the view will paginate objects with
:attr:`MultipleObjectMixin.paginate_by` objects per page. The view will
- expect either a ``page`` query string parameter (via ``GET``) or a
- ``page`` variable specified in the URLconf.
+ expect either a ``page`` query string parameter (via ``request.GET``)
+ or a ``page`` variable specified in the URLconf.
+
+ .. attribute:: page_kwarg
+
+ A string specifying the name to use for the page parameter.
+ The view will expect this prameter to be available either as a query
+ string parameter (via ``request.GET``) or as a kwarg variable specified
+ in the URLconf. Defaults to ``"page"``.
@apollo13

apollo13 Nov 9, 2012

Owner

There are two blanks, remove one. Also remove the " around page.

@jezdez jezdez added a commit that referenced this pull request Nov 17, 2012

@jezdez jezdez Merge pull request #467 from tomchristie/page-kwarg
Add 'page_kwarg' attribute to `MultipleObjectMixin`, removing hardcoded "page".
778b8bd

@jezdez jezdez merged commit 778b8bd into django:master Nov 17, 2012

Member

SmileyChris commented Nov 21, 2012

@jezdez, shouldn't this be pulled into 1.5.X if it has been marked as versionadded 1.5 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment