Deprecate weird taxon product filters #2408
Conversation
This doesn't look like it was ever used, and I can't imagine anyone finding it randomly and using it. Better to pull it out of core and let people add it themselves if they want it. Also, its untested.
|
@kennyadsl @mamhoff I threw another commit on here before I saw your approval. I can either split this up into another PR or you can weigh in on it being okay. |
cbrunsdon
changed the title
| return Spree::Core::ProductFilters.all_taxons if taxon.nil? | ||
| { | ||
| name: 'Taxons under ' + taxon.name, | ||
| scope: :taxons_id_in_tree_any, | ||
| labels: taxon.children.sort_by(&:position).map { |t| [t.name, t.id] }, | ||
| scope: :taxons_id_in_tree_any, labels: taxon.children.sort_by(&:position).map { |t| [t.name, t.id] }, |
There was a problem hiding this comment.
nice catch, sorry. fixed
I can't find any reasonable circumstance this (untested) method would ever ran in. These are all very difficult to understand or use, but would be easy for host applications to add in themselves if they wanted to continue using them.
cbrunsdon
force-pushed
the
deprecate_taxons_below
branch
from
95c1257 to
fff13ea
Compare
| @@ -1,4 +1,4 @@ | |||
| <% filters = @taxon ? @taxon.applicable_filters : [Spree::Core::ProductFilters.all_taxons] %> | |||
| <% filters = @taxon.applicable_filters %> | |||
There was a problem hiding this comment.
Maybe this partial is or was rendered in some view that doesn't have @taxon?
There was a problem hiding this comment.
not anywhere in solidus_frontend, the only time its rendered I can find is here:
And in that case only if @taxon exists.
I personally have a vendetta against these methods because I only barely understand what is happening and don't think they're used or valuable.
There was a problem hiding this comment.
Oh, yes, seems like it's safe to remove them. I agree they are pretty useless, thanks!
|
I keep my approval. |
This doesn't look like these were ever used, and I can't imagine anyone
finding them randomly and using them.
Better to pull these out of core and let people add it themselves if they
want it.
Also, they're untested.