Skip to content

Commit

Permalink
Merge pull request #3310 from vkWeb/fix/tag-len
Browse files Browse the repository at this point in the history
fix: don't publish tags greater than 30 chars
  • Loading branch information
bjester authored Mar 3, 2022
2 parents b0c97d6 + 2cb48b0 commit 2d73457
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 6 deletions.
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
2 changes: 1 addition & 1 deletion 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
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 @@ -660,7 +660,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 @@ -296,6 +297,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 NodeValidationError as e:
Expand Down Expand Up @@ -651,15 +654,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 @@ -337,6 +337,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

0 comments on commit 2d73457

Please sign in to comment.