Skip to content

[WIP/RFC] use field default value if not allow_null #5816

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

Closed

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Feb 7, 2018

This is meant to be used with FK remote field lookups (source = 'foo.bar'), where foo is a model and bar is None there.

 serializers.CharField(source='foo.bar', default='default')

No existing tests seem to be failing, and new one would be needed.
Not really sure if this is going to fly, so holding back on that for now.

This is meant to be used with FK remote field lookups (`source =
'foo.bar'`), where `foo` is a model and `bar` is `None` there.

     serializers.CharField(source='foo.bar', default='default')
@rpkilby rpkilby self-assigned this Feb 7, 2018
@tomchristie
Copy link
Member

@carltongibson @blueyed Is this obsolete since 3.8?

@carltongibson
Copy link
Collaborator

Does it still pass if rebased on master?

I haven’t yet examined this one explicitly.

@carltongibson
Copy link
Collaborator

Looking then, this was resolved by #5849 I think. @blueyed please comment if not correct and we can review.

@arpit-draup
Copy link

arpit-draup commented Jun 17, 2019

@carltongibson

Deserialization of the nullable foreign key field throwing the issue of 'NoneType' object has no attribute 'foreign_field'. Have tried setting default as None.

Django RestFramwork Version: 3.8.0

@rpkilby
Copy link
Member

rpkilby commented Jun 17, 2019

Hi @arpit-draup. If you could provide a failing test case (e.g., as a PR), that would be helpful.

@arpit-draup
Copy link

@rpkilby I have found the workaround -
demo = CharField(source='demo.name', allow_null=True)
but I have one question if we have already set null=True and default None in the model then why this None check has been removed after DRF 3.6.4 - link

@rpkilby
Copy link
Member

rpkilby commented Jun 17, 2019

Could you provide more details? I'm not following what you're asking.

@arpit-draup
Copy link

arpit-draup commented Jun 17, 2019

@rpkilby Can you please check this link : Stackoverflow. I have mentioned in detail.

@xordoquy
Copy link
Collaborator

xordoquy commented Jun 17, 2019

if we have already set null=True and default None in the model

Except that since you override the generated field with an explicit field, whatever you did set on the model doesn't count.

@arpit-draup
Copy link

arpit-draup commented Jun 17, 2019

@xordoquy okay For serializer we have to set explicitly Is it? but before it wasn't like the same
Why this None check is removed : Link

Edit :
Serializer works on the model field right if the model has a particular property it should follow while deserializing or serializing the data Isn't it ?

@xordoquy
Copy link
Collaborator

xordoquy commented Jun 17, 2019

There has been some changes the way default values are handled for serialization. As far as I can tell it's been in the release notes.

@arpit-draup
Copy link

arpit-draup commented Jun 17, 2019

@xordoquy Okay but you understood the point, I am trying to make here?
Before if we have allowed the null in the model, serialization and deserialization worked perfectly without explicitly allowing null into the serializer.

@carltongibson
Copy link
Collaborator

Why this None check is removed...

There were a whole bunch of issues around #5849 here. Unfortunately the old behaviour led to incorrect results, particularly with dotted-sources and default handling. We were forced to adjust, for v3.8.

It gets pretty complex here. Without a failing test case in a separate PR it's not something we can think about changing easily.

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.

6 participants