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

django from_form does not work with filefields #4093

Open
arjoonn-s opened this issue Aug 26, 2024 · 3 comments · May be fixed by #4121
Open

django from_form does not work with filefields #4093

arjoonn-s opened this issue Aug 26, 2024 · 3 comments · May be fixed by #4121
Labels
interop how to play nicely with other packages

Comments

@arjoonn-s
Copy link

arjoonn-s commented Aug 26, 2024

The from_form function does not bind files correctly.

From the source available I think it should be supplying file fields separately. Currently it is running:

    return _forms_impl(
        st.builds(
            partial(form, **form_kwargs),  # type: ignore
            data=st.fixed_dictionaries(field_strategies),  # type: ignore
        )
    )

I think it should have been something like:

    return _forms_impl(
        st.builds(
            partial(form, **form_kwargs),  # type: ignore
            data=st.fixed_dictionaries(field_strategies),  # type: ignore
            files=st.fixed_dictionaries(field_strategies),  # type: ignore
        )
    )

cc @sethuvishal


Is this expected? If not I think we're happy to submit a patch for this.

@HypothesisWorks HypothesisWorks deleted a comment Aug 26, 2024
@HypothesisWorks HypothesisWorks deleted a comment Aug 26, 2024
@Zac-HD Zac-HD added the interop how to play nicely with other packages label Sep 13, 2024
@Zac-HD
Copy link
Member

Zac-HD commented Sep 13, 2024

Hmm, unfortunately none of Hypothesis' regular contributors are Django users, so it's pretty plausible that we've missed an upstream change or something like that. I'd be happy to accept a patch, but would appreciate more-detailed-than-usual notes pointing to Django docs etc to explain what's going on 🙂

@arjoonn-s
Copy link
Author

More than happy to help! Hypo has been of great use to us in testing our django codebase.

  1. We mainly use hypo with django forms so I'll only talk about that.
  2. The forms are initialized as form = MyForm(data=request.POST, files=request.FILES) ref
  3. After this we can do form.is_valid() to check if the form is valid.

The problem arises when we have FileField in the form for handling file uploads.

  1. Hypo generates file data correctly, but does not pass it to the form class correctly as seen in the source.
  2. Since the file field is passed as data, the content of the file never reaches the django form.
  3. This causes it to treat the file as an empty file and so fail validation where we have added rules for ensuring that file should be non-empty.

The expected code would be to have the hypo code in the referred section as:

return _forms_impl(
        st.builds(
            partial(form, **form_kwargs),  # type: ignore
            data=st.fixed_dictionaries(non_file_field_strategies),  # type: ignore
            files=st.fixed_dictionaries(file_field_strategies),  # type: ignore
        )
    )

thus passing the strategies correctly to the form builder.

Does that make more sense? I'm happy to answer any other questions.

@Zac-HD
Copy link
Member

Zac-HD commented Sep 13, 2024

Sounds great!

@theSage21 theSage21 linked a pull request Sep 30, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop how to play nicely with other packages
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants