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

Fix #377, #378: Allow dropping edited resource properties. #379

Merged
merged 1 commit into from
Feb 2, 2017

Conversation

n1k0
Copy link
Contributor

@n1k0 n1k0 commented Jan 31, 2017

Refs #377, #378. Deployed to gh-pages.

r? @leplatrem

@n1k0 n1k0 requested a review from leplatrem January 31, 2017 17:22
@n1k0 n1k0 force-pushed the 377-remove-record-property branch from a249d84 to be155ce Compare February 1, 2017 07:28
Copy link
Contributor

@leplatrem leplatrem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit radical and I'm not sure it is the right behaviour (fields can be lost, and collection review data cannot be removed so it may fail). See #126, #256, #259

We have several strategies here...

  • Do a PUT for every form that allows to exist raw JSON or exhaustive properties. Leave a PATCH where we edit a partial list of fields (collection metadata, record schema...)
  • Use JSON merge header, that allows to remove fields by setting their value to null
  • Merge the state (previously obtained attributes) with the form data and remove fields whose value is explicitly set to null...

@n1k0
Copy link
Contributor Author

n1k0 commented Feb 1, 2017

(fields can be lost, and collection review data cannot be removed so it may fail)

I couldn't reproduce the loss of fields, can you point me at a reproducible scenario?

@leplatrem
Copy link
Contributor

can you point me at a reproducible scenario?

Indeed, I made a quick test and it seems to behave properly on collection metadata. How do we manage the difference between attributes that must removed versus. attributes that are missing from data and must remain intact ?

@n1k0
Copy link
Contributor Author

n1k0 commented Feb 1, 2017

How do we manage the difference between attributes that must removed versus. attributes that are missing from data and must remain intact ?

It's because initial collection data are passed as formData to the form, with preexisting fields kept untouched though submitted, so they're sent as well. Dropping a value by clearing a text field works because we updated to rjsf v0.42.0 which supports this new feature.

@leplatrem
Copy link
Contributor

@n1k0 n1k0 merged commit 1e18b07 into master Feb 2, 2017
@n1k0 n1k0 deleted the 377-remove-record-property branch February 2, 2017 07:59
n1k0 added a commit that referenced this pull request Feb 16, 2017
New features

* Fix #377, #378: Allow dropping edited resource properties. (#379)
* Fix #365: Render a JSON diff for history entries. (#380)
* Fix #376: Denote readonly buckets & collections in the sidebar. (#382)
* Fix #384: Live-searchable/filterable sidebar entries. (#385)
* Hide auth method selector when a single one is configured.

Bugfixes

* Do not store passwords. Fixes #364 (#386)
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.

2 participants