Skip to content

Commit

Permalink
Merge pull request #19 from freeletics/fix-ambiguous-column-issue
Browse files Browse the repository at this point in the history
Fix PG::AmbiguousColumn if we do a join on 2 tables that has the same array_enum col name
  • Loading branch information
khaledm1990 authored Apr 11, 2024
2 parents 31144c6 + f5ddfbe commit 0565942
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 104 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/ci-on-merge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
ruby-version: [2.7, '3.0', 3.1]
ruby-version: [3.0, 3.1, 3.3]

services:
postgres:
Expand Down Expand Up @@ -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.0
with:
ruby-version: ${{ matrix.ruby-version }}
bundler-cache: true
Expand Down
10 changes: 6 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
name: CI

on: [pull_request]
on:
- pull_request
- push

jobs:
test:
runs-on: ubuntu-latest
strategy:
matrix:
ruby-version: [2.7, '3.0', 3.1]
ruby-version: [3.0, 3.1, 3.3]

services:
postgres:
Expand Down Expand Up @@ -37,10 +39,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.0
with:
ruby-version: ${{ matrix.ruby-version }}
bundler-cache: true
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,5 @@
/pkg/
/spec/reports/
/tmp/
.tool-versions
.byebug_history
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
source "https://rubygems.org"
source 'https://rubygems.org'

# Specify your gem's dependencies in array_enum.gemspec
gemspec
40 changes: 25 additions & 15 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -40,4 +50,4 @@ DEPENDENCIES
rake (>= 10.0)

BUNDLED WITH
2.2.3
2.5.6
47 changes: 23 additions & 24 deletions array_enum.gemspec
Original file line number Diff line number Diff line change
@@ -1,45 +1,44 @@

lib = File.expand_path("../lib", __FILE__)
lib = File.expand_path('lib', __dir__)
$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
require "array_enum/version"
require 'array_enum/version'

Gem::Specification.new do |spec|
spec.name = "array_enum"
spec.name = 'array_enum'
spec.version = ArrayEnum::VERSION
spec.authors = ["Wojciech Wnętrzak"]
spec.email = ["w.wnetrzak@gmail.com", "eng@freeletics.com"]
spec.authors = ['Wojciech Wnętrzak']
spec.email = ['w.wnetrzak@gmail.com', 'eng@freeletics.com']

spec.summary = %q{String to integer mapping for PostgreSQL array columns.}
spec.description = %q{Extension for ActiveRecord that adds support for PostgreSQL array columns, mapping string values to integers.}
spec.homepage = "https://github.com/freeletics/array_enum"
spec.license = "MIT"
spec.summary = 'String to integer mapping for PostgreSQL array columns.'
spec.description = 'Extension for ActiveRecord that adds support for PostgreSQL array columns, mapping string values to integers.'
spec.homepage = 'https://github.com/freeletics/array_enum'
spec.license = 'MIT'

# Prevent pushing this gem to RubyGems.org. To allow pushes either set the 'allowed_push_host'
# to allow pushing to a single host or delete this section to allow pushing to any host.
if spec.respond_to?(:metadata)
spec.metadata["homepage_uri"] = spec.homepage
spec.metadata["source_code_uri"] = "https://github.com/freeletics/array_enum"
spec.metadata['homepage_uri'] = spec.homepage
spec.metadata['source_code_uri'] = 'https://github.com/freeletics/array_enum'
else
raise "RubyGems 2.0 or newer is required to protect against " \
"public gem pushes."
raise 'RubyGems 2.0 or newer is required to protect against ' \
'public gem pushes.'
end

# Specify which files should be added to the gem when it is released.
# The `git ls-files -z` loads the files in the RubyGem that have been added into git.
spec.files = Dir.chdir(File.expand_path('..', __FILE__)) do
spec.files = Dir.chdir(File.expand_path(__dir__)) do
`git ls-files -z`.split("\x0").reject { |f| f.match(%r{^(test|spec|features)/}) }
end
spec.bindir = "exe"
spec.bindir = 'exe'
spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) }
spec.require_paths = ["lib"]
spec.require_paths = ['lib']

spec.required_ruby_version = ">= 2.3"
spec.required_ruby_version = '>= 3.0'

spec.add_dependency "activemodel"
spec.add_dependency 'activemodel'

spec.add_development_dependency "activerecord"
spec.add_development_dependency "pg"
spec.add_development_dependency "bundler", ">= 1.17"
spec.add_development_dependency "rake", ">= 10.0"
spec.add_development_dependency "minitest", ">= 5.0"
spec.add_development_dependency 'activerecord'
spec.add_development_dependency 'bundler', '>= 1.17'
spec.add_development_dependency 'minitest', '>= 5.0'
spec.add_development_dependency 'pg'
spec.add_development_dependency 'rake', '>= 10.0'
end
18 changes: 9 additions & 9 deletions lib/array_enum.rb
Original file line number Diff line number Diff line change
@@ -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 = '%<value>s is not a valid value for %<attr>s'.freeze
private_constant :MISSING_VALUE_MESSAGE

def array_enum(definitions)
Expand All @@ -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

Expand All @@ -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
Expand Down
80 changes: 43 additions & 37 deletions test/array_enum_test.rb
Original file line number Diff line number Diff line change
@@ -1,120 +1,126 @@
require "test_helper"
require 'test_helper'

class ArrayEnumTest < Minitest::Test
def setup
User.delete_all
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
Loading

0 comments on commit 0565942

Please sign in to comment.