Merge pull request #1347 from matthewhegarty/error-on-delete-from-adm…

…in-432

Fix for crash when deleting via admin site

docs/changelog.rst

@@ -7,6 +7,7 @@ Changelog
 - Run compilemessages command to keep .mo files in sync (#1299)
 - Add ability to rollback the import on validation error (#1339)
 - Fix missing migration on example app (#1346)
+- Fix crash when deleting via admin site (#1347)
 
 2.6.1 (2021-09-30)
 ------------------

docs/conf.py

@@ -1,4 +1,5 @@
-import sys, os
+import os
+import sys
 
 # If extensions (or modules to document with autodoc) are in another directory,
 # add these directories to sys.path here. If the directory is relative to the
@@ -9,6 +10,7 @@
 os.environ['DJANGO_SETTINGS_MODULE'] = 'settings'
 
 import django
+
 django.setup()
 
 # -- General configuration -----------------------------------------------------
@@ -42,6 +44,7 @@
 #
 try:
     from import_export import __version__
+
     # The short X.Y version.
     version = '.'.join(__version__.split('.')[:2])
     # The full version, including alpha/beta/rc tags.

import_export/resources.py

@@ -664,6 +664,7 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals
                         diff.compare_with(self, None, dry_run)
                 else:
                     row_result.import_type = RowResult.IMPORT_TYPE_DELETE
+                    row_result.add_instance_info(instance)
                     self.delete_instance(instance, using_transactions, dry_run)
                     if not skip_diff:
                         diff.compare_with(self, None, dry_run)
@@ -682,9 +683,7 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals
                     self.validate_instance(instance, import_validation_errors)
                     self.save_instance(instance, using_transactions, dry_run)
                     self.save_m2m(instance, row, using_transactions, dry_run)
-                    # Add object info to RowResult for LogEntry
-                    row_result.object_id = instance.pk
-                    row_result.object_repr = force_str(instance)
+                    row_result.add_instance_info(instance)
                 if not skip_diff:
                     diff.compare_with(self, instance, dry_run)
 

import_export/results.py

@@ -1,6 +1,7 @@
 from collections import OrderedDict
 
 from django.core.exceptions import NON_FIELD_ERRORS
+from django.utils.encoding import force_str
 from tablib import Dataset
 
 
@@ -32,6 +33,14 @@ def __init__(self):
         self.diff = None
         self.import_type = None
         self.raw_values = {}
+        self.object_id = None
+        self.object_repr = None
+
+    def add_instance_info(self, instance):
+        if instance is not None:
+            # Add object info to RowResult (e.g. for LogEntry)
+            self.object_id = getattr(instance, "pk", None)
+            self.object_repr = force_str(instance)
 
 
 class InvalidRow:

requirements/test.txt

@@ -3,3 +3,4 @@ psycopg2-binary
 mysqlclient
 coveralls
 chardet
+pytz

tests/core/exports/books-for-delete.csv

@@ -0,0 +1,2 @@
+id,name,author_email
+1,,test@example.com

tests/core/tests/test_admin_integration.py

@@ -13,7 +13,7 @@
     ImportMixin,
 )
 from core.models import Author, Book, Category, EBook, Parent
-from django.contrib.admin.models import LogEntry
+from django.contrib.admin.models import DELETION, LogEntry
 from django.contrib.auth.models import User
 from django.core.exceptions import PermissionDenied
 from django.core.files.uploadedfile import SimpleUploadedFile
@@ -89,6 +89,38 @@ def test_import(self):
                 1, 0, Book._meta.verbose_name_plural)
         )
 
+    def test_delete_from_admin(self):
+        # test delete from admin site (see #432)
+
+        # create a book which can be deleted
+        b = Book.objects.create(id=1)
+
+        input_format = '0'
+        filename = os.path.join(
+            os.path.dirname(__file__),
+            os.path.pardir,
+            'exports',
+            'books-for-delete.csv')
+        with open(filename, "rb") as f:
+            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)
+
+        # check the LogEntry was created as expected
+        deleted_entry = LogEntry.objects.latest('id')
+        self.assertEqual("delete through import_export", deleted_entry.change_message)
+        self.assertEqual(DELETION, deleted_entry.action_flag)
+        self.assertEqual(b.id, int(deleted_entry.object_id))
+        self.assertEqual("", deleted_entry.object_repr)
+
     @override_settings(TEMPLATE_STRING_IF_INVALID='INVALID_VARIABLE')
     def test_import_mac(self):
         # GET the import form

tests/core/tests/test_results.py

@@ -1,3 +1,4 @@
+from core.models import Book
 from django.core.exceptions import ValidationError
 from django.test.testcases import TestCase
 from tablib import Dataset
@@ -29,4 +30,26 @@ def test_result_append_failed_row_with_wrapped_error(self):
         error = Error(ValidationError('some error'))
         row_result.errors = [error]
         self.result.append_failed_row(self.dataset.dict[0], row_result.errors[0])
-        self.assertEqual(target, self.result.failed_dataset.dict)
+        self.assertEqual(target, self.result.failed_dataset.dict)
+
+    def test_add_instance_info_null_instance(self):
+        row_result = RowResult()
+        row_result.add_instance_info(None)
+        self.assertEqual(None, row_result.object_id)
+        self.assertEqual(None, row_result.object_repr)
+
+    def test_add_instance_info_no_instance_pk(self):
+        row_result = RowResult()
+        row_result.add_instance_info(Book())
+        self.assertEqual(None, row_result.object_id)
+        self.assertEqual("", row_result.object_repr)
+
+    def test_add_instance_info(self):
+        class BookWithObjectRepr(Book):
+            def __str__(self):
+                return self.name
+
+        row_result = RowResult()
+        row_result.add_instance_info(BookWithObjectRepr(pk=1, name="some book"))
+        self.assertEqual(1, row_result.object_id)
+        self.assertEqual("some book", row_result.object_repr)