added explicit check for skip_diff inside skip_row()

import_export/resources.py

@@ -426,7 +426,12 @@ def skip_row(self, instance, original):
         """
         Returns ``True`` if ``row`` importing should be skipped.
 
-        Default implementation returns ``False`` unless skip_unchanged == True.
+        Default implementation returns ``False`` unless skip_unchanged == True,
+        or skip_diff == True.
+
+        If skip_diff is True, then no comparisons can be made because ``original``
+        will be None.
+
         Override this method to handle skipping rows meeting certain
         conditions.
 
@@ -438,7 +443,7 @@ def skip_row(self, instance, original):
                     return super(YourResource, self).skip_row(instance, original)
 
         """
-        if not self._meta.skip_unchanged:
+        if not self._meta.skip_unchanged or self._meta.skip_diff:
             return False
         for field in self.get_import_fields():
             try:
@@ -504,6 +509,7 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals
         """
         skip_diff = self._meta.skip_diff
         row_result = self.get_row_result_class()()
+        original = None
         try:
             self.before_import_row(row, **kwargs)
             instance, new = self.get_or_init_instance(instance_loader, row)
@@ -535,7 +541,7 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals
                     # validate_instance(), where they can be combined with model
                     # instance validation errors if necessary
                     import_validation_errors = e.update_error_dict(import_validation_errors)
-                if not skip_diff and self.skip_row(instance, original):
+                if self.skip_row(instance, original):
                     row_result.import_type = RowResult.IMPORT_TYPE_SKIP
                 else:
                     self.validate_instance(instance, import_validation_errors)

tests/core/tests/test_resources.py

@@ -1190,11 +1190,12 @@ def check_value(self, result, export_headers, expected_value):
                          expected_value)
 
 
-@mock.patch("copy.deepcopy")
 @mock.patch("import_export.resources.Diff", spec=True)
 class SkipDiffTest(TestCase):
     """
     Tests that the meta attribute 'skip_diff' means that no diff operations are called.
+    'copy.deepcopy' cannot be patched at class level because it causes interferes with
+    ``resources.Resource.__init__()``.
     """
     def setUp(self):
         class _BookResource(resources.ModelResource):
@@ -1207,13 +1208,14 @@ class Meta:
         self.dataset = tablib.Dataset(headers=['id', 'name', 'birthday'])
         self.dataset.append(['', 'A.A.Milne', '1882test-01-18'])
 
-    def test_skip_diff(self, mock_diff, mock_deep_copy):
-        self.resource.import_data(self.dataset, raise_errors=False)
-        mock_diff.return_value.compare_with.assert_not_called()
-        mock_diff.return_value.as_html.assert_not_called()
-        mock_deep_copy.assert_not_called()
+    def test_skip_diff(self, mock_diff):
+        with mock.patch("copy.deepcopy") as mock_deep_copy:
+            self.resource.import_data(self.dataset)
+            mock_diff.return_value.compare_with.assert_not_called()
+            mock_diff.return_value.as_html.assert_not_called()
+            mock_deep_copy.assert_not_called()
 
-    def test_skip_diff_for_delete_new_resource(self, mock_diff, mock_deep_copy):
+    def test_skip_diff_for_delete_new_resource(self, mock_diff):
         class BookResource(resources.ModelResource):
 
             class Meta:
@@ -1224,12 +1226,13 @@ def for_delete(self, row, instance):
                 return True
 
         resource = BookResource()
-        resource.import_data(self.dataset, raise_errors=False)
-        mock_diff.return_value.compare_with.assert_not_called()
-        mock_diff.return_value.as_html.assert_not_called()
-        mock_deep_copy.assert_not_called()
+        with mock.patch("copy.deepcopy") as mock_deep_copy:
+            resource.import_data(self.dataset)
+            mock_diff.return_value.compare_with.assert_not_called()
+            mock_diff.return_value.as_html.assert_not_called()
+            mock_deep_copy.assert_not_called()
 
-    def test_skip_diff_for_delete_existing_resource(self, mock_diff, mock_deep_copy):
+    def test_skip_diff_for_delete_existing_resource(self, mock_diff):
         book = Book.objects.create()
         class BookResource(resources.ModelResource):
 
@@ -1245,7 +1248,22 @@ def for_delete(self, row, instance):
 
         resource = BookResource()
 
-        resource.import_data(self.dataset, raise_errors=False, dry_run=True)
-        mock_diff.return_value.compare_with.assert_not_called()
-        mock_diff.return_value.as_html.assert_not_called()
-        mock_deep_copy.assert_not_called()
+        with mock.patch("copy.deepcopy") as mock_deep_copy:
+            resource.import_data(self.dataset, dry_run=True)
+            mock_diff.return_value.compare_with.assert_not_called()
+            mock_diff.return_value.as_html.assert_not_called()
+            mock_deep_copy.assert_not_called()
+
+    def test_skip_row_returns_false_when_skip_diff_is_true(self, mock_diff):
+        class BookResource(resources.ModelResource):
+
+            class Meta:
+                model = Book
+                skip_unchanged = True
+                skip_diff = True
+
+        resource = BookResource()
+
+        with mock.patch('import_export.resources.Resource.get_import_fields') as mock_get_import_fields:
+            resource.import_data(self.dataset, dry_run=True)
+            self.assertEquals(2, mock_get_import_fields.call_count)