Skip to content
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.

Sign up for GitHub

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

Deprecate weird taxon product filters #2408

Merged
merged 2 commits into from
Dec 20, 2017
Merged

Deprecate weird taxon product filters #2408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c56787b
Deprecate taxons_below
Nov 28, 2017
fff13ea
Deprecate all_taxons
Nov 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions core/app/models/spree/taxon.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ class Taxon < Spree::Base
# @return [Array] filters that should be used for a taxon
def applicable_filters
fs = []
# fs << ProductFilters.taxons_below(self)
## unless it's a root taxon? left open for demo purposes

fs << Spree::Core::ProductFilters.price_filter if Spree::Core::ProductFilters.respond_to?(:price_filter)
fs << Spree::Core::ProductFilters.brand_filter if Spree::Core::ProductFilters.respond_to?(:brand_filter)
fs
Expand Down
2 changes: 2 additions & 0 deletions core/lib/spree/core/product_filters.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ def self.selective_brand_filter(taxon = nil)
# This scope selects products in any of the active taxons or their children.
#
def self.taxons_below(taxon)
Spree::Deprecation.warn "taxons_below is deprecated in solidus_core. Please add it to your own application to continue using it."
return Spree::Core::ProductFilters.all_taxons if taxon.nil?
{
name: 'Taxons under ' + taxon.name,
Expand All @@ -180,6 +181,7 @@ def self.taxons_below(taxon)
#
# idea: expand the format to allow nesting of labels?
def self.all_taxons
Spree::Deprecation.warn "all_taxons is deprecated in solidus_core. Please add it to your own application to continue using it."
taxons = Spree::Taxonomy.all.map { |t| [t.root] + t.root.descendants }.flatten
{
name: 'All taxons',
Expand Down
2 changes: 1 addition & 1 deletion frontend/app/views/spree/shared/_filters.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<% filters = @taxon ? @taxon.applicable_filters : [Spree::Core::ProductFilters.all_taxons] %>
<% filters = @taxon.applicable_filters %>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this partial is or was rendered in some view that doesn't have @taxon?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not anywhere in solidus_frontend, the only time its rendered I can find is here:

https://github.com/solidusio/solidus/blob/master/frontend/app/views/spree/products/index.html.erb#L1-L9

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yes, seems like it's safe to remove them. I agree they are pretty useless, thanks!

<% unless filters.empty? %>
<%= form_tag '', method: :get, id: 'sidebar_products_search' do %>
<%= hidden_field_tag 'per_page', params[:per_page] %>
Expand Down