From 579790d01dcefa2579cad5b8a85633523a4386be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Thu, 9 Nov 2023 23:09:18 -0300 Subject: [PATCH] feat: add import taxonomy endpoint (#112) Adds endpoints to the taxonomy view to create-and-import tags (POST) or import-and-update tags on an existing taxonomy (PUT). --- openedx_learning/__init__.py | 2 +- .../core/tagging/rest_api/v1/serializers.py | 29 ++ .../core/tagging/rest_api/v1/views.py | 83 +++- openedx_tagging/core/tagging/rules.py | 1 - .../core/tagging/test_views.py | 393 ++++++++++++++++++ 5 files changed, 501 insertions(+), 7 deletions(-) diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index 711ab06c..a5b838c5 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -1,4 +1,4 @@ """ Open edX Learning ("Learning Core"). """ -__version__ = "0.3.2" +__version__ = "0.3.3" diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index 3281af3f..62d4ef3e 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -9,6 +9,7 @@ from rest_framework.reverse import reverse from openedx_tagging.core.tagging.data import TagData +from openedx_tagging.core.tagging.import_export.parsers import ParserFormat from openedx_tagging.core.tagging.models import ObjectTag, Tag, Taxonomy @@ -216,3 +217,31 @@ class TaxonomyTagDeleteBodySerializer(serializers.Serializer): # pylint: disabl child=serializers.CharField(), required=True ) with_subtags = serializers.BooleanField(required=False) + + +class TaxonomyImportBodySerializer(serializers.Serializer): # pylint: disable=abstract-method + """ + Serializer of the body for the Taxonomy Import request + """ + file = serializers.FileField(required=True) + + def validate(self, attrs): + """ + Validates the file extension and add parser_format to the data + """ + filename = attrs["file"].name + ext = filename.split('.')[-1] + parser_format = getattr(ParserFormat, ext.upper(), None) + if not parser_format: + raise serializers.ValidationError({"file": f'File type not supported: {ext.lower()}'}, 'file') + + attrs['parser_format'] = parser_format + return attrs + + +class TaxonomyImportNewBodySerializer(TaxonomyImportBodySerializer): # pylint: disable=abstract-method + """ + Serializer of the body for the Taxonomy Create and Import request + """ + taxonomy_name = serializers.CharField(required=True) + taxonomy_description = serializers.CharField(default="") diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index e2145200..170a80c5 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -11,6 +11,7 @@ from rest_framework.decorators import action from rest_framework.exceptions import MethodNotAllowed, PermissionDenied, ValidationError from rest_framework.generics import ListAPIView, RetrieveUpdateDestroyAPIView +from rest_framework.request import Request from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet, ModelViewSet @@ -26,7 +27,7 @@ update_tag_in_taxonomy, ) from ...data import TagDataQuerySet -from ...import_export.api import export_tags +from ...import_export.api import export_tags, get_last_import_log, import_tags from ...import_export.parsers import ParserFormat from ...models import ObjectTag, Taxonomy from ...rules import ObjectTagPermissionItem @@ -40,6 +41,8 @@ ObjectTagUpdateQueryParamsSerializer, TagDataSerializer, TaxonomyExportQueryParamsSerializer, + TaxonomyImportBodySerializer, + TaxonomyImportNewBodySerializer, TaxonomyListQueryParamsSerializer, TaxonomySerializer, TaxonomyTagCreateBodySerializer, @@ -52,7 +55,7 @@ @view_auth_classes class TaxonomyView(ModelViewSet): """ - View to list, create, retrieve, update, delete or export Taxonomies. + View to list, create, retrieve, update, delete, export or import Taxonomies. **List Query Parameters** * enabled (optional) - Filter by enabled status. Valid values: true, @@ -167,7 +170,29 @@ class TaxonomyView(ModelViewSet): * 400 - Invalid query parameter * 403 - Permission denied + **Import/Create Taxonomy Example Requests** + POST /tagging/rest_api/v1/taxonomy/import/ + { + "taxonomy_name": "Taxonomy Name", + "taxonomy_description": "This is a description", + "file": , + } + + **Import/Create Taxonomy Query Returns** + * 200 - Success + * 400 - Bad request + * 403 - Permission denied + + **Import/Update Taxonomy Example Requests** + PUT /tagging/rest_api/v1/taxonomy/:pk/tags/import/ + { + "file": , + } + **Import/Update Taxonomy Query Returns** + * 200 - Success + * 400 - Bad request + * 403 - Permission denied """ # System taxonomies use negative numbers for their primary keys @@ -216,9 +241,6 @@ def export(self, request, **_kwargs) -> HttpResponse: Export a taxonomy. """ taxonomy = self.get_object() - perm = "oel_tagging.export_taxonomy" - if not request.user.has_perm(perm, taxonomy): - raise PermissionDenied("You do not have permission to export this taxonomy.") query_params = TaxonomyExportQueryParamsSerializer( data=request.query_params.dict() ) @@ -243,6 +265,57 @@ def export(self, request, **_kwargs) -> HttpResponse: return HttpResponse(tags, content_type=content_type) + @action(detail=False, url_path="import", methods=["post"]) + def create_import(self, request: Request, **_kwargs) -> Response: + """ + Creates a new taxonomy and imports the tags from the uploaded file. + """ + body = TaxonomyImportNewBodySerializer(data=request.data) + body.is_valid(raise_exception=True) + + taxonomy_name = body.validated_data["taxonomy_name"] + taxonomy_description = body.validated_data["taxonomy_description"] + file = body.validated_data["file"].file + parser_format = body.validated_data["parser_format"] + + taxonomy = create_taxonomy(taxonomy_name, taxonomy_description) + try: + import_success = import_tags(taxonomy, file, parser_format) + + if import_success: + serializer = self.get_serializer(taxonomy) + return Response(serializer.data, status=status.HTTP_201_CREATED) + else: + import_error = get_last_import_log(taxonomy) + taxonomy.delete() + return Response(import_error, status=status.HTTP_400_BAD_REQUEST) + except ValueError as e: + return Response(str(e), status=status.HTTP_400_BAD_REQUEST) + + @action(detail=True, url_path="tags/import", methods=["put"]) + def update_import(self, request: Request, **_kwargs) -> Response: + """ + Imports tags from the uploaded file to an already created taxonomy. + """ + body = TaxonomyImportBodySerializer(data=request.data) + body.is_valid(raise_exception=True) + + file = body.validated_data["file"].file + parser_format = body.validated_data["parser_format"] + + taxonomy = self.get_object() + try: + import_success = import_tags(taxonomy, file, parser_format) + + if import_success: + serializer = self.get_serializer(taxonomy) + return Response(serializer.data) + else: + import_error = get_last_import_log(taxonomy) + return Response(import_error, status=status.HTTP_400_BAD_REQUEST) + except ValueError as e: + return Response(str(e), status=status.HTTP_400_BAD_REQUEST) + @view_auth_classes class ObjectTagView( diff --git a/openedx_tagging/core/tagging/rules.py b/openedx_tagging/core/tagging/rules.py index 97ca56de..76139244 100644 --- a/openedx_tagging/core/tagging/rules.py +++ b/openedx_tagging/core/tagging/rules.py @@ -171,7 +171,6 @@ def can_change_object_tag( rules.add_perm("oel_tagging.change_taxonomy", can_change_taxonomy) rules.add_perm("oel_tagging.delete_taxonomy", can_change_taxonomy) rules.add_perm("oel_tagging.view_taxonomy", can_view_taxonomy) -rules.add_perm("oel_tagging.export_taxonomy", can_view_taxonomy) # Tag rules.add_perm("oel_tagging.add_tag", can_change_tag) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 1a1c69ad..c11de1a2 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -3,12 +3,14 @@ """ from __future__ import annotations +import json from urllib.parse import parse_qs, quote, quote_plus, urlparse import ddt # type: ignore[import] # typing support in rules depends on https://github.com/dfunckt/django-rules/pull/177 import rules # type: ignore[import] from django.contrib.auth import get_user_model +from django.core.files.uploadedfile import SimpleUploadedFile from rest_framework import status from rest_framework.test import APITestCase @@ -29,6 +31,8 @@ TAXONOMY_DETAIL_URL = "/tagging/rest_api/v1/taxonomies/{pk}/" TAXONOMY_EXPORT_URL = "/tagging/rest_api/v1/taxonomies/{pk}/export/" TAXONOMY_TAGS_URL = "/tagging/rest_api/v1/taxonomies/{pk}/tags/" +TAXONOMY_TAGS_IMPORT_URL = "/tagging/rest_api/v1/taxonomies/{pk}/tags/import/" +TAXONOMY_CREATE_IMPORT_URL = "/tagging/rest_api/v1/taxonomies/import/" OBJECT_TAGS_RETRIEVE_URL = "/tagging/rest_api/v1/object_tags/{object_id}/" @@ -1974,3 +1978,392 @@ def test_delete_tag_in_taxonomy_without_subtags(self): # Check that Tag no longer exists with self.assertRaises(Tag.DoesNotExist): existing_tag.refresh_from_db() + + +class ImportTaxonomyMixin(TestTaxonomyViewMixin): + """ + 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") + + +@ddt.ddt +class TestCreateImportView(ImportTaxonomyMixin, APITestCase): + """ + Tests the create/import taxonomy action. + """ + @ddt.data( + "csv", + "json", + ) + def test_import(self, file_format: str) -> None: + """ + Tests importing a valid taxonomy file. + """ + 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"] + + 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() + + +@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 that importing tags into a free text taxonomy is not allowed. + """ + 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 was 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_403_FORBIDDEN + + # 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