Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: don't publish tags greater than 30 chars #3310

Merged
merged 5 commits into from
Mar 3, 2022
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
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,12 @@ docs/_build/
# PyBuilder
target/

# virtualenv
# virtualenv, pipenv
.env
venv
.venv
Pipfile
Pipfile.lock

Thumbs.db
.DS_Store
Expand Down
6 changes: 3 additions & 3 deletions contentcuration/contentcuration/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ def __init__(self, model, user_id, **kwargs):
queryset = model.objects.filter(user_id=user_id)\
.annotate(
tree_id=Unnest(ArrayRemove(Array(*self.tree_id_fields), None), output_field=models.IntegerField())
)
)
super(PermissionCTE, self).__init__(queryset=queryset.values("user_id", "channel_id", "tree_id"), **kwargs)

@classmethod
Expand Down Expand Up @@ -2095,7 +2095,7 @@ def clean(self, *args, **kwargs):
raise IntegrityError('Cannot self reference as prerequisite.')
# immediate cyclic exception
if PrerequisiteContentRelationship.objects.using(self._state.db) \
.filter(target_node=self.prerequisite, prerequisite=self.target_node):
.filter(target_node=self.prerequisite, prerequisite=self.target_node):
raise IntegrityError(
'Note: Prerequisite relationship is directional! %s and %s cannot be prerequisite of each other!'
% (self.target_node, self.prerequisite))
Expand Down Expand Up @@ -2128,7 +2128,7 @@ def save(self, *args, **kwargs):
raise IntegrityError('Cannot self reference as related.')
# handle immediate cyclic
if RelatedContentRelationship.objects.using(self._state.db) \
.filter(contentnode_1=self.contentnode_2, contentnode_2=self.contentnode_1):
.filter(contentnode_1=self.contentnode_2, contentnode_2=self.contentnode_1):
return # silently cancel the save
super(RelatedContentRelationship, self).save(*args, **kwargs)

Expand Down
15 changes: 15 additions & 0 deletions contentcuration/contentcuration/tests/test_exportchannel.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@ def setUp(self):
new_video.parent = new_node
new_video.save()

# Add a node with tags greater than 30 chars to ensure they get excluded.
new_video = create_node({'kind_id': 'video', 'tags': [{'tag_name': 'kolbasdasdasrissadasdwzxcztudio'}, {'tag_name': 'kolbasdasdasrissadasdwzxcztudi'},
{'tag_name': 'kolbasdasdasrissadasdwzxc'}], 'title': 'kolibri tag test', 'children': []})
new_video.complete = True
new_video.parent = self.content_channel.main_tree
new_video.save()

set_channel_icon_encoding(self.content_channel)
self.tempdb = create_content_database(self.content_channel, True, None, True)

Expand Down Expand Up @@ -120,6 +127,14 @@ def test_contentnode_incomplete_not_published(self):
for node in incomplete_nodes:
assert kolibri_nodes.filter(pk=node.node_id).count() == 0

def test_tags_greater_than_30_excluded(self):
tag_node = kolibri_models.ContentNode.objects.filter(title='kolibri tag test').first()
published_tags = tag_node.tags.all()

assert published_tags.count() == 2
for t in published_tags:
assert len(t.tag_name) <= 30

