From 8f18709d5f9935f19ce513eb4cc3672305a20f7c Mon Sep 17 00:00:00 2001 From: Eg0ra <egor.toryshak@saritasa.com> Date: Fri, 25 Oct 2024 10:29:30 +0700 Subject: [PATCH] Add tests for import admin actions --- .../admin/model_admins/export_job_admin.py | 54 +-- .../admin/model_admins/import_job_admin.py | 102 ++--- import_export_extensions/models/import_job.py | 43 +- import_export_extensions/widgets.py | 2 +- .../test_admin/test_export.py | 16 +- .../test_admin/test_import.py | 367 ++++++++++++++++++ .../tests/test_import_job/test_parse_data.py | 12 + 7 files changed, 457 insertions(+), 139 deletions(-) diff --git a/import_export_extensions/admin/model_admins/export_job_admin.py b/import_export_extensions/admin/model_admins/export_job_admin.py index 3efb78f..594a5bb 100644 --- a/import_export_extensions/admin/model_admins/export_job_admin.py +++ b/import_export_extensions/admin/model_admins/export_job_admin.py @@ -41,6 +41,19 @@ class ExportJobAdmin( actions = ( "cancel_jobs", ) + readonly_fields = ( + "export_status", + "traceback", + "file_format_path", + "created", + "export_started", + "export_finished", + "error_message", + "_model", + "resource_path", + "data_file", + "resource_kwargs", + ) def export_data_action( self, @@ -116,43 +129,17 @@ def export_job_progress_view( percent = int(100 / total * current) response_data.update( - dict( - state=job_progress["state"], - percent=percent, - total=total, - current=current, - ), + state=job_progress["state"], + percent=percent, + total=total, + current=current, ) return JsonResponse(response_data) - def get_readonly_fields(self, request, obj=None): - """Return readonly fields. - - Some fields are editable for new ExportJob. - - """ - base_readonly_fields = super().get_readonly_fields(request, obj) - readonly_fields = ( - *base_readonly_fields, - "export_status", - "traceback", - "file_format_path", - "created", - "export_started", - "export_finished", - "error_message", - "_model", - "resource_path", - "data_file", - "resource_kwargs", - ) - - return readonly_fields - def get_fieldsets( self, request: WSGIRequest, - obj: models.ExportJob | None = None, + obj: models.ExportJob, ): """Get fieldsets depending on object status.""" status = ( @@ -203,10 +190,7 @@ def get_fieldsets( }, ) - if ( - not obj - or obj.export_status == models.ExportJob.ExportStatus.CREATED - ): + if obj.export_status == models.ExportJob.ExportStatus.CREATED: return [status, export_params] if obj.export_status == models.ExportJob.ExportStatus.EXPORTED: diff --git a/import_export_extensions/admin/model_admins/import_job_admin.py b/import_export_extensions/admin/model_admins/import_job_admin.py index d524cb4..2c5f4fa 100644 --- a/import_export_extensions/admin/model_admins/import_job_admin.py +++ b/import_export_extensions/admin/model_admins/import_job_admin.py @@ -1,3 +1,4 @@ +import http from django.contrib import admin, messages from django.core.handlers.wsgi import WSGIRequest @@ -42,6 +43,22 @@ class ImportJobAdmin( "cancel_jobs", "confirm_jobs", ) + readonly_fields = ( + "import_status", + "_model", + "created_by", + "traceback", + "_show_results", + "_input_errors", + "created", + "parse_finished", + "import_started", + "import_finished", + "resource_path", + "input_errors_file", + "data_file", + "resource_kwargs", + ) def get_queryset(self, request: WSGIRequest): """Override `get_queryset`. @@ -88,66 +105,36 @@ def import_job_progress_view( id=job_id, ) except self.import_job_model.DoesNotExist as error: - return JsonResponse(dict(validation_error=error.args[0])) + return JsonResponse( + dict(validation_error=error.args[0]), + status=http.HTTPStatus.NOT_FOUND, + ) response_data = dict(status=job.import_status.title()) - if job.import_status in models.ImportJob.progress_statuses: - percent = 0 - total = 0 - current = 0 - info = job.progress["info"] - - if info and info["total"]: - percent = int(100 / info["total"] * info["current"]) - total = info["total"] - current = info["current"] - - response_data.update( - dict( - state=job.progress["state"], - percent=percent, - total=total, - current=current, - ), - ) + if job.import_status not in models.ImportJob.progress_statuses: + return JsonResponse(response_data) + + percent = 0 + total = 0 + current = 0 + job_progress = job.progress + progress_info = job_progress["info"] + + if progress_info and progress_info["total"]: + total = progress_info["total"] + current = progress_info["current"] + percent = int(100 / total * current) + + response_data.update( + state=job_progress["state"], + percent=percent, + total=total, + current=current, + ) return JsonResponse(response_data) - def get_readonly_fields( - self, - request: WSGIRequest, - obj: models.ImportJob | None = None, - ) -> list[str]: - """Return readonly fields. - - Some fields are editable for new ImportJob. - - """ - readonly_fields = [ - "import_status", - "_model", - "created_by", - "traceback_str", - "_show_results", - "_input_errors", - "created", - "parse_finished", - "import_started", - "import_finished", - ] - if obj: - readonly_fields.extend( - [ - "resource_path", - "input_errors_file", - "data_file", - "resource_kwargs", - ], - ) - - return readonly_fields - def _show_results( self, obj: models.ImportJob | None = None, @@ -188,7 +175,7 @@ def _input_errors(self, job: models.ImportJob): def get_fieldsets( self, request: WSGIRequest, - obj: models.ImportJob | None = None, + obj: models.ImportJob, ): """Get fieldsets depending on object status.""" status = ( @@ -249,13 +236,10 @@ def get_fieldsets( }, ) - if not obj: - return [status, import_params] - if obj.import_status == models.ImportJob.ImportStatus.CREATED: return [status, import_params] - if obj.import_status in models.ImportJob.results_statuses: + if obj.import_status in models.ImportJob.success_statuses: return [status, result, data, import_params] if obj.import_status in models.ImportJob.progress_statuses: diff --git a/import_export_extensions/models/import_job.py b/import_export_extensions/models/import_job.py index 3b3cb5c..41cc77b 100644 --- a/import_export_extensions/models/import_job.py +++ b/import_export_extensions/models/import_job.py @@ -249,47 +249,20 @@ def resource(self) -> CeleryResource: @property def progress(self) -> TaskStateInfo | None: - """Return dict with parsing state. - - Example for sync mode:: - - { - 'state': 'PARSING', - 'info': None - } - - Example for background (celery) mode:: - - { - 'state': 'PARSING', - 'info': {'current': 15, 'total': 100} - } - - Possible states: - 1. PENDING - 2. STARTED - 3. SUCCESS - 4. PARSING - custom status that also set importing info - - https://docs.celeryproject.org/en/latest/userguide/tasks.html#states - - """ - if self.import_status not in self.progress_statuses: - return None - - current_task = ( + """Return dict with parsing state.""" + current_task_id = ( self.parse_task_id if self.import_status == self.ImportStatus.PARSING else self.import_task_id ) - if not current_task or current_app.conf.task_always_eager: - return dict( - state=self.import_status.upper(), - info=None, - ) + if ( + not current_task_id + or self.import_status not in self.progress_statuses + ): + return None - return self._get_task_state(current_task) + return self._get_task_state(current_task_id) def _check_import_status_correctness( self, diff --git a/import_export_extensions/widgets.py b/import_export_extensions/widgets.py index 8ea7348..cf06f61 100644 --- a/import_export_extensions/widgets.py +++ b/import_export_extensions/widgets.py @@ -86,7 +86,7 @@ def render( ) -> str: """Return an export representation of a intermediate instances. - For atrists example should be returned something like + For artists example should be returned something like "5:1990-12-12;19:2005-08-16" where 5 is band id diff --git a/test_project/tests/integration_tests/test_admin/test_export.py b/test_project/tests/integration_tests/test_admin/test_export.py index 106aeb9..bc8935e 100644 --- a/test_project/tests/integration_tests/test_admin/test_export.py +++ b/test_project/tests/integration_tests/test_admin/test_export.py @@ -126,11 +126,7 @@ def test_export_progress_with_deleted_export_job( superuser: User, mocker: pytest_mock.MockerFixture, ): - """Test export job admin progress page with deleted export job. - - Check that page available, but return an error message. - - """ + """Test export job admin progress page with deleted export job.""" client.force_login(superuser) mocker.patch("import_export_extensions.tasks.export_data_task.apply_async") @@ -213,13 +209,14 @@ def test_cancel_export_admin_action( "action": "cancel_jobs", "_selected_action": [job.pk], }, + follow=True, ) job.refresh_from_db() - assert response.status_code == status.HTTP_302_FOUND + assert response.status_code == status.HTTP_200_OK assert job.export_status == ExportJob.ExportStatus.CANCELLED assert ( - response.wsgi_request._messages._queued_messages[0].message + response.context["messages"]._loaded_data[0].message == f"Export of {job} canceled" ) export_data_mock.assert_called_once() @@ -246,14 +243,15 @@ def test_cancel_export_admin_action_with_incorrect_export_job_status( "action": "cancel_jobs", "_selected_action": [job.pk], }, + follow=True, ) job.refresh_from_db() - assert response.status_code == status.HTTP_302_FOUND + assert response.status_code == status.HTTP_200_OK assert job.export_status == ExportJob.ExportStatus.EXPORTED assert ( expected_error_message - in response.wsgi_request._messages._queued_messages[0].message + in response.context["messages"]._loaded_data[0].message ) revoke_mock.assert_not_called() diff --git a/test_project/tests/integration_tests/test_admin/test_import.py b/test_project/tests/integration_tests/test_admin/test_import.py index 8b36fef..36f4ab8 100644 --- a/test_project/tests/integration_tests/test_admin/test_import.py +++ b/test_project/tests/integration_tests/test_admin/test_import.py @@ -8,8 +8,11 @@ from rest_framework import status import pytest +import pytest_mock +from celery import states from import_export_extensions.models import ImportJob +from test_project.fake_app.factories import ArtistImportJobFactory @pytest.mark.usefixtures("existing_artist") @@ -112,3 +115,367 @@ def test_import_admin_has_same_formats( import_response_form.fields["format"].choices == import_response_result_form.fields["format"].choices ) + + +@pytest.mark.django_db(transaction=True) +def test_import_progress_during_import( + client: Client, + superuser: User, + mocker: pytest_mock.MockerFixture, +): + """Test import job admin progress page during import.""" + client.force_login(superuser) + + # Prepare data to imitate intermediate task state + fake_progress_info = { + "current": 2, + "total": 3, + } + mocker.patch( + "celery.result.AsyncResult.info", + new=fake_progress_info, + ) + expected_percent = int( + fake_progress_info["current"] / fake_progress_info["total"] * 100, + ) + + artist_import_job = ArtistImportJobFactory( + skip_parse_step=True, + ) + artist_import_job.import_status = ImportJob.ImportStatus.IMPORTING + artist_import_job.save() + + response = client.post( + path=reverse( + "admin:import_job_progress", + kwargs={"job_id": artist_import_job.pk}, + ), + ) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == { + "status": ImportJob.ImportStatus.IMPORTING.title(), + "state": "SUCCESS", + "percent": expected_percent, + **fake_progress_info, + } + + +@pytest.mark.django_db(transaction=True) +def test_import_progress_after_complete_import( + client: Client, + superuser: User, +): + """Test import job admin progress page after complete import.""" + client.force_login(superuser) + + artist_import_job = ArtistImportJobFactory( + skip_parse_step=True, + ) + artist_import_job.refresh_from_db() + + response = client.post( + path=reverse( + "admin:import_job_progress", + kwargs={"job_id": artist_import_job.pk}, + ), + ) + assert response.status_code == status.HTTP_200_OK + assert response.json() == { + "status": artist_import_job.import_status.title(), + } + + +@pytest.mark.django_db(transaction=True) +def test_import_progress_with_deleted_import_job( + client: Client, + superuser: User, + mocker: pytest_mock.MockerFixture, +): + """Test import job admin progress page with deleted import job.""" + client.force_login(superuser) + + mocker.patch("import_export_extensions.tasks.import_data_task.apply_async") + artist_import_job = ArtistImportJobFactory() + job_id = artist_import_job.id + artist_import_job.delete() + + expected_error_message = "ImportJob matching query does not exist." + + response = client.post( + path=reverse( + "admin:import_job_progress", + kwargs={ + "job_id": job_id, + }, + ), + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.json()["validation_error"] == expected_error_message + + +@pytest.mark.django_db(transaction=True) +def test_import_progress_with_failed_celery_task( + client: Client, + superuser: User, + mocker: pytest_mock.MockerFixture, +): + """Test than after celery fail ImportJob will be in import error status.""" + client.force_login(superuser) + + expected_error_message = "Mocked Error Message" + mocker.patch( + "celery.result.AsyncResult.state", + new=states.FAILURE, + ) + mocker.patch( + "celery.result.AsyncResult.info", + new=ValueError(expected_error_message), + ) + artist_import_job = ArtistImportJobFactory() + artist_import_job.refresh_from_db() + artist_import_job.confirm_import() + artist_import_job.import_status = ImportJob.ImportStatus.IMPORTING + artist_import_job.save() + + response = client.post( + path=reverse( + "admin:import_job_progress", + kwargs={ + "job_id": artist_import_job.id, + }, + ), + ) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["state"] == states.FAILURE + artist_import_job.refresh_from_db() + assert ( + artist_import_job.import_status == ImportJob.ImportStatus.IMPORT_ERROR + ) + + +@pytest.mark.django_db(transaction=True) +def test_cancel_import_admin_action( + client: Client, + superuser: User, + mocker: pytest_mock.MockerFixture, +): + """Test `cancel_import` via admin action.""" + client.force_login(superuser) + + revoke_mock = mocker.patch("celery.current_app.control.revoke") + import_data_mock = mocker.patch( + "import_export_extensions.models.ImportJob.import_data", + ) + artist_import_job = ArtistImportJobFactory() + artist_import_job.refresh_from_db() + artist_import_job.confirm_import() + + response = client.post( + reverse("admin:import_export_extensions_importjob_changelist"), + data={ + "action": "cancel_jobs", + "_selected_action": [artist_import_job.pk], + }, + follow=True, + ) + artist_import_job.refresh_from_db() + + assert response.status_code == status.HTTP_200_OK + assert artist_import_job.import_status == ImportJob.ImportStatus.CANCELLED + assert ( + response.context["messages"]._loaded_data[0].message + == f"Import of {artist_import_job} canceled" + ) + import_data_mock.assert_called_once() + revoke_mock.assert_called_once_with( + artist_import_job.import_task_id, + terminate=True, + ) + + +@pytest.mark.django_db(transaction=True) +def test_cancel_import_admin_action_with_incorrect_import_job_status( + client: Client, + superuser: User, + mocker: pytest_mock.MockerFixture, +): + """Test `cancel_import` via admin action with wrong import job status.""" + client.force_login(superuser) + + revoke_mock = mocker.patch("celery.current_app.control.revoke") + artist_import_job = ArtistImportJobFactory() + + expected_error_message = ( + f"ImportJob with id {artist_import_job.pk} has incorrect status" + ) + + response = client.post( + reverse("admin:import_export_extensions_importjob_changelist"), + data={ + "action": "cancel_jobs", + "_selected_action": [artist_import_job.pk], + }, + follow=True, + ) + artist_import_job.refresh_from_db() + + assert response.status_code == status.HTTP_200_OK + assert artist_import_job.import_status == ImportJob.ImportStatus.PARSED + assert ( + expected_error_message + in response.context["messages"]._loaded_data[0].message + ) + revoke_mock.assert_not_called() + + +@pytest.mark.django_db(transaction=True) +def test_confirm_import_admin_action( + client: Client, + superuser: User, + mocker: pytest_mock.MockerFixture, +): + """Test `confirm_import` via admin action.""" + client.force_login(superuser) + + import_data_mock = mocker.patch( + "import_export_extensions.models.ImportJob.import_data", + ) + artist_import_job = ArtistImportJobFactory() + artist_import_job.refresh_from_db() + + response = client.post( + reverse("admin:import_export_extensions_importjob_changelist"), + data={ + "action": "confirm_jobs", + "_selected_action": [artist_import_job.pk], + }, + follow=True, + ) + artist_import_job.refresh_from_db() + + assert response.status_code == status.HTTP_200_OK + assert artist_import_job.import_status == ImportJob.ImportStatus.CONFIRMED + assert ( + response.context["messages"]._loaded_data[0].message + == f"Import of {artist_import_job} confirmed" + ) + import_data_mock.assert_called_once() + + +@pytest.mark.django_db(transaction=True) +def test_confirm_import_admin_action_with_incorrect_import_job_status( + client: Client, + superuser: User, +): + """Test `confirm_import` via admin action with wrong import job status.""" + client.force_login(superuser) + + artist_import_job = ArtistImportJobFactory() + artist_import_job.import_status = ImportJob.ImportStatus.CANCELLED + artist_import_job.save() + + expected_error_message = ( + f"ImportJob with id {artist_import_job.pk} has incorrect status" + ) + + response = client.post( + reverse("admin:import_export_extensions_importjob_changelist"), + data={ + "action": "confirm_jobs", + "_selected_action": [artist_import_job.pk], + }, + follow=True, + ) + artist_import_job.refresh_from_db() + + assert response.status_code == status.HTTP_200_OK + assert artist_import_job.import_status == ImportJob.ImportStatus.CANCELLED + assert ( + expected_error_message + in response.context["messages"]._loaded_data[0].message + ) + + +@pytest.mark.parametrize( + argnames=["job_status", "expected_fieldsets"], + argvalues=[ + pytest.param( + ImportJob.ImportStatus.CREATED, + tuple(), + id="Get fieldsets for job in status CREATED", + ), + pytest.param( + ImportJob.ImportStatus.IMPORTED, + ( + ("_show_results",), + ( + "input_errors_file", + "_input_errors", + ), + ), + id="Get fieldsets for job in status IMPORTED", + ), + pytest.param( + ImportJob.ImportStatus.IMPORTING, + ( + ( + "import_status", + "import_progressbar", + ), + ), + id="Get fieldsets for job in status IMPORTING", + ), + pytest.param( + ImportJob.ImportStatus.IMPORT_ERROR, + (("traceback",),), + id="Get fieldsets for job in status IMPORT_ERROR", + ), + ], +) +def test_get_fieldsets_by_import_job_status( + client: Client, + superuser: User, + job_status: ImportJob.ImportStatus, + expected_fieldsets: tuple[tuple[str]], + mocker: pytest_mock.MockerFixture, +): + """Test that appropriate fieldsets returned for different job statuses.""" + client.force_login(superuser) + + mocker.patch( + "import_export_extensions.models.ImportJob.import_data", + ) + artist_import_job = ArtistImportJobFactory() + artist_import_job.import_status = job_status + artist_import_job.save() + + response = client.get( + reverse( + "admin:import_export_extensions_importjob_change", + kwargs={"object_id": artist_import_job.pk}, + ), + ) + + fieldsets = response.context["adminform"].fieldsets + fields = [fields["fields"] for _, fields in fieldsets] + + assert tuple(fields) == ( + ( + "import_status", + "_model", + "created_by", + "created", + "parse_finished", + "import_started", + "import_finished", + ), + *expected_fieldsets, + ( + "data_file", + "resource_path", + "resource_kwargs", + ), + ) diff --git a/test_project/tests/test_import_job/test_parse_data.py b/test_project/tests/test_import_job/test_parse_data.py index 166edf8..b3b2e92 100644 --- a/test_project/tests/test_import_job/test_parse_data.py +++ b/test_project/tests/test_import_job/test_parse_data.py @@ -165,3 +165,15 @@ def test_parse_data_invalid_row_file( assert import_job.import_status == expected_status if expected_status == ImportJob.ImportStatus.INPUT_ERROR: assert import_job.input_errors_file is not None + + +def test_parse_data_with_incorrect_input_file_format(): + """Test that if file extension is incorrect.""" + incorrect_file_ext = ".incorrect_ext" + expected_error_message = f"Incorrect import format: {incorrect_file_ext}." + import_job: ImportJob = ArtistImportJobFactory( + data_file=f"input_file{incorrect_file_ext}", + ) + import_job.parse_data() + assert import_job.import_status == ImportJob.ImportStatus.PARSE_ERROR + assert expected_error_message in import_job.error_message