Fix: changeset.set('prop', undefined); does not update prop

[jelhan]

changeset.set('prop', undefined) does not change value.

Please not that #190 is needed to fix this if skipValidate is true. Since without #190 oldValue is always undefined, condition isEqual(oldValue, value) will always be true if value is undefined and therefore change will not be executed.

[jelhan]

I'm not quite sure why Ember.set fails to update value. I think changing property directly might be okay since we call this.notifyPropertyChange(CHANGES); later.

[BorigTheDwarf]

+1

[snewcomer]

@jelhan Is this still a problem for ya? We could put in a fix today (sry it's been so long)

[jelhan]

Don't have the time today. I think we should check if this scenario is already tested in master. If not, we could apply the test in this PR. If it fails, we might use the fix provided by this PR. Will try to do it over the weekend.

[jelhan]

Current code base is using provided patch. After a lot of refactoring the code line now lives at: https://github.com/poteto/ember-changeset/blob/master/addon/utils/set-nested-property.js#L41

After setting a property to undefined, changes contains the changed property. So original issue is fixed. But another one is added. change object does not contain a changed property with a value of undefined.

Simple reproduction:

changeset = new Changeset({ prop: 'foo' });
changeset.set('prop', undefined);
changeset.get('changes'); // [{ key: 'prop', value: undefined }]
changeset.get('change'); // {}

This PR now provides additional tests to cover both. Test for change property is failing.

[jelhan]

Reported the issue regarding change upstream: poteto/ember-deep-set#14

[algodave]

I have an address model with a belongsTo('state') association. I create a changeset from it. In my edit address form the following happens:

• when user selects a different state, it is correctly set in the changeset
• when user selects a different country, the state in the changeset should be unset; I tried setting it to either undefined or null, but none of them worked: the old state model stays there in the changeset!

In both cases, I'm using ember-changeset's overridden set method rather than Ember's standard set. I'm using a50fc86 in my project.

[jelhan]

@algodave You could try using this pull request for ember-deep-set. You could do so by selective dependency resolutions feature of yarn. The pull request should fix this issue. Please report your findings here.

[algodave]

Thanks @jelhan will do!

[snewcomer]

@jelhan Looks like a minor merge conflict here. ember-deep-set 0.1.4 was published fyi!

[jelhan]

Fixed merge conflicts, which were introduced by my own PR. 😆 Also updated ember-deep-set@0.1.4. Don't get confused by travis passing even without that update. It uses yarn install --no-lockfile.

[snewcomer]

Awesome work! Love it 💯