Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for 974 #1717

Merged
merged 2 commits into from
Dec 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Please refer to :doc:`release notes<release_notes>`.
- Fix issue where declared Resource fields not defined in ``fields`` are still imported (#1702)
- Added customizable ``MediaStorage`` (#1708)
- Relocated admin integration section from advanced_usage.rst into new file (#1713)
- Fix slow export with ForeignKey id (#1717)

4.0.0-beta.2 (2023-12-09)
-------------------------
Expand Down
13 changes: 11 additions & 2 deletions import_export/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from django.core.paginator import Paginator
from django.db import connections, router
from django.db.models import fields
from django.db.models.fields.related import ForeignKey
from django.db.models.query import QuerySet
from django.db.transaction import TransactionManagementError, set_rollback
from django.utils.encoding import force_str
Expand Down Expand Up @@ -1196,9 +1197,17 @@ def field_from_django_field(cls, field_name, django_field, readonly):

FieldWidget = cls.widget_from_django_field(django_field)
widget_kwargs = cls.widget_kwargs_for_field(field_name, django_field)

attribute = field_name
column_name = field_name
# To solve #974
if isinstance(django_field, ForeignKey) and "__" not in column_name:
matthewhegarty marked this conversation as resolved.
Show resolved Hide resolved
attribute += "_id"
widget_kwargs["key_is_id"] = True
Comment on lines +1201 to +1206
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I am wrong, please, but to me, it feels like this belongs more in widget_kwargs_for_field than here.


field = cls.DEFAULT_RESOURCE_FIELD(
attribute=field_name,
column_name=field_name,
attribute=attribute,
column_name=column_name,
widget=FieldWidget(**widget_kwargs),
readonly=readonly,
default=django_field.default,
Expand Down
19 changes: 17 additions & 2 deletions import_export/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,9 +476,17 @@ class Meta:
related object, default to False
"""

def __init__(self, model, field="pk", use_natural_foreign_keys=False, **kwargs):
def __init__(
self,
model,
field="pk",
use_natural_foreign_keys=False,
key_is_id=False,
**kwargs,
):
self.model = model
self.field = field
self.key_is_id = key_is_id
self.use_natural_foreign_keys = use_natural_foreign_keys
super().__init__(**kwargs)

Expand Down Expand Up @@ -528,7 +536,10 @@ def clean(self, value, row=None, **kwargs):
return self.model.objects.get_by_natural_key(*value)
else:
lookup_kwargs = self.get_lookup_kwargs(value, row, **kwargs)
return self.get_queryset(value, row, **kwargs).get(**lookup_kwargs)
obj = self.get_queryset(value, row, **kwargs).get(**lookup_kwargs)
if self.key_is_id:
return obj.id
return obj
else:
return None

Expand All @@ -551,6 +562,10 @@ def render(self, value, obj=None):
``coerce_to_string`` has no effect on the return value.
"""
self._obj_deprecation_warning(obj)

if self.key_is_id:
return value or ""

if value is None:
return ""

Expand Down
18 changes: 18 additions & 0 deletions tests/core/tests/test_resources/test_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,24 @@ def test_export(self):
dataset = self.resource.export(queryset=Book.objects.all())
self.assertEqual(len(dataset), 1)

@ignore_widget_deprecation_warning
def test_export_with_foreign_keys(self):
"""
Test that export() containing foreign keys doesn't generate
extra query for every row.
Fixes #974
"""
author = Author.objects.create()
self.book.author = author
self.book.save()
Book.objects.create(name="Second book", author=Author.objects.create())
Book.objects.create(name="Third book", author=Author.objects.create())

with self.assertNumQueries(3):
dataset = self.resource.export(Book.objects.prefetch_related("categories"))
self.assertEqual(dataset.dict[0]["author"], author.pk)
self.assertEqual(len(dataset), 3)

@ignore_widget_deprecation_warning
def test_export_iterable(self):
with self.assertNumQueries(2):
Expand Down