-
Notifications
You must be signed in to change notification settings - Fork 3
Add dataset filter methods #26
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -106,6 +106,34 @@ item.created? # => true | |||||||||||||||||||||||||
| item.selected? # => false | ||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ### Filter methods | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Plugin can define dataset methods for all enum values: | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ```ruby | ||||||||||||||||||||||||||
| Item.plugin :enum_values, filter_methods: true # default is `false` | ||||||||||||||||||||||||||
|
Comment on lines
+109
to
+114
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I agree, what about such changes then?
Suggested change
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Item.dataset.one # => #<Sequel::Dataset: "SELECT * FROM \"items\" WHERE (\"type\" = 'one')"> | ||||||||||||||||||||||||||
| Item.dataset.another # => #<Sequel::Dataset: "SELECT * FROM \"items\" WHERE (\"type\" = 'another')"> | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Item.dataset.created # => #<Sequel::Dataset: "SELECT * FROM \"items\" WHERE (\"status\" = 'created')"> | ||||||||||||||||||||||||||
| Item.dataset.selected # => #<Sequel::Dataset: "SELECT * FROM \"items\" WHERE (\"status\" = 'selected')"> | ||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Or just for specific fields: | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ```ruby | ||||||||||||||||||||||||||
| Item.plugin :enum_values, filter_methods: %i[status] | ||||||||||||||||||||||||||
| # or just `:status` for single value | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| item = Item.new(type: 'one', status: 'created') | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Item.dataset.one # => NoMethodError | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Item.dataset.created # => #<Sequel::Dataset: "SELECT * FROM \"items\" WHERE (\"status\" = 'created')"> | ||||||||||||||||||||||||||
| Item.dataset.selected # => #<Sequel::Dataset: "SELECT * FROM \"items\" WHERE (\"status\" = 'selected')"> | ||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ## Development | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| After checking out the repo, run `bundle install` to install dependencies. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -23,6 +23,9 @@ def self.apply(model, _options = {}) | |||||
| ## @option options [Boolean, Symbol, Array<Symbol>] | ||||||
| ## predicate_methods (false) | ||||||
| ## enum fields for which predicate methods will be defined | ||||||
| ## @option options [Boolean, Symbol, Array<Symbol>] | ||||||
| ## filter_methods (false) | ||||||
| ## enum fields for which dataset filter methods will be defined | ||||||
| ## @example Disable caching | ||||||
| ## Item.plugin :enum_values, caching: false | ||||||
| ## @example Define predicate methods for all enum fields | ||||||
|
|
@@ -31,18 +34,17 @@ def self.apply(model, _options = {}) | |||||
| ## Item.plugin :enum_values, predicate_methods: %[status type] | ||||||
| ## @example Define predicate methods for specific enum field | ||||||
| ## Item.plugin :enum_values, predicate_methods: :status | ||||||
| ## @example Define filter methods for all enum fields | ||||||
| ## Item.plugin :enum_values, filter_methods: true | ||||||
| ## @example Define filter methods for specific enum field | ||||||
| ## Item.plugin :enum_values, filter_methods: :status | ||||||
| ## @example Define filter methods for specific enum fields | ||||||
| ## Item.plugin :enum_values, filter_methods: %[status type] | ||||||
| def self.configure(model, options = {}) | ||||||
| model.instance_exec do | ||||||
| @enum_values_caching = options.fetch(:caching, @enum_values_caching) | ||||||
|
|
||||||
| predicate_methods = options.fetch(:predicate_methods, false) | ||||||
| @enum_values_options = options | ||||||
|
|
||||||
| transform_predicate_methods_to_enum_fields(predicate_methods) | ||||||
| .each do |field| | ||||||
| all_enum_fields[field][:enum_values].each do |enum_value| | ||||||
| define_predicate_method field, enum_value | ||||||
| end | ||||||
| end | ||||||
| process_enum_values_options | ||||||
WorstOfAny marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| end | ||||||
| end | ||||||
|
|
||||||
|
|
@@ -68,20 +70,30 @@ def enum_values(field) | |||||
|
|
||||||
| private | ||||||
|
|
||||||
| def transform_predicate_methods_to_enum_fields(predicate_methods) | ||||||
| case predicate_methods | ||||||
| when TrueClass | ||||||
| all_enum_fields.keys | ||||||
| when FalseClass | ||||||
| [] | ||||||
| else | ||||||
| Array(predicate_methods) | ||||||
| def process_enum_values_options | ||||||
| @enum_values_caching = | ||||||
| @enum_values_options.fetch(:caching, @enum_values_caching) | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It fits in single line, as I see. |
||||||
|
|
||||||
| process_enum_values_option(:filter_methods) do |field, value| | ||||||
| dataset.methods.include?(value.to_sym) && | ||||||
| warn("WARNING: Redefining method #{value}") | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
|
|
||||||
| dataset_module { where value, field => value } | ||||||
| end | ||||||
|
|
||||||
| process_enum_values_option(:predicate_methods) do |field, value| | ||||||
| define_method("#{value}?", -> { public_send(field) == value }) | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
But is here a really need to shorten this non-related code? |
||||||
| end | ||||||
| end | ||||||
|
|
||||||
| def define_predicate_method(field, enum_value) | ||||||
| define_method "#{enum_value}?" do | ||||||
| public_send(field) == enum_value | ||||||
| def process_enum_values_option(key) | ||||||
| return unless (option = @enum_values_options[key]) | ||||||
|
|
||||||
| enum_fields = all_enum_fields | ||||||
| enum_fields = enum_fields.slice(*option) unless option == true | ||||||
|
|
||||||
| enum_fields.each do |field, field_schema| | ||||||
| field_schema[:enum_values].each { |value| yield field, value } | ||||||
| end | ||||||
| end | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A small naming suggestion, we can discuss it, but this is what I saw:
You're calling them
dataset methods, it's actuallyDataset's methods, but you named the optionfilter_methods. A bit confusing, and I see no problem withdataset_methodsoption: I don't think that there will be other dataset methods, and it's all about enums.What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i can do it, but for me it's like change name of
predicate_methodstoinstance_methods. We can't filter an instance, only collection or dataset, so that's whyfilter_methodswas usedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i thought that
dataset_methodsisn't clear enough naming.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My response (with agreement and suggestion) somwhy moved below: #26 (comment)