diff --git a/arkindex/documents/api/elements.py b/arkindex/documents/api/elements.py index dbb7bc00af65f51711c41aa0f40328ffa07ff8e2..3ad90e8da1530fdb8f0685126d82acfbb3bcf8f9 100644 --- a/arkindex/documents/api/elements.py +++ b/arkindex/documents/api/elements.py @@ -561,14 +561,12 @@ class ElementsListBase(CorpusACLMixin, DestroyModelMixin, ListAPIView): return queryset.values('element_id') - def get_filters(self): - filters = { - 'corpus': self.selected_corpus - } + def get_filters(self) -> Q: + filters = Q(corpus=self.selected_corpus) errors = {} if 'name' in self.clean_params: - filters['name__icontains'] = self.clean_params['name'] + filters &= Q(name__icontains=self.clean_params['name']) if 'rotation_angle' in self.clean_params: try: @@ -577,13 +575,13 @@ class ElementsListBase(CorpusACLMixin, DestroyModelMixin, ListAPIView): except (AssertionError, TypeError, ValueError) as e: errors['rotation_angle'] = [str(e)] else: - filters['rotation_angle'] = rotation_angle + filters &= Q(rotation_angle=rotation_angle) if 'mirrored' in self.clean_params: - filters['mirrored'] = self.clean_params['mirrored'].lower() not in ('false', '0') + filters &= Q(mirrored=self.clean_params['mirrored'].lower() not in ('false', '0')) if self.type_filter: - filters['type'] = self.type_filter + filters &= Q(type=self.type_filter) elif self.folder_filter is not None: # When filtering for folder or non-folder elements, using only the type__folder filter # can cause Postgres to retrieve all the {non-}folder types on every corpus @@ -595,12 +593,11 @@ class ElementsListBase(CorpusACLMixin, DestroyModelMixin, ListAPIView): # on the type's corpus ID. The query planner's statistics will give it a very low estimation since there # rarely are a ton of types in a corpus, so Postgres will also use the type_id index on elements, which # will lower the amount of rows much more quickly, making it stop using multi-processing. - filters['type__corpus'] = self.selected_corpus - filters['type__folder'] = self.folder_filter + filters &= Q(type__corpus=self.selected_corpus, type__folder=self.folder_filter) if 'worker_version' in self.clean_params: try: - filters['worker_version_id'] = self.get_worker_version_filter() + filters &= Q(worker_version_id=self.get_worker_version_filter()) except ValidationError as e: errors['worker_version'] = e.detail @@ -610,7 +607,7 @@ class ElementsListBase(CorpusACLMixin, DestroyModelMixin, ListAPIView): errors.update(e.detail) else: if metadata_queryset is not None: - filters['id__in'] = metadata_queryset + filters &= Q(id__in=metadata_queryset) try: classification_queryset = self.get_classification_queryset() @@ -618,7 +615,7 @@ class ElementsListBase(CorpusACLMixin, DestroyModelMixin, ListAPIView): errors.update(e.detail) else: if classification_queryset is not None: - filters['id__in'] = classification_queryset + filters &= Q(id__in=classification_queryset) if errors: raise ValidationError(errors) @@ -669,7 +666,7 @@ class ElementsListBase(CorpusACLMixin, DestroyModelMixin, ListAPIView): def filter_queryset(self, queryset): queryset = queryset \ - .filter(**self.get_filters()) \ + .filter(self.get_filters()) \ .prefetch_related(*self.get_prefetch()) \ .order_by(*self.get_order_by()) @@ -764,13 +761,13 @@ class CorpusElements(ElementsListBase): assert self.selected_corpus, 'Missing corpus ID' return self.selected_corpus.elements.all() - def get_filters(self): + def get_filters(self) -> Q: filters = super().get_filters() if self.is_top_level is not None: # Only include elements with an empty path. # An empty path's last item will be NULL, so we can use the index on path__last too - filters['paths__path__last__isnull'] = self.is_top_level + filters &= Q(paths__path__last__isnull=self.is_top_level) return filters @@ -859,13 +856,13 @@ class ElementChildren(ElementsListBase): def selected_corpus(self): return self.element.corpus - def get_filters(self): + def get_filters(self) -> Q: filters = super().get_filters() if self.is_recursive: - filters['paths__path__overlap'] = [self.kwargs['pk']] + filters &= Q(paths__path__overlap=[self.kwargs['pk']]) else: - filters['paths__path__last'] = self.kwargs['pk'] + filters &= Q(paths__path__last=self.kwargs['pk']) return filters diff --git a/arkindex/documents/tests/test_corpus_elements.py b/arkindex/documents/tests/test_corpus_elements.py index f1fbe9f89d9bf32c131c591697b3ed35b03f2569..02a1e55fdfca80a3dbbfcb4277bc2876da837761 100644 --- a/arkindex/documents/tests/test_corpus_elements.py +++ b/arkindex/documents/tests/test_corpus_elements.py @@ -760,3 +760,32 @@ class TestListElements(FixtureAPITestCase): } ], }) + + def test_metadata_classification_filters(self): + """ + Using classification filters should not cause metadata filters to be ignored + """ + volume = self.corpus.elements.get(name='Volume 1') + page = self.corpus.elements.get(name='Volume 1, page 1r') + ml_class = self.corpus.ml_classes.create(name='Dallas') + + volume.classifications.create(ml_class=ml_class, confidence=1.0) + page.classifications.create(ml_class=ml_class, confidence=0.5) + + # The volume and the page have the Dallas MLClass + self.assertEqual(self.corpus.elements.filter(classifications__ml_class=ml_class).count(), 2) + # All 6 pages have a folio metadata + self.assertEqual(self.corpus.elements.filter(metadatas__name='folio').count(), 6) + # Only the page has both the folio metadata and the Dallas MLClass + self.assertEqual(self.corpus.elements.filter(classifications__ml_class=ml_class, metadatas__name='folio').count(), 1) + + with self.assertNumQueries(6): + response = self.client.get( + reverse('api:corpus-elements', kwargs={'corpus': self.corpus.id}), + {'metadata_name': 'folio', 'class_id': str(ml_class.id)} + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json() + self.assertEqual(data['count'], 1) + self.assertEqual(data['results'][0]['id'], str(page.id))