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

refactor(dao) better full update handling in DAO and API #820

Merged
merged 1 commit into from
Dec 23, 2015

Conversation

thibaultcha
Copy link
Member

This is a refactor that has the benefit of fixing two issues with PUT requests.

PS: the schema validation thing is a total mess. PUT method can be
improved (see @todo annotations).

- Stronger schema validation and more edge-cases to validation during
  partial and complete update. With tests.
- base_dao `update()` now fetches the old entity and compare the changes
  before performing any update, allowing us to cover more edge-cases.
- Admin API can directly use the `self.params` property since the CRUD
  `update()` method does not need the old entity anymore.
- Fix for #765
- Fix for #720

PS: the schema validation thing is a total mess. PUT method can be
improved (see @todo annotations).
@thibaultcha thibaultcha modified the milestone: Jan 2016 Dec 23, 2015
@thibaultcha thibaultcha added pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) and removed pr/status/needs review labels Dec 23, 2015
thibaultcha added a commit that referenced this pull request Dec 23, 2015
refactor(dao) better full update handling in DAO and API
@thibaultcha thibaultcha merged commit 3da4dff into next Dec 23, 2015
@thibaultcha thibaultcha deleted the refactor/dao-update branch December 23, 2015 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant