[Model Fragments 5.0] Composing fragments with `FactoryGuy#make/store#push` result in their properties being `undefined`

[patocallaghan]

I'm opening this here but to be honest I'm not sure if the issue is in the Model Fragments RecordData refactor or the Ember Data upgrade (considering we need to upgrade to Ember Data 3.16).

Background

The Factory Guy addon gives you a set of APIs to bootstrap your models in tests. The make helper allows you to push records directly into the store rather than need to use the push, pushPayload or createRecord APIs to bootstrap your models. A common pattern when using make is to compose your test models with other models created using make. For example take the following person Ember Data model which contains a fragment:

// models/user.js
export default class User extends Model {
  @fragment('info') info
}

// models/info.js
export default class Info extends Fragment {
  @attr name
}

This can be created in a test by doing the following (once you've created your factories correctly). Here we create a user model and pass it an info fragment model.

let user = make('user', 
  info: make('info'),
});

The problem

It appears passing a fragment to a model like above causes the properties of info to be undefined rather than the values which were originally on the model.

console.log(info.name); //=> "Jon Snow" 
console.log(user.info.name); //=> undefined

Strangely, if you pass a model to another model, or a fragment to another fragment it works correctly. It specifically seems to be the case where you pass a fragment to a model which isn't working. I've pushed a draft PR here which has a failing test (and some passing ones) to illustrate the problem.

Potential bug

The problem seems specifically to be an issue all the way down in EmberObject when we are pushing the _internalModel property on the object to create the model instance. Looking at the screenshot below, the passed in props have the correct value, but once the model is created they are "lost" and come back as undefined.

var instance = this.class.create(props);

And just to clarify, the failing test in the PR passes on the commit before the RecordData refactor.

/cc @igorT @richgt

[patocallaghan]

Just an update, I checked and this issue is still present when using Ember Data 3.13.0 so it's more than likely specifically related to the Model Fragments refactor and not Ember Data.

Looking at the commits between Ember Data 3.12.3 (which works with previous model fragments) and Ember 3.13.0 to my eye doesn't show anything that could make this happen warp-drive-data/warp-drive@284c8c3...df469d4

[patocallaghan]

So I've dug into this a little bit more and have been able to reproduce it without Factory Guy. It seems to be a consequence of using this.store.push Ember Data API and there's been a change in behaviour somewhere. I'm unsure whether it's due to Ember Data internals changing or Model Fragment internals.

Take the following code which is something Factory Guy does internally.

1. We create a person Fragment
2. We create a zoo model by pushing its data directly into the store using store#push. Notice rather than just passing JSON to push we are actually passing the Model Fragment attributes: { manager }.

let store = this.owner.lookup('service:store');
let manager = store.createFragment('person', {
  nickName: 'The White Wolf',
  title: 'Zoo Manager'
});
store.push({
  'data': {
    'type': 'zoo',
    'attributes': {
      manager
    },
    'id': 1
  }
});

// Model Fragments < 5.0 + Ember Data 3.12
store.peekRecord('zoo', 1).manager.nickName //=> 'The White Wolf'

// Model Fragments 5.0 + Ember Data 3.13+
store.peekRecord('zoo', 1).manager.nickName //=> undefined

So it looks like something changed where passing a fragment into store#push no longer works.

Saying that, should this have ever worked? Was it just a "happy accident" that this worked before? I always believed store#push only received JSON and the fact passing a fragment model worked surprised me. I experimented trying to pass an Ember Data model to push and that does not work.

So to round up I guess my questions are (specifically to @igorT)

1. Is this expected behaviour that passing Model Fragments to store#push should work? If so, is it a bug in Ember Data or Model Fragments?
2. If this was never expected to work, the way forward here would be to update Factory Guy to pass raw JSON instead?

Edit: I've updated my reproduction PR to remove Factory Guy and just use store#push to illustrate the problem #365

[patocallaghan]

Closing as this was fixed in Factory Guy in adopted-ember-addons/ember-data-factory-guy#449