Skip to content
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

[1452] update traits_for_enum to accept _prefix and _suffix #1513

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
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
10 changes: 6 additions & 4 deletions lib/factory_bot/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def initialize(name, base_traits = [])
@declarations = DeclarationList.new(name)
@callbacks = []
@defined_traits = Set.new
@registered_enums = []
@registered_enums = {}
@to_create = nil
@base_traits = base_traits
@additional_traits = []
Expand Down Expand Up @@ -86,7 +86,7 @@ def define_trait(trait)
end

def register_enum(enum)
@registered_enums << enum
@registered_enums[enum.attribute_name.to_sym] = enum
end

def define_constructor(&block)
Expand Down Expand Up @@ -164,7 +164,7 @@ def expand_enum_traits(klass)
automatically_register_defined_enums(klass)
end

registered_enums.each do |enum|
registered_enums.each do |_name, enum|
traits = enum.build_traits(klass)
traits.each { |trait| define_trait(trait) }
end
Expand All @@ -173,7 +173,9 @@ def expand_enum_traits(klass)
end

def automatically_register_defined_enums(klass)
klass.defined_enums.each_key { |name| register_enum(Enum.new(name)) }
klass.defined_enums.each_key do |name|
register_enum(Enum.new(name)) unless registered_enums.key? name.to_sym
end
end

def automatically_register_defined_enums?(klass)
Expand Down
58 changes: 56 additions & 2 deletions lib/factory_bot/definition_proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,50 @@ def trait(name, &block)
# end
# end
#
# Prefix and Suffixes can be used for trait definition.
# both can be set to true (prefix/suffix will be set to enum name), or to a custom string
#
# Example:
# factory :task do
# traits_for_enum :status, [:started, :finished], _prefix: true
# end
#
# Equivalent to:
# factory :task do
# traits_for_enum :status, [:started, :finished], _prefix: 'status'
# end
#
# Both Equivalent to:
# factory :task do
# trait :status_started do
# status { :started }
# end
#
# trait :status_finished do
# status { :finished }
# end
# end
#
# Example:
# factory :task do
# traits_for_enum :status, [:started, :finished], _suffix: true
# end
#
# Equivalent to:
# factory :task do
# traits_for_enum :status, [:started, :finished], _suffix: 'status'
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth pointing out we would need to update the docs here if we end up using the signature without the hash arguments.

# end
#
# Both Equivalent to:
# factory :task do
# trait :started_status do
# status { :started }
# end
#
# trait :finished_status do
# status { :finished }
# end
# end
#
# Arguments:
# attribute_name: +Symbol+ or +String+
Expand All @@ -230,8 +274,18 @@ def trait(name, &block)
# those traits. When this argument is not provided, factory_bot will
# attempt to get the values by calling the pluralized `attribute_name`
# class method.
def traits_for_enum(attribute_name, values = nil)
@definition.register_enum(Enum.new(attribute_name, values))
def traits_for_enum(attribute_name, values = nil, options = {})
if options.empty? && values.is_a?(Hash)
prefix = values.delete(:_prefix) { false }
suffix = values&.delete(:_suffix) { false }
values = nil if values.empty?
enum = Enum.new(attribute_name, values, prefix: prefix, suffix: suffix)
else
prefix = options.fetch(:_prefix, false)
suffix = options.fetch(:_suffix, false)
enum = Enum.new(attribute_name, values, prefix: prefix, suffix: suffix)
end
@definition.register_enum enum
end

def initialize_with(&block)
Expand Down
17 changes: 15 additions & 2 deletions lib/factory_bot/enum.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
module FactoryBot
# @api private
class Enum
def initialize(attribute_name, values = nil)
def initialize(attribute_name, values = nil, prefix: false, suffix: false)
@attribute_name = attribute_name
@values = values
@prefix = if prefix == true
"#{@attribute_name}_"
elsif prefix
"#{prefix}_"
end

@suffix = if suffix == true
"_#{@attribute_name}"
elsif suffix
"_#{suffix}"
end
end

def build_traits(klass)
Expand All @@ -12,14 +23,16 @@ def build_traits(klass)
end
end

attr_reader :attribute_name
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we using this anywhere?

Copy link
Author

@tjbarker tjbarker Oct 2, 2021

Choose a reason for hiding this comment

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

This was being used in lib/factory_bot/definition.rb to help with not redefining traits in the automatically_register_defined_enums method, it is now used in the Definition#register_enum method as the key in the registered_enum hash

Copy link
Author

Choose a reason for hiding this comment

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

this is now also being used in the registered_enums hash as the key to look up to ensure enums are not registered twice


private

def enum_values(klass)
@values || klass.send(@attribute_name.to_s.pluralize)
end

def build_trait(trait_name, attribute_name, value)
Trait.new(trait_name) do
Trait.new("#{@prefix}#{trait_name}#{@suffix}") do
add_attribute(attribute_name) { value }
end
end
Expand Down
205 changes: 205 additions & 0 deletions spec/acceptance/enum_traits_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,211 @@ def each(&block)
expect(task.status).to eq(trait_name)
end
end

context 'when prefix is used' do
context 'when values are a hash' do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do prefix and suffixes interact differently with a hash vs. list, or is there something specific we're trying to cover by testing against both? If not, I think we could choose one or the other to test with, since we've already got coverage of the various enumerable types.

Copy link
Author

Choose a reason for hiding this comment

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

With the current form of the code, no they don't behave differently, however with hash being passed in one of the tests (enum_for_trait_spec.rb:60) was failing as the keynames of the hash didn't match with the prefix, suffix options, so i chose to leave the 2 different tests in (for now at least) to ensure i pushed a solution that worked for all implementation types of the api

