Traits #251

Closed
yelled3 opened this Issue Nov 19, 2015 · 9 comments

Comments

Projects
None yet
3 participants
@yelled3

yelled3 commented Nov 19, 2015

is there a way to do something similar to factory_girl Traits?
https://github.com/thoughtbot/factory_girl/blob/master/GETTING_STARTED.md#traits

the closest thing I was able to come up with is doing:

class UserFactory(factory.Factory):
    class Meta:
        model = User
        exclude = ('admin',)

    admin = False
    role = 'regular'
    if admin:
        role = 'admin'

user = UserFactory.build()
admin_user = UserFactory.build(admin=True)

which is not all that bad - but I don't like the whole exclude hack.

@rbarrois

This comment has been minimized.

Show comment
Hide comment
@rbarrois

rbarrois Feb 9, 2016

Member

Well, it could be added yes :)

Since I've not used factory_girl nor its Traits, I'm not sure what a good API would be though.

If you have any ideas on this topic, I'd be happy to follow up :)

Member

rbarrois commented Feb 9, 2016

Well, it could be added yes :)

Since I've not used factory_girl nor its Traits, I'm not sure what a good API would be though.

If you have any ideas on this topic, I'd be happy to follow up :)

@yelled3

This comment has been minimized.

Show comment
Hide comment
@yelled3

yelled3 Feb 10, 2016

@rbarrois
IMO, traits are extremely usful feature in factory_girl.

that being said, how about we treat traits as just attributes that are always excluded.
the API can be something like:

class UserFactory(factory.Factory):
    class Meta:
        model = User

    role = 'regular'

    is_admin = factory.trait(lambda user: user.role = 'admin)

    # a dict might be more useful when using multiple attributes 

    is_admin = factory.trait({ "role": "admin" })

you can then use it like any other attribute

admin_user = UserFactory.build(is_admin=True)

another usage for traits is that you can use them in a subclass like any other attribute.
so maybe we can do:

class AdminUserFactory(UserFactory):
     class Meta:
        model = User
        traits = ('is_admin', )  # same as doing UserFactory.build(is_admin=True)

@rbarrois
Thoughts?

yelled3 commented Feb 10, 2016

@rbarrois
IMO, traits are extremely usful feature in factory_girl.

that being said, how about we treat traits as just attributes that are always excluded.
the API can be something like:

class UserFactory(factory.Factory):
    class Meta:
        model = User

    role = 'regular'

    is_admin = factory.trait(lambda user: user.role = 'admin)

    # a dict might be more useful when using multiple attributes 

    is_admin = factory.trait({ "role": "admin" })

you can then use it like any other attribute

admin_user = UserFactory.build(is_admin=True)

another usage for traits is that you can use them in a subclass like any other attribute.
so maybe we can do:

class AdminUserFactory(UserFactory):
     class Meta:
        model = User
        traits = ('is_admin', )  # same as doing UserFactory.build(is_admin=True)

@rbarrois
Thoughts?

@rbarrois

This comment has been minimized.

Show comment
Hide comment
@rbarrois

rbarrois Feb 11, 2016

Member

OK, so the idea would be that "a trait is a set of extra declarations for a factory, that can be simply toggled through a simple boolean field"?

How should we handle mixing a few traits?
For instance, with this declaration:

class AccountFactory(factory.Factory):
    class Meta:
        model = Account

    group = factory.SubFactory(GroupFactory)
    is_admin = factory.Trait(group__name='admin', is_staff=True)
    is_superuser = factory.Trait(group__name='superusers', is_staff=True, is_superuser=True)

What should we get when calling AccountFactory(is_admin=True, is_superuser=True): is the group name 'admin' or 'superusers'?

Member

rbarrois commented Feb 11, 2016

OK, so the idea would be that "a trait is a set of extra declarations for a factory, that can be simply toggled through a simple boolean field"?

How should we handle mixing a few traits?
For instance, with this declaration:

class AccountFactory(factory.Factory):
    class Meta:
        model = Account

    group = factory.SubFactory(GroupFactory)
    is_admin = factory.Trait(group__name='admin', is_staff=True)
    is_superuser = factory.Trait(group__name='superusers', is_staff=True, is_superuser=True)

What should we get when calling AccountFactory(is_admin=True, is_superuser=True): is the group name 'admin' or 'superusers'?

@rbarrois rbarrois removed the NeedInfo label Feb 11, 2016

@yelled3

This comment has been minimized.

Show comment
Hide comment
@yelled3

yelled3 Feb 12, 2016

@rbarrois although not ideal, I'm ok with just letting them override each other and adding a note on the documentation regarding this.

the way I look at traits, is that they are a shorthand to subclassing a factory and overriding some of the attributes with default values.

e.g

class UserFactory(factory.Factory):
    class Meta:
        model = User

    role = 'regular'

class AdminUserFactory(UserFactory):
     class Meta:
        model = User

     role = 'admin'

so instead of doing this, you can just use a trait like I showed before.
so in your example;
a User can't really be both is_admin=True AND is_superuser - it's up to you to use traits correctly.

WDYT?

yelled3 commented Feb 12, 2016

@rbarrois although not ideal, I'm ok with just letting them override each other and adding a note on the documentation regarding this.

the way I look at traits, is that they are a shorthand to subclassing a factory and overriding some of the attributes with default values.

e.g

class UserFactory(factory.Factory):
    class Meta:
        model = User

    role = 'regular'

class AdminUserFactory(UserFactory):
     class Meta:
        model = User

     role = 'admin'

so instead of doing this, you can just use a trait like I showed before.
so in your example;
a User can't really be both is_admin=True AND is_superuser - it's up to you to use traits correctly.

WDYT?

@rbarrois

This comment has been minimized.

Show comment
Hide comment
@rbarrois

rbarrois Feb 14, 2016

Member

That looks pretty neat to me :)
And regarding conflicting traits, we can say that traits are applied in alphabetical order, which clarifies the behavior.

So, now we'll just have to implement this ;)
This will require some changes into, probably, factory.base.BaseFactory.attributes() or factory.containers.AttributeBuilder, has the set of active traits will have to alter the passed-in declaration.

Are you interested in building this? I could guide you through the code ;)

