From d9bc8492725826123cfbba8d4dd8f876caa4cfac Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Mon, 2 Oct 2023 19:19:17 +0200 Subject: [PATCH 1/2] Fix MenuItem#match_path? specs subject definitions --- .../spree/backend_configuration/menu_item_spec.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/backend/spec/lib/spree/backend_configuration/menu_item_spec.rb b/backend/spec/lib/spree/backend_configuration/menu_item_spec.rb index 7405cb59538..639aad632c5 100644 --- a/backend/spec/lib/spree/backend_configuration/menu_item_spec.rb +++ b/backend/spec/lib/spree/backend_configuration/menu_item_spec.rb @@ -13,31 +13,31 @@ describe '#match_path?' do it 'matches a string using the admin path prefix' do - described_class.new(match_path: '/stock_items') + subject = described_class.new(match_path: '/stock_items') request = double(ActionDispatch::Request, fullpath: '/admin/stock_items/1/edit') - expect(subject.match_path?(request)).to be true + expect(subject.match_path?(request)).to be_truthy end it 'matches a proc accepting the request object' do request = double(ActionDispatch::Request, fullpath: '/foo/bar/baz') subject = described_class.new(match_path: -> { _1.fullpath.include? '/bar/' }) - expect(subject.match_path?(request)).to be true + expect(subject.match_path?(request)).to be_truthy end it 'matches a regexp' do - described_class.new(match_path: %r{/bar/}) + subject = described_class.new(match_path: %r{/bar/}) request = double(ActionDispatch::Request, fullpath: '/foo/bar/baz') - expect(subject.match_path?(request)).to be true + expect(subject.match_path?(request)).to be_truthy end it 'matches the item url as the fullpath prefix' do - described_class.new(url: '/foo/bar') + subject = described_class.new(url: '/foo/bar') request = double(ActionDispatch::Request, fullpath: '/foo/bar/baz') - expect(subject.match_path?(request)).to be true + expect(subject.match_path?(request)).to be_truthy end end From d6cb2389c09a413a6272e460d211e37218c7bc1c Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Mon, 2 Oct 2023 19:22:58 +0200 Subject: [PATCH 2/2] Include MenuItem#sections while matching path Using sections for matching the path was originally done inside the #tab helper, when the responsibility was moved to MenuItem this use- case was lost. Not anymore. --- .../spree/backend_configuration/menu_item.rb | 37 +++++++++---------- .../backend_configuration/menu_item_spec.rb | 20 ++++++++++ 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/backend/lib/spree/backend_configuration/menu_item.rb b/backend/lib/spree/backend_configuration/menu_item.rb index 6e42b8d39bd..9ba33cef66a 100644 --- a/backend/lib/spree/backend_configuration/menu_item.rb +++ b/backend/lib/spree/backend_configuration/menu_item.rb @@ -54,7 +54,7 @@ def initialize( end @condition = condition || -> { true } - @sections = sections + @sections = sections || [] @icon = icon @label = label @partial = partial @@ -78,27 +78,26 @@ def render_partial? end def match_path?(request) - if match_path.is_a? Regexp - request.fullpath =~ match_path - elsif match_path.respond_to?(:call) - match_path.call(request) - elsif match_path - request.fullpath.starts_with?("#{spree.admin_path}#{match_path}") - else - request.fullpath.to_s.starts_with?(url.to_s) - end + matches = + if match_path.is_a? Regexp + request.fullpath =~ match_path + elsif match_path.respond_to?(:call) + match_path.call(request) + elsif match_path + request.fullpath.starts_with?("#{spree.admin_path}#{match_path}") + end + matches ||= request.fullpath.to_s.starts_with?(url.to_s) if url.present? + matches ||= @sections.include?(request.controller_class.controller_name.to_sym) if @sections.present? + + matches end def url - if @url.respond_to?(:call) - @url.call - elsif @url.is_a?(Symbol) - spree.public_send(@url) - elsif @url.nil? && @label - spree.send("admin_#{@label}_path") - else - @url - end + url = @url.call if @url.respond_to?(:call) + url ||= spree.public_send(@url) if @url.is_a?(Symbol) && spree.respond_to?(@url) + url ||= spree.send("admin_#{@label}_path") if @url.nil? && @label && spree.respond_to?("admin_#{@label}_path") + url ||= @url.to_s + url end private diff --git a/backend/spec/lib/spree/backend_configuration/menu_item_spec.rb b/backend/spec/lib/spree/backend_configuration/menu_item_spec.rb index 639aad632c5..17100be1212 100644 --- a/backend/spec/lib/spree/backend_configuration/menu_item_spec.rb +++ b/backend/spec/lib/spree/backend_configuration/menu_item_spec.rb @@ -39,6 +39,26 @@ expect(subject.match_path?(request)).to be_truthy end + + it 'matches the item on the (deprecated) sections against the controller name' do + allow(Spree.deprecator).to receive(:warn).with(a_string_matching(/icon/)) + allow(Spree.deprecator).to receive(:warn).with(a_string_matching(/sections/)) + + subject = described_class.new([:foo, :bar], :baz_icon) + matching_request = double( + ActionDispatch::Request, + controller_class: double(ActionController::Base, controller_name: 'bar'), + fullpath: '/qux', + ) + other_request = double( + ActionDispatch::Request, + controller_class: double(ActionController::Base, controller_name: 'baz'), + fullpath: '/qux', + ) + + expect(subject.match_path?(matching_request)).to be true + expect(subject.match_path?(other_request)).to be false + end end describe "#url" do