Skip to content

Commit

Permalink
rollback when is new model fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@danielspaniel
danielspaniel committed Apr 5, 2018
1 parent bc37697 commit 1186b1a
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
4 changes: 3 additions & 1 deletion addon/model-ext.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ Model.reopen({
*
*/
rollback() {
let trackerInfo = Tracker.metaInfo(this);
const isNew = this.get('isNew');
this.rollbackAttributes();
if (isNew) { return; }
let trackerInfo = Tracker.metaInfo(this);
let rollbackData = Tracker.rollbackData(this, trackerInfo);
let normalized = Tracker.normalize(this, rollbackData);
this.store.push(normalized);
Expand Down
11 changes: 10 additions & 1 deletion tests/unit/model-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { run } from '@ember/runloop';
import FactoryGuy, {
build, make, makeList, mockUpdate, mockFindRecord, mockReload,
build, make, makeNew, makeList, mockUpdate, mockFindRecord, mockReload,
mockDelete, manualSetup, mockCreate
} from 'ember-data-factory-guy';
import { initializer as modelInitializer } from 'ember-data-change-tracker';
Expand Down Expand Up @@ -571,6 +571,15 @@ module('Unit | Model', function(hooks) {
assert.equal(company.get('currentState.stateName'), 'root.loaded.saved');
assert.deepEqual(company.get('blob'), [1, 2, 3]);
});

test('new model', function(assert) {
let user = makeNew('user');
run(() => user.setProperties({name: 'FROO'}));
run(() => user.rollback());

assert.equal(user.get('isNew'), false);
assert.equal(user.get('name'), undefined);
});
});

module('#isDirty', function() {
Expand Down

5 comments on commit 1186b1a

@DuckoDeath
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but if you set a 'company', it will not be 'rolled back'.
As well, the user is new before the rollback, why is it not new after the rollback?

@danielspaniel
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ember data rollbackAttributes does the isNew back to false .. which is odd ( your right )
and yes, your right , the belongs to can't be rolled back. I could probably fix that ( but it would be hacky )
and I would have to see the use case your doing.
for example with a new record that you set up, if you want to throw it away .. don't call rollback call unloadRecord .. right ?

@DuckoDeath
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the rollbackAttributes definitely seems like an ember-data bug.

Our use case is odd for sure.
We have a 'new dialog' and we allow the user to 'clear' and start again (odd I know, I don't make the requirements). Could definitely discard the record and create a new one if that's the only way.

2nd case is a bit of a workaround as well.
If we have a record that has tried to save, and and the adapter returns an error, the error is in the 'errors' on the record (which is good). If the user then 'reverts' their change (puts the original value back in), the record is really no longer dirty and we are trying to 'rollback' to get rid of the errors (can't seems to find any ember-data way of clearing errors without saving again).

Thanks!

@danielspaniel
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For 1st case, I think discard and replace with new one is easiest answer
For case 2 , I am confused because would not rollback / rollbackAtrributes fix the errors? I have not used the ember data errors to show the user, because what I do is use validations ( like ember-cp-validations ) before they can even submit it to prevent the save even happening. That solves this issue for 99.9999% of the time.

@DuckoDeath
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for case 2, the errors are on fields which are not changed (ie, this field is required).
rollbackAttributes only rolls back things which have changed.

All of the that doesn't matter because rollbackAttributes is broken in my view (on a new record). It changes it to 'not new' and marks it as 'deleted', which i really don't understand. So in my scenario, I can't use it. Thanks!

Please sign in to comment.