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

Regression from #168: unable to call get_cleaned_data_for_step on conditionally step #266

Open
mlorant opened this issue Feb 15, 2024 · 0 comments

Comments

@mlorant
Copy link

mlorant commented Feb 15, 2024

I have updated my project requirements from django-formtools 2.3 to 2.5.1. However some of my tests using WizardView are now broken and I can sort of bisect the problem between the 2.3.0 and 2.4.0 version.

Suppose a form where you're asked to upload an Excel file and process some data in it, you may need to select a sheet if multiple are present in the file, but that step can be ignored if the file has a single sheet. The WizardView would look like this:

class MyWizardView(WizardView):
    form_list = [
        ('excel_file_selection', ...),
        ('excel_sheet_selection', ...),
        ('do_other_stuff', ...),
        ...
    ]

    condition_dict = {
        'excel_sheet_selection': is_sheet_selection_needed,
        ...
    }

    def get_sheet_index(self):
        if cleaned_data := self.get_cleaned_data_for_step('excel_sheet_selection'):
            return int(cleaned_data['sheet_number'])
        
        return 0

In formtools 2.3 version, get_cleaned_data_for_step is returning None when the excel_sheet_selection step is passed (e.g because is_sheet_selection_needed returned False because there's a single sheet in the file in our case)

In formtools 2.4 and after, I get the following exception:

  File "<mycode>/views.py", line 177, in get_sheet_index
    cleaned_data = self.get_cleaned_data_for_step('excel_sheet_selection')
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<venv>/formtools/wizard/views.py", line 494, in get_cleaned_data_for_step
    form_obj = self.get_form(
               ^^^^^^^^^^^^^^
  File "<venv>/formtools/wizard/views.py", line 409, in get_form
    form_class = self.get_form_list()[step]
                 ~~~~~~~~~~~~~~~~~~~~^^^^^^
KeyError: 'excel_sheet_selection'

I guess the change happened with the issue #168 that is using get_form_list most of the time except in the first line of get_cleaned_data_step:

    def get_cleaned_data_for_step(self, step):
        """
        Returns the cleaned data for a given `step`. Before returning the
        cleaned data, the stored values are revalidated through the form.
        If the data doesn't validate, None will be returned.
        """
        if step in self.form_list:  # <---- Based on form_list declaration while get_form will hit `self.get_form_list()[step]` later
            form_obj = self.get_form(

By looking at the doc, this behaviour was undefined (it doesn't say if it should return None or raise an exception), but I think it should be clarified, either through a more explicit/nicer exception ("excel_sheet_selection step not found in forms submitted") or by returning None as before.

I have an easy way to fix that (by checking myself if the form is in get_form_list) but that feels wonky since the lib was doing it for me before.

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

No branches or pull requests

1 participant