diff --git a/contentcuration/contentcuration/db/models/manager.py b/contentcuration/contentcuration/db/models/manager.py index b733719156..4d833caf8f 100644 --- a/contentcuration/contentcuration/db/models/manager.py +++ b/contentcuration/contentcuration/db/models/manager.py @@ -47,6 +47,35 @@ def log_lock_time_spent(timespent): logging.debug("Spent {} seconds inside an mptt lock".format(timespent)) +# Fields that are allowed to be overridden on copies coming from a source that the user +# does not have edit rights to. +ALLOWED_OVERRIDES = { + "node_id", + "title", + "description", + "aggregator", + "provider", + "language_id", + "grade_levels", + "resource_types", + "learning_activities", + "accessibility_labels", + "categories", + "learner_needs", + "role", + "extra_fields", + "suggested_duration", +} + +EDIT_ALLOWED_OVERRIDES = ALLOWED_OVERRIDES.union({ + "license_id", + "license_description", + "extra_fields", + "copyright_holder", + "author", +}) + + class CustomContentNodeTreeManager(TreeManager.from_queryset(CustomTreeQuerySet)): # Added 7-31-2018. We can remove this once we are certain we have eliminated all cases # where root nodes are getting prepended rather than appended to the tree list. @@ -262,7 +291,10 @@ def _clone_node( copy.update(self.get_source_attributes(source)) if isinstance(mods, dict): - copy.update(mods) + allowed_keys = EDIT_ALLOWED_OVERRIDES if can_edit_source_channel else ALLOWED_OVERRIDES + for key, value in mods.items(): + if key in copy and key in allowed_keys: + copy[key] = value # There might be some legacy nodes that don't have these, so ensure they are added if ( diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index 0e2a2bf398..8eeeb064ec 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/contentcuration/contentcuration/tests/test_exportchannel.py @@ -27,6 +27,7 @@ from .testdata import create_studio_file from .testdata import node as create_node from .testdata import slideshow +from .testdata import thumbnail_bytes from contentcuration import models as cc from contentcuration.utils.publish import convert_channel_thumbnail from contentcuration.utils.publish import create_content_database @@ -39,9 +40,6 @@ pytestmark = pytest.mark.django_db -thumbnail_bytes = b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01\x08\x06\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\nIDATx\x9cc\x00\x01\x00\x00\x05\x00\x01\r\n-\xb4\x00\x00\x00\x00IEND\xaeB`\x82' # noqa E501 - - def description(): return "".join(random.sample(string.printable, 20)) diff --git a/contentcuration/contentcuration/tests/testdata.py b/contentcuration/contentcuration/tests/testdata.py index 6d91292154..3592c9df17 100644 --- a/contentcuration/contentcuration/tests/testdata.py +++ b/contentcuration/contentcuration/tests/testdata.py @@ -22,6 +22,9 @@ pytestmark = pytest.mark.django_db +thumbnail_bytes = b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01\x08\x06\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\nIDATx\x9cc\x00\x01\x00\x00\x05\x00\x01\r\n-\xb4\x00\x00\x00\x00IEND\xaeB`\x82' # noqa E501 + + def video(): """ Create a video content kind entry. diff --git a/contentcuration/contentcuration/tests/views/test_views_internal.py b/contentcuration/contentcuration/tests/views/test_views_internal.py index 0a40ab2136..58a6792884 100644 --- a/contentcuration/contentcuration/tests/views/test_views_internal.py +++ b/contentcuration/contentcuration/tests/views/test_views_internal.py @@ -8,6 +8,7 @@ from django.db import connections from django.urls import reverse_lazy +from le_utils.constants import content_kinds from le_utils.constants import format_presets from le_utils.constants.labels.accessibility_categories import ACCESSIBILITYCATEGORIESLIST from le_utils.constants.labels.learning_activities import LEARNINGACTIVITIESLIST @@ -27,8 +28,11 @@ from ..testdata import fileobj_exercise_graphie from ..testdata import fileobj_exercise_image from ..testdata import fileobj_video +from ..testdata import thumbnail_bytes from ..testdata import user from contentcuration import ricecooker_versions as rc +from contentcuration.db.models.manager import ALLOWED_OVERRIDES +from contentcuration.db.models.manager import EDIT_ALLOWED_OVERRIDES from contentcuration.models import Channel from contentcuration.models import ContentNode from contentcuration.views import internal @@ -784,3 +788,362 @@ def test_associates_extra_fields_with_root_node(self): self.fail("Channel was not created") self.assertEqual(c.chef_tree.extra_fields["modality"], "CUSTOM_NAVIGATION") + + +class ApiAddRemoteNodesToTreeTestCase(StudioTestCase): + """ + Tests for contentcuration.views.internal.api_add_nodes_to_tree function. + For adding nodes that already exist on Studio to a cheffed tree. + """ + + def _make_node_data(self): + random_data = mixer.blend(SampleContentNodeDataSchema) + fileobj = self.fileobj + return { + "source_channel_id": self.source_channel.id, + "source_node_id": self.source_video.node_id, + "source_content_id": self.source_video.content_id, + "title": random_data.title, + "language": "en", + "description": random_data.description, + "node_id": random_data.node_id, + "content_id": random_data.content_id, + "source_domain": random_data.source_domain, + "source_id": random_data.source_id, + "author": random_data.author, + "tags": ["oer", "edtech"], + "files": [ + { + "size": fileobj.file_size, + "preset": fileobj.preset_id, + "filename": fileobj.filename(), + "original_filename": fileobj.original_filename, + "language": fileobj.language, + "source_url": fileobj.source_url, + } + ], + "kind": "document", + "license": "CC BY", + "license_description": "This is a fake license", + "copyright_holder": random_data.copyright_holder, + "questions": [], + "extra_fields": "{}", + "role": "learner", + } + + def setUp(self): + super(ApiAddRemoteNodesToTreeTestCase, self).setUp() + self.source_channel = channel() + self.source_video = self.source_channel.main_tree.get_descendants().filter(kind_id=content_kinds.VIDEO).first() + + # first setup a test channel... + self.channel = channel() + self.root_node = self.channel.main_tree + + temp_file_dict = create_studio_file(thumbnail_bytes, preset=format_presets.VIDEO_THUMBNAIL, ext='jpg') + + # File used for every node + self.fileobj = temp_file_dict["db_file"] + + # Valid node + self.valid_node = self._make_node_data() + self.title = self.valid_node["title"] + + valid_metadata_labels = self._make_node_data() + valid_metadata_labels["title"] = "valid_metadata_labels" + for label, values in METADATA.items(): + valid_metadata_labels[label] = [values[0]] + + self.sample_data = { + "root_id": self.root_node.id, + "content_data": [ + self.valid_node, + valid_metadata_labels, + ], + } + + def test_404_no_permission(self): + client = APIClient() + client.force_authenticate(user()) + response = client.post( + reverse_lazy("api_add_nodes_to_tree"), self.sample_data, format="json" + ) + self.assertEqual(response.status_code, 404) + + def test_returns_200_status_code(self): + """ + Check that we return 200 if passed in a valid JSON. + """ + self.resp = self.admin_client().post( + reverse_lazy("api_add_nodes_to_tree"), data=self.sample_data, format="json" + ) + + # check that we returned 200 with that POST request + assert self.resp.status_code == 200, "Got a request error: {}".format( + self.resp.content + ) + + def test_creates_nodes(self): + """ + Test that it creates a node with the given title and parent. + """ + self.resp = self.admin_client().post( + reverse_lazy("api_add_nodes_to_tree"), data=self.sample_data, format="json" + ) + + # make sure a node with our given self.title exists, with the given parent. + assert ContentNode.get_nodes_with_title( + title=self.title, limit_to_children_of=self.root_node.id + ).exists() + + def test_associates_file_with_created_node(self): + """ + Check that the file we created beforehand is now associated + with the node we just created through add_nodes. + """ + self.resp = self.admin_client().post( + reverse_lazy("api_add_nodes_to_tree"), data=self.sample_data, format="json" + ) + + c = ContentNode.objects.get(title=self.title) + + # check that the file we associated has the same checksum + f = c.files.get(checksum=self.fileobj.checksum) + assert f + + # check that we can read the file and it's equivalent to + # our original file object + assert f.file_on_disk.read() == self.fileobj.file_on_disk.read() + + def test_metadata_properly_created(self): + self.resp = self.admin_client().post( + reverse_lazy("api_add_nodes_to_tree"), data=self.sample_data, format="json" + ) + + node = ContentNode.objects.get(title="valid_metadata_labels") + for label, values in METADATA.items(): + self.assertEqual(getattr(node, label), { + values[0]: True + }) + + def test_metadata_properly_screened_viewer(self): + self.root_node.get_descendants().delete() + new_user = user() + new_channel = channel() + new_channel.editors.add(new_user) + self.source_channel.viewers.add(new_user) + self.sample_data = { + "root_id": new_channel.main_tree.id, + "content_data": [ + self.valid_node, + ], + } + + client = APIClient() + client.force_authenticate(new_user) + client.post( + reverse_lazy("api_add_nodes_to_tree"), self.sample_data, format="json" + ) + + # Ensure that we do not allow disallowed mods to the copied files + node = ContentNode.objects.get(title=self.title) + for key, value in self.valid_node.items(): + if key not in METADATA: + if hasattr(node, key): + # These will be matching even though we don't overwrite them. + if key in ALLOWED_OVERRIDES or key in {"source_channel_id", "source_node_id"}: + self.assertEqual(getattr(node, key), value, key) + else: + self.assertNotEqual(getattr(node, key), value, key) + + def test_metadata_properly_screened_editor(self): + self.resp = self.admin_client().post( + reverse_lazy("api_add_nodes_to_tree"), data=self.sample_data, format="json" + ) + + # Ensure that we do not allow disallowed mods to the copied files + node = ContentNode.objects.get(title=self.title) + for key, value in self.valid_node.items(): + if key not in METADATA: + if hasattr(node, key): + # These will be matching even though we don't overwrite them. + if key in EDIT_ALLOWED_OVERRIDES or key in {"source_channel_id", "source_node_id"}: + self.assertEqual(getattr(node, key), value, key) + else: + self.assertNotEqual(getattr(node, key), value, key) + + 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) + + def test_invalid_metadata_label_excluded(self): + invalid_metadata_labels = self._make_node_data() + invalid_metadata_labels["title"] = "invalid_metadata_labels" + invalid_metadata_labels["categories"] = ["not a label!"] + + test_data = { + "root_id": self.root_node.id, + "content_data": [ + invalid_metadata_labels, + ], + } + + 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) + + def test_topic_excluded(self): + topic_data = self._make_node_data() + topic_data["source_node_id"] = self.root_node.node_id + topic_data["source_content_id"] = self.root_node.content_id + + test_data = { + "root_id": self.root_node.id, + "content_data": [ + topic_data, + ], + } + + 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) + + def test_null_source_channel_id(self): + null_source_channel_id = self._make_node_data() + null_source_channel_id["source_channel_id"] = None + + test_data = { + "root_id": self.root_node.id, + "content_data": [ + null_source_channel_id, + ], + } + + 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) + + def test_invalid_source_channel_id(self): + invalid_source_channel_id = self._make_node_data() + invalid_source_channel_id["source_channel_id"] = "not a channel id" + + test_data = { + "root_id": self.root_node.id, + "content_data": [ + invalid_source_channel_id, + ], + } + + 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) + + def test_null_node_id_and_content_id(self): + null_node_id_and_content_id = self._make_node_data() + null_node_id_and_content_id["source_node_id"] = None + null_node_id_and_content_id["source_content_id"] = None + + test_data = { + "root_id": self.root_node.id, + "content_data": [ + null_node_id_and_content_id, + ], + } + + 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) + + def test_invalid_node_id_and_content_id(self): + invalid_node_id_and_content_id = self._make_node_data() + invalid_node_id_and_content_id["source_node_id"] = "not valid" + invalid_node_id_and_content_id["source_content_id"] = "def not valid" + + test_data = { + "root_id": self.root_node.id, + "content_data": [ + invalid_node_id_and_content_id, + ], + } + + 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) + + def test_no_node_id(self): + no_node_id = self._make_node_data() + del no_node_id["source_node_id"] + + test_data = { + "root_id": self.root_node.id, + "content_data": [ + no_node_id, + ], + } + + response = self.admin_client().post( + reverse_lazy("api_add_nodes_to_tree"), data=test_data, format="json" + ) + + self.assertEqual(response.status_code, 200, response.content) + + def test_no_content_id(self): + no_content_id = self._make_node_data() + del no_content_id["source_content_id"] + + test_data = { + "root_id": self.root_node.id, + "content_data": [ + no_content_id, + ], + } + + response = self.admin_client().post( + reverse_lazy("api_add_nodes_to_tree"), data=test_data, format="json" + ) + + self.assertEqual(response.status_code, 200, response.content) + + def test_no_files(self): + no_files = self._make_node_data() + del no_files["files"] + + test_data = { + "root_id": self.root_node.id, + "content_data": [ + no_files, + ], + } + + response = self.admin_client().post( + reverse_lazy("api_add_nodes_to_tree"), data=test_data, format="json" + ) + + self.assertEqual(response.status_code, 200, response.content) diff --git a/contentcuration/contentcuration/utils/nodes.py b/contentcuration/contentcuration/utils/nodes.py index 2a1c2dad1a..61491305d1 100644 --- a/contentcuration/contentcuration/utils/nodes.py +++ b/contentcuration/contentcuration/utils/nodes.py @@ -83,6 +83,12 @@ def map_files_to_node(user, node, data): # noqa: C901 duration=file_data.get("duration"), ) resource_obj.file_on_disk.name = file_path + + if kind_preset and kind_preset.thumbnail: + # If this is a thumbnail, delete any other thumbnails that are + # already attached to the file. + node.files.filter(preset=kind_preset).delete() + resource_obj.save() # Handle thumbnail diff --git a/contentcuration/contentcuration/views/internal.py b/contentcuration/contentcuration/views/internal.py index acb3578ce0..4dbc51c8ef 100644 --- a/contentcuration/contentcuration/views/internal.py +++ b/contentcuration/contentcuration/views/internal.py @@ -547,6 +547,90 @@ def __init__(self, node, errors): super(IncompleteNodeError, self).__init__(self.message) +def add_tags(node, node_data): + 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: + 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.add(*tags) + + +def validate_metadata_labels(node_data): + metadata_labels = {} + + for label, valid_values in METADATA.items(): + if label in node_data: + if type(node_data[label]) is not list: + raise NodeValidationError("{} must pass a list of values".format(label)) + metadata_labels[label] = {} + for value in node_data[label]: + if value not in valid_values: + raise NodeValidationError("{} is not a valid value for {}".format(value, label)) + metadata_labels[label][value] = True + + return metadata_labels + + +def handle_remote_node(user, node_data, parent_node): + if node_data["source_channel_id"] is None: + raise NodeValidationError("source_channel_id was None") + source_channel_id = node_data["source_channel_id"] + + source_node_id = node_data.get("source_node_id") + + source_content_id = node_data.get("source_content_id") + + if not source_node_id and not source_content_id: + raise NodeValidationError("Both source_node_id and source_content_id are None") + + try: + channel = Channel.filter_view_queryset(Channel.objects.all(), user).get(id=source_channel_id) + except Channel.DoesNotExist: + raise NodeValidationError("source_channel_id does not exist") + + contentnode = None + + channel_resource_nodes = channel.main_tree.get_descendants().exclude(kind=content_kinds.TOPIC) + + if source_node_id: + try: + contentnode = channel_resource_nodes.get(node_id=source_node_id) + except ContentNode.DoesNotExist: + pass + + if contentnode is None and source_content_id: + contentnode = channel_resource_nodes.filter(content_id=source_content_id).first() + + if contentnode is None: + raise NodeValidationError( + "Unable to find node with node_id {} and/or content_id {} from channel {}".format( + source_node_id, source_content_id, source_channel_id + ) + ) + + metadata_labels = validate_metadata_labels(node_data) + + # Validate any metadata labels then overwrite the source data so that we are + # setting validated and appropriate mapped metadata. + node_data.update(metadata_labels) + + can_edit_source_channel = ContentNode.filter_edit_queryset( + ContentNode.filter_by_pk(pk=contentnode.id), user=user + ).exists() + + return contentnode.copy_to(target=parent_node, mods=node_data, can_edit_source_channel=can_edit_source_channel) + + @delay_user_storage_calculation def convert_data_to_nodes(user, content_data, parent_node): """ Parse dict and create nodes accordingly """ @@ -561,32 +645,40 @@ def convert_data_to_nodes(user, content_data, parent_node): for node_data in content_data: # Check if node id is already in the tree to avoid duplicates if node_data["node_id"] not in existing_node_ids: - # Create the node - new_node = create_node(node_data, parent_node, sort_order) + if "source_channel_id" in node_data: + new_node = handle_remote_node(user, node_data, parent_node) + + map_files_to_node(user, new_node, node_data.get("files", [])) + + add_tags(new_node, node_data) - # Create files associated with node - map_files_to_node(user, new_node, node_data["files"]) + else: + # Create the node + new_node = create_node(node_data, parent_node, sort_order) - # Create questions associated exercise nodes - create_exercises(user, new_node, node_data["questions"]) - sort_order += 1 + # Create files associated with node + map_files_to_node(user, new_node, node_data["files"]) - # Create Slideshow slides (if slideshow kind) - if node_data["kind"] == "slideshow": - extra_fields_unicode = node_data["extra_fields"] + # Create questions associated exercise nodes + create_exercises(user, new_node, node_data["questions"]) + sort_order += 1 - # Extra Fields comes as type - convert it to a dict and get slideshow_data - extra_fields_json = extra_fields_unicode.encode( - "ascii", "ignore" - ) - extra_fields = json.loads(extra_fields_json) + # Create Slideshow slides (if slideshow kind) + if node_data["kind"] == "slideshow": + extra_fields_unicode = node_data["extra_fields"] - slides = create_slides( - user, new_node, extra_fields.get("slideshow_data") - ) - map_files_to_slideshow_slide_item( - user, new_node, slides, node_data["files"] - ) + # Extra Fields comes as type - convert it to a dict and get slideshow_data + extra_fields_json = extra_fields_unicode.encode( + "ascii", "ignore" + ) + extra_fields = json.loads(extra_fields_json) + + slides = create_slides( + user, new_node, extra_fields.get("slideshow_data") + ) + map_files_to_slideshow_slide_item( + user, new_node, slides, node_data["files"] + ) # Wait until after files have been set on the node to check for node completeness # as some node kinds are counted as incomplete if they lack a default file. @@ -644,17 +736,7 @@ def create_node(node_data, parent_node, sort_order): # noqa: C901 license_description = node_data.get('license_description', "") copyright_holder = node_data.get('copyright_holder', "") - metadata_labels = {} - - for label, valid_values in METADATA.items(): - if label in node_data: - if type(node_data[label]) is not list: - raise NodeValidationError("{} must pass a list of values".format(label)) - metadata_labels[label] = {} - for value in node_data[label]: - if value not in valid_values: - raise NodeValidationError("{} is not a valid value for {}".format(value, label)) - metadata_labels[label][value] = True + metadata_labels = validate_metadata_labels(node_data) node = ContentNode.objects.create( title=title, @@ -685,21 +767,7 @@ def create_node(node_data, parent_node, sort_order): # noqa: C901 **metadata_labels ) - 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: - 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) + add_tags(node, node_data) return node