From 22a6360edf610b8fdf3672084f5dcb8ab46f750c Mon Sep 17 00:00:00 2001
From: "martin f. krafft" <madduck@madduck.net>
Date: Thu, 13 Jun 2024 16:46:18 +0200
Subject: [PATCH] Fix: default order of documents gets lost in QuerySet
 pipeline (#6982)

* 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 | 28 +++++++++++++--------
 src/documents/views.py                      |  1 +
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/src/documents/tests/test_api_permissions.py b/src/documents/tests/test_api_permissions.py
index d7131b834..7708b8541 100644
--- a/src/documents/tests/test_api_permissions.py
+++ b/src/documents/tests/test_api_permissions.py
@@ -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):
diff --git a/src/documents/views.py b/src/documents/views.py
index 9e8e7301d..72414d4f0 100644
--- a/src/documents/views.py
+++ b/src/documents/views.py
@@ -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")