Remove code duplication from mixins.py and admin.py

[matthewhegarty]

Problem

• A bug where args and kwargs are not passed to get_export_resource_kwargs()
• Removed code duplication in admin.py and mixins.py

Solution

When looking into this issue, I noticed that we have a fair amount of code duplication in mixins.py and admin.py.
I removed the duplication as part of this fix.

I tried to keep code changes to a minimum, and also not to affect the public API defined in mixins.
I added some tests to improve the coverage in this area.

Would appreciate a code review from @manelclos and @ZuluPro

Related comments by @manelclos

base mixin for import and base mixin for export, reused in the ViewMixin and the Admin Mixin, share code as much as possible when it makes sense.

Issue 1268

Added get_data_for_export() to remove duplication and to pass the args and kwargs to get_export_resource_kwargs() which was the original issue reported in #1268.
Also added a unit test for this.

Updated mixins.py

Removed the following duplicate methods from ExportViewMixin and added to
ImportExportMixin:

def get_resource_class(self):
def get_resource_kwargs(self, request, *args, **kwargs):

Removed the following duplicate methods from ExportViewMixin and added to
BaseImportMixin:

def get_import_resource_class(self):
def get_import_formats(self):
def get_import_resource_kwargs(self, request, *args, **kwargs):

Removed the following duplicate methods from ExportViewMixin and added to
BaseExportMixin:

def get_export_formats(self):
def get_export_resource_class(self):
def get_export_resource_kwargs(self, request, *args, **kwargs):
def get_export_filename(self, file_format):

Updated admin.py

I didn't rename any classes here because they might be part of the public API.

• ImportMixin now subclasses BaseImportMixin (and ImportExportMixinBase)

• Updated ImportMixin: Removed the following duplicate methods (now inherited
  from BaseImportMixin):

def get_resource_kwargs(self, request, *args, **kwargs):
def get_import_resource_kwargs(self, request, *args, **kwargs):
def get_resource_class(self):
def get_import_resource_class(self):
def get_import_formats(self):

• ExportMixin now subclasses BaseExportMixin (and ImportExportMixinBase)

• Updated ExportMixin: Removed the following duplicate methods (now inherited
  from BaseExportMixin):

def get_resource_kwargs(self, request, *args, **kwargs):
def get_export_resource_kwargs(self, request, *args, **kwargs):
def get_resource_class(self):
def get_export_resource_class(self):
def get_export_formats(self):

# I kept this filename for API compatibility, even though 'request' and
# 'queryset' are not used
def get_export_filename(self, request, queryset, file_format):

Acceptance Criteria

Test coverage

• increased admin.py coverage to 100%
• increased mixins.py coverage to 100%

Testing

• Ran test suite
• Manually exported & imported Book
• Manually exported Category (using 'action' menu)

[ZuluPro]

Hi @matthewhegarty

Looks good to me!
Could you just fix the failling test ?

[coveralls]

Coverage increased (+0.5%) to 97.315% when pulling 0acad28 on matthewhegarty:issue-1268 into 722c376 on django-import-export:master.

[matthewhegarty]

Thanks for reviewing @ZuluPro

[manelclos]

Looks good so far. 👏 for the refactor and the increased coverage.

[matthewhegarty]

@manelclos Many thanks for reviewing this. I have responded to your comments.

[manelclos]

LGTM, great work on the refactor and the tests, thanks!

[manelclos]

I'd say that you can keep this function as get_export_filename and don't implement it in ExportViewMixin, but not important.

[matthewhegarty]

I might have misunderstood you, but I think this runs into the problem of get_export_filename() being called in two places with the different args.

[manelclos]

Only the admin.py uses the different arguments, and you are already adapting that call inside ExportMixin:

https://github.com/django-import-export/django-import-export/pull/1277/files#diff-a6dabb2139e7ec78138476e9a12311c22cc93cbeab4a93994d4f95f64934805eR437-R438

But I could be wrong :)

[andrewgy8]

LGTM! Thanks for the work @matthewhegarty

[andrewgy8]

Very nice refactor. Its nice to see

data = resource_class(**self.get_export_resource_kwargs(request)).export(queryset, *args, **kwargs)

go.