django-rest-framework icon indicating copy to clipboard operation
django-rest-framework copied to clipboard
verified publisher icon encode

Propagate 'default' from model_field to serializer field

Open benesch opened this issue 4 years ago • 5 comments

Description

This is a resubmission of #8002 that will hopefully pass tests. The tl;dr is that field defaults are now propagated from models to model serializers.

Fix #7469.

benesch avatar Aug 13 '21 15:08 benesch

I'll close my issue.

john-parton avatar Aug 13 '21 17:08 john-parton

Just a note for discussion, part of the motivating factor for this change was to include the default value in the OPTIONS response. More info here: https://github.com/encode/django-rest-framework/discussions/8003

If this PR gets merged, I can rework 8003 as its own PR.

john-parton avatar Aug 13 '21 17:08 john-parton

Hi folks. 👋

So, if we did want to accept this, then I'd suggest that this pull request should also update the OPTIONS responses in order to include the default - https://github.com/encode/django-rest-framework/blob/master/rest_framework/metadata.py#L115

Staying in line with how we generate 'default' for schemas would make sense... https://github.com/encode/django-rest-framework/blob/6ea95b6ad1bc0d4a4234a267b1ba32701878c6bb/rest_framework/schemas/openapi.py#L535-L536

I'm a bit reluctant, simply because at this point in it's lifecycle almost any change in REST framework ends up having unintended side-effects for existing users, and I could easily see this being the case for this change. We've used "default on the model => not required on the serializer" since ~forever.

On the other hand, exposing default values on the schema and OPTIONS responses does seem like an improvement.

lovelydinosaur avatar Oct 06 '21 13:10 lovelydinosaur

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[bot] avatar Apr 17 '22 04:04 stale[bot]

I think this may also fix #7489 which is still current.

peterthomassen avatar Aug 25 '22 22:08 peterthomassen

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[bot] avatar Jan 22 '23 05:01 stale[bot]

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[bot] avatar Mar 25 '23 03:03 stale[bot]

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[bot] avatar Jun 18 '23 11:06 stale[bot]

Anyone should feel free to take over this PR. I’m not going to have time myself.

benesch avatar Jun 18 '23 13:06 benesch

@ashupednekar Do you have time for this?

(Asking due to https://github.com/encode/django-rest-framework/issues/7489#issuecomment-1556159800)

peterthomassen avatar Jun 19 '23 11:06 peterthomassen

@samims you can also check this out

auvipy avatar Jun 19 '23 13:06 auvipy

@auvipy I have made the changes requested in this PR. default fields propagated from model to serializers.

  1. Generate 'default' for serializer as generated for open API schema.
  2. Updated OPTIONS response to accommodate the default field there. Created Testcase to check default is generated for OPTIONS response for different fields.

I have created PR for this #9030 I can't update this PR as I don't have permission.

rnshaikh avatar Jul 01 '23 13:07 rnshaikh