mirror of
https://github.com/django-import-export/django-import-export.git
synced 2025-12-07 17:53:58 +03:00
Call get_export_fields() (not get_import_fields()) to populate admin UI export form (#2118)
* Fix export forms showing import fields instead of export fields (issue #2094) This commit addresses issue #2094 where export forms in the admin interface were incorrectly displaying import fields instead of export fields. ## Changes Made ### Core Fix - Added `get_user_visible_import_fields()` method to return fields for import interface - Added `get_user_visible_export_fields()` method to return fields for export interface - Updated admin.py to use context-specific methods: - Import contexts now use `get_user_visible_import_fields()` - Export contexts now use `get_user_visible_export_fields()` ### Deprecation - Deprecated `get_user_visible_fields()` in v4.4 for removal in v6.0 - Added clear deprecation warning with migration guidance - Method now issues DeprecationWarning directing users to new methods ### Tests - Added comprehensive tests for import/export field display in admin forms - Added test coverage for deprecation warning - All existing tests continue to pass ## Breaking Changes None - this is fully backward compatible. The deprecated method continues to work with a warning until v6.0. ## Migration Path Replace calls to: - `get_user_visible_fields()` in import contexts with `get_user_visible_import_fields()` - `get_user_visible_fields()` in export contexts with `get_user_visible_export_fields()` 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * tidied comments * Fix SelectableFieldsExportForm to respect overridden get_export_fields() SelectableFieldsExportForm was using get_export_order() instead of get_export_fields(), causing it to show all fields rather than only the fields returned by an overridden get_export_fields() method. Changes: - Modified _create_boolean_fields() to detect when get_export_fields() is overridden and use it instead of get_export_order() - Maintains backward compatibility by falling back to get_export_order() when get_export_fields() is not overridden - Added test to verify the fix works correctly 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * updated documentation --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
parent
15ecbb9dce
commit
e7988d7093
113
CLAUDE.md
Normal file
113
CLAUDE.md
Normal file
|
|
@ -0,0 +1,113 @@
|
|||
# CLAUDE.md
|
||||
|
||||
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
|
||||
|
||||
## Project Overview
|
||||
|
||||
**django-import-export** is a Django application and library for importing and exporting data with admin integration. It supports multiple file formats (CSV, XLSX, JSON, etc.) via the `tablib` library and provides both programmatic and Django Admin UI interfaces for data import/export operations.
|
||||
|
||||
## Development Commands
|
||||
|
||||
### Testing
|
||||
- **Run all tests**: `make test` or `PYTHONPATH=".:tests:${PYTHONPATH}" python -W error -m django test core --settings=settings`
|
||||
- **Run tests in parallel**: `make testp`
|
||||
- **Run tests with coverage**: `make coverage` or `coverage run tests/manage.py test core`
|
||||
- **Run comprehensive tests (all databases)**: `./runtests.sh` (requires Docker for MySQL/PostgreSQL)
|
||||
- **Run tests via tox**: `tox` (tests against multiple Python/Django combinations)
|
||||
|
||||
### Code Quality
|
||||
- **Linting and formatting**: Pre-commit hooks handle this automatically via:
|
||||
- `black` (code formatting)
|
||||
- `isort` (import sorting)
|
||||
- `flake8` (linting)
|
||||
- `django-upgrade` (Django version compatibility)
|
||||
- `pyupgrade` (Python syntax modernization)
|
||||
- **Run pre-commit manually**: `pre-commit run --all-files`
|
||||
- **Max line length**: 88 characters (Black standard)
|
||||
|
||||
### Documentation
|
||||
- **Build HTML docs**: `make build-html-doc`
|
||||
- First install docs requirements: `make install-docs-requirements` or `pip install -e .[docs]`
|
||||
- HTML output is generated in `docs/_build/html/`
|
||||
- Includes Sphinx documentation with RTD theme
|
||||
- **Generate locale files**: `make messages`
|
||||
- **Update documentation for releases**:
|
||||
- `docs/changelog.rst`: Add entries for new releases with format "X.Y.Z (unreleased)" or "X.Y.Z (YYYY-MM-DD)"
|
||||
- `docs/release_notes.rst`: Document breaking changes, deprecations, and migration guides
|
||||
- Follow existing format with bullet points, PR references like `(#XXXX <https://github.com/django-import-export/django-import-export/pull/XXXX>`_)
|
||||
- Include deprecation warnings with version numbers for when features will be removed
|
||||
|
||||
### Installation
|
||||
- **Base requirements**: `make install-base-requirements` or `pip install .`
|
||||
- **Test requirements**: `make install-test-requirements` or `pip install .[tests]`
|
||||
- **Docs requirements**: `make install-docs-requirements` or `pip install .[docs]`
|
||||
|
||||
## Architecture Overview
|
||||
|
||||
### Core Components
|
||||
|
||||
**Resources (`resources.py`)**
|
||||
- Central abstraction for data import/export operations
|
||||
- `ModelResource`: Maps Django models to import/export operations
|
||||
- Handles field mapping, data transformation, validation, and CRUD operations
|
||||
- Supports customizable field definitions, widgets, and instance loading
|
||||
|
||||
**Fields (`fields.py`)**
|
||||
- Maps between model attributes and export/import data representations
|
||||
- Configurable with widgets for data transformation
|
||||
- Supports readonly fields, default values, and custom attribute access
|
||||
|
||||
**Admin Integration (`admin.py`)**
|
||||
- Mixins: `ImportExportMixinBase`, `BaseImportMixin`, `BaseExportMixin`
|
||||
- Provides Django Admin UI for import/export operations
|
||||
- Handles file uploads, preview functionality, and batch operations
|
||||
- Integrates with Django's permission system
|
||||
|
||||
**Widgets (`widgets.py`)**
|
||||
- Handle data type conversion and formatting
|
||||
- Examples: `DateWidget`, `ForeignKeyWidget`, `ManyToManyWidget`
|
||||
- Extensible for custom data transformation needs
|
||||
|
||||
**Formats (`formats/`)**
|
||||
- Abstraction layer over `tablib` for different file formats
|
||||
- Supports CSV, XLSX, JSON, YAML, and other formats
|
||||
- Binary vs text format handling
|
||||
|
||||
### Key Patterns
|
||||
|
||||
**Declarative Configuration**
|
||||
- Resources use Django-style declarative syntax similar to ModelAdmin
|
||||
- Meta classes define model mappings, field selections, and options
|
||||
- Field definitions support widgets, column names, and transformation logic
|
||||
|
||||
**Transaction Handling**
|
||||
- Configurable transaction behavior via `IMPORT_EXPORT_USE_TRANSACTIONS`
|
||||
- Supports atomic operations for data integrity
|
||||
- Rollback capabilities on import errors
|
||||
|
||||
**Instance Loading**
|
||||
- Pluggable instance loaders for different update strategies
|
||||
- Default loaders handle create/update logic based on primary keys
|
||||
- Custom loaders support business-specific lookup patterns
|
||||
|
||||
## Testing Structure
|
||||
|
||||
**Test Organization**
|
||||
- Main tests in `tests/core/tests/`
|
||||
- Test models in `tests/core/models.py` (Author, Book, Category, etc.)
|
||||
- Settings in `tests/settings.py` with multi-database support
|
||||
- Docker setup for MySQL/PostgreSQL testing via `tests/docker-compose.yml`
|
||||
|
||||
**Test Database Configuration**
|
||||
- SQLite (default): Local file-based testing
|
||||
- MySQL: Via Docker with `IMPORT_EXPORT_TEST_TYPE=mysql-innodb`
|
||||
- PostgreSQL: Via Docker with `IMPORT_EXPORT_TEST_TYPE=postgres`
|
||||
- Environment variables for database credentials in `runtests.sh`
|
||||
|
||||
## File Structure
|
||||
- `import_export/`: Main package code
|
||||
- `tests/`: Test suite with core app for testing
|
||||
- `docs/`: Sphinx documentation
|
||||
- `tox.ini`: Multi-environment testing configuration
|
||||
- `pyproject.toml`: Project metadata and dependencies
|
||||
- `Makefile`: Development command shortcuts
|
||||
|
|
@ -3,7 +3,12 @@ Changelog
|
|||
|
||||
.. warning::
|
||||
|
||||
If upgrading from v3, v4 introduces breaking changes. Please refer to :doc:`release notes<release_notes>`.
|
||||
v5 introduces breaking changes and deprecations. Please refer to :doc:`release notes<release_notes>`.
|
||||
|
||||
5.0.0 (unreleased)
|
||||
------------------
|
||||
|
||||
- Fixed issue where export forms were incorrectly showing import fields instead of export fields (`2118 <https://github.com/django-import-export/django-import-export/pull/2118>`_)
|
||||
|
||||
4.3.10 (2025-09-26)
|
||||
-------------------
|
||||
|
|
|
|||
|
|
@ -2,6 +2,23 @@
|
|||
Release Notes
|
||||
=============
|
||||
|
||||
v5.0
|
||||
----
|
||||
|
||||
* Fixed issue where export forms were incorrectly showing import fields instead of export fields.
|
||||
This was resolved by introducing context-specific methods for field retrieval.
|
||||
See :ref:`deprecations <deprecations_v5>` and `PR 2118 <https://github.com/django-import-export/django-import-export/pull/2118>`_.
|
||||
|
||||
.. _deprecations_v5:
|
||||
|
||||
Deprecations
|
||||
^^^^^^^^^^^^
|
||||
|
||||
* The :meth:`~import_export.resources.Resource.get_user_visible_fields` method is now deprecated and will be removed in version 6.0.
|
||||
Use :meth:`~import_export.resources.Resource.get_user_visible_import_fields` for import contexts and
|
||||
:meth:`~import_export.resources.Resource.get_user_visible_export_fields` for export contexts instead.
|
||||
This change ensures that import and export operations show their respective field sets correctly in admin forms.
|
||||
|
||||
v4.2
|
||||
----
|
||||
|
||||
|
|
|
|||
|
|
@ -545,7 +545,7 @@ class ImportMixin(BaseImportMixin, ImportExportMixinBase):
|
|||
context["fields_list"] = [
|
||||
(
|
||||
resource.get_display_name(),
|
||||
[f.column_name for f in resource.get_user_visible_fields()],
|
||||
[f.column_name for f in resource.get_user_visible_import_fields()],
|
||||
)
|
||||
for resource in resources
|
||||
]
|
||||
|
|
@ -820,7 +820,7 @@ class ExportMixin(BaseExportMixin, ImportExportMixinBase):
|
|||
field.column_name
|
||||
for field in res(
|
||||
**self.get_export_resource_kwargs(request)
|
||||
).get_user_visible_fields()
|
||||
).get_user_visible_export_fields()
|
||||
],
|
||||
)
|
||||
for res in self.get_export_resource_classes(request)
|
||||
|
|
|
|||
|
|
@ -140,8 +140,30 @@ class SelectableFieldsExportForm(ExportForm):
|
|||
return title
|
||||
|
||||
def _create_boolean_fields(self, resource: ModelResource, index: int) -> None:
|
||||
# Initiate resource to get ordered export fields
|
||||
fields = resource().get_export_order()
|
||||
# Initiate resource to get export fields (respects overridden get_export_fields)
|
||||
resource_instance = resource()
|
||||
|
||||
# Check if get_export_fields() has been overridden
|
||||
# (not using default implementation) by comparing the class hierarchy
|
||||
export_fields_method = resource_instance.__class__.get_export_fields
|
||||
from import_export.resources import Resource
|
||||
|
||||
is_overridden = export_fields_method != Resource.get_export_fields
|
||||
|
||||
if is_overridden:
|
||||
# Use get_export_fields() when it's been overridden
|
||||
export_fields = resource_instance.get_export_fields()
|
||||
# Find the field names (keys) that correspond to these Field objects
|
||||
fields = []
|
||||
for export_field in export_fields:
|
||||
for field_name, field_obj in resource_instance.fields.items():
|
||||
if field_obj is export_field:
|
||||
fields.append(field_name)
|
||||
break
|
||||
else:
|
||||
# Fall back to get_export_order() for backward compatibility
|
||||
# when get_export_fields() is not overridden
|
||||
fields = resource_instance.get_export_order()
|
||||
boolean_fields = [] # will be used for ordering the fields
|
||||
is_initial_field = False
|
||||
|
||||
|
|
|
|||
|
|
@ -1054,8 +1054,33 @@ class Resource(metaclass=DeclarativeMetaclass):
|
|||
return [force_str(field.column_name) for field in export_fields if field]
|
||||
|
||||
def get_user_visible_fields(self):
|
||||
"""
|
||||
Get fields visible to users in admin interface.
|
||||
|
||||
.. deprecated:: 5
|
||||
Use ``get_user_visible_import_fields()`` or
|
||||
``get_user_visible_export_fields()`` instead for explicit
|
||||
context-aware field selection. This method will be removed in version 6.0.
|
||||
"""
|
||||
import warnings
|
||||
|
||||
warnings.warn(
|
||||
"get_user_visible_fields() is deprecated and will be removed in "
|
||||
"version 6.0. Use get_user_visible_import_fields() or "
|
||||
"get_user_visible_export_fields() instead.",
|
||||
DeprecationWarning,
|
||||
stacklevel=2,
|
||||
)
|
||||
return self.get_import_fields()
|
||||
|
||||
def get_user_visible_import_fields(self):
|
||||
"""Get fields visible to users in import interface"""
|
||||
return self.get_import_fields()
|
||||
|
||||
def get_user_visible_export_fields(self):
|
||||
"""Get fields visible to users in export interface"""
|
||||
return self.get_export_fields()
|
||||
|
||||
def iter_queryset(self, queryset):
|
||||
if not isinstance(queryset, QuerySet):
|
||||
yield from queryset
|
||||
|
|
|
|||
|
|
@ -958,3 +958,130 @@ class ExportInvalidCharTest(AdminTestMixin, TestCase):
|
|||
content = response.content
|
||||
wb = load_workbook(filename=BytesIO(content))
|
||||
self.assertEqual("invalid<EFBFBD>", wb.active["B2"].value)
|
||||
|
||||
|
||||
class GetExportFieldsTest(AdminTestMixin, TestCase):
|
||||
"""
|
||||
Test case for issue #2094: Export fields should use get_export_fields()
|
||||
instead of get_import_fields() when showing fields in export form
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
from core.models import Book
|
||||
|
||||
from import_export.fields import Field
|
||||
from import_export.resources import ModelResource
|
||||
|
||||
# Create a custom resource with different import and export fields
|
||||
class TestBookResource(ModelResource):
|
||||
# Only these fields should appear in import
|
||||
name = Field(attribute="name", column_name="book_name")
|
||||
price = Field(attribute="price", column_name="book_price")
|
||||
|
||||
# Only these fields should appear in export
|
||||
export_name = Field(attribute="name", column_name="exported_name")
|
||||
export_author_email = Field(
|
||||
attribute="author_email", column_name="exported_author_email"
|
||||
)
|
||||
|
||||
class Meta:
|
||||
model = Book
|
||||
|
||||
def get_import_fields(self):
|
||||
"""Return only import-specific fields"""
|
||||
return [self.fields["name"], self.fields["price"]]
|
||||
|
||||
def get_export_fields(self, selected_fields=None):
|
||||
"""Return only export-specific fields"""
|
||||
return [self.fields["export_name"], self.fields["export_author_email"]]
|
||||
|
||||
self.book_resource = TestBookResource()
|
||||
|
||||
@patch("core.admin.BookAdmin.get_export_resource_classes")
|
||||
def test_export_fields_shown_in_export_form_issue_2094(
|
||||
self, mock_get_export_resource_classes
|
||||
):
|
||||
"""Test that export form shows export fields, not import fields"""
|
||||
# Mock the admin to use our custom resource class (defined in setUp)
|
||||
mock_get_export_resource_classes.return_value = [type(self.book_resource)]
|
||||
|
||||
# GET the export page to see the form
|
||||
response = self._get_url_response(self.book_export_url)
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
# Check the context to see which fields are being displayed
|
||||
fields_list = response.context.get("fields_list", [])
|
||||
self.assertTrue(fields_list, "fields_list should not be empty")
|
||||
|
||||
resource_name, field_names = fields_list[0]
|
||||
|
||||
expected_export_fields = ["exported_name", "exported_author_email"]
|
||||
unexpected_import_fields = ["book_name", "book_price"]
|
||||
|
||||
for expected_field in expected_export_fields:
|
||||
self.assertIn(
|
||||
expected_field,
|
||||
field_names,
|
||||
)
|
||||
|
||||
# Import fields should NOT appear in export form
|
||||
for unexpected_field in unexpected_import_fields:
|
||||
self.assertNotIn(
|
||||
unexpected_field,
|
||||
field_names,
|
||||
f"IMPORT field '{unexpected_field}' should NOT be shown in "
|
||||
f"export form."
|
||||
f"Actual fields shown: {field_names}",
|
||||
)
|
||||
|
||||
@patch("core.admin.BookAdmin.get_export_resource_classes")
|
||||
def test_selectable_fields_export_form_respects_get_export_fields_issue_2094(
|
||||
self, mock_get_export_resource_classes
|
||||
):
|
||||
"""Test SelectableFieldsExportForm only shows fields from get_export_fields()"""
|
||||
from import_export.formats.base_formats import CSV
|
||||
from import_export.forms import SelectableFieldsExportForm
|
||||
|
||||
# Mock the admin to use our custom resource class (defined in setUp)
|
||||
mock_get_export_resource_classes.return_value = [type(self.book_resource)]
|
||||
|
||||
# Create the SelectableFieldsExportForm directly
|
||||
form = SelectableFieldsExportForm(
|
||||
formats=(CSV,), resources=[type(self.book_resource)]
|
||||
)
|
||||
|
||||
# Get the field names that are actually shown in the form
|
||||
form_field_names = []
|
||||
for field_name, field in form.fields.items():
|
||||
if hasattr(field, "is_selectable_field") and field.is_selectable_field:
|
||||
# Extract the actual field name from the form field name
|
||||
# Form field names are like "TestBookResource_export_name"
|
||||
actual_field_name = field_name.split("_", 1)[1]
|
||||
form_field_names.append(actual_field_name)
|
||||
|
||||
# Fields that should be shown (from get_export_fields)
|
||||
expected_export_fields = ["export_name", "export_author_email"]
|
||||
|
||||
# Fields that should NOT be shown (from get_import_fields or other fields)
|
||||
unexpected_fields = ["name", "price"] # These are the import-only fields
|
||||
|
||||
# Assert that export fields are shown
|
||||
for expected_field in expected_export_fields:
|
||||
self.assertIn(
|
||||
expected_field,
|
||||
form_field_names,
|
||||
f"Export field '{expected_field}' should be shown in "
|
||||
"SelectableFieldsExportForm. "
|
||||
f"Actual fields: {form_field_names}",
|
||||
)
|
||||
|
||||
# Assert that import-only fields are NOT shown
|
||||
for unexpected_field in unexpected_fields:
|
||||
self.assertNotIn(
|
||||
unexpected_field,
|
||||
form_field_names,
|
||||
f"Import field '{unexpected_field}' should NOT be shown in "
|
||||
"SelectableFieldsExportForm. "
|
||||
f"Actual fields: {form_field_names}",
|
||||
)
|
||||
|
|
|
|||
|
|
@ -574,3 +574,80 @@ class DeclaredImportOrderTest(AdminTestMixin, TestCase):
|
|||
r"[\\n\s]+"
|
||||
)
|
||||
self.assertRegex(response.content.decode(), target_re)
|
||||
|
||||
|
||||
class GetImportFieldsTest(AdminTestMixin, TestCase):
|
||||
"""
|
||||
Test case for issue #2094: Import fields should use get_import_fields()
|
||||
This tests the expected behavior (which should work correctly)
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
from core.models import Book
|
||||
|
||||
from import_export.fields import Field
|
||||
from import_export.resources import ModelResource
|
||||
|
||||
# Create a custom resource with different import and export fields
|
||||
class TestBookResource(ModelResource):
|
||||
# Only these fields should appear in import
|
||||
name = Field(attribute="name", column_name="book_name")
|
||||
price = Field(attribute="price", column_name="book_price")
|
||||
|
||||
# Only these fields should appear in export
|
||||
export_name = Field(attribute="name", column_name="exported_name")
|
||||
export_author_email = Field(
|
||||
attribute="author_email", column_name="exported_author_email"
|
||||
)
|
||||
|
||||
class Meta:
|
||||
model = Book
|
||||
|
||||
def get_import_fields(self):
|
||||
"""Return only import-specific fields"""
|
||||
return [self.fields["name"], self.fields["price"]]
|
||||
|
||||
def get_export_fields(self, selected_fields=None):
|
||||
"""Return only export-specific fields"""
|
||||
return [self.fields["export_name"], self.fields["export_author_email"]]
|
||||
|
||||
self.book_resource = TestBookResource()
|
||||
|
||||
@patch("core.admin.BookAdmin.get_import_resource_classes")
|
||||
def test_import_fields_shown_in_import_form_issue_2094(
|
||||
self, mock_get_import_resource_classes
|
||||
):
|
||||
"""Test that import form shows import fields (this should work correctly)"""
|
||||
# Mock the admin to use our custom resource class (defined in setUp)
|
||||
mock_get_import_resource_classes.return_value = [type(self.book_resource)]
|
||||
|
||||
# GET the import page to see the form
|
||||
response = self._get_url_response(self.book_import_url)
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
# Check the context to see which fields are being displayed
|
||||
fields_list = response.context.get("fields_list", [])
|
||||
|
||||
# For import, the fields_list should correctly show import fields
|
||||
if fields_list:
|
||||
resource_name, field_names = fields_list[0]
|
||||
|
||||
# Verify import fields are shown correctly
|
||||
expected_import_fields = ["book_name", "book_price"]
|
||||
unexpected_export_fields = ["exported_name", "exported_author_email"]
|
||||
|
||||
for expected_field in expected_import_fields:
|
||||
self.assertIn(
|
||||
expected_field,
|
||||
field_names,
|
||||
f"Import field '{expected_field}' should be in import fields_list",
|
||||
)
|
||||
|
||||
for unexpected_field in unexpected_export_fields:
|
||||
self.assertNotIn(
|
||||
unexpected_field,
|
||||
field_names,
|
||||
f"Export field '{unexpected_field}' should NOT be in "
|
||||
f"import fields_list",
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import warnings
|
||||
from collections import OrderedDict
|
||||
from unittest import mock
|
||||
from unittest.mock import patch
|
||||
|
|
@ -110,3 +111,26 @@ class ResourceTestCase(TestCase):
|
|||
full_clean_mock.assert_called_once_with(
|
||||
exclude=target.keys(), validate_unique=True
|
||||
)
|
||||
|
||||
def test_get_user_visible_fields_deprecation_warning(self):
|
||||
"""Test that get_user_visible_fields() issues a deprecation warning"""
|
||||
with warnings.catch_warnings(record=True) as warning_list:
|
||||
warnings.simplefilter("always") # Ensure warnings are captured
|
||||
|
||||
# Call the deprecated method
|
||||
fields = self.my_resource.get_user_visible_fields()
|
||||
|
||||
# Verify the warning was issued
|
||||
self.assertEqual(len(warning_list), 1)
|
||||
warning = warning_list[0]
|
||||
self.assertTrue(issubclass(warning.category, DeprecationWarning))
|
||||
self.assertIn(
|
||||
"get_user_visible_fields() is deprecated", str(warning.message)
|
||||
)
|
||||
self.assertIn("version 6.0", str(warning.message))
|
||||
self.assertIn("get_user_visible_import_fields", str(warning.message))
|
||||
self.assertIn("get_user_visible_export_fields", str(warning.message))
|
||||
|
||||
# Verify the method still returns the correct fields (import fields)
|
||||
import_fields = self.my_resource.get_import_fields()
|
||||
self.assertEqual(fields, import_fields)
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user