diff --git a/HISTORY.rst b/HISTORY.rst index 1206331..9b55517 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -2,10 +2,12 @@ History ======= -0.4.2 (2023-10-20) +UNRELEASED ------------------ -* Add base model for `ImportJob` and `ExportJob` +* Add base model for ``ImportJob`` and ``ExportJob`` * Extend import results template: show validation errors in table +* Add force-import feature: skip rows with errors while importing +* Add ``skip_parse_step`` parameter for importing API 0.4.1 (2023-09-25) ------------------ diff --git a/docs/_static/images/force_import_admin.png b/docs/_static/images/force_import_admin.png new file mode 100644 index 0000000..0f85357 Binary files /dev/null and b/docs/_static/images/force_import_admin.png differ diff --git a/docs/_static/images/force_import_results.png b/docs/_static/images/force_import_results.png new file mode 100644 index 0000000..c210bd4 Binary files /dev/null and b/docs/_static/images/force_import_results.png differ diff --git a/docs/_static/images/start_api.png b/docs/_static/images/start_api.png new file mode 100644 index 0000000..ce0834d Binary files /dev/null and b/docs/_static/images/start_api.png differ diff --git a/docs/extensions.rst b/docs/extensions.rst index 6ca8987..a1a5914 100644 --- a/docs/extensions.rst +++ b/docs/extensions.rst @@ -178,6 +178,38 @@ for filtering: .. figure:: _static/images/filters-openapi.png +------------ +Force import +------------ + +This package provides *force import* feature. When force import is enabled, +then rows with errors will be skipped and rest of the rows will be handled. + +__________ +Admin page +__________ + +This functionality available in admin: + +.. figure:: _static/images/force_import_admin.png + +In case if some rows contain any errors it will be reported on parse/import stage: + +.. figure:: _static/images/force_import_results.png + +___ +API +___ + +In api there're 2 fields: ``force_import`` and ``skip_parse_step``. + +- ``force_import`` allows you to skip rows with errors. + +- ``skip_parse_step`` allows you to run the import task immediately, without having to call the ``confirm`` endpoint. + +.. image:: _static/images/start_api.png + + ------- Widgets ------- diff --git a/import_export_extensions/admin/forms/__init__.py b/import_export_extensions/admin/forms/__init__.py index 783e864..8d2e385 100644 --- a/import_export_extensions/admin/forms/__init__.py +++ b/import_export_extensions/admin/forms/__init__.py @@ -1,2 +1,3 @@ -from .export_admin_form import ExportJobAdminForm -from .import_admin_form import ImportJobAdminForm +from .export_job_admin_form import ExportJobAdminForm +from .import_admin_form import ForceImportForm +from .import_job_admin_form import ImportJobAdminForm diff --git a/import_export_extensions/admin/forms/export_admin_form.py b/import_export_extensions/admin/forms/export_job_admin_form.py similarity index 100% rename from import_export_extensions/admin/forms/export_admin_form.py rename to import_export_extensions/admin/forms/export_job_admin_form.py diff --git a/import_export_extensions/admin/forms/import_admin_form.py b/import_export_extensions/admin/forms/import_admin_form.py index efd2b04..2472df0 100644 --- a/import_export_extensions/admin/forms/import_admin_form.py +++ b/import_export_extensions/admin/forms/import_admin_form.py @@ -1,57 +1,11 @@ from django import forms -from django.urls import reverse -from ... import models -from ..widgets import ProgressBarWidget +from import_export import forms as base_forms -class ImportJobAdminForm(forms.ModelForm): - """Admin form for ``ImportJob`` model. - - Adds custom `import_progressbar` field that displays current import - progress using AJAX requests to specified endpoint. Fields widget is - defined in `__init__` method. - - """ - - import_progressbar = forms.Field( +class ForceImportForm(base_forms.ImportForm): + """Import form with `force_import` option.""" + force_import = forms.BooleanField( required=False, + initial=False, ) - - def __init__( - self, - instance: models.ImportJob, - *args, - **kwargs, - ): - """Provide `import_progressbar` widget the ``ImportJob`` instance.""" - super().__init__(*args, instance=instance, **kwargs) - url_name = "admin:import_job_progress" - self.fields["import_progressbar"].label = ( - "Import progress" if - instance.import_status == models.ImportJob.ImportStatus.IMPORTING - else "Parsing progress" - ) - self.fields["import_progressbar"].widget = ProgressBarWidget( - job=instance, - url=reverse(url_name, args=(instance.id,)), - ) - - class Meta: - fields = ( - "import_status", - "resource_path", - "data_file", - "resource_kwargs", - "traceback", - "error_message", - "result", - "parse_task_id", - "import_task_id", - "parse_finished", - "import_started", - "import_finished", - "created_by", - "created", - "modified", - ) diff --git a/import_export_extensions/admin/forms/import_job_admin_form.py b/import_export_extensions/admin/forms/import_job_admin_form.py new file mode 100644 index 0000000..efd2b04 --- /dev/null +++ b/import_export_extensions/admin/forms/import_job_admin_form.py @@ -0,0 +1,57 @@ +from django import forms +from django.urls import reverse + +from ... import models +from ..widgets import ProgressBarWidget + + +class ImportJobAdminForm(forms.ModelForm): + """Admin form for ``ImportJob`` model. + + Adds custom `import_progressbar` field that displays current import + progress using AJAX requests to specified endpoint. Fields widget is + defined in `__init__` method. + + """ + + import_progressbar = forms.Field( + required=False, + ) + + def __init__( + self, + instance: models.ImportJob, + *args, + **kwargs, + ): + """Provide `import_progressbar` widget the ``ImportJob`` instance.""" + super().__init__(*args, instance=instance, **kwargs) + url_name = "admin:import_job_progress" + self.fields["import_progressbar"].label = ( + "Import progress" if + instance.import_status == models.ImportJob.ImportStatus.IMPORTING + else "Parsing progress" + ) + self.fields["import_progressbar"].widget = ProgressBarWidget( + job=instance, + url=reverse(url_name, args=(instance.id,)), + ) + + class Meta: + fields = ( + "import_status", + "resource_path", + "data_file", + "resource_kwargs", + "traceback", + "error_message", + "result", + "parse_task_id", + "import_task_id", + "parse_finished", + "import_started", + "import_finished", + "created_by", + "created", + "modified", + ) diff --git a/import_export_extensions/admin/mixins/export_mixin.py b/import_export_extensions/admin/mixins/export_mixin.py index 05a1ee4..a3b8e18 100644 --- a/import_export_extensions/admin/mixins/export_mixin.py +++ b/import_export_extensions/admin/mixins/export_mixin.py @@ -138,9 +138,9 @@ def celery_export_action(self, request, *args, **kwargs): resources=self.get_export_resource_classes(), ) resource_kwargs = self.get_export_resource_kwargs( - request=request, *args, **kwargs, + request=request, ) if request.method == "POST" and form.is_valid(): file_format = formats[int(form.cleaned_data["file_format"])] diff --git a/import_export_extensions/admin/mixins/import_mixin.py b/import_export_extensions/admin/mixins/import_mixin.py index 9475e79..e3607a9 100644 --- a/import_export_extensions/admin/mixins/import_mixin.py +++ b/import_export_extensions/admin/mixins/import_mixin.py @@ -16,10 +16,10 @@ from django.utils.translation import gettext_lazy as _ from import_export import admin as base_admin -from import_export import forms as base_forms from import_export import mixins as base_mixins from ... import models +from ..forms import ForceImportForm from . import types @@ -152,7 +152,7 @@ def celery_import_action( context = self.get_context_data(request) resource_classes = self.get_import_resource_classes() - form = base_forms.ImportForm( + form = ForceImportForm( self.get_import_formats(), request.POST or None, request.FILES or None, @@ -284,7 +284,7 @@ def celery_import_job_results_view( if job.import_status != models.ImportJob.ImportStatus.PARSED: # display import form - context["import_form"] = base_forms.ImportForm( + context["import_form"] = ForceImportForm( import_formats=self.get_import_formats(), ) else: @@ -324,6 +324,7 @@ def create_import_job( resource_kwargs=resource.resource_init_kwargs, created_by=request.user, skip_parse_step=getattr(settings, "IMPORT_EXPORT_SKIP_ADMIN_CONFIRM", False), + force_import=form.cleaned_data["force_import"], ) def get_import_job( diff --git a/import_export_extensions/api/serializers/import_job.py b/import_export_extensions/api/serializers/import_job.py index 0c63e4f..ec939f6 100644 --- a/import_export_extensions/api/serializers/import_job.py +++ b/import_export_extensions/api/serializers/import_job.py @@ -35,6 +35,7 @@ class Meta: "progress", "import_started", "import_finished", + "force_import", "created", "modified", ) @@ -50,6 +51,8 @@ class CreateImportJob(serializers.Serializer): resource_class: typing.Type[resources.CeleryModelResource] file = serializers.FileField(required=True) + force_import = serializers.BooleanField(default=False, required=False) + skip_parse_step = serializers.BooleanField(default=False, required=False) def __init__( self, @@ -70,6 +73,8 @@ def create( """Create import job.""" return models.ImportJob.objects.create( data_file=validated_data["file"], + force_import=validated_data["force_import"], + skip_parse_step=validated_data["skip_parse_step"], resource_path=self.resource_class.class_path, resource_kwargs=self._resource_kwargs, created_by=self._user, diff --git a/import_export_extensions/migrations/0005_importjob_force_import.py b/import_export_extensions/migrations/0005_importjob_force_import.py new file mode 100644 index 0000000..8d8577f --- /dev/null +++ b/import_export_extensions/migrations/0005_importjob_force_import.py @@ -0,0 +1,22 @@ +# Generated by Django 4.2.7 on 2023-11-16 10:49 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("import_export_extensions", "0004_alter_exportjob_created_by_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="importjob", + name="force_import", + field=models.BooleanField( + default=False, + help_text="Import data with skip invalid rows.", + verbose_name="Force import", + ), + ), + ] diff --git a/import_export_extensions/models/import_job.py b/import_export_extensions/models/import_job.py index d5018ad..36f2d57 100644 --- a/import_export_extensions/models/import_job.py +++ b/import_export_extensions/models/import_job.py @@ -175,6 +175,12 @@ class ImportStatus(models.TextChoices): verbose_name=_("Skip parse step"), ) + force_import = models.BooleanField( + default=False, + help_text=_("Import data with skip invalid rows."), + verbose_name=_("Force import"), + ) + class Meta: verbose_name = _("Import job") verbose_name_plural = _("Import jobs") @@ -355,6 +361,7 @@ def _parse_data_inner(self) -> Result: dry_run=True, raise_errors=False, collect_failures=True, + force_import=self.force_import, ) def confirm_import(self): @@ -445,6 +452,7 @@ def _import_data_inner(self) -> Result: raise_errors=True, use_transactions=True, collect_failures=True, + force_import=self.force_import, ) def _get_import_format_by_ext( diff --git a/import_export_extensions/resources.py b/import_export_extensions/resources.py index cd396bd..f4c42b1 100644 --- a/import_export_extensions/resources.py +++ b/import_export_extensions/resources.py @@ -1,3 +1,4 @@ +import collections import typing from enum import Enum @@ -12,25 +13,8 @@ from django_filters.utils import translate_validation from import_export import resources from import_export.formats import base_formats -from import_export.results import Error as BaseError - -class Error(BaseError): - """Customization of over base Error class from import export.""" - - def __repr__(self) -> str: - """Return object representation in string format.""" - return f"Error({self.error})" - - def __reduce__(self): - """Simplify Exception object for pickling. - - `error` object may contain not pickable objects (for example, django's - lazy text), so here it replaced with simple string. - - """ - self.error = str(self.error) - return super().__reduce__() +from .results import Error, Result, RowResult class TaskState(Enum): @@ -97,9 +81,14 @@ def import_data( use_transactions: typing.Optional[bool] = None, collect_failed_rows: bool = False, rollback_on_validation_errors: bool = False, + force_import: bool = False, **kwargs, ): - """Init task state before importing.""" + """Init task state before importing. + + If `force_import=True`, then rows with errors will be skipped. + + """ self.initialize_task_state( state=( TaskState.IMPORTING.name if not dry_run @@ -114,6 +103,7 @@ def import_data( use_transactions=use_transactions, collect_failed_rows=collect_failed_rows, rollback_on_validation_errors=rollback_on_validation_errors, + force_import=force_import, **kwargs, ) @@ -124,10 +114,16 @@ def import_row( using_transactions=True, dry_run=False, raise_errors=False, + force_import=False, **kwargs, ): - """Update task status as we import rows.""" - imported_row = super().import_row( + """Update task status as we import rows. + + If `force_import=True`, then row errors will be stored in + `field_skipped_errors` or `non_field_skipped_errors`. + + """ + imported_row: RowResult = super().import_row( row=row, instance_loader=instance_loader, using_transactions=using_transactions, @@ -141,8 +137,48 @@ def import_row( else TaskState.PARSING.name ), ) + if force_import and imported_row.has_error_import_type: + imported_row = self._skip_row_with_errors(imported_row, row) return imported_row + def _skip_row_with_errors( + self, + row_result: RowResult, + row_data: collections.OrderedDict[str, str], + ) -> RowResult: + """Process row as skipped. + + Move row errors to skipped errors attributes. + Change import type to skipped. + + """ + row_result.diff = [] + for field in self.get_fields(): + row_result.diff.append(row_data.get(field.column_name, "")) + + row_result.non_field_skipped_errors.extend( + row_result.errors, + ) + if row_result.validation_error is not None: + row_result.field_skipped_errors.update( + **row_result.validation_error.error_dict, + ) + row_result.errors = [] + row_result.validation_error = None + + row_result.import_type = RowResult.IMPORT_TYPE_SKIP + return row_result + + @classmethod + def get_row_result_class(self): + """Return custom row result class.""" + return RowResult + + @classmethod + def get_result_class(self): + """Geti custom result class.""" + return Result + def export( self, queryset: QuerySet = None, @@ -157,9 +193,9 @@ def export( queryset=queryset, ) return super().export( # type: ignore - queryset=queryset, *args, **kwargs, + queryset=queryset, ) def export_resource(self, obj): diff --git a/import_export_extensions/results.py b/import_export_extensions/results.py new file mode 100644 index 0000000..ce331fd --- /dev/null +++ b/import_export_extensions/results.py @@ -0,0 +1,69 @@ +from django.core.exceptions import ValidationError + +from import_export import results + + +class Error(results.Error): + """Customization of over base Error class from import export.""" + + def __repr__(self) -> str: + """Return object representation in string format.""" + return f"Error({self.error})" + + def __reduce__(self): + """Simplify Exception object for pickling. + + `error` object may contain not pickable objects (for example, django's + lazy text), so here it replaced with simple string. + + """ + self.error = str(self.error) + return super().__reduce__() + + +class RowResult(results.RowResult): + """Custom row result class with ability to store skipped errors in row.""" + def __init__(self): + self.non_field_skipped_errors: list[Error] = [] + self.field_skipped_errors: dict[str, list[ValidationError]] = dict() + super().__init__() + + @property + def has_skipped_errors(self) -> bool: + """Return True if row contain any skipped errors.""" + if len(self.non_field_skipped_errors) > 0 or len(self.field_skipped_errors) > 0: + return True + return False + + @property + def skipped_errors_count(self) -> int: + """Return count of skipped errors.""" + return ( + len(self.non_field_skipped_errors) + + len(self.field_skipped_errors) + ) + + @property + def has_error_import_type(self) -> bool: + """Return true if import type is not valid.""" + if self.import_type not in self.valid_import_types: + return True + return False + + +class Result(results.Result): + """Custom result class with ability to store info about skipped rows.""" + + @property + def has_skipped_rows(self) -> bool: + """Return True if contain any skipped rows.""" + if any(row.has_skipped_errors for row in self.rows): + return True + return False + + @property + def skipped_rows(self) -> list[RowResult]: + """Return all rows with skipped errors.""" + return list( + filter(lambda row: row.has_skipped_errors, self.rows), + ) diff --git a/import_export_extensions/templates/admin/import_export_extensions/celery_import_results.html b/import_export_extensions/templates/admin/import_export_extensions/celery_import_results.html index 4958acd..b68240f 100644 --- a/import_export_extensions/templates/admin/import_export_extensions/celery_import_results.html +++ b/import_export_extensions/templates/admin/import_export_extensions/celery_import_results.html @@ -162,7 +162,7 @@

{% trans "These elements are imported successfully" %} {% endif %}

- +
@@ -174,11 +174,38 @@

{% for row in result.rows %} {% if not row.errors %}

-
+ {% if row.import_type == 'new' %} {% trans "New" %} {% elif row.import_type == 'skip' %} {% trans "Skipped" %} + {{ row.skipped_errors_count}} +
+
    + {% for field_name, errors in row.field_skipped_errors.items %} +
  • + {{ field_name }} +
      + {% for error in errors %} +
    • + {{ error }} +
    • + {% endfor %} +
    +
  • + {% endfor %} + {% if row.non_field_skipped_errors %} +
  • + {% trans "Non field specific" %} +
      + {% for error in row.non_field_skipped_errors %} +
    • {{ error }}
    • + {% endfor %} +
    +
  • + {% endif %} +
+
{% elif row.import_type == 'delete' %} {% trans "Delete" %} {% elif row.import_type == 'update' %} diff --git a/tests/conftest.py b/tests/conftest.py index 7189b8f..9d5c04b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,10 @@ from django.conf import settings from django.contrib.auth import get_user_model +from django.contrib.auth.models import User from django.core.files.uploadedfile import SimpleUploadedFile +from rest_framework import test + import pytest from tests.fake_app.models import Artist, Band, Membership @@ -61,7 +64,7 @@ def membership(band: Band) -> Membership: @pytest.fixture -def uploaded_file(existing_artist: Artist): +def uploaded_file(existing_artist: Artist) -> SimpleUploadedFile: """Generate valid `Artist` import file.""" import_job = ArtistImportJobFactory.build(artists=[existing_artist]) return SimpleUploadedFile( @@ -71,6 +74,31 @@ def uploaded_file(existing_artist: Artist): ) +@pytest.fixture +def force_import_artist_job(new_artist: Artist) -> Artist: + """Return `ImportJob` with `force_import=True` and file with invalid row.""" + return ArtistImportJobFactory( + artists=[new_artist], + is_valid_file=False, + force_import=True, + ) + + +@pytest.fixture +def invalid_uploaded_file(new_artist: Artist) -> SimpleUploadedFile: + """Generate invalid `Artist` imort file.""" + import_job = ArtistImportJobFactory.build( + artists=[new_artist], + force_import=True, + is_valid_file=False, + ) + return SimpleUploadedFile( + "test_file.csv", + content=import_job.data_file.file.read().encode(), + content_type="text/plain", + ) + + @pytest.fixture def superuser(): """Return superuser instance.""" @@ -102,3 +130,19 @@ def temp_directory_for_media(tmpdir_factory): ) media = tmpdir_factory.mktemp("tmp_media") settings.MEDIA_ROOT = media + + +@pytest.fixture +def api_client() -> test.APIClient: + """Create api client.""" + return test.APIClient() + + +@pytest.fixture +def admin_api_client( + superuser: User, + api_client: test.APIClient, +) -> test.APIClient: + """Authenticate admin_user and return api client.""" + api_client.force_authenticate(user=superuser) + return api_client diff --git a/tests/fake_app/factories.py b/tests/fake_app/factories.py index cfc6264..79317bf 100644 --- a/tests/fake_app/factories.py +++ b/tests/fake_app/factories.py @@ -63,11 +63,20 @@ class Meta: class Params: artists: list[models.Artist] = [] + is_valid_file: bool = True @factory.lazy_attribute def data_file(self): """Generate `data_file` based on passed `artists`.""" resource = SimpleArtistResource() + + if not self.is_valid_file: + # Append not existing artist with a non-existent instrument + # to violate the not-null constraint + self.artists.append( + ArtistFactory.build(instrument=InstrumentFactory.build()), + ) + dataset = resource.export(self.artists) export_data = formats.CSV().export_data(dataset) content = django_files.ContentFile(export_data) diff --git a/tests/settings.py b/tests/settings.py index 3e3aa10..8a26705 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -9,6 +9,8 @@ # SECURITY WARNING: keep the secret key used in production secret! SECRET_KEY = "a87082n4v52u4rnvk2edv128eudfvn5" +ALLOWED_HOSTS = ["*"] + # SECURITY WARNING: don't run with debug turned on in production! DEBUG = True diff --git a/tests/test_api/test_import.py b/tests/test_api/test_import.py index f48ba00..59df495 100644 --- a/tests/test_api/test_import.py +++ b/tests/test_api/test_import.py @@ -1,9 +1,9 @@ from django.contrib.auth.models import User from django.core.files.uploadedfile import SimpleUploadedFile -from django.test.client import Client from django.urls import reverse from rest_framework import status +from rest_framework.test import APIClient import pytest @@ -12,13 +12,11 @@ @pytest.mark.django_db(transaction=True) def test_import_api_creates_import_job( - client: Client, - superuser: User, + admin_api_client: APIClient, uploaded_file: SimpleUploadedFile, ): """Ensure import start api creates new import job.""" - client.force_login(superuser) - response = client.post( + response = admin_api_client.post( path=reverse("import-artist-start"), data={ "file": uploaded_file, @@ -32,27 +30,66 @@ def test_import_api_creates_import_job( @pytest.mark.django_db(transaction=True) def test_import_api_detail( - client: Client, - superuser: User, + admin_api_client: APIClient, artist_import_job: ImportJob, ): """Ensure import detail api shows current import job status.""" - client.force_login(superuser) - response = client.get( + response = admin_api_client.get( path=reverse( "import-artist-detail", kwargs={"pk": artist_import_job.id}, ), ) + assert response.status_code == status.HTTP_200_OK, response.data assert response.data["import_status"] == ImportJob.ImportStatus.PARSED artist_import_job.refresh_from_db() artist_import_job.confirm_import() - response = client.get( + response = admin_api_client.get( path=reverse( "import-artist-detail", kwargs={"pk": artist_import_job.id}, ), ) + assert response.status_code == status.HTTP_200_OK, response.data + assert response.data["import_finished"] + + +@pytest.mark.django_db(transaction=True) +def test_force_import_api_detail( + admin_api_client: APIClient, + superuser: User, + force_import_artist_job: ImportJob, +): + """Test detail api for force import job. + + Ensure created import job with invalid file will have `PARSED` status. + Ensure force import job after confirmation will skip invalid rows + and create right ones. + + """ + response = admin_api_client.get( + path=reverse( + "import-artist-detail", + kwargs={"pk": force_import_artist_job.id}, + ), + ) + assert response.status_code == status.HTTP_200_OK, response.data + assert response.data["import_status"] == ImportJob.ImportStatus.PARSED + + force_import_artist_job.refresh_from_db() + force_import_artist_job.confirm_import() + + response = admin_api_client.get( + path=reverse( + "import-artist-detail", + kwargs={"pk": force_import_artist_job.id}, + ), + ) + assert response.status_code == status.HTTP_200_OK, response.data assert response.data["import_finished"] + + force_import_artist_job.refresh_from_db() + assert force_import_artist_job.result.totals["new"] == 1 + assert force_import_artist_job.result.totals["skip"] == 1 diff --git a/tests/test_models/test_import/test_import_data.py b/tests/test_models/test_import/test_import_data.py index 714c33e..9ff2e1e 100644 --- a/tests/test_models/test_import/test_import_data.py +++ b/tests/test_models/test_import/test_import_data.py @@ -82,3 +82,64 @@ def test_import_data_wrong_status(artist_import_job: ImportJob): match=f"ImportJob with id {artist_import_job.id} has incorrect status", ): artist_import_job.import_data() + + +@pytest.mark.parametrize( + [ + "skip_parse_step", + "is_valid_file", + "expected_status", + "is_instance_created", + ], + [ + [False, True, ImportJob.ImportStatus.PARSED, False], + [False, False, ImportJob.ImportStatus.INPUT_ERROR, False], + [True, True, ImportJob.ImportStatus.IMPORTED, True], + [True, False, ImportJob.ImportStatus.IMPORT_ERROR, False], + ], +) +@pytest.mark.django_db(transaction=True) +def test_import_data_skip_parse_step( + new_artist: Artist, + skip_parse_step: bool, + is_valid_file: bool, + expected_status: ImportJob.ImportStatus, + is_instance_created: bool, +): + """Test import job skip parse step logic. + + If `skip_parse_step=True`, + then instance will import data if no errors detected. + If `skip_parse_step=False`, then parse only. + + """ + import_job: ImportJob = ArtistImportJobFactory.build( + artists=[new_artist], + force_import=False, + skip_parse_step=skip_parse_step, + is_valid_file=is_valid_file, + ) + import_job.save() + import_job.refresh_from_db() + + assert import_job.import_status == expected_status + assert ( + Artist.objects.filter(name=new_artist.name).exists() + == is_instance_created + ) + + +def test_force_import_create_correct_rows( + new_artist: Artist, +): + """Test import job with `force_import=True` create correct rows.""" + import_job: ImportJob = ArtistImportJobFactory( + artists=[new_artist], + force_import=True, + skip_parse_step=True, + is_valid_file=False, + ) + import_job.import_data() + import_job.refresh_from_db() + assert import_job.import_status == import_job.ImportStatus.IMPORTED + assert Artist.objects.filter(name=new_artist.name).exists() diff --git a/tests/test_models/test_import/test_parse_data.py b/tests/test_models/test_import/test_parse_data.py index 5637de6..e61dbbd 100644 --- a/tests/test_models/test_import/test_parse_data.py +++ b/tests/test_models/test_import/test_parse_data.py @@ -3,6 +3,9 @@ from import_export_extensions.models import ImportJob +from ...fake_app.factories import ArtistImportJobFactory +from ...fake_app.models import Artist + def test_parse_valid_data_file(artist_import_job: ImportJob): """Test `parse_data` with valid importing data. @@ -126,3 +129,37 @@ def test_error_message_length( artist_import_job.parse_data() assert len(artist_import_job.error_message) == max_length + + +@pytest.mark.parametrize( + [ + "force_import", + "expected_status", + ], + [ + [False, ImportJob.ImportStatus.INPUT_ERROR], + [True, ImportJob.ImportStatus.PARSED], + ], +) +@pytest.mark.django_db(transaction=True) +def test_parse_data_invalid_row_file( + new_artist: Artist, + force_import: bool, + expected_status: ImportJob.ImportStatus, +): + """Test parse file with invalid row. + + If force_import = False, then job must finish with `INPUT_ERROR`. + If force_import = True, then job must finish with `PARSED` + and skip invalid rows. + + """ + import_job: ImportJob = ArtistImportJobFactory( + artists=[new_artist], + force_import=force_import, + is_valid_file=False, + ) + import_job.parse_data() + import_job.refresh_from_db() + + assert import_job.import_status == expected_status