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

Returning inherited models of various types #231

Open
ryedeer opened this issue Aug 22, 2016 · 6 comments
Open

Returning inherited models of various types #231

ryedeer opened this issue Aug 22, 2016 · 6 comments

Comments

@ryedeer
Copy link
Contributor

ryedeer commented Aug 22, 2016

Hello!

I ran into a problem while writing tests for our application. We have a certain backend route that can return various models inherited from a single base model. When I tried to mock it, I found out that the mock replaces the types of the supplied model instances with the type specified as the modelName for the mock, so the data aren't loaded properly.

To illustrate this, I wrote a simple test based on your test setup:

test("mock returns inherited models with proper types", function(assert) {
  const smallHat = make('small-hat');
  const bigHat = make('big-hat');
  let mock = mockQuery('hat', { size: 'all' }).returns({ models: [smallHat, bigHat] });
  let responseData = mock.responseJson.data;
  equal(responseData[0].type, 'small-hat');
  equal(responseData[1].type, 'big-hat');
});

As it turned out, the data type specified in the JSON is 'hat' in both cases.

I would greatly appreciate your suggestions on handling this situation.

@danielspaniel
Copy link
Collaborator

I will put this test to use in finding out what is going on ... on first glance it is probably due to the way polymorphism is handled in factory guy and that I have tested in a relationship setting and not freestyle as this one is ( load some polymorphic models on their own )

@danielspaniel
Copy link
Collaborator

this issue is now fixed in version 2.7.4 .. thanks for bringing it up

@ryedeer
Copy link
Contributor Author

ryedeer commented Aug 23, 2016

Thank you very much for fixing it! It will help us a lot.

@danielspaniel
Copy link
Collaborator

Your 100% welcome. I always like that you give me very clear idea of what is going on and write a test I can work with to fix the issue.

@ryedeer
Copy link
Contributor Author

ryedeer commented Apr 26, 2018

Hello! It seems that this issue strikes again. The polymorphic models are returned just fine when we use buildList (I see that a modified version of the test from my original post is included into the test suite), but the modelName is replaced with the one from the mock definition if we try to use returns({models: ...}). Here's the test I tried to include into the shared-adapter-behaviour.js:

test("using returns with model instances for a polymorphic route", function(assert) {
  run(() => {
    let done = assert.async();
    let bigHat = make('big-hat');

    console.log(bigHat.constructor.modelName); // "big-hat"

    mockQuery('hat', {size: 'all'}).returns({models: [bigHat]});

    FactoryGuy.store.query('hat', {size: 'all'}).then(function(hats) {
      assert.equal(hats.get('length'), 1);

      console.log(hats.get('firstObject').constructor.modelName); // "hat"

      assert.equal(hats.get('firstObject').constructor.modelName, bigHat.constructor.modelName) // fails
      assert.equal(FactoryGuy.store.peekAll('hat').get('content').length, 0, "does not make another hat"); // fails again
      done();
    });
  });
});

It seems that the previous behaviour was broken by this fix. Our test cases started to work just fine when we returned type: model.constructor.modelName into the model definition, but I see that it may break some other use cases. I don't know what to suggest... maybe we just need some kind of config option that would define the preferred behaviour for a mock.

@danielspaniel
Copy link
Collaborator

Interesting, I am going to try this out and see if i can figure this out.

@danielspaniel danielspaniel reopened this Apr 28, 2018
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

2 participants