From 034d6322d0183e0450e6f9f1d7668b114099bb01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Tue, 14 Nov 2023 16:12:18 -0300 Subject: [PATCH] feat: add import taxonomy endpoint to content_tagging (#33663) --- .../rest_api/v1/tests/test_views.py | 446 ++++++++++++++++++ .../content_tagging/rest_api/v1/views.py | 33 +- .../core/djangoapps/content_tagging/rules.py | 1 - requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 8 files changed, 482 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index ee6b3330a803..f3b82b831f94 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -5,10 +5,12 @@ from __future__ import annotations from urllib.parse import parse_qs, urlparse +import json import abc import ddt from django.contrib.auth import get_user_model +from django.core.files.uploadedfile import SimpleUploadedFile from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator from openedx_tagging.core.tagging.models import Tag, Taxonomy from openedx_tagging.core.tagging.models.system_defined import SystemDefinedTaxonomy @@ -43,6 +45,9 @@ TAXONOMY_ORG_UPDATE_ORG_URL = "/api/content_tagging/v1/taxonomies/{pk}/orgs/" OBJECT_TAG_UPDATE_URL = "/api/content_tagging/v1/object_tags/{object_id}/?taxonomy={taxonomy_id}" TAXONOMY_TEMPLATE_URL = "/api/content_tagging/v1/taxonomies/import/{filename}" +TAXONOMY_CREATE_IMPORT_URL = "/api/content_tagging/v1/taxonomies/import/" +TAXONOMY_TAGS_IMPORT_URL = "/api/content_tagging/v1/taxonomies/{pk}/tags/import/" +TAXONOMY_TAGS_URL = "/api/content_tagging/v1/taxonomies/{pk}/tags/" def check_taxonomy( @@ -1527,3 +1532,444 @@ def test_download_method_not_allowed(self): url = TAXONOMY_TEMPLATE_URL.format(filename="template.txt") response = self.client.post(url) assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED + + +class ImportTaxonomyMixin(TestTaxonomyObjectsMixin): + """ + Mixin to test importing taxonomies. + """ + def _get_file(self, tags: list, file_format: str) -> SimpleUploadedFile: + """ + Returns a file for the given format. + """ + if file_format == "csv": + csv_data = "id,value" + for tag in tags: + csv_data += f"\n{tag['id']},{tag['value']}" + return SimpleUploadedFile("taxonomy.csv", csv_data.encode(), content_type="text/csv") + else: # json + json_data = {"tags": tags} + return SimpleUploadedFile("taxonomy.json", json.dumps(json_data).encode(), content_type="application/json") + + +@skip_unless_cms +@ddt.ddt +class TestCreateImportView(ImportTaxonomyMixin, APITestCase): + """ + Tests the create/import taxonomy action. + """ + @ddt.data( + "csv", + "json", + ) + def test_import_global_admin(self, file_format: str) -> None: + """ + Tests importing a valid taxonomy file with a global admin. + """ + url = TAXONOMY_CREATE_IMPORT_URL + new_tags = [ + {"id": "tag_1", "value": "Tag 1"}, + {"id": "tag_2", "value": "Tag 2"}, + {"id": "tag_3", "value": "Tag 3"}, + {"id": "tag_4", "value": "Tag 4"}, + ] + file = self._get_file(new_tags, file_format) + + self.client.force_authenticate(user=self.staff) + response = self.client.post( + url, + { + "taxonomy_name": "Imported Taxonomy name", + "taxonomy_description": "Imported Taxonomy description", + "file": file, + }, + format="multipart" + ) + assert response.status_code == status.HTTP_201_CREATED + + # Check if the taxonomy was created + taxonomy = response.data + assert taxonomy["name"] == "Imported Taxonomy name" + assert taxonomy["description"] == "Imported Taxonomy description" + + # Check if the tags were created + url = TAXONOMY_TAGS_URL.format(pk=taxonomy["id"]) + response = self.client.get(url) + tags = response.data["results"] + assert len(tags) == len(new_tags) + for i, tag in enumerate(tags): + assert tag["value"] == new_tags[i]["value"] + + # Check if the taxonomy was no association with orgs + assert len(taxonomy["orgs"]) == 0 + + @ddt.data( + "csv", + "json", + ) + def test_import_orgA_admin(self, file_format: str) -> None: + """ + Tests importing a valid taxonomy file with a orgA admin. + """ + url = TAXONOMY_CREATE_IMPORT_URL + new_tags = [ + {"id": "tag_1", "value": "Tag 1"}, + {"id": "tag_2", "value": "Tag 2"}, + {"id": "tag_3", "value": "Tag 3"}, + {"id": "tag_4", "value": "Tag 4"}, + ] + file = self._get_file(new_tags, file_format) + + self.client.force_authenticate(user=self.staffA) + response = self.client.post( + url, + { + "taxonomy_name": "Imported Taxonomy name", + "taxonomy_description": "Imported Taxonomy description", + "file": file, + }, + format="multipart" + ) + assert response.status_code == status.HTTP_201_CREATED + + # Check if the taxonomy was created + taxonomy = response.data + assert taxonomy["name"] == "Imported Taxonomy name" + assert taxonomy["description"] == "Imported Taxonomy description" + + # Check if the tags were created + url = TAXONOMY_TAGS_URL.format(pk=taxonomy["id"]) + response = self.client.get(url) + tags = response.data["results"] + assert len(tags) == len(new_tags) + for i, tag in enumerate(tags): + assert tag["value"] == new_tags[i]["value"] + + # Check if the taxonomy was associated with the orgA + assert len(taxonomy["orgs"]) == 1 + assert taxonomy["orgs"][0] == self.orgA.short_name + + def test_import_no_file(self) -> None: + """ + Tests importing a taxonomy without a file. + """ + url = TAXONOMY_CREATE_IMPORT_URL + self.client.force_authenticate(user=self.staff) + response = self.client.post( + url, + { + "taxonomy_name": "Imported Taxonomy name", + "taxonomy_description": "Imported Taxonomy description", + }, + format="multipart" + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data["file"][0] == "No file was submitted." + + # Check if the taxonomy was not created + assert not Taxonomy.objects.filter(name="Imported Taxonomy name").exists() + + @ddt.data( + "csv", + "json", + ) + def test_import_no_name(self, file_format) -> None: + """ + Tests importing a taxonomy without specifing a name. + """ + url = TAXONOMY_CREATE_IMPORT_URL + file = SimpleUploadedFile(f"taxonomy.{file_format}", b"invalid file content") + self.client.force_authenticate(user=self.staff) + response = self.client.post( + url, + { + "taxonomy_description": "Imported Taxonomy description", + "file": file, + }, + format="multipart" + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data["taxonomy_name"][0] == "This field is required." + + # Check if the taxonomy was not created + assert not Taxonomy.objects.filter(name="Imported Taxonomy name").exists() + + def test_import_invalid_format(self) -> None: + """ + Tests importing a taxonomy with an invalid file format. + """ + url = TAXONOMY_CREATE_IMPORT_URL + file = SimpleUploadedFile("taxonomy.invalid", b"invalid file content") + self.client.force_authenticate(user=self.staff) + response = self.client.post( + url, + { + "taxonomy_name": "Imported Taxonomy name", + "taxonomy_description": "Imported Taxonomy description", + "file": file, + }, + format="multipart" + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data["file"][0] == "File type not supported: invalid" + + # Check if the taxonomy was not created + assert not Taxonomy.objects.filter(name="Imported Taxonomy name").exists() + + @ddt.data( + "csv", + "json", + ) + def test_import_invalid_content(self, file_format) -> None: + """ + Tests importing a taxonomy with an invalid file content. + """ + url = TAXONOMY_CREATE_IMPORT_URL + file = SimpleUploadedFile(f"taxonomy.{file_format}", b"invalid file content") + self.client.force_authenticate(user=self.staff) + response = self.client.post( + url, + { + "taxonomy_name": "Imported Taxonomy name", + "taxonomy_description": "Imported Taxonomy description", + "file": file, + }, + format="multipart" + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert f"Invalid '.{file_format}' format:" in response.data + + # Check if the taxonomy was not created + assert not Taxonomy.objects.filter(name="Imported Taxonomy name").exists() + + def test_import_no_perm(self) -> None: + """ + Tests importing a taxonomy using a user without permission. + """ + url = TAXONOMY_CREATE_IMPORT_URL + new_tags = [ + {"id": "tag_1", "value": "Tag 1"}, + {"id": "tag_2", "value": "Tag 2"}, + {"id": "tag_3", "value": "Tag 3"}, + {"id": "tag_4", "value": "Tag 4"}, + ] + file = self._get_file(new_tags, "json") + + self.client.force_authenticate(user=self.user) + response = self.client.post( + url, + { + "taxonomy_name": "Imported Taxonomy name", + "taxonomy_description": "Imported Taxonomy description", + "file": file, + }, + format="multipart" + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + # Check if the taxonomy was not created + assert not Taxonomy.objects.filter(name="Imported Taxonomy name").exists() + + +@skip_unless_cms +@ddt.ddt +class TestImportTagsView(ImportTaxonomyMixin, APITestCase): + """ + Tests the taxonomy import tags action. + """ + def setUp(self): + ImportTaxonomyMixin.setUp(self) + + self.taxonomy = Taxonomy.objects.create( + name="Test import taxonomy", + ) + tag_1 = Tag.objects.create( + taxonomy=self.taxonomy, + external_id="old_tag_1", + value="Old tag 1", + ) + tag_2 = Tag.objects.create( + taxonomy=self.taxonomy, + external_id="old_tag_2", + value="Old tag 2", + ) + self.old_tags = [tag_1, tag_2] + + @ddt.data( + "csv", + "json", + ) + def test_import(self, file_format: str) -> None: + """ + Tests importing a valid taxonomy file. + """ + url = TAXONOMY_TAGS_IMPORT_URL.format(pk=self.taxonomy.id) + new_tags = [ + {"id": "tag_1", "value": "Tag 1"}, + {"id": "tag_2", "value": "Tag 2"}, + {"id": "tag_3", "value": "Tag 3"}, + {"id": "tag_4", "value": "Tag 4"}, + ] + file = self._get_file(new_tags, file_format) + + self.client.force_authenticate(user=self.staff) + response = self.client.put( + url, + {"file": file}, + format="multipart" + ) + assert response.status_code == status.HTTP_200_OK + + # Check if the tags were created + url = TAXONOMY_TAGS_URL.format(pk=self.taxonomy.id) + response = self.client.get(url) + tags = response.data["results"] + all_tags = [{"value": tag.value} for tag in self.old_tags] + new_tags + assert len(tags) == len(all_tags) + for i, tag in enumerate(tags): + assert tag["value"] == all_tags[i]["value"] + + def test_import_no_file(self) -> None: + """ + Tests importing a taxonomy without a file. + """ + url = TAXONOMY_TAGS_IMPORT_URL.format(pk=self.taxonomy.id) + self.client.force_authenticate(user=self.staff) + response = self.client.put( + url, + {}, + format="multipart" + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data["file"][0] == "No file was submitted." + + # Check if the taxonomy was not changed + url = TAXONOMY_TAGS_URL.format(pk=self.taxonomy.id) + response = self.client.get(url) + tags = response.data["results"] + assert len(tags) == len(self.old_tags) + for i, tag in enumerate(tags): + assert tag["value"] == self.old_tags[i].value + + def test_import_invalid_format(self) -> None: + """ + Tests importing a taxonomy with an invalid file format. + """ + url = TAXONOMY_TAGS_IMPORT_URL.format(pk=self.taxonomy.id) + file = SimpleUploadedFile("taxonomy.invalid", b"invalid file content") + self.client.force_authenticate(user=self.staff) + response = self.client.put( + url, + {"file": file}, + format="multipart" + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data["file"][0] == "File type not supported: invalid" + + # Check if the taxonomy was not changed + url = TAXONOMY_TAGS_URL.format(pk=self.taxonomy.id) + response = self.client.get(url) + tags = response.data["results"] + assert len(tags) == len(self.old_tags) + for i, tag in enumerate(tags): + assert tag["value"] == self.old_tags[i].value + + @ddt.data( + "csv", + "json", + ) + def test_import_invalid_content(self, file_format) -> None: + """ + Tests importing a taxonomy with an invalid file content. + """ + url = TAXONOMY_TAGS_IMPORT_URL.format(pk=self.taxonomy.id) + file = SimpleUploadedFile(f"taxonomy.{file_format}", b"invalid file content") + self.client.force_authenticate(user=self.staff) + response = self.client.put( + url, + { + "taxonomy_name": "Imported Taxonomy name", + "taxonomy_description": "Imported Taxonomy description", + "file": file, + }, + format="multipart" + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert f"Invalid '.{file_format}' format:" in response.data + + # Check if the taxonomy was not changed + url = TAXONOMY_TAGS_URL.format(pk=self.taxonomy.id) + response = self.client.get(url) + tags = response.data["results"] + assert len(tags) == len(self.old_tags) + for i, tag in enumerate(tags): + assert tag["value"] == self.old_tags[i].value + + @ddt.data( + "csv", + "json", + ) + def test_import_free_text(self, file_format) -> None: + """ + Tests importing a taxonomy with an invalid file content. + """ + self.taxonomy.allow_free_text = True + self.taxonomy.save() + url = TAXONOMY_TAGS_IMPORT_URL.format(pk=self.taxonomy.id) + new_tags = [ + {"id": "tag_1", "value": "Tag 1"}, + {"id": "tag_2", "value": "Tag 2"}, + {"id": "tag_3", "value": "Tag 3"}, + {"id": "tag_4", "value": "Tag 4"}, + ] + file = self._get_file(new_tags, file_format) + + self.client.force_authenticate(user=self.staff) + response = self.client.put( + url, + {"file": file}, + format="multipart" + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data == f"Invalid taxonomy ({self.taxonomy.id}): You cannot import a free-form taxonomy." + + # Check if the taxonomy has no tags, since it is free text + url = TAXONOMY_TAGS_URL.format(pk=self.taxonomy.id) + response = self.client.get(url) + tags = response.data["results"] + assert len(tags) == 0 + + def test_import_no_perm(self) -> None: + """ + Tests importing a taxonomy using a user without permission. + """ + url = TAXONOMY_TAGS_IMPORT_URL.format(pk=self.taxonomy.id) + new_tags = [ + {"id": "tag_1", "value": "Tag 1"}, + {"id": "tag_2", "value": "Tag 2"}, + {"id": "tag_3", "value": "Tag 3"}, + {"id": "tag_4", "value": "Tag 4"}, + ] + file = self._get_file(new_tags, "json") + + self.client.force_authenticate(user=self.user) + response = self.client.put( + url, + { + "taxonomy_name": "Imported Taxonomy name", + "taxonomy_description": "Imported Taxonomy description", + "file": file, + }, + format="multipart" + ) + assert response.status_code == status.HTTP_404_NOT_FOUND + + # Check if the taxonomy was not changed + url = TAXONOMY_TAGS_URL.format(pk=self.taxonomy.id) + self.client.force_authenticate(user=self.staff) + response = self.client.get(url) + tags = response.data["results"] + assert len(tags) == len(self.old_tags) + for i, tag in enumerate(tags): + assert tag["value"] == self.old_tags[i].value diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index 90b2024e239a..a38aab8e01cb 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -1,14 +1,17 @@ """ Tagging Org API Views """ - +from openedx_tagging.core.tagging import rules as oel_tagging_rules from openedx_tagging.core.tagging.rest_api.v1.views import ObjectTagView, TaxonomyView +from rest_framework import status from rest_framework.decorators import action from rest_framework.exceptions import PermissionDenied +from rest_framework.request import Request from rest_framework.response import Response from ...api import ( create_taxonomy, + get_taxonomy, get_taxonomies, get_taxonomies_for_org, set_taxonomy_orgs, @@ -20,7 +23,7 @@ class TaxonomyOrgView(TaxonomyView): """ - View to list, create, retrieve, update, or delete Taxonomies. + View to list, create, retrieve, update, delete, export or import Taxonomies. This view extends the TaxonomyView to add Organization filters. Refer to TaxonomyView docstring for usage details. @@ -69,6 +72,32 @@ def perform_create(self, serializer): user_admin_orgs = get_admin_orgs(self.request.user) serializer.instance = create_taxonomy(**serializer.validated_data, orgs=user_admin_orgs) + @action(detail=False, url_path="import", methods=["post"]) + def create_import(self, request: Request, **kwargs) -> Response: + """ + Creates a new taxonomy with the given orgs and imports the tags from the uploaded file. + """ + response = super().create_import(request, **kwargs) + + # If creation was successful, set the orgs for the new taxonomy + if status.is_success(response.status_code): + # ToDo: This code is temporary + # In the future, the orgs parameter will be defined in the request body from the frontend + # See: https://github.com/openedx/modular-learning/issues/116 + if oel_tagging_rules.is_taxonomy_admin(request.user): + orgs = None + else: + orgs = get_admin_orgs(request.user) + + taxonomy = get_taxonomy(response.data["id"]) + assert taxonomy + set_taxonomy_orgs(taxonomy, all_orgs=False, orgs=orgs) + + serializer = self.get_serializer(taxonomy) + return Response(serializer.data, status=status.HTTP_201_CREATED) + + return response + @action(detail=True, methods=["put"]) def orgs(self, request, **_kwargs) -> Response: """ diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index cf2eafc73f58..348ab52c6aeb 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -281,7 +281,6 @@ def can_change_taxonomy_tag(user: UserType, tag: oel_tagging.Tag | None = None) rules.set_perm("oel_tagging.change_taxonomy", can_change_taxonomy) rules.set_perm("oel_tagging.delete_taxonomy", can_change_taxonomy) rules.set_perm("oel_tagging.view_taxonomy", can_view_taxonomy) -rules.set_perm("oel_tagging.export_taxonomy", can_view_taxonomy) rules.add_perm("oel_tagging.update_orgs", oel_tagging.is_taxonomy_admin) # Tag diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 26d907323ab9..a4c05a6bdbff 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -125,7 +125,7 @@ libsass==0.10.0 click==8.1.6 # pinning this version to avoid updates while the library is being developed -openedx-learning==0.3.2 +openedx-learning==0.3.3 # lti-consumer-xblock 9.6.2 contains a breaking change that makes # existing custom parameter configurations unusable. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index c07c73e0c4a8..714952a1b573 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -803,7 +803,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/kernel.in # lti-consumer-xblock -openedx-learning==0.3.2 +openedx-learning==0.3.3 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index d59dc859031e..f94fd4b028cb 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1331,7 +1331,7 @@ openedx-filters==1.6.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # lti-consumer-xblock -openedx-learning==0.3.2 +openedx-learning==0.3.3 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index cf65bdf87ce8..7006772a93bb 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -961,7 +961,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.3.2 +openedx-learning==0.3.3 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index eb9eee66041e..9d9a2ea89f3a 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1006,7 +1006,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.3.2 +openedx-learning==0.3.3 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt