From 69850b7f162b5551fdba5281aba8b5bf83ac2266 Mon Sep 17 00:00:00 2001 From: Clifton Barnes Date: Sun, 11 Aug 2019 12:51:51 -0400 Subject: [PATCH 1/5] Add tags to block diagrams --- api/tests/test_views.py | 61 ++++++++++++++++++- mission_control/fields.py | 11 ++++ .../migrations/0017_auto_20190811_1550.py | 33 ++++++++++ mission_control/models.py | 20 ++++++ mission_control/serializers.py | 19 +++++- 5 files changed, 141 insertions(+), 3 deletions(-) create mode 100644 mission_control/migrations/0017_auto_20190811_1550.py diff --git a/api/tests/test_views.py b/api/tests/test_views.py index 12e1dac..ef5ea4a 100644 --- a/api/tests/test_views.py +++ b/api/tests/test_views.py @@ -9,7 +9,9 @@ from rest_framework.test import APIClient from oauth2_provider.models import Application -from mission_control.models import Rover, BlockDiagram +from mission_control.models import BlockDiagram +from mission_control.models import Rover +from mission_control.models import Tag class BaseAuthenticatedTestCase(TestCase): @@ -427,13 +429,17 @@ def test_bd_create(self): self.authenticate() data = { 'name': 'test', - 'content': '' + 'content': '', + 'owner_tags': ['tag1', 'tag 2'], } response = self.client.post( reverse('api:v1:blockdiagram-list'), data) self.assertEqual(201, response.status_code) self.assertEqual(BlockDiagram.objects.last().user.id, self.admin.id) self.assertEqual(BlockDiagram.objects.last().name, data['name']) + model_tags = [t.name for t in BlockDiagram.objects.last().tags.all()] + self.assertIn('tag1', model_tags) + self.assertIn('tag 2', model_tags) def test_bd_create_name_exist(self): """Test creating block diagram when name already exists.""" @@ -521,3 +527,54 @@ def test_bd_update_as_invalid_user(self): b'["You may only modify your own block diagrams"]') self.assertEqual(BlockDiagram.objects.last().user.id, user.id) self.assertEqual(BlockDiagram.objects.last().name, 'test1') + + def test_bd_update_add_tags(self): + """Test updating block diagram to add tags.""" + self.authenticate() + bd = BlockDiagram.objects.create( + user=self.admin, + name='test', + content='', + ) + self.assertEqual(0, BlockDiagram.objects.get(id=bd.id).tags.count()) + + # Add the tag + data = { + 'owner_tags': ['test'], + } + response = self.client.patch( + reverse('api:v1:blockdiagram-detail', kwargs={'pk': bd.pk}), + json.dumps(data), content_type='application/json') + self.assertEqual(200, response.status_code) + self.assertEqual(BlockDiagram.objects.last().user.id, self.admin.id) + self.assertEqual(BlockDiagram.objects.last().name, 'test') + self.assertEqual(1, BlockDiagram.objects.last().tags.count()) + + response = self.client.get( + reverse('api:v1:blockdiagram-detail', kwargs={'pk': bd.pk})) + self.assertEqual(response.status_code, 200) + self.assertIn('test', response.data['tags']) + + def test_bd_update_remove_tags(self): + """Test updating block diagram to remove tags.""" + self.authenticate() + bd = BlockDiagram.objects.create( + user=self.admin, + name='test', + content='', + ) + tag = Tag.objects.create(name='tag1') + bd.owner_tags.add(tag) + self.assertEqual(1, BlockDiagram.objects.get(id=bd.id).tags.count()) + + # Remove the tag + data = { + 'owner_tags': [], + } + response = self.client.patch( + reverse('api:v1:blockdiagram-detail', kwargs={'pk': bd.pk}), + json.dumps(data), content_type='application/json') + self.assertEqual(200, response.status_code) + self.assertEqual(BlockDiagram.objects.last().user.id, self.admin.id) + self.assertEqual(BlockDiagram.objects.last().name, 'test') + self.assertEqual(0, BlockDiagram.objects.last().tags.count()) diff --git a/mission_control/fields.py b/mission_control/fields.py index 9ca103b..9bb486b 100644 --- a/mission_control/fields.py +++ b/mission_control/fields.py @@ -2,6 +2,8 @@ from django.contrib.auth import get_user_model from rest_framework import serializers +from mission_control.models import Tag + User = get_user_model() @@ -15,3 +17,12 @@ def to_internal_value(self, data): except User.DoesNotExist: raise serializers.ValidationError( 'User with username: {} not found'.format(data)) + + +class TagStringRelatedField(serializers.StringRelatedField): + """Custom field to allow for using tag strings in related fields.""" + + def to_internal_value(self, data): + """Convert a tag string into the primary key for the tag.""" + tag, _ = Tag.objects.get_or_create(name=data) + return tag.pk diff --git a/mission_control/migrations/0017_auto_20190811_1550.py b/mission_control/migrations/0017_auto_20190811_1550.py new file mode 100644 index 0000000..a2938be --- /dev/null +++ b/mission_control/migrations/0017_auto_20190811_1550.py @@ -0,0 +1,33 @@ +# Generated by Django 2.2.3 on 2019-08-11 15:50 + +import django.contrib.postgres.fields.citext +from django.contrib.postgres.operations import CITextExtension +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('mission_control', '0016_auto_20190801_0117'), + ] + + operations = [ + CITextExtension(), + migrations.CreateModel( + name='Tag', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', django.contrib.postgres.fields.citext.CICharField(max_length=30)), + ], + ), + migrations.AddField( + model_name='blockdiagram', + name='admin_tags', + field=models.ManyToManyField(blank=True, related_name='admin_block_diagrams', to='mission_control.Tag'), + ), + migrations.AddField( + model_name='blockdiagram', + name='owner_tags', + field=models.ManyToManyField(blank=True, related_name='owner_block_diagrams', to='mission_control.Tag'), + ), + ] diff --git a/mission_control/models.py b/mission_control/models.py index 621efdb..9ec6cab 100644 --- a/mission_control/models.py +++ b/mission_control/models.py @@ -1,6 +1,7 @@ """Mission Control models.""" from django.conf import settings from django.contrib.auth import get_user_model +from django.contrib.postgres.fields import CICharField from django.contrib.postgres.fields import JSONField from django.db import models from oauth2_provider.models import Application @@ -45,6 +46,10 @@ class BlockDiagram(models.Model): user = models.ForeignKey(User, on_delete=models.CASCADE) name = models.TextField() content = models.TextField() + admin_tags = models.ManyToManyField( + 'Tag', related_name='admin_block_diagrams', blank=True) + owner_tags = models.ManyToManyField( + 'Tag', related_name='owner_block_diagrams', blank=True) class Meta: """Meta class.""" @@ -55,3 +60,18 @@ class Meta: def __str__(self): """Convert the model to a human readable string.""" return self.name + + @property + def tags(self): + """All tags for the block diagram.""" + return (self.admin_tags.all() | self.owner_tags.all()).distinct() + + +class Tag(models.Model): + """Descriptor to add to another model.""" + + name = CICharField(max_length=30) + + def __str__(self): + """Convert the model to a human readable string.""" + return self.name diff --git a/mission_control/serializers.py b/mission_control/serializers.py index cfa0373..3579169 100644 --- a/mission_control/serializers.py +++ b/mission_control/serializers.py @@ -6,6 +6,7 @@ from rest_framework import serializers from oauth2_provider.models import Application +from .fields import TagStringRelatedField from .fields import UsernameStringRelatedField from .models import Rover, BlockDiagram @@ -73,6 +74,9 @@ class Meta: class BlockDiagramSerializer(serializers.ModelSerializer): """Block diagram model serializer.""" + admin_tags = serializers.StringRelatedField(read_only=True, many=True) + owner_tags = TagStringRelatedField(required=False, many=True) + tags = serializers.SerializerMethodField() user = UserSerializer(read_only=True) class Meta: @@ -81,9 +85,16 @@ class Meta: model = BlockDiagram fields = '__all__' + @staticmethod + def get_tags(obj): + """All tags for the block diagram.""" + return [str(tag) for tag in obj.tags.all()] + def create(self, validated_data): """Check for name conflict and create unique name if necessary.""" name = validated_data['name'] + owner_tags = validated_data.pop('owner_tags', []) + match = NAME_REGEX.search(name) if match: number = int(match.group('number')) @@ -100,4 +111,10 @@ def create(self, validated_data): name = re.sub(NAME_REGEX, '({})'.format(number), name) validated_data['name'] = name - return super().create(validated_data) + + block_diagram = super().create(validated_data) + + for tag in owner_tags: + block_diagram.owner_tags.add(tag) + + return block_diagram From 055bdc4a8872f118f4ebfe459c33884d887e16c3 Mon Sep 17 00:00:00 2001 From: Clifton Barnes Date: Sun, 11 Aug 2019 15:14:28 -0400 Subject: [PATCH 2/5] Add filtering on tags --- api/tests/test_views.py | 60 ++++++++++++++++++++++++++++++++++++++ mission_control/filters.py | 14 ++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/api/tests/test_views.py b/api/tests/test_views.py index ef5ea4a..47b744f 100644 --- a/api/tests/test_views.py +++ b/api/tests/test_views.py @@ -578,3 +578,63 @@ def test_bd_update_remove_tags(self): self.assertEqual(BlockDiagram.objects.last().user.id, self.admin.id) self.assertEqual(BlockDiagram.objects.last().name, 'test') self.assertEqual(0, BlockDiagram.objects.last().tags.count()) + + def test_bd_tag_filter(self): + """Test the block diagram API view filters on tags correctly.""" + self.authenticate() + user1 = self.make_user('user1') + bd1 = BlockDiagram.objects.create( + user=self.admin, + name='test1', + content='' + ) + bd2 = BlockDiagram.objects.create( + user=user1, + name='test2', + content='' + ) + tag1 = Tag.objects.create(name='tag1') + tag2 = Tag.objects.create(name='tag2') + tag3 = Tag.objects.create(name='tag3') + tag4 = Tag.objects.create(name='tag4') + bd1.owner_tags.set([tag1, tag2]) + bd1.admin_tags.add(tag3) + bd2.owner_tags.add(tag4) + bd2.admin_tags.add(tag3) + + response = self.get( + reverse('api:v1:blockdiagram-list') + '?tag={},{}'.format( + tag1.name, tag2.name)) + + self.assertEqual(200, response.status_code) + self.assertEqual(1, response.json()['total_pages']) + self.assertEqual(1, len(response.json()['results'])) + self.assertEqual(response.json()['results'][0]['id'], bd1.id) + self.assertDictEqual(response.json()['results'][0]['user'], { + 'username': self.admin.username, + }) + self.assertEqual(response.json()['results'][0]['name'], 'test1') + self.assertEqual( + response.json()['results'][0]['content'], '') + + response = self.get( + reverse('api:v1:blockdiagram-list') + '?tag=' + tag3.name) + + self.assertEqual(200, response.status_code) + self.assertEqual(1, response.json()['total_pages']) + self.assertEqual(2, len(response.json()['results'])) + + response = self.get( + reverse('api:v1:blockdiagram-list') + '?tag={},{}'.format( + tag2.name, tag3.name)) + + self.assertEqual(200, response.status_code) + self.assertEqual(1, response.json()['total_pages']) + self.assertEqual(2, len(response.json()['results'])) + + response = self.get( + reverse('api:v1:blockdiagram-list') + '?tag=' + 'nothing') + + self.assertEqual(200, response.status_code) + self.assertEqual(1, response.json()['total_pages']) + self.assertEqual(0, len(response.json()['results'])) diff --git a/mission_control/filters.py b/mission_control/filters.py index fbff0a8..9626060 100644 --- a/mission_control/filters.py +++ b/mission_control/filters.py @@ -1,4 +1,5 @@ """Mission Control filters.""" +from django.db.models import Q from django_filters.rest_framework import CharFilter from django_filters.rest_framework import FilterSet from django_filters.rest_framework import NumberFilter @@ -22,10 +23,21 @@ class Meta: class BlockDiagramFilter(FilterSet): """Filterset for the BlockDiagram model.""" + tag = CharFilter( + method='filter_tags', + ) user__not = NumberFilter(field_name='user', exclude=True) class Meta: """Meta class.""" model = BlockDiagram - fields = ['name', 'user', 'user__not'] + fields = ['name', 'tag', 'user', 'user__not'] + + @staticmethod + def filter_tags(queryset, _, value): + """Use all tags when filtering.""" + tags = value.split(',') + return queryset.filter( + Q(owner_tags__name__in=tags) | Q(admin_tags__name__in=tags) + ).distinct() From f1d3e460a2f9c759695d86cd0b458642d39dde01 Mon Sep 17 00:00:00 2001 From: Clifton Barnes Date: Sun, 11 Aug 2019 15:55:39 -0400 Subject: [PATCH 3/5] Restrict tag name length --- api/tests/test_views.py | 40 +++++++++++++++++++++++++++++++++++++++ mission_control/fields.py | 7 +++++++ 2 files changed, 47 insertions(+) diff --git a/api/tests/test_views.py b/api/tests/test_views.py index 47b744f..07b19d7 100644 --- a/api/tests/test_views.py +++ b/api/tests/test_views.py @@ -579,6 +579,46 @@ def test_bd_update_remove_tags(self): self.assertEqual(BlockDiagram.objects.last().name, 'test') self.assertEqual(0, BlockDiagram.objects.last().tags.count()) + def test_bd_update_add_tag_too_long(self): + """Test updating block diagram to add tag that is too long.""" + self.authenticate() + bd = BlockDiagram.objects.create( + user=self.admin, + name='test', + content='', + ) + self.assertEqual(0, BlockDiagram.objects.get(id=bd.id).tags.count()) + + # Add the tag + data = { + 'owner_tags': ['a'*100], + } + response = self.client.patch( + reverse('api:v1:blockdiagram-detail', kwargs={'pk': bd.pk}), + json.dumps(data), content_type='application/json') + self.assertEqual(400, response.status_code) + self.assertEqual(0, BlockDiagram.objects.get(id=bd.id).tags.count()) + + def test_bd_update_add_tag_too_short(self): + """Test updating block diagram to add tag that is too short.""" + self.authenticate() + bd = BlockDiagram.objects.create( + user=self.admin, + name='test', + content='', + ) + self.assertEqual(0, BlockDiagram.objects.get(id=bd.id).tags.count()) + + # Add the tag + data = { + 'owner_tags': ['a'], + } + response = self.client.patch( + reverse('api:v1:blockdiagram-detail', kwargs={'pk': bd.pk}), + json.dumps(data), content_type='application/json') + self.assertEqual(400, response.status_code) + self.assertEqual(0, BlockDiagram.objects.get(id=bd.id).tags.count()) + def test_bd_tag_filter(self): """Test the block diagram API view filters on tags correctly.""" self.authenticate() diff --git a/mission_control/fields.py b/mission_control/fields.py index 9bb486b..8f785f1 100644 --- a/mission_control/fields.py +++ b/mission_control/fields.py @@ -24,5 +24,12 @@ class TagStringRelatedField(serializers.StringRelatedField): def to_internal_value(self, data): """Convert a tag string into the primary key for the tag.""" + if len(data) < 3: + raise serializers.ValidationError( + 'Tags must be at least 3 characters') + elif len(data) > 30: + raise serializers.ValidationError( + 'Tags must be at most 30 characters') + tag, _ = Tag.objects.get_or_create(name=data) return tag.pk From 2588025f9faba945a55c231a16af1640ed73748c Mon Sep 17 00:00:00 2001 From: Clifton Barnes Date: Sun, 11 Aug 2019 16:04:51 -0400 Subject: [PATCH 4/5] Make tag names unique in the database --- .../migrations/0018_auto_20190811_2004.py | 19 +++++++++++++++++++ mission_control/models.py | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 mission_control/migrations/0018_auto_20190811_2004.py diff --git a/mission_control/migrations/0018_auto_20190811_2004.py b/mission_control/migrations/0018_auto_20190811_2004.py new file mode 100644 index 0000000..8b2ab96 --- /dev/null +++ b/mission_control/migrations/0018_auto_20190811_2004.py @@ -0,0 +1,19 @@ +# Generated by Django 2.2.4 on 2019-08-11 20:04 + +import django.contrib.postgres.fields.citext +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('mission_control', '0017_auto_20190811_1550'), + ] + + operations = [ + migrations.AlterField( + model_name='tag', + name='name', + field=django.contrib.postgres.fields.citext.CICharField(max_length=30, unique=True), + ), + ] diff --git a/mission_control/models.py b/mission_control/models.py index 9ec6cab..87d6a9a 100644 --- a/mission_control/models.py +++ b/mission_control/models.py @@ -70,7 +70,7 @@ def tags(self): class Tag(models.Model): """Descriptor to add to another model.""" - name = CICharField(max_length=30) + name = CICharField(max_length=30, unique=True) def __str__(self): """Convert the model to a human readable string.""" From ba9db61c722eb17c686ad6c19d8dfda1011ce2b0 Mon Sep 17 00:00:00 2001 From: Clifton Barnes Date: Tue, 13 Aug 2019 20:03:42 -0400 Subject: [PATCH 5/5] Add filtering by 'admin_tags' and 'owner_tags' --- api/tests/test_views.py | 17 +++++++++++++++++ mission_control/filters.py | 27 +++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/api/tests/test_views.py b/api/tests/test_views.py index 07b19d7..3358610 100644 --- a/api/tests/test_views.py +++ b/api/tests/test_views.py @@ -672,6 +672,23 @@ def test_bd_tag_filter(self): self.assertEqual(1, response.json()['total_pages']) self.assertEqual(2, len(response.json()['results'])) + response = self.get( + reverse('api:v1:blockdiagram-list') + '?owner_tags={},{}'.format( + tag4.name, tag3.name)) + + self.assertEqual(200, response.status_code) + self.assertEqual(1, response.json()['total_pages']) + self.assertEqual(1, len(response.json()['results'])) + self.assertEqual(response.json()['results'][0]['name'], 'test2') + + response = self.get( + reverse('api:v1:blockdiagram-list') + '?admin_tags={}'.format( + tag3.name)) + + self.assertEqual(200, response.status_code) + self.assertEqual(1, response.json()['total_pages']) + self.assertEqual(2, len(response.json()['results'])) + response = self.get( reverse('api:v1:blockdiagram-list') + '?tag=' + 'nothing') diff --git a/mission_control/filters.py b/mission_control/filters.py index 9626060..c4a965a 100644 --- a/mission_control/filters.py +++ b/mission_control/filters.py @@ -23,16 +23,23 @@ class Meta: class BlockDiagramFilter(FilterSet): """Filterset for the BlockDiagram model.""" - tag = CharFilter( - method='filter_tags', - ) + admin_tags = CharFilter(method='filter_admin_tags') + owner_tags = CharFilter(method='filter_owner_tags') + tag = CharFilter(method='filter_tags') user__not = NumberFilter(field_name='user', exclude=True) class Meta: """Meta class.""" model = BlockDiagram - fields = ['name', 'tag', 'user', 'user__not'] + fields = [ + 'admin_tags', + 'name', + 'owner_tags', + 'tag', + 'user', + 'user__not', + ] @staticmethod def filter_tags(queryset, _, value): @@ -41,3 +48,15 @@ def filter_tags(queryset, _, value): return queryset.filter( Q(owner_tags__name__in=tags) | Q(admin_tags__name__in=tags) ).distinct() + + @staticmethod + def filter_owner_tags(queryset, _, value): + """Filter on list of owner tags.""" + tags = value.split(',') + return queryset.filter(owner_tags__name__in=tags) + + @staticmethod + def filter_admin_tags(queryset, _, value): + """Filter on list of admin tags.""" + tags = value.split(',') + return queryset.filter(admin_tags__name__in=tags)