def test_contentnode_channel_id_data(self):
channel = kolibri_models.ChannelMetadata.objects.first()
nodes = kolibri_models.ContentNode.objects.all()
Expand Down
20 changes: 20 additions & 0 deletions contentcuration/contentcuration/tests/views/test_views_internal.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ def _make_node_data(self):
"source_domain": random_data.source_domain,
"source_id": random_data.source_id,
"author": random_data.author,
"tags": ["oer", "edtech"],
"files": [
{
"size": fileobj.file_size,
Expand Down Expand Up @@ -176,6 +177,25 @@ def test_invalid_nodes_are_not_complete(self):
self.assertFalse(node_2.complete)
self.assertFalse(node_3.complete)

def test_tag_greater_than_30_chars_excluded(self):
# Node with tag greater than 30 characters
invalid_tag_length = self._make_node_data()
invalid_tag_length["title"] = "invalid_tag_length"
invalid_tag_length["tags"] = ["kolibri studio, kolibri studio!"]

test_data = {
"root_id": self.root_node.id,
"content_data": [
invalid_tag_length,
],
}

response = self.admin_client().post(
reverse_lazy("api_add_nodes_to_tree"), data=test_data, format="json"
)

self.assertEqual(response.status_code, 400, response.content)


class ApiAddExerciseNodesToTreeTestCase(StudioTestCase):
"""
Expand Down
16 changes: 16 additions & 0 deletions contentcuration/contentcuration/tests/viewsets/test_contentnode.py
Original file line number Diff line number Diff line change
Expand Up @@ -1367,6 +1367,22 @@ def test_create_contentnode_tag(self):
.exists()
)

def test_contentnode_tag_greater_than_30_chars(self):
user = testdata.user()
tag = "kolibri studio, kolibri studio!"

self.client.force_authenticate(user=user)
contentnode = self.contentnode_metadata
contentnode["tags"] = {
tag: True,
}

response = self.client.post(
reverse("contentnode-list"), contentnode, format="json",
)

self.assertEqual(response.status_code, 400, response.content)

def test_update_contentnode(self):
user = testdata.user()
contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)
Expand Down
3 changes: 2 additions & 1 deletion contentcuration/contentcuration/utils/publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,8 @@ def map_tags_to_node(kolibrinode, ccnode):

for tag in ccnode.tags.all():
t, _new = kolibrimodels.ContentTag.objects.get_or_create(pk=tag.pk, tag_name=tag.tag_name)
tags_to_add.append(t)
if len(t.tag_name) <= 30:
tags_to_add.append(t)

kolibrinode.tags.set(tags_to_add)
kolibrinode.save()
Expand Down
13 changes: 10 additions & 3 deletions contentcuration/contentcuration/views/internal.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from django.core.exceptions import ObjectDoesNotExist
from django.core.exceptions import PermissionDenied
from django.core.exceptions import SuspiciousOperation
from django.core.exceptions import ValidationError
from django.db import transaction
from django.http import HttpResponseBadRequest
from django.http import HttpResponseForbidden
Expand Down Expand Up @@ -291,6 +292,8 @@ def api_add_nodes_to_tree(request):
})
except ContentNode.DoesNotExist:
return HttpResponseNotFound("No content matching: {}".format(parent_id))
except ValidationError as e:
return HttpResponseBadRequest(content=str(e))
except KeyError:
return HttpResponseBadRequest("Required attribute missing from data: {}".format(data))
except Exception as e:
Expand Down Expand Up @@ -637,15 +640,19 @@ def create_node(node_data, parent_node, sort_order): # noqa: C901
role_visibility=node_data.get('role') or roles.LEARNER,
complete=is_complete,
)

tags = []
channel = node.get_channel()
if "tags" in node_data:
tag_data = node_data["tags"]
if tag_data is not None:
for tag in tag_data:
tags.append(
ContentTag.objects.get_or_create(tag_name=tag, channel=channel)[0]
)
if len(tag) > 30:
raise ValidationError("tag is greater than 30 characters")
else:
tags.append(
ContentTag.objects.get_or_create(tag_name=tag, channel=channel)[0]
)

if len(tags) > 0:
node.tags.set(tags)
Expand Down
8 changes: 8 additions & 0 deletions contentcuration/contentcuration/viewsets/contentnode.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,14 @@ class Meta:
list_serializer_class = ContentNodeListSerializer
nested_writes = True

def validate(self, data):
tags = data.get("tags")
if tags is not None:
for tag in tags:
if len(tag) > 30:
raise ValidationError("tag is greater than 30 characters")
return data

def create(self, validated_data):
# Creating a new node, by default put it in the orphanage on initial creation.
if "parent" not in validated_data:
Expand Down