Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Copy link
Owner

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 actually Dataset's methods, but you named the option filter_methods. A bit confusing, and I see no problem with dataset_methods option: I don't think that there will be other dataset methods, and it's all about enums.

What do you think?

Copy link
Author

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_methods to instance_methods. We can't filter an instance, only collection or dataset, so that's why filter_methods was used

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i thought that dataset_methods isn't clear enough naming.

Copy link
Owner

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)

Comment on lines +109 to +114
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I agree, what about such changes then?

Suggested change
### Filter methods
Plugin can define dataset methods for all enum values:
```ruby
Item.plugin :enum_values, filter_methods: true # default is `false`
### Dataset filter methods
Plugin can define dataset filter methods for all enum values:
```ruby
Item.plugin :enum_values, filter_methods: true # default is `false`


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.
Expand Down
52 changes: 32 additions & 20 deletions lib/sequel/plugins/enum_values.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
end
end

Expand All @@ -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)
Copy link
Owner

Choose a reason for hiding this comment

The 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}")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Approach with && for condition very unclear, please use if/unless.
  2. I see no reason for parentheses of warn.
  3. It's a pretty good idea to warn, but I suggest to make it even strict: let raise an exception! And see what will happens with users.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. And there are should be tests for this behavior!


dataset_module { where value, field => value }
end

process_enum_values_option(:predicate_methods) do |field, value|
define_method("#{value}?", -> { public_send(field) == value })
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
define_method("#{value}?", -> { public_send(field) == value })
define_method("#{value}?") { public_send(field) == value }

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

Expand Down
119 changes: 98 additions & 21 deletions spec/sequel/plugins/enum_values_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal:true

describe Sequel::Plugins::EnumValues do
type_enum_values = %w[first second third]
type_enum_values = %w[one another yet_another]
status_enum_values = %w[created selected canceled]
item_enum_values = type_enum_values + status_enum_values

Expand Down Expand Up @@ -71,6 +71,10 @@
end

describe '.configure' do
let(:enum_values_options) { {} }

before { item_class.plugin :enum_values, **enum_values_options }

describe 'caching option' do
shared_examples 'works correctly' do
before do
Expand Down Expand Up @@ -107,10 +111,7 @@
5.times { item_class.enum_values(:type) }
end

before do
item_class.plugin :enum_values, caching: true
end

let(:enum_values_options) { { caching: true } }
let(:all_enum_fields_received_times) { 1 }
let(:schema_received_times) { 1 }

Expand All @@ -122,10 +123,7 @@
5.times { item_class.enum_values(:type) }
end

before do
item_class.plugin :enum_values, caching: false
end

let(:enum_values_options) { { caching: false } }
let(:all_enum_fields_received_times) { 5 }
let(:schema_received_times) { 5 }

Expand Down Expand Up @@ -180,29 +178,23 @@
end

context 'with true value' do
before do
item_class.plugin :enum_values, predicate_methods: true
end
let(:enum_values_options) { { predicate_methods: true } }

include_examples 'for all enum predicate methods',
item_enum_predicate_methods,
-> { expect(subject).to be false }
end

context 'with false value' do
before do
item_class.plugin :enum_values, predicate_methods: false
end
let(:enum_values_options) { { predicate_methods: false } }

include_examples 'for all enum predicate methods',
item_enum_predicate_methods,
-> { expect { subject }.to raise_error(NoMethodError) }
end

context 'with Array value' do
before do
item_class.plugin :enum_values, predicate_methods: %i[status]
end
let(:enum_values_options) { { predicate_methods: %i[status] } }

include_examples 'for all enum predicate methods',
status_enum_predicate_methods,
Expand All @@ -214,9 +206,7 @@
end

context 'with Symbol value' do
before do
item_class.plugin :enum_values, predicate_methods: :status
end
let(:enum_values_options) { { predicate_methods: :status } }

include_examples 'for all enum predicate methods',
status_enum_predicate_methods,
Expand All @@ -227,6 +217,93 @@
-> { expect { subject }.to raise_error(NoMethodError) }
end
end

shared_context('with filter_methods environment') do
subject(:enum_filter) { item_class.dataset.public_send(enum_value) }

let(:filter_sql) do
"SELECT * FROM \"items\" WHERE (\"#{field}\" = '#{enum_value}')"
end
end

shared_examples('returns correct sql') do |field, field_value|
subject { enum_filter.sql }

describe field_value do
let(:field) { field }
let(:enum_value) { field_value }

it { is_expected.to eq filter_sql }
end
end

shared_examples('raises NoMethodError') do |field_value|
describe field_value do
let(:enum_value) { field_value }

it { expect { enum_filter }.to raise_error(NoMethodError) }
end
end

describe 'filter_methods default option' do
include_context('with filter_methods environment')

item_enum_values.each do |filter_method_name|
include_examples('raises NoMethodError', filter_method_name)
end
end

describe 'filter_methods true option' do
let(:enum_values_options) { { filter_methods: true } }

include_context('with filter_methods environment')

type_enum_values.each do |filter_method_name|
include_examples('returns correct sql', :type, filter_method_name)
end

status_enum_values.each do |filter_method_name|
include_examples('returns correct sql', :status, filter_method_name)
end
end

describe 'filter_methods false option' do
let(:enum_values_options) { { filter_methods: false } }

include_context('with filter_methods environment')

item_enum_values.each do |filter_method_name|
include_examples('raises NoMethodError', filter_method_name)
end
end

describe 'filter_methods Array option' do
let(:enum_values_options) { { filter_methods: %i[status] } }

include_context('with filter_methods environment')

status_enum_values.each do |filter_method_name|
include_examples('returns correct sql', :status, filter_method_name)
end

type_enum_values.each do |filter_method_name|
include_examples('raises NoMethodError', filter_method_name)
end
end

describe 'filter_methods Symbol option' do
let(:enum_values_options) { { filter_methods: :status } }

include_context('with filter_methods environment')

status_enum_values.each do |filter_method_name|
include_examples('returns correct sql', :status, filter_method_name)
end

type_enum_values.each do |filter_method_name|
include_examples('raises NoMethodError', filter_method_name)
end
end
end
end
end
Expand Down