-
Notifications
You must be signed in to change notification settings - Fork 47
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
Added failing test #18
base: master
Are you sure you want to change the base?
Conversation
I was looking at your test. model.set(prop, 'A') |
My use case is that after saving an Let me know if you want me to make a PR with the code change. Thanks |
That is the point I was trying to make Lionel. The |
yes I can add it in the serializer. I don't want to save my model as it would send another request, just want to save the changes to tracker. If you think a PR is not the right thing to do then forget it. But it feels like the |
you are very right about that Lionel, that name is 100% confusing. I need to change it from 'saveChanges' to => resetTrackerState ( or something like that.) because it only saves the current changes to tracker. It does not clear the changed attributes for the model. It does not reinit relationships or any attributes as you are thinking ) /**
* Save change tracker attributes
*
* @param {DS.Model} model
*/
static saveChanges(model) {
let metaInfo = this.metaInfo(model);
Object.keys(metaInfo).forEach((key) => {
Tracker.saveKey(model, key); // saves changes to tracker ( and that is all it does )
});
} |
v0.7.4 adds the name saveTrackerChanges to the model so model.saveChanges() is now => model.saveTrackerChanges() not sure if this helps but let me know if it does and i will close this PR |
it doesn't, did you try adding the test to master? |
I did not add that test @Leooo , I think I am still confused as to what that test is supposed to show. |
Added test for #17