From 4c04ae785dab7b906ba0cea06ab8c559ff7db2cf Mon Sep 17 00:00:00 2001 From: Sam Pohlenz Date: Fri, 30 Aug 2024 21:51:00 +0930 Subject: [PATCH] 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 | 12 +++-- lib/trestle/navigation/item.rb | 23 ++++++++-- spec/trestle/navigation/block_spec.rb | 13 +++++- spec/trestle/navigation/item_spec.rb | 65 +++++++++++++++++++++------ 4 files changed, 88 insertions(+), 25 deletions(-) diff --git a/lib/trestle/navigation/block.rb b/lib/trestle/navigation/block.rb index 0923dacf..8d19695c 100644 --- a/lib/trestle/navigation/block.rb +++ b/lib/trestle/navigation/block.rb @@ -17,6 +17,8 @@ def items(context) class Evaluator include EvaluationContext + delegate :path, to: :@admin + attr_reader :items def initialize(admin=nil, context=nil) @@ -24,10 +26,6 @@ def initialize(admin=nil, context=nil) @items = [] end - def default_path - @admin ? @admin.path : nil - end - def item(name, path=nil, **options) if options[:group] group = Group.new(options[:group]) @@ -35,10 +33,10 @@ def item(name, path=nil, **options) 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) diff --git a/lib/trestle/navigation/item.rb b/lib/trestle/navigation/item.rb index f1681341..1f622eeb 100644 --- a/lib/trestle/navigation/item.rb +++ b/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) diff --git a/spec/trestle/navigation/block_spec.rb b/spec/trestle/navigation/block_spec.rb index 13676af8..8276c41a 100644 --- a/spec/trestle/navigation/block_spec.rb +++ b/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 diff --git a/spec/trestle/navigation/item_spec.rb b/spec/trestle/navigation/item_spec.rb index d0b58e17..07c5c870 100644 --- a/spec/trestle/navigation/item_spec.rb +++ b/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,23 +104,23 @@ 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") @@ -91,7 +128,7 @@ 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")