validate files at dummy-instance level

sdobbelaere commented 8 months ago

Referring to issue: https://github.com/strawberry-graphql/strawberry-graphql-django/issues/468 This PR will validate the images at dummy-instance level.

Types of Changes

[ ] Core
[x] Bugfix
[ ] New feature
[ ] Enhancement/optimization
[ ] Documentation

Issues Fixed or Closed by This PR

Fix https://github.com/strawberry-graphql/strawberry-graphql-django/issues/468

Checklist

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[x] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document.
[x] I have added tests to cover my changes.
[x] I have tested the changes and verified that they work and don't break anything (as well as I can manage).

bellini666 commented 7 months ago

Hey @sdobbelaere , thanks a lot for this PR :)

The code looks correct to me, but I'm not sure if there's something missing based on @yergom 's comments.

What do you 2 think?

yergom commented 7 months ago

@bellini666 @sdobbelaere I think it's safe to be merged. Thanks for all the effort!

bellini666 commented 7 months ago

@sdobbelaere anything missing from this or can I merge and release it?

sdobbelaere commented 7 months ago

@sdobbelaere anything missing from this or can I merge and release it?

Should be OK, the tests look ok to me. We have one that tests with the file present, and actually returns the name. The opposite exists as well, on create we have a missing field for the file - which correctly raises an error on the model since the mutation doesn't have field.

Ideally / optionally we should have one on the update as well. When we try to set a file to None - that's probably not covered. But under normal circumstances, the model Integrity error should kick in.

strawberry-graphql / strawberry-django