jazzband / django-formtools

A set of high-level abstractions for Django forms
https://django-formtools.readthedocs.io
BSD 3-Clause "New" or "Revised" License
794 stars 135 forks source link

Revisiting a form step does not refresh the return from storage.get_step_files(). #207

Open ShaheedHaque opened 2 years ago

ShaheedHaque commented 2 years ago

Let say you have a wizard with steps A, B, C... such that step A (for example) has an uploaded file. When step A is passed, form A is validated and the selected file is remembered by storage.get_step_files() in a cache called self._files, and now the current step is B.

If one navigates back to A, even if the selected file is changed, the cache is NOT refreshed because the logic for self._files look like this:

            if (step, field) not in self._files:
                self._files[(step, field)] = UploadedFile(
                    file=self.file_storage.open(tmp_name), **field_dict)

Since neither step nor field have changed, the cache is not refreshed!

The problem is most obvious when the physical name of the file changes, because the content of self.data[self.step_files_key][step] has the new file, but self.get_step_files(self.current_step) and thus current_step_files have the old file name.

I'm not 100% sure of the reason for the cache, but it seems to me that when a step is revisited, the cache MUST be cleared (even if the filename is the same, it might contain something new!!!).

ShaheedHaque commented 2 years ago

I suspect that clearing the cache, including deleting any previously stored files, would also address #191.

ShaheedHaque commented 2 years ago

An evolved version of the workaround from #191 that also caters for this problem is:

        for file in list(self.storage.current_step_files or []):
            self.storage._files.pop((self.steps.current, file))
            file = self.storage.data[self.storage.step_files_key][self.steps.current].pop(file)
            self.file_storage.delete(file['tmp_name'])
        #
        # Insert the above before the call to:
        #
        self.storage.set_step_files(self.steps.current, self.process_step_files(form))

Of course, putting the corresponding logic inside storage.set_step_files() would seem to be the right fix.