spree-multi-domain
spree-multi-domain copied to clipboard
[BUG] Loading wrong Taxonomies on Startpage
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
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 The second line operates on the value of the first line, so that seems to be correct behaviour.
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?
Oh yea that's what I get for not taking my time reading that.
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.
As far as I can see, #83, #84, #85, and #86 are all related. Why are there four threads for one discussion?
Let's keep the discussion of how to handle this here. I am leaving #84 open because it includes a possible fix.
@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?
@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.
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.
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.
@fu-media Thank you. I will leave this issue open as we will need to address it at some point.