Commit7f18ec1

authored

Revert "Ensure CursorPagination respects nulls in the ordering field (#8912)" (#9381)

This reverts commitb1cec51.

1 parentf96c065 commit7f18ec1Copy full SHA for 7f18ec1

File tree

2 files changed

-142

lines changed

rest_framework
- pagination.py
tests
- test_pagination.py

2 files changed

-142

lines changed

`‎rest_framework/pagination.py‎`

Lines changed: 5 additions & 11 deletions

Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,6 @@`
`11`	`11`
`12`	`12`	`fromdjango.core.paginatorimportInvalidPage`
`13`	`13`	`fromdjango.core.paginatorimportPaginatorasDjangoPaginator`
`14`		`-fromdjango.db.modelsimportQ`
`15`	`14`	`fromdjango.templateimportloader`
`16`	`15`	`fromdjango.utils.encodingimportforce_str`
`17`	`16`	`fromdjango.utils.translationimportgettext_lazyas_`
`@@ -631,7 +630,7 @@ def paginate_queryset(self, queryset, request, view=None):`
`631`	`630`	`queryset=queryset.order_by(*self.ordering)`
`632`	`631`
`633`	`632`	`# If we have a cursor with a fixed position then filter by that.`
`634`		`-ifstr(current_position)!='None':`
	`633`	`+ifcurrent_positionisnotNone:`
`635`	`634`	`order=self.ordering[0]`
`636`	`635`	`is_reversed=order.startswith('-')`
`637`	`636`	`order_attr=order.lstrip('-')`
`@@ -642,12 +641,7 @@ def paginate_queryset(self, queryset, request, view=None):`
`642`	`641`	`else:`
`643`	`642`	`kwargs= {order_attr+'__gt':current_position}`
`644`	`643`
`645`		`-filter_query=Q(**kwargs)`
`646`		`-# If some records contain a null for the ordering field, don't lose them.`
`647`		`-# When reverse ordering, nulls will come last and need to be included.`
`648`		`-if (reverseandnotis_reversed)oris_reversed:`
`649`		`-filter_query\|=Q(**{order_attr+'__isnull':True})`
`650`		`-queryset=queryset.filter(filter_query)`
	`644`	`+queryset=queryset.filter(**kwargs)`
`651`	`645`
`652`	`646`	`# If we have an offset cursor then offset the entire page by that amount.`
`653`	`647`	`# We also always fetch an extra item in order to determine if there is a`
`@@ -720,7 +714,7 @@ def get_next_link(self):`
`720`	`714`	`# The item in this position and the item following it`
`721`	`715`	`# have different positions. We can use this position as`
`722`	`716`	`# our marker.`
`723`		`-has_item_with_unique_position=positionisnotNone`
	`717`	`+has_item_with_unique_position=True`
`724`	`718`	`break`
`725`	`719`
`726`	`720`	`# The item in this position has the same position as the item`
`@@ -773,7 +767,7 @@ def get_previous_link(self):`
`773`	`767`	`# The item in this position and the item following it`
`774`	`768`	`# have different positions. We can use this position as`
`775`	`769`	`# our marker.`
`776`		`-has_item_with_unique_position=positionisnotNone`
	`770`	`+has_item_with_unique_position=True`
`777`	`771`	`break`
`778`	`772`
`779`	`773`	`# The item in this position has the same position as the item`
`@@ -896,7 +890,7 @@ def _get_position_from_instance(self, instance, ordering):`
`896`	`890`	`attr=instance[field_name]`
`897`	`891`	`else:`
`898`	`892`	`attr=getattr(instance,field_name)`
`899`		`-returnNoneifattrisNoneelsestr(attr)`
	`893`	`+returnstr(attr)`
`900`	`894`
`901`	`895`	`defget_paginated_response(self,data):`
`902`	`896`	`returnResponse({`

`‎tests/test_pagination.py‎`

Lines changed: 3 additions & 131 deletions

Original file line number	Diff line number	Diff line change
`@@ -972,24 +972,17 @@ class MockQuerySet:`
`972`	`972`	`def__init__(self,items):`
`973`	`973`	`self.items=items`
`974`	`974`
`975`		`-deffilter(self,q):`
`976`		`-q_args=dict(q.deconstruct()[1])`
`977`		`-ifnotq_args:`
`978`		`-# django 3.0.x artifact`
`979`		`-q_args=dict(q.deconstruct()[2])`
`980`		`-created__gt=q_args.get('created__gt')`
`981`		`-created__lt=q_args.get('created__lt')`
`982`		`-`
	`975`	`+deffilter(self,created__gt=None,created__lt=None):`
`983`	`976`	`ifcreated__gtisnotNone:`
`984`	`977`	`returnMockQuerySet([`
`985`	`978`	`itemforiteminself.items`
`986`		`-ifitem.createdisNoneoritem.created>int(created__gt)`
	`979`	`+ifitem.created>int(created__gt)`
`987`	`980`	`])`
`988`	`981`
`989`	`982`	`assertcreated__ltisnotNone`
`990`	`983`	`returnMockQuerySet([`
`991`	`984`	`itemforiteminself.items`
`992`		`-ifitem.createdisNoneoritem.created<int(created__lt)`
	`985`	`+ifitem.created<int(created__lt)`
`993`	`986`	`])`
`994`	`987`
`995`	`988`	`deforder_by(self,*ordering):`
`@@ -1108,127 +1101,6 @@ def get_pages(self, url):`
`1108`	`1101`	`return (previous,current,next,previous_url,next_url)`
`1109`	`1102`
`1110`	`1103`
`1111`		`-classNullableCursorPaginationModel(models.Model):`
`1112`		`-created=models.IntegerField(null=True)`
`1113`		`-`
`1114`		`-`
`1115`		`-classTestCursorPaginationWithNulls(TestCase):`
`1116`		`-"""`
`1117`		- Unit tests for `pagination.CursorPagination` with ordering on a nullable field.
`1118`		`- """`
`1119`		`-`
`1120`		`-defsetUp(self):`
`1121`		`-classExamplePagination(pagination.CursorPagination):`
`1122`		`-page_size=1`
`1123`		`-ordering='created'`
`1124`		`-`
`1125`		`-self.pagination=ExamplePagination()`
`1126`		`-data= [`
`1127`		`-None,None,3,4`
`1128`		`- ]`
`1129`		`-foridxindata:`
`1130`		`-NullableCursorPaginationModel.objects.create(created=idx)`
`1131`		`-`
`1132`		`-self.queryset=NullableCursorPaginationModel.objects.all()`
`1133`		`-`
`1134`		`-get_pages=TestCursorPagination.get_pages`
`1135`		`-`
`1136`		`-deftest_ascending(self):`
`1137`		`-"""Test paginating one row at a time, current should go 1, 2, 3, 4, 3, 2, 1."""`
`1138`		`- (previous,current,next,previous_url,next_url)=self.get_pages('/')`
`1139`		`-`
`1140`		`-assertpreviousisNone`
`1141`		`-assertcurrent== [None]`
`1142`		`-assertnext== [None]`
`1143`		`-`
`1144`		`- (previous,current,next,previous_url,next_url)=self.get_pages(next_url)`
`1145`		`-`
`1146`		`-assertprevious== [None]`
`1147`		`-assertcurrent== [None]`
`1148`		`-assertnext== [3]`
`1149`		`-`
`1150`		`- (previous,current,next,previous_url,next_url)=self.get_pages(next_url)`
`1151`		`-`
`1152`		`-assertprevious== [3]# [None] paging artifact documented at https://github.com/ddelange/django-rest-framework/blob/3.14.0/rest_framework/pagination.py#L789`
`1153`		`-assertcurrent== [3]`
`1154`		`-assertnext== [4]`
`1155`		`-`
`1156`		`- (previous,current,next,previous_url,next_url)=self.get_pages(next_url)`
`1157`		`-`
`1158`		`-assertprevious== [3]`
`1159`		`-assertcurrent== [4]`
`1160`		`-assertnextisNone`
`1161`		`-assertnext_urlisNone`
`1162`		`-`
`1163`		`- (previous,current,next,previous_url,next_url)=self.get_pages(previous_url)`
`1164`		`-`
`1165`		`-assertprevious== [None]`
`1166`		`-assertcurrent== [3]`
`1167`		`-assertnext== [4]`
`1168`		`-`
`1169`		`- (previous,current,next,previous_url,next_url)=self.get_pages(previous_url)`
`1170`		`-`
`1171`		`-assertprevious== [None]`
`1172`		`-assertcurrent== [None]`
`1173`		`-assertnext== [None]# [3] paging artifact documented at https://github.com/ddelange/django-rest-framework/blob/3.14.0/rest_framework/pagination.py#L731`
`1174`		`-`
`1175`		`- (previous,current,next,previous_url,next_url)=self.get_pages(previous_url)`
`1176`		`-`
`1177`		`-assertpreviousisNone`
`1178`		`-assertcurrent== [None]`
`1179`		`-assertnext== [None]`
`1180`		`-`
`1181`		`-deftest_descending(self):`
`1182`		`-"""Test paginating one row at a time, current should go 4, 3, 2, 1, 2, 3, 4."""`
`1183`		`-self.pagination.ordering= ('-created',)`
`1184`		`- (previous,current,next,previous_url,next_url)=self.get_pages('/')`
`1185`		`-`
`1186`		`-assertpreviousisNone`
`1187`		`-assertcurrent== [4]`
`1188`		`-assertnext== [3]`
`1189`		`-`
`1190`		`- (previous,current,next,previous_url,next_url)=self.get_pages(next_url)`
`1191`		`-`
`1192`		`-assertprevious== [None]# [4] paging artifact`
`1193`		`-assertcurrent== [3]`
`1194`		`-assertnext== [None]`
`1195`		`-`
`1196`		`- (previous,current,next,previous_url,next_url)=self.get_pages(next_url)`
`1197`		`-`
`1198`		`-assertprevious== [None]# [3] paging artifact`
`1199`		`-assertcurrent== [None]`
`1200`		`-assertnext== [None]`
`1201`		`-`
`1202`		`- (previous,current,next,previous_url,next_url)=self.get_pages(next_url)`
`1203`		`-`
`1204`		`-assertprevious== [None]`
`1205`		`-assertcurrent== [None]`
`1206`		`-assertnextisNone`
`1207`		`-assertnext_urlisNone`
`1208`		`-`
`1209`		`- (previous,current,next,previous_url,next_url)=self.get_pages(previous_url)`
`1210`		`-`
`1211`		`-assertprevious== [3]`
`1212`		`-assertcurrent== [None]`
`1213`		`-assertnext== [None]`
`1214`		`-`
`1215`		`- (previous,current,next,previous_url,next_url)=self.get_pages(previous_url)`
`1216`		`-`
`1217`		`-assertprevious== [None]`
`1218`		`-assertcurrent== [3]`
`1219`		`-assertnext== [3]# [4] paging artifact documented at https://github.com/ddelange/django-rest-framework/blob/3.14.0/rest_framework/pagination.py#L731`
`1220`		`-`
`1221`		`-# skip back artifact`
`1222`		`- (previous,current,next,previous_url,next_url)=self.get_pages(previous_url)`
`1223`		`- (previous,current,next,previous_url,next_url)=self.get_pages(previous_url)`
`1224`		`-`
`1225`		`- (previous,current,next,previous_url,next_url)=self.get_pages(previous_url)`
`1226`		`-`
`1227`		`-assertpreviousisNone`
`1228`		`-assertcurrent== [4]`
`1229`		`-assertnext== [3]`
`1230`		`-`
`1231`		`-`
`1232`	`1104`	`deftest_get_displayed_page_numbers():`
`1233`	`1105`	`"""`
`1234`	`1106`	`Test our contextual page display function.`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Commit7f18ec1

File tree

2 files changed

2 files changed

`‎rest_framework/pagination.py‎`

`‎tests/test_pagination.py‎`

0 commit comments