Skip to content

Commit

Permalink
Handle crash when importing custom column name (#1822)
Browse files Browse the repository at this point in the history
* updated ebook test

* added fix and tests

* updated tests

* tidied method implementations

* updated changelog

* fix don't crash for custom column_name declaration
  • Loading branch information
matthewhegarty committed May 13, 2024
1 parent bdc35eb commit 33f1807
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 9 deletions.
7 changes: 5 additions & 2 deletions docs/advanced_usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ Or the ``exclude`` option to blacklist fields::
If both ``fields`` and ``exclude`` are declared, the ``fields`` declaration takes precedence, and ``exclude`` is
ignored.

In cases where a :ref:`custom column name<field_declaration>` is used, declare the name of the model attribute in the
``fields`` list., not the column alias.

.. _field_ordering:

Field ordering
Expand Down Expand Up @@ -71,8 +74,6 @@ If no ``fields``, ``import_order`` or ``export_order`` is defined then fields ar
class. The order of declared fields in the model instance is preserved, and any non-model fields are last in the
ordering.

.. _field_declaration:

Model relations
---------------

Expand All @@ -91,6 +92,8 @@ export.
Note that declaring the relationship using this syntax sets ``field`` as readonly, meaning this field will be skipped
when importing data. To understand how to import model relations, see :ref:`import_model_relations`.

.. _field_declaration:

Explicit field declaration
--------------------------

Expand Down
2 changes: 2 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Changelog
4.0.2 (unreleased)
------------------

- fix export with custom column name (`1821 <https://github.com/django-import-export/django-import-export/pull/1821>`_)
- fix allow ``column_name`` to be declared in ``fields`` list (`1815 <https://github.com/django-import-export/django-import-export/pull/1815>`_)
- fix export with custom column name (`1821 <https://github.com/django-import-export/django-import-export/pull/1821>`_)

4.0.1 (2024-05-08)
Expand Down
7 changes: 6 additions & 1 deletion import_export/declarative.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ def __new__(cls, name, bases, attrs):
# 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:
column_name = field.column_name
if (
opts.fields is not None
and field_name not in opts.fields
and column_name not in opts.fields
):
continue
declared_fields[field_name] = field

Expand Down
20 changes: 16 additions & 4 deletions import_export/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,9 +432,17 @@ def import_field(self, field, instance, row, is_m2m=False, **kwargs):

def get_import_fields(self):
import_fields = []
for f in self.fields:
import_fields.append(f)
return [self.fields[f] for f in self.get_import_order()]
for field_name in self.get_import_order():
if field_name in self.fields:
import_fields.append(self.fields[field_name])
continue
# issue 1815
# allow for fields to be referenced by column_name in `fields` list
for field in self.fields.values():
if field.column_name == field_name:
import_fields.append(field)
continue
return import_fields

def import_obj(self, obj, data, dry_run, **kwargs):
warn(
Expand Down Expand Up @@ -1118,7 +1126,11 @@ def _get_ordered_field_names(self, order_field):

order = list()
[order.append(f) for f in defined_fields if f not in order]
return tuple(order) + tuple(k for k in self.fields if k not in order)
declared_fields = []
for field_name, field in self.fields.items():
if field_name not in order and field.column_name not in order:
declared_fields.append(field_name)
return tuple(order) + tuple(declared_fields)

def _is_using_transactions(self, kwargs):
return kwargs.get("using_transactions", False)
Expand Down
55 changes: 54 additions & 1 deletion tests/core/tests/admin_integration/test_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@
from unittest import mock
from unittest.mock import patch

from core.admin import AuthorAdmin, BookAdmin, CustomBookAdmin, ImportMixin
from core.admin import (
AuthorAdmin,
BookAdmin,
CustomBookAdmin,
EBookResource,
ImportMixin,
)
from core.models import Author, Book, EBook, Parent
from core.tests.admin_integration.mixins import AdminTestMixin
from django.contrib.admin.models import DELETION, LogEntry
Expand Down Expand Up @@ -954,3 +960,50 @@ def test_import_preview_order(self):
"</tr>"
)
self.assertRegex(str(response.content), target_row_re)


class CustomColumnNameImportTest(AdminTestMixin, TestCase):
"""Test preview order displayed correctly (issue 1815)."""

fixtures = ["author"]

def setUp(self):
super().setUp()
EBookResource._meta.fields = ("id", "author_email", "name", "published_date")

def tearDown(self):
super().tearDown()
EBookResource._meta.fields = ("id", "author_email", "name", "published")

def test_import_preview_order(self):
author_id = Author.objects.first().id
response = self._do_import_post(
self.ebook_import_url,
"books.csv",
input_format="0",
data={"author": author_id},
)
# test header rendered in correct order
target_header_re = (
r"<thead>[\\n\s]+"
r"<tr>[\\n\s]+"
r"<th></th>[\\n\s]+"
r"<th>id</th>[\\n\s]+"
r"<th>author_email</th>[\\n\s]+"
r"<th>name</th>[\\n\s]+"
r"<th>published_date</th>[\\n\s]+"
r"</tr>[\\n\s]+"
"</thead>"
)
self.assertRegex(str(response.content), target_header_re)
# test row rendered in correct order
target_row_re = (
r'<tr class="new">[\\n\s]+'
r'<td class="import-type">[\\n\s]+New[\\n\s]+</td>[\\n\s]+'
r'<td><ins style="background:#e6ffe6;">1</ins></td>[\\n\s]+'
r'<td><ins style="background:#e6ffe6;">test@example.com</ins></td>[\\n\s]+'
r'<td><ins style="background:#e6ffe6;">Some book</ins></td>[\\n\s]+'
r"<td><span>None</span></td>[\\n\s]+"
"</tr>"
)
self.assertRegex(str(response.content), target_row_re)
33 changes: 32 additions & 1 deletion tests/core/tests/test_resources/test_import_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@
from unittest.mock import patch

import tablib
from core.models import Author, Book, Category
from core.models import Author, Book, Category, EBook
from core.tests.resources import AuthorResource, BookResource
from django.test import TestCase

from import_export import exceptions, fields, resources, widgets
from import_export.fields import Field
from import_export.resources import ModelResource


class AfterImportComparisonTest(TestCase):
Expand Down Expand Up @@ -372,3 +374,32 @@ def test_import_row_with_no_defined_id_field(self):
self.resource = AuthorResource()
self.resource.import_data(dataset)
self.assertEqual(1, Author.objects.count())


class CustomColumnNameImportTest(TestCase):
"""
If a custom field is declared, import should work if either the Field's
attribute name or column name is referenced in the ``fields`` list (issue 1815).
"""

fixtures = ["author"]

class _EBookResource(ModelResource):
published = Field(attribute="published", column_name="published_date")

class Meta:
model = EBook
fields = ("id", "name", "published_date")

def setUp(self):
super().setUp()
self.resource = CustomColumnNameImportTest._EBookResource()

def test_import_with_column_alias_in_fields_list(self):
self.assertEqual(0, EBook.objects.count())
dataset = tablib.Dataset(
*[(1, "Moonraker", "1955-04-05")], headers=["id", "name", "published_date"]
)
self.resource.import_data(dataset, raise_errors=True)
self.assertEqual(1, EBook.objects.count())
self.assertEqual(date(1955, 4, 5), EBook.objects.first().published)

0 comments on commit 33f1807

Please sign in to comment.