Fix compatibility with ember-data-model-fragments

[whatthewhat]

This adds a workaround for compatibility with ember-data-model-fragments, as discussed in #182.

@patocallaghan you mentioned that the workaround did not work for you before, could you try this branch when you have the time? (you'll need to use "ember-data-factory-guy": "whatthewhat/ember-data-factory-guy#model-fragments-compatibility" in your package.json)
I only tested it on 1 app, but it seems to be working ok with ember-data-model-fragments 1.13.1 and ember-data 1.13.16

Closes #182

[patocallaghan]

@whatthewhat thanks for this. Going to try out the latest release. There was possibly something wrong with my factory's setup. On that note, what's the best way to include a model-fragment in a model factory? Can you only hard code the payload into the parent factory or can you create factories for your model fragments?

[danielspaniel]

@patocallaghan I was wondering that myself .. me curious ... would be neat-o if you could make factories for the fragments. If you add that explanation on the README would be nice, so others could get some hints.

[mattmcginnis]

I'm testing this with fragments v2.1.2 and ember-data 2.4.3 (with ember-data also specified in bower.json) The current issue I have is that "this" is undefined on the last line below.

import FragmentTransform from './fragment';
import map from '../../util/map';

/**
  @module ember-data-model-fragments
*/

/**
  Transform for `MF.fragmentArray` fragment attribute which delegates work to
  the fragment type's serializer

  @class FragmentArrayTransform
  @namespace MF
  @extends DS.Transform
*/
var FragmentArrayTransform = FragmentTransform.extend({
  deserialize: function deserializeFragmentArray(data) {
    if (data == null) {
      return null;
    }

    return map(data, function(datum) {
      return this.deserializeSingle(datum);
    }, this);
  },

  serialize: function serializeFragmentArray(snapshots) {
    if (!snapshots) {
      return null;
    }

    var store = this.store;

Any ideas on this? Maybe the versions I'm using are not compatible.

[danielspaniel]

can you try v2.4.4 factory guy ? might not help .. but it was a special patch for fragments .. so it's worth a shot

[mattmcginnis]

Yep, that's the factory guy version I'm using.

[danielspaniel]

I am not sure .. maybe wait till the other's like @whatthewhat have a say. Otherwise if you can let me clone the repo I can noodle around?

[mattmcginnis]

I'll have to wait since the repo is private. Thanks Daniel!

[danielspaniel]

ok .. np

[mattmcginnis]

@danielspaniel @whatthewhat I created an example repo If you get a chance to take a look that would be great!

[whatthewhat]

@mattmcginnis just tried you example and can confirm the error.

The reason it works for me is because we don't have any fragment data in factories at the moment (in your example if the factory does not have default values for the fragment the test passes).

[patocallaghan]

I had a quick dig into this, haven't really got anywhere with it but this is what I found

1. When transformValueFunction(fixture[attribute]) is called it has no context associated with it so when serialize for the ModelFragment is called, this is undefined and it blows up on this.store. You can get passed this error by doing the following on line 42 of converter/fixture-converter.js:

let fragmentType = serializer.transformFor(type);
return fragmentType.serialize.bind(fragmentType);

1. But then this leads to another problem, it fails with another error later on in the serialize function. Line 57 in the fixture-converter.js just passes a json payload but the ModelFragment's .serialize function is expecting the param to be of type DS.Snapshot. Internally serialize accesses several properties on that class, namely modelName and eachAttribute.

[danielspaniel]

I am about to fall over because this was the last thing on a long day of work but
.. I did a bigger dig into this and I think I found a bug in the fix that was done by whatthewhat.

I am too tired to say whether this is a real bug or my imagination, but your test now passes and all the fun data is there.

But ... I have to check out more scenarios, and also maybe add tests for model fragments and make sure I don't cause other MF errors.

I might take all this same info in your sample repo and put that in fg and write tests against it ( for all the different types of fragments ) .. should I be doing that? or is that MF writers job? not sure?

[danielspaniel]

@patocallaghan .. I did not read your post carefully enough, but you found the bug too.
serializer is looking to serialize a snapshot ..
and in this world of building fixture data I am not actually doing and ember-data serialize

anyway .. I can't think straight right now, but here was my fix:

let fragmentType = serializer.transformFor(type);
return fragmentType.deserialize.bind(fragmentType);

Let me know if that makes sense

[mattmcginnis]

Thanks guys! I was nearly at the same solution myself when I saw the comment. I created a pull request based on the snippet above. I also updated my sample repo with my FG fork and added some tests that modify attrs and save. Everything works as advertised.

[patocallaghan]

Nice work @danielspaniel and @mattmcginnis! ... Yeah I think it would be great to have some sort of testing against Model Fragments considering it's now supported. I don't mind helping out adding some tests for that if you want? I also think it warrants an update to the README (again I can add something there)

[whatthewhat]

Yep, my bad.. great work on investigating the issue 👍

[danielspaniel]

@patocallaghan .. I would be really happy if you added some good tests to make this a more rock solid commit, because I don't trust that things will work in every situation yet util those exist.

I am even thinking now that the deserialize is not even needed, and that just a no op function would have been fine. In other words I am questioning the value of transformValueFn itself .. does not seem to actually serve any purpose, but i will check later today to be sure.

[danielspaniel]

Any updates @patocallaghan ??
This is actually alittle bit tricky because model fragment is different serialize than an ember data attribute .. so , there has to be a check if this attribute is a model fragment ( not sure how to do that yet )
anyway .. if you have anything going on testing wise let me know.

[patocallaghan]

Hey @danielspaniel ..I have some tests but nothing extensive yet. I'll make time to get a PR into you in the next few days

[danielspaniel]

ok .. great .. I have come up with this

import EmberDataAttributeTransform from "ember-data/transform";

and then

    if ('function' === typeof serializer.transformFor && !(serializer.transformFor instanceof EmberDataAttributeTransform)) {
      return this.noTransformFn;
    }

as my sort of hackey solution .. and have added a few tests of my own already .. if you want to ScreenHero today or tomorrow let me know .. and we can share test info

[patocallaghan]

Hey, yep that sounds like a plan. How about Friday night GMT which I guess makes it in the afternoon for you?

[danielspaniel]

@patocallaghan sounds good .. I'll be available .. let's say 1 pm -> 4 pm EST .. let me know when you ready and I can get you SH info

[slindberg]

Just wanted to say thanks for everyone's work on this fix! I realize MF's weird use of serializers made it a little bit of a pain o_O

[danielspaniel]

No problem @slindberg .. it was really neat to be able to make this work because making factories for the fragment data is really nice. If you have any suggestions, bring them on. Always welcome to hear about a nice idea or two.