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

Compatibility with ember-data-model-fragments #182

Closed
whatthewhat opened this issue Mar 22, 2016 · 6 comments
Closed

Compatibility with ember-data-model-fragments #182

whatthewhat opened this issue Mar 22, 2016 · 6 comments

Comments

@whatthewhat
Copy link
Contributor

Hi, first of all great work with the library, really like the latest changes!

Currently there is an issue when trying to use latest factory-guy versions with ember-data-model-fragments.
It has to do with looking up transforms here, ember-data-model-fragments monkey patches the transformFor on the serializer to do it's magic and so when factory-guy tries to find the transform with container.lookup if fails with an error.

Admittedly this is not exactly a factory-guy problem, but it would be nice if the 2 libraries could work together again and it does not seem like ember-data-model-fragments can stop depending on transformFor any time soon.

So the question is, would a PR with a change similar to this be accepted (even though it basically adds a dependency on a private serializer method)?

  getTransformValueFunction(type, modelName) {
    // ...
    let serializer = this.store.serializerFor(modelName);
    if ('function' == typeof serializer.transformFor) {
      return serializer.transformFor(type);
    } else {
      // use existing container lookup
    }
  }

Link to the discussion in the ember-data-model-fragments repo: adopted-ember-addons/ember-data-model-fragments#185

@danielspaniel
Copy link
Collaborator

I think this would be an ok hackeroo because as long as it does not affect the usual factory guy hokus pokus ( the tests all still pass ) and it's labelled ( "note the hack here because of model fragment" ) then everyone knows what is going on and no one will know the difference

@whatthewhat
Copy link
Contributor Author

Great! I'll submit a PR some time this week.

@patocallaghan
Copy link
Collaborator

@whatthewhat just curious if you have tried out this fix? I tried it out but it then seemed to fail further down the line with another error. I'll dig it out when I get the chance.

@whatthewhat
Copy link
Contributor Author

@patocallaghan Did not have time to test this yet, unfortunately. Will definitely play with it on the weekend, let me know if you find anything!

whatthewhat added a commit to whatthewhat/ember-data-factory-guy that referenced this issue Mar 25, 2016
whatthewhat added a commit to whatthewhat/ember-data-factory-guy that referenced this issue Mar 26, 2016
@mattmcginnis
Copy link
Contributor

I created an example repo If anyone wants to take a look.

@danielspaniel
Copy link
Collaborator

@patocallaghan , @whatthewhat , @mattmcginnis
I just "fixed" model fragment factories to no longer output an id.

Not sure if you even care, but when using factories with fragments there was always an id put into the fixture.
I noticed, but I did not care until one day I was doing something and I realized:
"By golly there is an id in my fragment data"
because one of my tests was not loving that very much, and I realized I should stop doing that.
So in v2.7.0-beta.11
and onwards .. unless someone says otherwise:

The id is now only used for building ( see release notes ) but removed afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants