lolmaus / ember-cli-stained-by-children

Ember CLI addon: a mixin that makes the `isDirty` property of a record respect the dirtiness of its belongsTo/hasMany children
MIT License
25 stars 6 forks source link

A bug in clean-embedded-children #8

Open sergetab opened 9 years ago

sergetab commented 9 years ago

Found a bug in the clean-embedded-children.

After doing this

child.send('willCommit');
child.set('_attributes', {});
child.send('didCommit');

the child will become non-dirty, BUT all changed attributes will have old(original) value. So if, for example, you change a field again to it's previous value, it would not become dirty.

After some debugging, the right way to do it I think is

child.send('willCommit');
child._inFlightAttributes = child._attributes;
child._attributes = Ember.create(null);
child.send('didCommit');

That way after 'didCommit' all values from '_inFlightAttributes ' will be merged to '_data' automatically, so that values do update.

Also I think doing 'willCommit' and 'didCommit' for rollback is redundant.

lolmaus commented 9 years ago

Can you please PR a failing test case?

sergetab commented 9 years ago

Let me try to explain it in another way instead. It is quite simple really.

Attributes are stored in three places in Ember Data _data (saved data), _attributes (dirty attribute values), _inFlightAttributes (values that are being saved and about to be applied to _data).

In your code you just clear out _attributes like this

child.set('_attributes', {}). 

That way dirty values are lost. What is missing is

child._inFlightAttributes = child._attributes;

before that. If you assign _inFlightAttributes, didCommit will merge them back to _data.

lolmaus commented 9 years ago

Thank you for your explanation. But could you provide a sequence of steps of working with a StainedByChildren-powered model that result in the issue? I need a failing test case.

Mifrill commented 3 years ago

Hey guys @lolmaus , @sergetab , any updates here?

lolmaus commented 3 years ago

@Mifrill Hey comrade, this addon is entirely unmaintained and is likely to be incompatible with modern versions of Ember Data since it was relying on private API.

Mifrill commented 3 years ago

@lolmaus okay, thanks for the quick response, will deprecate this addon for our app then, thank you!