Skip to content
Snippets Groups Projects
Commit 1dcfba27 authored by ml bonhomme's avatar ml bonhomme :bee: Committed by Erwan Rouchet
Browse files

Remove type name usage in entity creation and update endpoints

parent 70c3f8a8
No related branches found
No related tags found
1 merge request!1995Remove type name usage in entity creation and update endpoints
Hide whitespace changes
Inline Side-by-side
arkindex/documents/serializers/entities.py
+ 13
− 48
View file @ 1dcfba27
......@@ -11,7 +11,7 @@ from arkindex.documents.serializers.light import CorpusLightSerializer, EntityTy
from arkindex.documents.serializers.ml import WorkerRunSummarySerializer
from arkindex.process.models import WorkerRun
from arkindex.project.serializer_fields import ForbiddenField
from arkindex.project.validators import ForbiddenValidator, XorValidator
from arkindex.project.validators import ForbiddenValidator
class EntityTypeSerializer(serializers.ModelSerializer):
......@@ -161,7 +161,7 @@ class EntityLinkSerializer(serializers.ModelSerializer):
@extend_schema_serializer(
deprecate_fields=('type_name', 'worker_version_id')
deprecate_fields=('worker_version_id')
)
class EntitySerializer(BaseEntitySerializer):
"""
......@@ -175,15 +175,9 @@ class EntitySerializer(BaseEntitySerializer):
type_id = serializers.PrimaryKeyRelatedField(
queryset=EntityType.objects.all(),
style={'base_template': 'input.html'},
required=False,
required=True,
write_only=True
)
type_name = serializers.CharField(
required=False,
write_only=True,
help_text=('The name of an existing entity type in the entity\'s corpus. '
'This field is deprecated: use `type_id` instead.')
)
type = EntityTypeLightSerializer(read_only=True)
class Meta:
......@@ -192,17 +186,13 @@ class EntitySerializer(BaseEntitySerializer):
'corpus',
'children',
'parents',
'type_id',
'type_name'
'type_id'
)
read_only_fields = BaseEntitySerializer.Meta.read_only_fields = (
'corpus',
'children',
'parents',
)
validators = [
XorValidator('type_name', 'type_id', required=True)
]
def validate(self, data):
errors = defaultdict(list)
......@@ -210,17 +200,10 @@ class EntitySerializer(BaseEntitySerializer):
if data.get('validated'):
data['moderator'] = self.context['request'].user
corpus = self.instance.corpus
type = data.get('type_id')
type_name = data.get('type_name')
if type_name:
try:
data['type'] = EntityType.objects.get(corpus=corpus, name=type_name)
except EntityType.DoesNotExist:
errors['type_name'].append(f'EntityType {type_name} does not exist in corpus {corpus.name}.')
elif type:
if type.corpus_id != corpus.id:
errors['type_id'].append(f'EntityType {type.id} does not exist in corpus {corpus.name}.')
data['type'] = type
type = data.get('type_id') or self.instance.type
if type.corpus_id != corpus.id:
errors['type_id'].append(f'EntityType {type.id} does not exist in corpus {corpus.name}.')
data['type'] = type
if errors:
raise ValidationError(errors)
......@@ -228,7 +211,7 @@ class EntitySerializer(BaseEntitySerializer):
@extend_schema_serializer(
deprecate_fields=('type_name', 'worker_version', 'worker_version_id')
deprecate_fields=('worker_version', 'worker_version_id')
)
class EntityCreateSerializer(BaseEntitySerializer):
"""
......@@ -260,17 +243,10 @@ class EntityCreateSerializer(BaseEntitySerializer):
source='worker_run',
style={'base_template': 'input.html'},
)
# When creating an entity, the type can be set either by using its EntityType UUID, or its name
# (in which case the serializer checks that an EntityType with this name exists in the corpus)
type_id = serializers.PrimaryKeyRelatedField(
queryset=EntityType.objects.all(),
style={'base_template': 'input.html'},
required=False,
)
type_name = serializers.CharField(
required=False,
help_text=('The name of an existing entity type in the target corpus. '
'This field is deprecated: use `type_id` instead.')
required=True,
)
class Meta:
......@@ -278,7 +254,6 @@ class EntityCreateSerializer(BaseEntitySerializer):
fields = (
'id',
'name',
'type_name',
'type_id',
'metas',
'validated',
......@@ -293,9 +268,6 @@ class EntityCreateSerializer(BaseEntitySerializer):
'children',
'parents',
)
validators = [
XorValidator('type_name', 'type_id', required=True)
]
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
......@@ -314,16 +286,9 @@ class EntityCreateSerializer(BaseEntitySerializer):
corpus = data.get('corpus')
type = data.get('type_id')
type_name = data.get('type_name')
if type_name:
try:
data['type'] = EntityType.objects.get(corpus=corpus, name=type_name)
except EntityType.DoesNotExist:
errors['type_name'].append(f'EntityType {type_name} does not exist in corpus {corpus.name}.')
else:
if type.corpus_id != corpus.id:
errors['type_id'].append(f'EntityType {type.id} does not exist in corpus {corpus.name}.')
data['type'] = type
if type.corpus_id != corpus.id:
errors['type_id'].append(f'EntityType {type.id} does not exist in corpus {corpus.name}.')
data['type'] = type
if errors:
raise ValidationError(errors)
......
arkindex/documents/tests/test_entities_api.py
+ 21
− 195
View file @ 1dcfba27
......@@ -292,63 +292,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
'child_type_id': [f'Type {str(ext_type_2.id)} does not exist in corpus Unit Tests.']
})
def test_create_entity_string_type(self):
data = {
'name': 'entity',
'type_name': 'person',
'corpus': str(self.corpus.id),
'metas': {
'key': 'value',
'other key': 'other value'
},
}
self.client.force_login(self.user)
with self.assertNumQueries(9):
response = self.client.post(reverse('api:entity-create'), data=data, format='json')
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
entity = Entity.objects.get(id=response.json()['id'])
self.assertEqual(entity.name, 'entity')
self.assertEqual(entity.worker_version, None)
self.assertEqual(entity.worker_run, None)
self.assertEqual(entity.type, self.person_type)
def test_create_entity_string_type_does_not_exist(self):
data = {
'name': 'entity',
'type_name': 'bonk',
'corpus': str(self.corpus.id),
'metas': {
'key': 'value',
'other key': 'other value'
},
}
self.client.force_login(self.user)
with self.assertNumQueries(6):
response = self.client.post(reverse('api:entity-create'), data=data, format='json')
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), {'type_name': ['EntityType bonk does not exist in corpus Unit Tests.']})
def test_create_entity_uuid_type(self):
data = {
'name': 'entity',
'type_id': str(self.location_type.id),
'corpus': str(self.corpus.id),
'metas': {
'key': 'value',
'other key': 'other value'
},
}
self.client.force_login(self.user)
with self.assertNumQueries(9):
response = self.client.post(reverse('api:entity-create'), data=data, format='json')
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
entity = Entity.objects.get(id=response.json()['id'])
self.assertEqual(entity.name, 'entity')
self.assertEqual(entity.worker_version, None)
self.assertEqual(entity.worker_run, None)
self.assertEqual(entity.type, self.location_type)
def test_create_entity_uuid_type_invalid(self):
def test_create_entity_type_invalid(self):
data = {
'name': 'entity',
'type_id': 'location',
......@@ -364,7 +308,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), {'type_id': ['“location” is not a valid UUID.']})
def test_create_entity_uuid_type_does_not_exist(self):
def test_create_entity_type_does_not_exist(self):
data = {
'name': 'entity',
'type_id': 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa',
......@@ -380,7 +324,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), {'type_id': ['Invalid pk "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" - object does not exist.']})
def test_create_entity_uuid_type_does_not_exist_in_corpus(self):
def test_create_entity_type_does_not_exist_in_corpus(self):
extraneous_type = EntityType.objects.create(name='Author', corpus=self.private_corpus)
data = {
'name': 'entity',
......@@ -397,23 +341,6 @@ class TestEntitiesAPI(FixtureAPITestCase):
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), {'type_id': [f'EntityType {str(extraneous_type.id)} does not exist in corpus Unit Tests.']})
def test_create_entity_string_or_uuid_type(self):
data = {
'name': 'entity',
'type_id': str(self.location_type.id),
'type_name': 'location',
'corpus': str(self.corpus.id),
'metas': {
'key': 'value',
'other key': 'other value'
},
}
self.client.force_login(self.user)
with self.assertNumQueries(6):
response = self.client.post(reverse('api:entity-create'), data=data, format='json')
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), {'non_field_errors': ['Only one of `type_name` and `type_id` may be set.']})
def test_create_entity_require_type(self):
data = {
'name': 'entity',
......@@ -427,12 +354,12 @@ class TestEntitiesAPI(FixtureAPITestCase):
with self.assertNumQueries(5):
response = self.client.post(reverse('api:entity-create'), data=data, format='json')
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), {'non_field_errors': ['One of `type_name` or `type_id` must be set.']})
self.assertDictEqual(response.json(), {'type_id': ['This field is required.']})
def test_create_entity_requires_login(self):
data = {
'name': '1789',
'type_name': 'date',
'name': 'Paris',
'type_id': str(self.location_type.id),
'corpus': str(self.corpus.id),
'metas': {
'key': 'value',
......@@ -446,7 +373,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
def test_create_entity_with_worker_version(self):
data = {
'name': 'entity',
'type_name': 'person',
'type_id': str(self.person_type.id),
'corpus': str(self.corpus.id),
'worker_version': str(self.worker_version_1.id),
'metas': {
......@@ -455,7 +382,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
},
}
self.client.force_login(self.user)
with self.assertNumQueries(5):
with self.assertNumQueries(6):
response = self.client.post(reverse('api:entity-create'), data=data, format='json')
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.json(), {
......@@ -465,7 +392,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
def test_create_entity_with_worker_run(self):
data = {
'name': 'entity',
'type_name': 'person',
'type_id': str(self.person_type.id),
'corpus': str(self.corpus.id),
'worker_run_id': str(self.worker_run_1.id),
'metas': {
......@@ -521,7 +448,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
def test_create_entity_with_unknown_worker_run(self):
data = {
'name': 'entity',
'type_name': 'person',
'type_id': str(self.person_type.id),
'corpus': str(self.corpus.id),
'worker_run_id': 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa',
'metas': {
......@@ -530,7 +457,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
},
}
self.client.force_login(self.user)
with self.assertNumQueries(6):
with self.assertNumQueries(7):
response = self.client.post(reverse('api:entity-create'), data=data, format='json')
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), {'worker_run_id': ['Invalid pk "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" - object does not exist.']})
......@@ -538,7 +465,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
def test_create_entity_with_worker_run_and_worker_version(self):
data = {
'name': 'entity',
'type_name': 'person',
'type_id': str(self.person_type.id),
'corpus': str(self.corpus.id),
'worker_version': str(self.worker_version_1.id),
'worker_run_id': str(self.worker_run_1.id),
......@@ -548,97 +475,13 @@ class TestEntitiesAPI(FixtureAPITestCase):
},
}
self.client.force_login(self.user)
with self.assertNumQueries(6):
with self.assertNumQueries(7):
response = self.client.post(reverse('api:entity-create'), data=data, format='json')
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), {
'worker_version': ['This field is forbidden.'],
})
def test_create_entity_use_existing_exists(self):
self.client.force_login(self.user)
self.assertEqual(Entity.objects.filter(corpus=self.corpus).count(), 2)
data = {
'name': self.entity.name,
'type_name': self.entity.type.name,
'corpus': str(self.corpus.id),
'use_existing': True,
'worker_run_id': str(self.worker_run_1.id),
'metas': {
'key': 'value',
'other key': 'other value'
},
}
with self.assertNumQueries(12):
response = self.client.post(reverse('api:entity-create'), data=data, format='json')
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertDictEqual(response.json(), {
'id': str(self.entity.id),
'name': 'entity 1',
'type': {
'id': str(self.person_type.id),
'name': 'person',
'color': 'ff0000'
},
'corpus': {
'id': str(self.corpus.id),
'name': 'Unit Tests',
'public': True
},
'validated': False,
'worker_version_id': str(self.worker_version_1.id),
'worker_run': None,
'metas': None,
'parents': [],
'children': []
})
self.assertEqual(Entity.objects.filter(corpus=self.corpus).count(), 2)
def test_create_entity_use_existing_does_not_exist(self):
self.client.force_login(self.user)
self.assertEqual(Entity.objects.filter(corpus=self.corpus).count(), 2)
data = {
'name': 'Maverick',
'type_id': str(self.person_type.id),
'corpus': str(self.corpus.id),
'use_existing': True,
'metas': {
'key': 'value',
'other key': 'other value'
},
}
with self.assertNumQueries(12):
response = self.client.post(reverse('api:entity-create'), data=data, format='json')
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
created_entity = Entity.objects.get(pk=response.json()['id'])
self.assertDictEqual(response.json(), {
'id': str(created_entity.id),
'name': 'Maverick',
'type': {
'id': str(self.person_type.id),
'name': 'person',
'color': 'ff0000'
},
'corpus': {
'id': str(self.corpus.id),
'name': 'Unit Tests',
'public': True
},
'validated': False,
'worker_version_id': None,
'worker_run': None,
'metas': {
'key': 'value',
'other key': 'other value'
},
'parents': [],
'children': []
})
self.assertEqual(created_entity.name, 'Maverick')
self.assertEqual(created_entity.type.id, self.person_type.id)
self.assertEqual(created_entity.metas, {'key': 'value', 'other key': 'other value'})
self.assertEqual(Entity.objects.filter(corpus=self.corpus).count(), 3)
def test_create_link(self):
child = Entity.objects.create(
type=self.location_type,
......@@ -1648,7 +1491,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
{'name': 'a new name'},
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.json(), {'non_field_errors': ['One of `type_name` or `type_id` must be set.']})
self.assertEqual(response.json(), {'type_id': ['This field is required.']})
def test_update_validate_entity(self):
self.client.force_login(self.user)
......@@ -1657,7 +1500,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
reverse('api:entity-details', kwargs={'pk': self.entity_bis.id}),
{
'name': self.entity_bis.name,
'type_name': 'location',
'type_id': str(self.location_type.id),
'validated': True
},
)
......@@ -1673,7 +1516,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
reverse('api:entity-details', kwargs={'pk': self.entity_bis.id}),
{
'name': self.entity_bis.name,
'type_name': 'location',
'type_id': str(self.location_type.id),
'validated': False
},
)
......@@ -1688,7 +1531,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
reverse('api:entity-details', kwargs={'pk': self.entity_bis.id}),
{
'name': self.entity_bis.name,
'type_name': 'location',
'type_id': str(self.location_type.id),
'validated': True
},
)
......@@ -1700,7 +1543,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
reverse('api:entity-details', kwargs={'pk': self.entity_bis.id}),
{
'name': self.entity_bis.name,
'type_name': 'location',
'type_id': str(self.location_type.id),
'validated': False
},
)
......@@ -1715,7 +1558,7 @@ class TestEntitiesAPI(FixtureAPITestCase):
reverse('api:entity-details', kwargs={'pk': self.entity_bis.id}),
{
'name': self.entity_bis.name,
'type_name': 'location',
'type_id': str(self.location_type.id),
'validated': True
},
)
......@@ -1730,30 +1573,13 @@ class TestEntitiesAPI(FixtureAPITestCase):
reverse('api:entity-details', kwargs={'pk': self.entity_bis.id}),
{
'name': self.entity_bis.name,
'type_name': 'location',
'type_id': str(self.location_type.id),
'validated': False
},
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
def test_update_entity_change_type_type_name(self):
self.client.force_login(self.user)
with self.assertNumQueries(12):
response = self.client.put(
reverse('api:entity-details', kwargs={'pk': self.entity_bis.id}),
{
'name': self.entity_bis.name,
'type_name': 'person',
'validated': False
},
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.entity_bis.refresh_from_db()
self.assertEqual(self.entity_bis.validated, False)
self.assertEqual(self.entity_bis.moderator, None)
self.assertEqual(self.entity_bis.type, self.person_type)
def test_update_entity_change_type_type_id(self):
def test_update_entity_change_type(self):
self.client.force_login(self.user)
with self.assertNumQueries(12):
response = self.client.put(
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment