solidus
solidus copied to clipboard
Product ordering on taxon show page
(I'm not using the issue template because I'm not actually sure of the correct/expected behaviour here...)
If a product (Product A) is attached to two taxons (Taxon A and Taxon B), which are related to each other as parent and child, that product could have two different positions, one for its association with Taxon A and one for its association with Taxon B. This causes a problem when trying to paginate a list of products that belong to Taxon A and Taxon A's children, because the list of distinct, sorted products to be paginated duplicates Product A - because Product A has two distinct position values. The first X number of products from the list actually has 1 fewer distinct products than expected.
It's also not clear how products should be ordered on a taxon page that includes products related to the parent and children taxons. One product could be positioned second on the parent taxon. Another product could be positioned first on the child taxon. Should the latter product come top? What if they are both positioned first in their respective taxons? And, related to the point above, what if a product is positioned first on the parent taxon and the same product is positioned second on the child taxon - which position should take precedence?
I'm aware I may be misunderstanding how taxons are designed to be used, in which case I'd really appreciate some clarity. If a product should only be related to one taxon per taxon tree, there should probably be a validation constraint to enforce this. But also, if that is the case, I'm still unsure how products should be ordered on a taxon page, and in particular how the admin could control the order on a given taxon page.
I found this bug too.
This behavior is caused by this code (https://github.com/solidusio/solidus/blob/master/core/app/models/spree/product/scopes.rb#L65)
add_search_scope :in_taxon do |taxon|
includes(:classifications).
where("spree_products_taxons.taxon_id" => taxon.self_and_descendants.pluck(:id)).
order(Spree::Classification.arel_table[:position].asc)
end
And there is not a single chance that it will work correctly for products attached to several taxons that coincide with the condition
where("spree_products_taxons.taxon_id" => taxon.self_and_descendants.pluck(:id))
It looks like this bug was born when positions was added to classifications (products_taxons).
For quick fix you can override this scope.
add_search_scope :in_taxon do |taxon|
includes(:classifications).
where("spree_products_taxons.taxon_id" => taxon.id).
order(Spree::Classification.arel_table[:position].asc)
end
I see this issue is old, I'm having a related issue with pagination.
Pages are calculated incorrectly as the scope is applying a distinct sql clause on spree_products_taxons.position which doesn't consider the possibility of having 2 products with the same position on 2 different taxonomies.
I'm currently patching this line https://github.com/solidusio/solidus/blob/3724171102e737afa6afed5f8eb9ed58c0063e7b/core/app/models/spree/product/scopes.rb#L73
replacing it with:
.select(Spree::Classification.arel_table[:product_id])
With this small tweak the distinct will be applied on product IDs.
And BTW, I'm currently overriding on my App the position ordering and using alphabetical, I'm not exactly sure if this breaks something else at this point.
This is an interesting one. I'd expect the the product to show multiple times if it was included multiple times, and that the pagination would correctly handle that situation.
Maybe I'm not following but why would one want a product to appear multiple times when listing products inside a given taxonomy?
Honestly, upon review, I'm not sure what the desired behaviour is here. The initial issue outlines possibilities:
It's also not clear how products should be ordered on a taxon page that includes products related to the parent and children taxons. One product could be positioned second on the parent taxon. Another product could be positioned first on the child taxon. Should the latter product come top? What if they are both positioned first in their respective taxons? And, related to the point above, what if a product is positioned first on the parent taxon and the same product is positioned second on the child taxon - which position should take precedence?
Yeah me neither, the sort order could be defined in different and arbitrary ways.
I think that order even if arbitrary is not that big of a deal. The big problem is that there are products that don't show up at all when they have the same position in a parent/child taxon hierarchy.
I did some monkey patching on Spree::Product to fix this issue locally. I can't say I'm super comfortable with the solution but it gets the job done. Here it is in case you want to take a look at it.
module ProductDecorator
def self.prepended(base)
base.class_eval do
# Hack to get around multiple products with same `position` in parent/child
# taxon relationship.
#
# See:
# https://github.com/solidusio/solidus/issues/2551
return unless ::Spree::Product.search_scopes.include?(:in_taxon)
singleton_class.send(:define_method, :in_taxon) do |taxon|
includes(:classifications)
.where("spree_products_taxons.taxon_id" => taxon.self_and_descendants.pluck(:id))
.select(Spree::Classification.arel_table[:product_id])
.order(Spree::Classification.arel_table[:position].asc)
end
end
end
::Spree::Product.prepend self
end
What's the use case for having a product in a taxon as well as one of its children?
I believe when you have a category/sub-category organization for products you'd like a product to appear both in the general category as well as in the more specific one. At least that is the use case I'm facing.
As I understand it, this issue relates to the situation when you're displaying the products of a taxon and its children. Perhaps I'm missing something.
I was also struggling with the same issue until i found this thread. in the latest master, i found this:
https://github.com/solidusio/solidus/blob/35d6d71cc660e9fd375d4d326266f031e2ffb0e1/core/app/models/spree/product/scopes.rb#L76-L83
If you don't want to monkey-patch, you can also adjust your base scope and change:
base_scope = base_scope.in_taxon(@properties[:taxon]) if @properties[:taxon].present? to
if @properties[:taxon].present?
base_scope = base_scope.in_taxons(@properties[:taxon])
base_scope = base_scope.select('spree_products.*, spree_products_taxons.position').distinct
.order('spree_products_taxons.position')
end
It seems to fix my problem, because in_taxons does the magic that is needed to filter out duplicates, but be aware that it does not order by position as the in_taxon did. therefore i added the ordering again, which seems not yet like the perfect solution.
But i think somewhere in in_taxons we could find the solution!