Merge pull request solidusio#5406 from solidusio/elia/menu-items-sect…

…ions

Restore using `MenuItem#sections` for matching paths

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

backend/spec/lib/spree/backend_configuration/menu_item_spec.rb

@@ -13,31 +13,51 @@
 
   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
+
+    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