Skip to content

Commit

Permalink
Replace custom product ransacker with association
Browse files Browse the repository at this point in the history
We introduced a custom Ransacker to allow filtering producs by
their variants option values in the new admin interface.

This ransacker is broken in latest Ransack v4.2 and overly
complicated.  By introducing a option_values association to
products, that does the same join over variants_including_master
and their distinct option_values we can use normal ransack search.

Quoting the Ransack documentation:

> "Ransackers, like scopes, are not a cure-all. Many use cases can be better solved with a standard Ransack search on a dedicated database search field, which is faster, index-able, and scales better than converting/ransacking data on the fly."
  • Loading branch information
tvdeyen committed Sep 12, 2024
1 parent a791a5f commit d3b9db3
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def filters
{
label: option_type.presentation,
combinator: 'or',
attribute: "variants_option_values",
attribute: "option_values_id",
predicate: "in",
options: option_type.option_values.pluck(:name, :id),
}
Expand Down
24 changes: 4 additions & 20 deletions core/app/models/spree/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ class Product < Spree::Base
has_many :line_items, through: :variants_including_master
has_many :orders, through: :line_items

has_many :option_values, -> { distinct }, through: :variants_including_master

scope :sort_by_master_default_price_amount_asc, -> {
with_default_price.order('spree_prices.amount ASC')
}
Expand Down Expand Up @@ -127,26 +129,8 @@ def find_or_build_master

alias :options :product_option_types

# The :variants_option_values ransacker filters Spree::Product based on
# variant option values ids.
#
# Usage:
# Spree::Product.ransack(
# variants_option_values_in: [option_value_id1, option_value_id2]
# ).result
ransacker :variants_option_values, formatter: proc { |v|
if OptionValue.exists?(v)
joins(variants_including_master: :option_values)
.where(spree_option_values: { id: v })
.distinct
.select(:id).arel
end
} do |parent|
parent.table[:id]
end

self.allowed_ransackable_associations = %w[stores variants_including_master master variants]
self.allowed_ransackable_attributes = %w[name slug variants_option_values]
self.allowed_ransackable_associations = %w[stores variants_including_master master variants option_values]
self.allowed_ransackable_attributes = %w[name slug]
self.allowed_ransackable_scopes = %i[available with_discarded with_all_variant_sku_cont with_kept_variant_sku_cont]

# @return [Boolean] true if there are any variants
Expand Down
12 changes: 6 additions & 6 deletions core/spec/models/spree/product_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -627,13 +627,13 @@ class Extension < Spree::Base
end
end

context "ransacker :variants_option_values" do
describe "ransack :option_values_id_in" do
it "filters products based on option values of their variants" do
product_1 = create(:product)
option_value_1 = create(:option_value)
create(:variant, product: product_1, option_values: [option_value_1])

result = Spree::Product.ransack(variants_option_values_in: [option_value_1.id]).result
result = Spree::Product.ransack(option_values_id_in: [option_value_1.id]).result
expect(result).to contain_exactly(product_1)
end

Expand All @@ -644,13 +644,13 @@ class Extension < Spree::Base
create(:variant, product: product_1, option_values: [option_value_1])
create(:variant, product: product_2, option_values: [option_value_1])

result = Spree::Product.ransack(variants_option_values_in: [option_value_1.id]).result
result = Spree::Product.ransack(option_values_id_in: [option_value_1.id]).result
expect(result).to contain_exactly(product_1, product_2)
end

it "returns no products if there is no match" do
non_existing_option_value_id = 99999
result = Spree::Product.ransack(variants_option_values_in: [non_existing_option_value_id]).result
result = Spree::Product.ransack(option_values_id_in: [non_existing_option_value_id]).result
expect(result).to be_empty
end

Expand All @@ -662,7 +662,7 @@ class Extension < Spree::Base
create(:variant, product: product_1, option_values: [option_value_1])
create(:variant, product: product_2, option_values: [option_value_2])

result = Spree::Product.ransack(variants_option_values_in: [option_value_1.id, option_value_2.id]).result
result = Spree::Product.ransack(option_values_id_in: [option_value_1.id, option_value_2.id]).result
expect(result).to contain_exactly(product_1, product_2)
end

Expand All @@ -674,7 +674,7 @@ class Extension < Spree::Base
create(:variant, product: product_1, option_values: [option_value_1])
create(:variant, product: product_2, option_values: [option_value_2])

result = Spree::Product.ransack(variants_option_values_in: [option_value_1.id]).result
result = Spree::Product.ransack(option_values_id_in: [option_value_1.id]).result
expect(result).not_to include(product_2)
end
end
Expand Down

0 comments on commit d3b9db3

Please sign in to comment.