From 40b44a71edaa3abec2c07ec106577047b4c483b3 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Wed, 28 Apr 2021 19:53:48 +0100 Subject: [PATCH 01/77] Use 'create' flag instead of instance.pk (#1274) changed 'is_create' to mandatory arg added 'pk_attr' property reverted self.save_instance() call params added tests for UUID model field fixed test_resources.py imports added is_create flag refactored migration name removed django_extensions from settings documented breaking change in changelog removed unused pk_attr attribute fixed indentation --- docs/changelog.rst | 11 +++- import_export/resources.py | 16 +++-- tests/core/migrations/0010_uuidbook.py | 21 +++++++ tests/core/models.py | 7 +++ tests/core/tests/test_resources.py | 83 +++++++++++++++++++++++--- 5 files changed, 124 insertions(+), 14 deletions(-) create mode 100644 tests/core/migrations/0010_uuidbook.py diff --git a/docs/changelog.rst b/docs/changelog.rst index 2ff4bf63f..ace32b905 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,11 +1,18 @@ Changelog ========= -2.7.2 (unreleased) +3.0.0 (unreleased) ------------------ -- Nothing changed yet. +Breaking changes +################ +This release makes the following changes to the API. You may need to update your implementation to +accommodate these changes. + +- Use 'create' flag instead of instance.pk (#1362) + - ``import_export.resources.save_instance()`` now takes an additional mandatory argument: `is_create`. + If you have over-ridden `save_instance()` in your own code, you will need to add this new argument. 2.7.1 (2021-12-23) ------------------ diff --git a/import_export/resources.py b/import_export/resources.py index e00d3ba0c..6fb574ab8 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -455,18 +455,24 @@ def validate_instance(self, instance, import_validation_errors=None, validate_un if errors: raise ValidationError(errors) - def save_instance(self, instance, using_transactions=True, dry_run=False): + def save_instance(self, instance, is_create, using_transactions=True, dry_run=False): """ Takes care of saving the object to the database. Objects can be created in bulk if ``use_bulk`` is enabled. + + :param instance: The instance of the object to be persisted. + :param is_create: A boolean flag to indicate whether this is a new object + to be created, or an existing object to be updated. + :param using_transactions: A flag to indicate whether db transactions are used. + :param dry_run: A flag to indicate dry-run mode. """ self.before_save_instance(instance, using_transactions, dry_run) if self._meta.use_bulk: - if instance.pk: - self.update_instances.append(instance) - else: + if is_create: self.create_instances.append(instance) + else: + self.update_instances.append(instance) else: if not using_transactions and dry_run: # we don't have transactions and we want to do a dry_run @@ -698,7 +704,7 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals row_result.import_type = RowResult.IMPORT_TYPE_SKIP else: self.validate_instance(instance, import_validation_errors) - self.save_instance(instance, using_transactions, dry_run) + self.save_instance(instance, new, using_transactions, dry_run) self.save_m2m(instance, row, using_transactions, dry_run) row_result.add_instance_info(instance) if not skip_diff: diff --git a/tests/core/migrations/0010_uuidbook.py b/tests/core/migrations/0010_uuidbook.py new file mode 100644 index 000000000..6d5cba9c2 --- /dev/null +++ b/tests/core/migrations/0010_uuidbook.py @@ -0,0 +1,21 @@ +# Generated by Django 2.2.7 on 2021-05-02 07:46 +import uuid + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0009_auto_20211111_0807'), + ] + + operations = [ + migrations.CreateModel( + name='UUIDBook', + fields=[ + ('id', models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)), + ('name', models.CharField(max_length=100, verbose_name='Book name')), + ], + ), + ] diff --git a/tests/core/models.py b/tests/core/models.py index 97f5d5c39..c8d1f1cff 100644 --- a/tests/core/models.py +++ b/tests/core/models.py @@ -1,5 +1,6 @@ import random import string +import uuid from django.core.exceptions import ValidationError from django.db import models @@ -104,3 +105,9 @@ class EBook(Book): """Book proxy model to have a separate admin url access and name""" class Meta: proxy = True + + +class UUIDBook(models.Model): + """A model which uses a UUID pk (issue 1274)""" + id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) + name = models.CharField('Book name', max_length=100) \ No newline at end of file diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 7e33edb0a..782879042 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -30,6 +30,7 @@ Person, Profile, Role, + UUIDBook, WithDefault, WithDynamicDefault, WithFloatField, @@ -652,8 +653,8 @@ def before_save_instance(self, instance, using_transactions, dry_run): self.before_save_instance_dry_run = True else: self.before_save_instance_dry_run = False - def save_instance(self, instance, using_transactions=True, dry_run=False): - super().save_instance(instance, using_transactions, dry_run) + def save_instance(self, instance, new, using_transactions=True, dry_run=False): + super().save_instance(instance, new, using_transactions, dry_run) if dry_run: self.save_instance_dry_run = True else: @@ -679,7 +680,7 @@ def after_save_instance(self, instance, using_transactions, dry_run): @mock.patch("core.models.Book.save") def test_save_instance_noop(self, mock_book): book = Book.objects.first() - self.resource.save_instance(book, using_transactions=False, dry_run=True) + self.resource.save_instance(book, is_create=False, using_transactions=False, dry_run=True) self.assertEqual(0, mock_book.call_count) @mock.patch("core.models.Book.save") @@ -1526,10 +1527,10 @@ class Meta: rows = [('book_name',)] * 10 self.dataset = tablib.Dataset(*rows, headers=['name']) - def init_update_test_data(self): - [Book.objects.create(name='book_name') for _ in range(10)] - self.assertEqual(10, Book.objects.count()) - rows = Book.objects.all().values_list('id', 'name') + def init_update_test_data(self, model=Book): + [model.objects.create(name='book_name') for _ in range(10)] + self.assertEqual(10, model.objects.count()) + rows = model.objects.all().values_list('id', 'name') updated_rows = [(r[0], 'UPDATED') for r in rows] self.dataset = tablib.Dataset(*updated_rows, headers=['id', 'name']) @@ -1557,6 +1558,25 @@ class Meta: mock_bulk_create.assert_called_with(mock.ANY, batch_size=5) self.assertEqual(10, result.total_rows) + @mock.patch('core.models.UUIDBook.objects.bulk_create') + def test_bulk_create_uuid_model(self, mock_bulk_create): + """Test create of a Model which defines uuid not pk (issue #1274)""" + class _UUIDBookResource(resources.ModelResource): + class Meta: + model = UUIDBook + use_bulk = True + batch_size = 5 + fields = ( + 'id', + 'name', + ) + + resource = _UUIDBookResource() + result = resource.import_data(self.dataset) + self.assertEqual(2, mock_bulk_create.call_count) + mock_bulk_create.assert_called_with(mock.ANY, batch_size=5) + self.assertEqual(10, result.total_rows) + @mock.patch('core.models.Book.objects.bulk_create') def test_bulk_create_no_batch_size(self, mock_bulk_create): class _BookResource(resources.ModelResource): @@ -1865,6 +1885,33 @@ class Meta: self.assertEqual(e, raised_exc) +@skipIf(django.VERSION[0] == 2 and django.VERSION[1] < 2, "bulk_update not supported in this version of django") +class BulkUUIDBookUpdateTest(BulkTest): + + def setUp(self): + super().setUp() + self.init_update_test_data(model=UUIDBook) + + @mock.patch('core.models.UUIDBook.objects.bulk_update') + def test_bulk_update_uuid_model(self, mock_bulk_update): + """Test update of a Model which defines uuid not pk (issue #1274)""" + class _UUIDBookResource(resources.ModelResource): + class Meta: + model = UUIDBook + use_bulk = True + batch_size = 5 + fields = ( + 'id', + 'name', + ) + + resource = _UUIDBookResource() + result = resource.import_data(self.dataset) + self.assertEqual(2, mock_bulk_update.call_count) + self.assertEqual(10, result.total_rows) + self.assertEqual(10, result.totals["update"]) + + class BulkDeleteTest(BulkTest): class DeleteBookResource(resources.ModelResource): def for_delete(self, row, instance): @@ -1981,3 +2028,25 @@ class Meta: with self.assertRaises(Exception) as raised_exc: resource.import_data(self.dataset, raise_errors=True) self.assertEqual(e, raised_exc) + + +class BulkUUIDBookDeleteTest(BulkTest): + class DeleteBookResource(resources.ModelResource): + def for_delete(self, row, instance): + return True + + class Meta: + model = UUIDBook + use_bulk = True + batch_size = 5 + + def setUp(self): + super().setUp() + self.resource = self.DeleteBookResource() + self.init_update_test_data(model=UUIDBook) + + def test_bulk_delete_batch_size_of_5(self): + self.assertEqual(10, UUIDBook.objects.count()) + self.resource.import_data(self.dataset) + self.assertEqual(0, UUIDBook.objects.count()) + From cefe1b4f37ce752579c2c7d9a317f6c4aba72cc3 Mon Sep 17 00:00:00 2001 From: Manel Clos Date: Thu, 15 Apr 2021 18:57:59 +0200 Subject: [PATCH 02/77] use future value of ManyToManyField to check if value would change add tests and reference the issue added some notes to tests updated changelog --- docs/changelog.rst | 6 +++ import_export/resources.py | 12 ++++-- tests/core/tests/test_resources.py | 68 ++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index ace32b905..154974fab 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,6 +10,12 @@ Breaking changes This release makes the following changes to the API. You may need to update your implementation to accommodate these changes. +- Check value of ManyToManyField in skip_row() (#1271) + - This fixes an issue where ManyToMany fields are not checked correctly in `skip_row()`. + This means that `skip_row()` now takes `row` as a mandatory arg. + If you have overridden `skip_row()` in your own implementation, you will need to add `row` + as an arg. + - Use 'create' flag instead of instance.pk (#1362) - ``import_export.resources.save_instance()`` now takes an additional mandatory argument: `is_create`. If you have over-ridden `save_instance()` in your own code, you will need to add this new argument. diff --git a/import_export/resources.py b/import_export/resources.py index 6fb574ab8..29618b8ac 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -577,7 +577,7 @@ def for_delete(self, row, instance): """ return False - def skip_row(self, instance, original): + def skip_row(self, instance, original, row): """ Returns ``True`` if ``row`` importing should be skipped. @@ -607,7 +607,13 @@ def skip_row(self, instance, original): try: # For fields that are models.fields.related.ManyRelatedManager # we need to compare the results - if list(field.get_value(instance).all()) != list(field.get_value(original).all()): + if isinstance(field.widget, widgets.ManyToManyWidget): + # compare with the future value to detect changes + instance_value = list(field.clean(row)) + else: + instance_value = list(field.get_value(instance).all()) + + if instance_value != list(field.get_value(original).all()): return False except AttributeError: if field.get_value(instance) != field.get_value(original): @@ -700,7 +706,7 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals # validate_instance(), where they can be combined with model # instance validation errors if necessary import_validation_errors = e.update_error_dict(import_validation_errors) - if self.skip_row(instance, original): + if self.skip_row(instance, original, row): row_result.import_type = RowResult.IMPORT_TYPE_SKIP else: self.validate_instance(instance, import_validation_errors) diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 782879042..0e7748712 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -1401,6 +1401,74 @@ def check_value(self, result, export_headers, expected_value): expected_value) +class ManyToManyWidgetDiffTest(TestCase): + # issue #1270 - ensure ManyToMany fields are correctly checked for + # changes when skip_unchanged=True + fixtures = ["category", "book"] + + def setUp(self): + pass + + def test_many_to_many_widget_create(self): + # the book is associated with 0 categories + # when we import a book with category 1, the book + # should be updated, not skipped + book = Book.objects.first() + book.categories.clear() + dataset_headers = ["id", "name", "categories"] + dataset_row = [book.id, book.name, "1"] + dataset = tablib.Dataset(headers=dataset_headers) + dataset.append(dataset_row) + + book_resource = BookResource() + book_resource._meta.skip_unchanged = True + self.assertEqual(0, book.categories.count()) + + result = book_resource.import_data(dataset, dry_run=False) + + book.refresh_from_db() + self.assertEqual(1, book.categories.count()) + self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_UPDATE) + self.assertEqual(Category.objects.first(), book.categories.first()) + + def test_many_to_many_widget_update(self): + # the book is associated with 1 category ('Category 2') + # when we import a book with category 1, the book + # should be updated, not skipped, so that Category 2 is replaced by Category 1 + book = Book.objects.first() + dataset_headers = ["id", "name", "categories"] + dataset_row = [book.id, book.name, "1"] + dataset = tablib.Dataset(headers=dataset_headers) + dataset.append(dataset_row) + + book_resource = BookResource() + book_resource._meta.skip_unchanged = True + self.assertEqual(1, book.categories.count()) + + result = book_resource.import_data(dataset, dry_run=False) + self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_UPDATE) + self.assertEqual(1, book.categories.count()) + self.assertEqual(Category.objects.first(), book.categories.first()) + + def test_many_to_many_widget_no_changes(self): + # the book is associated with 1 category ('Category 2') + # when we import a row with a book with category 1, the book + # should be skipped, because there is no change + book = Book.objects.first() + dataset_headers = ["id", "name", "categories"] + dataset_row = [book.id, book.name, book.categories.all()] + dataset = tablib.Dataset(headers=dataset_headers) + dataset.append(dataset_row) + + book_resource = BookResource() + book_resource._meta.skip_unchanged = True + + self.assertEqual(1, book.categories.count()) + result = book_resource.import_data(dataset, dry_run=False) + self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_SKIP) + self.assertEqual(1, book.categories.count()) + + @mock.patch("import_export.resources.Diff", spec=True) class SkipDiffTest(TestCase): """ From e671b1c7ca0c6fbdce886f5f124e4ab40271f73f Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Fri, 24 Dec 2021 19:01:49 +0000 Subject: [PATCH 03/77] added css dark mode to import.css (#1370) --- docs/changelog.rst | 9 +++++ import_export/static/import_export/import.css | 34 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 9623a0b5c..f6e46ab6e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,6 +1,15 @@ Changelog ========= +3.0.0 (unreleased) +------------------ + +Enhancements +############ + +- Updated import.css to support dark mode (#1370) + + 2.7.1 (2021-12-23) ------------------ diff --git a/import_export/static/import_export/import.css b/import_export/static/import_export/import.css index bb20ba2a1..739311c41 100644 --- a/import_export/static/import_export/import.css +++ b/import_export/static/import_export/import.css @@ -79,3 +79,37 @@ table.import-preview tr.update { font-weight: bold; font-size: 0.85em; } + +@media (prefers-color-scheme: dark) { + table.import-preview tr.skip { + background-color: #2d2d2d; + } + + table.import-preview tr.new { + background-color: #42274d; + } + + table.import-preview tr.delete { + background-color: #064140; + } + + table.import-preview tr.update { + background-color: #020230; + } + + .validation-error-container { + background-color: #003e3e; + } + + /* + these declarations are necessary to forcibly override the + formatting applied by the diff-match-patch python library + */ + table.import-preview td ins { + background-color: #190019 !important; + } + + table.import-preview td del { + background-color: #001919 !important; + } +} \ No newline at end of file From 7889bc294f5dd23d76838c09cf7b9b40251e5e38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Dlouh=C3=BD?= Date: Sat, 12 Dec 2020 09:00:29 +0100 Subject: [PATCH 04/77] add option to have multiple resource classes in ModelAdmin --- docs/changelog.rst | 5 + docs/getting_started.rst | 53 ++++++++- import_export/admin.py | 21 +++- import_export/forms.py | 23 +++- import_export/mixins.py | 90 ++++++++++++-- import_export/resources.py | 6 + .../templates/admin/import_export/import.html | 11 +- tests/core/admin.py | 10 +- tests/core/tests/test_admin_integration.py | 81 ++++++++++++- tests/core/tests/test_forms.py | 29 +++++ tests/core/tests/test_mixins.py | 111 +++++++++++++++++- tests/core/tests/test_resources.py | 14 +++ 12 files changed, 420 insertions(+), 34 deletions(-) create mode 100644 tests/core/tests/test_forms.py diff --git a/docs/changelog.rst b/docs/changelog.rst index c085b9790..01a82c64b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -19,6 +19,11 @@ accommodate these changes. - Use 'create' flag instead of instance.pk (#1362) - ``import_export.resources.save_instance()`` now takes an additional mandatory argument: `is_create`. If you have over-ridden `save_instance()` in your own code, you will need to add this new argument.# + +- Add support for multiple resources in ModelAdmin. (#1223) + - The `*Mixin.resource_class` accepting single resource has been deprecated (will work for few next versions) and + the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added. + - Same applies to all of the `get_resource_class`, `get_import_resource_class` and `get_export_resource_class` methods. Enhancements ############ diff --git a/docs/getting_started.rst b/docs/getting_started.rst index d95c30dc8..889d85500 100644 --- a/docs/getting_started.rst +++ b/docs/getting_started.rst @@ -336,7 +336,7 @@ mixins (:class:`~import_export.admin.ImportMixin`, from import_export.admin import ImportExportModelAdmin class BookAdmin(ImportExportModelAdmin): - resource_class = BookResource + resource_classes = [BookResource] admin.site.register(Book, BookAdmin) @@ -353,6 +353,13 @@ mixins (:class:`~import_export.admin.ImportMixin`, A screenshot of the confirm import view. +.. warning:: + + The `resource_class` parameter was deprecated in `django-import-export` 3.0. + Assign list or tuple with Resource(s) to `resource_classes` parameter now. + + + Exporting via admin action ~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -431,7 +438,7 @@ Customize forms:: Customize ``ModelAdmin``:: class CustomBookAdmin(ImportMixin, admin.ModelAdmin): - resource_class = BookResource + resource_classes = [BookResource] def get_import_form(self): return CustomImportForm @@ -460,7 +467,49 @@ Using the above methods it is possible to customize import form initialization as well as importing customizations. +.. warning:: + + The `resource_class` parameter was deprecated in `django-import-export` 3.0. + Assign list or tuple with Resource(s) to `resource_classes` parameter now. + + .. seealso:: :doc:`/api_admin` available mixins and options. + +Using multiple resources +------------------------ + +It is possible to set multiple resources both to import and export `ModelAdmin` classes. +The `ImportMixin`, `ExportMixin`, `ImportExportMixin` and `ImportExportModelAdmin` classes accepts +subscriptable type (list, tuple, ...) as `resource_classes` parameter. +The subscriptable could also be returned from one of the +`get_resource_classes()`, `get_import_resource_classes()`, `get_export_resource_classes()` classes. + +If there are multiple resources, the resource chooser appears in import/export admin form. +The displayed name of the resource can be changed through the `name` parameter of the `Meta` class. + + +Use multiple resources:: + + from import_export import resources + from core.models import Book + + + class BookResource(resources.ModelResource): + + class Meta: + model = Book + + + class BookNameResource(resources.ModelResource): + + class Meta: + model = Book + fields = ['id', 'name'] + name = "Export/Import only book names" + + + class CustomBookAdmin(ImportMixin, admin.ModelAdmin): + resource_classes = [BookResource, BookNameResource] diff --git a/import_export/admin.py b/import_export/admin.py index 1c552b277..bc4939b72 100644 --- a/import_export/admin.py +++ b/import_export/admin.py @@ -119,7 +119,7 @@ def process_import(self, request, *args, **kwargs): def process_dataset(self, dataset, confirm_form, request, *args, **kwargs): res_kwargs = self.get_import_resource_kwargs(request, form=confirm_form, *args, **kwargs) - resource = self.get_import_resource_class()(**res_kwargs) + resource = self.choose_import_resource_class(confirm_form)(**res_kwargs) imp_kwargs = self.get_import_data_kwargs(request, form=confirm_form, *args, **kwargs) return resource.import_data(dataset, @@ -238,6 +238,7 @@ def import_action(self, request, *args, **kwargs): form_type = self.get_import_form() form_kwargs = self.get_form_kwargs(form_type, *args, **kwargs) form = form_type(import_formats, + self.get_import_resource_classes(), request.POST or None, request.FILES or None, **form_kwargs) @@ -265,7 +266,8 @@ def import_action(self, request, *args, **kwargs): # prepare kwargs for import data, if needed res_kwargs = self.get_import_resource_kwargs(request, form=form, *args, **kwargs) - resource = self.get_import_resource_class()(**res_kwargs) + resource = self.choose_import_resource_class(form)(**res_kwargs) + resources = [resource] # prepare additional kwargs for import_data, if needed imp_kwargs = self.get_import_data_kwargs(request, form=form, *args, **kwargs) @@ -282,20 +284,25 @@ def import_action(self, request, *args, **kwargs): 'import_file_name': tmp_storage.name, 'original_file_name': import_file.name, 'input_format': form.cleaned_data['input_format'], + 'resource': request.POST.get('resource', ''), } confirm_form = self.get_confirm_import_form() initial = self.get_form_kwargs(form=form, **initial) context['confirm_form'] = confirm_form(initial=initial) else: res_kwargs = self.get_import_resource_kwargs(request, form=form, *args, **kwargs) - resource = self.get_import_resource_class()(**res_kwargs) + resource_classes = self.get_import_resource_classes() + resources = [resource_class(**res_kwargs) for resource_class in resource_classes] context.update(self.admin_site.each_context(request)) context['title'] = _("Import") context['form'] = form context['opts'] = self.model._meta - context['fields'] = [f.column_name for f in resource.get_user_visible_fields()] + context['fields_list'] = [ + (resource.get_display_name(), [f.column_name for f in resource.get_user_visible_fields()]) + for resource in resources + ] request.current_app = self.admin_site.name return TemplateResponse(request, [self.import_template_name], @@ -405,14 +412,16 @@ def export_action(self, request, *args, **kwargs): raise PermissionDenied formats = self.get_export_formats() - form = ExportForm(formats, request.POST or None) + form = ExportForm(formats, self.get_export_resource_classes(), request.POST or None) if form.is_valid(): file_format = formats[ int(form.cleaned_data['file_format']) ]() queryset = self.get_export_queryset(request) - export_data = self.get_export_data(file_format, queryset, request=request, encoding=self.to_encoding) + export_data = self.get_export_data( + file_format, queryset, request=request, encoding=self.to_encoding, export_form=form, + ) content_type = file_format.get_content_type() response = HttpResponse(export_data, content_type=content_type) response['Content-Disposition'] = 'attachment; filename="%s"' % ( diff --git a/import_export/forms.py b/import_export/forms.py index 76dbaf751..85f7855ab 100644 --- a/import_export/forms.py +++ b/import_export/forms.py @@ -5,7 +5,25 @@ from django.utils.translation import gettext_lazy as _ -class ImportForm(forms.Form): +class ImportExportFormBase(forms.Form): + resource = forms.ChoiceField( + label=_('Resource'), + choices=(), + required=False, + ) + + def __init__(self, resources=None, *args, **kwargs): + super().__init__(*args, **kwargs) + if resources and len(resources) > 1: + resource_choices = [] + for i, resource in enumerate(resources): + resource_choices.append((i, resource.get_display_name())) + self.fields['resource'].choices = resource_choices + else: + del self.fields['resource'] + + +class ImportForm(ImportExportFormBase): import_file = forms.FileField( label=_('File to import') ) @@ -29,6 +47,7 @@ class ConfirmImportForm(forms.Form): import_file_name = forms.CharField(widget=forms.HiddenInput()) original_file_name = forms.CharField(widget=forms.HiddenInput()) input_format = forms.CharField(widget=forms.HiddenInput()) + resource = forms.CharField(widget=forms.HiddenInput(), required=False) def clean_import_file_name(self): data = self.cleaned_data['import_file_name'] @@ -36,7 +55,7 @@ def clean_import_file_name(self): return data -class ExportForm(forms.Form): +class ExportForm(ImportExportFormBase): file_format = forms.ChoiceField( label=_('Format'), choices=(), diff --git a/import_export/mixins.py b/import_export/mixins.py index 5c2830ee7..c10e2fb24 100644 --- a/import_export/mixins.py +++ b/import_export/mixins.py @@ -1,3 +1,5 @@ +import warnings + from django.http import HttpResponse from django.utils.timezone import now from django.views.generic.edit import FormView @@ -11,22 +13,65 @@ class BaseImportExportMixin: formats = base_formats.DEFAULT_FORMATS resource_class = None - - def get_resource_class(self): - if not self.resource_class: - return modelresource_factory(self.model) - return self.resource_class + resource_classes = [] + + def check_resource_classes(self, resource_classes): + if resource_classes and not hasattr(resource_classes, '__getitem__'): + raise Exception("The resource_classes field type must be subscriptable (list, tuple, ...)") + + def get_resource_classes(self): + """ Return subscriptable type (list, tuple, ...) containing resource classes """ + if self.resource_classes and self.resource_class: + raise Exception("Only one of 'resource_class' and 'resource_classes' can be set") + if hasattr(self, 'get_resource_class'): + warnings.warn( + "The 'get_resource_class()' method has been deprecated. " + "Please implement the new 'get_resource_classes()' method", + DeprecationWarning, + ) + return self.get_resource_class() + if self.resource_class: + warnings.warn( + "The 'resource_class' field has been deprecated. " + "Please implement the new 'resource_classes' field", + DeprecationWarning, + ) + if not self.resource_classes and not self.resource_class: + return [modelresource_factory(self.model)] + if self.resource_classes: + return self.resource_classes + return [self.resource_class] def get_resource_kwargs(self, request, *args, **kwargs): return {} + def get_resource_index(self, form): + resource_index = 0 + if form and 'resource' in form.cleaned_data: + try: + resource_index = int(form.cleaned_data['resource']) + except ValueError: + pass + return resource_index + class BaseImportMixin(BaseImportExportMixin): - def get_import_resource_class(self): + + def get_import_resource_classes(self): """ - Returns ResourceClass to use for import. + Returns ResourceClass subscriptable (list, tuple, ...) to use for import. """ - return self.get_resource_class() + if hasattr(self, 'get_import_resource_class'): + warnings.warn( + "The 'get_import_resource_class()' method has been deprecated. " + "Please implement the new 'get_import_resource_classes()' method", + DeprecationWarning, + ) + return [self.get_import_resource_class()] + + resource_classes = self.get_resource_classes() + self.check_resource_classes(resource_classes) + return resource_classes def get_import_formats(self): """ @@ -37,6 +82,10 @@ def get_import_formats(self): def get_import_resource_kwargs(self, request, *args, **kwargs): return self.get_resource_kwargs(request, *args, **kwargs) + def choose_import_resource_class(self, form): + resource_index = self.get_resource_index(form) + return self.get_import_resource_classes()[resource_index] + class BaseExportMixin(BaseImportExportMixin): model = None @@ -47,18 +96,33 @@ def get_export_formats(self): """ return [f for f in self.formats if f().can_export()] - def get_export_resource_class(self): + def get_export_resource_classes(self): """ - Returns ResourceClass to use for export. + Returns ResourceClass subscriptable (list, tuple, ...) to use for export. """ - return self.get_resource_class() + if hasattr(self, 'get_export_resource_class'): + warnings.warn( + "The 'get_export_resource_class()' method has been deprecated. " + "Please implement the new 'get_export_resource_classes()' method", + DeprecationWarning, + ) + return [self.get_export_resource_class()] + + resource_classes = self.get_resource_classes() + self.check_resource_classes(resource_classes) + return resource_classes + + def choose_export_resource_class(self, form): + resource_index = self.get_resource_index(form) + return self.get_export_resource_classes()[resource_index] def get_export_resource_kwargs(self, request, *args, **kwargs): return self.get_resource_kwargs(request, *args, **kwargs) def get_data_for_export(self, request, queryset, *args, **kwargs): - resource_class = self.get_export_resource_class() - return resource_class(**self.get_export_resource_kwargs(request, *args, **kwargs))\ + export_form = kwargs.pop('export_form', None) + return self.choose_export_resource_class(export_form)\ + (**self.get_export_resource_kwargs(request, *args, **kwargs))\ .export(queryset, *args, **kwargs) def get_export_filename(self, file_format): diff --git a/import_export/resources.py b/import_export/resources.py index 29618b8ac..f3c051373 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -1174,6 +1174,12 @@ def after_import(self, dataset, result, using_transactions, dry_run, **kwargs): finally: cursor.close() + @classmethod + def get_display_name(cls): + if hasattr(cls._meta, 'name'): + return cls._meta.name + return cls.__name__ + def modelresource_factory(model, resource_class=ModelResource): """ diff --git a/import_export/templates/admin/import_export/import.html b/import_export/templates/admin/import_export/import.html index f791c790d..85ebf12f2 100644 --- a/import_export/templates/admin/import_export/import.html +++ b/import_export/templates/admin/import_export/import.html @@ -29,7 +29,16 @@

{% trans "This importer will import the following fields: " %} - {{ fields|join:", " }} + {% if fields_list|length <= 1 %} + {{ fields_list.0.1|join:", " }} + {% else %} +

+ {% for resource, fields in fields_list %} +
{{ resource }}
+
{{ fields|join:", " }}
+ {% endfor %} +
+ {% endif %}

diff --git a/tests/core/admin.py b/tests/core/admin.py index 90f007f20..257afdab7 100644 --- a/tests/core/admin.py +++ b/tests/core/admin.py @@ -20,10 +20,18 @@ def for_delete(self, row, instance): return self.fields['name'].clean(row) == '' +class BookNameResource(ModelResource): + + class Meta: + model = Book + fields = ['id', 'name'] + name = "Export/Import only book names" + + class BookAdmin(ImportExportMixin, admin.ModelAdmin): list_display = ('name', 'author', 'added') list_filter = ['categories', 'author'] - resource_class = BookResource + resource_classes = [BookResource, BookNameResource] class CategoryAdmin(ExportActionModelAdmin): diff --git a/tests/core/tests/test_admin_integration.py b/tests/core/tests/test_admin_integration.py index 43320a156..5998e5f5f 100644 --- a/tests/core/tests/test_admin_integration.py +++ b/tests/core/tests/test_admin_integration.py @@ -89,6 +89,51 @@ def test_import(self): 1, 0, Book._meta.verbose_name_plural) ) + @override_settings(TEMPLATE_STRING_IF_INVALID='INVALID_VARIABLE') + def test_import_second_resource(self): + Book.objects.create(id=1) + + # GET the import form + response = self.client.get('/admin/core/book/import/') + self.assertContains(response, "Export/Import only book names") + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'admin/import_export/import.html') + self.assertContains(response, 'form action=""') + + # POST the import form + input_format = '0' + filename = os.path.join( + os.path.dirname(__file__), + os.path.pardir, + 'exports', + 'books.csv') + with open(filename, "rb") as f: + data = { + 'input_format': input_format, + 'import_file': f, + 'resource': 1, + } + response = self.client.post('/admin/core/book/import/', data) + self.assertEqual(response.status_code, 200) + self.assertIn('result', response.context) + with open("response.html", "wb") as f: + f.write(response.content) + self.assertFalse(response.context['result'].has_errors()) + self.assertIn('confirm_form', response.context) + confirm_form = response.context['confirm_form'] + + data = confirm_form.initial + self.assertEqual(data['original_file_name'], 'books.csv') + response = self.client.post('/admin/core/book/process_import/', data, + follow=True) + self.assertEqual(response.status_code, 200) + self.assertContains(response, + _('Import finished, with {} new and {} updated {}.').format( + 0, 1, Book._meta.verbose_name_plural) + ) + # Check, that we really use second resource - author_email didn't get imported + self.assertEqual(Book.objects.get(id=1).author_email, "") + def test_delete_from_admin(self): # test delete from admin site (see #432) @@ -174,6 +219,31 @@ def test_export(self): response['Content-Disposition'], 'attachment; filename="Book-{}.csv"'.format(date_str) ) + self.assertEqual( + b"id,name,author,author_email,imported,published," + b"published_time,price,added,categories\r\n", + response.content, + ) + + def test_export_second_resource(self): + response = self.client.get('/admin/core/book/export/') + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Export/Import only book names") + + data = { + 'file_format': '0', + 'resource': 1, + } + date_str = datetime.now().strftime('%Y-%m-%d') + response = self.client.post('/admin/core/book/export/', data) + self.assertEqual(response.status_code, 200) + self.assertTrue(response.has_header("Content-Disposition")) + self.assertEqual(response['Content-Type'], 'text/csv') + self.assertEqual( + response['Content-Disposition'], + 'attachment; filename="Book-{}.csv"'.format(date_str) + ) + self.assertEqual(b"id,name\r\n", response.content) def test_returns_xlsx_export(self): response = self.client.get('/admin/core/book/export/') @@ -298,15 +368,15 @@ def import_obj(self, obj, data, dry_run, **kwargs): raise Exception @monkeypatch_method(BookAdmin) - def get_resource_class(self): - return R + def get_resource_classes(self): + return [R] # Verify that when an exception occurs in import_row, when raise_errors is False, # the returned row result has a correct import_type value, # so generating log entries does not fail. @monkeypatch_method(BookAdmin) def process_dataset(self, dataset, confirm_form, request, *args, **kwargs): - resource = self.get_import_resource_class()(**self.get_import_resource_kwargs(request, *args, **kwargs)) + resource = self.get_import_resource_classes()[0](**self.get_import_resource_kwargs(request, *args, **kwargs)) return resource.import_data(dataset, dry_run=False, raise_errors=False, @@ -459,6 +529,7 @@ def write_to_tmp_storage(self, import_file, input_format): class ImportActionDecodeErrorTest(TestCase): mock_model = mock.Mock(spec=Book) mock_model.__name__ = "mockModel" + mock_model._meta = mock.Mock(fields=[], many_to_many=[]) mock_site = mock.MagicMock() mock_request = MagicMock(spec=HttpRequest) mock_request.POST = {'a': 1} @@ -542,6 +613,8 @@ class TestExportEncoding(TestCase): mock_request.POST = {'file_format': 0} class TestMixin(ExportMixin): + model = Book + def __init__(self, test_str=None): self.test_str = test_str @@ -607,4 +680,4 @@ def get_export_filename(self, request, queryset, file_format): with mock.patch("import_export.admin.ExportMixin.get_export_data") as mock_get_export_data: self.export_mixin.export_admin_action(self.mock_request, list()) encoding_kwarg = mock_get_export_data.call_args_list[0][1]["encoding"] - self.assertEqual("utf-8", encoding_kwarg) \ No newline at end of file + self.assertEqual("utf-8", encoding_kwarg) diff --git a/tests/core/tests/test_forms.py b/tests/core/tests/test_forms.py new file mode 100644 index 000000000..76e0895a0 --- /dev/null +++ b/tests/core/tests/test_forms.py @@ -0,0 +1,29 @@ +from django.test import TestCase + +from import_export import forms, resources + + +class MyResource(resources.ModelResource): + class Meta: + name = "My super resource" + + +class FormTest(TestCase): + + def test_formbase_init_blank_resources(self): + resource_list = [] + form = forms.ImportExportFormBase(resource_list) + self.assertTrue('resource' not in form.fields) + + def test_formbase_init_one_resources(self): + resource_list = [resources.ModelResource] + form = forms.ImportExportFormBase(resource_list) + self.assertTrue('resource' not in form.fields) + + def test_formbase_init_two_resources(self): + resource_list = [resources.ModelResource, MyResource] + form = forms.ImportExportFormBase(resource_list) + self.assertEqual( + form.fields['resource'].choices, + [(0, 'ModelResource'), (1, "My super resource")], + ) diff --git a/tests/core/tests/test_mixins.py b/tests/core/tests/test_mixins.py index 0562fe221..14daaadf0 100644 --- a/tests/core/tests/test_mixins.py +++ b/tests/core/tests/test_mixins.py @@ -6,7 +6,7 @@ from django.test.testcases import TestCase from django.urls import reverse -from import_export import formats, forms, mixins +from import_export import formats, forms, mixins, resources class ExportViewMixinTest(TestCase): @@ -113,6 +113,10 @@ def __init__(self): self.assertEqual('CanImportFormat', formats[0].__name__) +class FooResource(resources.Resource): + pass + + class MixinModelAdminTest(TestCase): """ Tests for regression where methods in ModelAdmin with BaseImportMixin / BaseExportMixin @@ -124,7 +128,7 @@ class MixinModelAdminTest(TestCase): class BaseImportModelAdminTest(mixins.BaseImportMixin): call_count = 0 - def get_resource_class(self): + def get_resource_classes(self): self.call_count += 1 def get_resource_kwargs(self, request, *args, **kwargs): @@ -133,7 +137,7 @@ def get_resource_kwargs(self, request, *args, **kwargs): class BaseExportModelAdminTest(mixins.BaseExportMixin): call_count = 0 - def get_resource_class(self): + def get_resource_classes(self): self.call_count += 1 def get_resource_kwargs(self, request, *args, **kwargs): @@ -141,7 +145,7 @@ def get_resource_kwargs(self, request, *args, **kwargs): def test_get_import_resource_class_calls_self_get_resource_class(self): admin = self.BaseImportModelAdminTest() - admin.get_import_resource_class() + admin.get_import_resource_classes() self.assertEqual(1, admin.call_count) def test_get_import_resource_kwargs_calls_self_get_resource_kwargs(self): @@ -151,7 +155,7 @@ def test_get_import_resource_kwargs_calls_self_get_resource_kwargs(self): def test_get_export_resource_class_calls_self_get_resource_class(self): admin = self.BaseExportModelAdminTest() - admin.get_export_resource_class() + admin.get_export_resource_classes() self.assertEqual(1, admin.call_count) def test_get_export_resource_kwargs_calls_self_get_resource_kwargs(self): @@ -159,6 +163,103 @@ def test_get_export_resource_kwargs_calls_self_get_resource_kwargs(self): admin.get_export_resource_kwargs(self.request) self.assertEqual(1, admin.call_count) + class BaseModelResourceClassTest(mixins.BaseImportMixin, mixins.BaseExportMixin): + resource_class = resources.Resource + export_call_count = 0 + import_call_count = 0 + + def get_export_resource_class(self): + self.export_call_count += 1 + + def get_import_resource_class(self): + self.import_call_count += 1 + + def test_deprecated_resource_class_raises_warning(self): + """ Test that the mixin throws error if user didn't migrate to resource_classes """ + admin = self.BaseModelResourceClassTest() + with self.assertWarnsRegex( + DeprecationWarning, + r"^The 'get_export_resource_class\(\)' method has been deprecated. " + r"Please implement the new 'get_export_resource_classes\(\)' method$"): + admin.get_export_resource_classes() + + with self.assertWarnsRegex( + DeprecationWarning, + r"^The 'get_import_resource_class\(\)' method has been deprecated. " + r"Please implement the new 'get_import_resource_classes\(\)' method$"): + admin.get_import_resource_classes() + + with self.assertWarnsRegex( + DeprecationWarning, + r"^The 'resource_class' field has been deprecated. " + r"Please implement the new 'resource_classes' field$"): + self.assertEqual(admin.get_resource_classes(), [resources.Resource]) + + self.assertEqual(1, admin.export_call_count) + self.assertEqual(1, admin.import_call_count) + + class BaseModelGetExportResourceClassTest(mixins.BaseExportMixin): + def get_resource_class(self): + pass + + def test_deprecated_get_resource_class_raises_warning(self): + """ Test that the mixin throws error if user didn't migrate to resource_classes """ + admin = self.BaseModelGetExportResourceClassTest() + with self.assertWarnsRegex( + DeprecationWarning, + r"^The 'get_resource_class\(\)' method has been deprecated. " + r"Please implement the new 'get_resource_classes\(\)' method$"): + admin.get_resource_classes() + + class BaseModelAdminFaultyResourceClassesTest(mixins.BaseExportMixin): + resource_classes = resources.Resource + + def test_faulty_resource_class_raises_exception(self): + """ Test fallback mechanism to old get_export_resource_class() method """ + admin = self.BaseModelAdminFaultyResourceClassesTest() + with self.assertRaisesRegex( + Exception, + r"^The resource_classes field type must be subscriptable"): + admin.get_export_resource_classes() + + class BaseModelAdminBothResourceTest(mixins.BaseExportMixin): + call_count = 0 + + resource_class = resources.Resource + resource_classes = [resources.Resource] + + def test_both_resource_class_raises_exception(self): + """ Test fallback mechanism to old get_export_resource_class() method """ + admin = self.BaseModelAdminBothResourceTest() + with self.assertRaisesRegex( + Exception, + "Only one of 'resource_class' and 'resource_classes' can be set"): + admin.get_export_resource_classes() + + class BaseModelExportChooseTest(mixins.BaseExportMixin): + resource_classes = [resources.Resource, FooResource] + + @mock.patch("import_export.admin.ExportForm") + def test_choose_export_resource_class(self, form): + """ Test choose_export_resource_class() method """ + admin = self.BaseModelExportChooseTest() + self.assertEqual(admin.choose_export_resource_class(form), resources.Resource) + + form.cleaned_data = {'resource': 1} + self.assertEqual(admin.choose_export_resource_class(form), FooResource) + + class BaseModelImportChooseTest(mixins.BaseImportMixin): + resource_classes = [resources.Resource, FooResource] + + @mock.patch("import_export.admin.ImportForm") + def test_choose_import_resource_class(self, form): + """ Test choose_import_resource_class() method """ + admin = self.BaseModelImportChooseTest() + self.assertEqual(admin.choose_import_resource_class(form), resources.Resource) + + form.cleaned_data = {'resource': 1} + self.assertEqual(admin.choose_import_resource_class(form), FooResource) + class BaseExportMixinTest(TestCase): class TestBaseExportMixin(mixins.BaseExportMixin): diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 0e7748712..b390b868e 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -264,6 +264,20 @@ def test_fields_foreign_key(self): self.assertIsInstance(widget, widgets.ForeignKeyWidget) self.assertEqual(widget.model, Author) + def test_get_display_name(self): + display_name = self.resource.get_display_name() + self.assertEqual(display_name, 'BookResource') + + class BookResource(resources.ModelResource): + class Meta: + name = 'Foo Name' + model = Book + import_id_fields = ['name'] + + resource = BookResource() + display_name = resource.get_display_name() + self.assertEqual(display_name, 'Foo Name') + def test_fields_m2m(self): fields = self.resource.fields self.assertIn('categories', fields) From 355e66c43de8ac99f05daf4a739018e0da1c24a5 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 23 Dec 2021 18:36:45 +0000 Subject: [PATCH 05/77] drop support for django 2.2, 3.0, 3.1 (#1366) --- .github/workflows/django-import-export-ci.yml | 14 +------ docs/changelog.rst | 9 ++++- import_export/__init__.py | 2 +- import_export/admin.py | 3 +- import_export/resources.py | 26 +++---------- requirements/base.txt | 2 +- setup.py | 7 +--- tests/core/tests/test_resources.py | 38 ++++--------------- tests/settings.py | 3 +- tox.ini | 6 +-- 10 files changed, 29 insertions(+), 81 deletions(-) diff --git a/.github/workflows/django-import-export-ci.yml b/.github/workflows/django-import-export-ci.yml index 2372ff2d0..00981bf2c 100644 --- a/.github/workflows/django-import-export-ci.yml +++ b/.github/workflows/django-import-export-ci.yml @@ -18,28 +18,18 @@ jobs: max-parallel: 4 matrix: db: [ sqlite, postgres, mysql ] - python-version: [ 3.6, 3.7, 3.8, 3.9, "3.10" ] - django-version: [ 2.2, 3.0, 3.1, 3.2, 4.0, main ] + python-version: [ 3.7, 3.8, 3.9, "3.10" ] + django-version: [ 3.2, 4.0, main ] include: - db: postgres db_port: 5432 - db: mysql db_port: 3306 exclude: - - django-version: main - python-version: 3.6 - django-version: main python-version: 3.7 - - django-version: 4.0 - python-version: 3.6 - django-version: 4.0 python-version: 3.7 - - django-version: 2.2 - python-version: "3.10" - - django-version: 3.0 - python-version: "3.10" - - django-version: 3.1 - python-version: "3.10" - django-version: 3.2 python-version: "3.10" services: diff --git a/docs/changelog.rst b/docs/changelog.rst index 01a82c64b..ed88d1962 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -18,18 +18,23 @@ accommodate these changes. - Use 'create' flag instead of instance.pk (#1362) - ``import_export.resources.save_instance()`` now takes an additional mandatory argument: `is_create`. - If you have over-ridden `save_instance()` in your own code, you will need to add this new argument.# + If you have over-ridden `save_instance()` in your own code, you will need to add this new argument. - Add support for multiple resources in ModelAdmin. (#1223) - The `*Mixin.resource_class` accepting single resource has been deprecated (will work for few next versions) and the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added. - Same applies to all of the `get_resource_class`, `get_import_resource_class` and `get_export_resource_class` methods. - + Enhancements ############ - Updated import.css to support dark mode (#1370) +Development +########### + +- Drop support for python3.6, django 2.2, 3.0, 3.1 (#1366) + 2.7.1 (2021-12-23) ------------------ diff --git a/import_export/__init__.py b/import_export/__init__.py index 4e686b30e..db140818a 100644 --- a/import_export/__init__.py +++ b/import_export/__init__.py @@ -1 +1 @@ -__version__ = '2.7.2.dev0' +__version__ = '3.0.0.dev0' diff --git a/import_export/admin.py b/import_export/admin.py index bc4939b72..d36385838 100644 --- a/import_export/admin.py +++ b/import_export/admin.py @@ -378,8 +378,7 @@ def get_export_queryset(self, request): 'list_editable': self.list_editable, 'model_admin': self, } - if django.VERSION >= (2, 1): - changelist_kwargs['sortable_by'] = self.sortable_by + changelist_kwargs['sortable_by'] = self.sortable_by if django.VERSION >= (4, 0): changelist_kwargs['search_help_text'] = self.search_help_text cl = ChangeList(**changelist_kwargs) diff --git a/import_export/resources.py b/import_export/resources.py index f3c051373..cde097fad 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -4,11 +4,14 @@ from collections import OrderedDict from copy import deepcopy -import django import tablib from diff_match_patch import diff_match_patch from django.conf import settings -from django.core.exceptions import ImproperlyConfigured, ValidationError +from django.core.exceptions import ( + FieldDoesNotExist, + ImproperlyConfigured, + ValidationError, +) from django.core.management.color import no_style from django.core.paginator import Paginator from django.db import connections, router @@ -29,12 +32,6 @@ from .results import Error, Result, RowResult from .utils import atomic_if_using_transaction -if django.VERSION[0] >= 3: - from django.core.exceptions import FieldDoesNotExist -else: - from django.db.models.fields import FieldDoesNotExist - - logger = logging.getLogger(__name__) # Set default logging handler to avoid "No handler found" warnings. logger.addHandler(logging.NullHandler()) @@ -43,9 +40,6 @@ def get_related_model(field): if hasattr(field, 'related_model'): return field.related_model - # Django 1.6, 1.7 - if field.rel: - return field.rel.to class ResourceOptions: @@ -1057,6 +1051,7 @@ class ModelResource(Resource, metaclass=ModelDeclarativeMetaclass): 'BigAutoField': widgets.IntegerWidget, 'NullBooleanField': widgets.BooleanWidget, 'BooleanField': widgets.BooleanWidget, + 'JSONField': widgets.JSONWidget, } @classmethod @@ -1099,22 +1094,13 @@ def widget_from_django_field(cls, f, default=widgets.Widget): else: try: from django.contrib.postgres.fields import ArrayField - try: - from django.db.models import JSONField - except ImportError: - from django.contrib.postgres.fields import JSONField except ImportError: # ImportError: No module named psycopg2.extras class ArrayField: pass - class JSONField: - pass - if isinstance(f, ArrayField): return widgets.SimpleArrayWidget - elif isinstance(f, JSONField): - return widgets.JSONWidget return result diff --git a/requirements/base.txt b/requirements/base.txt index 9a47dd194..2180c1370 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -1,3 +1,3 @@ -Django>=2.2 +Django>=3.2 tablib[html,ods,xls,xlsx,yaml]>=3.0.0 diff-match-patch diff --git a/setup.py b/setup.py index 431533160..8514fec9f 100644 --- a/setup.py +++ b/setup.py @@ -6,8 +6,6 @@ CLASSIFIERS = [ 'Framework :: Django', - 'Framework :: Django :: 2.2', - 'Framework :: Django :: 3.1', 'Framework :: Django :: 3.2', 'Framework :: Django :: 4.0', 'Intended Audience :: Developers', @@ -15,7 +13,6 @@ 'Operating System :: OS Independent', 'Programming Language :: Python', 'Programming Language :: Python :: 3', - 'Programming Language :: Python :: 3.6', 'Programming Language :: Python :: 3.7', 'Programming Language :: Python :: 3.8', 'Programming Language :: Python :: 3.9', @@ -26,7 +23,7 @@ install_requires = [ 'diff-match-patch', - 'Django>=2.2', + 'Django>=3.2', 'tablib[html,ods,xls,xlsx,yaml]>=3.0.0', ] @@ -53,7 +50,7 @@ packages=find_packages(exclude=["tests"]), include_package_data=True, install_requires=install_requires, - python_requires=">=3.6", + python_requires=">=3.7", classifiers=CLASSIFIERS, zip_safe=False, ) diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index b390b868e..bd1cd1121 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -3,13 +3,16 @@ from copy import deepcopy from datetime import date from decimal import Decimal, InvalidOperation -from unittest import mock, skip, skipIf, skipUnless +from unittest import mock, skip, skipUnless -import django import tablib from django.conf import settings from django.contrib.auth.models import User -from django.core.exceptions import ImproperlyConfigured, ValidationError +from django.core.exceptions import ( + FieldDoesNotExist, + ImproperlyConfigured, + ValidationError, +) from django.core.paginator import Paginator from django.db import IntegrityError from django.db.models import Count @@ -36,11 +39,6 @@ WithFloatField, ) -if django.VERSION[0] >= 3: - from django.core.exceptions import FieldDoesNotExist -else: - from django.db.models.fields import FieldDoesNotExist - class MyResource(resources.Resource): name = fields.Field() @@ -424,21 +422,6 @@ def test_get_diff(self): 'other book') self.assertFalse(html[headers.index('author_email')]) - @skip("See: https://github.com/django-import-export/django-import-export/issues/311") - def test_get_diff_with_callable_related_manager(self): - resource = AuthorResource() - author = Author(name="Some author") - author.save() - author2 = Author(name="Some author") - self.book.author = author - self.book.save() - diff = Diff(self.resource, author, False) - diff.compare_with(self.resource, author2) - html = diff.as_html() - headers = resource.get_export_headers() - self.assertEqual(html[headers.index('books')], - 'core.Book.None') - def test_import_data(self): result = self.resource.import_data(self.dataset, raise_errors=True) @@ -1257,16 +1240,11 @@ def test_create_object_after_importing_dataset_with_id(self): if 'postgresql' in settings.DATABASES['default']['ENGINE']: from django.contrib.postgres.fields import ArrayField from django.db import models - try: - from django.db.models import JSONField - except ImportError: - from django.contrib.postgres.fields import JSONField - class BookWithChapters(models.Model): name = models.CharField('Book name', max_length=100) chapters = ArrayField(models.CharField(max_length=100), default=list) - data = JSONField(null=True) + data = models.JSONField(null=True) class BookWithChaptersResource(resources.ModelResource): @@ -1838,7 +1816,6 @@ class Meta: self.assertIsNotNone(resource.get_or_init_instance(ModelInstanceLoader(resource), self.dataset[0])) -@skipIf(django.VERSION[0] == 2 and django.VERSION[1] < 2, "bulk_update not supported in this version of django") class BulkUpdateTest(BulkTest): class _BookResource(resources.ModelResource): class Meta: @@ -1967,7 +1944,6 @@ class Meta: self.assertEqual(e, raised_exc) -@skipIf(django.VERSION[0] == 2 and django.VERSION[1] < 2, "bulk_update not supported in this version of django") class BulkUUIDBookUpdateTest(BulkTest): def setUp(self): diff --git a/tests/settings.py b/tests/settings.py index c63139cdf..1e37324bc 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -50,8 +50,7 @@ }, ] -if django.VERSION >= (3, 2): - DEFAULT_AUTO_FIELD = 'django.db.models.BigAutoField' +DEFAULT_AUTO_FIELD = 'django.db.models.BigAutoField' if os.environ.get('IMPORT_EXPORT_TEST_TYPE') == 'mysql-innodb': IMPORT_EXPORT_USE_TRANSACTIONS = True diff --git a/tox.ini b/tox.ini index bb05fffbe..3f0e4cf2a 100644 --- a/tox.ini +++ b/tox.ini @@ -1,9 +1,7 @@ [tox] envlist = isort - {py36,py37,py38,py39,py310}-django22-tablib{dev,stable} - {py36,py37,py38,py39,py310}-django31-tablib{dev,stable} - {py36,py37,py38,py39,py310}-django32-tablib{dev,stable} + {py37,py38,py39,py310}-django32-tablib{dev,stable} {py38,py39,py310}-django40-tablib{dev,stable} {py38,py39,py310}-djangomain-tablib{dev,stable} @@ -12,8 +10,6 @@ commands = python -W error::DeprecationWarning -W error::PendingDeprecationWarni deps = tablibdev: -egit+https://github.com/jazzband/tablib.git#egg=tablib tablibstable: tablib - django22: Django>=2.2,<3.0 - django31: Django>=3.1,<3.2 django32: Django>=3.2,<4.0 django40: Django>=4.0,<4.1 djangomain: https://github.com/django/django/archive/main.tar.gz From 0bbdba86e7059e40074f45d818233a6341acc400 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Sun, 26 Dec 2021 12:41:08 +0000 Subject: [PATCH 06/77] improve test coverage (#1372) --- .github/workflows/django-import-export-ci.yml | 114 ++++------- docs/changelog.rst | 3 + docs/conf.py | 2 +- docs/index.rst | 1 + docs/testing.rst | 49 +++++ import_export/exceptions.py | 5 + import_export/formats/base_formats.py | 15 +- import_export/resources.py | 2 +- requirements/test.txt | 4 +- runtests.sh | 31 ++- setup.cfg | 4 + tests/bulk/README.md | 192 ------------------ tests/bulk/docker-compose.yml | 23 --- tests/core/tests/test_base_formats.py | 10 + tests/core/tests/test_exceptions.py | 13 ++ tests/core/tests/test_import_export_tags.py | 10 + tests/core/tests/test_resources.py | 72 ++++++- tests/core/tests/test_widgets.py | 17 +- tests/docker-compose.yml | 39 ++++ tests/{bulk => }/docker/db/init-user-db.sh | 0 tests/scripts/bulk_import.py | 2 +- tests/settings.py | 4 +- tox.ini | 26 ++- 23 files changed, 324 insertions(+), 314 deletions(-) create mode 100644 docs/testing.rst delete mode 100644 tests/bulk/README.md delete mode 100644 tests/bulk/docker-compose.yml create mode 100644 tests/core/tests/test_exceptions.py create mode 100644 tests/core/tests/test_import_export_tags.py create mode 100644 tests/docker-compose.yml rename tests/{bulk => }/docker/db/init-user-db.sh (100%) diff --git a/.github/workflows/django-import-export-ci.yml b/.github/workflows/django-import-export-ci.yml index 00981bf2c..ddc5cb0bf 100644 --- a/.github/workflows/django-import-export-ci.yml +++ b/.github/workflows/django-import-export-ci.yml @@ -7,58 +7,32 @@ on: pull_request: branches: - main + # this is a temporary addition - can be removed after 3.0 release + - release-3-x jobs: test: runs-on: ubuntu-latest env: - USERNAME: testuser - PASSWD: ${{ secrets.TEST_PASSWD }} + DB_NAME: import_export + IMPORT_EXPORT_POSTGRESQL_USER: postgres + IMPORT_EXPORT_POSTGRESQL_PASSWORD: somepass + IMPORT_EXPORT_MYSQL_USER: root + IMPORT_EXPORT_MYSQL_PASSWORD: root strategy: - max-parallel: 4 matrix: - db: [ sqlite, postgres, mysql ] - python-version: [ 3.7, 3.8, 3.9, "3.10" ] - django-version: [ 3.2, 4.0, main ] - include: - - db: postgres - db_port: 5432 - - db: mysql - db_port: 3306 - exclude: - - django-version: main - python-version: 3.7 - - django-version: 4.0 - python-version: 3.7 - - django-version: 3.2 - python-version: "3.10" + python-version: + - '3.7' + - '3.8' + - '3.9' + - '3.10' services: - mysql: - image: mysql:8.0 - env: - IMPORT_EXPORT_TEST_TYPE: mysql-innodb - IMPORT_EXPORT_MYSQL_USER: ${{ env.TESTUSER }} - IMPORT_EXPORT_MYSQL_PASSWORD: ${{ env.PASSWD }} - MYSQL_USER: ${{ env.TESTUSER }} - MYSQL_PASSWORD: ${{ env.IMPORT_EXPORT_MYSQL_PASSWORD }} - MYSQL_ROOT_PASSWORD: root - MYSQL_DATABASE: import_export - ports: - - 3306:3306 - options: >- - --health-cmd="mysqladmin ping" - --health-interval=10s - --health-timeout=5s - --health-retries=3 postgres: image: postgres env: - IMPORT_EXPORT_TEST_TYPE: postgres - IMPORT_EXPORT_POSTGRESQL_USER: postgres - IMPORT_EXPORT_POSTGRESQL_PASSWORD: ${{ env.PASSWD }} - POSTGRES_USER: postgres - POSTGRES_PASSWORD: postgres - POSTGRES_DB: import_export + POSTGRES_USER: ${{ env.IMPORT_EXPORT_POSTGRESQL_USER }} + POSTGRES_PASSWORD: ${{ env.IMPORT_EXPORT_POSTGRESQL_PASSWORD }} + POSTGRES_DB: ${{ env.DB_NAME }} ports: - 5432:5432 options: >- @@ -66,53 +40,47 @@ jobs: --health-interval 10s --health-timeout 5s --health-retries 5 - steps: + - name: Set up MySQL + run: > + sudo /etc/init.d/mysql start + + mysql -e 'CREATE DATABASE ${{ env.DB_NAME }};' + -u${{ env.IMPORT_EXPORT_MYSQL_USER }} + -p${{ env.IMPORT_EXPORT_MYSQL_PASSWORD }} - name: Check out repository code uses: actions/checkout@v2 - name: Set up Python ${{ matrix.python-version }} uses: actions/setup-python@v2 with: python-version: ${{ matrix.python-version }} - - name: Run isort checks - uses: jamescurtin/isort-action@master + - uses: actions/cache@v2 with: - sortPaths: "import_export tests" - configuration: "--check-only" + path: ~/.cache/pip + key: ${{ runner.os }}-pip-${{ hashFiles('requirements/*.txt') }} + restore-keys: | + ${{ runner.os }}-pip- - name: Install Dependencies run: | - python -m pip install --upgrade pip - pip install -r requirements/base.txt - pip install -r requirements/test.txt - - if: matrix.django-version != 'main' - name: Upgrade Django version (release) - run: | - python -m pip install "Django~=${{ matrix.django-version }}.0" - - if: matrix.django-version == 'main' - name: Upgrade Django version (main) - run: | - python -m pip install "https://github.com/django/django/archive/main.tar.gz" - - name: List versions - run: | - echo "Python ${{ matrix.python-version }} -> Django ${{ matrix.django-version }}" - python --version - echo "Django `django-admin --version`" - - name: Run Tests + python -m pip install --upgrade pip + pip install tox tox-py coverage coveralls + - name: Run tox targets for ${{ matrix.python-version }} (sqlite) + run: tox --py current + - name: Run tox targets for ${{ matrix.python-version }} (postgres) + run: tox --py current + env: + IMPORT_EXPORT_TEST_TYPE: postgres + - name: Run tox targets for ${{ matrix.python-version }} (mysql) + run: tox --py current env: - DB: ${{ matrix.db }} - DB_HOST: 127.0.0.1 - DB_PORT: ${{ matrix.db_port }} - DB_PASSWORD: ${{ env.PASSWD }} - run: >- - PYTHONPATH=".:tests:$PYTHONPATH" python - -W error::DeprecationWarning -W error::PendingDeprecationWarning - -m coverage run --omit='setup.py,./tests/*,./import_export/locale/*' - --source=. tests/manage.py test core --settings= + IMPORT_EXPORT_TEST_TYPE: mysql-innodb + - name: Combine test coverage + run: coverage combine - name: Upload coverage data to coveralls.io run: coveralls --service=github env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - COVERALLS_FLAG_NAME: ${{ matrix.db }}-${{ matrix.django-version }}-${{ matrix.python-version }} + COVERALLS_FLAG_NAME: ${{ matrix.python-version }} COVERALLS_PARALLEL: true coveralls: diff --git a/docs/changelog.rst b/docs/changelog.rst index ed88d1962..6820ff73e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -25,6 +25,8 @@ accommodate these changes. the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added. - Same applies to all of the `get_resource_class`, `get_import_resource_class` and `get_export_resource_class` methods. +- Deprecated `exceptions.py` - this module will be removed in a future release (#1372) + Enhancements ############ @@ -34,6 +36,7 @@ Development ########### - Drop support for python3.6, django 2.2, 3.0, 3.1 (#1366) +- Increased test coverage, refactored CI build to use tox (#1372) 2.7.1 (2021-12-23) ------------------ diff --git a/docs/conf.py b/docs/conf.py index f753f30bf..1ffa30f85 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -36,7 +36,7 @@ # General information about the project. project = 'django-import-export' -copyright = '2012–2020, Bojan Mihelac' +copyright = '2012–2022, Bojan Mihelac' # The version info for the project you're documenting, acts as replacement for # |version| and |release|, also used in various other places throughout the diff --git a/docs/index.rst b/docs/index.rst index daf152284..ab96f6581 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -32,6 +32,7 @@ exporting data with included admin integration. import_workflow bulk_import celery + testing changelog .. toctree:: diff --git a/docs/testing.rst b/docs/testing.rst new file mode 100644 index 000000000..eb7070cde --- /dev/null +++ b/docs/testing.rst @@ -0,0 +1,49 @@ +Testing +======= + +All tests can be run using `tox `_ simply by running the `tox` command. By default, tests are run against a local sqlite2 instance. + +MySql / Postgres tests +###################### + +By using Docker, you can also run tests against either a MySQL db and/or Postgres. + +The ``IMPORT_EXPORT_TEST_TYPE`` must be set according to the type of tests you wish to run. Set to 'postgres' for postgres tests, and 'mysql-innodb' for mysql tests. If this environment variable is blank (or is any other value) then the default sqlite2 db will be used. + +This process is scripted in ``runtests.sh``. Assuming that you have docker installed on your system, running ``runtests.sh`` will run tox against sqlite2, mysql and postgres. You can edit this script to customise testing as you wish. + +Note that this is the process which is undertaken by CI builds. + +Coverage +######## + +Coverage data is written in parallel mode by default (defined in ``setup.cfg``). After a tox run, you can view coverage data as follows: + +.. code-block:: bash + + # combine all coverage data generated by tox into one file + coverage combine + + # produce an HTML coverage report + coverage html + +Check the output of the above commands to locate the coverage HTML file. + +Bulk testing +############ + +There is a helper script available to generate and profile bulk loads. See ``scripts/bulk_import.py``. + +You can use this script by configuring environment variables as defined above, and then installing and running the test application. In order to run the helper script, you will need to install ``requirements/test.txt``, and then add `django-extensions` to `settings.py` (`INSTALLED_APPS`). + +You can then run the script as follows: + +.. code-block:: bash + + # run creates, updates, and deletes + ./manage.py runscript bulk_import + + # pass 'create', 'update' or 'delete' to run the single test + ./manage.py runscript bulk_import --script-args create + + diff --git a/import_export/exceptions.py b/import_export/exceptions.py index 94a992e0c..33de2fc6e 100644 --- a/import_export/exceptions.py +++ b/import_export/exceptions.py @@ -1,3 +1,8 @@ +import warnings + +warnings.warn("the exceptions module is deprecated and will be removed in a future release", DeprecationWarning) + + class ImportExportError(Exception): """A generic exception for all others to extend.""" pass diff --git a/import_export/formats/base_formats.py b/import_export/formats/base_formats.py index d6bc2461b..accd0e161 100644 --- a/import_export/formats/base_formats.py +++ b/import_export/formats/base_formats.py @@ -1,6 +1,5 @@ -from importlib import import_module - import tablib +from tablib.formats import registry class Format: @@ -61,14 +60,10 @@ def get_format(self): """ Import and returns tablib module. """ - try: - # Available since tablib 1.0 - from tablib.formats import registry - except ImportError: - return import_module(self.TABLIB_MODULE) - else: - key = self.TABLIB_MODULE.split('.')[-1].replace('_', '') - return registry.get_format(key) + if not self.TABLIB_MODULE: + raise AttributeError("TABLIB_MODULE must be defined") + key = self.TABLIB_MODULE.split('.')[-1].replace('_', '') + return registry.get_format(key) @classmethod def is_available(cls): diff --git a/import_export/resources.py b/import_export/resources.py index cde097fad..66a7e4c0a 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -1078,7 +1078,7 @@ def widget_from_django_field(cls, f, default=widgets.Widget): Returns the widget that would likely be associated with each Django type. - Includes mapping of Postgres Array and JSON fields. In the case that + Includes mapping of Postgres Array field. In the case that psycopg2 is not installed, we consume the error and process the field regardless. """ diff --git a/requirements/test.txt b/requirements/test.txt index ef70b981b..94a0fc8f7 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -1,8 +1,8 @@ isort psycopg2-binary mysqlclient -coveralls chardet pytz memory-profiler -django-extensions \ No newline at end of file +django-extensions +coverage \ No newline at end of file diff --git a/runtests.sh b/runtests.sh index 3c627ca24..889da9e8b 100755 --- a/runtests.sh +++ b/runtests.sh @@ -1 +1,30 @@ -PYTHONPATH=".:tests:$PYTHONPATH" django-admin test core --settings=settings +#!/usr/bin/env sh + +# run tests against all supported databases +# postgres / mysql run via docker +# sqlite (default) runs against local database file (database.db) + +echo "starting local database instances" +docker-compose -f tests/docker-compose.yml up -d + +export DJANGO_SETTINGS_MODULE=settings + +export IMPORT_EXPORT_POSTGRESQL_USER=pguser +export IMPORT_EXPORT_POSTGRESQL_PASSWORD=pguserpass + +export IMPORT_EXPORT_MYSQL_USER=mysqluser +export IMPORT_EXPORT_MYSQL_PASSWORD=mysqluserpass + +echo "running tests (sqlite)" +tox + +echo "running tests (mysql)" +export IMPORT_EXPORT_TEST_TYPE=mysql-innodb +tox + +echo "running tests (postgres)" +export IMPORT_EXPORT_TEST_TYPE=postgres +tox + +echo "removing local database instances" +docker-compose -f tests/docker-compose.yml down -v \ No newline at end of file diff --git a/setup.cfg b/setup.cfg index 6b0cf2400..61dc5ef1d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -7,3 +7,7 @@ python-file-with-version = import_export/__init__.py [isort] profile = black + +[coverage:run] +parallel = true +source = import_export \ No newline at end of file diff --git a/tests/bulk/README.md b/tests/bulk/README.md deleted file mode 100644 index e5bc1505c..000000000 --- a/tests/bulk/README.md +++ /dev/null @@ -1,192 +0,0 @@ -## Bulk import testing - -This scripts outlines the steps used to profile bulk loading. -The `bulk_import.py` script is used to profile run duration and memory during bulk load testing. - -### Pre-requisites - -- [Docker](https://docker.com) -- [virtualenvwrapper](https://virtualenvwrapper.readthedocs.io/en/latest/command_ref.html) - -### Test environment - -The following tests were run on the following platform: - -- Thinkpad T470 i5 processor (Ubuntu 18.04) -- python 3.8.1 -- Postgres 10 (docker container) - -### Install dependencies - -```bash -# create venv and install django-import-export dependencies -cd -mkvirtualenv -p python3 djangoimportexport -pip install -r requirements/base.txt -r requirements/test.txt -``` - -### Create Postgres DB - -```bash -export IMPORT_EXPORT_TEST_TYPE=postgres -export IMPORT_EXPORT_POSTGRESQL_USER=pguser -export IMPORT_EXPORT_POSTGRESQL_PASSWORD=pguserpass -export DJANGO_SETTINGS_MODULE=settings - -cd /tests - -# start a local postgres instance -docker-compose -f bulk/docker-compose.yml up -d - -./manage.py migrate -./manage.py test - -# only required if you want to login to the Admin site -./manage.py createsuperuser --username admin --email=email@example.com -``` - -### Update settings - -In order to use the `runscript` command, add `django_extensions` to `settings.py` (`INSTALLED_APPS`). - -### Running script - -```bash -# run creates, updates, and deletes -./manage.py runscript bulk_import - -# pass 'create', 'update' or 'delete' to run the single test -./manage.py runscript bulk_import --script-args create -``` - -### Results - -- Used 20k book entries -- Memory is reported as the peak memory value whilst running the test script - -#### bulk_create - -##### Default settings - -- default settings -- uses `ModelInstanceLoader` by default - -| Condition | Time (secs) | Memory (MB) | -| ---------------------------------- | ----------- | ------------- | -| `use_bulk=False` | 42.67 | 16.22 | -| `use_bulk=True, batch_size=None` | 33.72 | 50.02 | -| `use_bulk=True, batch_size=1000` | 33.21 | 11.43 | - -##### Performance tweaks - -| use_bulk | batch_size | skip_diff | instance_loader | time (secs) | peak mem (MB) | -| -------- | ---------- | --------- | -------------------- | ----------- | ------------- | -| True | 1000 | True | force_init_instance | 9.60 | 9.4 | -| True | 1000 | False | CachedInstanceLoader | 13.72 | 9.9 | -| True | 1000 | True | CachedInstanceLoader | 16.12 | 9.5 | -| True | 1000 | False | force_init_instance | 19.93 | 10.5 | -| False | n/a | False | force_init_instance | 26.59 | 14.1 | -| True | 1000 | False | ModelInstanceLoader | 28.60 | 9.7 | -| True | 1000 | False | ModelInstanceLoader | 33.19 | 10.6 | -| False | n/a | False | ModelInstanceLoader | 45.32 | 16.3 | - -(`force_init_instance`) means overriding `get_or_init_instance()` - this can be done when you know for certain that you are importing new rows: - -```python -def get_or_init_instance(self, instance_loader, row): - return self._meta.model(), True -``` - -#### bulk_update - -```bash -./manage.py runscript bulk_import --script-args update -``` - -##### Default settings - -- `skip_diff = False` -- `instance_loader_class = ModelInstanceLoader` - -| Condition | Time (secs) | Memory (MB) | -| ---------------------------------- | ----------- | ------------- | -| `use_bulk=False` | 82.28 | 9.33 | -| `use_bulk=True, batch_size=None` | 92.41 | 202.26 | -| `use_bulk=True, batch_size=1000` | 52.63 | 11.25 | - -##### Performance tweaks - -- `skip_diff = True` -- `instance_loader_class = CachedInstanceLoader` - -| Condition | Time (secs) | Memory (MB) | -| ---------------------------------- | ----------- | ------------- | -| `use_bulk=False` | 28.85 | 20.71 | -| `use_bulk=True, batch_size=None` | 65.11 | 201.01 | -| `use_bulk=True, batch_size=1000` | 21.56 | 21.25 | - - -- `skip_diff = False` - -| Condition | Time (secs) | Memory (MB) | -| ---------------------------------------- | ----------- | ------------- | -| `use_bulk=True, batch_size=1000` | 9.26 | 8.51 | -| `skip_html_diff=True, batch_size=1000` | 8.69 | 7.50 | -| `skip_unchanged=True, batch_size=1000` | 5.42 | 7.34 | - - -#### bulk delete - -```bash -./manage.py runscript bulk_import --script-args delete -``` - -##### Default settings - -- `skip_diff = False` -- `instance_loader_class = ModelInstanceLoader` - -| Condition | Time (secs) | Memory (MB) | -| ---------------------------------- | ----------- | ------------- | -| `use_bulk=False` | 95.56 | 31.36 | -| `use_bulk=True, batch_size=None` | 50.20 | 64.66 | -| `use_bulk=True, batch_size=1000` | 43.77 | 33.123 | - -##### Performance tweaks - -- `skip_diff = True` -- `instance_loader_class = CachedInstanceLoader` - -| Condition | Time (secs) | Memory (MB) | -| ---------------------------------- | ----------- | ------------- | -| `use_bulk=False` | 61.66 | 31.94 | -| `use_bulk=True, batch_size=None` | 14.08 | 39.40 | -| `use_bulk=True, batch_size=1000` | 15.37 | 32.70 | - -### Checking DB - -Note that the db is cleared down after each test run. -You need to uncomment the `delete()` calls to be able to view data. - -```bash -./manage.py shell_plus - -Book.objects.all().count() -``` - -### Clear down - -Optional clear down of resources: - -```bash -# remove the test db container -docker-compose -f bulk/docker-compose.yml down -v - -# remove venv -deactivate -rmvirtualenv djangoimportexport -``` - -### References - -- https://hakibenita.com/fast-load-data-python-postgresql diff --git a/tests/bulk/docker-compose.yml b/tests/bulk/docker-compose.yml deleted file mode 100644 index b7bfa8e33..000000000 --- a/tests/bulk/docker-compose.yml +++ /dev/null @@ -1,23 +0,0 @@ -version: '3.3' -services: - db: - container_name: importexport_pgdb - environment: - IMPORT_EXPORT_TEST_TYPE: 'postgres' - DB_HOST: 'db' - DB_PORT: '5432' - DB_NAME: 'import_export' - IMPORT_EXPORT_POSTGRESQL_USER: ${IMPORT_EXPORT_POSTGRESQL_USER} - IMPORT_EXPORT_POSTGRESQL_PASSWORD: ${IMPORT_EXPORT_POSTGRESQL_PASSWORD} - POSTGRES_PASSWORD: ${IMPORT_EXPORT_POSTGRESQL_PASSWORD} - image: postgres:10 - restart: "no" - ports: - - 5432:5432 - volumes: - - ./docker/db/:/docker-entrypoint-initdb.d/ - - local-db-data:/var/lib/postgresql/data - -volumes: - local-db-data: - driver: local diff --git a/tests/core/tests/test_base_formats.py b/tests/core/tests/test_base_formats.py index 976e55e67..953b19b02 100644 --- a/tests/core/tests/test_base_formats.py +++ b/tests/core/tests/test_base_formats.py @@ -52,6 +52,16 @@ def test_can_import_default(self): def test_can_export_default(self): self.assertFalse(self.format.can_export()) + +class TablibFormatTest(TestCase): + def setUp(self): + self.format = base_formats.TablibFormat() + + def test_get_format_for_undefined_TABLIB_MODULE_raises_AttributeError(self): + with self.assertRaises(AttributeError): + self.format.get_format() + + class XLSTest(TestCase): def setUp(self): diff --git a/tests/core/tests/test_exceptions.py b/tests/core/tests/test_exceptions.py new file mode 100644 index 000000000..5eb41fe87 --- /dev/null +++ b/tests/core/tests/test_exceptions.py @@ -0,0 +1,13 @@ +import warnings +from unittest import TestCase + + +class ExceptionTest(TestCase): + # exceptions.py is deprecated but this test ensures + # there is code coverage + + def test_field_error(self): + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + from import_export import exceptions + exceptions.FieldError() \ No newline at end of file diff --git a/tests/core/tests/test_import_export_tags.py b/tests/core/tests/test_import_export_tags.py new file mode 100644 index 000000000..1c5dc5b8d --- /dev/null +++ b/tests/core/tests/test_import_export_tags.py @@ -0,0 +1,10 @@ +from unittest import TestCase + +from import_export.templatetags import import_export_tags + + +class TagsTest(TestCase): + + def test_compare_values(self): + target = 'ab' + self.assertEqual(target, import_export_tags.compare_values("a", "b")) \ No newline at end of file diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index bd1cd1121..52e2304c3 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -1,9 +1,11 @@ import json +import sys from collections import OrderedDict from copy import deepcopy from datetime import date from decimal import Decimal, InvalidOperation -from unittest import mock, skip, skipUnless +from unittest import mock, skipUnless +from unittest.mock import patch import tablib from django.conf import settings @@ -15,7 +17,7 @@ ) from django.core.paginator import Paginator from django.db import IntegrityError -from django.db.models import Count +from django.db.models import CharField, Count from django.db.utils import ConnectionDoesNotExist from django.test import TestCase, TransactionTestCase, skipUnlessDBFeature from django.utils.encoding import force_str @@ -106,6 +108,9 @@ def test_get_export_order(self): self.assertEqual(self.my_resource.get_export_headers(), ['email', 'name', 'extra']) + def test_default_after_import(self): + self.assertIsNone(self.my_resource.after_import(tablib.Dataset(), results.Result(), False, False)) + # Issue 140 Attributes aren't inherited by subclasses def test_inheritance(self): class A(MyResource): @@ -158,6 +163,18 @@ def test_init_instance_raises_NotImplementedError(self): with self.assertRaises(NotImplementedError): self.my_resource.init_instance([]) + @patch("core.models.Book.full_clean") + def test_validate_instance_called_with_import_validation_errors_as_None_creates_empty_dict(self, full_clean_mock): + # validate_instance() import_validation_errors is an optional kwarg + # If not provided, it defaults to an empty dict + # this tests that scenario by ensuring that an empty dict is passed + # to the model instance full_clean() method. + book = Book() + self.my_resource._meta.clean_model_instances = True + self.my_resource.validate_instance(book) + target = dict() + full_clean_mock.assert_called_once_with(exclude=target.keys(), validate_unique=True) + class AuthorResource(resources.ModelResource): @@ -234,6 +251,36 @@ def widget_from_django_field(cls, f, default=widgets.Widget): return result +class ModelResourcePostgresModuleLoadTest(TestCase): + pg_module_name = 'django.contrib.postgres.fields' + + class ImportRaiser: + def find_spec(self, fullname, path, target=None): + if fullname == ModelResourcePostgresModuleLoadTest.pg_module_name: + # we get here if the module is not loaded and not in sys.modules + raise ImportError() + + def setUp(self): + super().setUp() + self.resource = BookResource() + if self.pg_module_name in sys.modules: + self.pg_modules = sys.modules[self.pg_module_name] + del sys.modules[self.pg_module_name] + + def tearDown(self): + super().tearDown() + sys.modules[self.pg_module_name] = self.pg_modules + + def test_widget_from_django_field_cannot_import_postgres(self): + # test that default widget is returned if postgres extensions + # are not present + sys.meta_path.insert(0, self.ImportRaiser()) + + f = fields.Field() + res = self.resource.widget_from_django_field(f) + self.assertEqual(widgets.Widget, res) + + class ModelResourceTest(TestCase): def setUp(self): self.resource = BookResource() @@ -759,6 +806,21 @@ def dehydrate_full_title(self, obj): self.assertEqual(full_title, '%s by %s' % (self.book.name, self.book.author.name)) + def test_invalid_relation_field_name(self): + + class B(resources.ModelResource): + full_title = fields.Field(column_name="Full title") + + class Meta: + model = Book + # author_name is not a valid field or relation, + # so should be ignored + fields = ("author_name", "full_title") + + resource = B() + self.assertEqual(1, len(resource.fields)) + self.assertEqual("full_title", list(resource.fields.keys())[0]) + def test_widget_format_in_fk_field(self): class B(resources.ModelResource): @@ -1237,6 +1299,12 @@ def test_create_object_after_importing_dataset_with_id(self): except IntegrityError: self.fail('IntegrityError was raised.') + def test_widget_from_django_field_for_ArrayField_returns_SimpleArrayWidget(self): + f = ArrayField(CharField) + resource = BookResource() + res = resource.widget_from_django_field(f) + self.assertEqual(widgets.SimpleArrayWidget, res) + if 'postgresql' in settings.DATABASES['default']['ENGINE']: from django.contrib.postgres.fields import ArrayField from django.db import models diff --git a/tests/core/tests/test_widgets.py b/tests/core/tests/test_widgets.py index 01a8b7085..69e54779f 100644 --- a/tests/core/tests/test_widgets.py +++ b/tests/core/tests/test_widgets.py @@ -1,7 +1,8 @@ from datetime import date, datetime, time, timedelta from decimal import Decimal -from unittest import mock +from unittest import mock, skipUnless +import django import pytz from core.models import Author, Category from django.test import TestCase @@ -42,6 +43,20 @@ def test_render(self): self.assertEqual(self.widget.render(None), "") +class FormatDatetimeTest(TestCase): + date = date(10, 8, 2) + target_dt = "02.08.0010" + format = "%d.%m.%Y" + + @skipUnless(django.VERSION[0] < 4, f"skipping django {django.VERSION} version specific test") + def test_format_datetime_lt_django4(self): + self.assertEqual(self.target_dt, widgets.format_datetime(self.date, self.format)) + + @skipUnless(django.VERSION[0] >= 4, f"running django {django.VERSION} version specific test") + def test_format_datetime_gte_django4(self): + self.assertEqual(self.target_dt, widgets.format_datetime(self.date, self.format)) + + class DateWidgetTest(TestCase): def setUp(self): diff --git a/tests/docker-compose.yml b/tests/docker-compose.yml new file mode 100644 index 000000000..b9460478d --- /dev/null +++ b/tests/docker-compose.yml @@ -0,0 +1,39 @@ +version: '3.3' +services: + postgresdb: + container_name: importexport_pgdb + environment: + DB_HOST: db + DB_PORT: 5432 + DB_NAME: import_export + IMPORT_EXPORT_POSTGRESQL_USER: ${IMPORT_EXPORT_POSTGRESQL_USER} + IMPORT_EXPORT_POSTGRESQL_PASSWORD: ${IMPORT_EXPORT_POSTGRESQL_PASSWORD} + POSTGRES_PASSWORD: ${IMPORT_EXPORT_POSTGRESQL_PASSWORD} + image: postgres:10 + restart: "no" + ports: + - "5432:5432" + volumes: + - ./docker/db/:/docker-entrypoint-initdb.d/ + - postgres-db-data:/var/lib/postgresql/data + mysqldb: + container_name: importexport_mysqldb + image: mysql:5.7 + restart: "no" + environment: + MYSQL_DATABASE: import_export + MYSQL_USER: ${IMPORT_EXPORT_MYSQL_USER} + MYSQL_PASSWORD: ${IMPORT_EXPORT_MYSQL_PASSWORD} + MYSQL_ROOT_PASSWORD: ${IMPORT_EXPORT_MYSQL_PASSWORD} + ports: + - "3306:3306" + expose: + - '3306' + volumes: + - mysql-db-data:/var/lib/mysql + +volumes: + postgres-db-data: + driver: local + mysql-db-data: + driver: local diff --git a/tests/bulk/docker/db/init-user-db.sh b/tests/docker/db/init-user-db.sh similarity index 100% rename from tests/bulk/docker/db/init-user-db.sh rename to tests/docker/db/init-user-db.sh diff --git a/tests/scripts/bulk_import.py b/tests/scripts/bulk_import.py index 23eb79c51..359f3fa1e 100644 --- a/tests/scripts/bulk_import.py +++ b/tests/scripts/bulk_import.py @@ -1,7 +1,7 @@ """ Helper module for testing bulk imports. -See tests/bulk/README.md +See testing.rst """ import time from functools import wraps diff --git a/tests/settings.py b/tests/settings.py index 1e37324bc..e8bf5c00f 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -58,8 +58,8 @@ 'default': { 'ENGINE': 'django.db.backends.mysql', 'NAME': 'import_export', - 'USER': os.environ.get('IMPORT_EXPORT_MYSQL_USER', 'root'), - 'PASSWORD': os.environ.get('IMPORT_EXPORT_MYSQL_PASSWORD', 'password'), + 'USER': os.environ.get('IMPORT_EXPORT_MYSQL_USER'), + 'PASSWORD': os.environ.get('IMPORT_EXPORT_MYSQL_PASSWORD'), 'HOST': '127.0.0.1', 'PORT': 3306, 'TEST': { diff --git a/tox.ini b/tox.ini index 3f0e4cf2a..c8ba79f31 100644 --- a/tox.ini +++ b/tox.ini @@ -1,20 +1,36 @@ [tox] envlist = isort - {py37,py38,py39,py310}-django32-tablib{dev,stable} - {py38,py39,py310}-django40-tablib{dev,stable} - {py38,py39,py310}-djangomain-tablib{dev,stable} + {py37,py38,py39,py310}-{django32} + {py38,py39,py310}-{django40,djangomain} + py310-djangomain-tablibdev + +[gh-actions] +python = + 3.7: py37 + 3.8: py38 + 3.9: py39 + 3.10: py310 [testenv] -commands = python -W error::DeprecationWarning -W error::PendingDeprecationWarning {toxinidir}/tests/manage.py test core +setenv = PYTHONPATH = {toxinidir}/tests +commands = python -W error::DeprecationWarning -W error::PendingDeprecationWarning -m coverage run {toxinidir}/tests/manage.py test core deps = tablibdev: -egit+https://github.com/jazzband/tablib.git#egg=tablib - tablibstable: tablib django32: Django>=3.2,<4.0 django40: Django>=4.0,<4.1 djangomain: https://github.com/django/django/archive/main.tar.gz + tablib -rrequirements/test.txt +# if postgres / mysql environment variables exist, we can go ahead and run db specific tests +passenv = + IMPORT_EXPORT_POSTGRESQL_USER + IMPORT_EXPORT_POSTGRESQL_PASSWORD + IMPORT_EXPORT_MYSQL_USER + IMPORT_EXPORT_MYSQL_PASSWORD + IMPORT_EXPORT_TEST_TYPE + [testenv:isort] skip_install = True deps = isort From e7b6318b97c1ac634f4f191ca111db47cb2f83f7 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Wed, 29 Dec 2021 18:35:10 +0000 Subject: [PATCH 07/77] reformatted changelog --- docs/changelog.rst | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 6820ff73e..5acb438c7 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -7,23 +7,19 @@ Changelog Breaking changes ################ -This release makes the following changes to the API. You may need to update your implementation to -accommodate these changes. +This release makes the following changes to the API. You may need to update your implementation to accommodate these changes. - Check value of ManyToManyField in skip_row() (#1271) - - This fixes an issue where ManyToMany fields are not checked correctly in `skip_row()`. - This means that `skip_row()` now takes `row` as a mandatory arg. - If you have overridden `skip_row()` in your own implementation, you will need to add `row` - as an arg. + - This fixes an issue where ManyToMany fields are not checked correctly in `skip_row()`. This means that `skip_row()` now takes `row` as a mandatory arg. If you have overridden `skip_row()` in your own implementation, you will need to add `row` as an arg. - Use 'create' flag instead of instance.pk (#1362) - - ``import_export.resources.save_instance()`` now takes an additional mandatory argument: `is_create`. - If you have over-ridden `save_instance()` in your own code, you will need to add this new argument. + - `import_export.resources.save_instance()` now takes an additional mandatory argument: `is_create`. If you have over-ridden `save_instance()` in your own code, you will need to add this new argument. - Add support for multiple resources in ModelAdmin. (#1223) - - The `*Mixin.resource_class` accepting single resource has been deprecated (will work for few next versions) and - the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added. - - Same applies to all of the `get_resource_class`, `get_import_resource_class` and `get_export_resource_class` methods. + + - The `*Mixin.resource_class` accepting single resource has been deprecated (will work for few next versions) and the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added. + + - Same applies to all of the `get_resource_class`, `get_import_resource_class` and `get_export_resource_class` methods. - Deprecated `exceptions.py` - this module will be removed in a future release (#1372) From 47c2def82903e0efb3476c5295ee3d9d50c1d4b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Dlouh=C3=BD?= Date: Mon, 31 Jan 2022 15:12:53 +0100 Subject: [PATCH 08/77] fix and test for #1386 --- import_export/mixins.py | 2 +- tests/core/tests/test_mixins.py | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/import_export/mixins.py b/import_export/mixins.py index c10e2fb24..ce93231d9 100644 --- a/import_export/mixins.py +++ b/import_export/mixins.py @@ -29,7 +29,7 @@ def get_resource_classes(self): "Please implement the new 'get_resource_classes()' method", DeprecationWarning, ) - return self.get_resource_class() + return [self.get_resource_class()] if self.resource_class: warnings.warn( "The 'resource_class' field has been deprecated. " diff --git a/tests/core/tests/test_mixins.py b/tests/core/tests/test_mixins.py index 14daaadf0..6e4d6985e 100644 --- a/tests/core/tests/test_mixins.py +++ b/tests/core/tests/test_mixins.py @@ -260,6 +260,29 @@ def test_choose_import_resource_class(self, form): form.cleaned_data = {'resource': 1} self.assertEqual(admin.choose_import_resource_class(form), FooResource) + class BaseModelResourceClassOldTest(mixins.BaseImportMixin, mixins.BaseExportMixin): + + def get_resource_class(self): + return FooResource + + def test_get_resource_class_old(self): + """ + Test that if only the old get_resource_class() method is defined, + the get_export_resource_classes() and get_import_resource_classes() + still return list of resources. + """ + admin = self.BaseModelResourceClassOldTest() + with self.assertWarnsRegex( + DeprecationWarning, + r"^The 'get_resource_class\(\)' method has been deprecated. " + r"Please implement the new 'get_resource_classes\(\)' method$"): + self.assertEqual(admin.get_export_resource_classes(), [FooResource]) + with self.assertWarnsRegex( + DeprecationWarning, + r"^The 'get_resource_class\(\)' method has been deprecated. " + r"Please implement the new 'get_resource_classes\(\)' method$"): + self.assertEqual(admin.get_import_resource_classes(), [FooResource]) + class BaseExportMixinTest(TestCase): class TestBaseExportMixin(mixins.BaseExportMixin): From 89ddc72fbf1dab68d3b001845c046257970a0f32 Mon Sep 17 00:00:00 2001 From: naohide anahara <57.x.mas@gmail.com> Date: Tue, 8 Feb 2022 21:02:01 +0900 Subject: [PATCH 09/77] Add raw_values --- import_export/resources.py | 1 + 1 file changed, 1 insertion(+) diff --git a/import_export/resources.py b/import_export/resources.py index e00d3ba0c..e4e159e9e 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -661,6 +661,7 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals """ skip_diff = self._meta.skip_diff row_result = self.get_row_result_class()() + row_result.raw_values = row original = None try: self.before_import_row(row, **kwargs) From 4064a0506985c5801a38434bae7182453d50b809 Mon Sep 17 00:00:00 2001 From: naohide anahara <57.x.mas@gmail.com> Date: Tue, 8 Feb 2022 21:19:35 +0900 Subject: [PATCH 10/77] Add testcase --- tests/core/tests/test_resources.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 7e33edb0a..49ddd4d5c 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -432,6 +432,8 @@ def test_import_data(self): self.assertTrue(result.rows[0].diff) self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_UPDATE) + self.assertEqual(result.rows[0].raw_values.get('name'), 'Some book') + self.assertEqual(result.rows[0].raw_values.get('author_email'), 'test@example.com') instance = Book.objects.get(pk=self.book.pk) self.assertEqual(instance.author_email, 'test@example.com') From df4c4ba99799ac4cd046170c4e11cb0c7b71a1df Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 20 Jan 2022 11:45:33 +0000 Subject: [PATCH 11/77] Fix validation errors ignored when `skip_unchanged` is set (#1378) --- docs/changelog.rst | 3 +++ import_export/resources.py | 16 ++++++++++------ tests/core/tests/test_resources.py | 15 +++++++++++++++ 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 5acb438c7..2511c2686 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -12,6 +12,9 @@ This release makes the following changes to the API. You may need to update you - Check value of ManyToManyField in skip_row() (#1271) - This fixes an issue where ManyToMany fields are not checked correctly in `skip_row()`. This means that `skip_row()` now takes `row` as a mandatory arg. If you have overridden `skip_row()` in your own implementation, you will need to add `row` as an arg. +- Bug fix: validation errors were being ignored when `skip_unchanged` is set (#1378) + - If you have overridden `skip_row()` you can choose whether or not to skip rows if validation errors are present. The default behavior is to not to skip rows if there are validation errors during import. + - Use 'create' flag instead of instance.pk (#1362) - `import_export.resources.save_instance()` now takes an additional mandatory argument: `is_create`. If you have over-ridden `save_instance()` in your own code, you will need to add this new argument. diff --git a/import_export/resources.py b/import_export/resources.py index 66a7e4c0a..1d8a4f93d 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -571,7 +571,7 @@ def for_delete(self, row, instance): """ return False - def skip_row(self, instance, original, row): + def skip_row(self, instance, original, row, import_validation_errors=None): """ Returns ``True`` if ``row`` importing should be skipped. @@ -582,7 +582,11 @@ def skip_row(self, instance, original, row): will be None. When left unspecified, skip_diff and skip_unchanged both default to ``False``, - and rows are never skipped. + and rows are never skipped. + + By default, rows are not skipped if validation errors have been detected + during import. You can change this behavior and choose to ignore validation + errors by overriding this method. Override this method to handle skipping rows meeting certain conditions. @@ -590,12 +594,12 @@ def skip_row(self, instance, original, row): Use ``super`` if you want to preserve default handling while overriding :: class YourResource(ModelResource): - def skip_row(self, instance, original): + def skip_row(self, instance, original, row, import_validation_errors=None): # Add code here - return super(YourResource, self).skip_row(instance, original) + return super().skip_row(instance, original, row, import_validation_errors=import_validation_errors) """ - if not self._meta.skip_unchanged or self._meta.skip_diff: + if not self._meta.skip_unchanged or self._meta.skip_diff or import_validation_errors: return False for field in self.get_import_fields(): try: @@ -700,7 +704,7 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals # validate_instance(), where they can be combined with model # instance validation errors if necessary import_validation_errors = e.update_error_dict(import_validation_errors) - if self.skip_row(instance, original, row): + if self.skip_row(instance, original, row, import_validation_errors): row_result.import_type = RowResult.IMPORT_TYPE_SKIP else: self.validate_instance(instance, import_validation_errors) diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 52e2304c3..98b92d911 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -514,6 +514,21 @@ def test_import_data_raises_field_specific_validation_errors(self): self.assertIs(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_INVALID) self.assertIn('birthday', result.invalid_rows[0].field_specific_errors) + def test_import_data_raises_field_specific_validation_errors_with_skip_unchanged(self): + resource = AuthorResource() + resource._meta.skip_unchanged = True + + author = Author.objects.create(name="Some author") + + dataset = tablib.Dataset(headers=['id', 'birthday']) + dataset.append([author.id, '1882test-01-18']) + + result = resource.import_data(dataset, raise_errors=False) + + self.assertTrue(result.has_validation_errors()) + self.assertIs(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_INVALID) + self.assertIn('birthday', result.invalid_rows[0].field_specific_errors) + def test_collect_failed_rows(self): resource = ProfileResource() headers = ['id', 'user'] From 9fc5f538cf457dcaf6e4bd932fa6cd3222cef028 Mon Sep 17 00:00:00 2001 From: shimakaze-git Date: Tue, 8 Feb 2022 16:57:27 +0900 Subject: [PATCH 12/77] Customize default format selections on export action (#1389) --- AUTHORS | 1 + docs/changelog.rst | 1 + import_export/admin.py | 4 +- tests/core/tests/test_admin_integration.py | 59 ++++++++++++++++++++++ 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index b725be70b..eba018e37 100644 --- a/AUTHORS +++ b/AUTHORS @@ -121,3 +121,4 @@ The following is a list of much appreciated contributors: * striveforbest (Alex Zagoro) * josx (José Luis Di Biase) * Jan Rydzewski +* shimakaze-git diff --git a/docs/changelog.rst b/docs/changelog.rst index 2511c2686..a02ed1927 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -30,6 +30,7 @@ Enhancements ############ - Updated import.css to support dark mode (#1370) +- Default format selections set correctly for export action (#1389) Development ########### diff --git a/import_export/admin.py b/import_export/admin.py index d36385838..50d6b53eb 100644 --- a/import_export/admin.py +++ b/import_export/admin.py @@ -481,10 +481,12 @@ def __init__(self, *args, **kwargs): choices = [] formats = self.get_export_formats() if formats: - choices.append(('', '---')) for i, f in enumerate(formats): choices.append((str(i), f().get_title())) + if len(formats) > 1: + choices.insert(0, ('', '---')) + self.action_form = export_action_form_factory(choices) super().__init__(*args, **kwargs) diff --git a/tests/core/tests/test_admin_integration.py b/tests/core/tests/test_admin_integration.py index 5998e5f5f..586484341 100644 --- a/tests/core/tests/test_admin_integration.py +++ b/tests/core/tests/test_admin_integration.py @@ -30,6 +30,7 @@ ExportMixin, ImportExportActionModelAdmin, ) +from import_export.formats import base_formats from import_export.formats.base_formats import DEFAULT_FORMATS from import_export.tmp_storages import TempFolderStorage @@ -607,6 +608,64 @@ def has_export_permission(self, request): with self.assertRaises(PermissionDenied): m.get_export_data('0', Book.objects.none(), request=request) + def test_export_admin_action_one_formats(self): + mock_model = mock.MagicMock() + mock_site = mock.MagicMock() + + class TestCategoryAdmin(ExportActionModelAdmin): + def __init__(self): + super().__init__(mock_model, mock_site) + + formats = [base_formats.CSV] + + m = TestCategoryAdmin() + action_form = m.action_form + + items = list(action_form.base_fields.items()) + file_format = items[len(items)-1][1] + choices = file_format.choices + + self.assertNotEqual(choices[0][0], '---') + self.assertEqual(choices[0][1], "csv") + + def test_export_admin_action_formats(self): + + mock_model = mock.MagicMock() + mock_site = mock.MagicMock() + + class TestCategoryAdmin(ExportActionModelAdmin): + def __init__(self): + super().__init__(mock_model, mock_site) + + class TestFormatsCategoryAdmin(ExportActionModelAdmin): + def __init__(self): + super().__init__(mock_model, mock_site) + + formats = [base_formats.CSV, base_formats.JSON] + + m = TestCategoryAdmin() + action_form = m.action_form + + items = list(action_form.base_fields.items()) + file_format = items[len(items)-1][1] + choices = file_format.choices + + self.assertEqual(choices[0][1], "---") + self.assertEqual(len(choices), 9) + + m = TestFormatsCategoryAdmin() + action_form = m.action_form + + items = list(action_form.base_fields.items()) + file_format = items[len(items)-1][1] + choices = file_format.choices + + self.assertEqual(choices[0][1], "---") + self.assertEqual(len(m.formats) + 1, len(choices)) + + self.assertIn('csv', [c[1] for c in choices]) + self.assertIn('json', [c[1] for c in choices]) + class TestExportEncoding(TestCase): mock_request = MagicMock(spec=HttpRequest) From 64a8ffc3f8dac92358c1a991af2eabb44755115b Mon Sep 17 00:00:00 2001 From: naohide anahara <57.x.mas@gmail.com> Date: Sat, 12 Feb 2022 10:33:16 +0900 Subject: [PATCH 13/77] Add store_raw_values attribute --- import_export/resources.py | 9 ++++++++- tests/core/tests/test_resources.py | 31 ++++++++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/import_export/resources.py b/import_export/resources.py index bbd1a3ebf..d060812fa 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -176,6 +176,12 @@ class ResourceOptions: DEFAULT_DB_ALIAS constant ("default") is used. """ + store_raw_values = False + """ + If True, The raw data will be stored in the result. + Enabling this parameter is going to increase the memory usage and might cause unpredictable results. + """ + class DeclarativeMetaclass(type): @@ -667,7 +673,8 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals """ skip_diff = self._meta.skip_diff row_result = self.get_row_result_class()() - row_result.raw_values = row + if self._meta.store_raw_values: + row_result.raw_values = row original = None try: self.before_import_row(row, **kwargs) diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 1243d9ee5..5676f0592 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -477,8 +477,8 @@ def test_import_data(self): self.assertTrue(result.rows[0].diff) self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_UPDATE) - self.assertEqual(result.rows[0].raw_values.get('name'), 'Some book') - self.assertEqual(result.rows[0].raw_values.get('author_email'), 'test@example.com') + self.assertEqual(result.rows[0].raw_values.get('name'), None) + self.assertEqual(result.rows[0].raw_values.get('author_email'), None) instance = Book.objects.get(pk=self.book.pk) self.assertEqual(instance.author_email, 'test@example.com') @@ -2178,3 +2178,30 @@ def test_bulk_delete_batch_size_of_5(self): self.resource.import_data(self.dataset) self.assertEqual(0, UUIDBook.objects.count()) + +class RawValueTest(TestCase): + def setUp(self): + class _BookResource(resources.ModelResource): + class Meta: + model = Book + store_raw_values = True + + self.resource = _BookResource() + + self.book = Book.objects.create(name="Some book") + self.dataset = tablib.Dataset(headers=['id', 'name', 'author_email', + 'price']) + row = [self.book.pk, 'Some book', 'test@example.com', "10.25"] + self.dataset.append(row) + + def test_import_data(self): + result = self.resource.import_data(self.dataset, raise_errors=True) + + self.assertFalse(result.has_errors()) + self.assertEqual(len(result.rows), 1) + self.assertTrue(result.rows[0].diff) + self.assertEqual(result.rows[0].import_type, + results.RowResult.IMPORT_TYPE_UPDATE) + self.assertEqual(result.rows[0].raw_values.get('name'), 'Some book') + self.assertEqual(result.rows[0].raw_values.get('author_email'), 'test@example.com') + self.assertEqual(result.rows[0].raw_values.get('price'), '10.25') From bae0f3ff3905773c1edc420c493ef251ae4fcd9f Mon Sep 17 00:00:00 2001 From: naohide anahara <57.x.mas@gmail.com> Date: Mon, 14 Feb 2022 12:53:40 +0900 Subject: [PATCH 14/77] Fix property name and description --- import_export/resources.py | 8 ++++---- tests/core/tests/test_resources.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/import_export/resources.py b/import_export/resources.py index d060812fa..bfa359649 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -176,10 +176,10 @@ class ResourceOptions: DEFAULT_DB_ALIAS constant ("default") is used. """ - store_raw_values = False + store_row_values = False """ - If True, The raw data will be stored in the result. - Enabling this parameter is going to increase the memory usage and might cause unpredictable results. + If True, each row's raw data will be stored in each row result. + Enabling this parameter will increase the memory usage during import which should be considered when importing large datasets. """ @@ -673,7 +673,7 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals """ skip_diff = self._meta.skip_diff row_result = self.get_row_result_class()() - if self._meta.store_raw_values: + if self._meta.store_row_values: row_result.raw_values = row original = None try: diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 5676f0592..0b72fd5b8 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -2184,7 +2184,7 @@ def setUp(self): class _BookResource(resources.ModelResource): class Meta: model = Book - store_raw_values = True + store_row_values = True self.resource = _BookResource() From 576e425e866425f509e5def2e25a51730425cf4e Mon Sep 17 00:00:00 2001 From: naohide anahara <57.x.mas@gmail.com> Date: Mon, 14 Feb 2022 21:57:37 +0900 Subject: [PATCH 15/77] Fix raw_values to row_values --- AUTHORS | 1 + import_export/resources.py | 2 +- import_export/results.py | 2 +- tests/core/tests/test_resources.py | 10 +++++----- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/AUTHORS b/AUTHORS index b725be70b..fc817d612 100644 --- a/AUTHORS +++ b/AUTHORS @@ -121,3 +121,4 @@ The following is a list of much appreciated contributors: * striveforbest (Alex Zagoro) * josx (José Luis Di Biase) * Jan Rydzewski +* frgmt \ No newline at end of file diff --git a/import_export/resources.py b/import_export/resources.py index bfa359649..77a01aca7 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -674,7 +674,7 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals skip_diff = self._meta.skip_diff row_result = self.get_row_result_class()() if self._meta.store_row_values: - row_result.raw_values = row + row_result.row_values = row original = None try: self.before_import_row(row, **kwargs) diff --git a/import_export/results.py b/import_export/results.py index 9f6ac5d84..64088257b 100644 --- a/import_export/results.py +++ b/import_export/results.py @@ -32,7 +32,7 @@ def __init__(self): self.validation_error = None self.diff = None self.import_type = None - self.raw_values = {} + self.row_values = {} self.object_id = None self.object_repr = None diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 0b72fd5b8..76e32dc83 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -477,8 +477,8 @@ def test_import_data(self): self.assertTrue(result.rows[0].diff) self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_UPDATE) - self.assertEqual(result.rows[0].raw_values.get('name'), None) - self.assertEqual(result.rows[0].raw_values.get('author_email'), None) + self.assertEqual(result.rows[0].row_values.get('name'), None) + self.assertEqual(result.rows[0].row_values.get('author_email'), None) instance = Book.objects.get(pk=self.book.pk) self.assertEqual(instance.author_email, 'test@example.com') @@ -2202,6 +2202,6 @@ def test_import_data(self): self.assertTrue(result.rows[0].diff) self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_UPDATE) - self.assertEqual(result.rows[0].raw_values.get('name'), 'Some book') - self.assertEqual(result.rows[0].raw_values.get('author_email'), 'test@example.com') - self.assertEqual(result.rows[0].raw_values.get('price'), '10.25') + self.assertEqual(result.rows[0].row_values.get('name'), 'Some book') + self.assertEqual(result.rows[0].row_values.get('author_email'), 'test@example.com') + self.assertEqual(result.rows[0].row_values.get('price'), '10.25') From 3d130236100ea4a3d2d217955c9593f3c6ef2034 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Mon, 14 Feb 2022 17:45:34 +0000 Subject: [PATCH 16/77] updated changelog --- docs/changelog.rst | 1 + import_export/resources.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index a02ed1927..78a013278 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -31,6 +31,7 @@ Enhancements - Updated import.css to support dark mode (#1370) - Default format selections set correctly for export action (#1389) +- Added option to store raw row values in each row's `RowResult` (#1393) Development ########### diff --git a/import_export/resources.py b/import_export/resources.py index c7b4fee2e..0fb1b5746 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -179,7 +179,8 @@ class ResourceOptions: store_row_values = False """ If True, each row's raw data will be stored in each row result. - Enabling this parameter will increase the memory usage during import which should be considered when importing large datasets. + Enabling this parameter will increase the memory usage during import + which should be considered when importing large datasets. """ From 3a1a937f94ab7fa8b5554fa57a15b512dc0bb638 Mon Sep 17 00:00:00 2001 From: naohide anahara <57.x.mas@gmail.com> Date: Tue, 8 Feb 2022 21:02:01 +0900 Subject: [PATCH 17/77] enable storage of row values in RowResult --- AUTHORS | 1 + docs/changelog.rst | 5 ++ import_export/admin.py | 4 +- import_export/resources.py | 25 ++++++--- import_export/results.py | 2 +- tests/core/tests/test_admin_integration.py | 59 ++++++++++++++++++++++ tests/core/tests/test_resources.py | 44 ++++++++++++++++ 7 files changed, 132 insertions(+), 8 deletions(-) diff --git a/AUTHORS b/AUTHORS index b725be70b..fc817d612 100644 --- a/AUTHORS +++ b/AUTHORS @@ -121,3 +121,4 @@ The following is a list of much appreciated contributors: * striveforbest (Alex Zagoro) * josx (José Luis Di Biase) * Jan Rydzewski +* frgmt \ No newline at end of file diff --git a/docs/changelog.rst b/docs/changelog.rst index 5acb438c7..78a013278 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -12,6 +12,9 @@ This release makes the following changes to the API. You may need to update you - Check value of ManyToManyField in skip_row() (#1271) - This fixes an issue where ManyToMany fields are not checked correctly in `skip_row()`. This means that `skip_row()` now takes `row` as a mandatory arg. If you have overridden `skip_row()` in your own implementation, you will need to add `row` as an arg. +- Bug fix: validation errors were being ignored when `skip_unchanged` is set (#1378) + - If you have overridden `skip_row()` you can choose whether or not to skip rows if validation errors are present. The default behavior is to not to skip rows if there are validation errors during import. + - Use 'create' flag instead of instance.pk (#1362) - `import_export.resources.save_instance()` now takes an additional mandatory argument: `is_create`. If you have over-ridden `save_instance()` in your own code, you will need to add this new argument. @@ -27,6 +30,8 @@ Enhancements ############ - Updated import.css to support dark mode (#1370) +- Default format selections set correctly for export action (#1389) +- Added option to store raw row values in each row's `RowResult` (#1393) Development ########### diff --git a/import_export/admin.py b/import_export/admin.py index d36385838..50d6b53eb 100644 --- a/import_export/admin.py +++ b/import_export/admin.py @@ -481,10 +481,12 @@ def __init__(self, *args, **kwargs): choices = [] formats = self.get_export_formats() if formats: - choices.append(('', '---')) for i, f in enumerate(formats): choices.append((str(i), f().get_title())) + if len(formats) > 1: + choices.insert(0, ('', '---')) + self.action_form = export_action_form_factory(choices) super().__init__(*args, **kwargs) diff --git a/import_export/resources.py b/import_export/resources.py index 66a7e4c0a..0fb1b5746 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -176,6 +176,13 @@ class ResourceOptions: DEFAULT_DB_ALIAS constant ("default") is used. """ + store_row_values = False + """ + If True, each row's raw data will be stored in each row result. + Enabling this parameter will increase the memory usage during import + which should be considered when importing large datasets. + """ + class DeclarativeMetaclass(type): @@ -571,7 +578,7 @@ def for_delete(self, row, instance): """ return False - def skip_row(self, instance, original, row): + def skip_row(self, instance, original, row, import_validation_errors=None): """ Returns ``True`` if ``row`` importing should be skipped. @@ -582,7 +589,11 @@ def skip_row(self, instance, original, row): will be None. When left unspecified, skip_diff and skip_unchanged both default to ``False``, - and rows are never skipped. + and rows are never skipped. + + By default, rows are not skipped if validation errors have been detected + during import. You can change this behavior and choose to ignore validation + errors by overriding this method. Override this method to handle skipping rows meeting certain conditions. @@ -590,12 +601,12 @@ def skip_row(self, instance, original, row): Use ``super`` if you want to preserve default handling while overriding :: class YourResource(ModelResource): - def skip_row(self, instance, original): + def skip_row(self, instance, original, row, import_validation_errors=None): # Add code here - return super(YourResource, self).skip_row(instance, original) + return super().skip_row(instance, original, row, import_validation_errors=import_validation_errors) """ - if not self._meta.skip_unchanged or self._meta.skip_diff: + if not self._meta.skip_unchanged or self._meta.skip_diff or import_validation_errors: return False for field in self.get_import_fields(): try: @@ -667,6 +678,8 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals """ skip_diff = self._meta.skip_diff row_result = self.get_row_result_class()() + if self._meta.store_row_values: + row_result.row_values = row original = None try: self.before_import_row(row, **kwargs) @@ -700,7 +713,7 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals # validate_instance(), where they can be combined with model # instance validation errors if necessary import_validation_errors = e.update_error_dict(import_validation_errors) - if self.skip_row(instance, original, row): + if self.skip_row(instance, original, row, import_validation_errors): row_result.import_type = RowResult.IMPORT_TYPE_SKIP else: self.validate_instance(instance, import_validation_errors) diff --git a/import_export/results.py b/import_export/results.py index 9f6ac5d84..64088257b 100644 --- a/import_export/results.py +++ b/import_export/results.py @@ -32,7 +32,7 @@ def __init__(self): self.validation_error = None self.diff = None self.import_type = None - self.raw_values = {} + self.row_values = {} self.object_id = None self.object_repr = None diff --git a/tests/core/tests/test_admin_integration.py b/tests/core/tests/test_admin_integration.py index 5998e5f5f..586484341 100644 --- a/tests/core/tests/test_admin_integration.py +++ b/tests/core/tests/test_admin_integration.py @@ -30,6 +30,7 @@ ExportMixin, ImportExportActionModelAdmin, ) +from import_export.formats import base_formats from import_export.formats.base_formats import DEFAULT_FORMATS from import_export.tmp_storages import TempFolderStorage @@ -607,6 +608,64 @@ def has_export_permission(self, request): with self.assertRaises(PermissionDenied): m.get_export_data('0', Book.objects.none(), request=request) + def test_export_admin_action_one_formats(self): + mock_model = mock.MagicMock() + mock_site = mock.MagicMock() + + class TestCategoryAdmin(ExportActionModelAdmin): + def __init__(self): + super().__init__(mock_model, mock_site) + + formats = [base_formats.CSV] + + m = TestCategoryAdmin() + action_form = m.action_form + + items = list(action_form.base_fields.items()) + file_format = items[len(items)-1][1] + choices = file_format.choices + + self.assertNotEqual(choices[0][0], '---') + self.assertEqual(choices[0][1], "csv") + + def test_export_admin_action_formats(self): + + mock_model = mock.MagicMock() + mock_site = mock.MagicMock() + + class TestCategoryAdmin(ExportActionModelAdmin): + def __init__(self): + super().__init__(mock_model, mock_site) + + class TestFormatsCategoryAdmin(ExportActionModelAdmin): + def __init__(self): + super().__init__(mock_model, mock_site) + + formats = [base_formats.CSV, base_formats.JSON] + + m = TestCategoryAdmin() + action_form = m.action_form + + items = list(action_form.base_fields.items()) + file_format = items[len(items)-1][1] + choices = file_format.choices + + self.assertEqual(choices[0][1], "---") + self.assertEqual(len(choices), 9) + + m = TestFormatsCategoryAdmin() + action_form = m.action_form + + items = list(action_form.base_fields.items()) + file_format = items[len(items)-1][1] + choices = file_format.choices + + self.assertEqual(choices[0][1], "---") + self.assertEqual(len(m.formats) + 1, len(choices)) + + self.assertIn('csv', [c[1] for c in choices]) + self.assertIn('json', [c[1] for c in choices]) + class TestExportEncoding(TestCase): mock_request = MagicMock(spec=HttpRequest) diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 52e2304c3..c947b6195 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -477,6 +477,8 @@ def test_import_data(self): self.assertTrue(result.rows[0].diff) self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_UPDATE) + self.assertEqual(result.rows[0].row_values.get('name'), None) + self.assertEqual(result.rows[0].row_values.get('author_email'), None) instance = Book.objects.get(pk=self.book.pk) self.assertEqual(instance.author_email, 'test@example.com') @@ -514,6 +516,21 @@ def test_import_data_raises_field_specific_validation_errors(self): self.assertIs(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_INVALID) self.assertIn('birthday', result.invalid_rows[0].field_specific_errors) + def test_import_data_raises_field_specific_validation_errors_with_skip_unchanged(self): + resource = AuthorResource() + resource._meta.skip_unchanged = True + + author = Author.objects.create(name="Some author") + + dataset = tablib.Dataset(headers=['id', 'birthday']) + dataset.append([author.id, '1882test-01-18']) + + result = resource.import_data(dataset, raise_errors=False) + + self.assertTrue(result.has_validation_errors()) + self.assertIs(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_INVALID) + self.assertIn('birthday', result.invalid_rows[0].field_specific_errors) + def test_collect_failed_rows(self): resource = ProfileResource() headers = ['id', 'user'] @@ -2176,3 +2193,30 @@ def test_bulk_delete_batch_size_of_5(self): self.resource.import_data(self.dataset) self.assertEqual(0, UUIDBook.objects.count()) + +class RawValueTest(TestCase): + def setUp(self): + class _BookResource(resources.ModelResource): + class Meta: + model = Book + store_row_values = True + + self.resource = _BookResource() + + self.book = Book.objects.create(name="Some book") + self.dataset = tablib.Dataset(headers=['id', 'name', 'author_email', + 'price']) + row = [self.book.pk, 'Some book', 'test@example.com', "10.25"] + self.dataset.append(row) + + def test_import_data(self): + result = self.resource.import_data(self.dataset, raise_errors=True) + + self.assertFalse(result.has_errors()) + self.assertEqual(len(result.rows), 1) + self.assertTrue(result.rows[0].diff) + self.assertEqual(result.rows[0].import_type, + results.RowResult.IMPORT_TYPE_UPDATE) + self.assertEqual(result.rows[0].row_values.get('name'), 'Some book') + self.assertEqual(result.rows[0].row_values.get('author_email'), 'test@example.com') + self.assertEqual(result.rows[0].row_values.get('price'), '10.25') From 27317df2336ae16e95d33df908c5f9639bd1da2c Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 17 Feb 2022 08:40:45 +0000 Subject: [PATCH 18/77] corrected docstring for ForeignKeyWidget.get_queryset() (#1385) --- import_export/widgets.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/import_export/widgets.py b/import_export/widgets.py index 04662e78b..8ebabc1bd 100644 --- a/import_export/widgets.py +++ b/import_export/widgets.py @@ -392,7 +392,7 @@ def get_queryset(self, value, row, *args, **kwargs): like so:: class FullNameForeignKeyWidget(ForeignKeyWidget): - def get_queryset(self, value, row): + def get_queryset(self, value, row, *args, **kwargs): return self.model.objects.filter( first_name__iexact=row["first_name"], last_name__iexact=row["last_name"] From 461d8d8bd4285101e3c62240f7b39a3cb914b2c4 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 17 Feb 2022 09:15:27 +0000 Subject: [PATCH 19/77] clarified changelog --- docs/changelog.rst | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 78a013278..6498b02b2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -7,7 +7,7 @@ Changelog Breaking changes ################ -This release makes the following changes to the API. You may need to update your implementation to accommodate these changes. +This release makes some minor changes to the public API. If you have overridden any methods from the `resources` module, you may need to update your implementation to accommodate these changes. - Check value of ManyToManyField in skip_row() (#1271) - This fixes an issue where ManyToMany fields are not checked correctly in `skip_row()`. This means that `skip_row()` now takes `row` as a mandatory arg. If you have overridden `skip_row()` in your own implementation, you will need to add `row` as an arg. @@ -16,15 +16,20 @@ This release makes the following changes to the API. You may need to update you - If you have overridden `skip_row()` you can choose whether or not to skip rows if validation errors are present. The default behavior is to not to skip rows if there are validation errors during import. - Use 'create' flag instead of instance.pk (#1362) - - `import_export.resources.save_instance()` now takes an additional mandatory argument: `is_create`. If you have over-ridden `save_instance()` in your own code, you will need to add this new argument. + - `import_export.resources.save_instance()` now takes an additional mandatory argument: `is_create`. If you have overridden `save_instance()` in your own code, you will need to add this new argument. + +Deprecations +############ + +This release adds some deprecations which will be removed in the 3.1 release. - Add support for multiple resources in ModelAdmin. (#1223) - - The `*Mixin.resource_class` accepting single resource has been deprecated (will work for few next versions) and the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added. + - The `*Mixin.resource_class` accepting single resource has been deprecated and the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added. - Same applies to all of the `get_resource_class`, `get_import_resource_class` and `get_export_resource_class` methods. -- Deprecated `exceptions.py` - this module will be removed in a future release (#1372) +- Deprecated `exceptions.py` (#1372) Enhancements ############ @@ -39,6 +44,11 @@ Development - Drop support for python3.6, django 2.2, 3.0, 3.1 (#1366) - Increased test coverage, refactored CI build to use tox (#1372) +Documentation +============= + +- Corrected docstring for ForeignKeyWidget.get_queryset() (#1385) + 2.7.1 (2021-12-23) ------------------ From bb0c93bbbbebf510908278e713a5aa525199bda9 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Sun, 20 Feb 2022 13:01:22 +0000 Subject: [PATCH 20/77] fixed broken title formatting in changelog.rst --- docs/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 6498b02b2..feed46687 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -45,7 +45,7 @@ Development - Increased test coverage, refactored CI build to use tox (#1372) Documentation -============= +############# - Corrected docstring for ForeignKeyWidget.get_queryset() (#1385) From 88a7f6b307b238d015dbd54dda85e76b61e2db92 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Fri, 1 Apr 2022 11:23:00 +0100 Subject: [PATCH 21/77] removed duplication from changelog --- docs/changelog.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0d339da8f..97d575f9e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -34,20 +34,19 @@ This release adds some deprecations which will be removed in the 3.1 release. Enhancements ############ -- Updated import.css to support dark mode (#1370) - Default format selections set correctly for export action (#1389) - Added option to store raw row values in each row's `RowResult` (#1393) Development ########### -- Drop support for python3.6, django 2.2, 3.0, 3.1 (#1366) - Increased test coverage, refactored CI build to use tox (#1372) Documentation ############# - Corrected docstring for ForeignKeyWidget.get_queryset() (#1385) + 2.8.0 (2022-03-31) ------------------ From 92bfe67f3de03d00a15fab8c4e0cd136287c7fa1 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Fri, 1 Apr 2022 11:25:28 +0100 Subject: [PATCH 22/77] removed duplication from changelog --- docs/changelog.rst | 5 ----- 1 file changed, 5 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 97d575f9e..b4bcb121d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -42,11 +42,6 @@ Development - Increased test coverage, refactored CI build to use tox (#1372) -Documentation -############# - -- Corrected docstring for ForeignKeyWidget.get_queryset() (#1385) - 2.8.0 (2022-03-31) ------------------ From 1ec76bbb2ad6debf4b9a6694ef23ce5c7981ea17 Mon Sep 17 00:00:00 2001 From: Ryan <62414746+pokken-magic@users.noreply.github.com> Date: Mon, 4 Apr 2022 09:21:08 -0400 Subject: [PATCH 23/77] Add Natural Key support to ForeignKeyWidget (#1371) --- .gitignore | 3 ++ AUTHORS | 1 + docs/changelog.rst | 1 + docs/getting_started.rst | 52 ++++++++++++++++++++++++++++++++ import_export/widgets.py | 26 ++++++++++++---- tests/core/models.py | 47 ++++++++++++++++++++++++++++- tests/core/tests/test_widgets.py | 43 ++++++++++++++++++++++++-- 7 files changed, 163 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index 18322dcc8..74f37dbb4 100644 --- a/.gitignore +++ b/.gitignore @@ -13,4 +13,7 @@ dist/ .coverage *.sw[po] +# IDE support +.vscode + tests/database.db diff --git a/AUTHORS b/AUTHORS index 8f056c6df..2469657b4 100644 --- a/AUTHORS +++ b/AUTHORS @@ -121,6 +121,7 @@ The following is a list of much appreciated contributors: * striveforbest (Alex Zagoro) * josx (José Luis Di Biase) * Jan Rydzewski +* rpsands (Ryan P. Sands) * 2ykwang (Yeongkwang Yang) * KamilRizatdinov (Kamil Rizatdinov) * Mark Walker diff --git a/docs/changelog.rst b/docs/changelog.rst index b4bcb121d..8c1a1361c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -36,6 +36,7 @@ Enhancements - Default format selections set correctly for export action (#1389) - Added option to store raw row values in each row's `RowResult` (#1393) +- Add natural key support to ForeignKeyWidget (#1371) Development ########### diff --git a/docs/getting_started.rst b/docs/getting_started.rst index 889d85500..8de970697 100644 --- a/docs/getting_started.rst +++ b/docs/getting_started.rst @@ -235,6 +235,58 @@ and exporting resource. :doc:`/api_widgets` available widget types and options. +Django Natural Keys +=================== + +The ForeignKeyWidget also supports using Django's natural key functions. A +manager class with the get_by_natural_key function is require for importing +foreign key relationships by the field model's natural key, and the model must +have a natural_key function that can be serialized as a JSON list in order to +export data. + +The primary utility for natural key functionality is to enable exporting data +that can be imported into other Django environments with different numerical +primary key sequences. The natural key functionality enables handling more +complex data than specifying either a single field or the PK. + +The example below illustrates how to create a field on the BookResource that +imports and exports its author relationships using the natural key functions +on the Author model and modelmanager. + +:: + + from import_export.fields import Field + from import_export.widgets import ForeignKeyWidget + + class AuthorManager(models.Manager): + + def get_by_natural_key(self, name): + return self.get(name=name) + + class Author(models.Model): + + objects = AuthorManager() + + name = models.CharField(max_length=100) + birthday = models.DateTimeField(auto_now_add=True) + + def natural_key(self): + return (self.name,) + + class BookResource(resources.ModelResource): + + author = Field( + column_name = "author", + attribute = "author", + widget = ForeignKeyWidget(Author, use_natural_foreign_keys=True) + ) + + class Meta: + model = Book + +Read more at `Django Serialization `_ + + Importing data ============== diff --git a/import_export/widgets.py b/import_export/widgets.py index 8ebabc1bd..721f2907d 100644 --- a/import_export/widgets.py +++ b/import_export/widgets.py @@ -337,7 +337,8 @@ def render(self, value, obj=None): class ForeignKeyWidget(Widget): """ Widget for a ``ForeignKey`` field which looks up a related model using - "natural keys" in both export and import. + either the PK or a user specified field that uniquely identifies the + instance in both export and import. The lookup field defaults to using the primary key (``pk``) as lookup criterion but can be customised to use any field on the related model. @@ -368,13 +369,17 @@ class BookResource(resources.ModelResource): class Meta: fields = ('author',) - + :param model: The Model the ForeignKey refers to (required). - :param field: A field on the related model used for looking up a particular object. + :param field: A field on the related model used for looking up a particular + object. + :param use_natural_foreign_keys: Use natural key functions to identify + related object, default to False """ - def __init__(self, model, field='pk', *args, **kwargs): + def __init__(self, model, field='pk', use_natural_foreign_keys=False, *args, **kwargs): self.model = model self.field = field + self.use_natural_foreign_keys = use_natural_foreign_keys super().__init__(*args, **kwargs) def get_queryset(self, value, row, *args, **kwargs): @@ -403,7 +408,12 @@ def get_queryset(self, value, row, *args, **kwargs): def clean(self, value, row=None, *args, **kwargs): val = super().clean(value) if val: - return self.get_queryset(value, row, *args, **kwargs).get(**{self.field: val}) + if self.use_natural_foreign_keys: + # natural keys will always be a tuple, which ends up as a json list. + value = json.loads(value) + return self.model.objects.get_by_natural_key(*value) + else: + return self.get_queryset(value, row, *args, **kwargs).get(**{self.field: val}) else: return None @@ -414,7 +424,11 @@ def render(self, value, obj=None): attrs = self.field.split('__') for attr in attrs: try: - value = getattr(value, attr, None) + if self.use_natural_foreign_keys: + # inbound natural keys must be a json list. + return json.dumps(value.natural_key()) + else: + value = getattr(value, attr, None) except (ValueError, ObjectDoesNotExist): # needs to have a primary key value before a many-to-many # relationship can be used. diff --git a/tests/core/models.py b/tests/core/models.py index c8d1f1cff..487283b3b 100644 --- a/tests/core/models.py +++ b/tests/core/models.py @@ -6,10 +6,32 @@ from django.db import models +class AuthorManager(models.Manager): + """ + Used to enable the get_by_natural_key method. + NOTE: Manager classes are only required to enable + using the natural key functionality of ForeignKeyWidget + """ + + def get_by_natural_key(self, name): + """ + Django pattern function for finding an author by its name + """ + return self.get(name=name) class Author(models.Model): + + objects = AuthorManager() + name = models.CharField(max_length=100) birthday = models.DateTimeField(auto_now_add=True) + def natural_key(self): + """ + Django pattern function for serializing a model by its natural key + Used only by the ForeignKeyWidget using use_natural_foreign_keys. + """ + return (self.name,) + def __str__(self): return self.name @@ -32,8 +54,23 @@ class Category(models.Model): def __str__(self): return self.name +class BookManager(models.Manager): + """ + Added to enable get_by_natural_key method + NOTE: Manager classes are only required to enable + using the natural key functionality of ForeignKeyWidget + """ + + def get_by_natural_key(self, name, author): + """ + Django pattern function for returning a book by its natural key + """ + return self.get(name=name, author=Author.objects.get_by_natural_key(author)) class Book(models.Model): + + objects = BookManager() + name = models.CharField('Book name', max_length=100) author = models.ForeignKey(Author, blank=True, null=True, on_delete=models.CASCADE) author_email = models.EmailField('Author email', max_length=75, blank=True) @@ -45,6 +82,15 @@ class Book(models.Model): categories = models.ManyToManyField(Category, blank=True) + def natural_key(self): + """ + Django pattern function for serializing a book by its natural key. + Used only by the ForeignKeyWidget using use_natural_foreign_keys. + """ + return (self.name,) + self.author.natural_key() + + natural_key.dependencies = ['core.Author'] + def __str__(self): return self.name @@ -63,7 +109,6 @@ class Child(models.Model): def __str__(self): return '%s - child of %s' % (self.name, self.parent.name) - class Profile(models.Model): user = models.OneToOneField('auth.User', on_delete=models.CASCADE) is_private = models.BooleanField(default=True) diff --git a/tests/core/tests/test_widgets.py b/tests/core/tests/test_widgets.py index 08a1b01b4..51c1ff3e6 100644 --- a/tests/core/tests/test_widgets.py +++ b/tests/core/tests/test_widgets.py @@ -1,10 +1,11 @@ +import json from datetime import date, datetime, time, timedelta from decimal import Decimal from unittest import mock, skipUnless import django import pytz -from core.models import Author, Category +from core.models import Author, Book, Category from django.test import TestCase from django.test.utils import override_settings from django.utils import timezone @@ -292,7 +293,10 @@ class ForeignKeyWidgetTest(TestCase): def setUp(self): self.widget = widgets.ForeignKeyWidget(Author) + self.natural_key_author_widget = widgets.ForeignKeyWidget(Author, use_natural_foreign_keys=True) + self.natural_key_book_widget = widgets.ForeignKeyWidget(Book, use_natural_foreign_keys=True ) self.author = Author.objects.create(name='Foo') + self.book = Book.objects.create(name="Bar", author=self.author) def test_clean(self): self.assertEqual(self.widget.clean(self.author.id), self.author) @@ -340,8 +344,41 @@ def attr(self): t = TestObj() self.widget = widgets.ForeignKeyWidget(mock.Mock(), "attr") self.assertIsNone(self.widget.render(t)) - - + + def test_author_natural_key_clean(self): + """ + Ensure that we can import an author by its natural key. Note that + this will always need to be an iterable. + Generally this will be rendered as a list. + """ + self.assertEqual( + self.natural_key_author_widget.clean( json.dumps(self.author.natural_key()) ), self.author ) + + def test_author_natural_key_render(self): + """ + Ensure we can render an author by its natural key. Natural keys will always be + tuples. + """ + self.assertEqual( + self.natural_key_author_widget.render(self.author), json.dumps(self.author.natural_key()) ) + + def test_book_natural_key_clean(self): + """ + Use the book case to validate a composite natural key of book name and author + can be cleaned. + """ + self.assertEqual( + self.natural_key_book_widget.clean( json.dumps(self.book.natural_key())), self.book + ) + + def test_book_natural_key_render(self): + """ + Use the book case to validate a composite natural key of book name and author + can be rendered + """ + self.assertEqual( + self.natural_key_book_widget.render(self.book), json.dumps(self.book.natural_key()) + ) class ManyToManyWidget(TestCase): From 46fa6b04ccb66fffe40a31ed28ccf538dfb09238 Mon Sep 17 00:00:00 2001 From: Matt Hegarty Date: Tue, 5 Apr 2022 18:27:58 +0100 Subject: [PATCH 24/77] Fix widgets incorrect method definitions (#1413) --- docs/changelog.rst | 17 +++++++++++---- import_export/widgets.py | 36 ++++++++++++++++---------------- tests/core/tests/test_widgets.py | 8 +++---- 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 8c1a1361c..93cd71b4e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -7,17 +7,26 @@ Changelog Breaking changes ################ -This release makes some minor changes to the public API. If you have overridden any methods from the `resources` module, you may need to update your implementation to accommodate these changes. +This release makes some minor changes to the public API. If you have overridden any methods from the `resources` or `widgets` modules, you may need to update your implementation to accommodate these changes. - Check value of ManyToManyField in skip_row() (#1271) + - This fixes an issue where ManyToMany fields are not checked correctly in `skip_row()`. This means that `skip_row()` now takes `row` as a mandatory arg. If you have overridden `skip_row()` in your own implementation, you will need to add `row` as an arg. - Bug fix: validation errors were being ignored when `skip_unchanged` is set (#1378) - - If you have overridden `skip_row()` you can choose whether or not to skip rows if validation errors are present. The default behavior is to not to skip rows if there are validation errors during import. + + - If you have overridden `skip_row()` you can choose whether or not to skip rows if validation errors are present. The default behavior is to not to skip rows if there are validation errors during import. - Use 'create' flag instead of instance.pk (#1362) + - `import_export.resources.save_instance()` now takes an additional mandatory argument: `is_create`. If you have overridden `save_instance()` in your own code, you will need to add this new argument. +- `widgets`: Unused `*args` params have been removed from method definitions. (#1413) + + - If you have overridden `clean()` then you will need to update your method definition to reflect this change. + + - `widgets.ForeignKeyWidget` / `widgets.ManyToManyWidget`: The unused `*args` param has been removed from `__init__()`. If you have overridden `ForeignKeyWidget` or `ManyToManyWidget` you may need to update your implementation to reflect this change. + Deprecations ############ @@ -25,9 +34,9 @@ This release adds some deprecations which will be removed in the 3.1 release. - Add support for multiple resources in ModelAdmin. (#1223) - - The `*Mixin.resource_class` accepting single resource has been deprecated and the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added. + - The `*Mixin.resource_class` accepting single resource has been deprecated and the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added. - - Same applies to all of the `get_resource_class`, `get_import_resource_class` and `get_export_resource_class` methods. + - Same applies to all of the `get_resource_class`, `get_import_resource_class` and `get_export_resource_class` methods. - Deprecated `exceptions.py` (#1372) diff --git a/import_export/widgets.py b/import_export/widgets.py index 721f2907d..512813ce7 100644 --- a/import_export/widgets.py +++ b/import_export/widgets.py @@ -28,7 +28,7 @@ class Widget: :meth:`~import_export.widgets.Widget.clean` and :meth:`~import_export.widgets.Widget.render`. """ - def clean(self, value, row=None, *args, **kwargs): + def clean(self, value, row=None, **kwargs): """ Returns an appropriate Python object for an imported value. @@ -71,7 +71,7 @@ class FloatWidget(NumberWidget): Widget for converting floats fields. """ - def clean(self, value, row=None, *args, **kwargs): + def clean(self, value, row=None, **kwargs): if self.is_empty(value): return None return float(value) @@ -82,7 +82,7 @@ class IntegerWidget(NumberWidget): Widget for converting integer fields. """ - def clean(self, value, row=None, *args, **kwargs): + def clean(self, value, row=None, **kwargs): if self.is_empty(value): return None return int(Decimal(value)) @@ -93,7 +93,7 @@ class DecimalWidget(NumberWidget): Widget for converting decimal fields. """ - def clean(self, value, row=None, *args, **kwargs): + def clean(self, value, row=None, **kwargs): if self.is_empty(value): return None return Decimal(force_str(value)) @@ -152,7 +152,7 @@ def render(self, value, obj=None): return "" return self.TRUE_VALUES[0] if value else self.FALSE_VALUES[0] - def clean(self, value, row=None, *args, **kwargs): + def clean(self, value, row=None, **kwargs): if value in self.NULL_VALUES: return None return True if value in self.TRUE_VALUES else False @@ -175,7 +175,7 @@ def __init__(self, format=None): formats = (format,) self.formats = formats - def clean(self, value, row=None, *args, **kwargs): + def clean(self, value, row=None, **kwargs): if not value: return None if isinstance(value, date): @@ -211,7 +211,7 @@ def __init__(self, format=None): formats = (format,) self.formats = formats - def clean(self, value, row=None, *args, **kwargs): + def clean(self, value, row=None, **kwargs): if not value: return None if isinstance(value, datetime): @@ -254,7 +254,7 @@ def __init__(self, format=None): formats = (format,) self.formats = formats - def clean(self, value, row=None, *args, **kwargs): + def clean(self, value, row=None, **kwargs): if not value: return None if isinstance(value, time): @@ -277,7 +277,7 @@ class DurationWidget(Widget): Widget for converting time duration fields. """ - def clean(self, value, row=None, *args, **kwargs): + def clean(self, value, row=None, **kwargs): if not value: return None @@ -305,7 +305,7 @@ def __init__(self, separator=None): self.separator = separator super().__init__() - def clean(self, value, row=None, *args, **kwargs): + def clean(self, value, row=None, **kwargs): return value.split(self.separator) if value else [] def render(self, value, obj=None): @@ -321,7 +321,7 @@ class JSONWidget(Widget): tries to use single quotes and then convert it to proper JSON. """ - def clean(self, value, row=None, *args, **kwargs): + def clean(self, value, row=None, **kwargs): val = super().clean(value) if val: try: @@ -376,11 +376,11 @@ class Meta: :param use_natural_foreign_keys: Use natural key functions to identify related object, default to False """ - def __init__(self, model, field='pk', use_natural_foreign_keys=False, *args, **kwargs): + def __init__(self, model, field='pk', use_natural_foreign_keys=False, **kwargs): self.model = model self.field = field self.use_natural_foreign_keys = use_natural_foreign_keys - super().__init__(*args, **kwargs) + super().__init__(**kwargs) def get_queryset(self, value, row, *args, **kwargs): """ @@ -405,7 +405,7 @@ def get_queryset(self, value, row, *args, **kwargs): """ return self.model.objects.all() - def clean(self, value, row=None, *args, **kwargs): + def clean(self, value, row=None, **kwargs): val = super().clean(value) if val: if self.use_natural_foreign_keys: @@ -413,7 +413,7 @@ def clean(self, value, row=None, *args, **kwargs): value = json.loads(value) return self.model.objects.get_by_natural_key(*value) else: - return self.get_queryset(value, row, *args, **kwargs).get(**{self.field: val}) + return self.get_queryset(value, row, **kwargs).get(**{self.field: val}) else: return None @@ -449,7 +449,7 @@ class ManyToManyWidget(Widget): :param field: A field on the related model. Default is ``pk``. """ - def __init__(self, model, separator=None, field=None, *args, **kwargs): + def __init__(self, model, separator=None, field=None, **kwargs): if separator is None: separator = ',' if field is None: @@ -457,9 +457,9 @@ def __init__(self, model, separator=None, field=None, *args, **kwargs): self.model = model self.separator = separator self.field = field - super().__init__(*args, **kwargs) + super().__init__(**kwargs) - def clean(self, value, row=None, *args, **kwargs): + def clean(self, value, row=None, **kwargs): if not value: return self.model.objects.none() if isinstance(value, (float, int)): diff --git a/tests/core/tests/test_widgets.py b/tests/core/tests/test_widgets.py index 51c1ff3e6..0c3252f21 100644 --- a/tests/core/tests/test_widgets.py +++ b/tests/core/tests/test_widgets.py @@ -320,8 +320,8 @@ def get_queryset(self, value, row, *args, **kwargs): author2.birthday = "2016-01-01" author2.save() birthday_widget = BirthdayWidget(Author, 'name') - row = {'name': "Foo", 'birthday': author2.birthday} - self.assertEqual(birthday_widget.clean("Foo", row), author2) + row_dict = {'name': "Foo", 'birthday': author2.birthday} + self.assertEqual(birthday_widget.clean("Foo", row=row_dict), author2) def test_invalid_get_queryset(self): class BirthdayWidget(widgets.ForeignKeyWidget): @@ -331,9 +331,9 @@ def get_queryset(self, value, row): ) birthday_widget = BirthdayWidget(Author, 'name') - row = {'name': "Foo", 'age': 38} + row_dict = {'name': "Foo", 'age': 38} with self.assertRaises(TypeError): - birthday_widget.clean("Foo", row, row_number=1) + birthday_widget.clean("Foo", row=row_dict, row_number=1) def test_render_handles_value_error(self): class TestObj(object): From f335dba39aa4d64f6024d69ea3610bd116ba1d48 Mon Sep 17 00:00:00 2001 From: Matt Hegarty Date: Tue, 5 Apr 2022 20:13:03 +0100 Subject: [PATCH 25/77] optimised default instantiation of `CharWidget` (#1414) --- docs/changelog.rst | 5 +++-- import_export/resources.py | 1 + import_export/widgets.py | 4 +--- tests/core/tests/test_resources.py | 10 ++++++++++ tests/core/tests/test_widgets.py | 22 ++++++++++++++++++++++ 5 files changed, 37 insertions(+), 5 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 93cd71b4e..02ad8564a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,7 +9,7 @@ Breaking changes This release makes some minor changes to the public API. If you have overridden any methods from the `resources` or `widgets` modules, you may need to update your implementation to accommodate these changes. -- Check value of ManyToManyField in skip_row() (#1271) +- Check value of `ManyToManyField` in skip_row() (#1271) - This fixes an issue where ManyToMany fields are not checked correctly in `skip_row()`. This means that `skip_row()` now takes `row` as a mandatory arg. If you have overridden `skip_row()` in your own implementation, you will need to add `row` as an arg. @@ -45,7 +45,8 @@ Enhancements - Default format selections set correctly for export action (#1389) - Added option to store raw row values in each row's `RowResult` (#1393) -- Add natural key support to ForeignKeyWidget (#1371) +- Add natural key support to `ForeignKeyWidget` (#1371) +- Optimised default instantiation of `CharWidget` (#1414) Development ########### diff --git a/import_export/resources.py b/import_export/resources.py index 0fb1b5746..1c143b11c 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -1048,6 +1048,7 @@ class ModelResource(Resource, metaclass=ModelDeclarativeMetaclass): 'ManyToManyField': 'get_m2m_widget', 'OneToOneField': 'get_fk_widget', 'ForeignKey': 'get_fk_widget', + 'CharField': widgets.CharWidget, 'DecimalField': widgets.DecimalWidget, 'DateTimeField': widgets.DateTimeWidget, 'DateField': widgets.DateWidget, diff --git a/import_export/widgets.py b/import_export/widgets.py index 512813ce7..509a896d9 100644 --- a/import_export/widgets.py +++ b/import_export/widgets.py @@ -103,9 +103,7 @@ class CharWidget(Widget): """ Widget for converting text fields. """ - - def render(self, value, obj=None): - return force_str(value) + pass class BooleanWidget(Widget): diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 0b2458992..1aa612af9 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -1303,6 +1303,15 @@ def test_create(self): self.assertEqual(BookResource._meta.model, Book) +class WidgetFromDjangoFieldTest(TestCase): + + def test_widget_from_django_field_for_CharField_returns_CharWidget(self): + f = CharField() + resource = BookResource() + w = resource.widget_from_django_field(f) + self.assertEqual(widgets.CharWidget, w) + + @skipUnless( 'postgresql' in settings.DATABASES['default']['ENGINE'], 'Run only against Postgres') @@ -1327,6 +1336,7 @@ def test_widget_from_django_field_for_ArrayField_returns_SimpleArrayWidget(self) res = resource.widget_from_django_field(f) self.assertEqual(widgets.SimpleArrayWidget, res) + if 'postgresql' in settings.DATABASES['default']['ENGINE']: from django.contrib.postgres.fields import ArrayField from django.db import models diff --git a/tests/core/tests/test_widgets.py b/tests/core/tests/test_widgets.py index 0c3252f21..3cf3a9669 100644 --- a/tests/core/tests/test_widgets.py +++ b/tests/core/tests/test_widgets.py @@ -13,6 +13,28 @@ from import_export import widgets +class WidgetTest(TestCase): + def setUp(self): + self.widget = widgets.Widget() + + def test_clean(self): + self.assertEqual("a", self.widget.clean("a")) + + def test_render(self): + self.assertEqual("1", self.widget.render(1)) + + +class CharWidgetTest(TestCase): + def setUp(self): + self.widget = widgets.CharWidget() + + def test_clean(self): + self.assertEqual("a", self.widget.clean("a")) + + def test_render(self): + self.assertEqual("1", self.widget.render(1)) + + class BooleanWidgetTest(TestCase): def setUp(self): From 0ba35826da51f437f91c87a90029c17a928c921e Mon Sep 17 00:00:00 2001 From: Manel Clos Date: Wed, 6 Apr 2022 15:48:07 +0200 Subject: [PATCH 26/77] ManyToManyWidget: fix handling unordered lists in skip_row() (#1391) --- import_export/resources.py | 20 ++++++++--------- tests/core/tests/test_resources.py | 36 +++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/import_export/resources.py b/import_export/resources.py index 1c143b11c..87517a444 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -609,18 +609,17 @@ def skip_row(self, instance, original, row, import_validation_errors=None): if not self._meta.skip_unchanged or self._meta.skip_diff or import_validation_errors: return False for field in self.get_import_fields(): - try: - # For fields that are models.fields.related.ManyRelatedManager - # we need to compare the results - if isinstance(field.widget, widgets.ManyToManyWidget): - # compare with the future value to detect changes - instance_value = list(field.clean(row)) - else: - instance_value = list(field.get_value(instance).all()) + # For fields that are models.fields.related.ManyRelatedManager + # we need to compare the results + if isinstance(field.widget, widgets.ManyToManyWidget): + instance_values = list(field.clean(row)) + original_values = list(field.get_value(original).all()) + if len(instance_values) != len(original_values): + return False - if instance_value != list(field.get_value(original).all()): + if sorted(v.id for v in instance_values) != sorted(v.id for v in original_values): return False - except AttributeError: + else: if field.get_value(instance) != field.get_value(original): return False return True @@ -713,6 +712,7 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals # validate_instance(), where they can be combined with model # instance validation errors if necessary import_validation_errors = e.update_error_dict(import_validation_errors) + if self.skip_row(instance, original, row, import_validation_errors): row_result.import_type = RowResult.IMPORT_TYPE_SKIP else: diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 1aa612af9..7610e1e71 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -1548,7 +1548,7 @@ def test_many_to_many_widget_no_changes(self): # should be skipped, because there is no change book = Book.objects.first() dataset_headers = ["id", "name", "categories"] - dataset_row = [book.id, book.name, book.categories.all()] + dataset_row = [book.id, book.name, book.categories.first().id] dataset = tablib.Dataset(headers=dataset_headers) dataset.append(dataset_row) @@ -1560,6 +1560,40 @@ def test_many_to_many_widget_no_changes(self): self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_SKIP) self.assertEqual(1, book.categories.count()) + def test_many_to_many_widget_handles_ordering(self): + # the book is associated with 2 categories ('Category 1', 'Category 2') + # when we import a row with a book with both categories (in any order), the book + # should be skipped, because there is no change + book = Book.objects.first() + self.assertEqual(1, book.categories.count()) + cat1 = Category.objects.get(name="Category 1") + cat2 = Category.objects.get(name="Category 2") + book.categories.add(cat1) + book.save() + self.assertEqual(2, book.categories.count()) + dataset_headers = ["id", "name", "categories"] + + book_resource = BookResource() + book_resource._meta.skip_unchanged = True + + # import with natural order + dataset_row = [book.id, book.name, f"{cat1.id},{cat2.id}"] + dataset = tablib.Dataset(headers=dataset_headers) + dataset.append(dataset_row) + + result = book_resource.import_data(dataset, dry_run=False) + self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_SKIP) + + # import with reverse order + dataset_row = [book.id, book.name, f"{cat2.id},{cat1.id}"] + dataset = tablib.Dataset(headers=dataset_headers) + dataset.append(dataset_row) + + result = book_resource.import_data(dataset, dry_run=False) + self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_SKIP) + + self.assertEqual(2, book.categories.count()) + @mock.patch("import_export.resources.Diff", spec=True) class SkipDiffTest(TestCase): From c784d26dd271ab7b6bf87ad3da6627fff46b0735 Mon Sep 17 00:00:00 2001 From: Matt Hegarty Date: Wed, 6 Apr 2022 15:15:45 +0100 Subject: [PATCH 27/77] Refactor admin import to include encoding param (#1306) --- docs/changelog.rst | 21 +- docs/getting_started.rst | 43 +++ docs/installation.rst | 2 + import_export/admin.py | 107 ++++--- import_export/formats/base_formats.py | 12 +- import_export/tmp_storages.py | 45 +-- tests/core/exports/books-ISO-8859-1.csv | 2 + tests/core/tests/test_admin_integration.py | 326 +++++++++++++++++++-- tests/core/tests/test_tmp_storages.py | 45 ++- 9 files changed, 489 insertions(+), 114 deletions(-) create mode 100644 tests/core/exports/books-ISO-8859-1.csv diff --git a/docs/changelog.rst b/docs/changelog.rst index 02ad8564a..d2facb01f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,22 +10,20 @@ Breaking changes This release makes some minor changes to the public API. If you have overridden any methods from the `resources` or `widgets` modules, you may need to update your implementation to accommodate these changes. - Check value of `ManyToManyField` in skip_row() (#1271) - - This fixes an issue where ManyToMany fields are not checked correctly in `skip_row()`. This means that `skip_row()` now takes `row` as a mandatory arg. If you have overridden `skip_row()` in your own implementation, you will need to add `row` as an arg. - Bug fix: validation errors were being ignored when `skip_unchanged` is set (#1378) - - If you have overridden `skip_row()` you can choose whether or not to skip rows if validation errors are present. The default behavior is to not to skip rows if there are validation errors during import. - Use 'create' flag instead of instance.pk (#1362) - - `import_export.resources.save_instance()` now takes an additional mandatory argument: `is_create`. If you have overridden `save_instance()` in your own code, you will need to add this new argument. - `widgets`: Unused `*args` params have been removed from method definitions. (#1413) + - If you have overridden `clean()` then you should update your method definition to reflect this change. + - `widgets.ForeignKeyWidget` / `widgets.ManyToManyWidget`: The unused `*args` param has been removed from `__init__()`. If you have overridden `ForeignKeyWidget` or `ManyToManyWidget` you may need to update your implementation to reflect this change. - - If you have overridden `clean()` then you will need to update your method definition to reflect this change. - - - `widgets.ForeignKeyWidget` / `widgets.ManyToManyWidget`: The unused `*args` param has been removed from `__init__()`. If you have overridden `ForeignKeyWidget` or `ManyToManyWidget` you may need to update your implementation to reflect this change. +- Admin interface: Modified handling of import errors (#1306) + - Exceptions raised during the import process are now presented as form errors, instead of being wrapped in a \ tag in the response. If you have any custom logic which uses the error written directly into the response, then this may need to be changed. Deprecations ############ @@ -33,10 +31,8 @@ Deprecations This release adds some deprecations which will be removed in the 3.1 release. - Add support for multiple resources in ModelAdmin. (#1223) - - - The `*Mixin.resource_class` accepting single resource has been deprecated and the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added. - - - Same applies to all of the `get_resource_class`, `get_import_resource_class` and `get_export_resource_class` methods. + - The `*Mixin.resource_class` accepting single resource has been deprecated and the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added. + - Same applies to all of the `get_resource_class`, `get_import_resource_class` and `get_export_resource_class` methods. - Deprecated `exceptions.py` (#1372) @@ -53,6 +49,11 @@ Development - Increased test coverage, refactored CI build to use tox (#1372) +Documentation +############# + +- Clarified issues around the usage of temporary storage (#1306) + 2.8.0 (2022-03-31) ------------------ diff --git a/docs/getting_started.rst b/docs/getting_started.rst index 8de970697..51c7867fa 100644 --- a/docs/getting_started.rst +++ b/docs/getting_started.rst @@ -392,10 +392,14 @@ mixins (:class:`~import_export.admin.ImportMixin`, admin.site.register(Book, BookAdmin) +.. _change-screen-figure: + .. figure:: _static/images/django-import-export-change.png A screenshot of the change view with Import and Export buttons. +.. _confirm-import-figure: + .. figure:: _static/images/django-import-export-import.png A screenshot of the import view. @@ -530,6 +534,45 @@ as well as importing customizations. :doc:`/api_admin` available mixins and options. +Import confirmation +~~~~~~~~~~~~~~~~~~~ + +Importing in the Admin site is a two step process. + +#. Choose the file to import (:ref:`screenshot`). +#. Review changes and confirm import (:ref:`screenshot`). + +To support this, uploaded data is written to temporary storage after step 1, and read +back for final import after step 2. + +There are three mechanisms for temporary storage. + +#. Temporary file storage on the host server (default). This is suitable for development only. + Use of temporary filesystem storage is not recommended for production sites. + +#. The `Django cache `_. + +#. `Django storage `_. + +To modify which storage mechanism is used, please refer to the setting :ref:`IMPORT_EXPORT_TMP_STORAGE_CLASS`. + +Temporary resources are removed when data is successfully imported after the confirmation step. + +Your choice of temporary storage will be influenced by the following factors: + +* Sensitivity of the data being imported. +* Volume and frequency of uploads. +* File upload size. +* Use of containers or load-balanced servers. + +.. warning:: + + If users do not complete the confirmation step of the workflow, + or if there are errors during import, then temporary resources may not be deleted. + This will need to be understood and managed in production settings. + For example, using a cache expiration policy or cron job to clear stale resources. + + Using multiple resources ------------------------ diff --git a/docs/installation.rst b/docs/installation.rst index c1a7826c4..5901bdb14 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -65,6 +65,8 @@ of losing an audit trail. Can be overridden on a ``ModelAdmin`` class inheriting from ``ImportMixin`` by setting the ``skip_admin_log`` class attribute. +.. _IMPORT_EXPORT_TMP_STORAGE_CLASS: + ``IMPORT_EXPORT_TMP_STORAGE_CLASS`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/import_export/admin.py b/import_export/admin.py index f9fcbe0ff..8a8fdb9a7 100644 --- a/import_export/admin.py +++ b/import_export/admin.py @@ -10,7 +10,6 @@ from django.template.response import TemplateResponse from django.urls import path, reverse from django.utils.decorators import method_decorator -from django.utils.encoding import force_str from django.utils.module_loading import import_string from django.utils.translation import gettext_lazy as _ from django.views.decorators.http import require_POST @@ -19,7 +18,7 @@ from .mixins import BaseExportMixin, BaseImportMixin from .results import RowResult from .signals import post_export, post_import -from .tmp_storages import TempFolderStorage +from .tmp_storages import MediaStorage, TempFolderStorage class ImportExportMixinBase: @@ -41,7 +40,7 @@ class ImportMixin(BaseImportMixin, ImportExportMixinBase): #: template for import view import_template_name = 'admin/import_export/import.html' #: import data encoding - from_encoding = "utf-8" + from_encoding = "utf-8-sig" skip_admin_log = None # storage class for saving temporary files tmp_storage_class = None @@ -103,13 +102,17 @@ def process_import(self, request, *args, **kwargs): import_formats = self.get_import_formats() input_format = import_formats[ int(confirm_form.cleaned_data['input_format']) - ]() - tmp_storage = self.get_tmp_storage_class()(name=confirm_form.cleaned_data['import_file_name']) - data = tmp_storage.read(input_format.get_read_mode()) - if not input_format.is_binary() and self.from_encoding: - data = force_str(data, self.from_encoding) - dataset = input_format.create_dataset(data) + ](encoding=self.from_encoding) + encoding = None if input_format.is_binary() else self.from_encoding + tmp_storage_cls = self.get_tmp_storage_class() + tmp_storage = tmp_storage_cls( + name=confirm_form.cleaned_data['import_file_name'], + encoding=encoding, + read_mode=input_format.get_read_mode() + ) + data = tmp_storage.read() + dataset = input_format.create_dataset(data) result = self.process_dataset(dataset, confirm_form, request, *args, **kwargs) tmp_storage.remove() @@ -214,18 +217,26 @@ def get_import_data_kwargs(self, request, *args, **kwargs): return {} def write_to_tmp_storage(self, import_file, input_format): - tmp_storage = self.get_tmp_storage_class()() + encoding = None + if not input_format.is_binary(): + encoding = self.from_encoding + + tmp_storage_cls = self.get_tmp_storage_class() + tmp_storage = tmp_storage_cls(encoding=encoding, read_mode=input_format.get_read_mode()) data = bytes() for chunk in import_file.chunks(): data += chunk - tmp_storage.save(data, input_format.get_read_mode()) + if tmp_storage_cls == MediaStorage and not input_format.is_binary(): + data = data.decode(self.from_encoding) + + tmp_storage.save(data) return tmp_storage def import_action(self, request, *args, **kwargs): """ Perform a dry_run of the import to make sure the import will not - result in errors. If there where no error, save the user + result in errors. If there are no errors, save the user uploaded file to a local temp file that will be used by 'process_import' for the actual import. """ @@ -243,52 +254,56 @@ def import_action(self, request, *args, **kwargs): request.FILES or None, **form_kwargs) + resources = list() if request.POST and form.is_valid(): input_format = import_formats[ int(form.cleaned_data['input_format']) ]() + if not input_format.is_binary(): + input_format.encoding = self.from_encoding + import_file = form.cleaned_data['import_file'] # first always write the uploaded file to disk as it may be a # memory file or else based on settings upload handlers tmp_storage = self.write_to_tmp_storage(import_file, input_format) - # then read the file, using the proper format-specific mode - # warning, big files may exceed memory try: - data = tmp_storage.read(input_format.get_read_mode()) - if not input_format.is_binary() and self.from_encoding: - data = force_str(data, self.from_encoding) + # then read the file, using the proper format-specific mode + # warning, big files may exceed memory + data = tmp_storage.read() dataset = input_format.create_dataset(data) - except UnicodeDecodeError as e: - return HttpResponse(_(u"

