Polymorphism doesnt currently work with store.find.

[kylenathan]

App.Story = DS.Model.extend({
    user: DS.belongsTo('user');
    updated: DS.attr('number') 
});

App.PostStory = App.Story.extend({
   post: DS.belongsTo('post');
});

App.PhotoStory = App.Story.extend({
   photo: DS.belongsTo('photo');
});

Given the above models, the following returns models of the type App.Story regardless of the server response.

this.store.find('story', {userId: 1});

From what I can see this is because when defining polymorphic model relationships we have the {polymorphic: true} flag, but no such option exists for store.find (AFAIK).

I've hacked a quick, dirty fix that works by checking the data for the presence of a 'type' key and adjusting the type arg to recordForId accordingly, but that's not ideal. Should this be checking for a polymorphic flag in the args to store.find? Any other alternatives?

[hjdivad]

The original polymorphic feature was tied to associations because find returned the record immediately, so it was necessary to construct the object before receiving the payload. This is no longer true since the promisification of these APIs.

[tonywok]

I have a scenario as follows (contrived):

App.Task = DS.Model.extend({
  attachments: DS.hasMany('attachment', {polymorphic: true, inverse: 'task'}),
  body: DS.attr('string')
});
App.Attachment = DS.Model.extend({
  task: DS.belongsTo('task'),
  name: DS.attr('string'),
  type: DS.attr('string')
});
App.Image = Attachment.extend({
  foos: DS.hasMany('bar', {inverse: 'image'})
});
App.Video = Attachment.extend({
  bars: DS.hasMany('foo', {inverse: 'video'})
});

I'd prefer Video and Image be saved to the store as an Attachment, but upon pulling one out of the store, or finding one from the server, it would know that it is to become the model matching its type field. This is similar to, as you may have guessed, rails STI. I don't find myself using it hardly ever, but some colleagues of mine seem to have a decent use-case and I'm trying to help them make it work w/ ember data.

A long time ago, you were able to do this with https://coderwall.com/p/hi6hya method - but that code is long gone :)

The closest implementation I've seen recently would be #1845

@kylenathan Is this functionality similar to what you're looking for?

@hjdivad I was close to getting this to work, but I ran into the problem that the object is constructed before receiving the payload. If it's no longer necessary, do you know if there any plans on removing that functionality before 1.0?

[stefanpenner]

we should stop eager constructing these objects, as that causes other issues. Someone just needs to sit down and do the refactor.

[hjdivad]

yeah switching from types to strings was a great move; this is one of the loose ends that still needs to be tied up

[tonywok]

@stefanpenner Looking at the code, it seems like the create part of the find-or-create-esque functionality of recordForId can be removed, and the remaining code could just be hoisted up to getById. This would also clean up the guards in fetchRecord.

Does it sound like I'm on the right track? Basically passing along the type and id instead of constructing a record (and the much refactoring that would necessarily follow).

Also curious if this conflicts with the single-source-of-truth branch - need to get a firmer understanding on how that is going to work.

[kylenathan]

@stefanpenner possible to get some pointers on how to refactor this? Keen to get it working.

Worth doing or is ssot branch going to change everything?

[hjdivad]

@igorT feel free to correct me if I'm wrong but @kylenathan I don't think SSOT, at least at a high level, has any impact on this. I say "at a high level" b/c SSOT has some other misc changes to the store, but its focus sidesteps this.

The main issue API-wise is that to support polymorphism in find you must delay creating the object until you have the payload (as you will not know the concrete type until then). So the main refactoring task is to look for references to recordForId and delay them until you have the payload.

The above is a bit hand-wavy. For instance, getById may be okay as long as it's acceptable to return only the abstract type. From what I can see it's only called on a find rejection, in which case the abstract type is probably fine. However getById is not marked as private, so you'd want to first verify that it should be.

[kylenathan]

@hjdivad great, thanks. Digging into ED internals and will start having a play around.

[denisnazarov]

// polymorphic-find.js

import Ember from 'ember';
import ajax from 'ic-ajax';

export default function(url, id){
  var store = this.store;

  if (id){
    url = url + '/' + id;
  }

  return ajax(url).then(function(data){
    var barsPayload = {};

    data.bars.forEach(function(bar){
      var barType = Ember.String.dasherize(bar.links.bar_type);
      var typeArray = barsPayload[barType];
      if (!typeArray){
        typeArray = barsPayload[barType] = [];
      }
      typeArray.push(bar);
    });

    store.pushPayload(barsPayload);

    var bars = [];

    data.bars.forEach(function(bar){
      var id = bar.id;
      var type = Ember.String.dasherize(bar.links.bar_type);
      bars.push(store.getById(type, id));
    });

    if (id){
      return bars[0];
    }else{
      return bars;
    }
  });
};

On a project we are simulating the effect of polymorphic find with $.ajax. Maybe this will be helpful to someone. This breaks the payload up based on model types, pushes that into store (the barsPayload keys specify which model to create), and looks them up in the store in the same order they were in the original payload.

cc @krisselden

[joeyespo]

Anyone currently looking at this? It'd be a really useful addition. Will have to go with the solution @denisnazarov provided (thanks!), but it's not as clean as having Ember Data do it.

[solocommand]

Agreed, just ran into this issue as well.

[sly7-7]

@igorT Is this something we want to see implemented ?

[hjdivad]

@igorT keeping in mind that the answer is yes @sly7-7.

[igorT]

This will soon be possible to implement with #2991

[sly7-7]

I think this is implemented now, closing.

[kylenathan]

Not yet implemented but waiting on an RFC - emberjs/rfcs#67