From c1d5944106df2634e7fd11394e639dd6e7f114d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 1 Nov 2023 18:22:50 -0300 Subject: [PATCH 01/17] feat: add import taxonomy endpoint --- .../core/tagging/import_export/api.py | 20 ++++++++ .../core/tagging/rest_api/v1/serializers.py | 31 +++++++++++ .../core/tagging/rest_api/v1/urls.py | 5 ++ .../core/tagging/rest_api/v1/views_import.py | 51 ++++++++++++++++++- .../core/tagging/test_views_import.py | 44 ++++++++++++++++ 5 files changed, 150 insertions(+), 1 deletion(-) diff --git a/openedx_tagging/core/tagging/import_export/api.py b/openedx_tagging/core/tagging/import_export/api.py index 6afa9096a..7112a3cd9 100644 --- a/openedx_tagging/core/tagging/import_export/api.py +++ b/openedx_tagging/core/tagging/import_export/api.py @@ -48,12 +48,32 @@ from django.utils.translation import gettext as _ +from .. import api as taxonomy_api from ..models import TagImportTask, TagImportTaskState, Taxonomy from .exceptions import TagImportError from .import_plan import TagImportPlan, TagImportTask from .parsers import ParserFormat, get_parser +def create_taxonomy_and_import_tags( + taxonomy_name: str, + taxonomy_description: str, + file: BytesIO, + parser_format: ParserFormat +) -> bool: + """ + Create a taxonomy and import the tags from `file` + """ + taxonomy = taxonomy_api.create_taxonomy(taxonomy_name, taxonomy_description) + + import_success = import_tags(taxonomy, file, parser_format) + + if not import_success: + taxonomy.delete() + + return import_success + + def import_tags( taxonomy: Taxonomy, file: BytesIO, diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index 4fe62c72a..53607caad 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -8,6 +8,7 @@ from openedx_tagging.core.tagging.data import TagData from openedx_tagging.core.tagging.models import ObjectTag, Tag, Taxonomy +from openedx_tagging.core.tagging.import_export.parsers import ParserFormat class TaxonomyListQueryParamsSerializer(serializers.Serializer): # pylint: disable=abstract-method @@ -175,3 +176,33 @@ 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 action + """ + taxonomy_name = serializers.CharField(required=True) + taxonomy_description = serializers.CharField(default="") + file = serializers.FileField(required=True) + + def get_parser_format(self, obj): + """ + Returns the ParserFormat based on the file extension + """ + filename = obj["file"].name + ext = filename.split('.')[-1] + if ext.lower() == 'csv': + return ParserFormat.CSV + elif ext.lower() == 'json': + return ParserFormat.JSON + else: + raise serializers.ValidationError(f'File type not supported: ${ext.lower()}') + + def to_internal_value(self, data): + """ + Adds the parser_format to the validated data + """ + validated_data = super().to_internal_value(data) + validated_data['parser_format'] = self.get_parser_format(data) + return validated_data diff --git a/openedx_tagging/core/tagging/rest_api/v1/urls.py b/openedx_tagging/core/tagging/rest_api/v1/urls.py index 72ff87d0d..dc0e7a835 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/urls.py +++ b/openedx_tagging/core/tagging/rest_api/v1/urls.py @@ -23,4 +23,9 @@ views_import.TemplateView.as_view(), name="taxonomy-import-template", ), + path( + "import/", + views_import.ImportView.as_view(), + name="taxonomy-import", + ), ] diff --git a/openedx_tagging/core/tagging/rest_api/v1/views_import.py b/openedx_tagging/core/tagging/rest_api/v1/views_import.py index 6c3a63398..f3caed4f2 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views_import.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views_import.py @@ -5,10 +5,15 @@ import os -from django.http import FileResponse, Http404 +from django.http import FileResponse, Http404, HttpResponse from rest_framework.request import Request from rest_framework.views import APIView +from ...import_export import api +from .serializers import ( + TaxonomyImportBodySerializer, +) + class TemplateView(APIView): """ @@ -49,3 +54,47 @@ def get(self, request: Request, file_ext: str, *args, **kwargs) -> FileResponse: response = FileResponse(fh, content_type=content_type) response['Content-Disposition'] = content_disposition return response + + +class ImportView(APIView): + """ + View to import taxonomies + + **Example Requests** + POST /tagging/rest_api/v1/import/ + { + "taxonomy_name": "Taxonomy Name", + "taxonomy_description": "This is a description", + "file": , + } + + **Query Returns** + * 200 - Success + * 400 - Bad request + * 405 - Method not allowed + """ + http_method_names = ['post'] + + def post(self, request: Request, *args, **kwargs) -> HttpResponse: + """ + Imports the taxonomy from the uploaded file. + """ + body = TaxonomyImportBodySerializer(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"] + + import_success = api.create_taxonomy_and_import_tags( + taxonomy_name=taxonomy_name, + taxonomy_description=taxonomy_description, + file=file, + parser_format=parser_format, + ) + + if import_success: + return HttpResponse(status=200) + else: + return HttpResponse(status=400) diff --git a/tests/openedx_tagging/core/tagging/test_views_import.py b/tests/openedx_tagging/core/tagging/test_views_import.py index 4c2d94c9a..e362fff71 100644 --- a/tests/openedx_tagging/core/tagging/test_views_import.py +++ b/tests/openedx_tagging/core/tagging/test_views_import.py @@ -3,11 +3,17 @@ """ from __future__ import annotations +import json + import ddt # type: ignore[import] +from django.core.files.uploadedfile import SimpleUploadedFile from rest_framework import status from rest_framework.test import APITestCase +from openedx_tagging.core.tagging.models import Tag, Taxonomy + TAXONOMY_TEMPLATE_URL = "/tagging/rest_api/v1/import/{filename}" +TAXONOMY_IMPORT_URL = "/tagging/rest_api/v1/import/" @ddt.ddt @@ -37,3 +43,41 @@ 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 TestImportView(APITestCase): + """ + Tests the import taxonomy view. + """ + + def test_import(self): + url = TAXONOMY_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"}, + ] + json_data = {"tags": new_tags} + file = SimpleUploadedFile("taxonomy.json", json.dumps(json_data).encode(), content_type="application/json") + + 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_200_OK + + # Check if the taxonomy was created + taxonomy = Taxonomy.objects.get(name="Imported Taxonomy name") + assert taxonomy.description == "Imported Taxonomy description" + + # Check if the tags were created + tags = list(Tag.objects.filter(taxonomy=taxonomy)) + assert len(tags) == len(new_tags) + for i, tag in enumerate(tags): + assert tag.value == new_tags[i]["value"] From 05105bcc9e733619650c5041caa91215d9fa36d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 1 Nov 2023 18:51:50 -0300 Subject: [PATCH 02/17] style: fix pylint --- openedx_tagging/core/tagging/rest_api/v1/serializers.py | 2 +- openedx_tagging/core/tagging/rest_api/v1/views_import.py | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index 53607caad..c1a366a70 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -7,8 +7,8 @@ from rest_framework.reverse import reverse from openedx_tagging.core.tagging.data import TagData -from openedx_tagging.core.tagging.models import ObjectTag, Tag, Taxonomy from openedx_tagging.core.tagging.import_export.parsers import ParserFormat +from openedx_tagging.core.tagging.models import ObjectTag, Tag, Taxonomy class TaxonomyListQueryParamsSerializer(serializers.Serializer): # pylint: disable=abstract-method diff --git a/openedx_tagging/core/tagging/rest_api/v1/views_import.py b/openedx_tagging/core/tagging/rest_api/v1/views_import.py index f3caed4f2..d971bfcf1 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views_import.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views_import.py @@ -10,9 +10,7 @@ from rest_framework.views import APIView from ...import_export import api -from .serializers import ( - TaxonomyImportBodySerializer, -) +from .serializers import TaxonomyImportBodySerializer class TemplateView(APIView): From a6919a1110197ee3bb842cc4bef98b2a2282c3d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 3 Nov 2023 09:39:07 -0300 Subject: [PATCH 03/17] refactor: improve get_parse_format implementation Co-authored-by: Jillian --- openedx_tagging/core/tagging/rest_api/v1/serializers.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index c1a366a70..9c03ff6af 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -192,10 +192,9 @@ def get_parser_format(self, obj): """ filename = obj["file"].name ext = filename.split('.')[-1] - if ext.lower() == 'csv': - return ParserFormat.CSV - elif ext.lower() == 'json': - return ParserFormat.JSON + parser_format = getattr(ParserFormat, ext.upper(), None) + if parser_format: + return parser_format else: raise serializers.ValidationError(f'File type not supported: ${ext.lower()}') From f0307676e8458539fa6fb48cd69fabca013eb0c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 3 Nov 2023 12:04:23 -0300 Subject: [PATCH 04/17] test: add new tests --- .../core/tagging/rest_api/v1/serializers.py | 21 +++- .../core/tagging/rest_api/v1/views_import.py | 4 +- .../core/tagging/test_views_import.py | 105 +++++++++++++++++- 3 files changed, 119 insertions(+), 11 deletions(-) diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index 9c03ff6af..6134e6222 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -186,17 +186,28 @@ class TaxonomyImportBodySerializer(serializers.Serializer): # pylint: disable=a taxonomy_description = serializers.CharField(default="") file = serializers.FileField(required=True) - def get_parser_format(self, obj): + def validate_file(self, file): + """ + Validates the file extension + """ + filename = file.name + ext = filename.split('.')[-1] + parser_format = getattr(ParserFormat, ext.upper(), None) + if not parser_format: + raise serializers.ValidationError(f'File type not supported: {ext.lower()}') + + return file + + def get_parser_format(self, obj) -> ParserFormat: """ Returns the ParserFormat based on the file extension """ filename = obj["file"].name ext = filename.split('.')[-1] parser_format = getattr(ParserFormat, ext.upper(), None) - if parser_format: - return parser_format - else: - raise serializers.ValidationError(f'File type not supported: ${ext.lower()}') + assert parser_format, f'File type not supported: ${ext.lower()}' + + return parser_format def to_internal_value(self, data): """ diff --git a/openedx_tagging/core/tagging/rest_api/v1/views_import.py b/openedx_tagging/core/tagging/rest_api/v1/views_import.py index d971bfcf1..7b23816cb 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views_import.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views_import.py @@ -5,7 +5,7 @@ import os -from django.http import FileResponse, Http404, HttpResponse +from django.http import FileResponse, Http404, HttpResponse, HttpResponseBadRequest from rest_framework.request import Request from rest_framework.views import APIView @@ -95,4 +95,4 @@ def post(self, request: Request, *args, **kwargs) -> HttpResponse: if import_success: return HttpResponse(status=200) else: - return HttpResponse(status=400) + return HttpResponseBadRequest("Error importing taxonomy") diff --git a/tests/openedx_tagging/core/tagging/test_views_import.py b/tests/openedx_tagging/core/tagging/test_views_import.py index e362fff71..8271c7987 100644 --- a/tests/openedx_tagging/core/tagging/test_views_import.py +++ b/tests/openedx_tagging/core/tagging/test_views_import.py @@ -45,12 +45,33 @@ def test_download_method_not_allowed(self): assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED +@ddt.ddt class TestImportView(APITestCase): """ Tests the import taxonomy view. """ - def test_import(self): + 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.data( + "csv", + "json", + ) + def test_import(self, file_format: str) -> None: + """ + Tests importing a valid taxonomy file. + """ url = TAXONOMY_IMPORT_URL new_tags = [ {"id": "tag_1", "value": "Tag 1"}, @@ -58,15 +79,14 @@ def test_import(self): {"id": "tag_3", "value": "Tag 3"}, {"id": "tag_4", "value": "Tag 4"}, ] - json_data = {"tags": new_tags} - file = SimpleUploadedFile("taxonomy.json", json.dumps(json_data).encode(), content_type="application/json") + file = self._get_file(new_tags, file_format) response = self.client.post( url, { "taxonomy_name": "Imported Taxonomy name", "taxonomy_description": "Imported Taxonomy description", - "file": file + "file": file, }, format="multipart" ) @@ -81,3 +101,80 @@ def test_import(self): 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_IMPORT_URL + 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." + + @ddt.data( + "csv", + "json", + ) + def test_import_no_name(self, file_format) -> None: + """ + Tests importing a taxonomy without specifing a name. + """ + url = TAXONOMY_IMPORT_URL + file = SimpleUploadedFile(f"taxonomy.{file_format}", b"invalid file content") + 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." + + def test_import_invalid_format(self) -> None: + """ + Tests importing a taxonomy with an invalid file format. + """ + url = TAXONOMY_IMPORT_URL + file = SimpleUploadedFile("taxonomy.invalid", b"invalid file content") + 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" + + @ddt.data( + "csv", + "json", + ) + def test_import_invalid_content(self, file_format) -> None: + """ + Tests importing a taxonomy with an invalid file content. + """ + url = TAXONOMY_IMPORT_URL + file = SimpleUploadedFile(f"taxonomy.{file_format}", b"invalid file content") + 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.content == b"Error importing taxonomy" From 4fe2457e3f169ae8dc4ef081d7ceb0f8416dfed2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Mon, 6 Nov 2023 11:54:53 -0300 Subject: [PATCH 05/17] feat: add permission check --- .../core/tagging/rest_api/v1/views_import.py | 6 +++ openedx_tagging/core/tagging/rules.py | 1 + .../core/tagging/test_views_import.py | 46 +++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/openedx_tagging/core/tagging/rest_api/v1/views_import.py b/openedx_tagging/core/tagging/rest_api/v1/views_import.py index 7b23816cb..1644bee24 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views_import.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views_import.py @@ -6,6 +6,7 @@ import os from django.http import FileResponse, Http404, HttpResponse, HttpResponseBadRequest +from rest_framework.exceptions import PermissionDenied from rest_framework.request import Request from rest_framework.views import APIView @@ -77,6 +78,11 @@ def post(self, request: Request, *args, **kwargs) -> HttpResponse: """ Imports the taxonomy from the uploaded file. """ + perm = "oel_tagging.import_taxonomy" + if not request.user.has_perm(perm): + raise PermissionDenied("You do not have permission to import taxonomies") + + body = TaxonomyImportBodySerializer(data=request.data) body.is_valid(raise_exception=True) diff --git a/openedx_tagging/core/tagging/rules.py b/openedx_tagging/core/tagging/rules.py index 97ca56de2..2f9961651 100644 --- a/openedx_tagging/core/tagging/rules.py +++ b/openedx_tagging/core/tagging/rules.py @@ -172,6 +172,7 @@ def can_change_object_tag( 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) +rules.add_perm("oel_tagging.import_taxonomy", can_change_taxonomy) # Tag rules.add_perm("oel_tagging.add_tag", can_change_tag) diff --git a/tests/openedx_tagging/core/tagging/test_views_import.py b/tests/openedx_tagging/core/tagging/test_views_import.py index 8271c7987..12ec1a9d5 100644 --- a/tests/openedx_tagging/core/tagging/test_views_import.py +++ b/tests/openedx_tagging/core/tagging/test_views_import.py @@ -6,6 +6,7 @@ import json import ddt # 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 @@ -15,6 +16,7 @@ TAXONOMY_TEMPLATE_URL = "/tagging/rest_api/v1/import/{filename}" TAXONOMY_IMPORT_URL = "/tagging/rest_api/v1/import/" +User = get_user_model() @ddt.ddt class TestTemplateView(APITestCase): @@ -50,6 +52,20 @@ class TestImportView(APITestCase): """ Tests the import taxonomy view. """ + def setUp(self): + super().setUp() + + self.user = User.objects.create( + username="user", + email="user@example.com", + ) + + self.staff = User.objects.create( + username="staff", + email="staff@example.com", + is_staff=True, + ) + def _get_file(self, tags: list, file_format: str) -> SimpleUploadedFile: """ @@ -81,6 +97,7 @@ def test_import(self, file_format: str) -> None: ] file = self._get_file(new_tags, file_format) + self.client.force_authenticate(user=self.staff) response = self.client.post( url, { @@ -107,6 +124,7 @@ def test_import_no_file(self) -> None: Tests importing a taxonomy without a file. """ url = TAXONOMY_IMPORT_URL + self.client.force_authenticate(user=self.staff) response = self.client.post( url, { @@ -128,6 +146,7 @@ def test_import_no_name(self, file_format) -> None: """ url = TAXONOMY_IMPORT_URL file = SimpleUploadedFile(f"taxonomy.{file_format}", b"invalid file content") + self.client.force_authenticate(user=self.staff) response = self.client.post( url, { @@ -145,6 +164,7 @@ def test_import_invalid_format(self) -> None: """ url = TAXONOMY_IMPORT_URL file = SimpleUploadedFile("taxonomy.invalid", b"invalid file content") + self.client.force_authenticate(user=self.staff) response = self.client.post( url, { @@ -167,6 +187,7 @@ def test_import_invalid_content(self, file_format) -> None: """ url = TAXONOMY_IMPORT_URL file = SimpleUploadedFile(f"taxonomy.{file_format}", b"invalid file content") + self.client.force_authenticate(user=self.staff) response = self.client.post( url, { @@ -178,3 +199,28 @@ def test_import_invalid_content(self, file_format) -> None: ) assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.content == b"Error importing taxonomy" + + def test_import_no_perm(self) -> None: + """ + Tests importing a taxonomy using a user without permission. + """ + url = TAXONOMY_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 From 06efd55e15416dfe9a2a3ecfec62cf232e7cea80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Mon, 6 Nov 2023 11:59:48 -0300 Subject: [PATCH 06/17] style: fix pylint --- openedx_tagging/core/tagging/rest_api/v1/views_import.py | 1 - tests/openedx_tagging/core/tagging/test_views_import.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/openedx_tagging/core/tagging/rest_api/v1/views_import.py b/openedx_tagging/core/tagging/rest_api/v1/views_import.py index 1644bee24..80332838f 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views_import.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views_import.py @@ -82,7 +82,6 @@ def post(self, request: Request, *args, **kwargs) -> HttpResponse: if not request.user.has_perm(perm): raise PermissionDenied("You do not have permission to import taxonomies") - body = TaxonomyImportBodySerializer(data=request.data) body.is_valid(raise_exception=True) diff --git a/tests/openedx_tagging/core/tagging/test_views_import.py b/tests/openedx_tagging/core/tagging/test_views_import.py index 12ec1a9d5..206111282 100644 --- a/tests/openedx_tagging/core/tagging/test_views_import.py +++ b/tests/openedx_tagging/core/tagging/test_views_import.py @@ -18,6 +18,7 @@ User = get_user_model() + @ddt.ddt class TestTemplateView(APITestCase): """ @@ -66,7 +67,6 @@ def setUp(self): is_staff=True, ) - def _get_file(self, tags: list, file_format: str) -> SimpleUploadedFile: """ Returns a file for the given format. From d400a5418557b3622d9178be54c4de75372a90f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Tue, 7 Nov 2023 08:53:55 -0300 Subject: [PATCH 07/17] refactor: remove duplicated code --- .../core/tagging/rest_api/v1/serializers.py | 30 ++++--------------- 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index 6134e6222..a2a0cf4ec 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -186,33 +186,15 @@ class TaxonomyImportBodySerializer(serializers.Serializer): # pylint: disable=a taxonomy_description = serializers.CharField(default="") file = serializers.FileField(required=True) - def validate_file(self, file): + def validate(self, data): """ - Validates the file extension + Validates the file extension and add parser_format to the data """ - filename = file.name + filename = data["file"].name ext = filename.split('.')[-1] parser_format = getattr(ParserFormat, ext.upper(), None) if not parser_format: - raise serializers.ValidationError(f'File type not supported: {ext.lower()}') + raise serializers.ValidationError({"file": f'File type not supported: {ext.lower()}'}, 'file') - return file - - def get_parser_format(self, obj) -> ParserFormat: - """ - Returns the ParserFormat based on the file extension - """ - filename = obj["file"].name - ext = filename.split('.')[-1] - parser_format = getattr(ParserFormat, ext.upper(), None) - assert parser_format, f'File type not supported: ${ext.lower()}' - - return parser_format - - def to_internal_value(self, data): - """ - Adds the parser_format to the validated data - """ - validated_data = super().to_internal_value(data) - validated_data['parser_format'] = self.get_parser_format(data) - return validated_data + data['parser_format'] = parser_format + return data From a50e08f1cf1efd3bc92078a6d29a8decc91bc929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Tue, 7 Nov 2023 10:30:19 -0300 Subject: [PATCH 08/17] refactor: import taxonomy view --- .../core/tagging/import_export/api.py | 20 - .../core/tagging/rest_api/v1/serializers.py | 9 +- .../core/tagging/rest_api/v1/urls.py | 5 - .../core/tagging/rest_api/v1/views.py | 80 +++- .../core/tagging/rest_api/v1/views_import.py | 54 +-- .../core/tagging/test_views.py | 345 ++++++++++++++++++ .../core/tagging/test_views_import.py | 187 ---------- 7 files changed, 431 insertions(+), 269 deletions(-) diff --git a/openedx_tagging/core/tagging/import_export/api.py b/openedx_tagging/core/tagging/import_export/api.py index 7112a3cd9..6afa9096a 100644 --- a/openedx_tagging/core/tagging/import_export/api.py +++ b/openedx_tagging/core/tagging/import_export/api.py @@ -48,32 +48,12 @@ from django.utils.translation import gettext as _ -from .. import api as taxonomy_api from ..models import TagImportTask, TagImportTaskState, Taxonomy from .exceptions import TagImportError from .import_plan import TagImportPlan, TagImportTask from .parsers import ParserFormat, get_parser -def create_taxonomy_and_import_tags( - taxonomy_name: str, - taxonomy_description: str, - file: BytesIO, - parser_format: ParserFormat -) -> bool: - """ - Create a taxonomy and import the tags from `file` - """ - taxonomy = taxonomy_api.create_taxonomy(taxonomy_name, taxonomy_description) - - import_success = import_tags(taxonomy, file, parser_format) - - if not import_success: - taxonomy.delete() - - return import_success - - def import_tags( taxonomy: Taxonomy, file: BytesIO, diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index a2a0cf4ec..0cdb48269 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -182,8 +182,6 @@ class TaxonomyImportBodySerializer(serializers.Serializer): # pylint: disable=a """ Serializer of the body for the Taxonomy Import action """ - taxonomy_name = serializers.CharField(required=True) - taxonomy_description = serializers.CharField(default="") file = serializers.FileField(required=True) def validate(self, data): @@ -198,3 +196,10 @@ def validate(self, data): data['parser_format'] = parser_format return data + +class TaxonomyImportNewBodySerializer(TaxonomyImportBodySerializer): # pylint: disable=abstract-method + """ + Serializer of the body for the Taxonomy Create and Import action + """ + taxonomy_name = serializers.CharField(required=True) + taxonomy_description = serializers.CharField(default="") diff --git a/openedx_tagging/core/tagging/rest_api/v1/urls.py b/openedx_tagging/core/tagging/rest_api/v1/urls.py index dc0e7a835..72ff87d0d 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/urls.py +++ b/openedx_tagging/core/tagging/rest_api/v1/urls.py @@ -23,9 +23,4 @@ views_import.TemplateView.as_view(), name="taxonomy-import-template", ), - path( - "import/", - views_import.ImportView.as_view(), - name="taxonomy-import", - ), ] diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index 951c8e6f2..088be7f21 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -4,11 +4,12 @@ from __future__ import annotations from django.db import models -from django.http import Http404, HttpResponse +from django.http import Http404, HttpResponse, HttpResponseBadRequest from rest_framework import mixins, status 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 @@ -24,7 +25,7 @@ update_tag_in_taxonomy, ) from ...data import TagDataQuerySet -from ...import_export.api import export_tags +from ...import_export.api import export_tags, import_tags from ...import_export.parsers import ParserFormat from ...models import Taxonomy from ...rules import ObjectTagPermissionItem @@ -37,6 +38,8 @@ ObjectTagUpdateQueryParamsSerializer, TagDataSerializer, TaxonomyExportQueryParamsSerializer, + TaxonomyImportBodySerializer, + TaxonomyImportNewBodySerializer, TaxonomyListQueryParamsSerializer, TaxonomySerializer, TaxonomyTagCreateBodySerializer, @@ -164,7 +167,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 """ lookup_value_regex = r"\d+" @@ -239,6 +264,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) -> HttpResponse: + """ + Create a new taxonomy and imports the tags from the uploaded file. + """ + perm = "oel_tagging.import_taxonomy" + if not request.user.has_perm(perm): + raise PermissionDenied("You do not have permission to import taxonomies") + + 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) + import_success = import_tags(taxonomy, file, parser_format) + + if import_success: + return HttpResponse(status=200) + else: + import_error = "Error importing taxonomy" # ToDo: Get actual error message + taxonomy.delete() + return HttpResponseBadRequest(import_error) + + @action(detail=True, url_path="tags/import", methods=["put"]) + def update_import(self, request: Request, **_kwargs) -> HttpResponse: + """ + Imports tags from the uploaded file to an already created taxonomy. + """ + perm = "oel_tagging.import_taxonomy" + if not request.user.has_perm(perm): + raise PermissionDenied("You do not have permission to import taxonomies") + + 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() + import_success = import_tags(taxonomy, file, parser_format) + + if import_success: + return HttpResponse(status=200) + else: + import_error = "Error importing taxonomy" # ToDo: Get actual error message + return HttpResponseBadRequest(import_error) + @view_auth_classes class ObjectTagView( diff --git a/openedx_tagging/core/tagging/rest_api/v1/views_import.py b/openedx_tagging/core/tagging/rest_api/v1/views_import.py index 80332838f..6c3a63398 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views_import.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views_import.py @@ -5,14 +5,10 @@ import os -from django.http import FileResponse, Http404, HttpResponse, HttpResponseBadRequest -from rest_framework.exceptions import PermissionDenied +from django.http import FileResponse, Http404 from rest_framework.request import Request from rest_framework.views import APIView -from ...import_export import api -from .serializers import TaxonomyImportBodySerializer - class TemplateView(APIView): """ @@ -53,51 +49,3 @@ def get(self, request: Request, file_ext: str, *args, **kwargs) -> FileResponse: response = FileResponse(fh, content_type=content_type) response['Content-Disposition'] = content_disposition return response - - -class ImportView(APIView): - """ - View to import taxonomies - - **Example Requests** - POST /tagging/rest_api/v1/import/ - { - "taxonomy_name": "Taxonomy Name", - "taxonomy_description": "This is a description", - "file": , - } - - **Query Returns** - * 200 - Success - * 400 - Bad request - * 405 - Method not allowed - """ - http_method_names = ['post'] - - def post(self, request: Request, *args, **kwargs) -> HttpResponse: - """ - Imports the taxonomy from the uploaded file. - """ - perm = "oel_tagging.import_taxonomy" - if not request.user.has_perm(perm): - raise PermissionDenied("You do not have permission to import taxonomies") - - body = TaxonomyImportBodySerializer(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"] - - import_success = api.create_taxonomy_and_import_tags( - taxonomy_name=taxonomy_name, - taxonomy_description=taxonomy_description, - file=file, - parser_format=parser_format, - ) - - if import_success: - return HttpResponse(status=200) - else: - return HttpResponseBadRequest("Error importing taxonomy") diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 6a3125464..a5b69c253 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 @@ -27,6 +29,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}/" @@ -1856,3 +1860,344 @@ 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_200_OK + + # Check if the taxonomy was created + taxonomy = Taxonomy.objects.get(name="Imported Taxonomy name") + assert taxonomy.description == "Imported Taxonomy description" + + # Check if the tags were created + tags = list(Tag.objects.filter(taxonomy=taxonomy)) + 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 response.content == b"Error importing taxonomy" + + # 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 + tags = list(Tag.objects.filter(taxonomy=self.taxonomy)) + 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 + tags = list(Tag.objects.filter(taxonomy=self.taxonomy)) + 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 + tags = list(Tag.objects.filter(taxonomy=self.taxonomy)) + 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 response.content == b"Error importing taxonomy" + + # Check if the taxonomy was not changed + tags = list(Tag.objects.filter(taxonomy=self.taxonomy)) + assert len(tags) == len(self.old_tags) + for i, tag in enumerate(tags): + assert tag.value == self.old_tags[i].value + + 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 + tags = list(Tag.objects.filter(taxonomy=self.taxonomy)) + assert len(tags) == len(self.old_tags) + for i, tag in enumerate(tags): + assert tag.value == self.old_tags[i].value diff --git a/tests/openedx_tagging/core/tagging/test_views_import.py b/tests/openedx_tagging/core/tagging/test_views_import.py index 206111282..4c2d94c9a 100644 --- a/tests/openedx_tagging/core/tagging/test_views_import.py +++ b/tests/openedx_tagging/core/tagging/test_views_import.py @@ -3,20 +3,11 @@ """ from __future__ import annotations -import json - import ddt # 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 -from openedx_tagging.core.tagging.models import Tag, Taxonomy - TAXONOMY_TEMPLATE_URL = "/tagging/rest_api/v1/import/{filename}" -TAXONOMY_IMPORT_URL = "/tagging/rest_api/v1/import/" - -User = get_user_model() @ddt.ddt @@ -46,181 +37,3 @@ 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 - - -@ddt.ddt -class TestImportView(APITestCase): - """ - Tests the import taxonomy view. - """ - def setUp(self): - super().setUp() - - self.user = User.objects.create( - username="user", - email="user@example.com", - ) - - self.staff = User.objects.create( - username="staff", - email="staff@example.com", - is_staff=True, - ) - - 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.data( - "csv", - "json", - ) - def test_import(self, file_format: str) -> None: - """ - Tests importing a valid taxonomy file. - """ - url = TAXONOMY_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_200_OK - - # Check if the taxonomy was created - taxonomy = Taxonomy.objects.get(name="Imported Taxonomy name") - assert taxonomy.description == "Imported Taxonomy description" - - # Check if the tags were created - tags = list(Tag.objects.filter(taxonomy=taxonomy)) - 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_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." - - @ddt.data( - "csv", - "json", - ) - def test_import_no_name(self, file_format) -> None: - """ - Tests importing a taxonomy without specifing a name. - """ - url = TAXONOMY_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." - - def test_import_invalid_format(self) -> None: - """ - Tests importing a taxonomy with an invalid file format. - """ - url = TAXONOMY_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" - - @ddt.data( - "csv", - "json", - ) - def test_import_invalid_content(self, file_format) -> None: - """ - Tests importing a taxonomy with an invalid file content. - """ - url = TAXONOMY_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 response.content == b"Error importing taxonomy" - - def test_import_no_perm(self) -> None: - """ - Tests importing a taxonomy using a user without permission. - """ - url = TAXONOMY_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 From e1b178990b59c591793b1bea85c7fd9b01fcaa1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Tue, 7 Nov 2023 10:44:37 -0300 Subject: [PATCH 09/17] style: fix pylint --- openedx_tagging/core/tagging/rest_api/v1/serializers.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index 0cdb48269..35c630c93 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -184,18 +184,19 @@ class TaxonomyImportBodySerializer(serializers.Serializer): # pylint: disable=a """ file = serializers.FileField(required=True) - def validate(self, data): + def validate(self, attrs): """ Validates the file extension and add parser_format to the data """ - filename = data["file"].name + 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') - data['parser_format'] = parser_format - return data + attrs['parser_format'] = parser_format + return attrs + class TaxonomyImportNewBodySerializer(TaxonomyImportBodySerializer): # pylint: disable=abstract-method """ From d9950f6938d1dda8f87e395a02724e0d669c8146 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Tue, 7 Nov 2023 11:11:18 -0300 Subject: [PATCH 10/17] feat: add allow_free_text test and style fix --- .../core/tagging/rest_api/v1/views.py | 32 ++++++++++------- .../core/tagging/test_views.py | 36 +++++++++++++++++-- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index 088be7f21..594cf89e6 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -282,14 +282,17 @@ def create_import(self, request: Request, **_kwargs) -> HttpResponse: parser_format = body.validated_data["parser_format"] taxonomy = create_taxonomy(taxonomy_name, taxonomy_description) - import_success = import_tags(taxonomy, file, parser_format) + try: + import_success = import_tags(taxonomy, file, parser_format) - if import_success: - return HttpResponse(status=200) - else: - import_error = "Error importing taxonomy" # ToDo: Get actual error message - taxonomy.delete() - return HttpResponseBadRequest(import_error) + if import_success: + return HttpResponse(status=200) + else: + import_error = "Error importing taxonomy" # ToDo: Get actual error message + taxonomy.delete() + return HttpResponseBadRequest(import_error) + except ValueError as e: + return HttpResponseBadRequest(e) @action(detail=True, url_path="tags/import", methods=["put"]) def update_import(self, request: Request, **_kwargs) -> HttpResponse: @@ -307,13 +310,16 @@ def update_import(self, request: Request, **_kwargs) -> HttpResponse: parser_format = body.validated_data["parser_format"] taxonomy = self.get_object() - import_success = import_tags(taxonomy, file, parser_format) + try: + import_success = import_tags(taxonomy, file, parser_format) - if import_success: - return HttpResponse(status=200) - else: - import_error = "Error importing taxonomy" # ToDo: Get actual error message - return HttpResponseBadRequest(import_error) + if import_success: + return HttpResponse(status=200) + else: + import_error = "Error importing taxonomy" # ToDo: Get actual error message + return HttpResponseBadRequest(import_error) + except ValueError as e: + return HttpResponseBadRequest(e) @view_auth_classes diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index a5b69c253..412612582 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -2045,6 +2045,7 @@ def test_import_no_perm(self) -> None: # Check if the taxonomy was not created assert not Taxonomy.objects.filter(name="Imported Taxonomy name").exists() + @ddt.ddt class TestImportTagsView(ImportTaxonomyMixin, APITestCase): """ @@ -2088,7 +2089,7 @@ def test_import(self, file_format: str) -> None: self.client.force_authenticate(user=self.staff) response = self.client.put( url, - { "file": file }, + {"file": file}, format="multipart" ) assert response.status_code == status.HTTP_200_OK @@ -2129,7 +2130,7 @@ def test_import_invalid_format(self) -> None: self.client.force_authenticate(user=self.staff) response = self.client.put( url, - { "file": file }, + {"file": file}, format="multipart" ) assert response.status_code == status.HTTP_400_BAD_REQUEST @@ -2141,7 +2142,6 @@ def test_import_invalid_format(self) -> None: for i, tag in enumerate(tags): assert tag.value == self.old_tags[i].value - @ddt.data( "csv", "json", @@ -2171,6 +2171,36 @@ def test_import_invalid_content(self, file_format) -> None: 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 + error_msg = f"Invalid taxonomy ({self.taxonomy.id}): You cannot import a free-form taxonomy." + assert response.content == error_msg.encode() + def test_import_no_perm(self) -> None: """ Tests importing a taxonomy using a user without permission. From 7450737d3cd7cf99c6e48422c170741a791ecca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Tue, 7 Nov 2023 14:47:38 -0300 Subject: [PATCH 11/17] feat: add error messages to response --- openedx_tagging/core/tagging/rest_api/v1/views.py | 6 +++--- tests/openedx_tagging/core/tagging/test_views.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index 594cf89e6..3d63c1ada 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -25,7 +25,7 @@ update_tag_in_taxonomy, ) from ...data import TagDataQuerySet -from ...import_export.api import export_tags, import_tags +from ...import_export.api import export_tags, get_last_import_log, import_tags from ...import_export.parsers import ParserFormat from ...models import Taxonomy from ...rules import ObjectTagPermissionItem @@ -288,7 +288,7 @@ def create_import(self, request: Request, **_kwargs) -> HttpResponse: if import_success: return HttpResponse(status=200) else: - import_error = "Error importing taxonomy" # ToDo: Get actual error message + import_error = get_last_import_log(taxonomy) taxonomy.delete() return HttpResponseBadRequest(import_error) except ValueError as e: @@ -316,7 +316,7 @@ def update_import(self, request: Request, **_kwargs) -> HttpResponse: if import_success: return HttpResponse(status=200) else: - import_error = "Error importing taxonomy" # ToDo: Get actual error message + import_error = get_last_import_log(taxonomy) return HttpResponseBadRequest(import_error) except ValueError as e: return HttpResponseBadRequest(e) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 412612582..4cf794fdb 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -2012,7 +2012,7 @@ def test_import_invalid_content(self, file_format) -> None: format="multipart" ) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.content == b"Error importing taxonomy" + assert f"Invalid '.{file_format}' format:".encode() in response.content # Check if the taxonomy was not created assert not Taxonomy.objects.filter(name="Imported Taxonomy name").exists() @@ -2163,7 +2163,7 @@ def test_import_invalid_content(self, file_format) -> None: format="multipart" ) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.content == b"Error importing taxonomy" + assert f"Invalid '.{file_format}' format:".encode() in response.content # Check if the taxonomy was not changed tags = list(Tag.objects.filter(taxonomy=self.taxonomy)) From ae764004976bcc8801b86a409046c843ae3b4721 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 8 Nov 2023 10:50:14 -0300 Subject: [PATCH 12/17] refactor: remove redudant permissions --- openedx_tagging/core/tagging/rest_api/v1/views.py | 13 +------------ openedx_tagging/core/tagging/rules.py | 2 -- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index 3d63c1ada..d45bb1cc9 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -237,9 +237,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() ) @@ -267,12 +264,8 @@ def export(self, request, **_kwargs) -> HttpResponse: @action(detail=False, url_path="import", methods=["post"]) def create_import(self, request: Request, **_kwargs) -> HttpResponse: """ - Create a new taxonomy and imports the tags from the uploaded file. + Creates a new taxonomy and imports the tags from the uploaded file. """ - perm = "oel_tagging.import_taxonomy" - if not request.user.has_perm(perm): - raise PermissionDenied("You do not have permission to import taxonomies") - body = TaxonomyImportNewBodySerializer(data=request.data) body.is_valid(raise_exception=True) @@ -299,10 +292,6 @@ def update_import(self, request: Request, **_kwargs) -> HttpResponse: """ Imports tags from the uploaded file to an already created taxonomy. """ - perm = "oel_tagging.import_taxonomy" - if not request.user.has_perm(perm): - raise PermissionDenied("You do not have permission to import taxonomies") - body = TaxonomyImportBodySerializer(data=request.data) body.is_valid(raise_exception=True) diff --git a/openedx_tagging/core/tagging/rules.py b/openedx_tagging/core/tagging/rules.py index 2f9961651..761392448 100644 --- a/openedx_tagging/core/tagging/rules.py +++ b/openedx_tagging/core/tagging/rules.py @@ -171,8 +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) -rules.add_perm("oel_tagging.import_taxonomy", can_change_taxonomy) # Tag rules.add_perm("oel_tagging.add_tag", can_change_tag) From 62e2593e176d2aa4468c49047c0853322e0ffe48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 8 Nov 2023 12:01:57 -0300 Subject: [PATCH 13/17] refactor: return taxonomy after import tags --- .../core/tagging/rest_api/v1/views.py | 20 ++++++++++--------- .../core/tagging/test_views.py | 16 +++++++-------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index d45bb1cc9..1c50b5496 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -4,7 +4,7 @@ from __future__ import annotations from django.db import models -from django.http import Http404, HttpResponse, HttpResponseBadRequest +from django.http import Http404, HttpResponse from rest_framework import mixins, status from rest_framework.decorators import action from rest_framework.exceptions import MethodNotAllowed, PermissionDenied, ValidationError @@ -262,7 +262,7 @@ 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) -> HttpResponse: + def create_import(self, request: Request, **_kwargs) -> Response: """ Creates a new taxonomy and imports the tags from the uploaded file. """ @@ -279,16 +279,17 @@ def create_import(self, request: Request, **_kwargs) -> HttpResponse: import_success = import_tags(taxonomy, file, parser_format) if import_success: - return HttpResponse(status=200) + 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 HttpResponseBadRequest(import_error) + return Response(import_error, status=status.HTTP_400_BAD_REQUEST) except ValueError as e: - return HttpResponseBadRequest(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) -> HttpResponse: + def update_import(self, request: Request, **_kwargs) -> Response: """ Imports tags from the uploaded file to an already created taxonomy. """ @@ -303,12 +304,13 @@ def update_import(self, request: Request, **_kwargs) -> HttpResponse: import_success = import_tags(taxonomy, file, parser_format) if import_success: - return HttpResponse(status=200) + serializer = self.get_serializer(taxonomy) + return Response(serializer.data) else: import_error = get_last_import_log(taxonomy) - return HttpResponseBadRequest(import_error) + return Response(import_error, status=status.HTTP_400_BAD_REQUEST) except ValueError as e: - return HttpResponseBadRequest(e) + return Response(str(e), status=status.HTTP_400_BAD_REQUEST) @view_auth_classes diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 4cf794fdb..869dcba7e 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -1912,14 +1912,15 @@ def test_import(self, file_format: str) -> None: }, format="multipart" ) - assert response.status_code == status.HTTP_200_OK + assert response.status_code == status.HTTP_201_CREATED # Check if the taxonomy was created - taxonomy = Taxonomy.objects.get(name="Imported Taxonomy name") - assert taxonomy.description == "Imported Taxonomy description" + taxonomy = response.data + assert taxonomy["name"] == "Imported Taxonomy name" + assert taxonomy["description"] == "Imported Taxonomy description" # Check if the tags were created - tags = list(Tag.objects.filter(taxonomy=taxonomy)) + tags = list(Tag.objects.filter(taxonomy_id=taxonomy["id"])) assert len(tags) == len(new_tags) for i, tag in enumerate(tags): assert tag.value == new_tags[i]["value"] @@ -2012,7 +2013,7 @@ def test_import_invalid_content(self, file_format) -> None: format="multipart" ) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert f"Invalid '.{file_format}' format:".encode() in response.content + 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() @@ -2163,7 +2164,7 @@ def test_import_invalid_content(self, file_format) -> None: format="multipart" ) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert f"Invalid '.{file_format}' format:".encode() in response.content + assert f"Invalid '.{file_format}' format:" in response.data # Check if the taxonomy was not changed tags = list(Tag.objects.filter(taxonomy=self.taxonomy)) @@ -2198,8 +2199,7 @@ def test_import_free_text(self, file_format) -> None: ) assert response.status_code == status.HTTP_400_BAD_REQUEST - error_msg = f"Invalid taxonomy ({self.taxonomy.id}): You cannot import a free-form taxonomy." - assert response.content == error_msg.encode() + assert response.data == f"Invalid taxonomy ({self.taxonomy.id}): You cannot import a free-form taxonomy." def test_import_no_perm(self) -> None: """ From a827178ccc74d834ed4dbe085da83d04bcd6b598 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 8 Nov 2023 12:17:47 -0300 Subject: [PATCH 14/17] refactor: use rest api to get tags from taxonomies --- .../core/tagging/test_views.py | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 869dcba7e..9622602d9 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -1920,10 +1920,12 @@ def test_import(self, file_format: str) -> None: assert taxonomy["description"] == "Imported Taxonomy description" # Check if the tags were created - tags = list(Tag.objects.filter(taxonomy_id=taxonomy["id"])) + 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"] + assert tag["value"] == new_tags[i]["value"] def test_import_no_file(self) -> None: """ @@ -2096,11 +2098,13 @@ def test_import(self, file_format: str) -> None: assert response.status_code == status.HTTP_200_OK # Check if the tags were created - tags = list(Tag.objects.filter(taxonomy=self.taxonomy)) + 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"] + assert tag["value"] == all_tags[i]["value"] def test_import_no_file(self) -> None: """ @@ -2117,10 +2121,12 @@ def test_import_no_file(self) -> None: assert response.data["file"][0] == "No file was submitted." # Check if the taxonomy was not changed - tags = list(Tag.objects.filter(taxonomy=self.taxonomy)) + 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 + assert tag["value"] == self.old_tags[i].value def test_import_invalid_format(self) -> None: """ @@ -2138,10 +2144,12 @@ def test_import_invalid_format(self) -> None: assert response.data["file"][0] == "File type not supported: invalid" # Check if the taxonomy was not changed - tags = list(Tag.objects.filter(taxonomy=self.taxonomy)) + 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 + assert tag["value"] == self.old_tags[i].value @ddt.data( "csv", @@ -2167,10 +2175,12 @@ def test_import_invalid_content(self, file_format) -> None: assert f"Invalid '.{file_format}' format:" in response.data # Check if the taxonomy was not changed - tags = list(Tag.objects.filter(taxonomy=self.taxonomy)) + 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 + assert tag["value"] == self.old_tags[i].value @ddt.data( "csv", @@ -2201,6 +2211,12 @@ def test_import_free_text(self, file_format) -> None: 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. @@ -2227,7 +2243,9 @@ def test_import_no_perm(self) -> None: assert response.status_code == status.HTTP_403_FORBIDDEN # Check if the taxonomy was not changed - tags = list(Tag.objects.filter(taxonomy=self.taxonomy)) + 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 + assert tag["value"] == self.old_tags[i].value From 4e52c57f09c5964cf32d5d5c64dcf41a5ab0bf22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 8 Nov 2023 13:10:26 -0300 Subject: [PATCH 15/17] docs: fix docstring --- openedx_tagging/core/tagging/rest_api/v1/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index 42aa83271..aa456da46 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -55,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, From a8a0c7e22e15b8771cbfddd01a70939903a6a780 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Thu, 9 Nov 2023 17:11:52 -0300 Subject: [PATCH 16/17] docs: update docstring Co-authored-by: Braden MacDonald --- openedx_tagging/core/tagging/rest_api/v1/serializers.py | 4 ++-- tests/openedx_tagging/core/tagging/test_views.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index 3553e5f6e..62d4ef3ed 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -221,7 +221,7 @@ class TaxonomyTagDeleteBodySerializer(serializers.Serializer): # pylint: disabl class TaxonomyImportBodySerializer(serializers.Serializer): # pylint: disable=abstract-method """ - Serializer of the body for the Taxonomy Import action + Serializer of the body for the Taxonomy Import request """ file = serializers.FileField(required=True) @@ -241,7 +241,7 @@ def validate(self, attrs): class TaxonomyImportNewBodySerializer(TaxonomyImportBodySerializer): # pylint: disable=abstract-method """ - Serializer of the body for the Taxonomy Create and Import action + 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/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 1192933c4..404cb188a 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -2281,7 +2281,7 @@ def test_import_invalid_content(self, file_format) -> None: ) def test_import_free_text(self, file_format) -> None: """ - Tests importing a taxonomy with an invalid file content. + Tests that importing tags into a free text taxonomy is not allowed. """ self.taxonomy.allow_free_text = True self.taxonomy.save() From c4fe95dda4f8767b8e0e8bef7898b96bee65d063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Thu, 9 Nov 2023 20:56:28 -0300 Subject: [PATCH 17/17] chore: bump version --- openedx_learning/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index 711ab06c2..a5b838c53 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"