context 'when prefix is a boolean' do
it "builds traits for each enumerated value, applies default prefix and does not build the auto enum trait" do
statuses = { queued: 0, started: 1, finished: 2 }
define_model("Task", status: :integer) do
enum status: statuses, _prefix: true
end

FactoryBot.define do
factory :task do
traits_for_enum :status, _prefix: true
end
end

statuses.each_key do |trait_name|
task = FactoryBot.build(:task, "status_#{trait_name}")

expect{ FactoryBot.build(:task, trait_name) }.to raise_error KeyError, /Trait not registered/
expect(task.status).to eq(trait_name.to_s)
end

Task.reset_column_information
end
end

context 'when prefix is a string' do
it "builds traits for each enumerated value, applies custom prefix and does not build the auto enum trait" do
statuses = { queued: 0, started: 1, finished: 2 }
define_model("Task", status: :integer) do
enum status: statuses, _prefix: 'foobar'
end

FactoryBot.define do
factory :task do
traits_for_enum :status, _prefix: 'foobar'
end
end

statuses.each_key do |trait_name|
task = FactoryBot.build(:task, "foobar_#{trait_name}")

expect{ FactoryBot.build(:task, trait_name) }.to raise_error KeyError, /Trait not registered/
expect(task.status).to eq(trait_name.to_s)
end

Task.reset_column_information
end
end
end

context 'when values are a list' do
context 'when prefix is a boolean' do
it "builds traits for each enumerated value, applies default prefix and does not build the auto enum trait" do
statuses = %w[queued started finished]

define_model("Task", status: :integer) do
enum status: statuses, _prefix: true
end

FactoryBot.define do
factory :task do
traits_for_enum :status, _prefix: true
end
end

statuses.each do |trait_name|
task = FactoryBot.build(:task, "status_#{trait_name}")

expect{ FactoryBot.build(:task, trait_name) }.to raise_error KeyError, /Trait not registered/
expect(task.status).to eq(trait_name)
end

Task.reset_column_information
end
end

context 'when prefix is a string' do
it "builds traits for each enumerated value, applies custom prefix and does not build the auto enum trait" do
statuses = %w[queued started finished]
define_model("Task", status: :integer) do
enum status: statuses, _prefix: 'foobar'
end

FactoryBot.define do
factory :task do
traits_for_enum :status, _prefix: 'foobar'
end
end

statuses.each do |trait_name|
task = FactoryBot.build(:task, "foobar_#{trait_name}")

expect{ FactoryBot.build(:task, trait_name) }.to raise_error KeyError, /Trait not registered/
expect(task.status).to eq(trait_name)
end

Task.reset_column_information
end
end
end
end

context 'when suffix is used' do
context 'when values are a hash' do
context 'when suffix is a boolean' do
it "builds traits for each enumerated value, applies default suffix and does not build the auto enum trait" do
statuses = { queued: 0, started: 1, finished: 2 }
define_model("Task", status: :integer) do
enum status: statuses, _suffix: true
end

FactoryBot.define do
factory :task do
traits_for_enum :status, _suffix: true
end
end

statuses.each_key do |trait_name|
task = FactoryBot.build(:task, "#{trait_name}_status")

expect{ FactoryBot.build(:task, trait_name) }.to raise_error KeyError, /Trait not registered/
expect(task.status).to eq(trait_name.to_s)
end

Task.reset_column_information
end
end

context 'when suffix is a string' do
it "builds traits for each enumerated value, applies custom suffix and does not build the auto enum trait" do
statuses = {queued: 0, started: 1, finished: 2}
define_model("Task", status: :integer) do
enum status: statuses, _suffix: 'foobar'
end

FactoryBot.define do
factory :task do
traits_for_enum :status, _suffix: 'foobar'
end
end

statuses.each do |trait_name, _trait_value|
task = FactoryBot.build(:task, "#{trait_name}_foobar")

expect{ FactoryBot.build(:task, trait_name) }.to raise_error KeyError, /Trait not registered/
expect(task.status).to eq(trait_name.to_s)
end

Task.reset_column_information
end
end
end

context 'when values are a list' do
context 'when suffix is a boolean' do
it "builds traits for each enumerated value, applies default suffix and does not build the auto enum trait" do
statuses = %w[queued started finished]
define_model("Task", status: :integer) do
enum status: statuses, _suffix: true
end

FactoryBot.define do
factory :task do
traits_for_enum :status, _suffix: true
end
end

statuses.each do |trait_name|
task = FactoryBot.build(:task, "#{trait_name}_status")

expect{ FactoryBot.build(:task, trait_name) }.to raise_error KeyError, /Trait not registered/
expect(task.status).to eq(trait_name)
end

Task.reset_column_information
end
end

context 'when suffix is a string' do
it "builds traits for each enumerated value, applies custom suffix and does not build the auto enum trait" do
statuses = %w[queued started finished]
define_model("Task", status: :integer) do
enum status: statuses, _suffix: 'foobar'
end

FactoryBot.define do
factory :task do
traits_for_enum :status, _suffix: 'foobar'
end
end

statuses.each do |trait_name|
task = FactoryBot.build(:task, "#{trait_name}_foobar")

expect(task.status).to eq(trait_name)
expect{ FactoryBot.build(:task, trait_name) }.to raise_error KeyError, /Trait not registered/
end

Task.reset_column_information
end
end
end
end
end

context "when automatically_define_enum_traits is false" do
Expand Down
4 changes: 2 additions & 2 deletions spec/factory_bot/definition_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@
it "maintains a list of enum fields" do
definition = described_class.new(:name)

enum_field = double("enum_field")
enum_field = double("enum_field", attribute_name: 'enum')

definition.register_enum(enum_field)

expect(definition.registered_enums).to include(enum_field)
expect(definition.registered_enums[:enum]).to eq enum_field
end
end