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

Support null in content PATCH to delete a field value #187 #513

Merged
merged 5 commits into from Mar 13, 2018

Conversation

Projects
None yet
4 participants
@csenger
Contributor

csenger commented Mar 12, 2018

This fixed #187

@coveralls

This comment has been minimized.

coveralls commented Mar 12, 2018

Coverage Status

Coverage increased (+0.004%) to 96.526% when pulling 9087c1d on dx-null-value-support into ada28b9 on master.

@tisto tisto requested a review from lukasgraf Mar 12, 2018

tisto added some commits Mar 13, 2018

@tisto tisto self-requested a review Mar 13, 2018

@tisto

tisto approved these changes Mar 13, 2018

LGTM! I would like @lukasgraf to have a second look though if possible.

@lukasgraf

Just two minor comments, otherwise looks good to me. 👍

I pretty much had to make my peace with the fact that handling of default values and missing values is somewhat broken in Plone itself, and there's only so much we can do about that in plone.restapi.

Resetting the field to missing_value and documenting it as such feels like the least bad thing to do, and is what would meet people's expectations the most I would think.

),
'Setting it to null is not allowed.'
),
'error': None})

This comment has been minimized.

@lukasgraf

lukasgraf Mar 13, 2018

Member

Hmm, 'error': None feels a bit clunky. I don't have a clear suggestion in mind though.

PATCH allows to provide just a subset of the resource
(the values you actually want to change).
If you send the value `null` for a field, the field's content will be

This comment has been minimized.

@lukasgraf

lukasgraf Mar 13, 2018

Member

In reStructuredText these need to be double backticks (``) for code literals, single backticks are just emphasis 😉

@tisto tisto merged commit 08fa498 into master Mar 13, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.004%) to 96.526%
Details

@tisto tisto deleted the dx-null-value-support branch Mar 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment