From d5154abe00d132479cf017243d837e31f02681ec Mon Sep 17 00:00:00 2001 From: Joel AZEMAR Date: Mon, 3 Mar 2025 12:08:11 +0800 Subject: [PATCH 01/13] Make with_dynamic_columns generic --- .../import_by_static_columns_spec.rb | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb index 35cecb9..0c19405 100644 --- a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb +++ b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb @@ -42,22 +42,23 @@ def name "DynamicColumnsImportModel" end - def with_skills(skills) + def with_dynmic_columns(collection_name:, collection:) new_class = Class.new(self) do - @skill_columns = {} + instance_variable_set(:"@#{collection_name}_columns", {}) - skills.each.with_index do |skill, index| - column_name = :"skill_#{index}" - define_skill_column(skill, name: column_name) - @skill_columns[column_name] = columns[column_name] + collection.each.with_index do |entry, index| + column_name = :"#{collection_name}_#{index}" + define_dynamic_column(entry, column_name: column_name) + instance_variable_get(:"@#{collection_name}_columns")[column_name] = columns[column_name] end class << self attr_reader :skill_columns + # attr_reader :"#{collection_name}_columns" end - def skill_columns - self.class.skill_columns + define_method(:"#{collection_name}_columns") do + self.class.send(:"#{collection_name}_columns") end end @@ -67,9 +68,9 @@ def skill_columns new_class end - def define_skill_column(skill, name:) - column(name, header: skill.name, required: false) - validates(name, inclusion: %w[0 1], allow_blank: true) + def define_dynamic_column(entry, column_name:) + column(column_name, header: entry.name, required: false) + validates(column_name, inclusion: %w[0 1], allow_blank: true) end end end @@ -88,7 +89,7 @@ def define_skill_column(skill, name:) end context "with dynamic columns" do - let(:importer_with_dynamic_columns) { import_model.with_skills(Skill.all) } + let(:importer_with_dynamic_columns) { import_model.with_dynmic_columns(collection_name: :skill, collection: Skill.all) } describe "import" do let(:csv_source) do From 7d017d1336112e40691f40e901169c8efa1848be Mon Sep 17 00:00:00 2001 From: Joel AZEMAR Date: Mon, 3 Mar 2025 12:08:31 +0800 Subject: [PATCH 02/13] Update import_by_static_columns_spec.rb --- .../import/dynamic-columns/import_by_static_columns_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb index 0c19405..f7502bd 100644 --- a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb +++ b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb @@ -42,7 +42,7 @@ def name "DynamicColumnsImportModel" end - def with_dynmic_columns(collection_name:, collection:) + def with_dynamic_columns(collection_name:, collection:) new_class = Class.new(self) do instance_variable_set(:"@#{collection_name}_columns", {}) @@ -89,7 +89,7 @@ def define_dynamic_column(entry, column_name:) end context "with dynamic columns" do - let(:importer_with_dynamic_columns) { import_model.with_dynmic_columns(collection_name: :skill, collection: Skill.all) } + let(:importer_with_dynamic_columns) { import_model.with_dynamic_columns(collection_name: :skill, collection: Skill.all) } describe "import" do let(:csv_source) do From 6e8979d05d8766936592d56b9de71e8a4f9d42a9 Mon Sep 17 00:00:00 2001 From: Joel AZEMAR Date: Mon, 3 Mar 2025 12:31:20 +0800 Subject: [PATCH 03/13] Update import_by_static_columns_spec.rb --- .../import_by_static_columns_spec.rb | 80 ++++++++++++------- 1 file changed, 53 insertions(+), 27 deletions(-) diff --git a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb index f7502bd..749ed90 100644 --- a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb +++ b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb @@ -1,5 +1,55 @@ # frozen_string_literal: true +module Csvbuilder + module MetaDynamicColumns + def self.included(base) + base.extend ClassMethods + end + + module ClassMethods + def with_dynamic_columns(collection_name:, collection:) + Class.new(self) do + # Create a container to store the dynamic column definitions + instance_variable_set(:"@#{collection_name}_columns", {}) + + collection.each.with_index do |entry, index| + column_name = :"#{collection_name}_#{index}" + # Build the expected method name based on the collection name. + method_name = "define_#{collection_name}_dynamic_column" + raise NotImplementedError, "You must implement #{method_name} in #{name}" unless respond_to?(method_name) + + send(method_name, entry, column_name: column_name) + + # Store the column definition for later access. + instance_variable_get(:"@#{collection_name}_columns")[column_name] = columns[column_name] + end + + # Dynamically define a class-level reader for the dynamic columns. + singleton_class.send(:attr_reader, "#{collection_name}_columns") + + # Define an instance-level method to access the dynamic columns. + define_method(:"#{collection_name}_columns") do + self.class.send(:"#{collection_name}_columns") + end + end + end + + # If a dynamic column definition method is not defined, raise an error. + def method_missing(method_name, *args, **kwargs, &) + if /^define_.*_dynamic_column$/.match?(method_name.to_s) + raise NotImplementedError, "Please implement #{method_name} in your importer class" + end + + super + end + + def respond_to_missing?(method_name, include_private = false) + method_name.to_s =~ /^define_.*_dynamic_column$/ || super + end + end + end +end + RSpec.describe "Import With Metaprogramming Instead Of Dynamic Columns" do let(:row_model) do Class.new do @@ -31,6 +81,8 @@ def skill(value, skill_name) validates :first_name, presence: true, length: { minimum: 2 } validates :last_name, presence: true, length: { minimum: 2 } + include Csvbuilder::MetaDynamicColumns + # Skip if the row is not valid, # the user is not found or the user is not valid def skip? @@ -42,33 +94,7 @@ def name "DynamicColumnsImportModel" end - def with_dynamic_columns(collection_name:, collection:) - new_class = Class.new(self) do - instance_variable_set(:"@#{collection_name}_columns", {}) - - collection.each.with_index do |entry, index| - column_name = :"#{collection_name}_#{index}" - define_dynamic_column(entry, column_name: column_name) - instance_variable_get(:"@#{collection_name}_columns")[column_name] = columns[column_name] - end - - class << self - attr_reader :skill_columns - # attr_reader :"#{collection_name}_columns" - end - - define_method(:"#{collection_name}_columns") do - self.class.send(:"#{collection_name}_columns") - end - end - - Object.send(:remove_const, "DynamicColumnsImportModel") if Object.const_defined?(:DynamicColumnsImportModel) - Object.const_set(:DynamicColumnsImportModel, new_class) - - new_class - end - - def define_dynamic_column(entry, column_name:) + def define_skill_dynamic_column(entry, column_name:) column(column_name, header: entry.name, required: false) validates(column_name, inclusion: %w[0 1], allow_blank: true) end From 3af1d73b36b58b7152d6a8d6678773f8916d5706 Mon Sep 17 00:00:00 2001 From: Joel AZEMAR Date: Mon, 3 Mar 2025 12:40:10 +0800 Subject: [PATCH 04/13] Update import_by_static_columns_spec.rb --- .../import_by_static_columns_spec.rb | 82 ++++++++++++------- 1 file changed, 54 insertions(+), 28 deletions(-) diff --git a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb index 749ed90..e0a2dcb 100644 --- a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb +++ b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb @@ -7,45 +7,65 @@ def self.included(base) end module ClassMethods + # Store DSL definitions in a hash keyed by the dynamic column type + def dynamic_columns_definitions + @dynamic_columns_definitions ||= {} + end + + # DSL method to define a dynamic column + # e.g. dynamic_column :skill, header_method: :name, required: false, inclusion: ->(entry){ %w[0 1] }, allow_blank: true + def dynamic_column(column_type, **opts) + dynamic_columns_definitions[column_type] = opts + end + def with_dynamic_columns(collection_name:, collection:) + # Retrieve DSL options for the given collection name + dsl_opts = dynamic_columns_definitions[collection_name] + if dsl_opts.nil? + raise NotImplementedError, "No dynamic column definition found for #{collection_name}. Please define one using dynamic_column." + end + Class.new(self) do - # Create a container to store the dynamic column definitions + # Initialize a container for dynamic column definitions instance_variable_set(:"@#{collection_name}_columns", {}) collection.each.with_index do |entry, index| column_name = :"#{collection_name}_#{index}" - # Build the expected method name based on the collection name. - method_name = "define_#{collection_name}_dynamic_column" - raise NotImplementedError, "You must implement #{method_name} in #{name}" unless respond_to?(method_name) - send(method_name, entry, column_name: column_name) + # Determine the header value using either a proc or a symbol + header_value = if dsl_opts[:header_method].respond_to?(:call) + dsl_opts[:header_method].call(entry) + else + entry.send(dsl_opts[:header_method]) + end - # Store the column definition for later access. + # Determine the required value (default: false) + required_value = dsl_opts.fetch(:required, false) + column(column_name, header: header_value, required: required_value) + + # Evaluate the inclusion option, which can be a proc or a collection + inclusion_value = if dsl_opts[:inclusion].respond_to?(:call) + dsl_opts[:inclusion].call(entry) + else + dsl_opts[:inclusion] + end + + # Add validations based on DSL options + validates(column_name, inclusion: inclusion_value, allow_blank: dsl_opts[:allow_blank]) + + # Save the dynamic column definition for later reference instance_variable_get(:"@#{collection_name}_columns")[column_name] = columns[column_name] end - # Dynamically define a class-level reader for the dynamic columns. + # Define a class-level reader for the dynamic columns. singleton_class.send(:attr_reader, "#{collection_name}_columns") - # Define an instance-level method to access the dynamic columns. + # Define an instance-level accessor for the dynamic columns. define_method(:"#{collection_name}_columns") do self.class.send(:"#{collection_name}_columns") end end end - - # If a dynamic column definition method is not defined, raise an error. - def method_missing(method_name, *args, **kwargs, &) - if /^define_.*_dynamic_column$/.match?(method_name.to_s) - raise NotImplementedError, "Please implement #{method_name} in your importer class" - end - - super - end - - def respond_to_missing?(method_name, include_private = false) - method_name.to_s =~ /^define_.*_dynamic_column$/ || super - end end end end @@ -81,23 +101,29 @@ def skill(value, skill_name) validates :first_name, presence: true, length: { minimum: 2 } validates :last_name, presence: true, length: { minimum: 2 } - include Csvbuilder::MetaDynamicColumns - # Skip if the row is not valid, # the user is not found or the user is not valid def skip? super || user.nil? end + include Csvbuilder::MetaDynamicColumns + + include Csvbuilder::MetaDynamicColumns + + # Define the DSL for dynamic skill columns. + # The :skill dynamic column will extract its header using the `name` method (or proc) on each entry, + # and use the given options to set up validations. + dynamic_column :skill, + header_method: :name, + required: false, + inclusion: ->(_entry) { %w[0 1] }, + allow_blank: true + class << self def name "DynamicColumnsImportModel" end - - def define_skill_dynamic_column(entry, column_name:) - column(column_name, header: entry.name, required: false) - validates(column_name, inclusion: %w[0 1], allow_blank: true) - end end end end From 6c7e68742b22673b2759c3e69b6d91bbad7dc7ce Mon Sep 17 00:00:00 2001 From: Joel AZEMAR Date: Mon, 3 Mar 2025 13:01:32 +0800 Subject: [PATCH 05/13] Update import_by_static_columns_spec.rb --- .../import_by_static_columns_spec.rb | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb index e0a2dcb..54cbd4a 100644 --- a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb +++ b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb @@ -7,60 +7,57 @@ def self.included(base) end module ClassMethods - # Store DSL definitions in a hash keyed by the dynamic column type + # Ensure the DSL definitions are stored on the class and inherited def dynamic_columns_definitions - @dynamic_columns_definitions ||= {} + @dynamic_columns_definitions ||= if superclass.respond_to?(:dynamic_columns_definitions) + superclass.dynamic_columns_definitions.dup + else + {} + end end # DSL method to define a dynamic column - # e.g. dynamic_column :skill, header_method: :name, required: false, inclusion: ->(entry){ %w[0 1] }, allow_blank: true def dynamic_column(column_type, **opts) - dynamic_columns_definitions[column_type] = opts + dynamic_columns_definitions.merge!(column_type => opts) end def with_dynamic_columns(collection_name:, collection:) # Retrieve DSL options for the given collection name dsl_opts = dynamic_columns_definitions[collection_name] - if dsl_opts.nil? + unless dsl_opts raise NotImplementedError, "No dynamic column definition found for #{collection_name}. Please define one using dynamic_column." end Class.new(self) do - # Initialize a container for dynamic column definitions instance_variable_set(:"@#{collection_name}_columns", {}) collection.each.with_index do |entry, index| column_name = :"#{collection_name}_#{index}" - # Determine the header value using either a proc or a symbol + # Evaluate header value using a proc or symbol. header_value = if dsl_opts[:header_method].respond_to?(:call) dsl_opts[:header_method].call(entry) else entry.send(dsl_opts[:header_method]) end - # Determine the required value (default: false) required_value = dsl_opts.fetch(:required, false) column(column_name, header: header_value, required: required_value) - # Evaluate the inclusion option, which can be a proc or a collection inclusion_value = if dsl_opts[:inclusion].respond_to?(:call) dsl_opts[:inclusion].call(entry) else dsl_opts[:inclusion] end - # Add validations based on DSL options validates(column_name, inclusion: inclusion_value, allow_blank: dsl_opts[:allow_blank]) - - # Save the dynamic column definition for later reference instance_variable_get(:"@#{collection_name}_columns")[column_name] = columns[column_name] end - # Define a class-level reader for the dynamic columns. + # Dynamically define a class-level reader for the dynamic columns. singleton_class.send(:attr_reader, "#{collection_name}_columns") - # Define an instance-level accessor for the dynamic columns. + # And define an instance-level accessor. define_method(:"#{collection_name}_columns") do self.class.send(:"#{collection_name}_columns") end From 6d6e15aa0d0c6e0f2c013d73806ae9d4e763bbca Mon Sep 17 00:00:00 2001 From: Joel AZEMAR Date: Mon, 3 Mar 2025 13:01:54 +0800 Subject: [PATCH 06/13] Update import_by_static_columns_spec.rb --- .../import/dynamic-columns/import_by_static_columns_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb index 54cbd4a..a6d9171 100644 --- a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb +++ b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb @@ -106,8 +106,6 @@ def skip? include Csvbuilder::MetaDynamicColumns - include Csvbuilder::MetaDynamicColumns - # Define the DSL for dynamic skill columns. # The :skill dynamic column will extract its header using the `name` method (or proc) on each entry, # and use the given options to set up validations. From 9eb2d3854446c587d58f40ad0e30cb7113ca3fd9 Mon Sep 17 00:00:00 2001 From: Joel AZEMAR Date: Mon, 3 Mar 2025 13:32:27 +0800 Subject: [PATCH 07/13] Tidy up spec/spec_helper.rb --- .rubocop_todo.yml | 15 +++---- spec/spec_helper.rb | 47 ++++++-------------- spec/support/adapters/active_record.rb | 8 ++++ spec/support/data.rb | 15 +++++++ spec/support/databases/sqlite3/connection.rb | 13 ++++++ spec/support/databases/sqlite3/schema.rb | 20 +++++++++ 6 files changed, 75 insertions(+), 43 deletions(-) create mode 100644 spec/support/adapters/active_record.rb create mode 100644 spec/support/data.rb create mode 100644 spec/support/databases/sqlite3/connection.rb create mode 100644 spec/support/databases/sqlite3/schema.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 6530dd3..f892564 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,12 +1,12 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2025-03-02 07:09:13 UTC using RuboCop version 1.73.0. +# on 2025-03-03 05:30:15 UTC using RuboCop version 1.73.1. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 1 +# Offense count: 2 # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: Max, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns, SplitStrings. # URISchemes: http, https @@ -14,15 +14,15 @@ Layout/LineLength: Exclude: - 'spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb' -# Offense count: 1 +# Offense count: 2 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes. Metrics/AbcSize: - Max: 20 + Max: 33 # Offense count: 2 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. Metrics/MethodLength: - Max: 17 + Max: 28 # Offense count: 3 # Configuration parameters: EnforcedStyle, CheckMethodNames, CheckSymbols, AllowedIdentifiers, AllowedPatterns. @@ -70,11 +70,6 @@ RSpec/MultipleExpectations: RSpec/NestedGroups: Max: 6 -# Offense count: 1 -RSpec/RemoveConst: - Exclude: - - 'spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb' - # Offense count: 2 RSpec/RepeatedExample: Exclude: diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index bb48e9d..b4e302e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -7,11 +7,21 @@ # require "logger" require "tempfile" -ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:") -# ActiveRecord::Base.logger = Logger.new($stdout) -# ActiveRecord::Migration.verbose = true +# Load support files +Dir["./spec/support/**/*.rb"].each do |file| + next if /databases|data/.match?(file) -Dir["#{Dir.pwd}/spec/support/**/*.rb"].each { |f| require f } + puts("Loading #{file}") + + require file +end + +# Load database support files +ENV["DATABASE"] ||= "sqlite3" +database = ENV.fetch("DATABASE", nil) +require "support/databases/#{database}/connection" + +require "support/database_cleaner" RSpec.configure do |config| config.filter_run focus: true @@ -29,32 +39,3 @@ config.include CsvString end - -ActiveRecord::Schema.define do - create_table :users, force: true do |t| - t.string :first_name - t.string :last_name - t.string :full_name - end - create_table :skills_users, force: true do |t| - t.references :skill - t.references :user - end - create_table :skills, force: true do |t| - t.string :name - end -end - -class User < ActiveRecord::Base - self.table_name = :users - - validates :full_name, presence: true - - has_and_belongs_to_many :skills, join_table: :skills_users -end - -class Skill < ActiveRecord::Base - self.table_name = :skills - - validates :name, presence: true -end diff --git a/spec/support/adapters/active_record.rb b/spec/support/adapters/active_record.rb new file mode 100644 index 0000000..2649b0e --- /dev/null +++ b/spec/support/adapters/active_record.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +require "active_record" + +if ENV["DEBUG"] + require "logger" + ActiveRecord::Base.logger = Logger.new($stdout) +end diff --git a/spec/support/data.rb b/spec/support/data.rb new file mode 100644 index 0000000..a96a7df --- /dev/null +++ b/spec/support/data.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class User < ActiveRecord::Base + self.table_name = :users + + validates :full_name, presence: true + + has_and_belongs_to_many :skills, join_table: :skills_users +end + +class Skill < ActiveRecord::Base + self.table_name = :skills + + validates :name, presence: true +end diff --git a/spec/support/databases/sqlite3/connection.rb b/spec/support/databases/sqlite3/connection.rb new file mode 100644 index 0000000..b99b07b --- /dev/null +++ b/spec/support/databases/sqlite3/connection.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:") + +RSpec.configure do |config| + config.before(:suite) do + # Load the schema + load File.expand_path("../sqlite3/schema.rb", __dir__) + + # Load the data + load File.expand_path("../../data.rb", __dir__) + end +end diff --git a/spec/support/databases/sqlite3/schema.rb b/spec/support/databases/sqlite3/schema.rb new file mode 100644 index 0000000..cf524a4 --- /dev/null +++ b/spec/support/databases/sqlite3/schema.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +ActiveRecord::Schema.define do + self.verbose = ENV.fetch("DEBUG", nil) + + create_table :users, force: true do |t| + t.string :first_name + t.string :last_name + t.string :full_name + end + + create_table :skills_users, force: true do |t| + t.references :skill + t.references :user + end + + create_table :skills, force: true do |t| + t.string :name + end +end From 44fb2eeb3cb06247baef03ce4308cdfe7dfd9097 Mon Sep 17 00:00:00 2001 From: Joel AZEMAR Date: Mon, 3 Mar 2025 13:51:30 +0800 Subject: [PATCH 08/13] Add Tags --- .../import_by_static_columns_spec.rb | 2 +- spec/spec_helper.rb | 2 +- spec/support/data.rb | 34 +++++++++++++------ spec/support/databases/sqlite3/connection.rb | 3 ++ spec/support/databases/sqlite3/schema.rb | 14 ++++++++ spec/support/models.rb | 29 ++++++++++++++++ 6 files changed, 72 insertions(+), 12 deletions(-) create mode 100644 spec/support/models.rb diff --git a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb index a6d9171..219a4d5 100644 --- a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb +++ b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb @@ -112,7 +112,7 @@ def skip? dynamic_column :skill, header_method: :name, required: false, - inclusion: ->(_entry) { %w[0 1] }, + inclusion: %w[0 1], allow_blank: true class << self diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b4e302e..2f8b199 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -9,7 +9,7 @@ # Load support files Dir["./spec/support/**/*.rb"].each do |file| - next if /databases|data/.match?(file) + next if /databases|models|data/.match?(file) # "support/databases/#{database}/connection" load models and data puts("Loading #{file}") diff --git a/spec/support/data.rb b/spec/support/data.rb index a96a7df..2105b36 100644 --- a/spec/support/data.rb +++ b/spec/support/data.rb @@ -1,15 +1,29 @@ # frozen_string_literal: true -class User < ActiveRecord::Base - self.table_name = :users +def load_data! + { + areas: [ + { + name: "Area 1", + tags: [ + { name: "Tag 1" }, + { name: "Tag 2" } + ] + }, { + name: "Area 2", + tags: [ + { name: "Tag 3" }, + { name: "Tag 4" } + ] + } + ] + }[:areas].each do |area| + area_instance = Area.create!(name: area[:name]) - validates :full_name, presence: true - - has_and_belongs_to_many :skills, join_table: :skills_users + area[:tags].each do |tag| + Tag.create!(name: tag[:name], area: area_instance) + end + end end -class Skill < ActiveRecord::Base - self.table_name = :skills - - validates :name, presence: true -end +load_data! diff --git a/spec/support/databases/sqlite3/connection.rb b/spec/support/databases/sqlite3/connection.rb index b99b07b..ccb73ee 100644 --- a/spec/support/databases/sqlite3/connection.rb +++ b/spec/support/databases/sqlite3/connection.rb @@ -7,6 +7,9 @@ # Load the schema load File.expand_path("../sqlite3/schema.rb", __dir__) + # Load the models + load File.expand_path("../../models.rb", __dir__) + # Load the data load File.expand_path("../../data.rb", __dir__) end diff --git a/spec/support/databases/sqlite3/schema.rb b/spec/support/databases/sqlite3/schema.rb index cf524a4..c788092 100644 --- a/spec/support/databases/sqlite3/schema.rb +++ b/spec/support/databases/sqlite3/schema.rb @@ -17,4 +17,18 @@ create_table :skills, force: true do |t| t.string :name end + + create_table :areas, force: true do |t| + t.string :name + end + + create_table :tags, force: true do |t| + t.string :name + t.references(:area) + end + + create_table :taggings, force: true do |t| + t.references(:tag) + t.references(:source, polymorphic: true) + end end diff --git a/spec/support/models.rb b/spec/support/models.rb new file mode 100644 index 0000000..9c85832 --- /dev/null +++ b/spec/support/models.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class User < ActiveRecord::Base + self.table_name = :users + + validates :full_name, presence: true + + has_and_belongs_to_many :skills, join_table: :skills_users + has_many :taggings, as: :source +end + +class Skill < ActiveRecord::Base + self.table_name = :skills + + validates :name, presence: true +end + +class Area < ActiveRecord::Base + has_many :tags +end + +class Tag < ActiveRecord::Base + belongs_to :area +end + +class Tagging < ActiveRecord::Base + belongs_to :tag + belongs_to :source, polymorphic: true +end From 2be4318e8ca1d2344ef794a96c11f0783a385e8d Mon Sep 17 00:00:00 2001 From: Joel AZEMAR Date: Mon, 3 Mar 2025 14:29:24 +0800 Subject: [PATCH 09/13] Add Skill --- spec/spec_helper.rb | 5 ++++- spec/support/data.rb | 6 ++++++ spec/support/databases/sqlite3/connection.rb | 3 +++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2f8b199..e79ba35 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,7 +18,10 @@ # Load database support files ENV["DATABASE"] ||= "sqlite3" -database = ENV.fetch("DATABASE", nil) +database = ENV.fetch("DATABASE", "sqlite3") + +puts("Loading support/databases/#{database}/connection") + require "support/databases/#{database}/connection" require "support/database_cleaner" diff --git a/spec/support/data.rb b/spec/support/data.rb index 2105b36..2ec1b40 100644 --- a/spec/support/data.rb +++ b/spec/support/data.rb @@ -24,6 +24,12 @@ def load_data! Tag.create!(name: tag[:name], area: area_instance) end end + + %w[Ruby Python Javascript].each do |skill_name| + Skill.create(name: skill_name) + end + + puts "Data loaded" end load_data! diff --git a/spec/support/databases/sqlite3/connection.rb b/spec/support/databases/sqlite3/connection.rb index ccb73ee..a042aac 100644 --- a/spec/support/databases/sqlite3/connection.rb +++ b/spec/support/databases/sqlite3/connection.rb @@ -5,12 +5,15 @@ RSpec.configure do |config| config.before(:suite) do # Load the schema + puts "Loading schema" load File.expand_path("../sqlite3/schema.rb", __dir__) # Load the models + puts "Loading models" load File.expand_path("../../models.rb", __dir__) # Load the data + puts "Loading data" load File.expand_path("../../data.rb", __dir__) end end From 0da26ff6a42d6fc73823ca9ce35855592b7ccc7a Mon Sep 17 00:00:00 2001 From: Joel AZEMAR Date: Mon, 3 Mar 2025 16:09:28 +0800 Subject: [PATCH 10/13] Update import_by_static_columns_spec.rb --- .../import_by_static_columns_spec.rb | 86 ++++++++++++++++--- 1 file changed, 74 insertions(+), 12 deletions(-) diff --git a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb index 219a4d5..88cec1b 100644 --- a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb +++ b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb @@ -18,7 +18,7 @@ def dynamic_columns_definitions # DSL method to define a dynamic column def dynamic_column(column_type, **opts) - dynamic_columns_definitions.merge!(column_type => opts) + dynamic_columns_definitions.merge!(column_type => { allow_blank: false, required: true }.reverse_merge(opts)) end def with_dynamic_columns(collection_name:, collection:) @@ -28,7 +28,7 @@ def with_dynamic_columns(collection_name:, collection:) raise NotImplementedError, "No dynamic column definition found for #{collection_name}. Please define one using dynamic_column." end - Class.new(self) do + new_class = Class.new(self) do instance_variable_set(:"@#{collection_name}_columns", {}) collection.each.with_index do |entry, index| @@ -62,6 +62,16 @@ def with_dynamic_columns(collection_name:, collection:) self.class.send(:"#{collection_name}_columns") end end + + # Copy over any existing dynamic columns from the parent to the new subclass. + instance_variables.each do |var| + if var.to_s.end_with?("_columns") && var != :"@#{collection_name}_columns" + new_class.instance_variable_set(var, instance_variable_get(var)) + new_class.singleton_class.send(:attr_reader, var.to_s.delete_prefix("@")) + end + end + + new_class end end end @@ -115,6 +125,12 @@ def skip? inclusion: %w[0 1], allow_blank: true + dynamic_column :tag, + header_method: :name, + required: false, + inclusion: ->(area) { area.tags.pluck(:name) }, + allow_blank: true + class << self def name "DynamicColumnsImportModel" @@ -133,16 +149,44 @@ def name %w[Ruby Python Javascript].each do |skill_name| Skill.create(name: skill_name) end + + { + areas: [ + { + name: "Area 1", + tags: [ + { name: "Tag 1" }, + { name: "Tag 2" } + ] + }, { + name: "Area 2", + tags: [ + { name: "Tag 3" }, + { name: "Tag 4" } + ] + } + ] + }[:areas].each do |area| + area_instance = Area.create!(name: area[:name]) + + area[:tags].each do |tag| + Tag.create!(name: tag[:name], area: area_instance) + end + end end context "with dynamic columns" do - let(:importer_with_dynamic_columns) { import_model.with_dynamic_columns(collection_name: :skill, collection: Skill.all) } + let(:importer_with_dynamic_columns) do + import_model + .with_dynamic_columns(collection_name: :skill, collection: Skill.all) + .with_dynamic_columns(collection_name: :tag, collection: Area.all) + end describe "import" do let(:csv_source) do [ - %w[Name Surname Ruby Python Javascript], - %w[John Doe 1 0 1] + ["Name", "Surname", "Ruby", "Python", "Javascript", "Area 1", "Area 2"], + ["John", "Doe", "1", "0", "1", "Tag 1", "Tag 3"] ] end @@ -152,11 +196,11 @@ def name end it "adds skills to column names" do - expect(importer_with_dynamic_columns.column_names).to match_array(%i[first_name last_name skill_0 skill_1 skill_2]) + expect(importer_with_dynamic_columns.column_names).to match_array(%i[first_name last_name skill_0 skill_1 skill_2 tag_0 tag_1]) end it "adds skills to headers" do - expect(importer_with_dynamic_columns.headers).to match_array(%w[Name Surname Ruby Python Javascript]) + expect(importer_with_dynamic_columns.headers).to contain_exactly("Name", "Surname", "Ruby", "Python", "Javascript", "Area 1", "Area 2") end it "adds skills to users" do @@ -174,13 +218,31 @@ def name expect(row_model.user.skills.map(&:name)).to match_array(%w[Ruby Javascript]) end end + + it "adds tags to users" do + Csvbuilder::Import::File.new(file.path, importer_with_dynamic_columns, options).each do |row_model| + row_model.tag_columns.each do |column_name, tag_data| + row_cell_value = row_model.attribute_objects[column_name].value # Get the value of the cell + + area = Area.find_by(name: tag_data[:header]) + + row_model.user.taggings.create(tag: area.tags.find_by(name: row_cell_value)) if area + end + + expect(row_model.user.taggings).to be_truthy + expect(row_model.user.taggings.count).to eq(2) + expect( + row_model.user.taggings.map { |t| t.tag.name } + ).to eq(["Tag 1", "Tag 3"]) + end + end end context "with invalid data" do let(:csv_source) do [ - %w[Name Surname Ruby Python Javascript], - %w[John Doe 1 0 2] + ["Name", "Surname", "Ruby", "Python", "Javascript", "Area 1", "Area 2"], + ["John", "Doe", "1", "0", "2", "Tag 1", "Tag 3"] ] end @@ -203,8 +265,8 @@ def name context "with invalid headers" do let(:csv_source) do [ - ["Name", "Surname", "Ruby", "Python", "Visual Basic"], - %w[John Doe 1 0 2] + ["Name", "Surname", "Ruby", "Python", "Visual Basic", "Area 1", "Area 2"], + ["John", "Doe", "1", "0", "2", "Tag 1", "Tag 3"] ] end @@ -215,7 +277,7 @@ def name expect(importer.errors.full_messages).to eq( [ - "Headers mismatch. Given headers (Name, Surname, Ruby, Python, Visual Basic). Expected headers (Name, Surname, Ruby, Python, Javascript). Unrecognized headers (Visual Basic)." + "Headers mismatch. Given headers (Name, Surname, Ruby, Python, Visual Basic, Area 1, Area 2). Expected headers (Name, Surname, Ruby, Python, Javascript, Area 1, Area 2). Unrecognized headers (Visual Basic)." ] ) end From 619bf8dc68cb48b750f15ab5ea46c0d84fbe7318 Mon Sep 17 00:00:00 2001 From: Joel AZEMAR Date: Mon, 3 Mar 2025 16:09:32 +0800 Subject: [PATCH 11/13] Update .rubocop_todo.yml --- .rubocop_todo.yml | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index f892564..5975c7f 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,12 +1,12 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2025-03-03 05:30:15 UTC using RuboCop version 1.73.1. +# on 2025-03-03 08:09:12 UTC using RuboCop version 1.73.1. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 2 +# Offense count: 4 # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: Max, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns, SplitStrings. # URISchemes: http, https @@ -17,14 +17,24 @@ Layout/LineLength: # Offense count: 2 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes. Metrics/AbcSize: - Max: 33 + Max: 44 -# Offense count: 2 +# Offense count: 1 +# Configuration parameters: AllowedMethods, AllowedPatterns. +Metrics/CyclomaticComplexity: + Max: 8 + +# Offense count: 3 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. Metrics/MethodLength: - Max: 28 + Max: 35 -# Offense count: 3 +# Offense count: 1 +# Configuration parameters: AllowedMethods, AllowedPatterns. +Metrics/PerceivedComplexity: + Max: 10 + +# Offense count: 5 # Configuration parameters: EnforcedStyle, CheckMethodNames, CheckSymbols, AllowedIdentifiers, AllowedPatterns. # SupportedStyles: snake_case, normalcase, non_integer # AllowedIdentifiers: capture3, iso8601, rfc1123_date, rfc822, rfc2822, rfc3339, x86_64 @@ -56,12 +66,12 @@ RSpec/DescribeClass: - 'spec/integrations/import/dynamic-columns/import_spec.rb' - 'spec/integrations/import/dynamic-columns/validate_dynamic_header_values_spec.rb' -# Offense count: 17 +# Offense count: 18 # Configuration parameters: CountAsOne. RSpec/ExampleLength: Max: 31 -# Offense count: 14 +# Offense count: 15 RSpec/MultipleExpectations: Max: 14 From f8e0f0ed7d2c95e6d842d2fb8d917b2ffcf696a1 Mon Sep 17 00:00:00 2001 From: Joel AZEMAR Date: Mon, 3 Mar 2025 16:37:33 +0800 Subject: [PATCH 12/13] Update import_by_static_columns_spec.rb --- .../dynamic-columns/import_by_static_columns_spec.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb index 88cec1b..43f7e14 100644 --- a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb +++ b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb @@ -23,14 +23,18 @@ def dynamic_column(column_type, **opts) def with_dynamic_columns(collection_name:, collection:) # Retrieve DSL options for the given collection name + # i.e dynamic_column :tag, inclusion: %[operational strategic] + # dynamic_column :category, inclusion: ->(group) { group.categories.pluck(:name) }, dsl_opts = dynamic_columns_definitions[collection_name] unless dsl_opts raise NotImplementedError, "No dynamic column definition found for #{collection_name}. Please define one using dynamic_column." end + # Create a new subclass of the current class new_class = Class.new(self) do instance_variable_set(:"@#{collection_name}_columns", {}) + # Define a method to access the collection collection.each.with_index do |entry, index| column_name = :"#{collection_name}_#{index}" @@ -42,6 +46,7 @@ def with_dynamic_columns(collection_name:, collection:) end required_value = dsl_opts.fetch(:required, false) + # Define the dynamic column as static column column(column_name, header: header_value, required: required_value) inclusion_value = if dsl_opts[:inclusion].respond_to?(:call) @@ -50,6 +55,7 @@ def with_dynamic_columns(collection_name:, collection:) dsl_opts[:inclusion] end + # We add the inclusion validation to the column, which can depends upon the record. validates(column_name, inclusion: inclusion_value, allow_blank: dsl_opts[:allow_blank]) instance_variable_get(:"@#{collection_name}_columns")[column_name] = columns[column_name] end @@ -64,6 +70,7 @@ def with_dynamic_columns(collection_name:, collection:) end # Copy over any existing dynamic columns from the parent to the new subclass. + # Otherwise, the new subclass will not have access to the previous defined dynamic columns. instance_variables.each do |var| if var.to_s.end_with?("_columns") && var != :"@#{collection_name}_columns" new_class.instance_variable_set(var, instance_variable_get(var)) @@ -71,7 +78,7 @@ def with_dynamic_columns(collection_name:, collection:) end end - new_class + new_class # Masquerade the extends class and make it chainable end end end From ac33002bf440bf211c340e93cc9b16ba278449b9 Mon Sep 17 00:00:00 2001 From: Joel AZEMAR Date: Mon, 3 Mar 2025 17:39:38 +0800 Subject: [PATCH 13/13] Update import_by_static_columns_spec.rb --- .../import_by_static_columns_spec.rb | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb index 43f7e14..2af8189 100644 --- a/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb +++ b/spec/integrations/import/dynamic-columns/import_by_static_columns_spec.rb @@ -18,7 +18,7 @@ def dynamic_columns_definitions # DSL method to define a dynamic column def dynamic_column(column_type, **opts) - dynamic_columns_definitions.merge!(column_type => { allow_blank: false, required: true }.reverse_merge(opts)) + dynamic_columns_definitions.merge!(column_type => { allow_blank: false, required: true, header_method: :name }.reverse_merge(opts)) end def with_dynamic_columns(collection_name:, collection:) @@ -126,17 +126,8 @@ def skip? # Define the DSL for dynamic skill columns. # The :skill dynamic column will extract its header using the `name` method (or proc) on each entry, # and use the given options to set up validations. - dynamic_column :skill, - header_method: :name, - required: false, - inclusion: %w[0 1], - allow_blank: true - - dynamic_column :tag, - header_method: :name, - required: false, - inclusion: ->(area) { area.tags.pluck(:name) }, - allow_blank: true + dynamic_column :skill, inclusion: %w[0 1] + dynamic_column :tag, inclusion: ->(area) { area.tags.pluck(:name) } class << self def name