diff --git a/src/documents/filters.py b/src/documents/filters.py index fe15c927d..427745424 100644 --- a/src/documents/filters.py +++ b/src/documents/filters.py @@ -161,7 +161,9 @@ class ObjectFilter(Filter): class InboxFilter(Filter): def filter(self, qs, value): if value == "true": - return qs.filter(tags__is_inbox_tag=True) + # A document can have more than one tag flagged as an inbox tag + # (nothing enforces uniqueness), so this join can multiply rows. + return qs.filter(tags__is_inbox_tag=True).distinct() elif value == "false": return qs.exclude(tags__is_inbox_tag=True) else: @@ -268,6 +270,10 @@ class CustomFieldsFilter(Filter): for _, option in enumerate(options): if option.get("label").lower().find(value.lower()) != -1: option_ids.extend([option.get("id")]) + # A document with multiple custom field instances can match more + # than one of these OR-ed branches (or the same branch via + # different fields), each via its own join to custom_fields -- + # dedupe explicitly rather than relying on the caller to. return ( qs.filter(custom_fields__field__name__icontains=value) | qs.filter(custom_fields__value_text__icontains=value) @@ -280,7 +286,7 @@ class CustomFieldsFilter(Filter): | qs.filter(custom_fields__value_document_ids__icontains=value) | qs.filter(custom_fields__value_select__in=option_ids) | qs.filter(custom_fields__value_long_text__icontains=value) - ) + ).distinct() else: return qs diff --git a/src/documents/tests/test_api_documents.py b/src/documents/tests/test_api_documents.py index b0ec51d68..23a150918 100644 --- a/src/documents/tests/test_api_documents.py +++ b/src/documents/tests/test_api_documents.py @@ -883,6 +883,62 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase): results = response.data["results"] self.assertEqual(len(results), 3) + def test_is_in_inbox_filter_no_duplicates_with_multiple_inbox_tags(self) -> None: + """ + GIVEN: + - A document tagged with two different inbox tags + WHEN: + - The document list is filtered by is_in_inbox=true + THEN: + - The document appears exactly once, not once per matching tag + """ + doc = Document.objects.create(title="doc", checksum="c1") + inbox_1 = Tag.objects.create(name="inbox1", is_inbox_tag=True) + inbox_2 = Tag.objects.create(name="inbox2", is_inbox_tag=True) + doc.tags.add(inbox_1, inbox_2) + + response = self.client.get("/api/documents/?is_in_inbox=true") + self.assertEqual(response.status_code, status.HTTP_200_OK) + results = response.data["results"] + self.assertEqual(len(results), 1) + self.assertEqual(results[0]["id"], doc.id) + + def test_custom_fields_icontains_filter_no_duplicates(self) -> None: + """ + GIVEN: + - A document with two custom field instances that both match the + same custom_fields__icontains search term + WHEN: + - The document list is filtered by custom_fields__icontains + THEN: + - The document appears exactly once, not once per matching field + """ + doc = Document.objects.create(title="doc", checksum="c1") + field_1 = CustomField.objects.create( + name="apple", + data_type=CustomField.FieldDataType.STRING, + ) + field_2 = CustomField.objects.create( + name="apricot", + data_type=CustomField.FieldDataType.STRING, + ) + CustomFieldInstance.objects.create( + document=doc, + field=field_1, + value_text="something", + ) + CustomFieldInstance.objects.create( + document=doc, + field=field_2, + value_text="something else", + ) + + response = self.client.get("/api/documents/?custom_fields__icontains=ap") + self.assertEqual(response.status_code, status.HTTP_200_OK) + results = response.data["results"] + self.assertEqual(len(results), 1) + self.assertEqual(results[0]["id"], doc.id) + def test_custom_field_select_filter(self) -> None: """ GIVEN: diff --git a/src/documents/views.py b/src/documents/views.py index 066f8fd8a..750b54248 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -1063,9 +1063,16 @@ class DocumentViewSet( .values("count"), output_field=IntegerField(), ) + # No .distinct() here: nothing in this base queryset can produce + # duplicate document rows (select_related below is all FK-to-PK; + # permission filtering is a boolean id__in predicate, not a join). + # M2M-based filters that *do* introduce a join (e.g. tags__id__in) + # already call .distinct() themselves where they need it -- see + # ObjectFilter.filter(). A blanket .distinct() here forces the + # database to fully sort and dedupe every visible document before + # it can apply LIMIT, which is disastrous at scale. return ( Document.objects.filter(root_document__isnull=True) - .distinct() .order_by("-created", "-id") .annotate(effective_content=Coalesce(latest_version_content, F("content"))) .annotate(num_notes=Coalesce(note_count, 0))