django-import-export / django-import-export

Django application and library for importing and exporting data with admin integration.
https://django-import-export.readthedocs.org/en/latest/
BSD 2-Clause "Simplified" License
3.08k stars 800 forks source link

Custom import form data lost when admin confirmation is enabled #2007

Open marcinn opened 1 week ago

marcinn commented 1 week ago

Describe the bug Define custom import form and add field, like this

  class VehicleImportForm(ImportForm):                                                                                                                                                        
      status = forms.ChoiceField(                                                                                                                                                             
          choices=Vehicle.Status.choices,                                                                                                                                                     
          required=True,                                                                                                                                                                      
          label="Status",                                                                                                                                                                     
          help_text="Wybierz status dla importowanych rekordów",                                                                                                                              
      ) 

Admin:

  @admin.register(Vehicle)                                                                                                                                                                    
  class VehicleAdmin(ImportExportMixin, admin.ModelAdmin):                                                                                                                                    
      resource_classes = [VehicleResource]                                                                                                                                                    
      import_form_class = VehicleImportForm                                                                                                                                                   
      skip_import_confirm = False

      def get_confirm_form_initial(self, request, import_form):                                                                                                                               
          initial = super().get_confirm_form_initial(request, import_form)                                                                                                                    

          if import_form:                                                                                                                                                                     
              initial["status"] = import_form.cleaned_data["status"]                                                                                                                          
          return initial                                                                                                                                                                      

      def get_import_data_kwargs(self, **kwargs):                                                                                                                                             
          form = kwargs.get("form", None)                                                                                                                                                     
          if form and hasattr(form, "cleaned_data"):                                                                                                                                          
              status = form.cleaned_data.get("status")                                                                                                                                        
              if status:                                                                                                                                                                      
                  kwargs["status"] = status                                                                                                                                                   
          return kwargs  

Resource:

class VehicleResource(resources.ModelResource): 
     def after_init_instance(self, instance, *args, **kwargs):                                                                                                                               
          if "status" in kwargs:
              instance.status = kwargs["status"]

To Reproduce

Steps to reproduce the behavior:

  1. Make sure admin confirmation is enabled
  2. Set breakpoint in after_init_instance
  3. Run import and check 'status' in kwargs, will be True
  4. Disable breakpoint and run import once again. The confirmation page should be displayed.
  5. Set breakpoint again and confirm the import
  6. Check 'status' in kwargs, will be False - i.e. no status in kwargs.

Versions (please complete the following information):

Expected behavior In after_init_instance key status should be in kwargs. Import should set model's status field to a selected value from the input form.

Screenshots image image image

Additional context

matthewhegarty commented 1 week ago

Will take a look. One thing I notice, check that you have the correct definition of after_init_instance().

New definition is:

    def after_init_instance(self, instance, new, row, **kwargs):

This changed in v4. Please see release notes for more information.

matthewhegarty commented 1 week ago

The example app uses a Book import example, with 'author' as a selectable field. I just tried to reproduce your issue with the example app and found that the 'author' field is present after the 'confirm' step.

image

I wonder if your method declaration is the issue. Can you see if you can reproduce with the example app?

marcinnformula commented 6 days ago

The root cause of the comes from missing status field in confirm_form. So there is necessity to redeclare both forms: import_form and confirm_form, when confirmation step is enabled, like this:

  class VehicleImportForm(ImportForm):
      status = forms.ChoiceField(
          choices=Vehicle.Status.choices,
          required=True,
          label="Status",
          help_text="Wybierz status dla importowanych rekordów",
      )

  class VehicleConfirmImportForm(ConfirmImportForm):
      status = forms.ChoiceField(
          choices=Vehicle.Status.choices,
          required=True,
          label="Status",
          help_text="Wybierz status dla importowanych rekordów",
      )

 @admin.register(Vehicle)                                                                    
 class VehicleAdmin(ImportExportMixin, admin.ModelAdmin):
      resource_classes = [VehicleResource]
      import_form_class = VehicleImportForm
      confirm_form_class = VehicleConfirmImportForm
      skip_import_confirm = False

It was not so straightforward for me. I understood this after inspecting process_import() and import_action() with debugger.

I think there is no easy way to handle this case automatically... Of course I have an idea of finding "non-standard" fields in import_form_class during process_import, and add them as hidden fields to dynamically created confirm_form (based on generic ConfirmForm class), but this should be handled carefully and described in docs.

marcinn commented 6 days ago

Sorry, I replied from my 2nd account.