Imported file has a wrong encoding: %s

" % e)) except Exception as e: - return HttpResponse(_(u"

%s encountered while trying to read file: %s

" % (type(e).__name__, import_file.name))) - - # prepare kwargs for import data, if needed - res_kwargs = self.get_import_resource_kwargs(request, form=form, *args, **kwargs) - resource = self.choose_import_resource_class(form)(**res_kwargs) - resources = [resource] - - # prepare additional kwargs for import_data, if needed - imp_kwargs = self.get_import_data_kwargs(request, form=form, *args, **kwargs) - result = resource.import_data(dataset, dry_run=True, - raise_errors=False, - file_name=import_file.name, - user=request.user, - **imp_kwargs) - - context['result'] = result - - if not result.has_errors() and not result.has_validation_errors(): - initial = { - 'import_file_name': tmp_storage.name, - 'original_file_name': import_file.name, - 'input_format': form.cleaned_data['input_format'], - 'resource': request.POST.get('resource', ''), - } - confirm_form = self.get_confirm_import_form() - initial = self.get_form_kwargs(form=form, **initial) - context['confirm_form'] = confirm_form(initial=initial) + form.add_error('import_file', + _(f"'{type(e).__name__}' encountered while trying to read file. " + "Ensure you have chosen the correct format for the file. " + f"{str(e)}")) + + if not form.errors: + # prepare kwargs for import data, if needed + res_kwargs = self.get_import_resource_kwargs(request, form=form, *args, **kwargs) + resource = self.choose_import_resource_class(form)(**res_kwargs) + resources = [resource] + + # prepare additional kwargs for import_data, if needed + imp_kwargs = self.get_import_data_kwargs(request, form=form, *args, **kwargs) + result = resource.import_data(dataset, dry_run=True, + raise_errors=False, + file_name=import_file.name, + user=request.user, + **imp_kwargs) + + context['result'] = result + + if not result.has_errors() and not result.has_validation_errors(): + initial = { + 'import_file_name': tmp_storage.name, + 'original_file_name': import_file.name, + 'input_format': form.cleaned_data['input_format'], + 'resource': request.POST.get('resource', ''), + } + confirm_form = self.get_confirm_import_form() + initial = self.get_form_kwargs(form=form, **initial) + context['confirm_form'] = confirm_form(initial=initial) else: res_kwargs = self.get_import_resource_kwargs(request, form=form, *args, **kwargs) resource_classes = self.get_import_resource_classes() diff --git a/import_export/formats/base_formats.py b/import_export/formats/base_formats.py index accd0e161..f6994a2e4 100644 --- a/import_export/formats/base_formats.py +++ b/import_export/formats/base_formats.py @@ -56,6 +56,9 @@ class TablibFormat(Format): TABLIB_MODULE = None CONTENT_TYPE = 'application/octet-stream' + def __init__(self, encoding=None): + self.encoding = encoding + def get_format(self): """ Import and returns tablib module. @@ -96,6 +99,12 @@ def can_export(self): class TextFormat(TablibFormat): + + def create_dataset(self, in_stream, **kwargs): + if isinstance(in_stream, bytes) and self.encoding: + in_stream = in_stream.decode(self.encoding) + return super().create_dataset(in_stream, **kwargs) + def get_read_mode(self): return 'r' @@ -107,9 +116,6 @@ class CSV(TextFormat): TABLIB_MODULE = 'tablib.formats._csv' CONTENT_TYPE = 'text/csv' - def create_dataset(self, in_stream, **kwargs): - return super().create_dataset(in_stream, **kwargs) - class JSON(TextFormat): TABLIB_MODULE = 'tablib.formats._json' diff --git a/import_export/tmp_storages.py b/import_export/tmp_storages.py index e732218f8..0bbbdf34a 100644 --- a/import_export/tmp_storages.py +++ b/import_export/tmp_storages.py @@ -9,13 +9,15 @@ class BaseStorage: - def __init__(self, name=None): + def __init__(self, name=None, read_mode='r', encoding=None): self.name = name + self.read_mode = read_mode + self.encoding = encoding - def save(self, data, mode='w'): + def save(self, data): raise NotImplementedError - def read(self, read_mode='r'): + def read(self): raise NotImplementedError def remove(self): @@ -24,20 +26,12 @@ def remove(self): class TempFolderStorage(BaseStorage): - def open(self, mode='r'): - if self.name: - return open(self.get_full_path(), mode) - else: - tmp_file = tempfile.NamedTemporaryFile(delete=False) - self.name = tmp_file.name - return tmp_file - - def save(self, data, mode='w'): - with self.open(mode=mode) as file: + def save(self, data): + with self._open(mode='w') as file: file.write(data) - def read(self, mode='r'): - with self.open(mode=mode) as file: + def read(self): + with self._open(mode=self.read_mode) as file: return file.read() def remove(self): @@ -49,6 +43,14 @@ def get_full_path(self): self.name ) + def _open(self, mode='r'): + if self.name: + return open(self.get_full_path(), mode, encoding=self.encoding) + else: + tmp_file = tempfile.NamedTemporaryFile(delete=False) + self.name = tmp_file.name + return tmp_file + class CacheStorage(BaseStorage): """ @@ -57,12 +59,12 @@ class CacheStorage(BaseStorage): CACHE_LIFETIME = 86400 CACHE_PREFIX = 'django-import-export-' - def save(self, data, mode=None): + def save(self, data): if not self.name: self.name = uuid4().hex cache.set(self.CACHE_PREFIX + self.name, data, self.CACHE_LIFETIME) - def read(self, read_mode='r'): + def read(self): return cache.get(self.CACHE_PREFIX + self.name) def remove(self): @@ -72,13 +74,16 @@ def remove(self): class MediaStorage(BaseStorage): MEDIA_FOLDER = 'django-import-export' - def save(self, data, mode=None): + def __init__(self, name=None, read_mode='rb', encoding=None): + super().__init__(name, read_mode=read_mode, encoding=encoding) + + def save(self, data): if not self.name: self.name = uuid4().hex default_storage.save(self.get_full_path(), ContentFile(data)) - def read(self, read_mode='rb'): - with default_storage.open(self.get_full_path(), mode=read_mode) as f: + def read(self): + with default_storage.open(self.get_full_path(), mode=self.read_mode) as f: return f.read() def remove(self): diff --git a/tests/core/exports/books-ISO-8859-1.csv b/tests/core/exports/books-ISO-8859-1.csv new file mode 100644 index 000000000..b27422355 --- /dev/null +++ b/tests/core/exports/books-ISO-8859-1.csv @@ -0,0 +1,2 @@ +id,name,author_email +1,Merci à toi,test@example.com diff --git a/tests/core/tests/test_admin_integration.py b/tests/core/tests/test_admin_integration.py index 586484341..9e6008dfc 100644 --- a/tests/core/tests/test_admin_integration.py +++ b/tests/core/tests/test_admin_integration.py @@ -1,9 +1,11 @@ import os.path +import warnings from datetime import datetime -from unittest import mock +from unittest import mock, skip from unittest.mock import MagicMock import chardet +import django import tablib from core.admin import ( AuthorAdmin, @@ -135,6 +137,128 @@ def test_import_second_resource(self): # Check, that we really use second resource - author_email didn't get imported self.assertEqual(Book.objects.get(id=1).author_email, "") + def test_import_action_handles_UnicodeDecodeError_as_form_error(self): + # POST the import form + input_format = '0' + filename = os.path.join( + os.path.dirname(__file__), + os.path.pardir, + 'exports', + 'books.csv') + with open(filename, "rb") as f: + data = { + 'input_format': input_format, + 'import_file': f, + } + with mock.patch("import_export.admin.TempFolderStorage.read") as mock_tmp_folder_storage: + b_arr = b'\x00' + mock_tmp_folder_storage.side_effect = UnicodeDecodeError('codec', b_arr, 1, 2, 'fail!') + response = self.client.post('/admin/core/book/import/', data) + self.assertEqual(response.status_code, 200) + target_msg = ( + '\'UnicodeDecodeError\' encountered while trying to read file. ' + 'Ensure you have chosen the correct format for the file. ' + '\'codec\' codec can\'t decode bytes in position 1-1: fail!' + ) + # required for testing via tox + # remove after django 5.0 released + if django.VERSION >= (4, 0): + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + try: + self.assertFormError(response.context['form'], 'import_file', target_msg) + except TypeError: + self.assertFormError(response, 'form', 'import_file', target_msg) + else: + self.assertFormError(response, 'form', 'import_file', target_msg) + + def test_import_action_handles_ValueError_as_form_error(self): + # POST the import form + input_format = '0' + filename = os.path.join( + os.path.dirname(__file__), + os.path.pardir, + 'exports', + 'books.csv') + with open(filename, "rb") as f: + data = { + 'input_format': input_format, + 'import_file': f, + } + with mock.patch("import_export.admin.TempFolderStorage.read") as mock_tmp_folder_storage: + mock_tmp_folder_storage.side_effect = ValueError('some unknown error') + response = self.client.post('/admin/core/book/import/', data) + self.assertEqual(response.status_code, 200) + target_msg = ( + '\'ValueError\' encountered while trying to read file. ' + 'Ensure you have chosen the correct format for the file. ' + 'some unknown error' + ) + + # required for testing via tox + # remove after django 5.0 released + if django.VERSION >= (4, 0): + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + try: + self.assertFormError(response.context['form'], 'import_file', target_msg) + except TypeError: + self.assertFormError(response, 'form', 'import_file', target_msg) + else: + self.assertFormError(response, 'form', 'import_file', target_msg) + + def assert_string_in_response(self, filename, input_format, encoding=None): + input_format = input_format + filename = os.path.join( + os.path.dirname(__file__), + os.path.pardir, + 'exports', + filename) + with open(filename, "rb") as f: + data = { + 'input_format': input_format, + 'import_file': f, + } + if encoding: + BookAdmin.from_encoding = encoding + response = self.client.post('/admin/core/book/import/', data) + self.assertEqual(response.status_code, 200) + self.assertIn('result', response.context) + self.assertFalse(response.context['result'].has_errors()) + self.assertContains(response, 'test@example.com') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.TempFolderStorage') + def test_import_action_handles_TempFolderStorage_read(self): + self.assert_string_in_response('books.csv', '0') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.TempFolderStorage') + def test_import_action_handles_TempFolderStorage_read_iso_8859_1(self): + self.assert_string_in_response('books-ISO-8859-1.csv', '0', 'ISO-8859-1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.CacheStorage') + def test_import_action_handles_CacheStorage_read(self): + self.assert_string_in_response('books.csv', '0') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.CacheStorage') + def test_import_action_handles_CacheStorage_read_iso_8859_1(self): + self.assert_string_in_response('books-ISO-8859-1.csv', '0', 'ISO-8859-1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.CacheStorage') + def test_import_action_handles_CacheStorage_read_binary(self): + self.assert_string_in_response('books.xls', '1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.MediaStorage') + def test_import_action_handles_MediaStorage_read(self): + self.assert_string_in_response('books.csv', '0') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.MediaStorage') + def test_import_action_handles_MediaStorage_read_iso_8859_1(self): + self.assert_string_in_response('books-ISO-8859-1.csv', '0', 'ISO-8859-1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.MediaStorage') + def test_import_action_handles_MediaStorage_read_binary(self): + self.assert_string_in_response('books.xls', '1') + def test_delete_from_admin(self): # test delete from admin site (see #432) @@ -515,6 +639,175 @@ def __init__(self): self.assertEqual('import_export/action_formats.js', target_media._js[-1]) +class ConfirmImportEncodingTest(TestCase): + """Test handling 'confirm import' step using different file encodings + and storage types. + """ + + def setUp(self): + user = User.objects.create_user('admin', 'admin@example.com', + 'password') + user.is_staff = True + user.is_superuser = True + user.save() + self.client.login(username='admin', password='password') + + def assert_string_in_response(self, filename, input_format, encoding=None): + input_format = input_format + filename = os.path.join( + os.path.dirname(__file__), + os.path.pardir, + 'exports', + filename) + with open(filename, "rb") as f: + data = { + 'input_format': input_format, + 'import_file': f, + } + if encoding: + BookAdmin.from_encoding = encoding + response = self.client.post('/admin/core/book/import/', data) + self.assertEqual(response.status_code, 200) + self.assertIn('result', response.context) + self.assertFalse(response.context['result'].has_errors()) + self.assertContains(response, 'test@example.com') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.TempFolderStorage') + def test_import_action_handles_TempFolderStorage_read(self): + self.assert_string_in_response('books.csv', '0') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.TempFolderStorage') + def test_import_action_handles_TempFolderStorage_read_mac(self): + self.assert_string_in_response('books-mac.csv', '0') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.TempFolderStorage') + def test_import_action_handles_TempFolderStorage_read_iso_8859_1(self): + self.assert_string_in_response('books-ISO-8859-1.csv', '0', 'ISO-8859-1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.TempFolderStorage') + def test_import_action_handles_TempFolderStorage_read_binary(self): + self.assert_string_in_response('books.xls', '1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.CacheStorage') + def test_import_action_handles_CacheStorage_read(self): + self.assert_string_in_response('books.csv', '0') + + @skip("waiting for fix in tablib - see #1397") + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.CacheStorage') + def test_import_action_handles_CacheStorage_read_mac(self): + self.assert_string_in_response('books-mac.csv', '0') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.CacheStorage') + def test_import_action_handles_CacheStorage_read_iso_8859_1(self): + self.assert_string_in_response('books-ISO-8859-1.csv', '0', 'ISO-8859-1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.CacheStorage') + def test_import_action_handles_CacheStorage_read_binary(self): + self.assert_string_in_response('books.xls', '1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.MediaStorage') + def test_import_action_handles_MediaStorage_read(self): + self.assert_string_in_response('books.csv', '0') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.MediaStorage') + def test_import_action_handles_MediaStorage_read_mac(self): + self.assert_string_in_response('books-mac.csv', '0') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.MediaStorage') + def test_import_action_handles_MediaStorage_read_iso_8859_1(self): + self.assert_string_in_response('books-ISO-8859-1.csv', '0', 'ISO-8859-1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.MediaStorage') + def test_import_action_handles_MediaStorage_read_binary(self): + self.assert_string_in_response('books.xls', '1') + + +class CompleteImportEncodingTest(TestCase): + """Test handling 'complete import' step using different file encodings + and storage types. + """ + + def setUp(self): + user = User.objects.create_user('admin', 'admin@example.com', + 'password') + user.is_staff = True + user.is_superuser = True + user.save() + self.client.login(username='admin', password='password') + + def assert_string_in_response(self, filename, input_format, encoding=None): + input_format = input_format + filename = os.path.join( + os.path.dirname(__file__), + os.path.pardir, + 'exports', + filename) + with open(filename, "rb") as f: + data = { + 'input_format': input_format, + 'import_file': f, + } + if encoding: + BookAdmin.from_encoding = encoding + response = self.client.post('/admin/core/book/import/', data) + + confirm_form = response.context['confirm_form'] + data = confirm_form.initial + response = self.client.post('/admin/core/book/process_import/', data, follow=True) + + self.assertEqual(response.status_code, 200) + self.assertContains(response, 'Import finished, with 1 new and 0 updated books.') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.TempFolderStorage') + def test_import_action_handles_TempFolderStorage_read(self): + self.assert_string_in_response('books.csv', '0') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.TempFolderStorage') + def test_import_action_handles_TempFolderStorage_read_mac(self): + self.assert_string_in_response('books-mac.csv', '0') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.TempFolderStorage') + def test_import_action_handles_TempFolderStorage_read_iso_8859_1(self): + self.assert_string_in_response('books-ISO-8859-1.csv', '0', 'ISO-8859-1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.TempFolderStorage') + def test_import_action_handles_TempFolderStorage_read_binary(self): + self.assert_string_in_response('books.xls', '1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.CacheStorage') + def test_import_action_handles_CacheStorage_read(self): + self.assert_string_in_response('books.csv', '0') + + @skip("waiting for fix in tablib - see #1397") + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.CacheStorage') + def test_import_action_handles_CacheStorage_read_mac(self): + self.assert_string_in_response('books-mac.csv', '0') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.CacheStorage') + def test_import_action_handles_CacheStorage_read_iso_8859_1(self): + self.assert_string_in_response('books-ISO-8859-1.csv', '0', 'ISO-8859-1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.CacheStorage') + def test_import_action_handles_CacheStorage_read_binary(self): + self.assert_string_in_response('books.xls', '1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.MediaStorage') + def test_import_action_handles_MediaStorage_read(self): + self.assert_string_in_response('books.csv', '0') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.MediaStorage') + def test_import_action_handles_MediaStorage_read_mac(self): + self.assert_string_in_response('books-mac.csv', '0') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.MediaStorage') + def test_import_action_handles_MediaStorage_read_iso_8859_1(self): + self.assert_string_in_response('books-ISO-8859-1.csv', '0', 'ISO-8859-1') + + @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.MediaStorage') + def test_import_action_handles_MediaStorage_read_binary(self): + self.assert_string_in_response('books.xls', '1') + + class TestImportExportActionModelAdmin(ImportExportActionModelAdmin): def __init__(self, mock_model, mock_site, error_instance): self.error_instance = error_instance @@ -527,37 +820,6 @@ def write_to_tmp_storage(self, import_file, input_format): return mock_storage -class ImportActionDecodeErrorTest(TestCase): - mock_model = mock.Mock(spec=Book) - mock_model.__name__ = "mockModel" - mock_model._meta = mock.Mock(fields=[], many_to_many=[]) - mock_site = mock.MagicMock() - mock_request = MagicMock(spec=HttpRequest) - mock_request.POST = {'a': 1} - mock_request.FILES = {} - - @mock.patch("import_export.admin.ImportForm") - def test_import_action_handles_UnicodeDecodeError(self, mock_form): - mock_form.is_valid.return_value = True - b_arr = b'\x00\x00' - m = TestImportExportActionModelAdmin(self.mock_model, self.mock_site, - UnicodeDecodeError('codec', b_arr, 1, 2, 'fail!')) - res = m.import_action(self.mock_request) - self.assertEqual( - "

Imported file has a wrong encoding: \'codec\' codec can\'t decode byte 0x00 in position 1: fail!

", - res.content.decode()) - - @mock.patch("import_export.admin.ImportForm") - def test_import_action_handles_error(self, mock_form): - mock_form.is_valid.return_value = True - m = TestImportExportActionModelAdmin(self.mock_model, self.mock_site, - ValueError("fail")) - res = m.import_action(self.mock_request) - self.assertRegex( - res.content.decode(), - r"

ValueError encountered while trying to read file: .*

") - - class ExportActionAdminIntegrationTest(TestCase): def setUp(self): diff --git a/tests/core/tests/test_tmp_storages.py b/tests/core/tests/test_tmp_storages.py index cf4a0dce8..6a1e0b951 100644 --- a/tests/core/tests/test_tmp_storages.py +++ b/tests/core/tests/test_tmp_storages.py @@ -1,4 +1,5 @@ import os +from unittest.mock import mock_open, patch from django.core.cache import cache from django.core.files.storage import default_storage @@ -30,6 +31,15 @@ def test_remove(self): self.storage.remove() +class TestTempFolderStorage(TempFolderStorage): + def get_full_path(self): + return '/tmp/f' + + +class TestMediaStorage(MediaStorage): + def get_full_path(self): + return 'f' + class TempStoragesTest(TestCase): @@ -52,6 +62,13 @@ def test_temp_folder_storage(self): tmp_storage.remove() self.assertFalse(os.path.isfile(tmp_storage.get_full_path())) + def test_temp_folder_storage_read_with_encoding(self): + tmp_storage = TestTempFolderStorage(encoding='utf-8') + tmp_storage.name = 'f' + with patch("builtins.open", mock_open(read_data="data")) as mock_file: + tmp_storage.read() + mock_file.assert_called_with("/tmp/f", 'r', encoding='utf-8') + def test_cache_storage(self): tmp_storage = CacheStorage() tmp_storage.save(self.test_string) @@ -65,6 +82,20 @@ def test_cache_storage(self): tmp_storage.remove() self.assertEqual(cache.get(tmp_storage.name), None) + def test_cache_storage_read_with_encoding(self): + tmp_storage = CacheStorage() + tmp_storage.name = 'f' + cache.set("django-import-export-f", 101) + res = tmp_storage.read() + self.assertEqual(101, res) + + def test_cache_storage_read_with_encoding_unicode_chars(self): + tmp_storage = CacheStorage() + tmp_storage.name = 'f' + tmp_storage.save("àèìòùçñ") + res = tmp_storage.read() + self.assertEqual("àèìòùçñ", res) + def test_media_storage(self): tmp_storage = MediaStorage() tmp_storage.save(self.test_string) @@ -79,12 +110,20 @@ def test_media_storage(self): def test_media_storage_read_mode(self): # issue 416 - MediaStorage does not respect the read_mode parameter. + # tests that platform specific line endings are converted when reading in text mode + # see https://docs.python.org/3/tutorial/inputoutput.html#reading-and-writing-files test_string = self.test_string.replace(b'\n', b'\r') tmp_storage = MediaStorage() tmp_storage.save(test_string) name = tmp_storage.name - tmp_storage = MediaStorage(name=name) - self.assertEqual(self.test_string.decode(), - tmp_storage.read(read_mode='r')) + tmp_storage = MediaStorage(name=name, read_mode='r') + self.assertEqual(self.test_string.decode(), tmp_storage.read()) + + def test_media_storage_read_with_encoding(self): + tmp_storage = TestMediaStorage() + tmp_storage.name = 'f' + with patch("import_export.tmp_storages.default_storage.open") as mock_open: + tmp_storage.read() + mock_open.assert_called_with("f", mode='rb') \ No newline at end of file From db4690d610b81fbac06e5c12fd7721ab51339b64 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 7 Apr 2022 11:16:47 +0100 Subject: [PATCH 28/77] fixed runtests.sh --- runtests.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/runtests.sh b/runtests.sh index 889da9e8b..909ce658b 100755 --- a/runtests.sh +++ b/runtests.sh @@ -4,9 +4,6 @@ # postgres / mysql run via docker # sqlite (default) runs against local database file (database.db) -echo "starting local database instances" -docker-compose -f tests/docker-compose.yml up -d - export DJANGO_SETTINGS_MODULE=settings export IMPORT_EXPORT_POSTGRESQL_USER=pguser @@ -15,6 +12,9 @@ export IMPORT_EXPORT_POSTGRESQL_PASSWORD=pguserpass export IMPORT_EXPORT_MYSQL_USER=mysqluser export IMPORT_EXPORT_MYSQL_PASSWORD=mysqluserpass +echo "starting local database instances" +docker-compose -f tests/docker-compose.yml up -d + echo "running tests (sqlite)" tox @@ -27,4 +27,4 @@ export IMPORT_EXPORT_TEST_TYPE=postgres tox echo "removing local database instances" -docker-compose -f tests/docker-compose.yml down -v \ No newline at end of file +docker-compose -f tests/docker-compose.yml down -v From d940deecb54cfd52303f85125b10d055743beec5 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 7 Apr 2022 11:33:52 +0100 Subject: [PATCH 29/77] added notes on releasing --- RELEASE.md | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 RELEASE.md diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 000000000..7f148674f --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,45 @@ +## Release process + +#### Pre release + +- Set up `.pypirc` file to reference both pypi and testpypi. + +#### Release + +- Ensure that all code has been committed and integration tests have run on Github. +- `make messages` is intended to be run now to keep the translation files up-to-date. + - Run this if there have been any translations updates for the release. + - This creates updates to all translation files so there is no need to commit these unless there have been any translation changes. + +```bash +# checked out clean version +git clone git@github.com:django-import-export/django-import-export.git django-import-export-rel + +cd django-import-export-rel + +python3 -m venv relenv +source relenv/bin/activate + +pip install -r requirements/deploy.txt + +# zest.releaser pre-release +prerelease +``` + +#### Perform the release + +For the first pass you may choose not to upload only to testpypi (not pypi) so that you can check the release. You can check the release by manually downloading the files from testPyPI and checking the contents. + +Once the test file have been checked, run again to upload to PyPI. + +```bash +release + +# resets the version and pushes changes to origin +postrelease + +# remove the rel copy - no longer required +deactivate +cd .. +rm -rf django-import-export-rel +``` \ No newline at end of file From 2d87bca3e481a6f8645236ceee04f6a0f95fb5eb Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 7 Apr 2022 18:57:03 +0100 Subject: [PATCH 30/77] updated release doc --- RELEASE.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/RELEASE.md b/RELEASE.md index 7f148674f..3b3e8b24b 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -17,6 +17,9 @@ git clone git@github.com:django-import-export/django-import-export.git django-im cd django-import-export-rel +# checkout any feature branch at this point +# git checkout develop + python3 -m venv relenv source relenv/bin/activate From af21b3d4994bf4186d1d0d995da0afc1fd1616c0 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Fri, 8 Apr 2022 09:08:51 +0100 Subject: [PATCH 31/77] updated release doc --- RELEASE.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 3b3e8b24b..fc4376fe2 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -20,8 +20,9 @@ cd django-import-export-rel # checkout any feature branch at this point # git checkout develop -python3 -m venv relenv -source relenv/bin/activate +python3 -m venv venv +source venv/bin/activate +pip install -U pip setuptools wheel pip install -r requirements/deploy.txt From d3d43ecbafdcd375baeb9019b13ecd4c69e7f08d Mon Sep 17 00:00:00 2001 From: Matt Hegarty Date: Sat, 9 Apr 2022 19:44:33 +0100 Subject: [PATCH 32/77] Fix failure when CacheStorage enabled and file contains LF char (#1417) --- docs/changelog.rst | 1 + requirements/base.txt | 2 +- setup.py | 2 +- tests/core/tests/test_admin_integration.py | 4 +--- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index d2facb01f..36eb15c54 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -43,6 +43,7 @@ Enhancements - Added option to store raw row values in each row's `RowResult` (#1393) - Add natural key support to `ForeignKeyWidget` (#1371) - Optimised default instantiation of `CharWidget` (#1414) +- Fixed handling of LF character when using `CacheStorage` (#1417) Development ########### diff --git a/requirements/base.txt b/requirements/base.txt index 2180c1370..aac13ddc9 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -1,3 +1,3 @@ Django>=3.2 -tablib[html,ods,xls,xlsx,yaml]>=3.0.0 +tablib[html,ods,xls,xlsx,yaml]>=3.2.1 diff-match-patch diff --git a/setup.py b/setup.py index 8514fec9f..722311a3f 100644 --- a/setup.py +++ b/setup.py @@ -24,7 +24,7 @@ install_requires = [ 'diff-match-patch', 'Django>=3.2', - 'tablib[html,ods,xls,xlsx,yaml]>=3.0.0', + 'tablib[html,ods,xls,xlsx,yaml]>=3.2.1', ] diff --git a/tests/core/tests/test_admin_integration.py b/tests/core/tests/test_admin_integration.py index 9e6008dfc..86b82295d 100644 --- a/tests/core/tests/test_admin_integration.py +++ b/tests/core/tests/test_admin_integration.py @@ -1,7 +1,7 @@ import os.path import warnings from datetime import datetime -from unittest import mock, skip +from unittest import mock from unittest.mock import MagicMock import chardet @@ -692,7 +692,6 @@ def test_import_action_handles_TempFolderStorage_read_binary(self): def test_import_action_handles_CacheStorage_read(self): self.assert_string_in_response('books.csv', '0') - @skip("waiting for fix in tablib - see #1397") @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.CacheStorage') def test_import_action_handles_CacheStorage_read_mac(self): self.assert_string_in_response('books-mac.csv', '0') @@ -778,7 +777,6 @@ def test_import_action_handles_TempFolderStorage_read_binary(self): def test_import_action_handles_CacheStorage_read(self): self.assert_string_in_response('books.csv', '0') - @skip("waiting for fix in tablib - see #1397") @override_settings(IMPORT_EXPORT_TMP_STORAGE_CLASS='import_export.tmp_storages.CacheStorage') def test_import_action_handles_CacheStorage_read_mac(self): self.assert_string_in_response('books-mac.csv', '0') From 72c4d000d114b45a89e77a06723debeaa283b9b0 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Mon, 11 Apr 2022 09:28:37 +0100 Subject: [PATCH 33/77] updated changelog --- docs/changelog.rst | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 36eb15c54..e92f73d5c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,7 +1,12 @@ Changelog ========= -3.0.0 (unreleased) +3.0.0-beta.1 (2022-04-09) +------------------ + +- Fixed handling of LF character when using `CacheStorage` (#1417) + +3.0.0-beta (2022-04-09) ------------------ Breaking changes @@ -43,7 +48,6 @@ Enhancements - Added option to store raw row values in each row's `RowResult` (#1393) - Add natural key support to `ForeignKeyWidget` (#1371) - Optimised default instantiation of `CharWidget` (#1414) -- Fixed handling of LF character when using `CacheStorage` (#1417) Development ########### From f5636bd23c46bf25c53152b5ac882733b2078374 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Mon, 11 Apr 2022 09:29:22 +0100 Subject: [PATCH 34/77] updated changelog --- docs/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index e92f73d5c..c8aecc470 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,7 +1,7 @@ Changelog ========= -3.0.0-beta.1 (2022-04-09) +3.0.0-beta.1 (2022-04-11) ------------------ - Fixed handling of LF character when using `CacheStorage` (#1417) From cd56c82c350bc0741328c59c318843f90e121836 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Tue, 12 Apr 2022 10:11:36 +0100 Subject: [PATCH 35/77] fix broken image ref in `README.rst` --- README.rst | 2 +- docs/changelog.rst | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/README.rst b/README.rst index ed1977334..5796dff80 100644 --- a/README.rst +++ b/README.rst @@ -39,7 +39,7 @@ Features: * export data respecting admin filters -.. image:: docs/_static/images/django-import-export-change.png +.. image:: https://raw.githubusercontent.com/django-import-export/django-import-export/main/docs/_static/images/django-import-export-change.png * Documentation: https://django-import-export.readthedocs.io/en/stable/ diff --git a/docs/changelog.rst b/docs/changelog.rst index c8aecc470..261642928 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,13 +1,19 @@ Changelog ========= +3.0.0-beta.2 (unreleased) +------------------------- + +- fix broken link to tablib formats page (#1418) +- fix broken image ref in `README.rst` + 3.0.0-beta.1 (2022-04-11) ------------------- +------------------------- - Fixed handling of LF character when using `CacheStorage` (#1417) 3.0.0-beta (2022-04-09) ------------------- +----------------------- Breaking changes ################ From c766284140c79d7fb2b3661860f52f3ca8a43b3f Mon Sep 17 00:00:00 2001 From: Hrafn Malmquist Date: Wed, 13 Apr 2022 10:35:51 +0100 Subject: [PATCH 36/77] Fix broken link to tablib formats page (#1418) --- docs/installation.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/installation.rst b/docs/installation.rst index 5901bdb14..8c6467e0f 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -12,7 +12,7 @@ additional formats like ``cli`` or ``Pandas DataFrame``, you should install the appropriate tablib dependencies (e.g. ``pip install tablib[pandas]``). Read more on the `tablib format documentation page`_. -.. _tablib format documentation page: https://tablib.readthedocs.io/en/stable/formats/ +.. _tablib format documentation page: https://tablib.readthedocs.io/en/stable/formats.html Alternatively, you can install the git repository directly to obtain the development version:: From 19a2dcd56f1220ef282d24544e78dc9f0bed70db Mon Sep 17 00:00:00 2001 From: Ryan <62414746+pokken-magic@users.noreply.github.com> Date: Sat, 16 Apr 2022 16:59:08 -0400 Subject: [PATCH 37/77] Auto detect natural foreign key setting for widgets at the resource level (#1423) --- docs/getting_started.rst | 21 ++++++-- import_export/resources.py | 23 ++++++++- tests/core/tests/test_resources.py | 81 ++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 6 deletions(-) diff --git a/docs/getting_started.rst b/docs/getting_started.rst index 51c7867fa..cf59ebc1c 100644 --- a/docs/getting_started.rst +++ b/docs/getting_started.rst @@ -238,10 +238,10 @@ and exporting resource. Django Natural Keys =================== -The ForeignKeyWidget also supports using Django's natural key functions. A -manager class with the get_by_natural_key function is require for importing +The ``ForeignKeyWidget`` also supports using Django's natural key functions. A +manager class with the ``get_by_natural_key`` function is required for importing foreign key relationships by the field model's natural key, and the model must -have a natural_key function that can be serialized as a JSON list in order to +have a ``natural_key`` function that can be serialized as a JSON list in order to export data. The primary utility for natural key functionality is to enable exporting data @@ -249,9 +249,12 @@ that can be imported into other Django environments with different numerical primary key sequences. The natural key functionality enables handling more complex data than specifying either a single field or the PK. -The example below illustrates how to create a field on the BookResource that +The example below illustrates how to create a field on the ``BookResource`` that imports and exports its author relationships using the natural key functions -on the Author model and modelmanager. +on the ``Author`` model and modelmanager. + +The resource _meta option ``use_natural_foreign_keys`` enables this setting +for all Models that support it. :: @@ -273,6 +276,7 @@ on the Author model and modelmanager. def natural_key(self): return (self.name,) + # Only the author field uses natural foreign keys. class BookResource(resources.ModelResource): author = Field( @@ -283,6 +287,13 @@ on the Author model and modelmanager. class Meta: model = Book + + # All widgets with foreign key functions use them. + class BookResource(resources.ModelResource): + + class Meta: + model = Book + use_natural_foreign_keys = True Read more at `Django Serialization `_ diff --git a/import_export/resources.py b/import_export/resources.py index 87517a444..b6a1db4a8 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -41,6 +41,11 @@ def get_related_model(field): if hasattr(field, 'related_model'): return field.related_model +def has_natural_foreign_key(model): + """ + Determine if a model has natural foreign key functions + """ + return (hasattr(model, "natural_key") and hasattr(model.objects, "get_by_natural_key")) class ResourceOptions: """ @@ -183,6 +188,14 @@ class ResourceOptions: which should be considered when importing large datasets. """ + use_natural_foreign_keys = False + """ + If True, use_natural_foreign_keys = True will be passed to all foreign + key widget fields whose models support natural foreign keys. That is, + the model has a natural_key function and the manager has a + get_by_natural_key function. + """ + class DeclarativeMetaclass(type): @@ -1082,9 +1095,17 @@ def get_fk_widget(cls, field): """ Prepare widget for fk and o2o fields """ + + model = get_related_model(field) + + use_natural_foreign_keys = ( + has_natural_foreign_key(model) and cls._meta.use_natural_foreign_keys + ) + return functools.partial( widgets.ForeignKeyWidget, - model=get_related_model(field)) + model=model, + use_natural_foreign_keys=use_natural_foreign_keys) @classmethod def widget_from_django_field(cls, f, default=widgets.Widget): diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 7610e1e71..6fb4119d6 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -1181,6 +1181,65 @@ class Meta: BookResource().import_data(self.dataset) + def test_natural_foreign_key_detection(self): + """ + Test that when the _meta option for use_natural_foreign_keys + is set on a resource that foreign key widgets are created + with that flag, and when it's off they are not. + """ + + # For future proof testing, we have one resource with natural + # foreign keys on, and one off. If the default ever changes + # this should still work. + class _BookResource_Unfk(resources.ModelResource): + class Meta: + use_natural_foreign_keys = True + model = Book + + class _BookResource(resources.ModelResource): + + class Meta: + use_natural_foreign_keys = False + model = Book + + resource_with_nfks = _BookResource_Unfk() + author_field_widget = resource_with_nfks.fields["author"].widget + self.assertTrue(author_field_widget.use_natural_foreign_keys) + + resource_without_nfks = _BookResource() + author_field_widget = resource_without_nfks.fields["author"].widget + self.assertFalse(author_field_widget.use_natural_foreign_keys) + + def test_natural_foreign_key_false_positives(self): + """ + Ensure that if the field's model does not have natural foreign + key functions, it is not set to use natural foreign keys. + """ + from django.db import models + class RelatedModel(models.Model): + name = models.CharField() + class Meta: + app_label = "Test" + + class TestModel(models.Model): + related_field = models.ForeignKey(RelatedModel, on_delete=models.PROTECT) + + class Meta: + app_label = "Test" + + class TestModelResource(resources.ModelResource): + class Meta: + model = TestModel + fields = ( + 'id', + 'related_field' + ) + use_natural_foreign_keys = True + + resource = TestModelResource() + related_field_widget = resource.fields["related_field"].widget + self.assertFalse(related_field_widget.use_natural_foreign_keys) + class ModelResourceTransactionTest(TransactionTestCase): @skipUnlessDBFeature('supports_transactions') def test_m2m_import_with_transactions(self): @@ -2269,3 +2328,25 @@ def test_import_data(self): self.assertEqual(result.rows[0].row_values.get('name'), 'Some book') self.assertEqual(result.rows[0].row_values.get('author_email'), 'test@example.com') self.assertEqual(result.rows[0].row_values.get('price'), '10.25') + +class ResourcesHelperFunctionsTest(TestCase): + """ + Test the helper functions in resources. + """ + + def test_has_natural_foreign_key(self): + """ + Ensure that resources.has_natural_foreign_key detects correctly + whether a model has a natural foreign key + """ + cases = { + Book: True, + Author: True, + Category: False + } + + for model, expected_result in cases.items(): + self.assertEqual( + resources.has_natural_foreign_key(model), + expected_result + ) From b16037dfac19dded82e656c543c9e10392e7d73a Mon Sep 17 00:00:00 2001 From: Andrey Novikov Date: Sun, 17 Apr 2022 21:47:13 +0300 Subject: [PATCH 38/77] Include custom form media in templates (#1038) --- docs/changelog.rst | 5 +++-- import_export/templates/admin/import_export/export.html | 5 +++++ import_export/templates/admin/import_export/import.html | 9 +++++++++ tests/core/admin.py | 5 ++++- tests/core/forms.py | 7 ++++++- 5 files changed, 27 insertions(+), 4 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 261642928..f6d6a3a2d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,8 +4,9 @@ Changelog 3.0.0-beta.2 (unreleased) ------------------------- -- fix broken link to tablib formats page (#1418) -- fix broken image ref in `README.rst` +- Fix broken link to tablib formats page (#1418) +- Fix broken image ref in `README.rst` +- Include custom form media in templates (#1038) 3.0.0-beta.1 (2022-04-11) ------------------------- diff --git a/import_export/templates/admin/import_export/export.html b/import_export/templates/admin/import_export/export.html index 720efda74..7b144f67e 100644 --- a/import_export/templates/admin/import_export/export.html +++ b/import_export/templates/admin/import_export/export.html @@ -3,6 +3,11 @@ {% load admin_urls %} {% load import_export_tags %} +{% block extrahead %}{{ block.super }} + +{{ form.media }} +{% endblock %} + {% block breadcrumbs_last %} {% trans "Export" %} {% endblock %} diff --git a/import_export/templates/admin/import_export/import.html b/import_export/templates/admin/import_export/import.html index 85ebf12f2..b61ffa7bc 100644 --- a/import_export/templates/admin/import_export/import.html +++ b/import_export/templates/admin/import_export/import.html @@ -6,6 +6,15 @@ {% block extrastyle %}{{ block.super }}{% endblock %} +{% block extrahead %}{{ block.super }} + + {% if confirm_form %} + {{ confirm_form.media }} + {% else %} + {{ form.media }} + {% endif %} +{% endblock %} + {% block breadcrumbs_last %} {% trans "Import" %} {% endblock %} diff --git a/tests/core/admin.py b/tests/core/admin.py index 257afdab7..ba30fba6f 100644 --- a/tests/core/admin.py +++ b/tests/core/admin.py @@ -3,7 +3,7 @@ from import_export.admin import ExportActionModelAdmin, ImportExportMixin, ImportMixin from import_export.resources import ModelResource -from .forms import CustomConfirmImportForm, CustomImportForm +from .forms import CustomConfirmImportForm, CustomImportForm, CustomExportForm from .models import Author, Book, Category, Child, EBook @@ -51,6 +51,9 @@ def get_import_form(self): def get_confirm_import_form(self): return CustomConfirmImportForm + def get_export_form(self): + return CustomExportForm + def get_form_kwargs(self, form, *args, **kwargs): # update kwargs with authors (from CustomImportForm.cleaned_data) if isinstance(form, CustomImportForm): diff --git a/tests/core/forms.py b/tests/core/forms.py index 253d8b9c5..08f9ffdb4 100644 --- a/tests/core/forms.py +++ b/tests/core/forms.py @@ -1,6 +1,6 @@ from django import forms -from import_export.forms import ConfirmImportForm, ImportForm +from import_export.forms import ConfirmImportForm, ImportForm, ExportForm from .models import Author @@ -18,3 +18,8 @@ class CustomImportForm(AuthorFormMixin, ImportForm): class CustomConfirmImportForm(AuthorFormMixin, ConfirmImportForm): """Customized ConfirmImportForm, with author field required""" pass + + +class CustomExportForm(AuthorFormMixin, ExportForm): + """Customized ExportForm, with author field required""" + pass From 40f1445f18ed6c0a63860ab3fb6c778715bbaa15 Mon Sep 17 00:00:00 2001 From: Matt Hegarty Date: Thu, 21 Apr 2022 13:38:21 +0100 Subject: [PATCH 39/77] Remove unnecessary files generated when running tox locally (#1426) --- .github/workflows/django-import-export-ci.yml | 1 + docs/changelog.rst | 1 + runtests.py | 22 +++++++++++++++++++ tests/core/tests/test_admin_integration.py | 2 -- tox.ini | 3 ++- 5 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 runtests.py diff --git a/.github/workflows/django-import-export-ci.yml b/.github/workflows/django-import-export-ci.yml index ddc5cb0bf..de09b603d 100644 --- a/.github/workflows/django-import-export-ci.yml +++ b/.github/workflows/django-import-export-ci.yml @@ -19,6 +19,7 @@ jobs: IMPORT_EXPORT_POSTGRESQL_PASSWORD: somepass IMPORT_EXPORT_MYSQL_USER: root IMPORT_EXPORT_MYSQL_PASSWORD: root + COVERAGE: 1 strategy: matrix: python-version: diff --git a/docs/changelog.rst b/docs/changelog.rst index f6d6a3a2d..2ab716fe8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -7,6 +7,7 @@ Changelog - Fix broken link to tablib formats page (#1418) - Fix broken image ref in `README.rst` - Include custom form media in templates (#1038) +- Remove unnecessary files generated when running tox locally (#1426) 3.0.0-beta.1 (2022-04-11) ------------------------- diff --git a/runtests.py b/runtests.py new file mode 100644 index 000000000..e8be86168 --- /dev/null +++ b/runtests.py @@ -0,0 +1,22 @@ +""" +Helper script to generate coverage data only if running in CI. +This script is called from tox (see tox.ini). +Coverage files are generated only if a `COVERAGE` environment variable is present. +This is necessary to prevent unwanted coverage files when running locally (issue #1424) +""" +import os + + +def main(): + coverage_args = "-m coverage run" if os.environ.get("COVERAGE") else "" + + os.system( + "python -W error::DeprecationWarning -W error::PendingDeprecationWarning " + f"{coverage_args} " + "./tests/manage.py test core --settings=settings" + ) + + +if __name__ == "__main__": + main() + diff --git a/tests/core/tests/test_admin_integration.py b/tests/core/tests/test_admin_integration.py index 86b82295d..03e4a9498 100644 --- a/tests/core/tests/test_admin_integration.py +++ b/tests/core/tests/test_admin_integration.py @@ -119,8 +119,6 @@ def test_import_second_resource(self): response = self.client.post('/admin/core/book/import/', data) self.assertEqual(response.status_code, 200) self.assertIn('result', response.context) - with open("response.html", "wb") as f: - f.write(response.content) self.assertFalse(response.context['result'].has_errors()) self.assertIn('confirm_form', response.context) confirm_form = response.context['confirm_form'] diff --git a/tox.ini b/tox.ini index c8ba79f31..348b9ebb2 100644 --- a/tox.ini +++ b/tox.ini @@ -14,7 +14,7 @@ python = [testenv] setenv = PYTHONPATH = {toxinidir}/tests -commands = python -W error::DeprecationWarning -W error::PendingDeprecationWarning -m coverage run {toxinidir}/tests/manage.py test core +commands = python ./runtests.py deps = tablibdev: -egit+https://github.com/jazzband/tablib.git#egg=tablib django32: Django>=3.2,<4.0 @@ -25,6 +25,7 @@ deps = # if postgres / mysql environment variables exist, we can go ahead and run db specific tests passenv = + COVERAGE IMPORT_EXPORT_POSTGRESQL_USER IMPORT_EXPORT_POSTGRESQL_PASSWORD IMPORT_EXPORT_MYSQL_USER From 72321e43bf4728a5892ef62c751241bfb57a33c0 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 21 Apr 2022 15:17:29 +0100 Subject: [PATCH 40/77] version bump --- docs/changelog.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 2ab716fe8..949ba2cf3 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,9 @@ Changelog 3.0.0-beta.2 (unreleased) ------------------------- +3.0.0-beta.2 (2022-04-21) +------------------------- + - Fix broken link to tablib formats page (#1418) - Fix broken image ref in `README.rst` - Include custom form media in templates (#1038) From cdf745f5e4f5d23f1dd42163a89b02be93ffe831 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 21 Apr 2022 15:17:47 +0100 Subject: [PATCH 41/77] updated changelog --- docs/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 949ba2cf3..db55aab6c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,7 +1,7 @@ Changelog ========= -3.0.0-beta.2 (unreleased) +3.0.0-beta.3 (unreleased) ------------------------- 3.0.0-beta.2 (2022-04-21) From 12321dc8af5f0b90fe1f96b7336a5827d1c60dc1 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 21 Apr 2022 15:19:43 +0100 Subject: [PATCH 42/77] fixed changelog formatting --- docs/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index db55aab6c..b4bfba5c0 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -25,7 +25,7 @@ Breaking changes This release makes some minor changes to the public API. If you have overridden any methods from the `resources` or `widgets` modules, you may need to update your implementation to accommodate these changes. -- Check value of `ManyToManyField` in skip_row() (#1271) +- Check value of `ManyToManyField` in `skip_row()` (#1271) - This fixes an issue where ManyToMany fields are not checked correctly in `skip_row()`. This means that `skip_row()` now takes `row` as a mandatory arg. If you have overridden `skip_row()` in your own implementation, you will need to add `row` as an arg. - Bug fix: validation errors were being ignored when `skip_unchanged` is set (#1378) From 8ed35833792175c1f1d0de810e56aa436453c7e5 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Sat, 23 Apr 2022 21:06:23 +0100 Subject: [PATCH 43/77] added isort to CI --- .github/workflows/django-import-export-ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/django-import-export-ci.yml b/.github/workflows/django-import-export-ci.yml index de09b603d..8b17870d3 100644 --- a/.github/workflows/django-import-export-ci.yml +++ b/.github/workflows/django-import-export-ci.yml @@ -55,6 +55,10 @@ jobs: uses: actions/setup-python@v2 with: python-version: ${{ matrix.python-version }} + - name: isort + uses: jamescurtin/isort-action@master + with: + requirementsFiles: "requirements.txt" - uses: actions/cache@v2 with: path: ~/.cache/pip From ce8ccbf33e14be6dae6b7b62c3c3c03a2647856a Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Sat, 23 Apr 2022 21:08:50 +0100 Subject: [PATCH 44/77] added test requirements --- .github/workflows/django-import-export-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/django-import-export-ci.yml b/.github/workflows/django-import-export-ci.yml index 8b17870d3..651a4af25 100644 --- a/.github/workflows/django-import-export-ci.yml +++ b/.github/workflows/django-import-export-ci.yml @@ -58,7 +58,7 @@ jobs: - name: isort uses: jamescurtin/isort-action@master with: - requirementsFiles: "requirements.txt" + requirementsFiles: "requirements.txt test.txt" - uses: actions/cache@v2 with: path: ~/.cache/pip From aaa5f5a748b1a72d868aaa3bf86bf0f5015c1ceb Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Sat, 23 Apr 2022 21:10:48 +0100 Subject: [PATCH 45/77] added CI build step --- .github/workflows/django-import-export-ci.yml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/workflows/django-import-export-ci.yml b/.github/workflows/django-import-export-ci.yml index 651a4af25..eb0dadc19 100644 --- a/.github/workflows/django-import-export-ci.yml +++ b/.github/workflows/django-import-export-ci.yml @@ -11,6 +11,16 @@ on: - release-3-x jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-python@v2 + with: + python-version: 3.9 + - uses: jamescurtin/isort-action@master + with: + requirementsFiles: "requirements.txt test.txt" test: runs-on: ubuntu-latest env: @@ -55,10 +65,6 @@ jobs: uses: actions/setup-python@v2 with: python-version: ${{ matrix.python-version }} - - name: isort - uses: jamescurtin/isort-action@master - with: - requirementsFiles: "requirements.txt test.txt" - uses: actions/cache@v2 with: path: ~/.cache/pip From 54e9b819e69877ae02932303e1e0d54f147c728c Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Sat, 23 Apr 2022 21:19:43 +0100 Subject: [PATCH 46/77] fixed requirements --- .github/workflows/django-import-export-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/django-import-export-ci.yml b/.github/workflows/django-import-export-ci.yml index eb0dadc19..548eb899b 100644 --- a/.github/workflows/django-import-export-ci.yml +++ b/.github/workflows/django-import-export-ci.yml @@ -20,7 +20,7 @@ jobs: python-version: 3.9 - uses: jamescurtin/isort-action@master with: - requirementsFiles: "requirements.txt test.txt" + requirementsFiles: "requirements/base.txt requirements/test.txt" test: runs-on: ubuntu-latest env: From ff873900e6d183b4e5e9d8d3302befdd135f428a Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Sat, 23 Apr 2022 21:25:27 +0100 Subject: [PATCH 47/77] added isort as step --- .github/workflows/django-import-export-ci.yml | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/.github/workflows/django-import-export-ci.yml b/.github/workflows/django-import-export-ci.yml index 548eb899b..2b29724d2 100644 --- a/.github/workflows/django-import-export-ci.yml +++ b/.github/workflows/django-import-export-ci.yml @@ -11,16 +11,6 @@ on: - release-3-x jobs: - build: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 - with: - python-version: 3.9 - - uses: jamescurtin/isort-action@master - with: - requirementsFiles: "requirements/base.txt requirements/test.txt" test: runs-on: ubuntu-latest env: @@ -52,6 +42,14 @@ jobs: --health-timeout 5s --health-retries 5 steps: + - name: isort + - uses: actions/checkout@v2 + - uses: actions/setup-python@v2 + with: + python-version: 3.9 + - uses: jamescurtin/isort-action@master + with: + requirementsFiles: "requirements/base.txt requirements/test.txt" - name: Set up MySQL run: > sudo /etc/init.d/mysql start From 8451a387483330f3c1fa24483297ffe8dc320b12 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Sat, 23 Apr 2022 21:27:29 +0100 Subject: [PATCH 48/77] fixed step --- .github/workflows/django-import-export-ci.yml | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/.github/workflows/django-import-export-ci.yml b/.github/workflows/django-import-export-ci.yml index 2b29724d2..d4ce087e7 100644 --- a/.github/workflows/django-import-export-ci.yml +++ b/.github/workflows/django-import-export-ci.yml @@ -42,14 +42,6 @@ jobs: --health-timeout 5s --health-retries 5 steps: - - name: isort - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 - with: - python-version: 3.9 - - uses: jamescurtin/isort-action@master - with: - requirementsFiles: "requirements/base.txt requirements/test.txt" - name: Set up MySQL run: > sudo /etc/init.d/mysql start @@ -63,6 +55,10 @@ jobs: uses: actions/setup-python@v2 with: python-version: ${{ matrix.python-version }} + - name: isort + uses: jamescurtin/isort-action@master + with: + requirementsFiles: "requirements/base.txt requirements/test.txt" - uses: actions/cache@v2 with: path: ~/.cache/pip From 7f3585cff4406781c44739e9afe33d1be852c34b Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Sat, 23 Apr 2022 21:28:51 +0100 Subject: [PATCH 49/77] added fail-fast --- .github/workflows/django-import-export-ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/django-import-export-ci.yml b/.github/workflows/django-import-export-ci.yml index d4ce087e7..1125ec639 100644 --- a/.github/workflows/django-import-export-ci.yml +++ b/.github/workflows/django-import-export-ci.yml @@ -21,6 +21,7 @@ jobs: IMPORT_EXPORT_MYSQL_PASSWORD: root COVERAGE: 1 strategy: + fail-fast: true matrix: python-version: - '3.7' From a183ccea48f9e97cd7b5571304117dc6e3d5a33d Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Sat, 23 Apr 2022 21:30:30 +0100 Subject: [PATCH 50/77] removed requirements files --- .github/workflows/django-import-export-ci.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/django-import-export-ci.yml b/.github/workflows/django-import-export-ci.yml index 1125ec639..10d0eda17 100644 --- a/.github/workflows/django-import-export-ci.yml +++ b/.github/workflows/django-import-export-ci.yml @@ -58,8 +58,6 @@ jobs: python-version: ${{ matrix.python-version }} - name: isort uses: jamescurtin/isort-action@master - with: - requirementsFiles: "requirements/base.txt requirements/test.txt" - uses: actions/cache@v2 with: path: ~/.cache/pip From 336bf5ce5959fd466b55a83b3eb5435c778e3a98 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Sat, 23 Apr 2022 21:32:05 +0100 Subject: [PATCH 51/77] fixed isort issues --- tests/core/admin.py | 2 +- tests/core/forms.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/core/admin.py b/tests/core/admin.py index ba30fba6f..81e758628 100644 --- a/tests/core/admin.py +++ b/tests/core/admin.py @@ -3,7 +3,7 @@ from import_export.admin import ExportActionModelAdmin, ImportExportMixin, ImportMixin from import_export.resources import ModelResource -from .forms import CustomConfirmImportForm, CustomImportForm, CustomExportForm +from .forms import CustomConfirmImportForm, CustomExportForm, CustomImportForm from .models import Author, Book, Category, Child, EBook diff --git a/tests/core/forms.py b/tests/core/forms.py index 08f9ffdb4..9936c9026 100644 --- a/tests/core/forms.py +++ b/tests/core/forms.py @@ -1,6 +1,6 @@ from django import forms -from import_export.forms import ConfirmImportForm, ImportForm, ExportForm +from import_export.forms import ConfirmImportForm, ExportForm, ImportForm from .models import Author From ad08cdfdb3cbc4b3403b3165499c0febbd653d68 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Mon, 25 Apr 2022 12:04:49 +0100 Subject: [PATCH 52/77] return non-zero if tests fail (#1429) --- runtests.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/runtests.py b/runtests.py index e8be86168..19f1dc641 100644 --- a/runtests.py +++ b/runtests.py @@ -10,11 +10,14 @@ def main(): coverage_args = "-m coverage run" if os.environ.get("COVERAGE") else "" - os.system( + retval = os.system( "python -W error::DeprecationWarning -W error::PendingDeprecationWarning " f"{coverage_args} " "./tests/manage.py test core --settings=settings" ) + if retval != 0: + exit(1) + exit(0) if __name__ == "__main__": From cd0ab61955998ab27840d2ee2b6898d9d892a695 Mon Sep 17 00:00:00 2001 From: Andy Babic Date: Mon, 2 May 2022 18:03:56 +0100 Subject: [PATCH 53/77] Refactor form-related methods on ImportMixin (#1147) * Improve form-related methods on ImportMixin * Update docs * De-privatise new form instantiation methods * fixed compatibility with release-3-x * corrected formatting * Update references to package versions * Improve deprecation warnings to mention EOL and to recommend the 'import_form_class' and 'confirm_form_class' attributes for using custom form classes * Be less explicit about when the deprecated functionality will be removed * Remove unnecessary new lines * Simplify new methods as much as possible * Move conditional logic to import_action() and process_import() * Restore and deprecate get_export_form() * Use 'confirm_form' and 'import_form' as variable names for forms to improve readability * added integration tests * fixed get_confirm_form_initial() method signature * simplified declaration of CustomBookAdmin * tidied CustomBookAdmin.get_confirm_form_initial() * corrected get_export_form() deprecation * Restore deprecation warning in get_export_form() * Do not interpret 'import form' data as data for the 'confirm form' * fixed get_export_form() deprecation test * fixed CustomBookAdmin.get_confirm_form_initial() * added tests to prove 'legacy' deprecated code works correctly * fixed isort issue * bugfix: legacy import never called in import_action() * updated deprecation message Co-authored-by: matthewhegarty --- docs/changelog.rst | 7 +- docs/getting_started.rst | 59 ++-- import_export/admin.py | 299 +++++++++++++++++---- import_export/utils.py | 10 + tests/core/admin.py | 40 ++- tests/core/models.py | 11 + tests/core/tests/test_admin_integration.py | 169 +++++++++++- tests/core/tests/test_mixins.py | 2 +- 8 files changed, 502 insertions(+), 95 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index b4bfba5c0..d46ac0fcc 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,8 @@ Changelog 3.0.0-beta.3 (unreleased) ------------------------- +- Refactored form-related methods on `ImportMixin` / `ExportMixin` (#1147) + 3.0.0-beta.2 (2022-04-21) ------------------------- @@ -44,7 +46,7 @@ This release makes some minor changes to the public API. If you have overridden Deprecations ############ -This release adds some deprecations which will be removed in the 3.1 release. +This release adds some deprecations which will be removed in a future release. - Add support for multiple resources in ModelAdmin. (#1223) - The `*Mixin.resource_class` accepting single resource has been deprecated and the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added. @@ -52,6 +54,9 @@ This release adds some deprecations which will be removed in the 3.1 release. - Deprecated `exceptions.py` (#1372) +- Refactored form-related methods on `ImportMixin` / `ExportMixin` (#1147) + - The following are deprecated: `get_import_form()`, `get_confirm_import_form()`, `get_form_kwargs()`, `get_export_form()` + Enhancements ############ diff --git a/docs/getting_started.rst b/docs/getting_started.rst index cf59ebc1c..59496a750 100644 --- a/docs/getting_started.rst +++ b/docs/getting_started.rst @@ -145,6 +145,12 @@ the import preview page:: report_skipped = False fields = ('id', 'name', 'price',) +To further customize the resources, you might like to consider overriding the following +:class:`~import_export.admin.ImportMixin` methods: +:meth:`~import_export.admin.ImportMixin.get_resource_kwargs`, +:meth:`~import_export.admin.ImportMixin.get_resource_class`, +:meth:`~import_export.admin.ImportMixin.get_export_resource_kwargs`, + .. seealso:: :doc:`/api_resources` @@ -242,16 +248,16 @@ The ``ForeignKeyWidget`` also supports using Django's natural key functions. A manager class with the ``get_by_natural_key`` function is required for importing foreign key relationships by the field model's natural key, and the model must have a ``natural_key`` function that can be serialized as a JSON list in order to -export data. +export data. The primary utility for natural key functionality is to enable exporting data that can be imported into other Django environments with different numerical primary key sequences. The natural key functionality enables handling more -complex data than specifying either a single field or the PK. +complex data than specifying either a single field or the PK. The example below illustrates how to create a field on the ``BookResource`` that imports and exports its author relationships using the natural key functions -on the ``Author`` model and modelmanager. +on the ``Author`` model and modelmanager. The resource _meta option ``use_natural_foreign_keys`` enables this setting for all Models that support it. @@ -287,7 +293,7 @@ for all Models that support it. class Meta: model = Book - + # All widgets with foreign key functions use them. class BookResource(resources.ModelResource): @@ -480,10 +486,10 @@ example, to add an additional field in the import form, subclass and extend the consider :class:`~import_export.forms.ConfirmImportForm` as importing is a two-step process). -To use the customized form(s), overload -:class:`~import_export.admin.ImportMixin` respective methods, i.e. -:meth:`~import_export.admin.ImportMixin.get_import_form`, and also -:meth:`~import_export.admin.ImportMixin.get_confirm_import_form` if need be. +To use your customized form(s), change the respective attributes on your +``ModelAdmin`` class: +* :attr:`~import_export.admin.ImportMixin.import_form_class` +* :attr:`~import_export.admin.ImportMixin.confirm_form_class`. For example, imagine you want to import books for a specific author. You can extend the import forms to include ``author`` field to select the author from. @@ -506,33 +512,26 @@ Customize ``ModelAdmin``:: class CustomBookAdmin(ImportMixin, admin.ModelAdmin): resource_classes = [BookResource] + import_form_class = CustomImportForm + confirm_form_class = CustomConfirmImportForm - def get_import_form(self): - return CustomImportForm - - def get_confirm_import_form(self): - return CustomConfirmImportForm - - def get_form_kwargs(self, form, *args, **kwargs): - # pass on `author` to the kwargs for the custom confirm form - if isinstance(form, CustomImportForm): - if form.is_valid(): - author = form.cleaned_data['author'] - kwargs.update({'author': author.id}) - return kwargs - + def get_confirm_form_initial(self, request, import_form): + initial = super().get_confirm_form_initial(request, import_form) + # Pass on the `author` value from the import form to + # the confirm form (if provided) + if import_form: + initial['author'] = import_form.cleaned_data['author'] + return initial admin.site.register(Book, CustomBookAdmin) -To further customize admin imports, consider modifying the following +To further customize the import forms, you might like to consider overriding the following :class:`~import_export.admin.ImportMixin` methods: -:meth:`~import_export.admin.ImportMixin.get_form_kwargs`, -:meth:`~import_export.admin.ImportMixin.get_import_resource_kwargs`, -:meth:`~import_export.admin.ImportMixin.get_import_data_kwargs`. - -Using the above methods it is possible to customize import form initialization -as well as importing customizations. - +:meth:`~import_export.admin.ImportMixin.get_import_form_class`, +:meth:`~import_export.admin.ImportMixin.get_import_form_kwargs`, +:meth:`~import_export.admin.ImportMixin.get_import_form_initial`, +:meth:`~import_export.admin.ImportMixin.get_confirm_form_class`, +:meth:`~import_export.admin.ImportMixin.get_confirm_form_kwargs`, .. warning:: diff --git a/import_export/admin.py b/import_export/admin.py index 8a8fdb9a7..67bb65f86 100644 --- a/import_export/admin.py +++ b/import_export/admin.py @@ -1,3 +1,5 @@ +import warnings + import django from django import forms from django.conf import settings @@ -14,11 +16,13 @@ from django.utils.translation import gettext_lazy as _ from django.views.decorators.http import require_POST +from .formats.base_formats import DEFAULT_FORMATS from .forms import ConfirmImportForm, ExportForm, ImportForm, export_action_form_factory from .mixins import BaseExportMixin, BaseImportMixin from .results import RowResult from .signals import post_export, post_import from .tmp_storages import MediaStorage, TempFolderStorage +from .utils import original class ImportExportMixinBase: @@ -39,6 +43,12 @@ class ImportMixin(BaseImportMixin, ImportExportMixinBase): change_list_template = 'admin/import_export/change_list_import.html' #: template for import view import_template_name = 'admin/import_export/import.html' + #: available import formats + formats = DEFAULT_FORMATS + #: form class to use for the initial import step + import_form_class = ImportForm + #: form class to use for the confirm import step + confirm_form_class = ConfirmImportForm #: import data encoding from_encoding = "utf-8-sig" skip_admin_log = None @@ -96,8 +106,12 @@ def process_import(self, request, *args, **kwargs): if not self.has_import_permission(request): raise PermissionDenied - form_type = self.get_confirm_import_form() - confirm_form = form_type(request.POST) + if getattr(self.get_confirm_import_form, 'is_original', False): + confirm_form = self.create_confirm_form(request) + else: + form_type = self.get_confirm_import_form() + confirm_form = form_type(request.POST) + if confirm_form.is_valid(): import_formats = self.get_import_formats() input_format = import_formats[ @@ -177,34 +191,191 @@ def get_import_context_data(self, **kwargs): def get_context_data(self, **kwargs): return {} + @original def get_import_form(self): """ - Get the form type used to read the import format and file. + .. deprecated:: 3.0 + Use :meth:`~import_export.admin.ImportMixin.get_import_form_class` or set the new + :attr:`~import_export.admin.ImportMixin.import_form_class` attribute. """ - return ImportForm + warnings.warn( + "ImportMixin.get_import_form() is deprecated and will be removed in " + "a future release. Please use get_import_form_class() instead.", + category=DeprecationWarning + ) + return self.import_form_class + @original def get_confirm_import_form(self): """ - Get the form type (class) used to confirm the import. + .. deprecated:: 3.0 + Use :func:`~import_export.admin.ImportMixin.get_confirm_form_class` or set the new + :attr:`~import_export.admin.ImportMixin.confirm_form_class` attribute. """ - return ConfirmImportForm + warnings.warn( + "ImportMixin.get_confirm_import_form() is deprecated and will be removed in " + "a future release. Please use get_confirm_form_class() instead.", + category=DeprecationWarning + ) + return self.confirm_form_class + @original def get_form_kwargs(self, form, *args, **kwargs): """ - Prepare/returns kwargs for the import form. + .. deprecated:: 3.0 + Use :meth:`~import_export.admin.ImportMixin.get_import_form_kwargs` or + :meth:`~import_export.admin.ImportMixin.get_confirm_form_kwargs` + instead, depending on which form you wish to customise. + """ + warnings.warn( + "ImportMixin.get_form_kwargs() is deprecated and will be removed " + "in a future release. Please use get_import_form_kwargs() or " + "get_confirm_form_kwargs() instead.", + category=DeprecationWarning + ) + return kwargs - To distinguish between import and confirm import forms, - the following approach may be used: + def create_import_form(self, request): + """ + .. versionadded:: 3.0 + + Return a form instance to use for the 'initial' import step. + This method can be extended to make dynamic form updates to the + form after it has been instantiated. You might also look to + override the following: - if isinstance(form, ImportForm): - # your code here for the import form kwargs - # e.g. update.kwargs({...}) - elif isinstance(form, ConfirmImportForm): - # your code here for the confirm import form kwargs - # e.g. update.kwargs({...}) - ... + * :meth:`~import_export.admin.ImportMixin.get_import_form_class` + * :meth:`~import_export.admin.ImportMixin.get_import_form_kwargs` + * :meth:`~import_export.admin.ImportMixin.get_import_form_initial` + * :meth:`~import_export.mixins.BaseImportMixin.get_import_resource_classes` """ - return kwargs + formats = self.get_import_formats() + form_class = self.get_import_form_class(request) + kwargs = self.get_import_form_kwargs(request) + return form_class(formats, self.get_import_resource_classes(), **kwargs) + + def get_import_form_class(self, request): + """ + .. versionadded:: 3.0 + + Return the form class to use for the 'import' step. If you only have + a single custom form class, you can set the ``import_form_class`` + attribute to change this for your subclass. + """ + # TODO: Remove following conditional when get_import_form() is removed + if not getattr(self.get_import_form, 'is_original', False): + warnings.warn( + "ImportMixin.get_import_form() is deprecated and will be " + "removed in a future release. Please use the new " + "'import_form_class' attribute to specify a custom form " + "class, or override the get_import_form_class() method if " + "your requirements are more complex.", + category=DeprecationWarning + ) + return self.get_import_form() + # Return the class attribute value + return self.import_form_class + + def get_import_form_kwargs(self, request): + """ + .. versionadded:: 3.0 + + Return a dictionary of values with which to initialize the 'import' + form (including the initial values returned by + :meth:`~import_export.admin.ImportMixin.get_import_form_initial`). + """ + return { + "data": request.POST or None, + "files": request.FILES or None, + "initial": self.get_import_form_initial(request), + } + + def get_import_form_initial(self, request): + """ + .. versionadded:: 3.0 + + Return a dictionary of initial field values to be provided to the + 'import' form. + """ + return {} + + def create_confirm_form(self, request, import_form=None): + """ + .. versionadded:: 3.0 + + Return a form instance to use for the 'confirm' import step. + This method can be extended to make dynamic form updates to the + form after it has been instantiated. You might also look to + override the following: + + * :meth:`~import_export.admin.ImportMixin.get_confirm_form_class` + * :meth:`~import_export.admin.ImportMixin.get_confirm_form_kwargs` + * :meth:`~import_export.admin.ImportMixin.get_confirm_form_initial` + """ + form_class = self.get_confirm_form_class(request) + kwargs = self.get_confirm_form_kwargs(request, import_form) + return form_class(**kwargs) + + def get_confirm_form_class(self, request): + """ + .. versionadded:: 3.0 + + Return the form class to use for the 'confirm' import step. If you only + have a single custom form class, you can set the ``confirm_form_class`` + attribute to change this for your subclass. + """ + # TODO: Remove following conditional when get_confirm_import_form() is removed + if not getattr(self.get_confirm_import_form, 'is_original', False): + warnings.warn( + "ImportMixin.get_confirm_import_form() is deprecated and will " + "be removed in a future release. Please use the new " + "'confirm_form_class' attribute to specify a custom form " + "class, or override the get_confirm_form_class() method if " + "your requirements are more complex.", + category=DeprecationWarning + ) + return self.get_confirm_import_form() + # Return the class attribute value + return self.confirm_form_class + + def get_confirm_form_kwargs(self, request, import_form=None): + """ + .. versionadded:: 3.0 + + Return a dictionary of values with which to initialize the 'confirm' + form (including the initial values returned by + :meth:`~import_export.admin.ImportMixin.get_confirm_form_initial`). + """ + if import_form: + # When initiated from `import_action()`, the 'posted' data + # is for the 'import' form, not this one. + data = None + files = None + else: + data = request.POST or None + files = request.FILES or None + + return { + "data": data, + "files": files, + "initial": self.get_confirm_form_initial(request, import_form), + } + + def get_confirm_form_initial(self, request, import_form): + """ + .. versionadded:: 3.0 + + Return a dictionary of initial field values to be provided to the + 'confirm' form. + """ + if import_form is None: + return {} + return { + 'import_file_name': import_form.cleaned_data["import_file"].tmp_storage_name, + 'original_file_name': import_form.cleaned_data["import_file"].name, + 'input_format': import_form.cleaned_data["input_format"], + 'resource': import_form.cleaned_data.get("resource", ""), + } def get_import_data_kwargs(self, request, *args, **kwargs): """ @@ -246,26 +417,32 @@ def import_action(self, request, *args, **kwargs): context = self.get_import_context_data() import_formats = self.get_import_formats() - form_type = self.get_import_form() - form_kwargs = self.get_form_kwargs(form_type, *args, **kwargs) - form = form_type(import_formats, - self.get_import_resource_classes(), - request.POST or None, - request.FILES or None, - **form_kwargs) + if getattr(self.get_form_kwargs, "is_original", False): + # Use new API + import_form = self.create_import_form(request) + else: + form_class = self.get_import_form_class(request) + form_kwargs = self.get_form_kwargs(form_class, *args, **kwargs) + import_form = form_class( + import_formats, + self.get_import_resource_classes(), + request.POST or None, + request.FILES or None, + **form_kwargs + ) resources = list() - if request.POST and form.is_valid(): - input_format = import_formats[ - int(form.cleaned_data['input_format']) - ]() + if request.POST and import_form.is_valid(): + input_format = import_formats[int(import_form.cleaned_data['input_format'])]() if not input_format.is_binary(): input_format.encoding = self.from_encoding - - import_file = form.cleaned_data['import_file'] + import_file = import_form.cleaned_data['import_file'] # first always write the uploaded file to disk as it may be a # memory file or else based on settings upload handlers tmp_storage = self.write_to_tmp_storage(import_file, input_format) + # allows get_confirm_form_initial() to include both the + # original and saved file names from form.cleaned_data + import_file.tmp_storage_name = tmp_storage.name try: # then read the file, using the proper format-specific mode @@ -273,19 +450,19 @@ def import_action(self, request, *args, **kwargs): data = tmp_storage.read() dataset = input_format.create_dataset(data) except Exception as e: - form.add_error('import_file', + import_form.add_error('import_file', _(f"'{type(e).__name__}' encountered while trying to read file. " "Ensure you have chosen the correct format for the file. " f"{str(e)}")) - if not form.errors: + if not import_form.errors: # prepare kwargs for import data, if needed - res_kwargs = self.get_import_resource_kwargs(request, form=form, *args, **kwargs) - resource = self.choose_import_resource_class(form)(**res_kwargs) + res_kwargs = self.get_import_resource_kwargs(request, form=import_form, *args, **kwargs) + resource = self.choose_import_resource_class(import_form)(**res_kwargs) resources = [resource] # prepare additional kwargs for import_data, if needed - imp_kwargs = self.get_import_data_kwargs(request, form=form, *args, **kwargs) + imp_kwargs = self.get_import_data_kwargs(request, form=import_form, *args, **kwargs) result = resource.import_data(dataset, dry_run=True, raise_errors=False, file_name=import_file.name, @@ -295,24 +472,26 @@ def import_action(self, request, *args, **kwargs): context['result'] = result if not result.has_errors() and not result.has_validation_errors(): - initial = { - 'import_file_name': tmp_storage.name, - 'original_file_name': import_file.name, - 'input_format': form.cleaned_data['input_format'], - 'resource': request.POST.get('resource', ''), - } - confirm_form = self.get_confirm_import_form() - initial = self.get_form_kwargs(form=form, **initial) - context['confirm_form'] = confirm_form(initial=initial) + if getattr(self.get_form_kwargs, "is_original", False): + # Use new API + context["confirm_form"] = self.create_confirm_form( + request, import_form=import_form + ) + else: + confirm_form_class = self.get_confirm_form_class(request) + initial = self.get_confirm_form_initial(request, import_form) + context["confirm_form"] = confirm_form_class( + initial=self.get_form_kwargs(form=import_form, **initial) + ) else: - res_kwargs = self.get_import_resource_kwargs(request, form=form, *args, **kwargs) + res_kwargs = self.get_import_resource_kwargs(request, form=import_form, *args, **kwargs) resource_classes = self.get_import_resource_classes() resources = [resource_class(**res_kwargs) for resource_class in resource_classes] context.update(self.admin_site.each_context(request)) context['title'] = _("Import") - context['form'] = form + context['form'] = import_form context['opts'] = self.model._meta context['fields_list'] = [ (resource.get_display_name(), [f.column_name for f in resource.get_user_visible_fields()]) @@ -343,6 +522,8 @@ class ExportMixin(BaseExportMixin, ImportExportMixinBase): export_template_name = 'admin/import_export/export.html' #: export data encoding to_encoding = None + #: form class to use for the initial import step + export_form_class = ExportForm def get_urls(self): urls = super().get_urls() @@ -421,18 +602,38 @@ def get_export_context_data(self, **kwargs): def get_context_data(self, **kwargs): return {} + @original def get_export_form(self): """ - Get the form type used to read the export format. + .. deprecated:: 3.0 + Use :meth:`~import_export.admin.ExportMixin.get_export_form_class` or set the new + :attr:`~import_export.admin.ExportMixin.export_form_class` attribute. """ - return ExportForm + warnings.warn( + "ExportMixin.get_export_form() is deprecated and will " + "be removed in a future release. Please use the new " + "'export_form_class' attribute to specify a custom form " + "class, or override the get_export_form_class() method if " + "your requirements are more complex.", + category=DeprecationWarning + ) + return self.export_form_class + + def get_export_form_class(self): + """ + Get the form class used to read the export format. + """ + return self.export_form_class def export_action(self, request, *args, **kwargs): if not self.has_export_permission(request): raise PermissionDenied + if getattr(self.get_export_form, 'is_original', False): + form_type = self.get_export_form_class() + else: + form_type = self.get_export_form() formats = self.get_export_formats() - form_type = self.get_export_form() form = form_type(formats, self.get_export_resource_classes(), request.POST or None) if form.is_valid(): file_format = formats[ diff --git a/import_export/utils.py b/import_export/utils.py index f7f019791..c4a2a2099 100644 --- a/import_export/utils.py +++ b/import_export/utils.py @@ -23,3 +23,13 @@ def __enter__(self): def __exit__(self, *args): if self.using_transactions: self.context_manager.__exit__(*args) + + +def original(method): + """ + A decorator used to mark some class methods as 'original', + making it easy to detect whether they have been overridden + by a subclass. Useful for method deprecation. + """ + method.is_original = True + return method diff --git a/tests/core/admin.py b/tests/core/admin.py index 81e758628..4ddadb0f9 100644 --- a/tests/core/admin.py +++ b/tests/core/admin.py @@ -4,7 +4,7 @@ from import_export.resources import ModelResource from .forms import CustomConfirmImportForm, CustomExportForm, CustomImportForm -from .models import Author, Book, Category, Child, EBook +from .models import Author, Book, Category, Child, EBook, LegacyBook class ChildAdmin(ImportMixin, admin.ModelAdmin): @@ -44,23 +44,38 @@ class AuthorAdmin(ImportMixin, admin.ModelAdmin): class CustomBookAdmin(BookAdmin): """BookAdmin with custom import forms""" + import_form_class = CustomImportForm + confirm_form_class = CustomConfirmImportForm + export_form_class = CustomExportForm + + def get_confirm_form_initial(self, request, import_form): + initial = super().get_confirm_form_initial(request, import_form) + # Pass on the `author` value from the import form to + # the confirm form (if provided) + if import_form: + initial['author'] = import_form.cleaned_data['author'].id + return initial + + +class LegacyBookAdmin(BookAdmin): + """ + BookAdmin with deprecated function overrides. + This class exists solely to test import works correctly using the deprecated + functions. + This class can be removed when the deprecated code is removed. + """ def get_import_form(self): - return CustomImportForm + return super().get_import_form() def get_confirm_import_form(self): - return CustomConfirmImportForm - - def get_export_form(self): - return CustomExportForm + return super().get_confirm_import_form() def get_form_kwargs(self, form, *args, **kwargs): - # update kwargs with authors (from CustomImportForm.cleaned_data) - if isinstance(form, CustomImportForm): - if form.is_valid(): - author = form.cleaned_data['author'] - kwargs.update({'author': author.id}) - return kwargs + return super().get_form_kwargs(form, *args, **kwargs) + + def get_export_form(self): + return super().get_export_form() admin.site.register(Book, BookAdmin) @@ -68,3 +83,4 @@ def get_form_kwargs(self, form, *args, **kwargs): admin.site.register(Author, AuthorAdmin) admin.site.register(Child, ChildAdmin) admin.site.register(EBook, CustomBookAdmin) +admin.site.register(LegacyBook, LegacyBookAdmin) diff --git a/tests/core/models.py b/tests/core/models.py index 487283b3b..39eb8ca96 100644 --- a/tests/core/models.py +++ b/tests/core/models.py @@ -152,6 +152,17 @@ class Meta: proxy = True +class LegacyBook(Book): + """ + Book proxy model to have a separate admin url access and name. + This class exists solely to test import works correctly using the deprecated + functions. + This class can be removed when the deprecated code is removed. + """ + class Meta: + proxy = True + + class UUIDBook(models.Model): """A model which uses a UUID pk (issue 1274)""" id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) diff --git a/tests/core/tests/test_admin_integration.py b/tests/core/tests/test_admin_integration.py index 03e4a9498..98004dde0 100644 --- a/tests/core/tests/test_admin_integration.py +++ b/tests/core/tests/test_admin_integration.py @@ -14,7 +14,7 @@ CustomBookAdmin, ImportMixin, ) -from core.models import Author, Book, Category, EBook, Parent +from core.models import Author, Book, Category, EBook, LegacyBook, Parent from django.contrib.admin.models import DELETION, LogEntry from django.contrib.auth.models import User from django.core.exceptions import PermissionDenied @@ -135,6 +135,48 @@ def test_import_second_resource(self): # Check, that we really use second resource - author_email didn't get imported self.assertEqual(Book.objects.get(id=1).author_email, "") + def test_import_legacy_book(self): + """ + This test exists solely to test import works correctly using the deprecated + functions. + This test can be removed when the deprecated code is removed. + """ + warnings.filterwarnings("ignore", category=DeprecationWarning) + Book.objects.create(id=1) + + # GET the import form + response = self.client.get('/admin/core/legacybook/import/') + self.assertContains(response, "Export/Import only book names") + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'admin/import_export/import.html') + self.assertContains(response, 'form action=""') + + # POST the import form + input_format = '0' + filename = os.path.join( + os.path.dirname(__file__), + os.path.pardir, + 'exports', + 'books.csv') + with open(filename, "rb") as f: + data = { + 'input_format': input_format, + 'import_file': f, + 'resource': 1, + } + response = self.client.post('/admin/core/legacybook/import/', data) + self.assertEqual(response.status_code, 200) + self.assertIn('result', response.context) + self.assertFalse(response.context['result'].has_errors()) + self.assertIn('confirm_form', response.context) + confirm_form = response.context['confirm_form'] + + data = confirm_form.initial + self.assertEqual(data['original_file_name'], 'books.csv') + response = self.client.post('/admin/core/legacybook/process_import/', data, follow=True) + self.assertEqual(response.status_code, 200) + self.assertContains(response, 'Import finished, with 0 new and 1 updated legacy books.') + def test_import_action_handles_UnicodeDecodeError_as_form_error(self): # POST the import form input_format = '0' @@ -368,6 +410,32 @@ def test_export_second_resource(self): ) self.assertEqual(b"id,name\r\n", response.content) + def test_export_legacy_resource(self): + """ + This test exists solely to test import works correctly using the deprecated + functions. + This test can be removed when the deprecated code is removed. + """ + warnings.filterwarnings("ignore", category=DeprecationWarning) + response = self.client.get('/admin/core/legacybook/export/') + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Export/Import only book names") + + data = { + 'file_format': '0', + 'resource': 1, + } + date_str = datetime.now().strftime('%Y-%m-%d') + response = self.client.post('/admin/core/legacybook/export/', data) + self.assertEqual(response.status_code, 200) + self.assertTrue(response.has_header("Content-Disposition")) + self.assertEqual(response['Content-Type'], 'text/csv') + self.assertEqual( + response['Content-Disposition'], + 'attachment; filename="LegacyBook-{}.csv"'.format(date_str) + ) + self.assertEqual(b"id,name\r\n", response.content) + def test_returns_xlsx_export(self): response = self.client.get('/admin/core/book/export/') self.assertEqual(response.status_code, 200) @@ -560,7 +628,7 @@ def test_import_with_customized_forms(self): confirm_form = response.context['confirm_form'] self.assertIsInstance(confirm_form, CustomBookAdmin(EBook, 'ebook/import') - .get_confirm_import_form()) + .get_confirm_form_class(None)) data = confirm_form.initial self.assertEqual(data['original_file_name'], 'books.csv') @@ -998,3 +1066,100 @@ def get_export_filename(self, request, queryset, file_format): self.export_mixin.export_admin_action(self.mock_request, list()) encoding_kwarg = mock_get_export_data.call_args_list[0][1]["encoding"] self.assertEqual("utf-8", encoding_kwarg) + + +class TestImportMixinDeprecationWarnings(TestCase): + class TestMixin(ImportMixin): + """ + TestMixin is a subclass which mimics a + class which the user may have created + """ + + def get_import_form(self): + return super().get_import_form() + + def get_confirm_import_form(self): + return super().get_confirm_import_form() + + def get_form_kwargs(self, form_class, **kwargs): + return super().get_form_kwargs(form_class, **kwargs) + + def setUp(self): + super().setUp() + self.import_mixin = ImportMixin() + + def test_get_import_form_warning(self): + target_msg = ( + "ImportMixin.get_import_form() is deprecated and will be removed in a future release. " + "Please use get_import_form_class() instead." + ) + with self.assertWarns(DeprecationWarning) as w: + self.import_mixin.get_import_form() + self.assertEqual(target_msg, str(w.warnings[0].message)) + + def test_get_confirm_import_form_warning(self): + target_msg = ( + "ImportMixin.get_confirm_import_form() is deprecated and will be removed in a future release. " + "Please use get_confirm_form_class() instead." + ) + with self.assertWarns(DeprecationWarning) as w: + self.import_mixin.get_confirm_import_form() + self.assertEqual(target_msg, str(w.warnings[0].message)) + + def test_get_form_kwargs_warning(self): + target_msg = ( + "ImportMixin.get_form_kwargs() is deprecated and will be removed in a future release. " + "Please use get_import_form_kwargs() or get_confirm_form_kwargs() instead." + ) + with self.assertWarns(DeprecationWarning) as w: + self.import_mixin.get_form_kwargs(None) + self.assertEqual(target_msg, str(w.warnings[0].message)) + + def test_get_import_form_class_warning(self): + self.import_mixin = self.TestMixin() + target_msg = ( + "ImportMixin.get_import_form() is deprecated and will be removed in a future release. " + "Please use the new 'import_form_class' attribute to specify a custom form class, " + "or override the get_import_form_class() method if your requirements are more complex." + ) + with self.assertWarns(DeprecationWarning) as w: + self.import_mixin.get_import_form_class(None) + self.assertEqual(target_msg, str(w.warnings[0].message)) + + def test_get_confirm_form_class_warning(self): + self.import_mixin = self.TestMixin() + target_msg = ( + "ImportMixin.get_confirm_import_form() is deprecated and will be removed in a future release. " + "Please use the new 'confirm_form_class' attribute to specify a custom form class, " + "or override the get_confirm_form_class() method if your requirements are more complex." + ) + with self.assertWarns(DeprecationWarning) as w: + self.import_mixin.get_confirm_form_class(None) + self.assertEqual(target_msg, str(w.warnings[0].message)) + + +class TestExportMixinDeprecationWarnings(TestCase): + class TestMixin(ExportMixin): + """ + TestMixin is a subclass which mimics a + class which the user may have created + """ + + def get_export_form(self): + return super().get_export_form() + + def setUp(self): + super().setUp() + self.export_mixin = self.TestMixin() + + def test_get_export_form_warning(self): + target_msg = ( + "ExportMixin.get_export_form() is deprecated and will " + "be removed in a future release. Please use the new " + "'export_form_class' attribute to specify a custom form " + "class, or override the get_export_form_class() method if " + "your requirements are more complex." + ) + with self.assertWarns(DeprecationWarning) as w: + self.export_mixin.get_export_form() + self.assertEqual(target_msg, str(w.warnings[0].message)) diff --git a/tests/core/tests/test_mixins.py b/tests/core/tests/test_mixins.py index 4280aafe2..0d51a2092 100644 --- a/tests/core/tests/test_mixins.py +++ b/tests/core/tests/test_mixins.py @@ -343,7 +343,7 @@ class TestExportForm(forms.ExportForm): def test_get_export_form(self): m = admin.ExportMixin() - self.assertEqual(forms.ExportForm, m.get_export_form()) + self.assertEqual(forms.ExportForm, m.get_export_form_class()) def test_get_export_form_with_custom_form(self): m = self.TestExportMixin(self.TestExportForm) From d60e5ab4357500934bd96ad48a0fb1d9a6fdaa4e Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Wed, 4 May 2022 16:37:11 +0100 Subject: [PATCH 54/77] Preparing release 3.0.0-beta.3 --- docs/changelog.rst | 2 +- import_export/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index d46ac0fcc..a803502e1 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,7 +1,7 @@ Changelog ========= -3.0.0-beta.3 (unreleased) +3.0.0-beta.3 (2022-05-04) ------------------------- - Refactored form-related methods on `ImportMixin` / `ExportMixin` (#1147) diff --git a/import_export/__init__.py b/import_export/__init__.py index db140818a..64d7455d4 100644 --- a/import_export/__init__.py +++ b/import_export/__init__.py @@ -1 +1 @@ -__version__ = '3.0.0.dev0' +__version__ = '3.0.0-beta.3' From c5820376a194ba953d68210ccdd1161c416a8ad6 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Wed, 4 May 2022 16:43:39 +0100 Subject: [PATCH 55/77] Back to development: 3.0.0-beta.4 --- docs/changelog.rst | 6 ++++++ import_export/__init__.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index a803502e1..306665835 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,6 +1,12 @@ Changelog ========= +3.0.0-beta.4 (unreleased) +------------------------- + +- Nothing changed yet. + + 3.0.0-beta.3 (2022-05-04) ------------------------- diff --git a/import_export/__init__.py b/import_export/__init__.py index 64d7455d4..277d61ff2 100644 --- a/import_export/__init__.py +++ b/import_export/__init__.py @@ -1 +1 @@ -__version__ = '3.0.0-beta.3' +__version__ = '3.0.0-beta.4.dev0' From 7e2a7fcf93c8ecea4e4753c90183f5862d824973 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Sat, 7 May 2022 13:53:34 +0100 Subject: [PATCH 56/77] re-added lost newlines --- tests/core/models.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/core/models.py b/tests/core/models.py index 39eb8ca96..02802600b 100644 --- a/tests/core/models.py +++ b/tests/core/models.py @@ -18,6 +18,8 @@ def get_by_natural_key(self, name): Django pattern function for finding an author by its name """ return self.get(name=name) + + class Author(models.Model): objects = AuthorManager() @@ -54,6 +56,7 @@ class Category(models.Model): def __str__(self): return self.name + class BookManager(models.Manager): """ Added to enable get_by_natural_key method @@ -67,6 +70,7 @@ def get_by_natural_key(self, name, author): """ return self.get(name=name, author=Author.objects.get_by_natural_key(author)) + class Book(models.Model): objects = BookManager() @@ -109,6 +113,7 @@ class Child(models.Model): def __str__(self): return '%s - child of %s' % (self.name, self.parent.name) + class Profile(models.Model): user = models.OneToOneField('auth.User', on_delete=models.CASCADE) is_private = models.BooleanField(default=True) From 04840618cde39fe7adae950d6086d813644116b2 Mon Sep 17 00:00:00 2001 From: Manel Clos Date: Wed, 11 May 2022 14:30:06 +0200 Subject: [PATCH 57/77] fix get_resource_class reference in documentation --- docs/getting_started.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/getting_started.rst b/docs/getting_started.rst index 59496a750..e9069b961 100644 --- a/docs/getting_started.rst +++ b/docs/getting_started.rst @@ -148,7 +148,7 @@ the import preview page:: To further customize the resources, you might like to consider overriding the following :class:`~import_export.admin.ImportMixin` methods: :meth:`~import_export.admin.ImportMixin.get_resource_kwargs`, -:meth:`~import_export.admin.ImportMixin.get_resource_class`, +:meth:`~import_export.admin.ImportMixin.get_resource_classes`, :meth:`~import_export.admin.ImportMixin.get_export_resource_kwargs`, .. seealso:: From ed439cb12c5ee7d883e358280a43a9b7fec3907f Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 12 May 2022 18:47:24 +0100 Subject: [PATCH 58/77] added tests for #1436 --- ...y_legacybook_alter_uuidbook_id_and_more.py | 42 +++++++++++++++++++ tests/core/models.py | 8 +++- tests/core/tests/test_resources.py | 24 ++++++++++- 3 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 tests/core/migrations/0011_uuidcategory_legacybook_alter_uuidbook_id_and_more.py diff --git a/tests/core/migrations/0011_uuidcategory_legacybook_alter_uuidbook_id_and_more.py b/tests/core/migrations/0011_uuidcategory_legacybook_alter_uuidbook_id_and_more.py new file mode 100644 index 000000000..21382a7c1 --- /dev/null +++ b/tests/core/migrations/0011_uuidcategory_legacybook_alter_uuidbook_id_and_more.py @@ -0,0 +1,42 @@ +# Generated by Django 4.0.4 on 2022-05-12 12:39 + +from django.db import migrations, models +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0010_uuidbook'), + ] + + operations = [ + migrations.CreateModel( + name='UUIDCategory', + fields=[ + ('catid', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)), + ('name', models.CharField(max_length=32)), + ], + ), + migrations.CreateModel( + name='LegacyBook', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('core.book',), + ), + migrations.AlterField( + model_name='uuidbook', + name='id', + field=models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False), + ), + migrations.AddField( + model_name='uuidbook', + name='categories', + field=models.ManyToManyField(blank=True, to='core.uuidcategory'), + ), + ] diff --git a/tests/core/models.py b/tests/core/models.py index 02802600b..07892bfdb 100644 --- a/tests/core/models.py +++ b/tests/core/models.py @@ -168,7 +168,13 @@ class Meta: proxy = True +class UUIDCategory(models.Model): + catid = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) + name = models.CharField(max_length=32) + + class UUIDBook(models.Model): """A model which uses a UUID pk (issue 1274)""" id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) - name = models.CharField('Book name', max_length=100) \ No newline at end of file + name = models.CharField('Book name', max_length=100) + categories = models.ManyToManyField(UUIDCategory, blank=True) \ No newline at end of file diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 6fb4119d6..fe2ae7240 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -38,7 +38,7 @@ UUIDBook, WithDefault, WithDynamicDefault, - WithFloatField, + WithFloatField, UUIDCategory, ) @@ -1653,6 +1653,28 @@ def test_many_to_many_widget_handles_ordering(self): self.assertEqual(2, book.categories.count()) + def test_many_to_many_widget_handles_uuid(self): + class _UUIDBookResource(resources.ModelResource): + class Meta: + model = UUIDBook + + uuid_resource = _UUIDBookResource() + uuid_resource._meta.skip_unchanged = True + cat1 = UUIDCategory.objects.create(name="Category 1") + cat2 = UUIDCategory.objects.create(name="Category 2") + uuid_book = UUIDBook.objects.create(name="uuid book") + uuid_book.categories.add(cat1, cat2) + uuid_book.save() + + # import with natural order + dataset_headers = ["id", "name", "categories"] + dataset_row = [uuid_book.id, uuid_book.name, f"{cat1.catid},{cat2.catid}"] + dataset = tablib.Dataset(headers=dataset_headers) + dataset.append(dataset_row) + result = uuid_resource.import_data(dataset, dry_run=False) + print(result) + self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_SKIP) + @mock.patch("import_export.resources.Diff", spec=True) class SkipDiffTest(TestCase): From 721a03333d747cde0d78bc12ca6758fc10739824 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 12 May 2022 18:52:52 +0100 Subject: [PATCH 59/77] added comment --- tests/core/tests/test_resources.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index fe2ae7240..70aa722d7 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -1654,6 +1654,7 @@ def test_many_to_many_widget_handles_ordering(self): self.assertEqual(2, book.categories.count()) def test_many_to_many_widget_handles_uuid(self): + # Test for #1435 - skip_row() handles M2M widget when UUID pk used class _UUIDBookResource(resources.ModelResource): class Meta: model = UUIDBook @@ -1666,13 +1667,11 @@ class Meta: uuid_book.categories.add(cat1, cat2) uuid_book.save() - # import with natural order dataset_headers = ["id", "name", "categories"] dataset_row = [uuid_book.id, uuid_book.name, f"{cat1.catid},{cat2.catid}"] dataset = tablib.Dataset(headers=dataset_headers) dataset.append(dataset_row) result = uuid_resource.import_data(dataset, dry_run=False) - print(result) self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_SKIP) From 649fbd21b6039d74f3e1c7ad643721159995f9fa Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 12 May 2022 18:54:04 +0100 Subject: [PATCH 60/77] isort fix --- .../0011_uuidcategory_legacybook_alter_uuidbook_id_and_more.py | 3 ++- tests/core/tests/test_resources.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/core/migrations/0011_uuidcategory_legacybook_alter_uuidbook_id_and_more.py b/tests/core/migrations/0011_uuidcategory_legacybook_alter_uuidbook_id_and_more.py index 21382a7c1..6915ee352 100644 --- a/tests/core/migrations/0011_uuidcategory_legacybook_alter_uuidbook_id_and_more.py +++ b/tests/core/migrations/0011_uuidcategory_legacybook_alter_uuidbook_id_and_more.py @@ -1,8 +1,9 @@ # Generated by Django 4.0.4 on 2022-05-12 12:39 -from django.db import migrations, models import uuid +from django.db import migrations, models + class Migration(migrations.Migration): diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 70aa722d7..83f896160 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -36,9 +36,10 @@ Profile, Role, UUIDBook, + UUIDCategory, WithDefault, WithDynamicDefault, - WithFloatField, UUIDCategory, + WithFloatField, ) From 4e3ffac4ce69125f90c70c6267f7474cdfd432a5 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Thu, 12 May 2022 18:57:51 +0100 Subject: [PATCH 61/77] skip_row() handles M2M field when UUID pk used --- docs/changelog.rst | 2 +- import_export/resources.py | 2 +- tests/core/tests/test_resources.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 306665835..8b83313bf 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,7 +4,7 @@ Changelog 3.0.0-beta.4 (unreleased) ------------------------- -- Nothing changed yet. +- bugfix: `skip_row()` handles M2M field when UUID pk used 3.0.0-beta.3 (2022-05-04) diff --git a/import_export/resources.py b/import_export/resources.py index b6a1db4a8..fa74e74c3 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -630,7 +630,7 @@ def skip_row(self, instance, original, row, import_validation_errors=None): if len(instance_values) != len(original_values): return False - if sorted(v.id for v in instance_values) != sorted(v.id for v in original_values): + if sorted(v.pk for v in instance_values) != sorted(v.pk for v in original_values): return False else: if field.get_value(instance) != field.get_value(original): diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index 83f896160..f6453c3a3 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -1655,7 +1655,7 @@ def test_many_to_many_widget_handles_ordering(self): self.assertEqual(2, book.categories.count()) def test_many_to_many_widget_handles_uuid(self): - # Test for #1435 - skip_row() handles M2M widget when UUID pk used + # Test for #1435 - skip_row() handles M2M field when UUID pk used class _UUIDBookResource(resources.ModelResource): class Meta: model = UUIDBook From 01f041f3d903295b03db80b2fd27dbf7611951ef Mon Sep 17 00:00:00 2001 From: Manel Clos Date: Tue, 17 May 2022 16:51:27 +0200 Subject: [PATCH 62/77] improve compatibility with previous ImportForm signature (#1434) --- docs/changelog.rst | 3 ++ import_export/admin.py | 44 +++++++++++++++++---- tests/core/tests/test_admin_integration.py | 46 ++++++++++++++++++++-- 3 files changed, 82 insertions(+), 11 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 8b83313bf..19f9225a1 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -49,6 +49,9 @@ This release makes some minor changes to the public API. If you have overridden - Admin interface: Modified handling of import errors (#1306) - Exceptions raised during the import process are now presented as form errors, instead of being wrapped in a \ tag in the response. If you have any custom logic which uses the error written directly into the response, then this may need to be changed. +- ImportForm: improve compatibility with previous signature (#1434) + - Previous `ImportForm` implementation was based on Django's `forms.Form`, if you have any custom ImportForm you now need to inherit from `import_export.forms.ImportExportFormBase`. + Deprecations ############ diff --git a/import_export/admin.py b/import_export/admin.py index 67bb65f86..bbb13a6ef 100644 --- a/import_export/admin.py +++ b/import_export/admin.py @@ -17,7 +17,13 @@ from django.views.decorators.http import require_POST from .formats.base_formats import DEFAULT_FORMATS -from .forms import ConfirmImportForm, ExportForm, ImportForm, export_action_form_factory +from .forms import ( + ConfirmImportForm, + ExportForm, + ImportExportFormBase, + ImportForm, + export_action_form_factory, +) from .mixins import BaseExportMixin, BaseImportMixin from .results import RowResult from .signals import post_export, post_import @@ -252,6 +258,14 @@ def create_import_form(self, request): formats = self.get_import_formats() form_class = self.get_import_form_class(request) kwargs = self.get_import_form_kwargs(request) + + if not issubclass(form_class, ImportExportFormBase): + warnings.warn( + "The ImportForm class must inherit from ImportExportFormBase, " + "this is needed for multiple resource classes to work properly. ", + category=DeprecationWarning + ) + return form_class(formats, **kwargs) return form_class(formats, self.get_import_resource_classes(), **kwargs) def get_import_form_class(self, request): @@ -423,13 +437,27 @@ def import_action(self, request, *args, **kwargs): else: form_class = self.get_import_form_class(request) form_kwargs = self.get_form_kwargs(form_class, *args, **kwargs) - import_form = form_class( - import_formats, - self.get_import_resource_classes(), - request.POST or None, - request.FILES or None, - **form_kwargs - ) + + if issubclass(form_class, ImportExportFormBase): + import_form = form_class( + import_formats, + self.get_import_resource_classes(), + request.POST or None, + request.FILES or None, + **form_kwargs + ) + else: + warnings.warn( + "The ImportForm class must inherit from ImportExportFormBase, " + "this is needed for multiple resource classes to work properly. ", + category=DeprecationWarning + ) + import_form = form_class( + import_formats, + request.POST or None, + request.FILES or None, + **form_kwargs + ) resources = list() if request.POST and import_form.is_valid(): diff --git a/tests/core/tests/test_admin_integration.py b/tests/core/tests/test_admin_integration.py index 98004dde0..b1b13e93c 100644 --- a/tests/core/tests/test_admin_integration.py +++ b/tests/core/tests/test_admin_integration.py @@ -641,6 +641,46 @@ def test_import_with_customized_forms(self): 1, 0, EBook._meta.verbose_name_plural) ) + @mock.patch('core.admin.BookAdmin.get_import_form_class') + def test_deprecated_importform_new_api_raises_warning(self, mock_get_import_form): + class DjangoImportForm(django.forms.Form): + def __init__(self, import_formats, *args, **kwargs): + super().__init__(*args, **kwargs) + + mock_get_import_form.return_value = DjangoImportForm + + with self.assertWarnsRegex( + DeprecationWarning, + r"^The ImportForm class must inherit from ImportExportFormBase, " + r"this is needed for multiple resource classes to work properly. $" + ): + # GET the import form + response = self.client.get('/admin/core/book/import/') + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'admin/import_export/import.html') + self.assertContains(response, 'form action=""') + + @mock.patch('core.admin.BookAdmin.get_import_form_class') + @mock.patch('core.admin.BookAdmin.get_form_kwargs') + def test_deprecated_importform_raises_warning(self, mock_get_form_kwargs, mock_get_import_form): + class DjangoImportForm(django.forms.Form): + def __init__(self, import_formats, *args, **kwargs): + super().__init__(*args, **kwargs) + + mock_get_form_kwargs.is_original = False + mock_get_import_form.return_value = DjangoImportForm + + with self.assertWarnsRegex( + DeprecationWarning, + r"^The ImportForm class must inherit from ImportExportFormBase, " + r"this is needed for multiple resource classes to work properly. $" + ): + # GET the import form + response = self.client.get('/admin/core/book/import/') + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'admin/import_export/import.html') + self.assertContains(response, 'form action=""') + def test_get_skip_admin_log_attribute(self): m = ImportMixin() m.skip_admin_log = True @@ -946,7 +986,7 @@ def __init__(self): m = TestCategoryAdmin() action_form = m.action_form - + items = list(action_form.base_fields.items()) file_format = items[len(items)-1][1] choices = file_format.choices @@ -971,7 +1011,7 @@ def __init__(self): m = TestCategoryAdmin() action_form = m.action_form - + items = list(action_form.base_fields.items()) file_format = items[len(items)-1][1] choices = file_format.choices @@ -981,7 +1021,7 @@ def __init__(self): m = TestFormatsCategoryAdmin() action_form = m.action_form - + items = list(action_form.base_fields.items()) file_format = items[len(items)-1][1] choices = file_format.choices From 9a698733f1f5123a81670a6a71e6d11f7376ed4f Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Tue, 17 May 2022 15:55:23 +0100 Subject: [PATCH 63/77] updated changelog for 3.0.0b4 --- docs/changelog.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 19f9225a1..cda5a9e92 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -5,6 +5,7 @@ Changelog ------------------------- - bugfix: `skip_row()` handles M2M field when UUID pk used +- improve compatibility with previous ImportForm signature (#1434) 3.0.0-beta.3 (2022-05-04) From 2643f8c76fc72976ad7c2c2162cc9736fd4fba51 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Tue, 17 May 2022 15:57:19 +0100 Subject: [PATCH 64/77] Preparing release 3.0.0-beta.4 --- docs/changelog.rst | 2 +- import_export/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index cda5a9e92..5fe00c6eb 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,7 +1,7 @@ Changelog ========= -3.0.0-beta.4 (unreleased) +3.0.0-beta.4 (2022-05-17) ------------------------- - bugfix: `skip_row()` handles M2M field when UUID pk used diff --git a/import_export/__init__.py b/import_export/__init__.py index 277d61ff2..0e7909d3e 100644 --- a/import_export/__init__.py +++ b/import_export/__init__.py @@ -1 +1 @@ -__version__ = '3.0.0-beta.4.dev0' +__version__ = '3.0.0-beta.4' From 061374f81bc312c10e45406e67885ce8bb34f2ce Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Tue, 17 May 2022 16:09:10 +0100 Subject: [PATCH 65/77] Back to development: 3.0.0-beta.5 --- docs/changelog.rst | 6 ++++++ import_export/__init__.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 5fe00c6eb..d8eb757e1 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,6 +1,12 @@ Changelog ========= +3.0.0-beta.5 (unreleased) +------------------------- + +- Nothing changed yet. + + 3.0.0-beta.4 (2022-05-17) ------------------------- diff --git a/import_export/__init__.py b/import_export/__init__.py index 0e7909d3e..67336d9cd 100644 --- a/import_export/__init__.py +++ b/import_export/__init__.py @@ -1 +1 @@ -__version__ = '3.0.0-beta.4' +__version__ = '3.0.0-beta.5.dev0' From d8b9f3ebe8fe13ecd2451773d8c33e98af299b3c Mon Sep 17 00:00:00 2001 From: Matt Hegarty Date: Mon, 23 May 2022 17:14:49 +0100 Subject: [PATCH 66/77] skip_row() handle m2m field not present in import file (#1439) --- docs/changelog.rst | 3 +-- import_export/resources.py | 5 +++++ tests/core/tests/test_resources.py | 20 +++++++++++++++++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index d8eb757e1..72077cf9d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,8 +4,7 @@ Changelog 3.0.0-beta.5 (unreleased) ------------------------- -- Nothing changed yet. - +- bugfix: `skip_row()` fix crash when model has m2m field and none is provided in upload (#1439) 3.0.0-beta.4 (2022-05-17) ------------------------- diff --git a/import_export/resources.py b/import_export/resources.py index fa74e74c3..c378d83b4 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -625,6 +625,11 @@ def skip_row(self, instance, original, row, import_validation_errors=None): # For fields that are models.fields.related.ManyRelatedManager # we need to compare the results if isinstance(field.widget, widgets.ManyToManyWidget): + # #1437 - handle m2m field not present in import file + if field.column_name not in row.keys(): + continue + # m2m instance values are taken from the 'row' because they + # have not been written to the 'instance' at this point instance_values = list(field.clean(row)) original_values = list(field.get_value(original).all()) if len(instance_values) != len(original_values): diff --git a/tests/core/tests/test_resources.py b/tests/core/tests/test_resources.py index f6453c3a3..00608abb4 100644 --- a/tests/core/tests/test_resources.py +++ b/tests/core/tests/test_resources.py @@ -965,7 +965,7 @@ def test_empty_get_queryset(self): self.assertEqual(len(dataset), 0) def test_import_data_skip_unchanged(self): - def attempted_save(instance, real_dry_run): + def attempted_save(instance, new, using_transactions, real_dry_run): self.fail('Resource attempted to save instead of skipping') # Make sure we test with ManyToMany related objects @@ -1675,6 +1675,24 @@ class Meta: result = uuid_resource.import_data(dataset, dry_run=False) self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_SKIP) + def test_skip_row_no_m2m_data_supplied(self): + # issue #1437 + # test skip_row() when the model defines a m2m field + # but it is not present in the dataset + book = Book.objects.first() + dataset_headers = ["id", "name"] + dataset_row = [book.id, book.name] + dataset = tablib.Dataset(headers=dataset_headers) + dataset.append(dataset_row) + + book_resource = BookResource() + book_resource._meta.skip_unchanged = True + + self.assertEqual(1, book.categories.count()) + result = book_resource.import_data(dataset, dry_run=False) + self.assertEqual(result.rows[0].import_type, results.RowResult.IMPORT_TYPE_SKIP) + self.assertEqual(1, book.categories.count()) + @mock.patch("import_export.resources.Diff", spec=True) class SkipDiffTest(TestCase): From 990cf7fadfca325f7e673536eef391b649ffc018 Mon Sep 17 00:00:00 2001 From: matthewhegarty Date: Fri, 2 Sep 2022 14:41:10 +0100 Subject: [PATCH 67/77] fixed main branch merge issues --- import_export/forms.py | 13 +++++---- .../templates/admin/import_export/import.html | 1 - import_export/widgets.py | 1 + tests/core/tests/test_admin_integration.py | 28 ++++++++++++++++--- 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/import_export/forms.py b/import_export/forms.py index c635bd76d..80b400d6d 100644 --- a/import_export/forms.py +++ b/import_export/forms.py @@ -46,12 +46,15 @@ def __init__(self, import_formats, *args, **kwargs): self.fields['input_format'].choices = choices - class Media: + @property + def media(self): extra = "" if settings.DEBUG else ".min" - js = ( - f'admin/js/vendor/jquery/jquery{extra}.js', - 'admin/js/jquery.init.js', - 'import_export/guess_format.js', + return forms.Media( + js=( + f'admin/js/vendor/jquery/jquery{extra}.js', + 'admin/js/jquery.init.js', + 'import_export/guess_format.js', + ) ) diff --git a/import_export/templates/admin/import_export/import.html b/import_export/templates/admin/import_export/import.html index 6e4c27679..b61ffa7bc 100644 --- a/import_export/templates/admin/import_export/import.html +++ b/import_export/templates/admin/import_export/import.html @@ -5,7 +5,6 @@ {% load static %} {% block extrastyle %}{{ block.super }}{% endblock %} -{% block extrahead %}{{ block.super }}{{ media.js }}{% endblock %} {% block extrahead %}{{ block.super }} diff --git a/import_export/widgets.py b/import_export/widgets.py index 2ffe1e0a2..18c373966 100644 --- a/import_export/widgets.py +++ b/import_export/widgets.py @@ -210,6 +210,7 @@ def __init__(self, format=None): self.formats = formats def clean(self, value, row=None, **kwargs): + dt = None if not value: return None if isinstance(value, datetime): diff --git a/tests/core/tests/test_admin_integration.py b/tests/core/tests/test_admin_integration.py index b6702df2e..229d333a8 100644 --- a/tests/core/tests/test_admin_integration.py +++ b/tests/core/tests/test_admin_integration.py @@ -14,7 +14,7 @@ CustomBookAdmin, ImportMixin, ) -from core.models import Author, Book, Category, EBook, LegacyBook, Parent +from core.models import Author, Book, Category, EBook, Parent from django.contrib.admin.models import DELETION, LogEntry from django.contrib.auth.models import User from django.core.exceptions import PermissionDenied @@ -40,6 +40,7 @@ class ImportExportAdminIntegrationTest(TestCase): def setUp(self): + super().setUp() user = User.objects.create_user('admin', 'admin@example.com', 'password') user.is_staff = True @@ -62,9 +63,6 @@ def test_import(self): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'admin/import_export/import.html') self.assertContains(response, 'form action=""') - self.assertContains(response, '