Member

rbarrois commented Feb 14, 2016

That looks pretty neat to me :)
And regarding conflicting traits, we can say that traits are applied in alphabetical order, which clarifies the behavior.

So, now we'll just have to implement this ;)
This will require some changes into, probably, factory.base.BaseFactory.attributes() or factory.containers.AttributeBuilder, has the set of active traits will have to alter the passed-in declaration.

Are you interested in building this? I could guide you through the code ;)

@yelled3

This comment has been minimized.

Show comment
Hide comment
@yelled3

yelled3 Feb 15, 2016

I would - but I can't say when I'll have time to commit to this, sorry :-/

yelled3 commented Feb 15, 2016

I would - but I can't say when I'll have time to commit to this, sorry :-/

@iwz

This comment has been minimized.

Show comment
Hide comment
@iwz

iwz Feb 16, 2016

This would be an very nice enhancement to factory_boy. @rbarrois have you started on implementation?

iwz commented Feb 16, 2016

This would be an very nice enhancement to factory_boy. @rbarrois have you started on implementation?

@rbarrois

This comment has been minimized.

Show comment
Hide comment
@rbarrois

rbarrois Feb 21, 2016

Member

@iwz nope, I'm short on time recently.

I will happily guide someone willing to implement this feature, but I have other almost-finished ideas around that I'd like to implement first.

Member

rbarrois commented Feb 21, 2016

@iwz nope, I'm short on time recently.

I will happily guide someone willing to implement this feature, but I have other almost-finished ideas around that I'd like to implement first.

@iwz

This comment has been minimized.

Show comment
Hide comment
@iwz

iwz Apr 3, 2016

Nice! Thanks @rbarrois!!

iwz commented Apr 3, 2016

Nice! Thanks @rbarrois!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment