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

Duplicating assigned attributes #72

Closed
jonatanklosko opened this issue Sep 10, 2016 · 8 comments
Closed

Duplicating assigned attributes #72

jonatanklosko opened this issue Sep 10, 2016 · 8 comments

Comments

@jonatanklosko
Copy link

Consider the following factory:

const User = require('models/user');

factory.define('user', User, {
  username: factory.sequence(n => `sherlock_${n}`),
  name: "Sherlock Holmes",
  password: "Password",
  passwordConfirmation: function() { return this.password; }
});

I'd expect the following to pass.

factory.build('user', { password: 'secret' })
  .then(user => expect(user.passwordConfirmation).to.equal('secret');

The syntax could differ but I think it's common use case to duplicate (or do something else) on an already assigned attribute.

@chetanism
Copy link
Collaborator

chetanism commented Sep 11, 2016

Hi @jonatanklosko,
You can try following to achieve the same effect:

Factory.define('user', User, {
  username: Factory.sequence(n => `sherlock_${n}`),
  name: 'Sherlock Holmes',
  password: 'password',
  passwordConfirmation: null,
}, {
  afterBuild: model => {
    model.passwordConfirmation = model.password;
    return model;
  },
});

Just to add some details, the problem with having it the way you suggested is how to ensure that passwordConfirmation is evaluated after password.

You can use the afterBuild callback to modify the built object anyway you want!

Let us know in case you face any issues.

@jonatanklosko
Copy link
Author

Hey @chetanism,
I've already tried this and actually it should be

afterBuild: model => {
  model.passwordConfirmation = model.passwordConfirmation || model.password;
  return model;
},

The problem with this solution is that afterBuild is not invoked when you use create.

I can think of a feature, that would allow us to do:

/* ... */
password: "secret",
passwordConfirmation: factory.duplicate('password')
/* ... */

/* Or even more flexible */

/* ... */
password: "secret",
passwordConfirmation: factory.after('password', password => /* do something and return */)
/* ... */

This way you can evaluate all normal attributes and those duplicate/after calls at the end.
What are your thoughts =) ?

@simonexmachina
Copy link
Owner

@chetanism I think maybe we should consider calling both afterBuild and afterCreate when creating. I think this makes sense because create does follow build.

@chetanism
Copy link
Collaborator

@jonatanklosko, sorry for a long delay!
I have raised #73 as per @aexmachina 's suggestion. The afterBuild callback should now be invoked on create calls as well (once the PR gets merged :)).

@jonatanklosko
Copy link
Author

Yeah this behavior makes more sense. The PR looks great, thanks! =)

@simonexmachina
Copy link
Owner

#73 has been merged, closing this issue now. Please advise if there's any issues.

@jonatanklosko
Copy link
Author

I've just realised that it's not invoked after #attrs as well.

@chetanism
Copy link
Collaborator

Hi @jonatanklosko, the order of execution for these methods is attrs -> build -> create. As of now, we just have callbacks for afterBuild and afterCreate, hence the callbacks are not executed for attrs call. I'll open another issue to have an afterAttrs callback.

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

3 participants