Merge branch 'main' into issue-1823-readd-resource-deprecation-warnings

Author: matthewhegarty

AUTHORS

@@ -151,3 +151,4 @@ The following is a list of much appreciated contributors:
 * RobTilton (Robert Tilton)
 * ulliholtgrave
 * mishka251 (Mikhail Belov)
+* jhthompson (Jeremy Thompson)

docs/advanced_usage.rst

@@ -577,6 +577,48 @@ For example, you can use the 'isbn' number instead of 'id' to uniquely identify
     field(s) select more than one row, then a ``MultipleObjectsReturned`` exception will be raised.  If no row is
     identified, then ``DoesNotExist`` exception will be raised.
 
+.. _dynamic_fields:
+
+Using 'dynamic fields' to identify existing instances
+-----------------------------------------------------
+
+There are some use-cases where a field defined in ``import_id_fields`` is not present in the dataset.  An example of
+this would be dynamic fields, where a field is generated from other data and then used as an identifier.  For example::
+
+    class BookResource(resources.ModelResource):
+
+        def before_import_row(self, row, **kwargs):
+            # generate a value for an existing field, based on another field
+            row["hash_id"] = hashlib.sha256(row["name"].encode()).hexdigest()
+
+        class Meta:
+            model = Book
+            # A 'dynamic field' - i.e. is used to identify existing rows
+            # but is not present in the dataset
+            import_id_fields = ("hash_id",)
+
+In the above example, a dynamic field called *hash_id* is generated and added to the dataset.  In this example, an
+error will be raised because *hash_id* is not present in the dataset.  To resolve this, update the dataset before
+import to add the dynamic field as a header::
+
+    class BookResource(resources.ModelResource):
+
+        def before_import(self, dataset, **kwargs):
+            # mimic a 'dynamic field' - i.e. append field which exists on
+            # Book model, but not in dataset
+            dataset.headers.append("hash_id")
+            super().before_import(dataset, **kwargs)
+
+        def before_import_row(self, row, **kwargs):
+            row["hash_id"] = hashlib.sha256(row["name"].encode()).hexdigest()
+
+        class Meta:
+            model = Book
+            # A 'dynamic field' - i.e. is used to identify existing rows
+            # but is not present in the dataset
+            import_id_fields = ("hash_id",)
+
+
 Access instances after import
 =============================
 

docs/changelog.rst

@@ -8,6 +8,7 @@ Changelog
 4.0.3 (unreleased)
 ------------------
 
+- Clarified documentation when importing with ``import_id_fields``  (`1836 <https://github.com/django-import-export/django-import-export/pull/1836>`_)
 - re-add ``resource_class`` deprecation warning (`1837 <https://github.com/django-import-export/django-import-export/pull/1837>`_)
 
 4.0.2 (2024-05-13)

docs/faq.rst

@@ -30,8 +30,10 @@ We encourage you to read the :doc:`contributing guidelines <contributing>`.
 Common issues
 =============
 
-FieldError on import
---------------------
+.. _import_id_fields_error_on_import:
+
+``import_id_fields`` error on import
+------------------------------------
 
 The following error message can be seen on import:
 
@@ -40,9 +42,11 @@ The following error message can be seen on import:
 This indicates that the Resource has not been configured correctly, and the import logic fails.  Specifically,
 the import process is attempting to use either the defined or default values for
 :attr:`~import_export.options.ResourceOptions.import_id_fields` and no matching field has been detected in the resource
-fields.
+fields. See :ref:`advanced_usage:Create or update model instances`.
 
-See :ref:`advanced_usage:Create or update model instances`.
+In cases where you are deliberately using generated fields in ``import_id_fields`` and these fields are not present in
+the dataset, then you need to modify the resource definition to accommodate this.
+See :ref:`dynamic_fields`.
 
 How to handle double-save from Signals
 --------------------------------------

docs/release_notes.rst

@@ -69,6 +69,15 @@ of :class:`~import_export.exceptions.ImportError` is raised.  This exception wra
 
 See `this PR <https://github.com/django-import-export/django-import-export/issues/1729>`_.
 
