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

ensure_inclusion_of(:admin).in_array([true, false]), with datatype Boolean #291

Closed
awilkening opened this Issue May 10, 2013 · 9 comments

Comments

Projects
None yet
9 participants
@awilkening

awilkening commented May 10, 2013

Using rails 3.2.13, ruby 1.9.3p194

Rails converts all input for attribute :admin to TrueClass or FalseClass if the backing column data type is Boolean.

user = User.new
user.admin = 1
user.admin
=> true
user.admin = 0
user.admin
=> false
user.admin = nil
user.admin
=> nil
user.admin = EnsureInclusionOfMatcher::ARBITRARY_OUTSIDE_STRING
user.adimn
=> false

The only purpose of the validation matcher becomes to check for allow_nil values. Everything else will always result in a true, false value. In that case, this is not the correct matcher for this validation. Perhaps some documentation for this little gotcha would be nice.

@mxie

This comment has been minimized.

Show comment
Hide comment
@mxie

mxie May 24, 2013

Contributor

Just to note: this issue has been partially discussed in #179, but I'm unsure if they're completely related, i.e. need the same solution.

Contributor

mxie commented May 24, 2013

Just to note: this issue has been partially discussed in #179, but I'm unsure if they're completely related, i.e. need the same solution.

@anthonator

This comment has been minimized.

Show comment
Hide comment
@anthonator

anthonator Jun 24, 2013

I'm seeing this problem as well.

anthonator commented Jun 24, 2013

I'm seeing this problem as well.

@StanBoyet

This comment has been minimized.

Show comment
Hide comment
@StanBoyet

StanBoyet Aug 30, 2013

So we are still "stuck" with the :

should allow_value(true)
should allow_value(false)
should_not allow_value(nil)

Am I right ? (from the #179 )

StanBoyet commented Aug 30, 2013

So we are still "stuck" with the :

should allow_value(true)
should allow_value(false)
should_not allow_value(nil)

Am I right ? (from the #179 )

@drapergeek

This comment has been minimized.

Show comment
Hide comment
@drapergeek

drapergeek Aug 30, 2013

Contributor

@StanBoyet Right now yes, I would be happy to review a PR if you have a proposed solution.

Contributor

drapergeek commented Aug 30, 2013

@StanBoyet Right now yes, I would be happy to review a PR if you have a proposed solution.

@organicweb

This comment has been minimized.

Show comment
Hide comment
@organicweb

organicweb Sep 20, 2013

@StanBoyet you could try :
should allow_value(%w(true false))

one line less :)

organicweb commented Sep 20, 2013

@StanBoyet you could try :
should allow_value(%w(true false))

one line less :)

@mpugach

This comment has been minimized.

Show comment
Hide comment
@mpugach

mpugach Dec 1, 2013

@organicweb you meant:

should allow_value(true, false)

because true and false are not strings and
you can't pass an Array to allow_value - it will be handled like a single element

mpugach commented Dec 1, 2013

@organicweb you meant:

should allow_value(true, false)

because true and false are not strings and
you can't pass an Array to allow_value - it will be handled like a single element

@mcmire

This comment has been minimized.

Show comment
Hide comment
@mcmire

mcmire Jan 8, 2014

Collaborator

So if I understand this issue correctly...

A boolean column accepts true and false at least. It may, or may not, also accept null. If you have a boolean column that does not accept null, then it would make sense to say:

it { should allow_value(nil).for(:attr) }
it { should ensure_inclusion_of(:attr).in_array([nil]) }

It would never, however, make sense to say:

it { should allow_value(true, false).for(:attr) }
it { should ensure_inclusion_of(:attr).in_array([true, false]) }

The question, then, is: Should using allow_value, where one of the values given is true or false, against a column whose type is boolean, raise an error? (Similar for ensure_inclusion_of.) Would it be helpful to receive a warning such as:

You have declared an expectation that attr should allow true or false values. We advise against this -- as attr is a boolean column, this is a redundant test.

Collaborator

mcmire commented Jan 8, 2014

So if I understand this issue correctly...

A boolean column accepts true and false at least. It may, or may not, also accept null. If you have a boolean column that does not accept null, then it would make sense to say:

it { should allow_value(nil).for(:attr) }
it { should ensure_inclusion_of(:attr).in_array([nil]) }

It would never, however, make sense to say:

it { should allow_value(true, false).for(:attr) }
it { should ensure_inclusion_of(:attr).in_array([true, false]) }

The question, then, is: Should using allow_value, where one of the values given is true or false, against a column whose type is boolean, raise an error? (Similar for ensure_inclusion_of.) Would it be helpful to receive a warning such as:

You have declared an expectation that attr should allow true or false values. We advise against this -- as attr is a boolean column, this is a redundant test.

@sarahleeashraf

This comment has been minimized.

Show comment
Hide comment
@sarahleeashraf

sarahleeashraf commented Feb 11, 2014

+1

@mcmire

This comment has been minimized.

Show comment
Hide comment
@mcmire

mcmire May 1, 2014

Collaborator

This was fixed by #428, so closing.

Collaborator

mcmire commented May 1, 2014

This was fixed by #428, so closing.

@mcmire mcmire closed this May 1, 2014

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