From 7f953d89f6d838ff8b7cdfe1b15d12bc6d399333 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 6 Dec 2023 19:51:45 -0300 Subject: [PATCH 1/5] feat: add `plan_only` param to api and create import/plan endpoint --- openedx_learning/__init__.py | 2 +- .../core/tagging/import_export/api.py | 22 ++- .../core/tagging/import_export/tasks.py | 5 +- .../core/tagging/rest_api/v1/serializers.py | 41 +++- .../core/tagging/rest_api/v1/views.py | 50 ++++- .../core/tagging/import_export/test_api.py | 35 +++- .../core/tagging/import_export/test_tasks.py | 6 +- .../tagging/import_export/test_template.py | 5 +- .../core/tagging/test_views.py | 179 ++++++++++++++++++ 9 files changed, 314 insertions(+), 31 deletions(-) diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index 8b9f3bfcf..b2647f381 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -1,4 +1,4 @@ """ Open edX Learning ("Learning Core"). """ -__version__ = "0.4.0" +__version__ = "0.4.1" diff --git a/openedx_tagging/core/tagging/import_export/api.py b/openedx_tagging/core/tagging/import_export/api.py index 7d6ae5e75..34c496878 100644 --- a/openedx_tagging/core/tagging/import_export/api.py +++ b/openedx_tagging/core/tagging/import_export/api.py @@ -58,7 +58,8 @@ def import_tags( file: BinaryIO, parser_format: ParserFormat, replace=False, -) -> bool: + plan_only=False, +) -> tuple[bool, TagImportTask, TagImportPlan | None]: """ Execute the necessary actions to import the tags from `file` @@ -73,6 +74,8 @@ def import_tags( Ex. Given a taxonomy with `tag_1`, `tag_2` and `tag_3`. If there is only `tag_1` in the file (regardless of action), then `tag_2` and `tag_3` will be deleted if `replace=True` + + Set `plan_only` to True to only generate the actions and not execute them. """ _import_validations(taxonomy) @@ -97,7 +100,7 @@ def import_tags( # Check if there are errors in the parse if errors: task.handle_parser_errors(errors) - return False + return False, task, None task.log_parser_end() @@ -109,16 +112,19 @@ def import_tags( if tag_import_plan.errors: task.handle_plan_errors() - return False + return False, task, tag_import_plan + + if not plan_only: + task.log_start_execute() + tag_import_plan.execute(task) - task.log_start_execute() - tag_import_plan.execute(task) task.end_success() - return True - except Exception as exception: + + return True, task, tag_import_plan + except Exception as exception: # pylint: disable=broad-exception-caught # Log any exception task.log_exception(exception) - return False + return False, task, None def get_last_import_status(taxonomy: Taxonomy) -> TagImportTaskState: diff --git a/openedx_tagging/core/tagging/import_export/tasks.py b/openedx_tagging/core/tagging/import_export/tasks.py index f351ea8ec..3a79ab5ca 100644 --- a/openedx_tagging/core/tagging/import_export/tasks.py +++ b/openedx_tagging/core/tagging/import_export/tasks.py @@ -1,6 +1,8 @@ """ Import and export celery tasks """ +from __future__ import annotations + from io import BytesIO from celery import shared_task # type: ignore[import] @@ -8,6 +10,7 @@ import openedx_tagging.core.tagging.import_export.api as import_export_api from ..models import Taxonomy +from .import_plan import TagImportPlan, TagImportTask from .parsers import ParserFormat @@ -17,7 +20,7 @@ def import_tags_task( file: BytesIO, parser_format: ParserFormat, replace=False, -) -> bool: +) -> tuple[bool, TagImportTask, TagImportPlan | None]: """ Runs import on a celery task """ diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index 211f3d534..a875c98b9 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -11,7 +11,7 @@ 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 +from openedx_tagging.core.tagging.models import ObjectTag, Tag, TagImportTask, Taxonomy class TaxonomyListQueryParamsSerializer(serializers.Serializer): # pylint: disable=abstract-method @@ -257,3 +257,42 @@ class TaxonomyImportNewBodySerializer(TaxonomyImportBodySerializer): # pylint: """ taxonomy_name = serializers.CharField(required=True) taxonomy_description = serializers.CharField(default="") + + +class TagImportTaskSerializer(serializers.ModelSerializer): + """ + Serializer for the TagImportTask model. + """ + class Meta: + model = TagImportTask + fields = [ + "id", + "log", + "status", + "creation_date", + ] + + +class TaxonomyImportPlanResponseSerializer(serializers.Serializer): + """ + Serializer for the response of the Taxonomy Import Plan request + """ + task = TagImportTaskSerializer() + plan = serializers.SerializerMethodField() + error = serializers.CharField(required=False, allow_null=True) + + def get_plan(self, obj): + """ + Returns the plan of the import + """ + plan = obj.get("plan", None) + if plan: + return plan.plan() + + return None + + def update(self, instance, validated_data): + raise RuntimeError('`update()` is not supported by the TagImportTask serializer.') + + def create(self, validated_data): + raise RuntimeError('`create()` is not supported by the TagImportTask serializer.') diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index 591e95e15..99d8f8bfb 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -26,7 +26,7 @@ update_tag_in_taxonomy, ) from ...data import TagDataQuerySet -from ...import_export.api import export_tags, get_last_import_log, import_tags +from ...import_export.api import export_tags, import_tags from ...import_export.parsers import ParserFormat from ...models import Taxonomy from ...rules import ObjectTagPermissionItem @@ -42,6 +42,7 @@ TaxonomyExportQueryParamsSerializer, TaxonomyImportBodySerializer, TaxonomyImportNewBodySerializer, + TaxonomyImportPlanResponseSerializer, TaxonomyListQueryParamsSerializer, TaxonomySerializer, TaxonomyTagCreateBodySerializer, @@ -192,6 +193,17 @@ class TaxonomyView(ModelViewSet): * 200 - Success * 400 - Bad request * 403 - Permission denied + + **Plan Import/Update Taxonomy Example Requests** + PUT /tagging/rest_api/v1/taxonomy/:pk/tags/import/plan + { + "file": , + } + + **Plan Import/Update Taxonomy Query Returns** + * 200 - Success + * 400 - Bad request + * 403 - Permission denied """ # System taxonomies use negative numbers for their primary keys @@ -279,15 +291,14 @@ def create_import(self, request: Request, **_kwargs) -> Response: taxonomy = create_taxonomy(taxonomy_name, taxonomy_description) try: - import_success = import_tags(taxonomy, file, parser_format) + import_success, task, _plan = 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) + return Response(task.log, status=status.HTTP_400_BAD_REQUEST) except ValueError as e: return Response(str(e), status=status.HTTP_400_BAD_REQUEST) @@ -305,17 +316,42 @@ def update_import(self, request: Request, **_kwargs) -> Response: taxonomy = self.get_object() try: - import_success = import_tags(taxonomy, file, parser_format, replace=True) + import_success, task, _plan = import_tags(taxonomy, file, parser_format, replace=True) 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) + return Response(task.log, 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/plan", methods=["put"]) + def plan_update_import(self, request: Request, **_kwargs) -> Response: + """ + Plan import tags from the uploaded file to an already created taxonomy, + overwriting any existing tags. + """ + 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, task, plan = import_tags(taxonomy, file, parser_format, replace=True, plan_only=True) + + if import_success: + serializer = TaxonomyImportPlanResponseSerializer({"task": task, "plan": plan}) + return Response(serializer.data) + else: + serializer = TaxonomyImportPlanResponseSerializer({"task": task, "plan": plan, "error": task.log}) + return Response(serializer.data, status=status.HTTP_400_BAD_REQUEST) + except ValueError as e: + serializer = TaxonomyImportPlanResponseSerializer({"error": str(e)}) + return Response({"error": str(e)}, status=status.HTTP_400_BAD_REQUEST) + @view_auth_classes class ObjectTagView( diff --git a/tests/openedx_tagging/core/tagging/import_export/test_api.py b/tests/openedx_tagging/core/tagging/import_export/test_api.py index 039933d44..a02a939c2 100644 --- a/tests/openedx_tagging/core/tagging/import_export/test_api.py +++ b/tests/openedx_tagging/core/tagging/import_export/test_api.py @@ -91,37 +91,46 @@ def test_import_export_validations(self) -> None: def test_with_python_error(self) -> None: self.file.close() - assert not import_export_api.import_tags( + result, task, _plan = import_export_api.import_tags( self.taxonomy, self.file, self.parser_format, ) + assert not result status = import_export_api.get_last_import_status(self.taxonomy) log = import_export_api.get_last_import_log(self.taxonomy) + assert status == TagImportTaskState(task.status) assert status == TagImportTaskState.ERROR + assert log == task.log assert "ValueError('I/O operation on closed file.')" in log def test_with_parser_error(self) -> None: - assert not import_export_api.import_tags( + result, task, _plan = import_export_api.import_tags( self.taxonomy, self.invalid_parser_file, self.parser_format, ) + assert not result status = import_export_api.get_last_import_status(self.taxonomy) log = import_export_api.get_last_import_log(self.taxonomy) + assert status == TagImportTaskState(task.status) assert status == TagImportTaskState.ERROR + assert log == task.log assert "Starting to load data from file" in log assert "Invalid '.json' format" in log def test_with_plan_errors(self) -> None: - assert not import_export_api.import_tags( + result, task, _plan = import_export_api.import_tags( self.taxonomy, self.invalid_plan_file, self.parser_format, ) + assert not result status = import_export_api.get_last_import_status(self.taxonomy) log = import_export_api.get_last_import_log(self.taxonomy) + assert status == TagImportTaskState(task.status) assert status == TagImportTaskState.ERROR + assert log == task.log assert "Starting to load data from file" in log assert "Load data finished" in log assert "Starting plan actions" in log @@ -129,15 +138,18 @@ def test_with_plan_errors(self) -> None: assert "Conflict with 'create'" in log def test_valid(self) -> None: - assert import_export_api.import_tags( + result, task, _plan = import_export_api.import_tags( self.taxonomy, self.file, self.parser_format, replace=True, ) + assert result status = import_export_api.get_last_import_status(self.taxonomy) log = import_export_api.get_last_import_log(self.taxonomy) + assert status == TagImportTaskState(task.status) assert status == TagImportTaskState.SUCCESS + assert log == task.log assert "Starting to load data from file" in log assert "Load data finished" in log assert "Starting plan actions" in log @@ -146,33 +158,37 @@ def test_valid(self) -> None: assert "Execution finished" in log def test_start_task_after_error(self) -> None: - assert not import_export_api.import_tags( + result, _task, _plan = import_export_api.import_tags( self.taxonomy, self.invalid_parser_file, self.parser_format, ) - assert import_export_api.import_tags( + assert not result + result, _task, _plan = import_export_api.import_tags( self.taxonomy, self.file, self.parser_format, ) + assert result def test_start_task_after_success(self) -> None: - assert import_export_api.import_tags( + result, _task, _plan = import_export_api.import_tags( self.taxonomy, self.file, self.parser_format, ) + assert result # Opening again the file json_data = {"tags": self.tags} self.file = BytesIO(json.dumps(json_data).encode()) - assert import_export_api.import_tags( + result, _task, _plan = import_export_api.import_tags( self.taxonomy, self.file, self.parser_format, ) + assert result def test_import_with_export_output(self) -> None: for parser_format in ParserFormat: @@ -183,11 +199,12 @@ def test_import_with_export_output(self) -> None: file = BytesIO(output.encode()) new_taxonomy = Taxonomy(name="New taxonomy") new_taxonomy.save() - assert import_export_api.import_tags( + result, _task, _plan = import_export_api.import_tags( new_taxonomy, file, parser_format, ) + assert result old_tags = self.taxonomy.tag_set.all() assert len(old_tags) == new_taxonomy.tag_set.count() diff --git a/tests/openedx_tagging/core/tagging/import_export/test_tasks.py b/tests/openedx_tagging/core/tagging/import_export/test_tasks.py index c3c3e3764..7091382fe 100644 --- a/tests/openedx_tagging/core/tagging/import_export/test_tasks.py +++ b/tests/openedx_tagging/core/tagging/import_export/test_tasks.py @@ -23,9 +23,11 @@ def test_import_tags_task(self): replace = True with patch('openedx_tagging.core.tagging.import_export.api.import_tags') as mock_import_tags: - mock_import_tags.return_value = True + mock_import_tags.return_value = (True, None, None) - result = import_export_tasks.import_tags_task(self.taxonomy, file, parser_format, replace) + result, _result_task, _result_plan = import_export_tasks.import_tags_task( + self.taxonomy, file, parser_format, replace + ) self.assertTrue(result) mock_import_tags.assert_called_once_with(self.taxonomy, file, parser_format, replace) diff --git a/tests/openedx_tagging/core/tagging/import_export/test_template.py b/tests/openedx_tagging/core/tagging/import_export/test_template.py index 9947de6bd..89889f956 100644 --- a/tests/openedx_tagging/core/tagging/import_export/test_template.py +++ b/tests/openedx_tagging/core/tagging/import_export/test_template.py @@ -41,12 +41,13 @@ def open_template_file(self, template_file): @ddt.unpack def test_import_template(self, template_file, parser_format): with self.open_template_file(template_file) as import_file: - assert import_api.import_tags( + result, _task, _plan = import_api.import_tags( self.taxonomy, import_file, parser_format, replace=True, - ), import_api.get_last_import_log(self.taxonomy) + ) + assert result, import_api.get_last_import_log(self.taxonomy) assert pretty_format_tags(get_tags(self.taxonomy), external_id=True) == [ 'Electronic instruments (ELECTRIC) (None) (children: 2)', diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 00151f307..062e83a0d 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -32,6 +32,7 @@ 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_TAGS_IMPORT_PLAN_URL = "/tagging/rest_api/v1/taxonomies/{pk}/tags/import/plan/" TAXONOMY_CREATE_IMPORT_URL = "/tagging/rest_api/v1/taxonomies/import/" @@ -2399,6 +2400,39 @@ def test_import(self, file_format: str) -> None: for i, tag in enumerate(tags): assert tag["value"] == new_tags[i]["value"] + @ddt.data( + "csv", + "json", + ) + def test_import_plan(self, file_format: str) -> None: + """ + Tests planning import a valid taxonomy file. + """ + url = TAXONOMY_TAGS_IMPORT_PLAN_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 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_no_file(self) -> None: """ Tests importing a taxonomy without a file. @@ -2421,6 +2455,28 @@ def test_import_no_file(self) -> None: for i, tag in enumerate(tags): assert tag["value"] == self.old_tags[i].value + def test_import_plan_no_file(self) -> None: + """ + Tests planning impprt a taxonomy without a file. + """ + url = TAXONOMY_TAGS_IMPORT_PLAN_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. @@ -2444,6 +2500,29 @@ def test_import_invalid_format(self) -> None: for i, tag in enumerate(tags): assert tag["value"] == self.old_tags[i].value + def test_import_plan_invalid_format(self) -> None: + """ + Tests planning import a taxonomy with an invalid file format. + """ + url = TAXONOMY_TAGS_IMPORT_PLAN_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", @@ -2475,6 +2554,37 @@ 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_plan_invalid_content(self, file_format) -> None: + """ + Tests planning import a taxonomy with an invalid file content. + """ + url = TAXONOMY_TAGS_IMPORT_PLAN_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["error"] + + # 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", @@ -2510,6 +2620,42 @@ def test_import_free_text(self, file_format) -> None: tags = response.data["results"] assert len(tags) == 0 + @ddt.data( + "csv", + "json", + ) + def test_import_plan_free_text(self, file_format) -> None: + """ + Tests that planning import tags into a free text taxonomy is not allowed. + """ + self.taxonomy.allow_free_text = True + self.taxonomy.save() + url = TAXONOMY_TAGS_IMPORT_PLAN_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 + expected_message = f"Invalid taxonomy ({self.taxonomy.id}): You cannot import a free-form taxonomy." + assert response.data["error"] == expected_message + + # 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. @@ -2542,3 +2688,36 @@ def test_import_no_perm(self) -> None: assert len(tags) == len(self.old_tags) for i, tag in enumerate(tags): assert tag["value"] == self.old_tags[i].value + + def test_import_plan_no_perm(self) -> None: + """ + Tests planning import a taxonomy using a user without permission. + """ + url = TAXONOMY_TAGS_IMPORT_PLAN_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 From fd7d98e27423691ea68bbcee266d9abfb3b7a115 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 8 Dec 2023 07:47:05 -0300 Subject: [PATCH 2/5] test: fix docstring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Chris Chávez --- tests/openedx_tagging/core/tagging/test_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 062e83a0d..b2ae795b3 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -2457,7 +2457,7 @@ def test_import_no_file(self) -> None: def test_import_plan_no_file(self) -> None: """ - Tests planning impprt a taxonomy without a file. + Tests planning import a taxonomy without a file. """ url = TAXONOMY_TAGS_IMPORT_PLAN_URL.format(pk=self.taxonomy.id) self.client.force_authenticate(user=self.staff) From fc3a6daf0f2c9f3a9f7796d67fcc374212a11b03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 8 Dec 2023 07:54:59 -0300 Subject: [PATCH 3/5] test: refactor to remove duplicated code --- .../core/tagging/test_views.py | 84 +++++-------------- 1 file changed, 21 insertions(+), 63 deletions(-) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index b2ae795b3..8ef026212 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -2349,6 +2349,17 @@ class TestImportTagsView(ImportTaxonomyMixin, APITestCase): """ Tests the taxonomy import tags action. """ + def _check_taxonomy_not_changed(self) -> None: + """ + Checks if the self.taxonomy have the original tags. + """ + 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 setUp(self): ImportTaxonomyMixin.setUp(self) @@ -2425,13 +2436,7 @@ def test_import_plan(self, file_format: str) -> None: ) assert response.status_code == status.HTTP_200_OK - # 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 + self._check_taxonomy_not_changed() def test_import_no_file(self) -> None: """ @@ -2447,13 +2452,7 @@ def test_import_no_file(self) -> None: 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 + self._check_taxonomy_not_changed() def test_import_plan_no_file(self) -> None: """ @@ -2469,13 +2468,7 @@ def test_import_plan_no_file(self) -> None: 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 + self._check_taxonomy_not_changed() def test_import_invalid_format(self) -> None: """ @@ -2492,13 +2485,7 @@ def test_import_invalid_format(self) -> None: 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 + self._check_taxonomy_not_changed() def test_import_plan_invalid_format(self) -> None: """ @@ -2515,13 +2502,7 @@ def test_import_plan_invalid_format(self) -> None: 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 + self._check_taxonomy_not_changed() @ddt.data( "csv", @@ -2546,13 +2527,7 @@ def test_import_invalid_content(self, file_format) -> None: 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 + self._check_taxonomy_not_changed() @ddt.data( "csv", @@ -2577,13 +2552,7 @@ def test_import_plan_invalid_content(self, file_format) -> None: assert response.status_code == status.HTTP_400_BAD_REQUEST assert f"Invalid '.{file_format}' format:" in response.data["error"] - # 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 + self._check_taxonomy_not_changed() @ddt.data( "csv", @@ -2681,13 +2650,7 @@ def test_import_no_perm(self) -> None: ) 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 + self._check_taxonomy_not_changed() def test_import_plan_no_perm(self) -> None: """ @@ -2714,10 +2677,5 @@ def test_import_plan_no_perm(self) -> None: ) 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 + self._check_taxonomy_not_changed() + From e0fe6e8eb4cd6a03042707027d225593a1875fd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 8 Dec 2023 08:06:20 -0300 Subject: [PATCH 4/5] test: check result data on import plan --- tests/openedx_tagging/core/tagging/test_views.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 8ef026212..b6b6d63ac 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -2435,6 +2435,17 @@ def test_import_plan(self, file_format: str) -> None: format="multipart" ) assert response.status_code == status.HTTP_200_OK + assert response.data["error"] is None + assert response.data["task"]["status"] == "success" + expected_plan = "Import plan for Test import taxonomy\n" \ + + "--------------------------------\n" \ + + "#1: Create a new tag with values (external_id=tag_1, value=Tag 1, parent_id=None).\n" \ + + "#2: Create a new tag with values (external_id=tag_2, value=Tag 2, parent_id=None).\n" \ + + "#3: Create a new tag with values (external_id=tag_3, value=Tag 3, parent_id=None).\n" \ + + "#4: Create a new tag with values (external_id=tag_4, value=Tag 4, parent_id=None).\n" \ + + "#5: Delete tag (external_id=old_tag_1)\n" \ + + "#6: Delete tag (external_id=old_tag_2)\n" + assert response.data["plan"] == expected_plan self._check_taxonomy_not_changed() From e9b4071b1520f6644e3326fad07489941e42f1f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 8 Dec 2023 08:09:26 -0300 Subject: [PATCH 5/5] test: fix pylint --- tests/openedx_tagging/core/tagging/test_views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index b6b6d63ac..6d1cf0d10 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -2689,4 +2689,3 @@ def test_import_plan_no_perm(self) -> None: assert response.status_code == status.HTTP_403_FORBIDDEN self._check_taxonomy_not_changed() -