Skip to content
Snippets Groups Projects

Compare revisions

Changes are shown as if the source revision was being merged into the target revision. Learn more about comparing revisions.

Source

Select target project
No results found

Target

Select target project
  • arkindex/backend
1 result
Show changes
Commits on Source (11)
1.1.1-beta2
1.1.1-rc1
......@@ -134,6 +134,8 @@ def _fetch_has_children(elements):
METADATA_OPERATORS = {'eq', 'lt', 'gt', 'lte', 'gte'}
ELEMENT_ORDERING_FIELDS = {'name', 'created'}
class ElementsListAutoSchema(AutoSchema):
"""
......@@ -213,6 +215,20 @@ class ElementsListAutoSchema(AutoSchema):
default='eq',
required=False,
),
OpenApiParameter(
'order',
description='Sort elements by a specific field',
enum=ELEMENT_ORDERING_FIELDS,
default='name',
required=False,
),
OpenApiParameter(
'order_direction',
description='Direction in which to sort elements',
enum={'asc', 'desc'},
default='asc',
required=False,
),
OpenApiParameter(
'with_best_classes',
description='Returns best classifications for each element. '
......@@ -499,8 +515,21 @@ class ElementsListBase(CorpusACLMixin, DestroyModelMixin, ListAPIView):
return prefetch
def get_order_by(self):
# Ordering by ID is required by postgres to order elements with the same name
return ('name', 'id')
errors = {}
sort_field = self.clean_params.get('order', 'name').lower()
if sort_field not in ELEMENT_ORDERING_FIELDS:
errors['order'] = ['Unknown sorting field']
direction = self.clean_params.get('order_direction', 'asc').lower()
if direction not in ('asc', 'desc'):
errors['order_direction'] = ['Unknown sorting direction']
if errors:
raise ValidationError(errors)
# The sorting field is not unique; fallback on ordering by ID to ensure a consistent ordering
return (f'{"-" if direction == "desc" else ""}{sort_field}', 'id')
def filter_queryset(self, queryset):
queryset = queryset \
......@@ -573,7 +602,7 @@ class ElementsListBase(CorpusACLMixin, DestroyModelMixin, ListAPIView):
parameters=[
OpenApiParameter(
'top_level',
description='Only include folder elements without parent elements (top-level elements).',
description='Restrict to or exclude folder elements without parent elements (top-level elements).',
type=bool,
required=False,
)
......@@ -583,19 +612,21 @@ class ElementsListBase(CorpusACLMixin, DestroyModelMixin, ListAPIView):
get=extend_schema(operation_id='ListElements'),
delete=extend_schema(operation_id='DestroyElements', description=(
'Destroy elements in bulk.\n\n'
'Requires an **admin** access to the process.'
'Requires an **admin** access to the corpus.'
)),
)
class CorpusElements(ElementsListBase):
"""
List elements in a corpus and filter by type, name, ML class.
List elements in a corpus.
Requires an **read** access to the corpus.
Requires a **read** access to the corpus.
"""
@property
def is_top_level(self):
return self.clean_params.get('top_level') not in (None, 'false', '0')
if self.clean_params.get('top_level') is None:
return None
return self.clean_params['top_level'].lower() not in ('false', '0')
def get_queryset(self):
# Should not be possible due to the URL
......@@ -605,14 +636,9 @@ class CorpusElements(ElementsListBase):
def get_filters(self):
filters = super().get_filters()
if self.is_top_level:
# Filter elements with no parent
filters['paths__isnull'] = True
# First filter elements by type to make the SQL query more performant
if self.selected_corpus.top_level_type_id:
filters['type_id'] = self.selected_corpus.top_level_type_id
else :
filters['type__in'] = self.selected_corpus.types.filter(folder=True)
if self.is_top_level is not None:
# Filter elements with no parent or with any parent
filters['paths__isnull'] = self.is_top_level
return filters
......@@ -671,7 +697,15 @@ class ElementParents(ElementsListBase):
]
)
@extend_schema_view(
get=extend_schema(operation_id='ListElementChildren'),
get=extend_schema(operation_id='ListElementChildren', parameters=[
OpenApiParameter(
'order',
description='Sort elements by a specific field',
enum=ELEMENT_ORDERING_FIELDS | {'position'},
default='name',
required=False,
)
]),
delete=extend_schema(operation_id='DestroyElementChildren', description=(
'Delete child elements in bulk.\n\n'
"Requires an **admin** access to the element's corpus."
......@@ -714,7 +748,14 @@ class ElementChildren(ElementsListBase):
return filters
def get_order_by(self):
return ('paths__ordering', 'id')
"""
Override the ElementsListBase ordering to default by path ordering.
The only endpoint where a path ordering is made visible in the API
is ListElementNeighbors, in a field named `position`.
"""
if self.clean_params.get('order', 'position').lower() == 'position':
return ('paths__ordering', 'id')
return super().get_order_by()
def get_queryset(self):
self.element = get_object_or_404(
......
......@@ -148,6 +148,12 @@ class CorpusSerializer(serializers.ModelSerializer):
"""
rights = serializers.SerializerMethodField(read_only=True)
types = ElementTypeLightSerializer(many=True, read_only=True)
top_level_type = serializers.SlugRelatedField(
queryset=ElementType.objects.none(),
slug_field='slug',
allow_null=True,
default=None,
)
authorized_users = serializers.SerializerMethodField(read_only=True)
thumbnail = serializers.PrimaryKeyRelatedField(
queryset=Element.objects.none(),
......@@ -166,6 +172,7 @@ class CorpusSerializer(serializers.ModelSerializer):
'public',
'rights',
'types',
'top_level_type',
'created',
'authorized_users',
'indexable',
......@@ -176,8 +183,9 @@ class CorpusSerializer(serializers.ModelSerializer):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if isinstance(self.instance, Corpus):
# There is an existing corpus; allow one of its elements as the thumbnail
# There is an existing corpus; allow its elements as thumbnails and its types as top level types
self.fields['thumbnail'].queryset = self.instance.elements.exclude(image_id=None)
self.fields['top_level_type'].queryset = self.instance.types.all()
@extend_schema_field(serializers.ListField(child=serializers.ChoiceField(['read', 'write', 'admin'])))
def get_rights(self, corpus):
......
import uuid
from datetime import datetime, timedelta, timezone
from django.urls import reverse
from rest_framework import status
......@@ -477,3 +478,71 @@ class TestChildrenElements(FixtureAPITestCase):
[element['name'] for element in response.json()['results']],
expected_elements
)
def test_children_invalid_sort(self):
cases = [
({'order': 'blah', 'order_direction': 'asc'}, {'order': ['Unknown sorting field']}),
({'order': 'name', 'order_direction': 'left'}, {'order_direction': ['Unknown sorting direction']}),
(
{'order': 'blah', 'order_direction': 'left'},
{'order': ['Unknown sorting field'], 'order_direction': ['Unknown sorting direction']}
),
]
for params, expected in cases:
with self.subTest(**params):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': self.vol.id}),
params,
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), expected)
def test_children_sort_created(self):
elements = list(Element.objects.get_descending(self.vol.id).order_by('?').only('id'))
created = datetime(2020, 2, 2, tzinfo=timezone.utc)
for element in elements:
element.created = created
created += timedelta(seconds=1)
Element.objects.bulk_update(elements, ['created'])
cases = [
('asc', elements[:20]),
('desc', reversed(elements[-20:])),
]
for direction, expected in cases:
with self.subTest(direction=direction), self.assertNumQueries(5):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': self.vol.id}),
{'order': 'created', 'order_direction': direction, 'recursive': True}
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertListEqual(
[element['id'] for element in response.json()['results']],
[str(element.id) for element in expected]
)
def test_children_sort_name(self):
names = list(Element.objects.get_descending(self.vol.id).order_by('name', 'id').values_list('name', flat=True))
# Ensure the default position ordering returns a different ordering than by name;
# otherwise, this test would be useless
self.assertNotEqual(
list(Element.objects.get_descending(self.vol.id).values_list('name', flat=True)),
names,
)
cases = [
('asc', names[:20]),
('desc', list(reversed(names[-20:]))),
]
for direction, expected in cases:
with self.subTest(direction=direction), self.assertNumQueries(5):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': self.vol.id}),
{'order': 'name', 'order_direction': direction, 'recursive': True}
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertListEqual(
[element['name'] for element in response.json()['results']],
expected
)
......@@ -94,6 +94,7 @@ class TestCorpus(FixtureAPITestCase):
'authorized_users': 1,
'thumbnail': None,
'thumbnail_url': None,
'top_level_type': None,
}
]
)
......@@ -126,6 +127,7 @@ class TestCorpus(FixtureAPITestCase):
'authorized_users': 2,
'thumbnail': None,
'thumbnail_url': None,
'top_level_type': None,
},
{
'id': str(self.corpus_public.id),
......@@ -138,6 +140,7 @@ class TestCorpus(FixtureAPITestCase):
'authorized_users': 1,
'thumbnail': None,
'thumbnail_url': None,
'top_level_type': None,
}
]
)
......@@ -172,6 +175,7 @@ class TestCorpus(FixtureAPITestCase):
'authorized_users': 2,
'thumbnail': None,
'thumbnail_url': None,
'top_level_type': None,
},
{
'id': str(self.corpus_hidden.id),
......@@ -184,6 +188,7 @@ class TestCorpus(FixtureAPITestCase):
'authorized_users': 0,
'thumbnail': None,
'thumbnail_url': None,
'top_level_type': None,
},
{
'id': str(self.corpus_public.id),
......@@ -196,6 +201,7 @@ class TestCorpus(FixtureAPITestCase):
'authorized_users': 1,
'thumbnail': None,
'thumbnail_url': None,
'top_level_type': None,
}
]
)
......@@ -306,6 +312,7 @@ class TestCorpus(FixtureAPITestCase):
'authorized_users': 1,
'thumbnail': None,
'thumbnail_url': None,
'top_level_type': None,
})
def test_retrieve(self):
......@@ -325,6 +332,7 @@ class TestCorpus(FixtureAPITestCase):
'authorized_users': 2,
'thumbnail': None,
'thumbnail_url': None,
'top_level_type': None,
})
def test_retrieve_requires_login(self):
......@@ -462,6 +470,29 @@ class TestCorpus(FixtureAPITestCase):
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertDictEqual(response.json(), {'detail': 'You do not have admin access to this corpus.'})
def test_update_top_level_type_same_corpus(self):
"""
Cannot set a corpus' top-level type to a type from another corpus
"""
self.client.force_login(self.superuser)
self.corpus_private.types.create(slug='nope')
self.assertFalse(self.corpus_public.types.filter(slug='nope').exists())
response = self.client.patch(reverse('api:corpus-retrieve', kwargs={'pk': self.corpus_public.id}), {
'top_level_type': 'nope',
})
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), {'top_level_type': ['Object with slug=nope does not exist.']})
def test_update_top_level_type(self):
self.client.force_login(self.superuser)
self.assertIsNone(self.corpus_public.top_level_type)
response = self.client.patch(reverse('api:corpus-retrieve', kwargs={'pk': self.corpus_public.id}), {
'top_level_type': 'page',
})
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.corpus_public.refresh_from_db()
self.assertEqual(self.corpus_public.top_level_type.slug, 'page')
def test_delete_requires_login(self):
response = self.client.delete(reverse('api:corpus-retrieve', kwargs={'pk': self.corpus_private.id}))
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
......
import uuid
from datetime import datetime, timedelta, timezone
import sqlparse
from django.db.models.sql.constants import LOUTER
......@@ -242,7 +243,8 @@ class TestListElements(FixtureAPITestCase):
def test_list_top_level_default_type(self):
"""
Use the corpus top level type to filter top level elements when available
Do not use the corpus top level type to filter top level elements when available;
the frontend or API users are in charge of that.
"""
# Add a top level element of type page
element_type = self.corpus.types.create(slug='top_level', display_name='Some type')
......@@ -258,7 +260,59 @@ class TestListElements(FixtureAPITestCase):
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertListEqual(
[element['name'] for element in response.json()['results']],
['Generic element'],
['Generic element', 'Volume 1', 'Volume 2'],
)
def test_list_top_level_non_folder(self):
"""
The top_level filter should allow non-folder elements
"""
self.corpus.elements.create(type=self.corpus.types.filter(folder=False).first(), name='lonely page')
with self.assertNumQueries(4):
response = self.client.get(
reverse('api:corpus-elements', kwargs={'corpus': self.corpus.id}),
data={'top_level': True},
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertListEqual(
[element['name'] for element in response.json()['results']],
['lonely page', 'Volume 1', 'Volume 2']
)
def test_list_non_top_level(self):
with self.assertNumQueries(5):
response = self.client.get(
reverse('api:corpus-elements', kwargs={'corpus': self.corpus.id}),
data={'top_level': False},
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertListEqual(
[element['name'] for element in response.json()['results']],
[
'Act 1',
'Act 2',
'Act 3',
'Act 4',
'Act 5',
'DATUM',
'DATUM',
'DATUM',
'PARIS',
'PARIS',
'PARIS',
'ROY',
'ROY',
'ROY',
'Surface A',
'Surface B',
'Surface C',
'Surface D',
'Surface E',
'Surface F',
]
)
def test_list_elements_filter_no_worker_version(self):
......@@ -518,3 +572,45 @@ class TestListElements(FixtureAPITestCase):
data = response.json()
self.assertEqual(len(data['results']), 500)
self.assertEqual(data['count'], 999)
def test_list_elements_invalid_sort(self):
cases = [
({'order': 'blah', 'order_direction': 'asc'}, {'order': ['Unknown sorting field']}),
({'order': 'name', 'order_direction': 'left'}, {'order_direction': ['Unknown sorting direction']}),
(
{'order': 'blah', 'order_direction': 'left'},
{'order': ['Unknown sorting field'], 'order_direction': ['Unknown sorting direction']}
),
]
for params, expected in cases:
with self.subTest(**params):
response = self.client.get(
reverse('api:corpus-elements', kwargs={'corpus': self.corpus.id}),
params,
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), expected)
def test_list_elements_sort_created(self):
elements = list(self.corpus.elements.order_by('?').only('id'))
created = datetime(2020, 2, 2, tzinfo=timezone.utc)
for element in elements:
element.created = created
created += timedelta(seconds=1)
Element.objects.bulk_update(elements, ['created'])
cases = [
('asc', elements[:20]),
('desc', reversed(elements[-20:])),
]
for direction, expected in cases:
with self.subTest(direction=direction), self.assertNumQueries(5):
response = self.client.get(
reverse('api:corpus-elements', kwargs={'corpus': self.corpus.id}),
{'order': 'created', 'order_direction': direction}
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertListEqual(
[element['id'] for element in response.json()['results']],
[str(element.id) for element in expected]
)
import uuid
from datetime import datetime, timedelta, timezone
from django.urls import reverse
from rest_framework import status
......@@ -294,3 +295,45 @@ class TestParentsElements(FixtureAPITestCase):
[element['name'] for element in response.json()['results']],
expected_elements
)
def test_parents_invalid_sort(self):
cases = [
({'order': 'blah', 'order_direction': 'asc'}, {'order': ['Unknown sorting field']}),
({'order': 'name', 'order_direction': 'left'}, {'order_direction': ['Unknown sorting direction']}),
(
{'order': 'blah', 'order_direction': 'left'},
{'order': ['Unknown sorting field'], 'order_direction': ['Unknown sorting direction']}
),
]
for params, expected in cases:
with self.subTest(**params):
response = self.client.get(
reverse('api:elements-parents', kwargs={'pk': self.page.id}),
params,
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), expected)
def test_parents_sort_created(self):
elements = list(Element.objects.get_ascending(self.page.id).order_by('?').only('id'))
created = datetime(2020, 2, 2, tzinfo=timezone.utc)
for element in elements:
element.created = created
created += timedelta(seconds=1)
Element.objects.bulk_update(elements, ['created'])
cases = [
('asc', elements[:20]),
('desc', reversed(elements[-20:])),
]
for direction, expected in cases:
with self.subTest(direction=direction), self.assertNumQueries(4):
response = self.client.get(
reverse('api:elements-parents', kwargs={'pk': self.page.id}),
{'order': 'created', 'order_direction': direction, 'recursive': True}
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertListEqual(
[element['id'] for element in response.json()['results']],
[str(element.id) for element in expected]
)
......@@ -10,13 +10,13 @@ django-redis==5.0.0
django-rq==2.4.1
djangorestframework==3.12.4
drf-spectacular==0.18.2
gitpython==3.1.20
gitpython==3.1.24
python-gitlab==2.10.1
python-memcached==1.59
pytz==2021.1
PyYAML==5.4.1
requests==2.26.0
sentry-sdk==1.3.1
sentry-sdk==1.4.1
SolrClient==0.3.1
teklia-toolbox==0.1.2
tenacity==8.0.1
......