Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Make confirm_form accessible in get_import_resource_kwargs #994

Merged

Conversation

chrismerck
Copy link
Contributor

Problem

get_import_resource_kwargs should allow an ImportForm (and ConfirmImportForm) to provide customization of a Resource instance.

get_import_resource_kwargs is called in two places:

  1. when processing the ImportForm
  2. when processing the ConfirmImportForm

The first call passes form=form keyword argument, but the latter call does not pass form=confirm_form.

Solution

I simply set the form keyword in the second call to get_import_resource_kwargs.

This permits usage like this:

class BookAdmin(ImportMixin, admin.ModelAdmin):
...
    def get_resource_kwargs(self, request, *args, **kwargs):
        form = kwargs['form']
        rv = {}
        if isinstance(form, BookFormMixin):
                if form.is_valid():
                    author = form.cleaned_data['author']
                    rv.update({'author': author.pk})
        return rv

Acceptance Criteria

I'm using this change in my application under development, but have not created a stripped-down example of this usage. I can do so if this change is not clearly desirable.

The import form was accessible already,
but the form keyword was not set for the confirmation step.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.675% when pulling c60a417 on chrismerck:chrismerck/girk_confirm_form into 141932b on django-import-export:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.675% when pulling c60a417 on chrismerck:chrismerck/girk_confirm_form into 141932b on django-import-export:master.

@andreynovikov
Copy link
Contributor

This will partially fix #1034

@milutinke-vivify
Copy link

Can you merge this please :)

@milutinke-vivify
Copy link

@andreynovikov not quite I tried.
get_confirm_import_form(self) must be used, instead of explicitly using confirm_form = ConfirmImportForm(request.POST)

Copy link
Member

@andrewgy8 andrewgy8 left a comment

Choose a reason for hiding this comment

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

Thanks! I just merged #1108 which is also very similar

@andrewgy8 andrewgy8 changed the title make confirm_form accessible in get_import_resource_kwargs Make confirm_form accessible in get_import_resource_kwargs Apr 26, 2020
@andrewgy8 andrewgy8 merged commit b18041b into django-import-export:master Apr 26, 2020
ZuluPro pushed a commit to ZuluPro/django-import-export that referenced this pull request Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants