Navigation item path improvements (#480)

* Allow navigation item paths to be set using admin/action options as symbols
* Delegate #path method to bound admin when used within a navigation block
* Cleanup: remove Navigation::Item path from specs where not required

lib/trestle/navigation/block.rb

@@ -17,28 +17,26 @@ def items(context)
       class Evaluator
         include EvaluationContext
 
+        delegate :path, to: :@admin
+
         attr_reader :items
 
         def initialize(admin=nil, context=nil)
           @admin, @context = admin, context
           @items = []
         end
 
-        def default_path
-          @admin ? @admin.path : nil
-        end
-
         def item(name, path=nil, **options)
           if options[:group]
             group = Group.new(options[:group])
           elsif @current_group
             group = @current_group
           end
 
-          options = options.merge(group: group) if group
-          options = options.merge(admin: @admin) if @admin
+          options.merge!(group: group) if group
+          options.merge!(admin: @admin) if @admin
 
-          items << Item.new(name, path || default_path, **options)
+          items << Item.new(name, path, **options)
         end
 
         def group(name, **options)

lib/trestle/navigation/item.rb

@@ -1,7 +1,7 @@
 module Trestle
   class Navigation
     class Item
-      attr_reader :name, :path, :options
+      attr_reader :name, :options
 
       def initialize(name, path=nil, **options)
         @name, @path, @options = name.to_s, path, options
@@ -35,8 +35,25 @@ def group
         options[:group] || NullGroup.new
       end
 
+      def path
+        if @path
+          @path
+        elsif admin = self.admin
+          admin.path(options[:action] || :index)
+        else
+          "#"
+        end
+      end
+
       def admin
-        options[:admin]
+        case options[:admin]
+        when nil, false
+          return
+        when Symbol, String
+          Trestle.lookup(options[:admin]) or raise ActionController::UrlGenerationError, "No admin found named #{options[:admin].inspect}"
+        else
+          options[:admin]
+        end
       end
 
       def label
@@ -56,7 +73,7 @@ def badge
       end
 
       def html_options
-        options.except(:admin, :badge, :group, :icon, :if, :label, :priority, :unless)
+        options.except(:action, :admin, :badge, :group, :icon, :if, :label, :priority, :unless)
       end
 
       def visible?(context)

spec/trestle/navigation/block_spec.rb

@@ -18,7 +18,7 @@
 
       expect(items[1]).to eq(Trestle::Navigation::Item.new(:with_path, "/123"))
 
-      expect(items[2]).to eq(Trestle::Navigation::Item.new(:with_options, nil))
+      expect(items[2]).to eq(Trestle::Navigation::Item.new(:with_options))
       expect(items[2].options).to eq(icon: "fa fa-plus")
 
       expect(items[3]).to eq(Trestle::Navigation::Item.new(:with_path_and_options, "/path"))
@@ -74,6 +74,17 @@
       expect(items[1].path).to eq("/custom")
     end
 
+    it "delegates the path method to the admin" do
+      expect(admin).to receive(:path).with(:show).and_return("/456")
+
+      block = Trestle::Navigation::Block.new(admin) do
+        item :path_method, path(:show)
+      end
+
+      items = block.items(context)
+      expect(items[0].path).to eq("/456")
+    end
+
     it "yields the admin to the block" do
       expect { |b| Trestle::Navigation::Block.new(admin, &b).items(context) }.to yield_with_args(admin)
     end

spec/trestle/navigation/item_spec.rb

@@ -9,7 +9,7 @@
   end
 
   it "can override the label from options" do
-    item = Trestle::Navigation::Item.new(:test, nil, label: "Custom Label")
+    item = Trestle::Navigation::Item.new(:test, label: "Custom Label")
     expect(item.label).to eq("Custom Label")
   end
 
@@ -23,7 +23,7 @@
 
   it "sets the group from options" do
     group = Trestle::Navigation::Group.new(:test)
-    item = Trestle::Navigation::Item.new(:test, nil, group: group)
+    item = Trestle::Navigation::Item.new(:test, group: group)
 
     expect(item.group).to eq(group)
   end
@@ -33,23 +33,60 @@
   end
 
   it "sets the icon from options" do
-    item = Trestle::Navigation::Item.new(:test, nil, icon: "fa fa-user")
+    item = Trestle::Navigation::Item.new(:test, icon: "fa fa-user")
     expect(item.icon).to eq("fa fa-user")
   end
 
+  it "sets the path from parameters" do
+    item = Trestle::Navigation::Item.new(:test, "/path")
+    expect(item.path).to eq("/path")
+  end
+
+  it "sets the path from the admin via options (using symbol)" do
+    admin = double(path: "/admin")
+    expect(Trestle).to receive(:lookup).with(:admin).and_return(admin)
+    item = Trestle::Navigation::Item.new(:test, admin: :admin)
+    expect(item.path).to eq("/admin")
+  end
+
+  it "sets the path from the admin and action via options (using symbol)" do
+    admin = double
+    expect(admin).to receive(:path).with(:show).and_return("/admin")
+    expect(Trestle).to receive(:lookup).with(:admin).and_return(admin)
+    item = Trestle::Navigation::Item.new(:test, admin: :admin, action: :show)
+    expect(item.path).to eq("/admin")
+  end
+
+  it "sets the path from the admin via options (using class)" do
+    admin = double(path: "/admin")
+    item = Trestle::Navigation::Item.new(:test, admin: admin)
+    expect(item.path).to eq("/admin")
+  end
+
+  it "raises an error if symbol admin via options can't be found" do
+    expect(Trestle).to receive(:lookup).with(:admin).and_return(nil)
+    item = Trestle::Navigation::Item.new(:test, admin: :admin)
+    expect { item.path }.to raise_error(ActionController::UrlGenerationError, "No admin found named :admin")
+  end
+
+  it "uses # as the fallback path if no path or admin provided" do
+    item = Trestle::Navigation::Item.new(:test)
+    expect(item.path).to eq("#")
+  end
+
   it "sorts by priority" do
     i1 = Trestle::Navigation::Item.new(:test1)
-    i2 = Trestle::Navigation::Item.new(:test2, nil, priority: :first)
-    i3 = Trestle::Navigation::Item.new(:test3, nil, priority: :last)
-    i4 = Trestle::Navigation::Item.new(:test4, nil, priority: 50)
+    i2 = Trestle::Navigation::Item.new(:test2, priority: :first)
+    i3 = Trestle::Navigation::Item.new(:test3, priority: :last)
+    i4 = Trestle::Navigation::Item.new(:test4, priority: 50)
 
     expect([i1, i2, i3, i4].sort).to eq([i2, i1, i4, i3])
   end
 
   it "sorts by name if priority is equal" do
-    i1 = Trestle::Navigation::Item.new(:test1, nil, priority: 1)
-    i2 = Trestle::Navigation::Item.new(:test2, nil, priority: 1)
-    i3 = Trestle::Navigation::Item.new(:test3, nil, priority: 1)
+    i1 = Trestle::Navigation::Item.new(:test1, priority: 1)
+    i2 = Trestle::Navigation::Item.new(:test2, priority: 1)
+    i3 = Trestle::Navigation::Item.new(:test3, priority: 1)
 
     expect([i3, i1, i2].sort).to eq([i1, i2, i3])
   end
@@ -67,31 +104,31 @@
   end
 
   it "is not visible if options[:if] is provided and evaluates to false" do
-    item = Trestle::Navigation::Item.new(:test, nil, if: -> { false })
+    item = Trestle::Navigation::Item.new(:test, if: -> { false })
     expect(item.visible?(self)).to be false
   end
 
   it "is not visible if options[:unless] if provided and evaluates to true" do
-    item = Trestle::Navigation::Item.new(:test, nil, unless: -> { true })
+    item = Trestle::Navigation::Item.new(:test, unless: -> { true })
     expect(item.visible?(self)).to be false
   end
 
   it "can set html_options" do
-    item = Trestle::Navigation::Item.new(:test, nil, icon: "fa", class: "text-danger")
+    item = Trestle::Navigation::Item.new(:test, icon: "fa", class: "text-danger")
     expect(item.html_options).to eq({ class: "text-danger" })
   end
 
   context "with a badge" do
     it "has a badge" do
-      item = Trestle::Navigation::Item.new(:test, nil, badge: { text: "123", class: "badge-success" })
+      item = Trestle::Navigation::Item.new(:test, badge: { text: "123", class: "badge-success" })
 
       expect(item.badge?).to be true
       expect(item.badge.text).to eq("123")
       expect(item.badge.html_class).to eq("badge-success")
     end
 
     it "has a badge with a default class if full options not provided" do
-      item = Trestle::Navigation::Item.new(:test, nil, badge: "123")
+      item = Trestle::Navigation::Item.new(:test, badge: "123")
 
       expect(item.badge?).to be true
       expect(item.badge.text).to eq("123")