[JSONAPI] Using EmbeddedRecordsMixin puts the embedded records in the wrong place

[jagthedrummer]

I'm trying to send a Product and a collection of Screenshots to the server in one request.

My product serializer looks like this:

// app/serializers/product.js
import DS from 'ember-data';
export default DS.JSONAPISerializer.extend(DS.EmbeddedRecordsMixin,{
  attrs: {
    screenshots: { embedded: 'always' }
  }
});

When I save a Product that has one Screenshot in the collection I'm getting this payload:

{
  "data"=>{
    "attributes"=>{
      "name"=>"embedded save test",
      "price"=>"42",
    },
    "relationships"=>{
      "category"=>{
        "data"=>{"type"=>"categories", "id"=>"1"}
      }
    },
    "screenshots"=>[{
      "data"=>{
        "attributes"=>{
          "name"=>"testing the screenshots",
          "description"=>"heloo there",
          "cloudinary-image-id"=>"g3ljvsgc1j0jskrgmexd"
        },
        "relationships"=>{
          "product"=>{"data"=>{"type"=>"products", "id"=>nil}}
        },
        "type"=>"screenshots"
      }
    }],
    "type"=>"products"
  }
}

The screenshots object is part of the root data object instead of being in the relationships object along with category.

[wecc]

I would say that the EmbeddedRecordsMixin does not support JSONAPISerializer right now as the JSON API spec itself doesn't mention anything about embedding resources (are they relationships? keep them as complex attributes?). It shouldn't be too hard to fix but it's currently very untested.

[fivetanley]

I would say that if you're using JSONAPI you shouldn't be using embedded records. You should be able to move those records to included

[csantero]

JSON API does not define any semantics for using included in a POST/PATCH. The way I've dealt with saving multiple related records in one request is to turn the related records into complex attributes on the primary record. It's ugly, but I haven't found any better way to do it.

There is discussion about how to support this use case and it's milestoned for the next version of the spec, but nothing concrete yet.

[MiracleBlue]

Darn, so this is what I have been dealing with all morning. Glad to have found this thread. I'll switch to just not using embedded records for the time being and side-load it in included.

[vladimir-e]

I ran into same problem today. If you use JSONAPISerializer and you need to save a record and update its hasMany relationships in one request, DS.EmbeddedRecordsMixin is still okay, if you really need to make it work.

UPD: changed the code below after looking at PR #3848

Here's a patch I ended up creating:

I created mixin/embedded-jsonapi-records.js that adds another type of serialization ids-and-type which is json-api compliant:

import Ember from 'ember';
import {pluralize} from 'ember-inflector';

export default Ember.Mixin.create({

  // add json-api compliant serialization type
  hasSerializeIdsAndTypeOption: function(attr) {
    var option = this.attrsOption(attr);
    return option && option.serialize === 'ids-and-type';
  },

  serializeHasMany: function(snapshot, json, relationship) {
    var attr = relationship.key;
    if (this.noSerializeOptionSpecified(attr)) {
      this._super(snapshot, json, relationship);
      return;
    }
    var includeIds = this.hasSerializeIdsOption(attr);
    var includeIdsAndType = this.hasSerializeIdsAndTypeOption(attr);
    var includeRecords = this.hasSerializeRecordsOption(attr);
    if (includeIdsAndType){
      let serializedKey = this.keyForRelationship(attr, relationship.kind, 'serialize'),
          serializedRecords = snapshot.hasMany(attr, { ids: true }).map(function(id) {
            return { id: id, type: pluralize(relationship.type)};
          });
      if (!json['relationships']) {
        json['relationships'] = {};
      }
      json['relationships'][serializedKey] = { data: serializedRecords };
    } else if (includeIds) {
      let serializedKey = this.keyForRelationship(attr, relationship.kind, 'serialize');
      json[serializedKey] = snapshot.hasMany(attr, { ids: true });
    } else if (includeRecords) {
      this._serializeEmbeddedHasMany(snapshot, json, relationship);
    }
  },

});

Example:

app/serializers/client.js

import DS from 'ember-data';
import EmbeddedJSONapiRecordsMixin from '../mixins/embedded-jsonapi-records';

export default DS.JSONAPISerializer.extend(DS.EmbeddedRecordsMixin, EmbeddedJSONapiRecordsMixin, {
  attrs: {
    pets: {
      serialize: 'ids-and-type',
      deserialize: 'records'
    }
  }
});

[bmac]

I have opened a pr so Ember Data will warn in the future when JSONAPISerializer and EmbeddedRecordsMixin are combined. https://github.com/emberjs/data/pull/4259

I don't think JSONAPISerializer and EmbeddedRecordsMixin will work together out of the box until JSON API has a more detailed specification on how it supports embedded records. https://github.com/json-api/json-api/issues/795

[michaeljbishop]

Do we necessarily need something more from JSON-API at this point for this to work? Why couldn't the payload in the above example look like this?

{
  "data"=>{
    "attributes"=>{
      "name"=>"embedded save test",
      "price"=>"42",
      "screenshots"=>[{
        "name"=>"testing the screenshots",
        "description"=>"heloo there",
        "cloudinary-image-id"=>"g3ljvsgc1j0jskrgmexd"
      }],
    },
    "relationships"=>{
      "category"=>{
        "data"=>{"type"=>"categories", "id"=>"1"}
      }
    },
    "type"=>"products"
  }
}

In other words, if the value of the "attribute" is an embedded record, why don't we serialize it as a JSON blob and submit (and retrieve) that as the value?

[taras]

@bmac is it possible to suppress this error for those who're using JSONAPI & EmbeddedRecordsMixin intentionally?

[danilovaz]

anything about it? this is very frustrating. I use JSONAPISERIALIZER. I have a User model containing many Tags. What can I do when I need to make a Patch request, which can be editing the user name and selected Tags ids?

[fivetanley]

@taras I don't think we'll allow disabling the warning until JSONAPI takes a stance on this. The JSONAPI adapter/serializer's goals are to only support stuff in the spec or in a spec extension. A better route for now might be to fork or copy the EmbeddedRecordsMixin into your app and make the changes you need there for now.

I know this is kind of not the greatest response in the world, and I apologize. EmbeddedRecordsMixin has in the past been pretty complex to support across a variety of APIs, so we want to reduce the maintenance burden until something is standardized so everyone has an idea of what their server should be sending.

[fivetanley]

We should come up with something in the short term so at least IDs can be serialized to the server.

[sandstrom]

FWIW here is the discussion on embedded records in json-api:

https://github.com/json-api/json-api/issues/795#issuecomment-245016601

[taras]

I know this is kind of not the greatest response in the world, and I apologize.

No worries, it makes sense.

Would it make sense to pull EmbeddedRecordsMixin mixing into an addon? It can stay in Ember Data and people who want to use it can use it without the warning can use the addon. It'll make their opt-in explicit.

[pixelhandler]

@jagthedrummer Just saw this, https://github.com/json-api/json-api/issues/1089 seems that there are still a lot of question to resolve before the JSON API adapter/serializer can support embedded records.

@taras since the EmbeddedRecordsMixin originally supported the ActiveModel adapter/serializer it makes sense to extract as an addon.

[sandstrom]

This PR may have solved this issue: https://github.com/emberjs/data/pull/7126

(I haven't tested myself yet though, saw it only a few minutes ago)

[runspired]

Closing as both identifiers resolved much of the original issues faced here and because serializers are an antiquated pattern that are optional today and don't have a lasting future (see https://github.com/emberjs/rfcs/pull/860)