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

model.rollbackAttributes with errors doesn't clear hasDirtyAttributes #3499

Closed
smcclstocks opened this issue Jul 6, 2015 · 5 comments
Closed

Comments

@smcclstocks
Copy link

Steps to reproduce:

  1. dirty a model in a way that causes validation errors
  2. save
  3. rollback
  4. model.hasDirtyAttributes == true

If I call model.rollbackAttributes() twice hasDirtyAttributes == false. Also, my models value do appear to be reset.. meaning the changes are reverted but hasDirtyAttributes reflects incorrectly.

DEBUG: -------------------------------
ember.debug.js:5361DEBUG: Ember : 1.13.2
ember.debug.js:5361DEBUG: Ember Data : 1.13.4
ember.debug.js:5361DEBUG: jQuery : 1.11.3
ember.debug.js:5361DEBUG: Ember Simple Auth : 0.8.0
ember.debug.js:5361DEBUG: Ember Simple Auth Cookie Store : 0.8.0
ember.debug.js:5361DEBUG: Ember Simple Auth OAuth 2.0 : 0.8.0
ember.debug.js:5361DEBUG: -------------------------------

@wecc
Copy link
Contributor

wecc commented Jul 7, 2015

Closing as a dupe of #3498, please feel free to reopen if I'm mistaken.

@wecc wecc closed this as completed Jul 7, 2015
@smcclstocks
Copy link
Author

I'm not smart enough with ember data to know. If #3498 is resolved & model.hasDirtyAttributes is still true then I'll reopen. I guess I can't really know until that one is fixed.

@RobIsHere
Copy link

I had some quality time with ember data internals today. Also new to the low level ember data stuff ;)

I'm experiencing this issue here while not having problems with attribute errors like detailed in #3498. In the debugger i see they are cleared. But hasDirtyAttributes is not.

The state changes after saving an invalid value that is rejected by the api:
invalid
uncommitted (after first call to rollbackAttributes)
loaded.saved (after second call to rollbackAttributes)

I'm just figuring out how everything works. My opinion is: The bug is around line 345 of states.js.

rolledBack: function (internalModel) {
  internalModel.clearErrorMessages();
  internalModel.triggerLater('ready');
},

becameValid: function (internalModel) {
  internalModel.transitionTo('uncommitted');
},

rolledBack is called, errors are cleared and a call to becameValid is triggered by that.
This moves the model into uncommitted state. For becameValid alone this behavior makes sense, there could be edited values that are valid.

That's the reason why it stays dirty: uncommitted = dirty

After rollback keeping it dirty makes no sense.
Maybe changing rolledBack could fix it? Like:

rolledBack: function (internalModel) {
  internalModel.clearErrorMessages();
  internalModel.transitionTo('loaded.saved');
  internalModel.triggerLater('ready');
},

That is what the first plus second rollback does put in one place. Trigger the transition to loaded.saved.

@wecc
Copy link
Contributor

wecc commented Jul 9, 2015

I was probably too quick to judge this as a dupe of #3498.

@wecc wecc reopened this Jul 9, 2015
@tchak
Copy link
Member

tchak commented Jul 12, 2015

Sounds like @RobIsHere is right, I can see what is happening here. I will try to work on a fix tomorrow morning.

tchak added a commit to tchak/data that referenced this issue Jul 17, 2015
tchak added a commit to tchak/data that referenced this issue Jul 18, 2015
tchak added a commit to tchak/data that referenced this issue Jul 18, 2015
tchak added a commit to tchak/data that referenced this issue Jul 18, 2015
tchak added a commit to tchak/data that referenced this issue Jul 19, 2015
bmac pushed a commit to bmac/data that referenced this issue Jul 21, 2015
bmac pushed a commit that referenced this issue Jul 22, 2015
jcope2013 pushed a commit to jcope2013/data that referenced this issue Aug 3, 2015
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

No branches or pull requests

4 participants