+Check ``import_id_fields``
+--------------------------
+
+Prior to v4 we had numerous issues where users were confused when imports failed due to declared ``import_id_fields``
+not being present in the dataset.  We added functionality in v4 to check for this and to raise a clearer error message.
+
+In some use-cases, it is a requirement that ``import_id_fields`` are not in the dataset, and are generated dynamically.
+If this affects your implementation, start with the documentation :ref:`here<import_id_fields_error_on_import>`.
+
 Deprecations
 ============
 

import_export/forms.py

@@ -70,8 +70,9 @@ def __init__(self, formats, resources, **kwargs):
 
     @property
     def media(self):
+        media = super().media
         extra = "" if settings.DEBUG else ".min"
-        return forms.Media(
+        return media + forms.Media(
             js=(
                 f"admin/js/vendor/jquery/jquery{extra}.js",
                 "admin/js/jquery.init.js",

import_export/resources.py

@@ -1153,6 +1153,13 @@ def _is_dry_run(self, kwargs):
         return kwargs.get("dry_run", False)
 
     def _check_import_id_fields(self, headers):
+        """
+        Provides a safety check with a meaningful error message for cases where
+        the ``import_id_fields`` declaration contains a field which is not in the
+        dataset.  For most use-cases this is an error, so we detect and raise.
+        There are conditions, such as 'dynamic fields' where this does not apply.
+        See issue 1834 for more information.
+        """
         import_id_fields = list()
         missing_fields = list()
         missing_headers = list()

tests/core/tests/test_forms.py

@@ -1,4 +1,5 @@
 import django.forms
+from core.models import Author
 from django.test import TestCase
 
 from import_export import forms, resources
@@ -33,6 +34,59 @@ def test_formbase_init_two_resources(self):
         )
 
 
+class ImportFormMediaTest(TestCase):
+    def test_import_form_media(self):
+        form = forms.ImportForm([CSV], [MyResource])
+        media = form.media
+        self.assertEqual(
+            media._css,
+            {},
+        )
+        self.assertEqual(
+            media._js,
+            [
+                "admin/js/vendor/jquery/jquery.min.js",
+                "admin/js/jquery.init.js",
+                "import_export/guess_format.js",
+            ],
+        )
+
+    def test_import_form_and_custom_widget_media(self):
+        class TestMediaWidget(django.forms.TextInput):
+            """Dummy test widget with associated CSS and JS media."""
+
+            class Media:
+                css = {
+                    "all": ["test.css"],
+                }
+                js = ["test.js"]
+
+        class CustomImportForm(forms.ImportForm):
+            """Dummy custom import form with a custom widget."""
+
+            author = django.forms.ModelChoiceField(
+                queryset=Author.objects.none(),
+                required=True,
+                widget=TestMediaWidget,
+            )
+
+        form = CustomImportForm([CSV], [MyResource])
+        media = form.media
+        self.assertEqual(
+            media._css,
+            {"all": ["test.css"]},
+        )
+        self.assertEqual(
+            media._js,
+            [
+                "test.js",
+                "admin/js/vendor/jquery/jquery.min.js",
+                "admin/js/jquery.init.js",
+                "import_export/guess_format.js",
+            ],
+        )
+
+
 class SelectableFieldsExportFormTest(TestCase):
     @classmethod
     def setUpTestData(cls) -> None:

tests/core/tests/test_resources/test_import_export.py

@@ -330,6 +330,27 @@ class Meta:
             str(e.exception),
         )
 
+    def test_dynamic_import_id_fields(self):
+        # issue 1834
+        class BookResource(resources.ModelResource):
+            def before_import(self, dataset, **kwargs):
+                # mimic a 'dynamic field' - i.e. append field which exists on
+                # Book model, but not in dataset
+                dataset.headers.append("price")
+                super().before_import(dataset, **kwargs)
+
+            class Meta:
+                model = Book
+                import_id_fields = ("price",)
+
+        self.resource = BookResource()
+        dataset = tablib.Dataset(
+            *[(self.book.pk, "Goldeneye", "[email protected]")],
+            headers=["id", "name", "author_email"],
+        )
+        self.resource.import_data(dataset, raise_errors=True)
+        self.assertEqual("Goldeneye", Book.objects.latest("id").name)
+
 
 class ImportWithMissingFields(TestCase):
     # issue 1517