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

Sideloaded nested relationships stopped working #5124

Closed

Conversation

nbrookie
Copy link
Contributor

@nbrookie nbrookie commented Aug 13, 2017

Say I was to load a model, sideloading parent models as well:

// app/routes/index.js
let kids = this.store.findAll('kid', { include: 'middle-ager.elder' })
return Ember.RSVP.hash({ kids })

Which generates this JSONAPI response:

JSONAPI Response
{
  "data":[
    {
      "id":"1",
      "type":"kids",
      "links":{
        "self":"/kids/1"
      },
      "attributes":{
        "name":"Kid 1"
      },
      "relationships":{
        "middle-ager":{
          "links":{
            "self":"/kids/1/relationships/middle-ager",
            "related":"/kids/1/middle-ager"
          },
          "data":{
            "type":"middle-agers",
            "id":"1"
          }
        }
      }
    }
  ],
  "included":[
    {
      "id":"1",
      "type":"middle-agers",
      "links":{
        "self":"/middle-ager/1"
      },
      "attributes":{
        "name":"Middle Ager 1"
      },
      "relationships":{
        "elder":{
          "links":{
            "self":"/middle-agers/1/relationships/elder",
            "related":"/middle-agers/1/elder"
          },
          "data":{
            "type":"elders",
            "id":"1"
          }
        },
        "kids":{
          "links":{
            "self":"/middle-agers/1/relationships/kids",
            "related":"/middle-agers/1/kids"
          }
        }
      }
    },
    {
      "id":"1",
      "type":"elders",
      "links":{
        "self":"/elders/1"
      },
      "attributes":{
        "name":"Elder 1"
      },
      "relationships":{
        "middle-agers":{
          "links":{
            "self":"/elders/1/relationships/middle-agers",
            "related":"/elders/1/middle-agers"
          }
        }
      }
    }
  ]
}

Then in a template:

{{#each model.kids as |kid|
  {{kid.name}} <!-- THIS IS OK -->
  {{kid.middleAger.name}} <!-- THIS IS OK -->
  {{kid.middleAger.elder.name}} <!-- THIS IS BROKEN -->
{{/each}}

It seems that accessing the nested model that is two levels deep no longer works.

This PR includes a test that replicates the issue.

Versions effected:

2.14.0-beta.1 - 2.16.0-canary

This problem appears to have been introduced in this commit: a38f408

Versions not effected

Anything below 2.13.2

Relevant Twiddles:

ember-data 2.12.2 works fine: Twiddle
(ember-data 2.13.2 works fine too, but twiddle does not work with ember-data 2.13.2 yet

ember-data 2.14.10 does not work: Twiddle

Related issues

#4982

@nbrookie nbrookie changed the title Nested relationships stopped working Sideloaded nested relationships stopped working Aug 13, 2017
@nbrookie nbrookie force-pushed the nested-relationships-bug-2.16-canary branch from 013f205 to 2063848 Compare August 14, 2017 17:12
@rlivsey
Copy link

rlivsey commented Aug 16, 2017

@nbrookie thanks reporting this and adding the failing test!

Just ran into this myself and had to roll-back to 2.13 again.

@gabz75
Copy link

gabz75 commented Aug 22, 2017

Hi there,

I believe I'm facing a very similar issue. I'm happy to open a new PR if you think it's different. Here is a repository to reproduce with a failing test:

https://github.com/gabz75/ember-data-bug-with-sideloaded-relationships

@stefanpenner
Copy link
Member

started looking at this, I have an idea (thanks for the failing test) going to grab some food, then take another look later today.

@stefanpenner stefanpenner self-requested a review August 24, 2017 14:52
@stefanpenner
Copy link
Member

stefanpenner commented Aug 24, 2017

I believe _removeInverse

/**
Remove the relationship in `previousPayload` from its inverse(s), because
this relationship payload has just been updated (eg because the same
relationship had multiple payloads pushed before the relationship was
initialized).
@method
*/
_removeInverse(id, previousPayload, inverseIdToPayloads) {
let data = previousPayload && previousPayload.data;
if (!data) {
// either this is the first time we've seen a payload for this id, or its
// previous payload indicated that it had no inverse, eg a belongsTo
// relationship with payload { data: null }
//
// In either case there's nothing that needs to be removed from the
// inverse map of payloads
return;
}
if (Array.isArray(data)) {
// TODO: diff rather than removeall addall?
for (let i=0; i<data.length; ++i) {
this._removeFromInverse(id, data[i].id, inverseIdToPayloads);
}
} else {
this._removeFromInverse(id, data.id, inverseIdToPayloads);
}
}
/**
Remove `id` from its inverse record with id `inverseId`. If the inverse
relationship is a belongsTo, this means just setting it to null, if the
inverse relationship is a hasMany, then remove that id from its array of ids.
@method
*/
_removeFromInverse(id, inverseId, inversePayloads) {
let inversePayload = inversePayloads[inverseId];
let data = inversePayload && inversePayload.data;
if (!data) { return; }
if (Array.isArray(data)) {
inversePayload.data = data.filter((x) => x.id !== id);
} else {
inversePayloads[inverseId] = {
data: null
};
}
}
}
is being called spuriously, incorrectly evicting the payload information.

@stefanpenner
Copy link
Member

fixed -> #5147

@stefanpenner
Copy link
Member

thanks for the test case, made this very actionable!

@runspired runspired added 🏷️ bug This PR primarily fixes a reported issue and removed Bug labels Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bug This PR primarily fixes a reported issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants