From 5763e807d19f1222e604b77e92ccc9369aa84fe2 Mon Sep 17 00:00:00 2001 From: Khaled Muamar Date: Wed, 3 Apr 2024 15:41:13 +0200 Subject: [PATCH] Fix PG::AmbiguousColumn if we do a join on 2 tables that has the same array_enum col name --- .byebug_history | 2 + .github/workflows/ci-on-merge.yml | 6 +-- .gitignore | 1 + Gemfile | 2 +- Gemfile.lock | 40 ++++++++++------ lib/array_enum.rb | 18 +++---- test/array_enum_test.rb | 80 +++++++++++++++++-------------- test/test_helper.rb | 38 ++++++++++----- 8 files changed, 111 insertions(+), 76 deletions(-) create mode 100644 .byebug_history diff --git a/.byebug_history b/.byebug_history new file mode 100644 index 0000000..6b2050a --- /dev/null +++ b/.byebug_history @@ -0,0 +1,2 @@ +c +User.with_any_of_favourite_colors(%w[red]) diff --git a/.github/workflows/ci-on-merge.yml b/.github/workflows/ci-on-merge.yml index 48811a0..b1dbdcf 100644 --- a/.github/workflows/ci-on-merge.yml +++ b/.github/workflows/ci-on-merge.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - ruby-version: [2.7, '3.0', 3.1] + ruby-version: [2.7, 3.0, 3.1, 3.3] services: postgres: @@ -40,10 +40,10 @@ jobs: psql -c 'create database "array_enum_test";' - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v4.1.1 - name : Ruby Setup - uses: ruby/setup-ruby@v1 + uses: ruby/setup-ruby@v1.173 with: ruby-version: ${{ matrix.ruby-version }} bundler-cache: true diff --git a/.gitignore b/.gitignore index 9106b2a..541d400 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ /pkg/ /spec/reports/ /tmp/ +.tool-versions \ No newline at end of file diff --git a/Gemfile b/Gemfile index b9289ab..149969f 100644 --- a/Gemfile +++ b/Gemfile @@ -1,4 +1,4 @@ -source "https://rubygems.org" +source 'https://rubygems.org' # Specify your gem's dependencies in array_enum.gemspec gemspec diff --git a/Gemfile.lock b/Gemfile.lock index e32337e..6e1d423 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,26 +7,36 @@ PATH GEM remote: https://rubygems.org/ specs: - activemodel (6.1.3) - activesupport (= 6.1.3) - activerecord (6.1.3) - activemodel (= 6.1.3) - activesupport (= 6.1.3) - activesupport (6.1.3) + activemodel (7.1.3.2) + activesupport (= 7.1.3.2) + activerecord (7.1.3.2) + activemodel (= 7.1.3.2) + activesupport (= 7.1.3.2) + timeout (>= 0.4.0) + activesupport (7.1.3.2) + base64 + bigdecimal concurrent-ruby (~> 1.0, >= 1.0.2) + connection_pool (>= 2.2.5) + drb i18n (>= 1.6, < 2) minitest (>= 5.1) + mutex_m tzinfo (~> 2.0) - zeitwerk (~> 2.3) - concurrent-ruby (1.1.8) - i18n (1.8.9) + base64 (0.2.0) + bigdecimal (3.1.7) + concurrent-ruby (1.2.3) + connection_pool (2.4.1) + drb (2.2.1) + i18n (1.14.4) concurrent-ruby (~> 1.0) - minitest (5.11.3) - pg (1.1.4) - rake (12.3.3) - tzinfo (2.0.4) + minitest (5.22.3) + mutex_m (0.2.0) + pg (1.5.6) + rake (13.2.0) + timeout (0.4.1) + tzinfo (2.0.6) concurrent-ruby (~> 1.0) - zeitwerk (2.4.2) PLATFORMS ruby @@ -40,4 +50,4 @@ DEPENDENCIES rake (>= 10.0) BUNDLED WITH - 2.2.3 + 2.5.6 diff --git a/lib/array_enum.rb b/lib/array_enum.rb index 9e774ba..1613416 100644 --- a/lib/array_enum.rb +++ b/lib/array_enum.rb @@ -1,11 +1,11 @@ -require "array_enum/version" -require "array_enum/subset_validator" -require "array_enum/railtie" if defined?(Rails::Railtie) -require "active_support/hash_with_indifferent_access" -require "active_support/core_ext/string/inflections" +require 'array_enum/version' +require 'array_enum/subset_validator' +require 'array_enum/railtie' if defined?(Rails::Railtie) +require 'active_support/hash_with_indifferent_access' +require 'active_support/core_ext/string/inflections' module ArrayEnum - MISSING_VALUE_MESSAGE = "%{value} is not a valid value for %{attr}".freeze + MISSING_VALUE_MESSAGE = '%s is not a valid value for %s'.freeze private_constant :MISSING_VALUE_MESSAGE def array_enum(definitions) @@ -24,9 +24,9 @@ def array_enum(definitions) }.each do |method_name, comparison_operator| define_singleton_method(method_name.to_sym) do |values| db_values = Array(values).map do |value| - mapping_hash[value] || raise(ArgumentError, MISSING_VALUE_MESSAGE % {value: value, attr: attr_name}) + mapping_hash[value] || raise(ArgumentError, format(MISSING_VALUE_MESSAGE, value: value, attr: attr_name)) end - where("#{attr_name} #{comparison_operator} ARRAY[:db_values]", db_values: db_values) + where("#{table_name}.#{attr_name} #{comparison_operator} ARRAY[:db_values]", db_values: db_values) end end @@ -36,7 +36,7 @@ def array_enum(definitions) define_method("#{attr_name}=".to_sym) do |values| self[attr_symbol] = Array(values).map do |value| - mapping_hash[value] || raise(ArgumentError, MISSING_VALUE_MESSAGE % {value: value, attr: attr_name}) + mapping_hash[value] || raise(ArgumentError, format(MISSING_VALUE_MESSAGE, value: value, attr: attr_name)) end.uniq end end diff --git a/test/array_enum_test.rb b/test/array_enum_test.rb index e3d0c28..882da41 100644 --- a/test/array_enum_test.rb +++ b/test/array_enum_test.rb @@ -1,4 +1,4 @@ -require "test_helper" +require 'test_helper' class ArrayEnumTest < Minitest::Test def setup @@ -6,115 +6,121 @@ def setup end def test_assigning_enum_values - user = User.new(favourite_colors: ["red", "blue"]) - assert_equal ["red", "blue"], user.favourite_colors + user = User.new(favourite_colors: %w[red blue]) + assert_equal %w[red blue], user.favourite_colors end def test_assigning_enum_values_as_symbols - user = User.new(favourite_colors: [:red, :blue]) - assert_equal ["red", "blue"], user.favourite_colors + user = User.new(favourite_colors: %i[red blue]) + assert_equal %w[red blue], user.favourite_colors end def test_raising_error_on_unknown_value error = assert_raises(ArgumentError) do - User.new(favourite_colors: ["black"]) + User.new(favourite_colors: ['black']) end assert_match(/black is not a valid value for favourite_colors/, error.message) end def test_storing_values_as_integers - user = User.create!(favourite_colors: ["red"]) + user = User.create!(favourite_colors: ['red']) user.reload - assert_equal "{1}", user.read_attribute_before_type_cast("favourite_colors") + assert_equal '{1}', user.read_attribute_before_type_cast('favourite_colors') end # with_attr scope def test_quering_db_with_single_matching_value - user = User.create!(favourite_colors: ["red"]) - assert_equal [user], User.with_favourite_colors("red") + user = User.create!(favourite_colors: ['red']) + assert_equal [user], User.with_favourite_colors('red') end def test_quering_db_with_single_matching_symbol_value - user = User.create!(favourite_colors: ["red"]) + user = User.create!(favourite_colors: ['red']) assert_equal [user], User.with_favourite_colors(:red) end def test_quering_db_by_one_of_matching_value - user = User.create!(favourite_colors: ["red", "blue"]) - assert_equal [user], User.with_favourite_colors("red") + user = User.create!(favourite_colors: %w[red blue]) + assert_equal [user], User.with_favourite_colors('red') end def test_quering_db_by_excluded_value_does_not_return_record - User.create!(favourite_colors: ["red", "blue"]) - assert_equal [], User.with_favourite_colors("green") + User.create!(favourite_colors: %w[red blue]) + assert_equal [], User.with_favourite_colors('green') end def test_quering_db_by_many_values_does_not_return_record_on_excluded_value - User.create!(favourite_colors: ["red", "blue"]) - assert_equal [], User.with_favourite_colors(["red", "green"]) + User.create!(favourite_colors: %w[red blue]) + assert_equal [], User.with_favourite_colors(%w[red green]) end def test_quering_db_only_with_single_matching_value - user = User.create!(favourite_colors: ["red"]) - assert_equal [user], User.only_with_favourite_colors(["red"]) + user = User.create!(favourite_colors: ['red']) + assert_equal [user], User.only_with_favourite_colors(['red']) end def test_quering_db_only_with_single_matching_value_from_many_values_does_not_return_record - User.create!(favourite_colors: ["red", "blue"]) - assert_equal [], User.only_with_favourite_colors(["red"]) + User.create!(favourite_colors: %w[red blue]) + assert_equal [], User.only_with_favourite_colors(['red']) end def test_quering_db_by_non_existing_value_raises_error - User.create!(favourite_colors: ["red", "blue"]) + User.create!(favourite_colors: %w[red blue]) error = assert_raises(ArgumentError) do - User.with_favourite_colors("black") + User.with_favourite_colors('black') end assert_match(/black is not a valid value for favourite_colors/, error.message) end # with_any_of_attr scope def test_with_any_of_attr_matching_value - user = User.create!(favourite_colors: ["red"]) - assert_equal [user], User.with_any_of_favourite_colors("red") + user = User.create!(favourite_colors: ['red']) + assert_equal [user], User.with_any_of_favourite_colors('red') end def test_with_any_of_attr_matching_symbol_value - user = User.create!(favourite_colors: ["red"]) + user = User.create!(favourite_colors: ['red']) assert_equal [user], User.with_any_of_favourite_colors(:red) end def test_with_any_of_attr_one_of_matching_value - user = User.create!(favourite_colors: ["red", "blue"]) - assert_equal [user], User.with_any_of_favourite_colors("red") + user = User.create!(favourite_colors: %w[red blue]) + assert_equal [user], User.with_any_of_favourite_colors('red') end def test_with_any_of_attr_excluded_value_does_not_return_record - User.create!(favourite_colors: ["red", "blue"]) - assert_equal [], User.with_any_of_favourite_colors("green") + User.create!(favourite_colors: %w[red blue]) + assert_equal [], User.with_any_of_favourite_colors('green') end def test_with_any_of_attr_many_value_when_single_match - user = User.create!(favourite_colors: ["red", "blue"]) - assert_equal [user], User.with_any_of_favourite_colors(["red", "green"]) + user = User.create!(favourite_colors: %w[red blue]) + assert_equal [user], User.with_any_of_favourite_colors(%w[red green]) end def test_with_any_of_attr_by_non_existing_value_raises_error - User.create!(favourite_colors: ["red", "blue"]) + User.create!(favourite_colors: %w[red blue]) error = assert_raises(ArgumentError) do - User.with_any_of_favourite_colors("black") + User.with_any_of_favourite_colors('black') end assert_match(/black is not a valid value for favourite_colors/, error.message) end - def test_lists_values - assert_equal User.favourite_colors, {"red"=>1, "blue"=>2, "green"=>3} + assert_equal User.favourite_colors, { 'red' => 1, 'blue' => 2, 'green' => 3 } end def test_values_can_be_accessed_indifferently assert_equal User.favourite_colors[:red], 1 assert_equal User.favourite_colors[:blue], 2 assert_equal User.favourite_colors[:green], 3 - assert_equal User.favourite_colors["red"], 1 + assert_equal User.favourite_colors['red'], 1 + end + + def test_user_with_join_table_which_has_same_array_enum_col + user = User.create!(favourite_colors: %w[red blue], + profiles: [Profile.new(slug: 'profile_slug')]) + + assert_equal [user], User.joins(:profiles).with_any_of_favourite_colors(%w[red]) end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 038b41b..f74f061 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,16 +1,16 @@ -$LOAD_PATH.unshift File.expand_path("../../lib", __FILE__) -require "active_record" -require "array_enum" -require "minitest/autorun" +$LOAD_PATH.unshift File.expand_path('../lib', __dir__) +require 'active_record' +require 'array_enum' +require 'minitest/autorun' ActiveRecord::Base.establish_connection( - adapter: "postgresql", - host: "localhost", - port: "5432", - username: "postgres", - database: "array_enum_test" + adapter: 'postgresql', + host: 'localhost', + port: '5432', + username: 'postgres', + database: 'array_enum_test' ) -ActiveRecord::Base.connection.execute("SELECT 1") # catch db errors early +ActiveRecord::Base.connection.execute('SELECT 1') # catch db errors early ActiveRecord::Schema.define do self.verbose = false @@ -20,10 +20,26 @@ end end +ActiveRecord::Schema.define do + self.verbose = false + + create_table :profiles, force: true do |t| + t.integer :favourite_colors, array: true + t.string :slug + t.integer :user_id + end +end + class User < ActiveRecord::Base extend ArrayEnum + has_many :profiles + array_enum favourite_colors: { 'red' => 1, 'blue' => 2, 'green' => 3 } +end + +class Profile < ActiveRecord::Base + extend ArrayEnum - array_enum favourite_colors: {"red" => 1, "blue" => 2, "green" => 3} + array_enum favourite_colors: { 'red' => 1, 'blue' => 2, 'green' => 3 } end # Mimic constant assigment from railtie