From 8cf0897ec5df9b4a2b74f175fe4ffee0a7843ff5 Mon Sep 17 00:00:00 2001 From: jedi Date: Mon, 11 Mar 2024 17:37:56 +0100 Subject: [PATCH] make dataset parsing more robust --- backend/configure.py | 21 ++++- backend/hostadmin/admin.py | 10 ++- .../0003_importedidentifiersets_hash.py | 39 ++++++++++ ...04_alter_importedidentifiersets_options.py | 17 ++++ backend/hostadmin/models.py | 4 + backend/hostadmin/serializers.py | 71 +++++++++++++++-- backend/hostadmin/tests.py | 77 +++++++++++++++++-- backend/toolshed/admin.py | 18 +++-- ...ag_options_alter_category_name_and_more.py | 67 ++++++++++++++++ backend/toolshed/models.py | 32 ++++++-- backend/toolshed/tests/fixtures.py | 3 +- backend/toolshed/tests/test_api.py | 3 +- backend/toolshed/tests/test_category.py | 11 ++- 13 files changed, 337 insertions(+), 36 deletions(-) create mode 100644 backend/hostadmin/migrations/0003_importedidentifiersets_hash.py create mode 100644 backend/hostadmin/migrations/0004_alter_importedidentifiersets_options.py create mode 100644 backend/toolshed/migrations/0006_alter_tag_options_alter_category_name_and_more.py diff --git a/backend/configure.py b/backend/configure.py index 67b87b5..ee26cac 100755 --- a/backend/configure.py +++ b/backend/configure.py @@ -83,10 +83,12 @@ def configure(): if yesno("Do you want to import all categories, properties and tags contained in this repository?", default=True): from hostadmin.serializers import CategorySerializer, PropertySerializer, TagSerializer from hostadmin.models import ImportedIdentifierSets + from hashlib import sha256 if not os.path.exists('shared_data'): os.mkdir('shared_data') files = os.listdir('shared_data') idsets = {} + hashes = {} for file in files: if file.endswith('.json'): name = "git:" + file[:-5] @@ -94,6 +96,8 @@ def configure(): try: idset = json.load(f) idsets[name] = idset + f.seek(0) + hashes[name] = sha256(f.read().encode()).hexdigest() except json.decoder.JSONDecodeError: print('Error: invalid JSON in file {}'.format(file)) imported_sets = ImportedIdentifierSets.objects.all() @@ -108,9 +112,13 @@ def configure(): unmet_deps = [dep for dep in idset['depends'] if not imported_sets.filter(name=dep).exists()] if unmet_deps: if all([dep in idsets.keys() for dep in unmet_deps]): - print('Not all dependencies for {} are imported, postponing'.format(name)) - queue.append(name) - continue + if all([dep in queue for dep in unmet_deps]): + print('Not all dependencies for {} are imported, postponing'.format(name)) + queue.append(name) + continue + else: + print('Error: unresolvable dependencies for {}: {}'.format(name, unmet_deps)) + continue else: print('unknown dependencies for {}: {}'.format(name, unmet_deps)) continue @@ -131,10 +139,15 @@ def configure(): serializer = TagSerializer(data=tag) if serializer.is_valid(): serializer.save(origin=name) - imported_sets.create(name=name) + imported_sets.create(name=name, hash=hashes[name]) except IntegrityError: print('Error: integrity error while importing {}\n\tmight be cause by name conflicts with existing' ' categories, properties or tags'.format(name)) + transaction.set_rollback(True) + continue + except Exception as e: + print('Error: {}'.format(e)) + transaction.set_rollback(True) continue diff --git a/backend/hostadmin/admin.py b/backend/hostadmin/admin.py index 84e6155..4dbabc7 100644 --- a/backend/hostadmin/admin.py +++ b/backend/hostadmin/admin.py @@ -1,6 +1,6 @@ from django.contrib import admin -from .models import Domain +from .models import Domain, ImportedIdentifierSets class DomainAdmin(admin.ModelAdmin): @@ -9,3 +9,11 @@ class DomainAdmin(admin.ModelAdmin): admin.site.register(Domain, DomainAdmin) + + +class ImportedIdentifierSetsAdmin(admin.ModelAdmin): + list_display = ('name', 'hash', 'created_at') + list_filter = ('name', 'hash', 'created_at') + + +admin.site.register(ImportedIdentifierSets, ImportedIdentifierSetsAdmin) diff --git a/backend/hostadmin/migrations/0003_importedidentifiersets_hash.py b/backend/hostadmin/migrations/0003_importedidentifiersets_hash.py new file mode 100644 index 0000000..d62713f --- /dev/null +++ b/backend/hostadmin/migrations/0003_importedidentifiersets_hash.py @@ -0,0 +1,39 @@ +# Generated by Django 4.2.2 on 2024-03-11 15:19 +import os + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ('hostadmin', '0002_importedidentifiersets'), + ] + + def calculate_hash(apps, schema_editor): + from hostadmin.models import ImportedIdentifierSets + for identifier_set in ImportedIdentifierSets.objects.all(): + if not identifier_set.hash: + print("update", identifier_set.name) + filename = "shared_data/" + identifier_set.name.strip('git:') + ".json" + if not os.path.exists(filename): + continue + from hashlib import sha256 + with open(filename, 'r') as file: + data = file.read() + identifier_set.hash = sha256(data.encode()).hexdigest() + identifier_set.save() + + operations = [ + migrations.AddField( + model_name='importedidentifiersets', + name='hash', + field=models.CharField(blank=True, max_length=255, null=True), + + ), + migrations.RunPython(calculate_hash), + migrations.AlterField( + model_name='importedidentifiersets', + name='hash', + field=models.CharField(max_length=255, unique=True), + ), + ] diff --git a/backend/hostadmin/migrations/0004_alter_importedidentifiersets_options.py b/backend/hostadmin/migrations/0004_alter_importedidentifiersets_options.py new file mode 100644 index 0000000..0d0404d --- /dev/null +++ b/backend/hostadmin/migrations/0004_alter_importedidentifiersets_options.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.2 on 2024-03-14 16:33 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('hostadmin', '0003_importedidentifiersets_hash'), + ] + + operations = [ + migrations.AlterModelOptions( + name='importedidentifiersets', + options={'verbose_name_plural': 'imported identifier sets'}, + ), + ] diff --git a/backend/hostadmin/models.py b/backend/hostadmin/models.py index bac6ac1..da29811 100644 --- a/backend/hostadmin/models.py +++ b/backend/hostadmin/models.py @@ -12,4 +12,8 @@ class Domain(models.Model): class ImportedIdentifierSets(models.Model): name = models.CharField(max_length=255, unique=True) + hash = models.CharField(max_length=255, unique=True) created_at = models.DateTimeField(auto_now_add=True) + + class Meta: + verbose_name_plural = 'imported identifier sets' diff --git a/backend/hostadmin/serializers.py b/backend/hostadmin/serializers.py index 72bacba..02fadb6 100644 --- a/backend/hostadmin/serializers.py +++ b/backend/hostadmin/serializers.py @@ -5,6 +5,32 @@ from hostadmin.models import Domain from toolshed.models import Category, Property, Tag +class SlugPathField(serializers.SlugRelatedField): + def to_internal_value(self, data): + path = data.split('/') if '/' in data else [data] + candidates = self.get_queryset().filter(name=path[-1]) + if len(candidates) == 1: + return candidates.first() + if len(candidates) == 0: + raise serializers.ValidationError( + "No {} with name '{}' found".format(self.queryset.model.__name__, path[-1])) + if len(candidates) > 1 and len(path) == 1: + raise serializers.ValidationError("Multiple {}s with name '{}' found, please specify the parent".format( + self.queryset.model.__name__, path[-1])) + parent = self.to_internal_value('/'.join(path[:-1])) + candidates = self.get_queryset().filter(name=path[-1], parent=parent) + if len(candidates) == 1: + return candidates.first() + if len(candidates) == 0: + raise serializers.ValidationError( + "No {} with name '{}' found".format(self.queryset.model.__name__, path[-1])) + + def to_representation(self, value): + source = getattr(value, self.field_name, None) # should this use self.source? + prefix = self.to_representation(source) + '/' if source else '' + return prefix + getattr(value, self.slug_field) + + class DomainSerializer(serializers.ModelSerializer): owner = OwnerSerializer(read_only=True) @@ -12,12 +38,21 @@ class DomainSerializer(serializers.ModelSerializer): model = Domain fields = ['name', 'owner', 'open_registration'] - def create(self, validated_data): - return super().create(validated_data) - class CategorySerializer(serializers.ModelSerializer): - parent = serializers.SlugRelatedField(slug_field='name', queryset=Category.objects.all(), required=False) + parent = SlugPathField(slug_field='name', queryset=Category.objects.all(), required=False) + + def validate(self, attrs): + if 'name' in attrs: + if '/' in attrs['name']: + raise serializers.ValidationError("Category name cannot contain '/'") + return attrs + + def create(self, validated_data): + try: + return Category.objects.create(**validated_data) + except Exception as e: + raise serializers.ValidationError(e) class Meta: model = Category @@ -27,7 +62,19 @@ class CategorySerializer(serializers.ModelSerializer): class PropertySerializer(serializers.ModelSerializer): - category = serializers.SlugRelatedField(slug_field='name', queryset=Category.objects.all(), required=False) + category = SlugPathField(slug_field='name', queryset=Category.objects.all(), required=False) + + def validate(self, attrs): + if 'name' in attrs: + if '/' in attrs['name']: + raise serializers.ValidationError("Property name cannot contain '/'") + return attrs + + def create(self, validated_data): + try: + return Property.objects.create(**validated_data) + except Exception as e: + raise serializers.ValidationError(e) class Meta: model = Property @@ -38,7 +85,19 @@ class PropertySerializer(serializers.ModelSerializer): class TagSerializer(serializers.ModelSerializer): - category = serializers.SlugRelatedField(slug_field='name', queryset=Category.objects.all(), required=False) + category = SlugPathField(slug_field='name', queryset=Category.objects.all(), required=False) + + def validate(self, attrs): + if 'name' in attrs: + if '/' in attrs['name']: + raise serializers.ValidationError("Tag name cannot contain '/'") + return attrs + + def create(self, validated_data): + try: + return Tag.objects.create(**validated_data) + except Exception as e: + raise serializers.ValidationError(e) class Meta: model = Tag diff --git a/backend/hostadmin/tests.py b/backend/hostadmin/tests.py index 513d705..0df45c2 100644 --- a/backend/hostadmin/tests.py +++ b/backend/hostadmin/tests.py @@ -100,7 +100,8 @@ class CategoryApiTestCase(UserTestMixin, CategoryTestMixin, ToolshedTestCase): response = client.get('/api/categories/', self.f['local_user1']) self.assertEqual(response.status_code, 200) self.assertEqual(response.json(), - ["cat1", "cat2", "cat3", "cat1/subcat1", "cat1/subcat2", "cat1/subcat1/subcat3"]) + ["cat1", "cat2", "cat3", "cat1/subcat1", + "cat1/subcat2", "cat1/subcat1/subcat1", "cat1/subcat1/subcat2"]) def test_admin_get_categories_fail(self): response = client.get('/admin/categories/', self.f['local_user1']) @@ -109,7 +110,7 @@ class CategoryApiTestCase(UserTestMixin, CategoryTestMixin, ToolshedTestCase): def test_admin_get_categories(self): response = client.get('/admin/categories/', self.f['admin']) self.assertEqual(response.status_code, 200) - self.assertEqual(len(response.json()), 6) + self.assertEqual(len(response.json()), 7) self.assertEqual(response.json()[0]['name'], 'cat1') self.assertEqual(response.json()[1]['name'], 'cat2') self.assertEqual(response.json()[2]['name'], 'cat3') @@ -117,10 +118,12 @@ class CategoryApiTestCase(UserTestMixin, CategoryTestMixin, ToolshedTestCase): self.assertEqual(response.json()[3]['parent'], 'cat1') self.assertEqual(response.json()[4]['name'], 'subcat2') self.assertEqual(response.json()[4]['parent'], 'cat1') - self.assertEqual(response.json()[5]['name'], 'subcat3') - self.assertEqual(response.json()[5]['parent'], 'subcat1') + self.assertEqual(response.json()[5]['name'], 'subcat1') + self.assertEqual(response.json()[5]['parent'], 'cat1/subcat1') + self.assertEqual(response.json()[6]['name'], 'subcat2') + self.assertEqual(response.json()[6]['parent'], 'cat1/subcat1') - def test_admin_create_category(self): + def test_admin_post_category(self): response = client.post('/admin/categories/', self.f['admin'], {'name': 'cat4'}) self.assertEqual(response.status_code, 201) self.assertEqual(response.json()['name'], 'cat4') @@ -128,6 +131,40 @@ class CategoryApiTestCase(UserTestMixin, CategoryTestMixin, ToolshedTestCase): self.assertEqual(response.json()['parent'], None) self.assertEqual(response.json()['origin'], 'api') + def test_admin_post_category_duplicate(self): + response = client.post('/admin/categories/', self.f['admin'], {'name': 'cat3'}) + self.assertEqual(response.status_code, 400) + + def test_admin_post_category_invalid(self): + response = client.post('/admin/categories/', self.f['admin'], {'name': 'cat/4'}) + self.assertEqual(response.status_code, 400) + + def test_admin_post_category_parent_not_found(self): + response = client.post('/admin/categories/', self.f['admin'], {'name': 'subcat4', 'parent': 'cat4'}) + self.assertEqual(response.status_code, 400) + + def test_admin_post_category_parent_ambiguous(self): + response = client.post('/admin/categories/', self.f['admin'], {'name': 'subcat4', 'parent': 'subcat1'}) + self.assertEqual(response.status_code, 400) + + def test_admin_post_category_parent_subcategory(self): + response = client.post('/admin/categories/', self.f['admin'], {'name': 'subcat4', 'parent': 'cat1/subcat1'}) + self.assertEqual(response.status_code, 201) + self.assertEqual(response.json()['name'], 'subcat4') + self.assertEqual(response.json()['description'], None) + self.assertEqual(response.json()['parent'], 'cat1/subcat1') + self.assertEqual(response.json()['origin'], 'api') + + def test_admin_post_category_parent_subcategory_not_found(self): + response = client.post('/admin/categories/', self.f['admin'], {'name': 'subcat4', 'parent': 'cat2/subcat1'}) + self.assertEqual(response.status_code, 400) + + def test_admin_post_category_parent_subcategory_ambiguous(self): + from toolshed.models import Category + self.f['subcat111'] = Category.objects.create(name='subcat1', parent=self.f['subcat11'], origin='test') + response = client.post('/admin/categories/', self.f['admin'], {'name': 'subcat4', 'parent': 'subcat1/subcat1'}) + self.assertEqual(response.status_code, 400) + def test_admin_post_subcategory(self): response = client.post('/admin/categories/', self.f['admin'], {'name': 'subcat4', 'parent': 'cat1'}) self.assertEqual(response.status_code, 201) @@ -136,6 +173,18 @@ class CategoryApiTestCase(UserTestMixin, CategoryTestMixin, ToolshedTestCase): self.assertEqual(response.json()['parent'], 'cat1') self.assertEqual(response.json()['origin'], 'api') + def test_admin_post_subcategory_duplicate(self): + response = client.post('/admin/categories/', self.f['admin'], {'name': 'subcat2', 'parent': 'cat1'}) + self.assertEqual(response.status_code, 400) + + def test_admin_post_subcategory_distinct_duplicate(self): + response = client.post('/admin/categories/', self.f['admin'], {'name': 'subcat2', 'parent': 'cat2'}) + self.assertEqual(response.status_code, 201) + self.assertEqual(response.json()['name'], 'subcat2') + self.assertEqual(response.json()['description'], None) + self.assertEqual(response.json()['parent'], 'cat2') + self.assertEqual(response.json()['origin'], 'api') + def test_admin_put_category(self): response = client.put('/admin/categories/1/', self.f['admin'], {'name': 'cat5'}) self.assertEqual(response.status_code, 200) @@ -188,6 +237,14 @@ class TagApiTestCase(UserTestMixin, CategoryTestMixin, TagTestMixin, ToolshedTes self.assertEqual(response.json()['origin'], 'api') self.assertEqual(response.json()['category'], None) + def test_admin_create_tag_duplicate(self): + response = client.post('/admin/tags/', self.f['admin'], {'name': 'tag3'}) + self.assertEqual(response.status_code, 400) + + def test_admin_create_tag_invalid(self): + response = client.post('/admin/tags/', self.f['admin'], {'name': 'tag/4'}) + self.assertEqual(response.status_code, 400) + def test_admin_put_tag(self): response = client.put('/admin/tags/1/', self.f['admin'], {'name': 'tag5'}) self.assertEqual(response.status_code, 200) @@ -250,7 +307,13 @@ class PropertyApiTestCase(UserTestMixin, CategoryTestMixin, PropertyTestMixin, T self.assertEqual(response.json()['base2_prefix'], False) self.assertEqual(response.json()['dimensions'], 1) - # self.assertEqual(response.json()['sort_lexicographically'], False) + def test_admin_create_property_duplicate(self): + response = client.post('/admin/properties/', self.f['admin'], {'name': 'prop3', 'category': 'cat1'}) + self.assertEqual(response.status_code, 400) + + def test_admin_create_property_invalid(self): + response = client.post('/admin/properties/', self.f['admin'], {'name': 'prop/4'}) + self.assertEqual(response.status_code, 400) def test_admin_put_property(self): response = client.put('/admin/properties/1/', self.f['admin'], {'name': 'prop5'}) @@ -265,8 +328,6 @@ class PropertyApiTestCase(UserTestMixin, CategoryTestMixin, PropertyTestMixin, T self.assertEqual(response.json()['base2_prefix'], False) self.assertEqual(response.json()['dimensions'], 1) - # self.assertEqual(response.json()['sort_lexicographically'], False) - def test_admin_patch_property(self): response = client.patch('/admin/properties/1/', self.f['admin'], {'name': 'prop5'}) self.assertEqual(response.status_code, 200) diff --git a/backend/toolshed/admin.py b/backend/toolshed/admin.py index 4b3174e..aeefc0c 100644 --- a/backend/toolshed/admin.py +++ b/backend/toolshed/admin.py @@ -1,6 +1,6 @@ from django.contrib import admin -from toolshed.models import InventoryItem, Property, Tag, ItemProperty, ItemTag +from toolshed.models import InventoryItem, Property, Tag, Category class InventoryItemAdmin(admin.ModelAdmin): @@ -12,16 +12,24 @@ admin.site.register(InventoryItem, InventoryItemAdmin) class PropertyAdmin(admin.ModelAdmin): - list_display = ('name',) - search_fields = ('name',) + list_display = ('name', 'description', 'category', 'unit_symbol', 'base2_prefix', 'dimensions', 'origin') + search_fields = ('name', 'description', 'category', 'unit_symbol', 'base2_prefix', 'dimensions', 'origin') admin.site.register(Property, PropertyAdmin) class TagAdmin(admin.ModelAdmin): - list_display = ('name',) - search_fields = ('name',) + list_display = ('name', 'description', 'category', 'origin') + search_fields = ('name', 'description', 'category', 'origin') admin.site.register(Tag, TagAdmin) + + +class CategoryAdmin(admin.ModelAdmin): + list_display = ('name', 'description', 'parent', 'origin') + search_fields = ('name', 'description', 'parent', 'origin') + + +admin.site.register(Category, CategoryAdmin) diff --git a/backend/toolshed/migrations/0006_alter_tag_options_alter_category_name_and_more.py b/backend/toolshed/migrations/0006_alter_tag_options_alter_category_name_and_more.py new file mode 100644 index 0000000..603a215 --- /dev/null +++ b/backend/toolshed/migrations/0006_alter_tag_options_alter_category_name_and_more.py @@ -0,0 +1,67 @@ +# Generated by Django 4.2.2 on 2024-03-14 16:54 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('toolshed', '0005_alter_inventoryitem_availability_policy'), + ] + + operations = [ + migrations.AlterModelOptions( + name='tag', + options={'verbose_name_plural': 'tags'}, + ), + migrations.AlterField( + model_name='category', + name='name', + field=models.CharField(max_length=255), + ), + migrations.AlterField( + model_name='category', + name='parent', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='children', to='toolshed.category'), + ), + migrations.AlterField( + model_name='inventoryitem', + name='category', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='inventory_items', to='toolshed.category'), + ), + migrations.AlterField( + model_name='property', + name='category', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='properties', to='toolshed.category'), + ), + migrations.AlterField( + model_name='tag', + name='category', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='tags', to='toolshed.category'), + ), + migrations.AddConstraint( + model_name='category', + constraint=models.UniqueConstraint(condition=models.Q(('parent__isnull', False)), fields=('name', 'parent'), name='category_unique_name_parent'), + ), + migrations.AddConstraint( + model_name='category', + constraint=models.UniqueConstraint(condition=models.Q(('parent__isnull', True)), fields=('name',), name='category_unique_name_no_parent'), + ), + migrations.AddConstraint( + model_name='property', + constraint=models.UniqueConstraint(condition=models.Q(('category__isnull', False)), fields=('name', 'category'), name='property_unique_name_category'), + ), + migrations.AddConstraint( + model_name='property', + constraint=models.UniqueConstraint(condition=models.Q(('category__isnull', True)), fields=('name',), name='property_unique_name_no_category'), + ), + migrations.AddConstraint( + model_name='tag', + constraint=models.UniqueConstraint(condition=models.Q(('category__isnull', False)), fields=('name', 'category'), name='tag_unique_name_category'), + ), + migrations.AddConstraint( + model_name='tag', + constraint=models.UniqueConstraint(condition=models.Q(('category__isnull', True)), fields=('name',), name='tag_unique_name_no_category'), + ), + ] diff --git a/backend/toolshed/models.py b/backend/toolshed/models.py index de32826..2dd0713 100644 --- a/backend/toolshed/models.py +++ b/backend/toolshed/models.py @@ -8,13 +8,19 @@ from files.models import File class Category(SoftDeleteModel): - name = models.CharField(max_length=255, unique=True) + name = models.CharField(max_length=255) description = models.TextField(null=True, blank=True) - parent = models.ForeignKey('self', on_delete=models.CASCADE, null=True, blank=True, related_name='children') + parent = models.ForeignKey('self', on_delete=models.CASCADE, null=True, related_name='children') origin = models.CharField(max_length=255, null=False, blank=False) class Meta: verbose_name_plural = 'categories' + constraints = [ + models.UniqueConstraint(fields=['name', 'parent'], condition=models.Q(parent__isnull=False), + name='category_unique_name_parent'), + models.UniqueConstraint(fields=['name'], condition=models.Q(parent__isnull=True), + name='category_unique_name_no_parent') + ] def __str__(self): parent = str(self.parent) + "/" if self.parent else "" @@ -24,7 +30,7 @@ class Category(SoftDeleteModel): class Property(models.Model): name = models.CharField(max_length=255) description = models.TextField(null=True, blank=True) - category = models.ForeignKey(Category, on_delete=models.CASCADE, null=True, blank=True, related_name='properties') + category = models.ForeignKey(Category, on_delete=models.CASCADE, null=True, related_name='properties') unit_symbol = models.CharField(max_length=16, null=True, blank=True) unit_name = models.CharField(max_length=255, null=True, blank=True) unit_name_plural = models.CharField(max_length=255, null=True, blank=True) @@ -34,6 +40,12 @@ class Property(models.Model): class Meta: verbose_name_plural = 'properties' + constraints = [ + models.UniqueConstraint(fields=['name', 'category'], condition=models.Q(category__isnull=False), + name='property_unique_name_category'), + models.UniqueConstraint(fields=['name'], condition=models.Q(category__isnull=True), + name='property_unique_name_no_category') + ] def __str__(self): return self.name @@ -42,9 +54,18 @@ class Property(models.Model): class Tag(models.Model): name = models.CharField(max_length=255) description = models.TextField(null=True, blank=True) - category = models.ForeignKey(Category, on_delete=models.CASCADE, null=True, blank=True, related_name='tags') + category = models.ForeignKey(Category, on_delete=models.CASCADE, null=True, related_name='tags') origin = models.CharField(max_length=255, null=False, blank=False) + class Meta: + verbose_name_plural = 'tags' + constraints = [ + models.UniqueConstraint(fields=['name', 'category'], condition=models.Q(category__isnull=False), + name='tag_unique_name_category'), + models.UniqueConstraint(fields=['name'], condition=models.Q(category__isnull=True), + name='tag_unique_name_no_category') + ] + def __str__(self): return self.name @@ -61,8 +82,7 @@ class InventoryItem(SoftDeleteModel): published = models.BooleanField(default=False) name = models.CharField(max_length=255, null=True, blank=True) description = models.TextField(null=True, blank=True) - category = models.ForeignKey(Category, on_delete=models.CASCADE, null=True, blank=True, - related_name='inventory_items') + category = models.ForeignKey(Category, on_delete=models.CASCADE, null=True, related_name='inventory_items') availability_policy = models.CharField(max_length=20, choices=AVAILABILITY_POLICY_CHOICES, default='private') owned_quantity = models.IntegerField(default=1, validators=[MinValueValidator(0)]) owner = models.ForeignKey(ToolshedUser, on_delete=models.CASCADE, related_name='inventory_items') diff --git a/backend/toolshed/tests/fixtures.py b/backend/toolshed/tests/fixtures.py index 6b866cc..5ee9d8b 100644 --- a/backend/toolshed/tests/fixtures.py +++ b/backend/toolshed/tests/fixtures.py @@ -8,7 +8,8 @@ class CategoryTestMixin: self.f['cat3'] = Category.objects.create(name='cat3', origin='test') self.f['subcat1'] = Category.objects.create(name='subcat1', parent=self.f['cat1'], origin='test') self.f['subcat2'] = Category.objects.create(name='subcat2', parent=self.f['cat1'], origin='test') - self.f['subcat3'] = Category.objects.create(name='subcat3', parent=self.f['subcat1'], origin='test') + self.f['subcat11'] = Category.objects.create(name='subcat1', parent=self.f['subcat1'], origin='test') + self.f['subcat12'] = Category.objects.create(name='subcat2', parent=self.f['subcat1'], origin='test') class TagTestMixin: diff --git a/backend/toolshed/tests/test_api.py b/backend/toolshed/tests/test_api.py index 716b670..d77a0eb 100644 --- a/backend/toolshed/tests/test_api.py +++ b/backend/toolshed/tests/test_api.py @@ -56,7 +56,8 @@ class CombinedApiTestCase(UserTestMixin, CategoryTestMixin, TagTestMixin, Proper self.assertEqual(response.json()['availability_policies'], [['sell', 'Sell'], ['rent', 'Rent'], ['lend', 'Lend'], ['share', 'Share'], ['private', 'Private']]) self.assertEqual(response.json()['categories'], - ['cat1', 'cat2', 'cat3', 'cat1/subcat1', 'cat1/subcat2', 'cat1/subcat1/subcat3']) + ['cat1', 'cat2', 'cat3', 'cat1/subcat1', 'cat1/subcat2', 'cat1/subcat1/subcat1', + 'cat1/subcat1/subcat2']) self.assertEqual(response.json()['tags'], ['tag1', 'tag2', 'tag3']) self.assertEqual([p['name'] for p in response.json()['properties']], ['prop1', 'prop2', 'prop3']) self.assertEqual(response.json()['domains'], ['example.com']) diff --git a/backend/toolshed/tests/test_category.py b/backend/toolshed/tests/test_category.py index 114e7ad..10a552c 100644 --- a/backend/toolshed/tests/test_category.py +++ b/backend/toolshed/tests/test_category.py @@ -17,10 +17,11 @@ class CategoryTestCase(CategoryTestMixin, UserTestMixin, ToolshedTestCase): self.assertEqual(self.f['cat1'].children.last(), self.f['subcat2']) self.assertEqual(self.f['subcat1'].parent, self.f['cat1']) self.assertEqual(self.f['subcat2'].parent, self.f['cat1']) - self.assertEqual(self.f['subcat1'].children.count(), 1) + self.assertEqual(self.f['subcat1'].children.count(), 2) self.assertEqual(str(self.f['subcat1']), 'cat1/subcat1') self.assertEqual(str(self.f['subcat2']), 'cat1/subcat2') - self.assertEqual(str(self.f['subcat3']), 'cat1/subcat1/subcat3') + self.assertEqual(str(self.f['subcat11']), 'cat1/subcat1/subcat1') + self.assertEqual(str(self.f['subcat12']), 'cat1/subcat1/subcat2') class CategoryApiTestCase(CategoryTestMixin, UserTestMixin, ToolshedTestCase): @@ -33,10 +34,12 @@ class CategoryApiTestCase(CategoryTestMixin, UserTestMixin, ToolshedTestCase): def test_get_categories(self): reply = client.get('/api/categories/', self.f['local_user1']) self.assertEqual(reply.status_code, 200) - self.assertEqual(len(reply.json()), 6) + self.assertEqual(len(reply.json()), 7) self.assertEqual(reply.json()[0], 'cat1') self.assertEqual(reply.json()[1], 'cat2') self.assertEqual(reply.json()[2], 'cat3') self.assertEqual(reply.json()[3], 'cat1/subcat1') self.assertEqual(reply.json()[4], 'cat1/subcat2') - self.assertEqual(reply.json()[5], 'cat1/subcat1/subcat3') + self.assertEqual(reply.json()[5], 'cat1/subcat1/subcat1') + self.assertEqual(reply.json()[6], 'cat1/subcat1/subcat2') +