Better handle conflicting keys in set_values by tienne-B · Pull Request #7671 · encode/django-rest-framework (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation4 Commits1 Checks0 Files changed

Conversation

This file contains hidden or 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

[ Show hidden characters]({{ revealButtonHref }})

tienne-B

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')

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.

@tienne-B

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

@tienne-B

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.

@kevin-brown

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.

@tienne-B

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.

@tomchristie

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

@tienne-B

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.

@tienne-B

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

@tienne-B

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.

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

@tienne-B @saadullahaleem

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.

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

@tienne-B @saadullahaleem

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.

These tests follow the examples given in the method.

auvipy added a commit that referenced this pull request

May 29, 2023

Django URLs of the documentation of reverse and reverse_lazy were wrong.

Co-authored-by: Jorn van Wier jorn.van.wier@thunderbyte.ai Co-authored-by: Asif Saif Uddin auvipy@gmail.com

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.

These tests follow the examples given in the method.

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.

These tests follow the examples given in the method.

Co-authored-by: Sergei Shishov sshishov@users.noreply.github.com

Co-authored-by: Sergei Shishov sshishov@users.noreply.github.com

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