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

Include serialized model data when validating #8004

Closed
wants to merge 2 commits into from

Conversation

john-parton
Copy link
Contributor

@john-parton john-parton commented May 21, 2021

Description

Potential fix for: #7489

When an instance is passed to a ModelSerializer, we might need fields
that aren't specified in initial_data to run a validator. Specifically
for UniqueTogether

More details

There's potentially a backwards-incompatible change, that should at the very least be documented.

Previously there could be a dirty value in the database, and so long as you didn't try to touch it, it wouldn't cause re-validation.

For instance, suppose you have a validator on some_bad_field that prohibits "bad_value", and another unrelated field 'foo'.

# OK
instance = MyModel.objects.create(some_bad_field='bad_value')

# Old way didn't complain
serializer = MyModelSerializer(instance=instance, data={'foo': 'bar'})

# New way will re-validate ALL the fields, including some_bad_field, and fail
serializer = MyModelSerializer(instance=instance, data={'foo': 'bar'})

Whether that's desirable behavior or not should probably be discussed.

When an instance is passed to a ModelSerializer, we might need fields 
that aren't specified in initial_data to run a validator. Specifically 
for UniqueTogether
@john-parton
Copy link
Contributor Author

I found some tests that were failing with the previous change.

I reworked it so that this behavior is opt-in, but now the tests don't necessarily cover this case.

Is this a desirable change, or should I abandon this?

Still needs docs.

@stale
Copy link

stale bot commented Apr 28, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 28, 2022
@@ -124,6 +124,9 @@
'retrieve': 'read',
'destroy': 'delete'
},

# Validation
'VALIDATE_ENTIRE_INSTANCE': False,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think adding a new settings is a good idea at this stage of the project

@stale stale bot removed stale labels Dec 18, 2022
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

Successfully merging this pull request may close these issues.

None yet

3 participants