simple_enum
simple_enum copied to clipboard
Ability to create selectors passing symbols
Let's say we have a simple Mongoid model:
class Article
include SimpleEnum::Mongoid
VALID_TYPES = { blog: 0, vlog: 1, xlog: 2 }
# ...
as_enum :type, VALID_TYPES, source: :type
end
As far as I can see (correct me if I'm wrong!), it's not possible to query using a selector that takes the symbol corresponding to a value of an enum, like this:
Article.where(type: :blog).count
while instead is possible to do this:
Article.where(type: 0).count
Article.where(type: Article::VALID_TYPES[:blog]).count # or this, don't like it though
I'm aware of the fact that it's possible to use chained selectors, like this:
Article.blogs.count
but in our codebase we often build selectors as hashes that are passed to instances of different classes.
Besides this maybe not being the best approach of all times, IMHO it'd be very beneficial to be able to consume the gem in a consistent way, always passing the symbol (key of the hash), not the value.
Do you think it's a complicated matter? Can you give me pointers to where to look at, so maybe I can submit a PR?
That can actually become nasty !
User.find_or_create_by(name: "Test user", gender: :male)
... I was wondering why I had so many users in my test database :P
(as_enum: :gender
)
Personally I really think we should not override things like e.g. where
and mess with how selection works.
You're right. But maybe we could define an extra convenience method that preprocesses the inputs so as to replace "meaningful enums" by their "meaningless database representation"
Something like that
def preprocess_enums_in_hash(class, hash={})
preprocessed_hash = {}
hash.each do |key,val|
# The key corresponds to an enum value ? if yes convert it to {key_cd, integer_value_of(value)}
# EX : {gender: :male} should be converted to {gender_cd: 0} because we know :gender is an enum declared in the class class
if is_enum_of(class, key)?
enum_symbol = "#{key.to_s}_cd" # Also handle the case where enums are prefixed
enum_value = integer_value_of_enum(class, enum_symbol, val)
preprocessed_hash << {:enum_symbol => enum_value}
else
# Don't modify the current (key, val) of the hash
preprocessed_hash << {key => val}
end
end
preprocessed_hash
end
Then
Person.where(preprocess_enums_in_hash(Person, {age: 21, gender: :male}
Would become
Person.where(age: 21, gender_cd: 0}
All right if somebody can start a Pull Request I'm happy to merge, one thing I'd prefer it to be optional, so that an additional file needs to be required or something and that it registers itself as extension.
See https://github.com/lwe/simple_enum/blob/e65073193ed4a50bb1bac365b1620eb5a8123650/lib/simple_enum/attribute.rb#L85-96
Update: from an "API" perspective I'd rather have it be something like Person.expand_enums_in_hash
, rather than an external utility, or not? Also a good / clear method name needs to be found :smile:
I vote for the Person.expand_enums_in_hash
approach. The other option (preprocess_enums_in_hash(Person, {age: 21, gender: :male})
) would feel unnatural to use (and hard to read).
As per the name, something like .enums_to_selector
?
I vote for expand_enums_in_hash
(unless better choice appears)
.enums_to_selector
makes it feel like we're going to use the hash for a DB query, which isn't necessarily the case ? (Haven't used SimpleEnum outside the scope of mongoid/active_record, but then it's possible)
In my mind, it was external because it reminded me of enum_option_pairs(class_name, enum)
, but it does make more sense to have Person.expand_enums_in_hash
@Startouf as per the nature of the gem, I was imagining it used for DB queries only, otherwise a simple to_hash
would be enough.
Also, in my mind, the method should also take care of dealing with lists of values and translate them into the right DB selector, e.g.:
Person.enums_to_selector(gender: :male) # --> { gender: 0 }
Person.enums_to_selector(gender: [:male, :female]) # --> { :gender.in => [0, 1] }
Hence the name I came up with.
Does this make sense to everybody?
We should go with enums_to_selector
, because as @lucke84 pointed out this method is only ever used in combination within a where
clause.
I believe ActiveRecord/Mongoid are already doing the transformations from {gender: [0, 1]}
to {:gender.in => [0,1]}
Yes, correct, I think at the moment we should focus on the following:
{ gender: :male } # => { gender: 0 }
{ gender: [:male, :female] } # => { gender: [0, 1] }
{ foo: "bar", gender: :female } # => { foo: "bar", gender: 1 }
Do we need the inverse operation ?
.enums_to_int / .int_to_enums
.expand_enums
.meaningless (enum to int) /.meaningful (int to enum)
.as_int / .as_enum (name collision ?)
No, I don't think we need the inversions.
An intriguing idea was given in #101, to basically add a method like
Klass.gender_scope(:female) # => where(gender_cd: 1)
this could be extend to allow both single arguments or hashes, with the expansion / support as described above?
Klass.gender_scope(gender: :male, foo: "bar") # => where(gender_cd: 0, foo: "bar")
One thing that often happens to me is to recycle a selector (not a scope) with different classes that extend/include a certain field.
The main purpose is to be able to pass a selector to another method without necessarily knowing which class is going to use it (something you can do with whatever other key/value pair).
Example:
class A
STATUSES = [:active, :paused, :inactive]
as_enum :status, STATUSES, source: :status
end
class B
include A
# more fields
end
class C
include A
# more fields
end
Somewhere else:
selector = { status: A::STATUSES[:active], another_field: 'abc' } # ideally it'd be `{ status: :active, another_field: 'abc' }`
x = B.where(selector.merge({ yet_another_field: true })).count
y = C.where(selector.merge({ one_more_field: 25 })).first
I don't think the suggested method would make easier achieving such goal, unless we provide also the ability to get the selector only.
@lucke84 why isn't it possible with the ..._scope
to reuse selectors? Like e.g.
selector = { status: :active, field: 'abc' }
x = B.xxx_scope(selector.merge(yet_another_field: true)).count
y = C.xxx_scope(selector.merge(one_more_field: 25)).first
That's right, it should work. Cool!
Can we also think about adding support for a generic enum_scope
(or where_with_enum
, or else) that doesn't necessarily know which field is gonna be involved:
e.g.:
Klass.where_with_enum({ status: :active, another_enum: :something }).first
instead of
Klass.status_scope({ status: :active, another_enum: :something }).first
Hey, so I introduced status_scope
on my project simply because I joined this team a few weeks ago, and everywhere I saw
Project.where(
status_cd: [
Project.statuses(:pending),
Project.statuses(:active)
])
So I added the scope, and now I can just say
Project.status_scope([:pending, :active])
If I want to chain these
Project.status_scope(:active).language_scope(:ruby)
I can see why it might be cool to write in a single call
Project.where_with_enum(status: :active, language: :ruby)
(In my head I'd call it enum_scope
, but I think I'm too old skool)
I should also say, I'm well into making sure that we raise an exception for any invalid values.
ie.
Project.status_scope(:doesnt_exist)
and
Project.where_with_enum(status: :doesn't_exist)
should both raise an InvalidEnum
exception.
+1 @matthewrudy that is exactly my point :)
@matthewrudy, wow, nice work 👍 and I assume you basically delegate xyz_scope
to where_with_enum(xyz: ...)
?
Any chance you can already create WIP pull request so we can take a look at the code?
I might be too late but i would prefer
- remove default scopes, ex:
Project.pendings
- add a single enum scope
Project.with_enum(status: :pending).where(...)
:+1: thanks for the input and no you are not too late, development of v3 has been kind of stale recently, sorry for that.
Update and my tendency is also towards with_enum
and not single scopes like e.g. pendings
I am for not having single scopes too. Has any work been done in this direction?
@aledalgrande, no AFAIK no work has been done into this direction yet.