Skip to content

Commit

Permalink
Deduplicate menu item visibility checks
Browse files Browse the repository at this point in the history
The condition option was duplicating what the children were already
checking.
  • Loading branch information
elia committed Sep 21, 2023
1 parent b41415d commit ff604f6
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 52 deletions.
4 changes: 2 additions & 2 deletions backend/app/views/spree/admin/shared/_tabs.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<% Spree::Backend::Config.menu_items.sort_by { |item| item.position || Float::INFINITY }.each do |menu_item| %>
<% if instance_exec(&menu_item.condition) %>
<% if menu_item.render_in?(self) %>
<%=
tab(
icon: menu_item.icon,
Expand All @@ -18,7 +18,7 @@
label: child.label,
url: child.url,
selected: child.match_path?(request),
) if instance_exec(&child.condition) %>
) if child.render_in?(self) %>
<% end %>
</ul>
<% end %>
Expand Down
40 changes: 1 addition & 39 deletions backend/lib/spree/backend_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,6 @@ def menu_items
label: :products,
icon: 'th-large',
condition: -> { can?(:admin, Spree::Product) },
match_path: %r{/(
option_types|
product_properties|
products|
properties|
taxonomies|
taxons|
variants
)}x,
partial: 'spree/admin/shared/product_sub_menu',
position: 1,
data_hook: :admin_product_sub_tabs,
Expand Down Expand Up @@ -134,7 +125,6 @@ def menu_items
MenuItem.new(
label: :promotions,
icon: 'bullhorn',
match_path: %r{/(promotions|promotion_categories)},
partial: 'spree/admin/shared/promotion_sub_menu',
condition: -> { can?(:admin, Spree::Promotion) },
url: :admin_promotions_path,
Expand Down Expand Up @@ -170,37 +160,9 @@ def menu_items
MenuItem.new(
label: :settings,
icon: 'wrench',
match_path: %r{/(
adjustment_reasons|
payment_methods|
refund_reasons|
reimbursement_types|
return_reasons|
shipping_categories|
shipping_methods|
stock_locations|
store_credit_reasons|
stores|
tax_categories|
tax_rates|
zones
)}x,
data_hook: :admin_settings_sub_tabs,
condition: -> {
can?(:admin, Spree::Store) ||
can?(:admin, Spree::AdjustmentReason) ||
can?(:admin, Spree::PaymentMethod) ||
can?(:admin, Spree::RefundReason) ||
can?(:admin, Spree::ReimbursementType) ||
can?(:admin, Spree::ShippingCategory) ||
can?(:admin, Spree::ShippingMethod) ||
can?(:admin, Spree::StockLocation) ||
can?(:admin, Spree::TaxCategory) ||
can?(:admin, Spree::TaxRate) ||
can?(:admin, Spree::ReturnReason) ||
can?(:admin, Spree::Zone)
},
partial: 'spree/admin/shared/settings_sub_menu',
condition: -> { can? :admin, Spree::Store },
url: :admin_stores_path,
position: 5,
children: [
Expand Down
5 changes: 5 additions & 0 deletions backend/lib/spree/backend_configuration/menu_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ def initialize(
@match_path = match_path
end

def render_in?(view_context)
view_context.instance_exec(&@condition) ||
children.any? { |child| child.render_in?(view_context) }
end

def render_partial?
return false if partial.blank?

Expand Down
20 changes: 9 additions & 11 deletions backend/spec/lib/spree/backend_configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,18 @@
let(:menu_item) { subject.find { |item| item.label == :settings } }
let(:view) { double("view") }

it 'is shown if any of its submenus are present' do
allow(view).to receive(:can?).and_return(true, false)
describe '#render_in?' do
it 'is shown if any of its submenus are present' do
allow(view).to receive(:can?).and_return(true, false)

result = view.instance_exec(&menu_item.condition)

expect(result).to eq(true)
end

it 'is not shown if none of its submenus are present' do
expect(view).to receive(:can?).exactly(12).times.and_return(false)
expect(menu_item.render_in?(view)).to eq(true)
end

result = view.instance_exec(&menu_item.condition)
it 'is not shown if none of its submenus are present' do
expect(view).to receive(:can?).exactly(13).times.and_return(false)

expect(result).to eq(false)
expect(menu_item.render_in?(view)).to eq(false)
end
end
end
end
Expand Down

0 comments on commit ff604f6

Please sign in to comment.