Skip to content

Sometimes you need to invert boolean field so you need to use 0 as a che... #991

Merged
merged 1 commit into from Jul 26, 2014

3 participants

@hexdigest

Sometimes you need to invert meaning of boolean attribute on a form, for example display model.enabled attribute as "Disable" checkbox. To do this you need to invert the meaning of an attribute. You can do it by setting

checked_value: 0, unchecked_value: 1

but unfortunately current version of formtastic would not work becaus 0 is hardcoded in boolean_checked? method for integer values.

@justinfrench
Owner

Interesting!

Did you review the tests to ensure we don't need any extra coverage? If I'm understanding you correctly, I think you have MyModel#enabled as a DB field, but you'd like to invert this for the views, with a MyModel#disabled. It's easy for you to re-label the field on the form, but it's difficult for you to invert the 0 and 1 values.

I'm a little bit uneasy that we'd encourage this — it feels like an ant-pattern, feels like a future "WTF moment" when they discover that 1 could mean 0 in this special case.

Why wouldn't you create an MyModel#disabled accessor and handle the inversion model-side in a before_validation callback?

@hexdigest

Did you review the tests to ensure we don't need any extra coverage?

Nope, I know I should :)

Well I partially agree with you and I had many doubts before I made this pull request :)
But to me it is seems more like broken "checked_value/unchecked_value" functionality and it's more about presentation then model.

when they discover that 1 could mean 0 in this special case.

yes, but only when they use 0 for checked_value

we can make additional check for this particular case

0 == checked_value ? value == 0 : value != 0

so it will work as before unless checked_value is set to 0

Anyway it's up to you buddy I can leave with monkey patch :)

@justinfrench
Owner

That looks like the cleverest line of code ever, please don't commit that ;) I'm convinced this is a model or presenter problem rather than a view problem, but I could be swayed if some other voices want to chime in. @sobrinho?

@sobrinho
Collaborator

I agree that may be handled by the model or a form object but since I needed exactly the same thing in a project a while ago, I think this should be fixed.

While this issue report the problem for boolean, the issue will happen for any other value than 0 and 1 that we may want to use for checked and unchecked.

👍

But we need at least one test case to ensure it will be kept working in future releases.

@justinfrench
Owner

Ok, good point, let's get this done. Thanks!

@justinfrench
Owner

Can we get a test case for this? Would love to merge.

@justinfrench justinfrench merged commit acdccf5 into justinfrench:master Jul 26, 2014

1 check passed

Details default The Travis CI build passed
@justinfrench
Owner

Added my own spec, merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.