Categories page needs a check

[blocknotes]

These are our products (12) in sandbox app:

[{:id=>1, :taxons=>["categories/clothing/t-shirts", "brand/solidus"]},
 {:id=>2, :taxons=>["categories/clothing/shirts", "brand/solidus"]},
 {:id=>3, :taxons=>["categories/clothing/shirts", "brand/solidus"]},
 {:id=>4, :taxons=>["categories/caps", "brand/solidus"]},
 {:id=>5, :taxons=>["categories/clothing/hoodie", "brand/solidus"]},
 {:id=>6, :taxons=>["categories/clothing/hoodie", "brand/ruby"]},
 {:id=>7, :taxons=>["categories/clothing/hoodie", "brand/ruby"]},
 {:id=>8, :taxons=>["categories/clothing/shirts", "brand/ruby"]},
 {:id=>9, :taxons=>["categories/mugs", "brand/solidus"]},
 {:id=>10, :taxons=>["categories/mugs", "brand/ruby"]},
 {:id=>11, :taxons=>["categories/bags", "brand/solidus"]},
 {:id=>12, :taxons=>["categories/bags", "brand/ruby"]}]

But in the categories page (http://localhost:3000/t/categories) we have

• duplicated products
• pagination is broken?
  • http://localhost:3000/t/categories?per_page=4 => no pagination links
  • http://localhost:3000/t/categories/clothing?per_page=1 => is it considering each sub taxon?

[kennyadsl]

Not sure it's related but the clothing category is empty in the demo. http://demo.solidus.io/t/categories/clothing

[waiting-for-dev]

Some comments after investigating from my side.

I'm new to solidus, but something that stroke me is that http://localhost:3000/t/categories is a link to a taxon page in opposition to a taxonomy page. My understanding is that taxonomies are higher-level hierarchies, and I had expected that such a top position in the sitemap would have corresponded to one of them:

In fact, we have both taxonomy and taxon with the name Categories in the sandbox application:

irb(main):144:0> Taxonomy.find_by_name('Categories')
  Spree::Taxonomy Load (0.2ms)  SELECT "spree_taxonomies".* FROM "spree_taxonomies" WHERE "spree_taxonomies"."name" = ? ORDER BY "spree_taxonomies"."position" ASC LIMIT ?  [["name", "Categories"], ["LIMIT", 1]]
=> #<Spree::Taxonomy id: 1, name: "Categories", created_at: "2021-03-02 04:44:17.869375000 +0000", updated_at: "2021-03-02 04:45:00.185461000 +0000", position: 1>
irb(main):145:0> Taxon.find_by_name('Categories')
  Spree::Taxon Load (0.1ms)  SELECT "spree_taxons".* FROM "spree_taxons" WHERE "spree_taxons"."name" = ? LIMIT ?  [["name", "Categories"], ["LIMIT", 1]]
=> #<Spree::Taxon id: 1, parent_id: nil, position: 0, name: "Categories", permalink: "categories", taxonomy_id: 1, lft: 1, rgt: 16, icon_file_name: nil, icon_content_type: nil, icon_file_size: nil, icon_updated_at: nil, description: nil, created_at: "2021-03-02 04:44:17.893612000 +0000", updated_at: "2021-03-02 04:45:00.184130000 +0000", meta_title: nil, meta_description: nil, meta_keywords: nil, depth: 0>

duplicated products

With the previous observation in mind, it's not unexpected to have duplicated products on that page. The top Categories section renders all the products that belong to that taxonomy, while each of the sections below is refining down to a particular child. For instance, given the case of "Solidus Long Sleeve" product:

irb(main):152:0> Product.find_by_name('Solidus Long Sleeve').taxons.pluck(:name)
  Spree::Product Load (0.4ms)  SELECT "spree_products".* FROM "spree_products" WHERE "spree_products"."deleted_at" IS NULL AND "spree_products"."name" = ? LIMIT ?  [["name", "Solidus Long Sleeve"], ["LIMIT", 1]]
   (0.3ms)  SELECT "spree_taxons"."name" FROM "spree_taxons" INNER JOIN "spree_products_taxons" ON "spree_taxons"."id" = "spree_products_taxons"."taxon_id" WHERE "spree_products_taxons"."product_id" = ?  [["product_id", 2]]
=> ["Shirts", "Solidus"]
irb(main):153:0> Taxon.find_by_name('Shirts').parent.name
  Spree::Taxon Load (0.2ms)  SELECT "spree_taxons".* FROM "spree_taxons" WHERE "spree_taxons"."name" = ? LIMIT ?  [["name", "Shirts"], ["LIMIT", 1]]
  Spree::Taxon Load (0.1ms)  SELECT "spree_taxons".* FROM "spree_taxons" WHERE "spree_taxons"."id" = ? LIMIT ?  [["id", 3], ["LIMIT", 1]]
=> "Clothing"
irb(main):154:0> Taxon.find_by_name('Shirts').parent.parent.name
  Spree::Taxon Load (0.2ms)  SELECT "spree_taxons".* FROM "spree_taxons" WHERE "spree_taxons"."name" = ? LIMIT ?  [["name", "Shirts"], ["LIMIT", 1]]
  Spree::Taxon Load (0.1ms)  SELECT "spree_taxons".* FROM "spree_taxons" WHERE "spree_taxons"."id" = ? LIMIT ?  [["id", 3], ["LIMIT", 1]]
  Spree::Taxon Load (0.1ms)  SELECT "spree_taxons".* FROM "spree_taxons" WHERE "spree_taxons"."id" = ? LIMIT ?  [["id", 1], ["LIMIT", 1]]
=> "Categories"

http://localhost:3000/t/categories?per_page=4 => no pagination links

It seems they do appear now (however not with the best style):

http://localhost:3000/t/categories/clothing?per_page=1 => is it considering each sub taxon?

It seems it does now:

[kennyadsl]

Having each taxonomy with a "root" taxon inside with the same name is a design choice: we want to have a root node in the tree that represents the main level of the category.

You might argue why we need both taxons and taxonomies, it would be a good question. I think the answer is to conceptually split the ways to classify products, for example: Brands and Categories are probably hard to push in the same bucket of taxons conceptually.

More info here: https://guides.solidus.io/developers/products-and-variants/taxonomies-and-taxons.html

So, yes it's expected to have this kind of duplication. I think this is more a UX issue than a technical one, what we see reflects what the code is supposed to work.

[waiting-for-dev]

You might argue why we need both taxons and taxonomies, it would be a good question. I think the answer is to conceptually split the ways to classify products, for example: Brands and Categories are probably hard to push in the same bucket of taxons conceptually.

IMHO it's kind of confusing. I'd say that the very fundamental purpose of taxonomies is that of splitting up conceptually different taxons: A taxon belongs to a taxonomy. It's not such a big deal, because, in the end, we're just talking about a design decision made in the sample application. But it's true that it could be interpreted as a recommended practice.

[kennyadsl]

I agree and I think part of the issue is due to trying to make this fit into the gem we use for this.

[waiting-for-dev]

@kennyadsl I now understand what you meant by a design choice. I see it's a design choice in solidus itself:

https://github.com/solidusio/solidus/blob/93bfdb4ab11951b4bccc3d8a42f9bde3b77205ac/core/app/models/spree/taxonomy.rb#L10

The behavior is so embedded that this consistency is enforced automatically by a hook:

https://github.com/solidusio/solidus/blob/93bfdb4ab11951b4bccc3d8a42f9bde3b77205ac/core/app/models/spree/taxonomy.rb#L12

https://github.com/solidusio/solidus/blob/93bfdb4ab11951b4bccc3d8a42f9bde3b77205ac/core/app/models/spree/taxonomy.rb#L20-L29

You can create a new taxonomy in the UI:

But when you edit it you're doing so in relation to this automatically created taxon:

This can lead to inconsistencies. For instance, when you rename a taxonomy its root taxon gets also updated, but it doesn't happen if what you rename is the root taxon in the first place:

I think that this behavior is brittle and breaks with the principle of least surprise. On top of that, it's something that is not documented and, unless I'm missing something, makes the taxonomy model completely useless. I'm not sure about which options are available for us because a fix for it would probably have backward incompatibility issues. In a free of any constraints scenario, probably the best solution would be to deprecate the taxonomy model.

[kennyadsl]

Yes, I think that deprecating taxonomies is a good idea for keeping backward compatibility, and as soon as we provide a valid alternative and a clear migration path, that's totally doable. I think we may also have a terminology problem removing taxonomies. For example, when you need a new classification in your store (for example "brands"), you have to technically create a taxon, but that's not a normal taxon, it is a "root taxon" because it doesn't have any taxon parent. I think (maybe) that's part of why the taxonomy model has been added.

[waiting-for-dev]

Yeah, the other option is to remove root taxons and maintain taxonomies. Probably it would be more correct semantically but it would be harder to integrate. But, anyway, the possibility of inconsistencies is inherent to parent_id, as you can have circular references at the data layer, for instance. One way or the other you need to put constraints on the application layer to enforce consistency, so having root taxons is not something that looks ugly to me.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It might be closed if no further activity occurs. Thank you for your contributions.