thoughtbot/fishery

Traits are ignored when providing build params

doutatsu opened this issue · 6 comments

Taking FactoryBot as a comparison, I am trying to do something like: create(:user, :admin, confirmed: false), meaning I want an admin trait, but at the same time, I would to overwrite one of the default values for the returning object.

Now, when trying to do the same in Fishery:

import { Factory } from 'fishery';

class UserFactory extends Factory {
  admin() {
    return this.params({
      profile: {
        admin: true
      },
    });
  }
}

export default UserFactory.define(({ sequence }) => ({
  id: sequence,
  email: 'test@example.com',
  registered: true,
}));

If I run this with default build: factory.admin().build(), I get what I expect:

{
  id: 1,
  email: 'test@example.com',
  registered: true,
  profile: {
    admin: true
  }
}

But the moment I try to provide anything to the build: factory.admin().build({ registered: false }), I don't get admin attributes anymore:

{
  id: 1,
  email: 'test@example.com',
  registered: false,
}

What I do want instead is:

{
  id: 1,
  email: 'test@example.com',
  registered: false,
  profile: {
    admin: true
  }
}

I am not sure if it's a bug or an intended behaviour. If it's the latter, I'd like to understand the reasoning behind it. If I can't have slight variations on the traits or using traits with slightly different defaults, it makes using traits not that much useful, as the moment I need to have both, I just have to provide single build and write up everything I need from stratch

Trying some things out, it seems like it's a problem with deep-merging objects. So for something like this:

{
  attributes: {
    registered: true,
    profile: {
      admin: true
    }
  }
}

doing factory.admin().build({ attributes: { registered: false } }) would have this issue, as I presume the whole registered gets overwritten, rather than merged together. Not sure if there is a way to make this work 🤔

Any thoughts @stevehanson?

@doutatsu thanks for reporting this. I know you've likely moved on at this point, but if not, would you be able to share which version of Fishery you were using? I am not able to recreate it using this test case:

it('preserves nested objects when params passed', () => {
  type User = {
    registered: boolean;
    profile?: { admin: boolean };
  };
  class UserFactory extends Factory<User> {
    admin() {
      return this.params({
        profile: {
          admin: true,
        },
      });
    }
  }

  const userFactory = UserFactory.define(() => ({ registered: true }));
  const user = userFactory.admin().build({ registered: false });
  expect(user.profile?.admin).toBe(true);
  expect(user.registered).toBe(false);
});

@doutatsu: please disregard my last comment. I just looked closer at your second comment and see that you have an additional layer of nesting inside of attributes. I'm able to recreate this now!

@doutatsu I got it figured out and fixed. Many thanks for reporting this issue!

@stevehanson This is great, thank you for fixing this! I can go back and clean up my tests now 😄