Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 176 additions & 2 deletions api/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -427,13 +429,17 @@ def test_bd_create(self):
self.authenticate()
data = {
'name': 'test',
'content': '<xml></xml>'
'content': '<xml></xml>',
'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."""
Expand Down Expand Up @@ -521,3 +527,171 @@ 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='<xml></xml>',
)
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='<xml></xml>',
)
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())

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='<xml></xml>',
)
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='<xml></xml>',
)
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()
user1 = self.make_user('user1')
bd1 = BlockDiagram.objects.create(
user=self.admin,
name='test1',
content='<xml></xml>'
)
bd2 = BlockDiagram.objects.create(
user=user1,
name='test2',
content='<xml></xml>'
)
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'], '<xml></xml>')

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') + '?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')

self.assertEqual(200, response.status_code)
self.assertEqual(1, response.json()['total_pages'])
self.assertEqual(0, len(response.json()['results']))
18 changes: 18 additions & 0 deletions mission_control/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()


Expand All @@ -15,3 +17,19 @@ 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."""
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
33 changes: 32 additions & 1 deletion mission_control/filters.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -22,10 +23,40 @@ class Meta:
class BlockDiagramFilter(FilterSet):
"""Filterset for the BlockDiagram model."""

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', 'user', 'user__not']
fields = [
'admin_tags',
'name',
'owner_tags',
'tag',
'user',
'user__not',
]

@staticmethod
def filter_tags(queryset, _, value):
"""Use all tags when filtering."""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there is a derived property tags that is the combination of owner_tags and admin_tags, and that's what the query filter searches. Is that right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can't use the property since that's not available for the database query. But, it does something similar in this method:

Q(owner_tags__name__in=tags) | Q(admin_tags__name__in=tags)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see.

I like that, because it would allow me the admin to go set the "lesson 1" admin)tag on a program if the user does not set it as a owner_tag, and user searches will return it.

But, one use case I was thinking of was a section on the landing page for "Featured Programs". I imagined it being populated by querying for admin_tag="featured program". For that, we'd need to be able to query by admin_tag alone, right? Otherwise, a user could set an owner tag of "featured program" and put their program in the section on the landing page.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for that feature, we'll need to add admin_tags to the BlockDiagramFilter. I'll go ahead and make that change. I can make a filter on owner_tags as well if we think that might be a use case in the future as well

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, probably can't hurt to have every option?

tags = value.split(',')
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)
33 changes: 33 additions & 0 deletions mission_control/migrations/0017_auto_20190811_1550.py
Original file line number Diff line number Diff line change
@@ -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'),
),
]
19 changes: 19 additions & 0 deletions mission_control/migrations/0018_auto_20190811_2004.py
Original file line number Diff line number Diff line change
@@ -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),
),
]
20 changes: 20 additions & 0 deletions mission_control/models.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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."""
Expand All @@ -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, unique=True)

def __str__(self):
"""Convert the model to a human readable string."""
return self.name
Loading