Fix for declared fields being processed when not in fields list (#1702)

* added test

* added comment

* added fix

* added deprecation warnings

* updated tests

* updated tests

* updated tests and docs

* added test

* updated changelog formatting

* updated changelog

docs/advanced_usage.rst

@@ -39,6 +39,9 @@ Or the ``exclude`` option to blacklist fields::
             model = Book
             exclude = ('imported', )
 
+If both ``fields`` and ``exclude`` are declared, the ``fields`` declaration takes precedence, and ``exclude`` is
+ignored.
+
 .. _field_ordering:
 
 Field ordering

docs/changelog.rst

@@ -3,10 +3,15 @@ Changelog
 
 Please refer to :doc:`release notes<release_notes>`.
 
+4.0.0-beta.3 (unreleased)
+-------------------------
+
+- Fix issue where declared Resource fields not defined in ``fields`` are still imported (#1702)
+
 4.0.0-beta.2 (2023-12-09)
---------------------------
+-------------------------
 
-- fix declaring existing model field(s) in ModelResource altering export order (#1663)
+- Fix declaring existing model field(s) in ModelResource altering export order (#1663)
 - Updated `docker-compose` command with latest version syntax in `runtests.sh` (#1686)
 - Support export from model change form (#1687)
 - Updated Admin UI to track deleted and skipped Imports (#1691)

import_export/declarative.py

@@ -73,7 +73,16 @@ def __new__(cls, name, bases, attrs):
 
         if opts.model:
             model_opts = opts.model._meta
-            declared_fields = new_class.fields
+
+            # #1693 check the fields explicitly declared as attributes of the Resource
+            # class.
+            # if 'fields' property is defined, declared fields can only be included
+            # if they appear in the 'fields' iterable.
+            declared_fields = dict()
+            for field_name, field in new_class.fields.items():
+                if opts.fields is not None and field_name not in opts.fields:
+                    continue
+                declared_fields[field_name] = field
 
             field_list = []
             for f in sorted(model_opts.fields + model_opts.many_to_many):
@@ -98,7 +107,7 @@ def __new__(cls, name, bases, attrs):
                 )
 
             # Order as model fields first then declared fields by default
-            new_class.fields = OrderedDict([*field_list, *new_class.fields.items()])
+            new_class.fields = OrderedDict([*field_list, *declared_fields.items()])
 
             # add fields that follow relationships
             if opts.fields is not None:

tests/core/tests/admin_integration/test_action.py

@@ -104,7 +104,7 @@ def test_export_admin_action(self):
                     "_selected_action": "0",
                 },
             )
-            assert 200 <= response.status_code <= 399
+            self.assertTrue(200 <= response.status_code <= 399)
             mock_export_admin_action.assert_called()
 
     def test_get_export_data_raises_PermissionDenied_when_no_export_permission_assigned(

tests/core/tests/test_resources/test_resources.py

@@ -1260,9 +1260,9 @@ class EntryResource(resources.ModelResource):
 
             class Meta:
                 model = Entry
-                fields = ("id",)
+                fields = ("id", "username")
 
-            def after_save_instance(self, instance, row, **kwargs):
+            def after_save_instance(self, instance, row_, **kwargs):
                 using_transactions = kwargs.get("using_transactions", False)
                 dry_run = kwargs.get("dry_run", False)
                 if not using_transactions and dry_run:
@@ -1404,6 +1404,180 @@ class Meta:
         self.assertFalse(related_field_widget.use_natural_foreign_keys)
 
 
+class ModelResourceFieldDeclarations(TestCase):
+    class MyBookResource(resources.ModelResource):
+        author_email = fields.Field(
+            attribute="author_email", column_name="author_email"
+        )
+
+        class Meta:
+            model = Book
+            fields = ("id", "price")
+
+    def setUp(self):
+        self.book = Book.objects.create(name="Moonraker", price=".99")
+        self.resource = ModelResourceFieldDeclarations.MyBookResource()
+
+    @ignore_widget_deprecation_warning
+    def test_declared_field_not_imported(self):
+        self.assertEqual("", self.book.author_email)
+        rows = [
+            (self.book.id, "12.99", "jj@example.com"),
+        ]
+        dataset = tablib.Dataset(*rows, headers=["id", "price", "author_email"])
+        self.resource.import_data(dataset, raise_errors=True)
+        self.book.refresh_from_db()
+        # email should not be updated
+        self.assertEqual("", self.book.author_email)
+
+    @ignore_widget_deprecation_warning
+    def test_declared_field_not_exported(self):
+        self.assertEqual("", self.book.author_email)
+        data = self.resource.export()
+        self.assertFalse("author_email" in data.dict[0])
+
+
+class ModelResourceNoFieldDeclarations(TestCase):
+    # No `fields` declaration so all fields should be included
+    class MyBookResource(resources.ModelResource):
+        author_email = fields.Field(
+            attribute="author_email", column_name="author_email"
+        )
+
+        class Meta:
+            model = Book
+
+    def setUp(self):
+        self.book = Book.objects.create(name="Moonraker", price=".99")
+        self.resource = ModelResourceNoFieldDeclarations.MyBookResource()
+
+    @ignore_widget_deprecation_warning
+    def test_declared_field_imported(self):
+        self.assertEqual("", self.book.author_email)
+        rows = [
+            (self.book.id, "12.99", "jj@example.com"),
+        ]
+        dataset = tablib.Dataset(*rows, headers=["id", "price", "author_email"])
+        self.resource.import_data(dataset, raise_errors=True)
+        self.book.refresh_from_db()
+        # email should be updated
+        self.assertEqual("jj@example.com", self.book.author_email)
+
+    @ignore_widget_deprecation_warning
+    def test_declared_field_not_exported(self):
+        self.assertEqual("", self.book.author_email)
+        data = self.resource.export()
+        self.assertTrue("author_email" in data.dict[0])
+
+
+class ModelResourceExcludeDeclarations(TestCase):
+    class MyBookResource(resources.ModelResource):
+        author_email = fields.Field(
+            attribute="author_email", column_name="author_email"
+        )
+
+        class Meta:
+            model = Book
+            fields = ("id", "price")
+            exclude = ("author_email",)
+
+    def setUp(self):
+        self.book = Book.objects.create(name="Moonraker", price=".99")
+        self.resource = ModelResourceExcludeDeclarations.MyBookResource()
+
+    @ignore_widget_deprecation_warning
+    def test_excluded_field_not_imported(self):
+        self.assertEqual("", self.book.author_email)
+        rows = [
+            (self.book.id, "12.99", "jj@example.com"),
+        ]
+        dataset = tablib.Dataset(*rows, headers=["id", "price", "author_email"])
+        self.resource.import_data(dataset, raise_errors=True)
+        self.book.refresh_from_db()
+        # email should not be updated
+        self.assertEqual("", self.book.author_email)
+
+    @ignore_widget_deprecation_warning
+    def test_declared_field_not_exported(self):
+        self.assertEqual("", self.book.author_email)
+        data = self.resource.export()
+        self.assertFalse("author_email" in data.dict[0])
+
+
+class ModelResourceFieldsAndExcludeDeclarations(TestCase):
+    # Include the same field in both `fields` and `exclude`.
+    # `fields` should take precedence.
+    class MyBookResource(resources.ModelResource):
+        author_email = fields.Field(
+            attribute="author_email", column_name="author_email"
+        )
+
+        class Meta:
+            model = Book
+            fields = ("id", "price", "author_email")
+            exclude = ("author_email",)
+
+    def setUp(self):
+        self.book = Book.objects.create(name="Moonraker", price=".99")
+        self.resource = ModelResourceFieldsAndExcludeDeclarations.MyBookResource()
+
+    @ignore_widget_deprecation_warning
+    def test_excluded_field_not_imported(self):
+        self.assertEqual("", self.book.author_email)
+        rows = [
+            (self.book.id, "12.99", "jj@example.com"),
+        ]
+        dataset = tablib.Dataset(*rows, headers=["id", "price", "author_email"])
+        self.resource.import_data(dataset, raise_errors=True)
+        self.book.refresh_from_db()
+        # email should be updated
+        self.assertEqual("jj@example.com", self.book.author_email)
+
+    @ignore_widget_deprecation_warning
+    def test_declared_field_not_exported(self):
+        self.assertEqual("", self.book.author_email)
+        data = self.resource.export()
+        self.assertTrue("author_email" in data.dict[0])
+
+
+class ModelResourceDeclarationsNotInImportTest(TestCase):
+    # issue 1697
+    # Add a declared field to the Resource, which is not present in the import file.
+    # The import should succeed without issue.
+    class MyBookResource(resources.ModelResource):
+        author_email = fields.Field(
+            attribute="author_email", column_name="author_email"
+        )
+
+        class Meta:
+            model = Book
+            fields = (
+                "id",
+                "price",
+            )
+
+    def setUp(self):
+        self.resource = ModelResourceDeclarationsNotInImportTest.MyBookResource()
+
+    @ignore_widget_deprecation_warning
+    def test_excluded_field_not_imported(self):
+        rows = [
+            ("1", "12.99"),
+        ]
+        dataset = tablib.Dataset(*rows, headers=["id", "price"])
+        result = self.resource.import_data(dataset, raise_errors=True)
+        book = Book.objects.first()
+        self.assertEqual("", book.author_email)
+        self.assertEqual(1, result.totals["new"])
+
+    @ignore_widget_deprecation_warning
+    def test_excluded_field_not_exported(self):
+        self.book = Book.objects.create(name="Moonraker", price=".99")
+        self.assertEqual("", self.book.author_email)
+        data = self.resource.export()
+        self.assertFalse("author_email" in data.dict[0])
+
+
 class ModelResourceTransactionTest(TransactionTestCase):
     @skipUnlessDBFeature("supports_transactions")
     @ignore_widget_deprecation_warning
@@ -1660,8 +1834,7 @@ def setUp(self):
         def test_export_field_with_appropriate_format(self):
             resource = resources.modelresource_factory(model=BookWithChapters)()
             result = resource.export(BookWithChapters.objects.all())
-
-            assert result[0][3] == json.dumps(self.json_data)
+            self.assertEqual(result[0][3], json.dumps(self.json_data))
 
     class TestImportJsonField(TestCase):
         def setUp(self):