diff --git a/.gitignore b/.gitignore index d6a2fb4eb3..19bfec72a5 100644 --- a/.gitignore +++ b/.gitignore @@ -65,10 +65,12 @@ docs/_build/ # PyBuilder target/ -# virtualenv +# virtualenv, pipenv +.env venv .venv Pipfile +Pipfile.lock Thumbs.db .DS_Store diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 2d35b33ab8..2aec84d673 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -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 @@ -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)) @@ -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) diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index 2184030af6..25edd36f7f 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/contentcuration/contentcuration/tests/test_exportchannel.py @@ -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) @@ -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() diff --git a/contentcuration/contentcuration/tests/views/test_views_internal.py b/contentcuration/contentcuration/tests/views/test_views_internal.py index 8f0718443e..b00e96229a 100644 --- a/contentcuration/contentcuration/tests/views/test_views_internal.py +++ b/contentcuration/contentcuration/tests/views/test_views_internal.py @@ -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, @@ -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): """ diff --git a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py index 55872a58e4..ce05fd2aaa 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py +++ b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py @@ -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) diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py index 70232a55ac..10f22e0303 100644 --- a/contentcuration/contentcuration/utils/publish.py +++ b/contentcuration/contentcuration/utils/publish.py @@ -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() diff --git a/contentcuration/contentcuration/views/internal.py b/contentcuration/contentcuration/views/internal.py index 9b35f753bf..28ee1e18ad 100644 --- a/contentcuration/contentcuration/views/internal.py +++ b/contentcuration/contentcuration/views/internal.py @@ -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 @@ -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: @@ -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) diff --git a/contentcuration/contentcuration/viewsets/contentnode.py b/contentcuration/contentcuration/viewsets/contentnode.py index 7c96d99caa..cb9e4c9718 100644 --- a/contentcuration/contentcuration/viewsets/contentnode.py +++ b/contentcuration/contentcuration/viewsets/contentnode.py @@ -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: