Added failing test
Added test for https://github.com/danielspaniel/ember-data-change-tracker/issues/17
I was looking at your test. It seems like you are wanting to:
model.set(prop, 'A')
// YOU: please pretend that this is saved without having to save the model so it does not show up in changed attributes anymore
// ME: um .. err .. you realize this is an illegal procedure, in fact it is so illegal a procedure that ember data has NO PUBLIC METHOD to allow it.
// YOU: ok .. fine .. I will use private method to set a dirty state to clean and wipe out the changedAttributes.
// ME: uh .. why you doing this? me confused. there is a way to do it and i would have to make a special method in change tracker to make it happen. but realize that this is not a normal situation and is extra credit. so you going to have to explain to me why you need it, or figure out how to do the PR yourself and convince me to add it. or suffer with private method hacking .. or rethink what you are doing because it might actually be not the greatest idea in the first place.
// YOU: well .. I appreciate the use of the term "illegal procedure", reminds me of the good old days in medical school. But see here Dan, we really must get to the bottom of this. I thought your method model.saveChanges() was supposed to save the changes. Is this not so?
// ME: Well .. now that is a fine point my good man Lionel. But change tracker 'saveChanges' is different than your idea of save changes! Change tracker uses that method to save the current dirty state to its private tracking store. So that method is like 'rememberChanges'.
My use case is that after saving an appointment model, my API doesn't properly returns back one of the attributes (the status), so I need to set this status attribute myself. Then I'm using the savedChanges method so that the attribute is not flagged as changed (thought that's what this method was about no?). In the appointment booking route, when user leaves the journey before confirming, I'm rolling back the appointment model and its relationship, and I don't want the status to be set back to a wrong value.
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 model.saveChanges does NOT save the changes to ember-data model .. it saves the changes to tracker.
To accomplish what you want to achieve you could setup some code in the serializer to alter the payload ( add the status flag ).
I am not going to accept a PR that does what you want to achieve if that is to save changes to model without calling save because as I said before .. that is an illegal procedure.
But I can try and help you figure out an alternative ( more proper solution ) .. which the serializer idea is ( sort of )
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 saveChanges name method is very confusing then, as I suppose most people would expect it to save the changes to tracker and reinit the changed flags (which it does for relationships and other kind of attributes), while it doesn't.
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.