]> git.ipfire.org Git - thirdparty/paperless-ngx.git/commitdiff
Fix: default order of documents gets lost in QuerySet pipeline (#6982)
authormartin f. krafft <madduck@madduck.net>
Thu, 13 Jun 2024 14:46:18 +0000 (16:46 +0200)
committerGitHub <noreply@github.com>
Thu, 13 Jun 2024 14:46:18 +0000 (14:46 +0000)
* Send ordered document list to Django REST pagination

Currently, when pages of documents are requested from the API, the
webserver logs a warning:

```
gunicorn[1550]: /home/madduck/code/paperless-ngx/.direnv/python-3.11.2/lib/python3.11/site-packages/rest_framework/pagination.py:200: UnorderedObjectListWarning: Pagination may yield inconsistent results with an unordered object_list: <class 'documents.models.Document'> QuerySet.
```

This can yield unexpected and problematic results, including duplicate
and missing IDs in the enumeration, as demonstrated in
https://github.com/paperless-ngx/paperless-ngx/discussions/6859

The patch is simple: turn the unordered Documents QuerySet into
one that's ordered by reverse creation date, which is the default
ordering for `Document`.

Note that the default ordering for `Document` means that
`QuerySet.ordered` is actually `True` following the call to
`distinct()`, but after `annotate()`, the flag changes to `False`,
unless `order_by()` is used explicitly, as per this patch.

Closes: https://github.com/paperless-ngx/paperless-ngx/discussions/6859
Signed-off-by: martin f. krafft <madduck@madduck.net>
* Ensure order of documents in permissions test

The patch for #6982 changes the ordering of documents returned by the
API, which was previously implicit, and is now explicit. Therefore,
this patch masssages the API result to ensure the previous order.

Signed-off-by: martin f. krafft <madduck@madduck.net>
---------

Signed-off-by: martin f. krafft <madduck@madduck.net>
src/documents/tests/test_api_permissions.py
src/documents/views.py

index d7131b834015a36c1c57a11bd4763a74dc160438..7708b85413ee5a6947448efa1dd0dec539e0b398 100644 (file)
@@ -432,13 +432,18 @@ class TestApiAuth(DirectoriesMixin, APITestCase):
 
         resp_data = response.json()
 
-        self.assertNotIn("permissions", resp_data["results"][0])
-        self.assertIn("user_can_change", resp_data["results"][0])
-        self.assertTrue(resp_data["results"][0]["user_can_change"])  # doc1
-        self.assertFalse(resp_data["results"][0]["is_shared_by_requester"])  # doc1
-        self.assertFalse(resp_data["results"][1]["user_can_change"])  # doc2
-        self.assertTrue(resp_data["results"][2]["user_can_change"])  # doc3
-        self.assertTrue(resp_data["results"][3]["is_shared_by_requester"])  # doc4
+        # The response will contain the documents in reversed order of creation
+        # due to #6982, but previously this code relied on implicit ordering
+        # so let's ensure the order is as expected:
+        results = resp_data["results"][::-1]
+
+        self.assertNotIn("permissions", results[0])
+        self.assertIn("user_can_change", results[0])
+        self.assertTrue(results[0]["user_can_change"])  # doc1
+        self.assertFalse(results[0]["is_shared_by_requester"])  # doc1
+        self.assertFalse(results[1]["user_can_change"])  # doc2
+        self.assertTrue(results[2]["user_can_change"])  # doc3
+        self.assertTrue(results[3]["is_shared_by_requester"])  # doc4
 
         response = self.client.get(
             "/api/documents/?full_perms=true",
@@ -449,9 +454,12 @@ class TestApiAuth(DirectoriesMixin, APITestCase):
 
         resp_data = response.json()
 
-        self.assertIn("permissions", resp_data["results"][0])
-        self.assertNotIn("user_can_change", resp_data["results"][0])
-        self.assertNotIn("is_shared_by_requester", resp_data["results"][0])
+        # See above about response ordering
+        results = resp_data["results"][::-1]
+
+        self.assertIn("permissions", results[0])
+        self.assertNotIn("user_can_change", results[0])
+        self.assertNotIn("is_shared_by_requester", results[0])
 
 
 class TestApiUser(DirectoriesMixin, APITestCase):
index 9e8e7301d5427595c6b775fe8f591972f933315e..72414d4f01836ba7c960bd74d29be7a3e5f6a7dd 100644 (file)
@@ -362,6 +362,7 @@ class DocumentViewSet(
     def get_queryset(self):
         return (
             Document.objects.distinct()
+            .order_by("-created")
             .annotate(num_notes=Count("notes"))
             .select_related("correspondent", "storage_path", "document_type", "owner")
             .prefetch_related("tags", "custom_fields", "notes")