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

[Test]: ember-data nested set #595

Closed
wants to merge 1 commit into from
Closed

[Test]: ember-data nested set #595

wants to merge 1 commit into from

Conversation

snewcomer
Copy link
Collaborator

@snewcomer snewcomer commented May 17, 2021

ref #592

ref #594

@snewcomer snewcomer self-assigned this May 17, 2021
@snewcomer
Copy link
Collaborator Author

@flynnawtc Let me know how I can improve this test to make it fail!

@flynnawtc
Copy link

Hi @snewcomer
I really appreciate your taking an interest in this.
Here is an updated test that tries all the various means of accessing the properties of the nested object. Some of these fail.
My motivation for following this up, is to use ember-power-select on forms using ember-data models and changesets.

  test('#set Ember.set with Ember Data Object actually does work TWICE for nested', async function (assert) {
    let store = this.owner.lookup('service:store');

    let mockProfileModel = store.createRecord('profile');
    let mockUserModel = store.createRecord('user', {
      profile: mockProfileModel,
      save: function () {
        return Promise.resolve(this);
      },
    });

    let dummyChangeset = Changeset(mockUserModel);
    let pet1 = store.createRecord('dog', { breed: 'jazzy' });
    let pet2 = store.createRecord('dog', { breed: 'hands' });

    set(dummyChangeset, 'profile.pet', pet1);

    assert.equal(dummyChangeset.profile.pet.breed, 'jazzy', 'should have new change');
    assert.equal(dummyChangeset.get('profile.pet.breed'), 'jazzy', 'should have new change using get');
    assert.equal(dummyChangeset.get('profile.pet').breed, 'jazzy', 'should have new change using get-object.property');
    assert.equal(dummyChangeset.get('profile.pet').get('breed'), 'jazzy', 'should have new change using get-object.get-property');
    let p1 = dummyChangeset.get('profile.pet');
    assert.ok(p1 !== null);
    assert.equal(p1.breed, 'jazzy', 'got object should have new change');
    assert.equal(p1.get('breed'), 'jazzy', 'got object should have new change using get-property');

    p1 = dummyChangeset.profile.pet;
    assert.ok(p1 !== null);
    assert.equal(p1.breed, 'jazzy', 'object should have new change');
    assert.equal(p1.get('breed'), 'jazzy', 'object should have new change using get-property');


    let changes = dummyChangeset.changes;
    assert.equal(changes[0].value.breed, 'jazzy', 'changes with nested key Ember.set');

    set(dummyChangeset, 'profile.pet', pet2);

    assert.equal(dummyChangeset.profile.pet.breed, 'hands', 'should have new change');
    assert.equal(dummyChangeset.get('profile.pet.breed'), 'hands', 'should have new change using get');
    assert.equal(dummyChangeset.get('profile.pet').breed, 'hands', 'should have new change using get-object.property');
    assert.equal(dummyChangeset.get('profile.pet').get('breed'), 'hands', 'should have new change using get-object.get-property');
    let p2 = dummyChangeset.get('profile.pet');
    assert.ok(p2 !== null);
    assert.equal(p2.breed, 'hands', 'got object should have new change');
    assert.equal(p2.get('breed'), 'hands', 'got object should have new change using get-property');

    p2 = dummyChangeset.profile.pet;
    assert.ok(p2 !== null);
    assert.equal(p2.breed, 'hands', 'object should have new change');
    assert.equal(p2.get('breed'), 'hands', 'object should have new change using get-property');

    changes = dummyChangeset.changes;
    assert.equal(changes[0].value.breed, 'hands', 'changes with nested key Ember.set');

    dummyChangeset.execute();

    assert.equal(get(mockUserModel, 'profile.pet.breed'), 'hands', 'has new property');
  });

Hope this makes sense.
Adrian

@snewcomer
Copy link
Collaborator Author

You are right. I'm thinking there are two issues at play.

  1. async vs sync relationships. I imagine the latter is easier.
  2. properties on an ember data model return hasOwnProperty('breed') === false. See here for reference.

The get with the full path should work every time. But the cases you have laid out .get('profile.pet').breed might not.

To fix 2, I wonder if we should try accessing the property instead of checking hasOwnProperty 🤔. I don't remember why I did it that way.

@flynnawtc
Copy link

Because we use RESTAdapter rather than JSONAPIAdapter to suit our back-end, we have never needed to consider sync/async as we retrieve 'fully populated' object models from the server each time.

I'm afraid you're gone beyond me when you delve in to the workings of an ember-data model. It's something I have never really looked at.

The problems have arisen when building forms with a selection box that, say, allows a user to choose a pet for the user.
The user model becomes the basis for the changeset.
The list of pets is passed to ember-power-select for display to the user.
=> we are dealing with nested properties (profile.pet)

I have added some sample usages to my test project https://github.com/flynnawtc/cs-test if you have a moment to take a look.

The documented recommendation to use changeset-get for nested properties does not work, while Ember-get seems to work for ember data models. When using regular Javascript objects, then both methods work.
This was my initial starting point which resulted in the tests above.

In my production code I have now worked around the issue as per testselect2 in the example but it would probably be worthwhile to figure out why testselect is failing.

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

Successfully merging this pull request may close these issues.

2 participants