Better handle conflicting keys in set_values #7671
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit adds logic to set_values to move values on keys to be added
upon to a blank key inside the value's previous key. This would allow
constructions like:
```
fk_object = HyperlinkedRelatedField(source='fk')
fk_name = CharField('fk.field')
```
Tests are also added specifically for set_values with the examples given
in the function description as well as for this change.
tienne-B
added a commit
to tienne-B/tabbycat
that referenced
this pull request
Dec 24, 2020
As was planned in the specification of the API, Motions are now by Tournament through it's own Tournament field, but requiring no change in the schema. Depends on encode/django-rest-framework#7671 to have full abilities on the 'Motions' field of the rounds.
|
This seems... wrong. While a blank key is a valid use case, it seems like you're trying to support having multiple fields writing to different levels of the same target which would open up a mess of edge cases. And writing to an empty key would break the existing expected functionality. |
|
Yes, I am trying to have multiple fields going to the same target, where I want to be able to either change which object is the foreign key, or change fields of that foreign key. The worry would be that if the FK is changed, but the other fields are left as-is, the new FK object's fields would be modified. I don't know about a "mess of edge cases" or the current expected functionality; but I added tests for the function to demonstrate the functionality I expect and kept all existing tests passing. |
|
Going to close this off as insufficiently clear. |
tienne-B
added a commit
to tienne-B/django-rest-framework
that referenced
this pull request
May 20, 2021
As an alternative to encode#7671, let the method be overridden if needed. As the function is only used for serializers, it has a better place in the Serializer class.
|
It has been a while, but I am a little disheartened by how my PR was treated. What could I have said to clarify this PR? I am lost with the comment about opening up edge cases (like what?) and am disappointed my reply didn't get any follow-up. |
auvipy
pushed a commit
that referenced
this pull request
May 24, 2023
* Make set_value a static method for Serializers As an alternative to #7671, let the method be overridden if needed. As the function is only used for serializers, it has a better place in the Serializer class. * Set `set_value` as an object (non-static) method * Add tests for set_value() These tests follow the examples given in the method.
saadullahaleem
pushed a commit
to saadullahaleem/django-rest-framework
that referenced
this pull request
May 27, 2023
* Make set_value a static method for Serializers As an alternative to encode#7671, let the method be overridden if needed. As the function is only used for serializers, it has a better place in the Serializer class. * Set `set_value` as an object (non-static) method * Add tests for set_value() These tests follow the examples given in the method.
saadullahaleem
pushed a commit
to saadullahaleem/django-rest-framework
that referenced
this pull request
May 27, 2023
* Make set_value a static method for Serializers As an alternative to encode#7671, let the method be overridden if needed. As the function is only used for serializers, it has a better place in the Serializer class. * Set `set_value` as an object (non-static) method * Add tests for set_value() These tests follow the examples given in the method.
auvipy
added a commit
that referenced
this pull request
May 29, 2023
* fix: Make the instance variable of child serializer point to the correct list object instead of the entire list when validating ListSerializer * fix formatting issues for list serializer validation fix * fix imports sorting for list serializer tests * remove django 2.2 from docs index (#8982) * Declared Django 4.2 support in README.md (#8985) * Fix Links in Documentation to Django `reverse` and `reverse_lazy` (#8986) * Fix Django Docs url in reverse.md Django URLs of the documentation of `reverse` and `reverse_lazy` were wrong. * Update reverse.md * fix URLPathVersioning reverse fallback (#7247) * fix URLPathVersioning reverse fallback * add test for URLPathVersioning reverse fallback * Update tests/test_versioning.py --------- Co-authored-by: Jorn van Wier <jorn.van.wier@thunderbyte.ai> Co-authored-by: Asif Saif Uddin <auvipy@gmail.com> * Make set_value a method within `Serializer` (#8001) * Make set_value a static method for Serializers As an alternative to #7671, let the method be overridden if needed. As the function is only used for serializers, it has a better place in the Serializer class. * Set `set_value` as an object (non-static) method * Add tests for set_value() These tests follow the examples given in the method. * fix: Make the instance variable of child serializer point to the correct list object instead of the entire list when validating ListSerializer * Make set_value a method within `Serializer` (#8001) * Make set_value a static method for Serializers As an alternative to #7671, let the method be overridden if needed. As the function is only used for serializers, it has a better place in the Serializer class. * Set `set_value` as an object (non-static) method * Add tests for set_value() These tests follow the examples given in the method. * fix: Make the instance variable of child serializer point to the correct list object instead of the entire list when validating ListSerializer * fix: Make the instance variable of child serializer point to the correct list object instead of the entire list when validating ListSerializer * fix formatting issues for list serializer validation fix * fix: Make the instance variable of child serializer point to the correct list object instead of the entire list when validating ListSerializer * fix formatting issues for list serializer validation fix * fix linting * Update rest_framework/serializers.py Co-authored-by: Sergei Shishov <sshishov@users.noreply.github.com> * Update rest_framework/serializers.py Co-authored-by: Sergei Shishov <sshishov@users.noreply.github.com> * fix: instance variable in list serializer, remove commented code --------- Co-authored-by: Mathieu Dupuy <deronnax@gmail.com> Co-authored-by: Mehraz Hossain Rumman <59512321+MehrazRumman@users.noreply.github.com> Co-authored-by: Dominik Bruhn <dominik@dbruhn.de> Co-authored-by: jornvanwier <mail@jornvanwier.com> Co-authored-by: Jorn van Wier <jorn.van.wier@thunderbyte.ai> Co-authored-by: Asif Saif Uddin <auvipy@gmail.com> Co-authored-by: Étienne Beaulé <beauleetienne0@gmail.com> Co-authored-by: Sergei Shishov <sshishov@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds logic to set_values to move values on keys to be added upon to a blank key inside the value's previous key. This would allow constructions like:
This is important for my use-case to allow linking to an existing object, or creating/modifying it from a related field's serializer.
Tests are also added specifically for set_values with the examples given in the function description as well as for this change.