Skip to content

Commit fc0f03b

Browse files
mak626mak626
committed
fix: pagination errors
hasNextPage didn't become false when using after and first together Fixed reverse Querying using last and before, in compliance to graphql relay spec https://relay.dev/graphql/connections.htm#sec-Backward-pagination-arguments https://relay.dev/graphql/connections.htm#sec-undefined.PageInfo
1 parent b31b0b9 commit fc0f03b

File tree

3 files changed

+61
-121
lines changed

3 files changed

+61
-121
lines changed

‎graphene_mongo/fields.py‎

Lines changed: 35 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
find_skip_and_limit,
4040
get_model_reference_fields,
4141
get_query_fields,
42+
has_page_info,
4243
)
4344

4445
PYMONGO_VERSION = tuple(pymongo.version_tuple[:2])
@@ -276,7 +277,7 @@ def fields(self):
276277
return self._type._meta.fields
277278

278279
def get_queryset(
279-
self, model, info, required_fields=None, skip=None, limit=None, reversed=False, **args
280+
self, model, info, required_fields=None, skip=None, limit=None, **args
280281
) -> QuerySet:
281282
if required_fields is None:
282283
required_fields = list()
@@ -325,49 +326,22 @@ def get_queryset(
325326
else:
326327
args.update(queryset_or_filters)
327328
if limit is not None:
328-
if reversed:
329-
if self.order_by:
330-
order_by = self.order_by + ",-pk"
331-
else:
332-
order_by = "-pk"
333-
return (
334-
model.objects(**args)
335-
.no_dereference()
336-
.only(*required_fields)
337-
.order_by(order_by)
338-
.skip(skip if skip else 0)
339-
.limit(limit)
340-
)
341-
else:
342-
return (
343-
model.objects(**args)
344-
.no_dereference()
345-
.only(*required_fields)
346-
.order_by(self.order_by)
347-
.skip(skip if skip else 0)
348-
.limit(limit)
349-
)
329+
return (
330+
model.objects(**args)
331+
.no_dereference()
332+
.only(*required_fields)
333+
.order_by(self.order_by)
334+
.skip(skip if skip else 0)
335+
.limit(limit)
336+
)
350337
elif skip is not None:
351-
if reversed:
352-
if self.order_by:
353-
order_by = self.order_by + ",-pk"
354-
else:
355-
order_by = "-pk"
356-
return (
357-
model.objects(**args)
358-
.no_dereference()
359-
.only(*required_fields)
360-
.order_by(order_by)
361-
.skip(skip)
362-
)
363-
else:
364-
return (
365-
model.objects(**args)
366-
.no_dereference()
367-
.only(*required_fields)
368-
.order_by(self.order_by)
369-
.skip(skip)
370-
)
338+
return (
339+
model.objects(**args)
340+
.no_dereference()
341+
.only(*required_fields)
342+
.order_by(self.order_by)
343+
.skip(skip)
344+
)
371345
return model.objects(**args).no_dereference().only(*required_fields).order_by(self.order_by)
372346

373347
def default_resolver(self, _root, info, required_fields=None, resolved=None, **args):
@@ -401,7 +375,6 @@ def default_resolver(self, _root, info, required_fields=None, resolved=None, **a
401375
skip = 0
402376
count = 0
403377
limit = None
404-
reverse = False
405378
first = args.pop("first", None)
406379
after = args.pop("after", None)
407380
if after:
@@ -410,14 +383,15 @@ def default_resolver(self, _root, info, required_fields=None, resolved=None, **a
410383
before = args.pop("before", None)
411384
if before:
412385
before = cursor_to_offset(before)
386+
requires_page_info = has_page_info(info)
413387
has_next_page = False
414388

415389
if resolved is not None:
416390
items = resolved
417391

418392
if isinstance(items, QuerySet):
419393
try:
420-
if last is not None and after is not None:
394+
if last is not None:
421395
count = items.count(with_limit_and_skip=False)
422396
else:
423397
count = None
@@ -426,29 +400,24 @@ def default_resolver(self, _root, info, required_fields=None, resolved=None, **a
426400
else:
427401
count = len(items)
428402

429-
skip, limit, reverse = find_skip_and_limit(
403+
skip, limit = find_skip_and_limit(
430404
first=first, last=last, after=after, before=before, count=count
431405
)
432406

433407
if isinstance(items, QuerySet):
434408
if limit:
435-
_base_query: QuerySet = (
436-
items.order_by("-pk").skip(skip) if reverse else items.skip(skip)
437-
)
409+
_base_query: QuerySet = items.skip(skip)
438410
items = _base_query.limit(limit)
439-
has_next_page = len(_base_query.skip(limit).only("id").limit(1)) != 0
411+
has_next_page = len(_base_query.skip(skip + limit).only("id").limit(1)) != 0
440412
elif skip:
441413
items = items.skip(skip)
442414
else:
443415
if limit:
444-
if reverse:
445-
_base_query = items[::-1]
446-
items = _base_query[skip : skip + limit]
447-
has_next_page = (skip + limit) < len(_base_query)
448-
else:
449-
_base_query = items
450-
items = items[skip : skip + limit]
451-
has_next_page = (skip + limit) < len(_base_query)
416+
_base_query = items
417+
items = items[skip : skip + limit]
418+
has_next_page = (
419+
(skip + limit) < len(_base_query) if requires_page_info else False
420+
)
452421
elif skip:
453422
items = items[skip:]
454423
iterables = list(items)
@@ -503,11 +472,11 @@ def default_resolver(self, _root, info, required_fields=None, resolved=None, **a
503472
else:
504473
count = self.model.objects(args_copy).count()
505474
if count != 0:
506-
skip, limit, reverse = find_skip_and_limit(
475+
skip, limit = find_skip_and_limit(
507476
first=first, after=after, last=last, before=before, count=count
508477
)
509478
iterables = self.get_queryset(
510-
self.model, info, required_fields, skip, limit, reverse, **args
479+
self.model, info, required_fields, skip, limit, **args
511480
)
512481
list_length = len(iterables)
513482
if isinstance(info, GraphQLResolveInfo):
@@ -519,14 +488,11 @@ def default_resolver(self, _root, info, required_fields=None, resolved=None, **a
519488

520489
elif "pk__in" in args and args["pk__in"]:
521490
count = len(args["pk__in"])
522-
skip, limit, reverse = find_skip_and_limit(
491+
skip, limit = find_skip_and_limit(
523492
first=first, last=last, after=after, before=before, count=count
524493
)
525494
if limit:
526-
if reverse:
527-
args["pk__in"] = args["pk__in"][::-1][skip : skip + limit]
528-
else:
529-
args["pk__in"] = args["pk__in"][skip : skip + limit]
495+
args["pk__in"] = args["pk__in"][skip : skip + limit]
530496
elif skip:
531497
args["pk__in"] = args["pk__in"][skip:]
532498
iterables = self.get_queryset(self.model, info, required_fields, **args)
@@ -542,18 +508,13 @@ def default_resolver(self, _root, info, required_fields=None, resolved=None, **a
542508
field_name = to_snake_case(info.field_name)
543509
items = getattr(_root, field_name, [])
544510
count = len(items)
545-
skip, limit, reverse = find_skip_and_limit(
511+
skip, limit = find_skip_and_limit(
546512
first=first, last=last, after=after, before=before, count=count
547513
)
548514
if limit:
549-
if reverse:
550-
_base_query = items[::-1]
551-
items = _base_query[skip : skip + limit]
552-
has_next_page = (skip + limit) < len(_base_query)
553-
else:
554-
_base_query = items
555-
items = items[skip : skip + limit]
556-
has_next_page = (skip + limit) < len(_base_query)
515+
_base_query = items
516+
items = items[skip : skip + limit]
517+
has_next_page = (skip + limit) < len(_base_query) if requires_page_info else False
557518
elif skip:
558519
items = items[skip:]
559520
iterables = items
@@ -567,11 +528,6 @@ def default_resolver(self, _root, info, required_fields=None, resolved=None, **a
567528
)
568529
has_previous_page = True if skip else False
569530

570-
if reverse:
571-
iterables = list(iterables)
572-
iterables.reverse()
573-
skip = limit
574-
575531
connection = connection_from_iterables(
576532
edges=iterables,
577533
start_offset=skip,

‎graphene_mongo/fields_async.py‎

Lines changed: 18 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ async def default_resolver(self, _root, info, required_fields=None, resolved=Non
9292
skip = 0
9393
count = 0
9494
limit = None
95-
reverse = False
9695
first = args.pop("first", None)
9796
after = args.pop("after", None)
9897
if after:
@@ -109,7 +108,7 @@ async def default_resolver(self, _root, info, required_fields=None, resolved=Non
109108

110109
if isinstance(items, QuerySet):
111110
try:
112-
if last is not None and after is not None:
111+
if last is not None:
113112
count = await sync_to_async(items.count)(with_limit_and_skip=False)
114113
else:
115114
count = None
@@ -118,33 +117,30 @@ async def default_resolver(self, _root, info, required_fields=None, resolved=Non
118117
else:
119118
count = len(items)
120119

121-
skip, limit, reverse = find_skip_and_limit(
120+
skip, limit = find_skip_and_limit(
122121
first=first, last=last, after=after, before=before, count=count
123122
)
124123

125124
if isinstance(items, QuerySet):
126125
if limit:
127-
_base_query: QuerySet = (
128-
await sync_to_async(items.order_by("-pk").skip)(skip)
129-
if reverse
130-
else await sync_to_async(items.skip)(skip)
131-
)
126+
_base_query: QuerySet = await sync_to_async(items.skip)(skip)
132127
items = await sync_to_async(_base_query.limit)(limit)
133128
has_next_page = (
134-
(await sync_to_async(len)(_base_query.skip(limit).only("id").limit(1)) != 0)
129+
(
130+
await sync_to_async(len)(
131+
_base_query.skip(skip + limit).only("id").limit(1)
132+
)
133+
!= 0
134+
)
135135
if requires_page_info
136136
else False
137137
)
138138
elif skip:
139139
items = await sync_to_async(items.skip)(skip)
140140
else:
141141
if limit:
142-
if reverse:
143-
_base_query = items[::-1]
144-
items = _base_query[skip : skip + limit]
145-
else:
146-
_base_query = items
147-
items = items[skip : skip + limit]
142+
_base_query = items
143+
items = items[skip : skip + limit]
148144
has_next_page = (
149145
(skip + limit) < len(_base_query) if requires_page_info else False
150146
)
@@ -195,11 +191,11 @@ async def default_resolver(self, _root, info, required_fields=None, resolved=Non
195191
else:
196192
count = await sync_to_async(self.model.objects(args_copy).count)()
197193
if count != 0:
198-
skip, limit, reverse = find_skip_and_limit(
194+
skip, limit = find_skip_and_limit(
199195
first=first, after=after, last=last, before=before, count=count
200196
)
201197
iterables = self.get_queryset(
202-
self.model, info, required_fields, skip, limit, reverse, **args
198+
self.model, info, required_fields, skip, limit, **args
203199
)
204200
iterables = await sync_to_async(list)(iterables)
205201
list_length = len(iterables)
@@ -212,14 +208,11 @@ async def default_resolver(self, _root, info, required_fields=None, resolved=Non
212208

213209
elif "pk__in" in args and args["pk__in"]:
214210
count = len(args["pk__in"])
215-
skip, limit, reverse = find_skip_and_limit(
211+
skip, limit = find_skip_and_limit(
216212
first=first, last=last, after=after, before=before, count=count
217213
)
218214
if limit:
219-
if reverse:
220-
args["pk__in"] = args["pk__in"][::-1][skip : skip + limit]
221-
else:
222-
args["pk__in"] = args["pk__in"][skip : skip + limit]
215+
args["pk__in"] = args["pk__in"][skip : skip + limit]
223216
elif skip:
224217
args["pk__in"] = args["pk__in"][skip:]
225218
iterables = self.get_queryset(self.model, info, required_fields, **args)
@@ -236,16 +229,12 @@ async def default_resolver(self, _root, info, required_fields=None, resolved=Non
236229
field_name = to_snake_case(info.field_name)
237230
items = getattr(_root, field_name, [])
238231
count = len(items)
239-
skip, limit, reverse = find_skip_and_limit(
232+
skip, limit = find_skip_and_limit(
240233
first=first, last=last, after=after, before=before, count=count
241234
)
242235
if limit:
243-
if reverse:
244-
_base_query = items[::-1]
245-
items = _base_query[skip : skip + limit]
246-
else:
247-
_base_query = items
248-
items = items[skip : skip + limit]
236+
_base_query = items
237+
items = items[skip : skip + limit]
249238
has_next_page = (skip + limit) < len(_base_query) if requires_page_info else False
250239
elif skip:
251240
items = items[skip:]
@@ -261,11 +250,6 @@ async def default_resolver(self, _root, info, required_fields=None, resolved=Non
261250
)
262251
has_previous_page = True if requires_page_info and skip else False
263252

264-
if reverse:
265-
iterables = await sync_to_async(list)(iterables)
266-
iterables.reverse()
267-
skip = limit
268-
269253
connection = connection_from_iterables(
270254
edges=iterables,
271255
start_offset=skip,

‎graphene_mongo/utils.py‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -259,9 +259,12 @@ def ast_to_dict(node, include_loc=False):
259259

260260

261261
def find_skip_and_limit(first, last, after, before, count=None):
262-
reverse = False
263262
skip = 0
264263
limit = None
264+
265+
if last is not None and count is None:
266+
raise ValueError("Count Missing")
267+
265268
if first is not None and after is not None:
266269
skip = after + 1
267270
limit = first
@@ -274,29 +277,26 @@ def find_skip_and_limit(first, last, after, before, count=None):
274277
skip = 0
275278
limit = first
276279
elif last is not None and before is not None:
277-
reverse = False
278280
if last >= before:
279281
limit = before
280282
else:
281283
limit = last
282284
skip = before - last
283285
elif last is not None and after is not None:
284-
if not count:
285-
raise ValueError("Count Missing")
286-
reverse = True
286+
skip = after + 1
287287
if last + after < count:
288288
limit = last
289289
else:
290290
limit = count - after - 1
291291
elif last is not None:
292-
skip = 0
292+
skip = count - last
293293
limit = last
294-
reverse = True
295294
elif after is not None:
296295
skip = after + 1
297296
elif before is not None:
298297
limit = before
299-
return skip, limit, reverse
298+
299+
return skip, limit
300300

301301

302302
def connection_from_iterables(

0 commit comments

Comments
 (0)