merge release-4 branch

docs/admin_integration.rst

@@ -37,7 +37,7 @@ dropdown in the UI.
 .. figure:: _static/images/django-import-export-change.png
 
    A screenshot of the change view with Import and Export buttons.
-   
+
 Importing
 ---------
 
@@ -137,6 +137,14 @@ For example, if using django-storages, you can configure s3 as a temporary stora
         },
     }
 
+.. _format_ui_error_messages:
+
+How to format UI error messages
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Admin UI import error messages can be formatted using the :attr:`~import_export.admin.ImportMixin.import_error_display`
+attribute.
+
 Exporting
 ---------
 

docs/changelog.rst

@@ -1,7 +1,9 @@
 Changelog
 =========
 
-Please refer to :doc:`release notes<release_notes>`.
+.. warning::
+
+    Version 4 introduces breaking changes.  Please refer to :doc:`release notes<release_notes>`.
 
 4.0.0-beta.3 (unreleased)
 -------------------------
@@ -10,6 +12,7 @@ Please refer to :doc:`release notes<release_notes>`.
 - Added customizable ``MediaStorage`` (#1708)
 - Relocated admin integration section from advanced_usage.rst into new file (#1713)
 - Fix slow export with ForeignKey id (#1717)
+- Added customization of Admin UI import error messages (#1727)
 
 4.0.0-beta.2 (2023-12-09)
 -------------------------
@@ -19,7 +22,7 @@ Please refer to :doc:`release notes<release_notes>`.
 - Support export from model change form (#1687)
 - Updated Admin UI to track deleted and skipped Imports (#1691)
 - Import form defaults to read-only field if only one format defined (#1690)
-- Refactored :module:`~import_export.resources` into separate modules for ``declarative`` and ``options`` (#1695)
+- Refactored :mod:`~import_export.resources` into separate modules for ``declarative`` and ``options`` (#1695)
 - fix multiple inheritance not setting options (#1696)
 - Refactored tests to remove dependencies between tests (#1703)
 - Handle python3.12 datetime deprecations (#1705)

docs/faq.rst

@@ -104,7 +104,7 @@ Please refer to `this issue <https://github.com/django-import-export/django-impo
 How to hide stack trace in UI error messages
 --------------------------------------------
 
-Please refer to `this issue <https://github.com/django-import-export/django-import-export/issues/1257#issuecomment-952276485>`_.
+Please refer to :ref:`format_ui_error_messages`.
 
 Ids incremented twice during import
 -----------------------------------

docs/release_notes.rst

@@ -70,6 +70,8 @@ Deprecations
 
 * Use of ``ExportViewFormMixin`` is deprecated.  See `this issue <https://github.com/django-import-export/django-import-export/issues/1666>`_.
 
+* See :ref:`renamed_methods`.
+
 Admin UI
 ========
 
@@ -97,6 +99,21 @@ Success message
 The success message shown on successful import has been updated to include the number of 'deleted' and 'skipped' rows.
 See `this PR <https://github.com/django-import-export/django-import-export/issues/1691>`_.
 
+Import error messages
+---------------------
+
+The default error message for import errors has been modified to simplify the format.
+Error messages now contain the error message only by default.  The row and traceback are not presented.
+
+The original format can be restored by setting :attr:`~import_export.admin.ImportMixin.import_error_display` on the
+Admin class definition.  For example::
+
+  class BookAdmin(ImportExportModelAdmin):
+    import_error_display = ("message", "row", "traceback")
+
+
+See `this issue <https://github.com/django-import-export/django-import-export/issues/1724>`_.
+
 API changes
 ===========
 
@@ -121,6 +138,8 @@ method calls, and to allow easier extensibility.
 :class:`import_export.resources.Resource`
 -----------------------------------------
 
+.. _renamed_methods:
+
 Renamed methods
 ^^^^^^^^^^^^^^^
 
@@ -185,41 +204,41 @@ This section describes methods in which the parameters have changed.
 
    * - ``save_m2m(self, obj, data, using_transactions, dry_run)``
      - ``save_m2m(self, instance, row, **kwargs)``
-     - * ``dry_run`` param now in ``kwargs``
-       * ``using_transactions`` param now in ``kwargs``
-       * ``row`` added as mandatory arg
+     - * ``row`` added as mandatory arg
        * ``obj`` renamed to ``instance``
        * ``data`` renamed to ``row``
+       * ``dry_run`` param now in ``kwargs``
+       * ``using_transactions`` param now in ``kwargs``
 
    * - ``before_save_instance(self, instance, using_transactions, dry_run)``
      - ``before_save_instance(self, instance, row, **kwargs)``
-     - * ``dry_run`` param now in ``kwargs``
+     - * ``row`` added as mandatory arg
+       * ``dry_run`` param now in ``kwargs``
        * ``using_transactions`` param now in ``kwargs``
-       * ``row`` added as mandatory arg
 
    * - ``after_save_instance(self, instance, using_transactions, dry_run)``
      - ``after_save_instance(self, instance, row, **kwargs)``
-     - * ``dry_run`` param now in ``kwargs``
+     - * ``row`` added as mandatory arg
+       * ``dry_run`` param now in ``kwargs``
        * ``using_transactions`` param now in ``kwargs``
-       * ``row`` added as mandatory arg
 
    * - ``delete_instance(self, instance, using_transactions=True, dry_run=False)``
      - ``delete_instance(self, instance, row, **kwargs)``
-     - * ``dry_run`` param now in ``kwargs``
+     - * ``row`` added as mandatory arg
+       * ``dry_run`` param now in ``kwargs``
        * ``using_transactions`` param now in ``kwargs``
-       * ``row`` added as mandatory arg
 
    * - ``before_delete_instance(self, instance, dry_run)``
      - ``before_delete_instance(self, instance, row, **kwargs)``
-     - * ``dry_run`` param now in ``kwargs``
+     - * ``row`` added as mandatory arg
+       * ``dry_run`` param now in ``kwargs``
        * ``using_transactions`` param now in ``kwargs``
-       * ``row`` added as mandatory arg
 
    * - ``after_delete_instance(self, instance, dry_run)``
      - ``after_delete_instance(self, instance, row, **kwargs)``
-     - * ``dry_run`` param now in ``kwargs``
+     - * ``row`` added as mandatory arg
+       * ``dry_run`` param now in ``kwargs``
        * ``using_transactions`` param now in ``kwargs``
-       * ``row`` added as mandatory arg
 
    * - ``before_export(self, queryset, *args, **kwargs)``
      - ``before_export(self, queryset, **kwargs)``

import_export/admin.py

@@ -83,6 +83,10 @@ class ImportMixin(BaseImportMixin, ImportExportMixinBase):
     confirm_form_class = ConfirmImportForm
     #: import data encoding
     from_encoding = "utf-8-sig"
+    #: control which UI elements appear when import errors are displayed.
+    #: Available options: 'message', 'row', 'traceback'
+    import_error_display = ("message",)
+
     skip_admin_log = None
     # storage class for saving temporary files
     tmp_storage_class = None
@@ -535,6 +539,7 @@ def import_action(self, request, *args, **kwargs):
             )
             for resource in resources
         ]
+        context["import_error_display"] = self.import_error_display
 
         request.current_app = self.admin_site.name
         return TemplateResponse(request, [self.import_template_name], context)

import_export/resources.py

@@ -4,6 +4,7 @@
 from collections import OrderedDict
 from copy import deepcopy
 from html import escape
+from warnings import warn
 
 import tablib
 from diff_match_patch import diff_match_patch
@@ -423,6 +424,18 @@ def import_field(self, field, instance, row, is_m2m=False, **kwargs):
     def get_import_fields(self):
         return [self.fields[f] for f in self.get_import_order()]
 
+    def import_obj(self, obj, data, dry_run, **kwargs):
+        warn(
+            "The 'import_obj' method is deprecated and will be replaced "
+            "with 'import_instance(self, instance, row, **kwargs)' "
+            "in a future release.  Refer to Release Notes for details.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        if dry_run is True:
+            kwargs.update({"dry_run": dry_run})
+        self.import_instance(obj, data, **kwargs)
+
     def import_instance(self, instance, row, **kwargs):
         r"""
         Traverses every field in this Resource and calls
@@ -615,6 +628,18 @@ def after_import_row(self, row, row_result, **kwargs):
         """
         pass
 
+    def after_import_instance(self, instance, new, row_number=None, **kwargs):
+        warn(
+            "The 'after_import_instance' method is deprecated and will be replaced "
+            "with 'after_init_instance(self, instance, new, row, **kwargs)' "
+            "in a future release.  Refer to Release Notes for details.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        if row_number is not None:
+            kwargs.update({"row_number": row_number})
+        self.after_init_instance(instance, new, None, **kwargs)
+
     def after_init_instance(self, instance, new, row, **kwargs):
         r"""
         Override to add additional logic. Does nothing by default.

import_export/templates/admin/import_export/import.html

@@ -85,15 +85,25 @@ <h2>{% translate "Errors" %}</h2>
           <div class="traceback">{{ error.traceback|linebreaks }}</div>
         </li>
         {% endfor %}
+        {% block import_error_list %}
         {% for line, errors in result.row_errors %}
           {% for error in errors %}
-            <li>
-              {% translate "Line number" %}: {{ line }} - {{ error.error }}
-              <div><code>{{ error.row.values|join:", " }}</code></div>
-              <div class="traceback">{{ error.traceback|linebreaks }}</div>
+            {% block import_error_list_item %}
+            <li class="import-error-li">
+              {% if "message" in import_error_display %}
+                <div class="import-error-display-message">{% translate "Line number" %}: {{ line }} - {{ error.error }}</div>
+              {% endif %}
+              {% if "row" in import_error_display %}
+                <div class="import-error-display-row">{{ error.row.values|join:", " }}</div>
+              {% endif %}
+              {% if "traceback" in import_error_display %}
+              <div class="import-error-display-traceback">{{ error.traceback|linebreaks }}</div>
+              {% endif %}
             </li>
+            {% endblock %}
           {% endfor %}
         {% endfor %}
+        {% endblock %}
       </ul>
     {% endblock %}
 

tests/core/tests/admin_integration/test_import.py

@@ -1,5 +1,6 @@
 import os
 import warnings
+from io import StringIO
 from unittest import mock
 from unittest.mock import MagicMock, patch
 
@@ -9,7 +10,10 @@
 from core.tests.admin_integration.mixins import AdminTestMixin
 from core.tests.utils import ignore_widget_deprecation_warning
 from django.contrib.admin.models import DELETION, LogEntry
+from django.contrib.admin.sites import AdminSite
+from django.contrib.auth.models import User
 from django.http import HttpRequest
+from django.test import RequestFactory
 from django.test.testcases import TestCase, TransactionTestCase
 from django.test.utils import override_settings
 from django.utils.translation import gettext_lazy as _
@@ -480,6 +484,75 @@ def test_export_empty_import_formats(self):
             self.client.get(self.book_import_url)
 
 
+class TestImportErrorMessageFormat(AdminTestMixin, TestCase):
+    # issue 1724
+
+    def setUp(self):
+        super().setUp()
+        self.csvdata = "id,name,author\r\n" "1,Ulysses,666\r\n"
+        self.filedata = StringIO(self.csvdata)
+        self.data = {"format": "0", "import_file": self.filedata}
+        self.model_admin = BookAdmin(Book, AdminSite())
+
+        factory = RequestFactory()
+        self.request = factory.post(self.book_import_url, self.data, follow=True)
+        self.request.user = User.objects.create_user("admin1")
+
+    @ignore_widget_deprecation_warning
+    def test_result_error_display_default(self):
+        response = self.model_admin.import_action(self.request)
+        response.render()
+        self.assertIn("import-error-display-message", str(response.content))
+        self.assertIn(
+            "Line number: 1 - Author matching query does not exist.",
+            str(response.content),
+        )
+        self.assertNotIn("import-error-display-row", str(response.content))
+        self.assertNotIn("import-error-display-traceback", str(response.content))
+
+    @ignore_widget_deprecation_warning
+    def test_result_error_display_message_only(self):
+        self.model_admin.import_error_display = ("message",)
+
+        response = self.model_admin.import_action(self.request)
+        response.render()
+        self.assertIn(
+            "Line number: 1 - Author matching query does not exist.",
+            str(response.content),
+        )
+        self.assertIn("import-error-display-message", str(response.content))
+        self.assertNotIn("import-error-display-row", str(response.content))
+        self.assertNotIn("import-error-display-traceback", str(response.content))
+
+    @ignore_widget_deprecation_warning
+    def test_result_error_display_row_only(self):
+        self.model_admin.import_error_display = ("row",)
+
+        response = self.model_admin.import_action(self.request)
+        response.render()
+        self.assertNotIn(
+            "Line number: 1 - Author matching query does not exist.",
+            str(response.content),
+        )
+        self.assertNotIn("import-error-display-message", str(response.content))
+        self.assertIn("import-error-display-row", str(response.content))
+        self.assertNotIn("import-error-display-traceback", str(response.content))
+
+    @ignore_widget_deprecation_warning
+    def test_result_error_display_traceback_only(self):
+        self.model_admin.import_error_display = ("traceback",)
+
+        response = self.model_admin.import_action(self.request)
+        response.render()
+        self.assertNotIn(
+            "Line number: 1 - Author matching query does not exist.",
+            str(response.content),
+        )
+        self.assertNotIn("import-error-display-message", str(response.content))
+        self.assertNotIn("import-error-display-row", str(response.content))
+        self.assertIn("import-error-display-traceback", str(response.content))
+
+
 class ConfirmImportEncodingTest(AdminTestMixin, TestCase):
     """Test handling 'confirm import' step using different file encodings
     and storage types.