Merge pull request #543 from copperleaftech/fix-logentry-creation-exc…

…eption

Fix exception in generate_log_entries()

import_export/resources.py

@@ -428,7 +428,6 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals
             will be rolled back.
         """
         row_result = self.get_row_result_class()()
-        row_result.import_type = RowResult.IMPORT_TYPE_ERROR
         try:
             self.before_import_row(row, **kwargs)
             instance, new = self.get_or_init_instance(instance_loader, row)
@@ -464,6 +463,7 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals
                 row_result.object_repr = force_text(instance)
             self.after_import_row(row, row_result, **kwargs)
         except Exception as e:
+            row_result.import_type = RowResult.IMPORT_TYPE_ERROR
             # There is no point logging a transaction error for each row
             # when only the original error is likely to be relevant
             if not isinstance(e, TransactionManagementError):

tests/core/tests/admin_integration_tests.py

@@ -5,10 +5,12 @@
 from django.test.utils import override_settings
 from django.test.testcases import TestCase
 from django.contrib.auth.models import User
+from django.core.files.uploadedfile import SimpleUploadedFile
 from django.utils.translation import ugettext_lazy as _
 from django.contrib.admin.models import LogEntry
+from tablib import Dataset
 
-from core.admin import BookAdmin, AuthorAdmin
+from core.admin import BookAdmin, AuthorAdmin, BookResource
 from core.models import Category, Parent
 
 
@@ -198,6 +200,57 @@ def test_import_log_entry_with_fk(self):
         self.assertEqual(child.object_repr, 'Some - child of Some Parent')
         self.assertEqual(child.object_id, str(1))
 
+    def test_logentry_creation_with_import_obj_exception(self):
+        # from http://mail.python.org/pipermail/python-dev/2008-January/076194.html
+        def monkeypatch_method(cls):
+            def decorator(func):
+                setattr(cls, func.__name__, func)
+                return func
+            return decorator
+
+        # Cause an exception in import_row, but only after import is confirmed,
+        # so a failure only occurs when ImportMixin.process_import is called.
+        class R(BookResource):
+            def import_obj(self, obj, data, dry_run):
+                if dry_run:
+                    super(R, self).import_obj(obj, data, dry_run)
+                else:
+                    raise Exception
+
+        @monkeypatch_method(BookAdmin)
+        def get_resource_class(self):
+            return R
+
+        # Verify that when an exception occurs in import_row, when raise_errors is False,
+        # the returned row result has a correct import_type value,
+        # so generating log entries does not fail.
+        @monkeypatch_method(BookAdmin)
+        def process_dataset(self, dataset, confirm_form, request, *args, **kwargs):
+            resource = self.get_import_resource_class()(**self.get_import_resource_kwargs(request, *args, **kwargs))
+            return resource.import_data(dataset,
+                                        dry_run=False,
+                                        raise_errors=False,
+                                        file_name=confirm_form.cleaned_data['original_file_name'],
+                                        user=request.user,
+                                        **kwargs)
+
+        dataset = Dataset(headers=["id","name","author_email"])
+        dataset.append([1, "Test 1", "test@example.com"])
+        input_format = '0'
+        content = dataset.csv
+        f = SimpleUploadedFile("data.csv", content.encode("utf-8"), content_type="text/csv")
+        data = {
+            "input_format": input_format,
+            "import_file": f,
+        }
+        response = self.client.post('/admin/core/book/import/', data)
+        self.assertEqual(response.status_code, 200)
+        confirm_form = response.context['confirm_form']
+        data = confirm_form.initial
+        response = self.client.post('/admin/core/book/process_import/', data,
+                                    follow=True)
+        self.assertEqual(response.status_code, 200)
+
 
 class ExportActionAdminIntegrationTest(TestCase):