fixed issue with natural_foreign_keys and key_is_id (#1824)

* fixed issue with natural_foreign_keys and key_is_id

* updated release notes

* updated changelog

* added WidgetException and check in ForeignKeyWidget

* removed unsupported arg

docs/changelog.rst

@@ -8,9 +8,9 @@ Changelog
 4.0.2 (unreleased)
 ------------------
 
-- fix export with custom column name (`1821 <https://github.com/django-import-export/django-import-export/pull/1821>`_)
 - fix allow ``column_name`` to be declared in ``fields`` list (`1815 <https://github.com/django-import-export/django-import-export/pull/1815>`_)
 - fix export with custom column name (`1821 <https://github.com/django-import-export/django-import-export/pull/1821>`_)
+- fix clash between ``key_is_id`` and ``use_natural_foreign_keys`` (`1824 <https://github.com/django-import-export/django-import-export/pull/1824>`_)
 
 4.0.1 (2024-05-08)
 ------------------

import_export/exceptions.py

@@ -10,6 +10,12 @@ class FieldError(ImportExportError):
     pass
 
 
+class WidgetError(ImportExportError):
+    """Raised when there is a misconfiguration with a Widget."""
+
+    pass
+
+
 class ImportError(ImportExportError):
     def __init__(self, error, number=None, row=None):
         """A wrapper for errors thrown from the import process.

import_export/resources.py

@@ -1306,7 +1306,11 @@ def field_from_django_field(cls, field_name, django_field, readonly):
         attribute = field_name
         column_name = field_name
         # To solve #974
-        if isinstance(django_field, ForeignKey) and "__" not in column_name:
+        if (
+            isinstance(django_field, ForeignKey)
+            and "__" not in column_name
+            and not cls._meta.use_natural_foreign_keys
+        ):
             attribute += "_id"
             widget_kwargs["key_is_id"] = True
 

import_export/widgets.py

@@ -14,6 +14,8 @@
 from django.utils.formats import number_format
 from django.utils.translation import gettext_lazy as _
 
+from import_export.exceptions import WidgetError
+
 logger = logging.getLogger(__name__)
 
 
@@ -497,6 +499,10 @@ def __init__(
         self.field = field
         self.key_is_id = key_is_id
         self.use_natural_foreign_keys = use_natural_foreign_keys
+        if use_natural_foreign_keys is True and key_is_id is True:
+            raise WidgetError(
+                _("use_natural_foreign_keys and key_is_id cannot both be True")
+            )
         super().__init__(**kwargs)
 
     def get_queryset(self, value, row, *args, **kwargs):

tests/core/tests/test_resources/test_natural_foreign_key.py

@@ -0,0 +1,64 @@
+import tablib
+from core.models import Author, Book
+from django.test import TestCase
+
+from import_export import fields, resources, widgets
+
+
+class BookUsingNaturalKeys(resources.ModelResource):
+    class Meta:
+        model = Book
+        fields = ["name", "author"]
+        use_natural_foreign_keys = True
+
+
+class BookUsingAuthorNaturalKey(resources.ModelResource):
+    class Meta:
+        model = Book
+        fields = ["name", "author"]
+
+    author = fields.Field(
+        attribute="author",
+        column_name="author",
+        widget=widgets.ForeignKeyWidget(
+            Author,
+            use_natural_foreign_keys=True,
+        ),
+    )
+
+
+class TestNaturalKeys(TestCase):
+    """Tests for issue 1816."""
+
+    def setUp(self) -> None:
+        author = Author.objects.create(name="J. R. R. Tolkien")
+        Book.objects.create(author=author, name="The Hobbit")
+        self.expected_dataset = tablib.Dataset(headers=["name", "author"])
+        row = ["The Hobbit", '["J. R. R. Tolkien"]']
+        self.expected_dataset.append(row)
+
+    def test_resource_use_natural_keys(self):
+        """
+        test with ModelResource.Meta.use_natural_foreign_keys=True
+        Reproduces this problem
+        """
+        resource = BookUsingNaturalKeys()
+        exported_dataset = resource.export(Book.objects.all())
+        self.assertDatasetEqual(self.expected_dataset, exported_dataset)
+
+    def test_field_use_natural_keys(self):
+        """
+        test with ModelResource.field.widget.use_natural_foreign_keys=True
+        Example of correct behaviour
+        """
+        resource = BookUsingAuthorNaturalKey()
+        exported_dataset = resource.export(Book.objects.all())
+        self.assertDatasetEqual(self.expected_dataset, exported_dataset)
+
+    def assertDatasetEqual(self, expected_dataset, actual_dataset, message=None):
+        """
+        Util for comparing datasets
+        """
+        self.assertEqual(len(expected_dataset), len(actual_dataset), message)
+        for expected_row, actual_row in zip(expected_dataset, actual_dataset):
+            self.assertEqual(expected_row, actual_row, message)

tests/core/tests/test_widgets.py

@@ -12,6 +12,7 @@
 from django.utils import timezone
 
 from import_export import widgets
+from import_export.exceptions import WidgetError
 
 
 class WidgetTest(TestCase):
@@ -582,6 +583,16 @@ def test_book_natural_key_render(self):
             json.dumps(self.book.natural_key()),
         )
 
+    def test_natural_foreign_key_with_key_is_id(self):
+        with self.assertRaises(WidgetError) as e:
+            widgets.ForeignKeyWidget(
+                Author, use_natural_foreign_keys=True, key_is_id=True
+            )
+        self.assertEqual(
+            "use_natural_foreign_keys and key_is_id " "cannot both be True",
+            str(e.exception),
+        )
+
 
 class ManyToManyWidget(TestCase, RowDeprecationTestMixin):
     def setUp(self):