Skip to content

Commit

Permalink
Added overridable method to restrict pks when exporting (#1827)
Browse files Browse the repository at this point in the history
* updated ebook test

* code modifications

* reverted changes from other PR

* added performance fix and test

* updated changelog

* added documentation warning

* updated changelog
  • Loading branch information
matthewhegarty committed May 13, 2024
1 parent 2f108b6 commit 0e0071a
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 11 deletions.
7 changes: 6 additions & 1 deletion docs/admin_integration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,12 @@ this to refer to your own model instances. In the example application, the 'Cat

When 'Go' is clicked for the selected items, the user will be directed to the
:ref:`export 'confirm' page<export_confirm>`. It is possible to disable this extra step by setting the
:ref:`import_export_skip_admin_action_export_ui` flag
:ref:`import_export_skip_admin_action_export_ui` flag.

.. note::

If deploying to a multi-tenant environment, you may need to use the to ensure that one set of users cannot export
data belonging to another set. See :meth:`~import_export.admin.ExportMixin.get_valid_export_item_pks`.

Export from model instance change form
--------------------------------------
Expand Down
1 change: 1 addition & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Changelog
- fix clash between ``key_is_id`` and ``use_natural_foreign_keys`` (`1824 <https://github.com/django-import-export/django-import-export/pull/1824>`_)
- remove unreachable code (`1825 <https://github.com/django-import-export/django-import-export/pull/1825>`_)
- fix issue with widget assignment for custom ``ForeignKey`` subclasses (`1826 <https://github.com/django-import-export/django-import-export/pull/1826>`_)
- performance: select of valid pks for export restricted to action exports (`1827 <https://github.com/django-import-export/django-import-export/pull/1827>`_)

4.0.1 (2024-05-08)
------------------
Expand Down
33 changes: 24 additions & 9 deletions import_export/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -721,11 +721,14 @@ def export_action(self, request):
self.get_export_resource_classes(request),
data=request.POST or None,
)
form.fields["export_items"] = MultipleChoiceField(
widget=MultipleHiddenInput,
required=False,
choices=[(o.pk, o.pk) for o in self.model.objects.all()],
)
if request.POST and "export_items" in request.POST:
# this field is instantiated if the export is POSTed from the
# 'action' drop down
form.fields["export_items"] = MultipleChoiceField(
widget=MultipleHiddenInput,
required=False,
choices=[(pk, pk) for pk in self.get_valid_export_item_pks(request)],
)
if form.is_valid():
file_format = formats[int(form.cleaned_data["format"])]()

Expand Down Expand Up @@ -762,6 +765,18 @@ def export_action(self, request):
request.current_app = self.admin_site.name
return TemplateResponse(request, [self.export_template_name], context=context)

def get_valid_export_item_pks(self, request):
"""
Returns a list of valid pks for export.
This is used to validate which objects can be exported when exports are
triggered from the Admin UI 'action' dropdown.
This can be overridden to filter returned pks for performance and/or security
reasons.
:param request: The request object.
:returns: a list of valid pks (by default is all pks in table).
"""
return self.model.objects.all().values_list("pk", flat=True)

def changelist_view(self, request, extra_context=None):
if extra_context is None:
extra_context = {}
Expand Down Expand Up @@ -835,6 +850,7 @@ def change_view(self, request, object_id, form_url="", extra_context=None):
)

def response_change(self, request, obj):
# called if the export is triggered from the instance detail page.
if "_export-item" in request.POST:
return self.export_admin_action(
request, self.model.objects.filter(id=obj.id)
Expand Down Expand Up @@ -864,16 +880,15 @@ def export_admin_action(self, request, queryset):

form_type = self.get_export_form_class()
formats = self.get_export_formats()
export_items = list(queryset.values_list("pk", flat=True))
form = form_type(
formats=formats,
resources=self.get_export_resource_classes(request),
initial={"export_items": list(queryset.values_list("pk", flat=True))},
initial={"export_items": export_items},
)
# selected items are to be stored as a hidden input on the form
form.fields["export_items"] = MultipleChoiceField(
widget=MultipleHiddenInput,
required=False,
choices=[(str(o), str(o)) for o in queryset.all()],
widget=MultipleHiddenInput, required=False, choices=export_items
)
context = self.init_request_context_data(request, form)

Expand Down
3 changes: 3 additions & 0 deletions import_export/templates/admin/import_export/export.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
{% endblock %}

{% block content %}
{% if form.errors %}
{{ form.errors }}
{% endif %}
<form action="{{ export_url }}" method="POST">
{% csrf_token %}
{# export request has originated from an Admin UI action #}
Expand Down
19 changes: 19 additions & 0 deletions tests/core/tests/admin_integration/test_action_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,25 @@ def test_export_admin_action(self):
self.assertTrue(200 <= response.status_code <= 399)
mock_export_admin_action.assert_called()

def test_export_admin_action_with_restricted_pks(self):
data = {
"format": "0",
"export_items": [str(self.cat1.id)],
**self.resource_fields_payload,
}
# mock returning a set of pks which is not in the submitted range
with mock.patch(
"import_export.admin.ExportMixin.get_valid_export_item_pks"
) as mock_valid_pks:
mock_valid_pks.return_value = [999]
response = self.client.post(self.category_export_url, data)
self.assertEqual(response.status_code, 200)
self.assertIn(
"Select a valid choice. "
f"{self.cat1.id} is not one of the available choices.",
str(response.content),
)

def test_get_export_data_raises_PermissionDenied_when_no_export_permission_assigned(
self,
):
Expand Down
2 changes: 1 addition & 1 deletion tests/core/tests/admin_integration/test_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def test_export(self):
data = {"format": "0", **self.bookresource_export_fields_payload}
date_str = datetime.now().strftime("%Y-%m-%d")
# Should not contain COUNT queries from ModelAdmin.get_results()
with self.assertNumQueries(6):
with self.assertNumQueries(5):
response = self.client.post(self.book_export_url, data)
self.assertEqual(response.status_code, 200)
self.assertTrue(response.has_header("Content-Disposition"))
Expand Down

0 comments on commit 0e0071a

Please sign in to comment.