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

Make nullable fields non-required in creation endpoints #240

Open
swrichards opened this issue Sep 10, 2024 · 2 comments · May be fixed by #332
Open

Make nullable fields non-required in creation endpoints #240

swrichards opened this issue Sep 10, 2024 · 2 comments · May be fixed by #332
Assignees
Labels
Milestone

Comments

@swrichards
Copy link
Contributor

Thema / Theme

Klantinteracties API

Omschrijving / Description

The request here is to make all nullable fields in resource creation endpoints non-required.

Whenever fields are marked both as required and as {type} or null in creation endpoints, the field has to be explicitly included in the payload and set to null. If the field is omitted, a validation error is given.

For example, voorkeursDigitaalAdres in partijenCreate is defined in this way (required, but also accepts null in lieu of an object):

Screenshot from 2024-09-10 09-53-45

Omitting the field from a create payload yields:

Invalid parameters:
{'code': 'required',
 'name': 'voorkeursDigitaalAdres',
 'reason': 'Dit veld is vereist.'}

Which is fixed by adding:

"voorkeursDigitaalAdres" : null

If a field is nullable, then null is a sane default value that the API can set for the client, without having to explicate the fields in the payload.

Toegevoegde waarde / Added value

The current behavior is undesirable because it makes constructing requests very verbose and generally raises the friction of building a valid request. This is especially visible in tests, where the current approach leads to the need for a lot of factories and fixtures. I would say providing sane defaults wherever possible makes for a better developer experience.

Aanvullende opmerkingen / Additional context

No response

@alextreme
Copy link
Member

@Hugo-ter-Doest

@swrichards
Copy link
Contributor Author

Discussed this with @danielmursa-dev . To clarify: the issue here specifically refers to creation endpoints. This in contrast to PUT where, if an optional field is absent, you have two possible behaviors:

  1. Interpret the missing optional field as "reset to None"
  2. Interpret the missing optional field as "keep the existing value"

The default DRF behavior seems to be 2. Personally, I think this is not correct behavior: PUT effectively means "replace the resource's state to the provider body", which I think further means optional fields would behave as they would with a POST (PUT should also be idempotent, which it would not be if other clients could PATCH the optional field in the meantime`).

But that's a discussion for another day: for now, we agreed to keep the existing behavior (2) and only default to None in creation endpoints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

4 participants