spree-multi-domain icon indicating copy to clipboard operation
spree-multi-domain copied to clipboard

[BUG] Loading wrong Taxonomies on Startpage

Open ghost opened this issue 10 years ago • 12 comments

I installed SpreeCommerce and added multi-domain-support setted up the domains

  • store-a.local (store_id 1)
  • store-b.local (store_id 2)

i created 2 taxonomies

  • store-a-taxonomie (store_id 1)
  • store-b-taxonomie (store_id 2)

i expect the behavior, that store-a.local shows me store-a-taxonomie. unfortunetly if i visit store-a.local or store-b.local BOTH taxonomies are shown.

i checked out the code and figured out that the basic spree controller is not loading the taxonomies by the shop correctly.

https://github.com/spree/spree-multi-domain/blob/master/lib/spree_multi_domain/multi_domain_helpers.rb provides us the method

def get_taxonomies
  @taxonomies ||= current_store.present? ? Spree::Taxonomy.where(["store_id = ?", current_store.id]) : Spree::Taxonomy
  @taxonomies = @taxonomies.includes(:root => :children)
  @taxonomies
end

https://github.com/spree/spree/blob/master/frontend/app/controllers/spree/home_controller.rb

When i looked up the HomeController of Spree i pointed out, that the taxonomies are not loaded regarding the store_id

def index
  @searcher = build_searcher(params)
  @products = @searcher.retrieve_products
  @taxonomies = Spree::Taxonomy.includes(root: :children)
end

it needs to be changed to

@taxonomies = get_taxonomies

to work the proper way, as expected.

Hotfix would be overriding the controller in my app/controllers/spree/home_controller.rb are there any other suggestions how to get the correct behaviour working?

cheers

ghost avatar Mar 11 '14 12:03 ghost

Could you please provide a pull request fixing this behavior with a regression test.

The second line of the get_taxonomies method should probably be ||= not = as that will always set them wrong.

JDutil avatar Mar 11 '14 12:03 JDutil

@jdutil The second line operates on the value of the first line, so that seems to be correct behaviour.

phillipp avatar Mar 11 '14 12:03 phillipp

Well, just discovered that it also needs to be fixed inside the TaxonsController. Still wondering that no one have reported such behaviour. Is there any chance my missunderstanding the way how multi-domain-spree should work?

ghost avatar Mar 11 '14 12:03 ghost

Oh yea that's what I get for not taking my time reading that.

JDutil avatar Mar 11 '14 12:03 JDutil

No your understanding is correct on how it should work, but as you've mentioned it is not going to work properly if get_taxonomies is not the method called to find the taxonomies. This is likely a behavior that has changed in Spree causing this extension to break.

JDutil avatar Mar 11 '14 12:03 JDutil

As far as I can see, #83, #84, #85, and #86 are all related. Why are there four threads for one discussion?

radar avatar Mar 13 '14 03:03 radar

Let's keep the discussion of how to handle this here. I am leaving #84 open because it includes a possible fix.

radar avatar Mar 13 '14 03:03 radar

@radar - thats a good question. currently i'm setting up a multi-store environment with 15 stores, so i see all this issues. you are right - it is one discussion, but these are 4 different problems - regarding one thing: wrong taxonomy associations. i'm just reporting all these "wrong" behaviours so we can have a list of all issues and if i find a way to hotfix it, im doin it so far. is that the wrong way to contribute to a repository?

ghost avatar Mar 13 '14 09:03 ghost

@radar It seems to mee that @fu-media is right, these are seperate issues. Why not keep them seperated for better clarity of what is still pending? IMHO a pull request should be small and fix one thing, not fix every problem in this whole extension.

phillipp avatar Mar 13 '14 14:03 phillipp

It sounds like it could all be fixed in one place: where the taxonomies are being looked up. I'm fine with one PR which fixes both the frontend and backend, because they're related fixes. Let's keep the discussion on as few threads as possible. Thanks.

radar avatar Mar 13 '14 22:03 radar

Okay, then i'm "out" of this, since im not so familar with social-coding and github. I struggle just with making a little pull-request - and the last one was horror since there have been 2 other open ones.

i'll just go on with my project and make a list of all bugs need to be fixed, which i will contribute.

ghost avatar Mar 14 '14 00:03 ghost

@fu-media Thank you. I will leave this issue open as we will need to address it at some point.

radar avatar Mar 17 '14 05:03 radar