shoulda-matchers: PR #829 breaks "define_enum_for" matcher for enums that do not actually use integer columns
Say we want a status enum that looks like
enum status: { active: "active", pending: "pending", rejected: "rejected" }
With the changes made in PR #829, we can no longer spec for that enum with
it { should define_enum_for(:status).with(active: "active", pending: "pending", rejected: "rejected")
Are active record enums with strings as underlying column types not allowed? I couldn’t find any information that would explicitly make it so.
I believe the matcher should only enforce integer type if the provided expected enum value is an array, or if it’s a hash with values all being integer.
About this issue
- Original URL
- State: closed
- Created 8 years ago
- Reactions: 26
- Comments: 15
@valscion Yes! Sorry this took me so long. A fix for this is now in the master branch. To address @begedin’s original problem, you’d be able to fix this by saying:
I agree. There is absolutely no reason to enforce integers.
It started with #827, but in the author’s example, he is storing an
:integer
in a column of type:string
, and his enum maps the keys to integers. If he wanted to use integers, he could just use an array of symbols.Had he done that, than yes, the matcher could enforce integer type in case of array, or enforce nothing otherwise.
Sorry for the lack of reply on this, I’ve been out.
I agree that this is a bug. I’m not sure why we had the matcher assume integers except that that was the original use case.
I can’t say when I’ll get to this, but I’ve at least seen it. 😃
@rafaeldev The
backed_by_column_of_type
qualifier is available in 4.0.0! Upgrade to get it.@guilhermearaujo yeah, that’s what i meant, sorry for that.
Going through the commit history for the enum module, i finally found a test for strings as enum values. (https://github.com/rails/rails/commit/67c1719012506c3387df067961252b5df50a97ce)
Since there was never any mention about using string as values in the documentation or code I assumed that the behaviour of using strings for values is rather a coincidence.
Therefore I agree, that shoulda should not decide if the values used for enums should be integer or string.
(@tiii I suppose that by ‘key’ in your message you mean ‘value’, as in the value held by the key being a string, and not the key itself. Is that correct?)
I have to disagree. If your database has a
varchar
column to store your enum data and your enum maps asymbol
to aninteger
, IMHO you are doing it wrong. On the other hand, if your column is of typeinteger
and your enum maps asymbol
to astring
, you are doing it wrong too.In my opinion, if you have:
you should create a column of type
varchar
;Accordingly, if you have:
then your column should be of type
integer
. As simple as that.Should/can Rails be smart enough to make the required type conversions? Possibly yes. Should all enums have
integer
values? This is debatable. Is it up to shoulda-matchers decide whether they should beinteger
orstring
? My opinion is: definitely not.