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

Refresh model attributes after a save operation #2012

Merged
merged 13 commits into from Sep 4, 2019

Conversation

@ricardograca
Copy link
Member

commented Sep 2, 2019

Introduction

Modify Model#save() so that it returns a fresh copy of the model from the database after a successful save.

Motivation

It is often necessary to get all the attributes from the database immediately after a save. The current behavior is also not very intuitive, where after a save some attributes will be set but not others. For example, the timestamp columns might be missing entirely if the user didn't set them manually (#507).

This should lead to a much better experience when working with models, since the model will always have the most up-to-date attributes without needing to manually call Model#refresh.

Fixes #507. Closes #1665.

Proposed solution

This will call Model#refresh after a successful save on databases that do not support the RETURNING statement. On those that do a more efficient single INSERT OR UPDATE query will be generated allowing this feature to have negligible performance impact in that case.

Note that the after save event signatures were changed to make them more consistent. Namely the second argument was removed since it wasn't very useful in any case (for update events it was always 1 and for inserts it was [1] or whatever the new inserted id was). Nonetheless this is a small breaking change, so a migration guide has been added.

Current PR Issues

Requires two queries on database that do not support RETURNING, but there's no way around that.

ricardograca added 10 commits Sep 2, 2019
- This makes them more accurate when working with MySQL that doesn't 
support millisecond timestamp columns by default when used with Knex.
- This will only be used if it's present and the unparsed idAttribute is 
not.
- Previously any method that called _doFetch() would get the previous 
attributes reset, but we don't want to do this when calling refresh from 
the save method.
@ricardograca ricardograca added the feature label Sep 2, 2019
ricardograca added 3 commits Sep 3, 2019
- With the recent change of refreshing the model after save both the 
`this` and `updatedModel` were the same thing. 

- This change also makes all the "after" save events more consistent 
since the arguments are all the same. 

- Finally, the previous values returned as the second argument just 
weren't very useful and the id of the inserted model is available in the 
updated model, so no data is lost.
@ricardograca ricardograca changed the title WIP: Refresh model attributes after a save operation Refresh model attributes after a save operation Sep 3, 2019
@ricardograca ricardograca added the change label Sep 3, 2019
@ricardograca ricardograca merged commit e717863 into master Sep 4, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ricardograca ricardograca deleted the rg-refresh-model branch Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.