Merge pull request #54 from saritasa-nest/feature/improve-coverage

Add tests for import with errors, for export with querystring

import_export_extensions/api/serializers/import_job_details.py

@@ -3,8 +3,6 @@
 
 from rest_framework import serializers
 
-from import_export.results import RowResult
-
 from ... import models
 
 
@@ -62,10 +60,6 @@ def to_representation(self, instance: models.ImportJob):
         rows = []
         resource = instance.resource
         for row in instance.result.rows:
-            # errors displayed in input_error.row_errors(InputErrorSerializer)
-            if row.import_type == RowResult.IMPORT_TYPE_ERROR:
-                continue
-
             original_fields = [
                 resource.export_field(field, row.original)
                 if row.original

test_project/fake_app/factories.py

@@ -22,6 +22,7 @@ class ArtistFactory(factory.django.DjangoModelFactory):
     """Simple factory for ``Artist`` model."""
 
     name = factory.Faker("name")
+    external_id = factory.Faker("uuid4")
     instrument = factory.SubFactory(InstrumentFactory)
 
     class Meta:

test_project/fake_app/migrations/0001_initial.py

@@ -1,4 +1,4 @@
-# Generated by Django 4.1 on 2022-09-01 10:47
+# Generated by Django 5.1.1 on 2024-10-14 08:56
 
 import django.db.models.deletion
 from django.db import migrations, models
@@ -11,7 +11,7 @@ class Migration(migrations.Migration):
 
     operations = [
         migrations.CreateModel(
-            name="Artist",
+            name="Band",
             fields=[
                 (
                     "id",
@@ -22,15 +22,15 @@ class Migration(migrations.Migration):
                         verbose_name="ID",
                     ),
                 ),
-                ("name", models.CharField(max_length=100, unique=True)),
+                ("title", models.CharField(max_length=100)),
             ],
             options={
-                "verbose_name": "Artist",
-                "verbose_name_plural": "Artists",
+                "verbose_name": "Band",
+                "verbose_name_plural": "Bands",
             },
         ),
         migrations.CreateModel(
-            name="Band",
+            name="Instrument",
             fields=[
                 (
                     "id",
@@ -44,12 +44,12 @@ class Migration(migrations.Migration):
                 ("title", models.CharField(max_length=100)),
             ],
             options={
-                "verbose_name": "Band",
-                "verbose_name_plural": "Bands",
+                "verbose_name": "Instrument",
+                "verbose_name_plural": "Instruments",
             },
         ),
         migrations.CreateModel(
-            name="Instrument",
+            name="Artist",
             fields=[
                 (
                     "id",
@@ -60,11 +60,28 @@ class Migration(migrations.Migration):
                         verbose_name="ID",
                     ),
                 ),
-                ("title", models.CharField(max_length=100)),
+                ("name", models.CharField(max_length=100, unique=True)),
+                (
+                    "external_id",
+                    models.CharField(
+                        editable=False,
+                        help_text="External ID for sync import objects.",
+                        null=True,
+                        unique=True,
+                        verbose_name="External ID",
+                    ),
+                ),
+                (
+                    "instrument",
+                    models.ForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE,
+                        to="fake_app.instrument",
+                    ),
+                ),
             ],
             options={
-                "verbose_name": "Instrument",
-                "verbose_name_plural": "Instruments",
+                "verbose_name": "Artist",
+                "verbose_name_plural": "Artists",
             },
         ),
         migrations.CreateModel(
@@ -104,16 +121,9 @@ class Migration(migrations.Migration):
             model_name="artist",
             name="bands",
             field=models.ManyToManyField(
+                related_name="artists",
                 through="fake_app.Membership",
                 to="fake_app.band",
             ),
         ),
-        migrations.AddField(
-            model_name="artist",
-            name="instrument",
-            field=models.ForeignKey(
-                on_delete=django.db.models.deletion.CASCADE,
-                to="fake_app.instrument",
-            ),
-        ),
     ]

test_project/fake_app/models.py

@@ -31,6 +31,14 @@ class Artist(models.Model):
         on_delete=models.CASCADE,
     )
 
+    external_id = models.CharField(  # noqa DJ01
+        verbose_name=_("External ID"),
+        help_text=_("External ID for sync import objects."),
+        null=True,
+        unique=True,
+        editable=False,
+    )
+
     class Meta:
         verbose_name = _("Artist")
         verbose_name_plural = _("Artists")

test_project/fake_app/resources.py

@@ -13,9 +13,11 @@ class SimpleArtistResource(CeleryModelResource):
 
     class Meta:
         model = Artist
+        import_id_fields = ["external_id"]
         clean_model_instances = True
         fields = [
             "id",
+            "external_id",
             "name",
             "instrument",
         ]

test_project/tests/integration_tests/test_api/test_export.py

@@ -8,19 +8,50 @@
 
 
 @pytest.mark.django_db(transaction=True)
-def test_export_api_creates_export_job(admin_api_client: test.APIClient):
+@pytest.mark.parametrize(
+    argnames=["export_url"],
+    argvalues=[
+        pytest.param(
+            reverse("export-artist-start"),
+            id="Url without filter_kwargs",
+        ),
+        pytest.param(
+            f"{reverse('export-artist-start')}?name=Artist",
+            id="Url with valid filter_kwargs",
+        ),
+    ],
+)
+def test_export_api_creates_export_job(
+    admin_api_client: test.APIClient,
+    export_url: str,
+):
     """Ensure export start API creates new export job."""
     response = admin_api_client.post(
-        path=reverse("export-artist-start"),
+        path=export_url,
         data={
             "file_format": "csv",
         },
     )
-    assert response.status_code == status.HTTP_201_CREATED
+    assert response.status_code == status.HTTP_201_CREATED, response.data
     assert response.data["export_status"] == ExportJob.ExportStatus.CREATED
     assert ExportJob.objects.filter(id=response.data["id"]).exists()
 
 
+@pytest.mark.django_db(transaction=True)
+def test_export_api_create_export_job_with_invalid_filter_kwargs(
+    admin_api_client: test.APIClient,
+):
+    """Ensure export start API with invalid kwargs return an error."""
+    response = admin_api_client.post(
+        path=f"{reverse('export-artist-start')}?id=invalid_id",
+        data={
+            "file_format": "csv",
+        },
+    )
+    assert response.status_code == status.HTTP_400_BAD_REQUEST, response.data
+    assert str(response.data["id"][0]) == "Enter a number."
+
+
 @pytest.mark.django_db(transaction=True)
 def test_export_api_detail(
     admin_api_client: test.APIClient,
@@ -33,5 +64,5 @@ def test_export_api_detail(
             kwargs={"pk": artist_export_job.id},
         ),
     )
-    assert response.status_code == status.HTTP_200_OK
+    assert response.status_code == status.HTTP_200_OK, response.data
     assert response.data["export_finished"]

test_project/tests/integration_tests/test_api/test_import.py

@@ -1,4 +1,5 @@
 from django.contrib.auth.models import User
+from django.core.files import base as django_files
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.urls import reverse
 
@@ -8,6 +9,8 @@
 import pytest
 
 from import_export_extensions.models.import_job import ImportJob
+from test_project.fake_app.factories import ArtistImportJobFactory
+from test_project.fake_app.models import Artist
 
 
 @pytest.mark.django_db(transaction=True)
@@ -94,3 +97,80 @@ def test_force_import_api_detail(
     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
+
+
+@pytest.mark.django_db(transaction=True)
+def test_import_api_detail_with_row_errors(
+    admin_api_client: APIClient,
+    existing_artist: Artist,
+):
+    """Ensure import detail api shows row errors."""
+    expected_error_message = "Instrument matching query does not exist."
+    invalid_row_values = [
+        str(existing_artist.id),
+        existing_artist.external_id,
+        existing_artist.name,
+        str(existing_artist.instrument_id),
+    ]
+
+    import_artist_job = ArtistImportJobFactory(
+        artists=[existing_artist],
+    )
+    # Remove instrument to trigger row error
+    existing_artist.instrument.delete()
+    import_artist_job.parse_data()
+
+    response = admin_api_client.get(
+        path=reverse(
+            "import-artist-detail",
+            kwargs={"pk": import_artist_job.id},
+        ),
+    )
+
+    assert response.status_code == status.HTTP_200_OK, response.data
+    assert response.data["import_status"] == ImportJob.ImportStatus.INPUT_ERROR
+    row_error = response.data["input_error"]["row_errors"][0][0]
+    assert row_error["line"] == 1
+    assert row_error["error"] == expected_error_message
+    assert row_error["row"] == invalid_row_values
+
+
+@pytest.mark.django_db(transaction=True)
+def test_import_api_detail_with_base_errors(
+    admin_api_client: APIClient,
+    existing_artist: Artist,
+):
+    """Ensure import detail api shows base errors."""
+    expected_error_message = (
+        "The following fields are declared in 'import_id_fields' but are not "
+        "present in the file headers: external_id"
+    )
+
+    # Create file with missing external_id header
+    file_content = django_files.ContentFile(
+        "id,name,instrument\n"
+        f"{existing_artist.id},{existing_artist.name},"
+        f"{existing_artist.instrument_id}\n",
+    )
+    uploaded_file = django_files.File(file_content.file, "data.csv")
+
+    import_artist_job = ArtistImportJobFactory(
+        artists=[existing_artist],
+        data_file=uploaded_file,
+        force_import=True,
+    )
+    import_artist_job.parse_data()
+
+    response = admin_api_client.get(
+        path=reverse(
+            "import-artist-detail",
+            kwargs={"pk": import_artist_job.id},
+        ),
+    )
+
+    assert response.status_code == status.HTTP_200_OK, response.data
+    assert response.data["import_status"] == ImportJob.ImportStatus.INPUT_ERROR
+    assert (
+        response.data["input_error"]["base_errors"][0]
+        == expected_error_message
+    )