[bugfix beta] Add inverse relationship on payload when missing

[fivetanley]

Always pushes the left hand side of a relationship, even when the adapter does not return a payload with the left hand side's ids in relationships.

What a mouthful. Let's unpack that.

Given the following model definitions:

// app/models/person.js

export default DS.Model.extend({
  name: attr(),
  dogs: hasMany('dog', { async: true })
});

// app/models/dog.js

export default DS.Model.extend({
  name: attr(),
  person: belongsTo('person', { async: true })
});

Given PersonAdapter#findRecord returns the following payload and is pushed into the store:

const payload = {
  data: {
    type: 'person',
    id: '1',
    attributes: {
      name: 'John Churchill'
    },
    relationships: {
      dogs: {
        links: {
          related: 'http://exmaple.com/person/1/dogs'
        }
      }
    }
  }
};

and the result of DogAdapter#findHasMany resolves with the following
payload:

{
  data: [
    { id: 1, type: 'dog', attributes: { name: 'Scooby' } },
    { id: 2, type: 'dog', attributes: { name: 'Scrappy' } }
  ]
}

Notice how the payload for DogAdapter#findHasMany does not have a relationships key for any of the returned dogs. This is technically valid JSONAPI, but in today's Ember Data, for explicit inverses only (notice how we specified 'person' on the Dog model; an implicit relationship would be generated for us if we left it out and used internally for tracking various things), the inverse relationship does not get set up. For example, dog.get('person.id') would still be null when it should be 1 (the id of the Person it belongsTo).

This commit fixes that by always pushing the inverse relationship using
the store's RelationshipPayloadsManager. In this example, the following
relationship info is pushed:

{
  data: {
    id: '1',
    type: 'person'
  }
}

This makes the result of requesting DogAdapter#findHasMany effectively the following payload:

{
  data: [
    {
      id: 1,
      type: 'dog',
      attributes: { name: 'Scooby' },
      relationships: { id: '1', type: 'person' }
    },
    {
      id: 2,
      type: 'dog',
      attributes: { name: 'Scrappy' },
      relationships: { id: '1', type: 'person' }
    }
  ]
}

Adding this information makes the relationships line up correctly.

I can also confirm this makes all the tests in #5480 pass (except for the one about destroyRecord unloading the record, which is copied over from #5455 and is a separate issue).

[evoactivity]

I'm seeing some odd behaviour that I believe is related to this.

Let's say you have 10 dogs and 1 person. You have a route where each dog is listed and it's person is listed for each dog in the list.

The behaviour I'm seeing is only the last resolved person will be rendered with each dog before having it's relationship removed.

eg

Dog 1 - 
Dog 2 -
Dog 3 -
Dog 4 -
Dog 5 -
Dog 6 -
Dog 7 -
Dog 8 -
Dog 9 -
Dog 10 - Person 1

I think what's happening is the relationship is requested from the related link, it's then pushed to the store with the parent inverse data appened to the relationship, then it moves onto the next one, again the relationship is requested from the related link, it gets pushed into the store with it's parent inverse data, but because it's the same model the data is overwritten instead of pushed to, leaving the "person" relationship only pointing to one dog instead of many.

[makepanic]

I'm seeing some odd behaviour that I believe is related to this.

Let's say you have 10 dogs and 1 person. You have a route where each dog is listed and it's person is listed for each dog in the list.

The behaviour I'm seeing is only the last resolved person will be rendered with each dog before having it's relationship removed.

eg

Dog 1 - 
Dog 2 -
Dog 3 -
Dog 4 -
Dog 5 -
Dog 6 -
Dog 7 -
Dog 8 -
Dog 9 -
Dog 10 - Person 1

I think what's happening is the relationship is requested from the related link, it's then pushed to the store with the parent inverse data appened to the relationship, then it moves onto the next one, again the relationship is requested from the related link, it gets pushed into the store with it's parent inverse data, but because it's the same model the data is overwritten instead of pushed to, leaving the "person" relationship only pointing to one dog instead of many.

Sounds similar to what we've observed in #5706 (comment)

[runspired]

@makepanic yeah I dug in last night and confirmed how this happens :'( I'm sad I missed the case when we introduced this. I'm even more sad because pre 3.5 ember-data this would have been much more trivial to fix as there we had the concept of "partial" updates to canonical data. We don't currently, but I'm working on bringing them back.

That said, I think there are two fail scenarios here, and one is addressable more easily and possibly more common. I'll try to write some additional tests this weekend.

[Leooo]

@runspired I know it's not a LTS anymore, but any idea if a PR with the bugfix above for 3.4 would be easy? This one is a blocker to our Ember upgrade across several apps (from 2.14 to 3.4), and having to do all upgrades again to the next LTS (3.8) without splitting to 3.4 first would be a massive work.. thanks

[runspired]

@Leooo it should be decently trivial to make your serializer do this

[Leooo]

@runspired thanks, for now my update to Ember 3.4.4 plus ember-data 3.5 seems to be smooth and fixes this issue.

[Leooo]

had to go back to ember-data 3.4.4 because of another ember-data 3.5 bug

I fixed this bug in our adapter with the below for now:

findHasMany(store, snapshot/*, url, relationship*/) {
    const childrenPromise = this._super(...arguments);
    const modelName = snapshot.modelName;
    return childrenPromise.then((children) => {
      this._addParentIdToChildrenPayload(
        children.data,
        modelName,
        snapshot
      );
      return children;
    });
  },
  findRecord(x, model, id, snapshot) {
    const res = this._super(...arguments);
    const modelName = model.modelName;
    return this._super(...arguments).then((payload) => {
      if (payload.included) {
        this._addParentIdToChildrenPayload(
          payload.included,
          modelName,
          snapshot
        );
      }
      return payload;
    });
  },
  //"fix" https://github.com/emberjs/data/pull/5608
  _addParentIdToChildrenPayload(children, modelName, snapshot) {
    children.forEach((child) => {
      child.relationships = child.relationships || {};
      //suppose it's manyToOne, for now
      child.relationships[modelName] = child.relationships[modelName] || {};
      child.relationships[modelName].data = {
        type: pluralize(modelName),
        id: snapshot.id
      };
    });
    return children;
  }