New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicated products when viewing by taxon #1917

Closed
greinacker opened this Issue Sep 5, 2012 · 6 comments

Comments

Projects
None yet
3 participants
@greinacker
Contributor

greinacker commented Sep 5, 2012

Just ran into this on Spree 1.2.0:

Suppose a simple taxonomy:

Animals

  • Dogs
  • Furry

If I add a product "Retriever" to the Dogs and Furry taxons, then I click on the "Animals" taxon in the app (the store side, not the admin side), I will see:

Animals:

  • Retriever
  • Retriever

Dogs:

  • Retriever

Furry:

  • Retriever

This is all as one would expect, except that the Retriever product is unexpectedly shown twice under the Animals taxon.

If I change this line of code:

https://github.com/spree/spree/blob/master/core/app/views/spree/shared/_products.html.erb#L14

to

<% products.uniq.each do |product| %>

That resolves the problem...however, I was thinking it might be better to fix the scope that's providing duplicates.

Thoughts?

@greinacker

This comment has been minimized.

Show comment
Hide comment
@greinacker

greinacker Sep 5, 2012

Contributor

FYI, I'm using the following as a workaround for now:

Deface::Override.new(:virtual_path => "spree/shared/_products",
                    :name => "products_uniqueify",
                    :replace => "code[erb-silent]:contains('products.each do |product|')",
                    :original => "<% products.each do |product| %>",
                    :text => "<% products.uniq.each do |product| %>")
Contributor

greinacker commented Sep 5, 2012

FYI, I'm using the following as a workaround for now:

Deface::Override.new(:virtual_path => "spree/shared/_products",
                    :name => "products_uniqueify",
                    :replace => "code[erb-silent]:contains('products.each do |product|')",
                    :original => "<% products.each do |product| %>",
                    :text => "<% products.uniq.each do |product| %>")
@ehoch

This comment has been minimized.

Show comment
Hide comment
@ehoch

ehoch Sep 11, 2012

Contributor

Funny, I'm using the same workaround. There's gotta be a more efficient SQL query than uniq but it goes beyond my active record skills.

Hey, core team, if I make failing specs, you guys down to fix?

Contributor

ehoch commented Sep 11, 2012

Funny, I'm using the same workaround. There's gotta be a more efficient SQL query than uniq but it goes beyond my active record skills.

Hey, core team, if I make failing specs, you guys down to fix?

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Sep 12, 2012

Member

Yeah, I think there's a better fix than uniq as well. If you can make up some failing specs that'd help a lot.

On Tuesday, 11 September 2012 at 9:03 PM, Eric Hochberger wrote:

Funny, I'm using the same workaround. There's gotta be a more efficient SQL query than uniq but it goes beyond my active record skills.
Hey, core team, if I make failing specs, you guys down to fix?


Reply to this email directly or view it on GitHub (#1917 (comment)).

Member

radar commented Sep 12, 2012

Yeah, I think there's a better fix than uniq as well. If you can make up some failing specs that'd help a lot.

On Tuesday, 11 September 2012 at 9:03 PM, Eric Hochberger wrote:

Funny, I'm using the same workaround. There's gotta be a more efficient SQL query than uniq but it goes beyond my active record skills.
Hey, core team, if I make failing specs, you guys down to fix?


Reply to this email directly or view it on GitHub (#1917 (comment)).

radar added a commit that referenced this issue Oct 18, 2012

@radar radar closed this in 8a303e1 Oct 18, 2012

radar added a commit that referenced this issue Oct 18, 2012

Fix multiple products from appearing when using Product.in_taxons scope
Fixes #1917

Fixes #1974

Fixes #1962

Conflicts:

	core/app/models/spree/product/scopes.rb
	core/spec/models/product/scopes_spec.rb

radar added a commit that referenced this issue Oct 18, 2012

Fix multiple products from appearing when using Product.in_taxons scope
Fixes #1917

Fixes #1974

Fixes #1962

Conflicts:

	core/app/models/spree/product/scopes.rb
	core/spec/models/product/scopes_spec.rb
@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Oct 18, 2012

Member

I think this problem is now fixed in the latest master, 1-1-stable and 1-2-stable branches.

Member

radar commented Oct 18, 2012

I think this problem is now fixed in the latest master, 1-1-stable and 1-2-stable branches.

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Oct 18, 2012

Member

Little caveat that you may already know: you need to pass the :distinct option when counting products returned from this query.

Spree::Product.in_taxon(taxon).count(:distinct => true)
Member

radar commented Oct 18, 2012

Little caveat that you may already know: you need to pass the :distinct option when counting products returned from this query.

Spree::Product.in_taxon(taxon).count(:distinct => true)
@ehoch

This comment has been minimized.

Show comment
Hide comment
@ehoch

ehoch Oct 18, 2012

Contributor

@radar Yeah. Which means we probably have issues with Kaminari and pagination...

Contributor

ehoch commented Oct 18, 2012

@radar Yeah. Which means we probably have issues with Kaminari and pagination...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment