diff --git a/README.md b/README.md index 4466705..ecdec56 100644 --- a/README.md +++ b/README.md @@ -89,14 +89,14 @@ end Render it somewhere. E.g. `app/views/layouts/application.html.erb`: ```erb -<%= Omnibar.render %> +<%= Omnibar.render(self) %> ``` If you have a fully decoupled frontend, use `Omnibar.html_url` instead, fetch the omnibar HTML from there, and inject it. ### Authorization -You can limit access to commands (e.g. search commands). This will not limit access to plain items. +You can limit access to commands (e.g. search commands) and items. #### Option 1: globally limit engine access @@ -119,19 +119,35 @@ MyOmnibar.auth = ->(controller, omnibar:) do end ``` +#### Option 3: Item-level conditionality + +Items and commands can have an `if` proc that is executed in the controller context. If it returns false, the item is not shown and the command is not executed. + +```ruby +MyOmnibar.add_item( + title: 'Admin link', + url: admin_url, + if: ->{ current_user.admin? } +) +``` + +For this to work, the controller context must be given to the omnibar when rendering (e.g. by passing `self` in a view): + +```erb +<%= Omnibar.render(self) %> +``` + ### Using multiple different omnibars ```ruby -BaseOmnibar = Class.new(RailsOmnibar) -BaseOmnibar.configure do |c| +BaseOmnibar = Class.new(RailsOmnibar).configure do |c| c.add_item( title: 'Log in', url: Rails.application.routes.url_helpers.log_in_url ) end -UserOmnibar = Class.new(RailsOmnibar) -UserOmnibar.configure do |c| +UserOmnibar = Class.new(RailsOmnibar).configure do |c| c.auth = ->{ user_signed_in? } c.add_item( title: 'Log out', @@ -143,7 +159,15 @@ end Then, in some layout: ```erb -<%= (user_signed_in? ? UserOmnibar : BaseOmnibar).render %> +<%= (user_signed_in? ? UserOmnibar : BaseOmnibar).render(self) %> +``` + +Omnibars can also inherit commands, configuration, and items from existing omnibars: + +```ruby +SuperuserOmnibar = Class.new(UserOmnibar).configure do |c| + # add more items that only superusers should get +end ``` ### Other options and usage patterns @@ -172,7 +196,7 @@ end ```ruby module AddOmnibar def build_page(...) - within(super) { text_node(MyOmnibar.render) } + within(super) { text_node(MyOmnibar.render(self)) } end end ActiveAdmin::Views::Pages::Base.prepend(AddOmnibar) @@ -180,7 +204,7 @@ ActiveAdmin::Views::Pages::Base.prepend(AddOmnibar) ##### To render in RailsAdmin -Add `MyOmnibar.render` to `app/views/layouts/rails_admin/application.*`. +Add `MyOmnibar.render(self)` to `app/views/layouts/rails_admin/application.*`. #### Adding all index routes as searchable items diff --git a/app/controllers/rails_omnibar/html_controller.rb b/app/controllers/rails_omnibar/html_controller.rb index 87e622c..46bf9d7 100644 --- a/app/controllers/rails_omnibar/html_controller.rb +++ b/app/controllers/rails_omnibar/html_controller.rb @@ -1,5 +1,5 @@ class RailsOmnibar::HtmlController < RailsOmnibar::BaseController def show - render html: omnibar.render + render html: omnibar.render(self) end end diff --git a/bin/console b/bin/console index fe6187e..72848fd 100755 --- a/bin/console +++ b/bin/console @@ -2,6 +2,7 @@ require 'bundler/setup' require 'rails_omnibar' +require_relative '../spec/dummy/config/environment' require 'irb' IRB.start(__FILE__) diff --git a/lib/rails_omnibar/command/base.rb b/lib/rails_omnibar/command/base.rb index 7781452..8cb1bf6 100644 --- a/lib/rails_omnibar/command/base.rb +++ b/lib/rails_omnibar/command/base.rb @@ -1,13 +1,14 @@ class RailsOmnibar module Command class Base - attr_reader :pattern, :resolver, :description, :example + attr_reader :pattern, :resolver, :description, :example, :if - def initialize(pattern:, resolver:, description: nil, example: nil) + def initialize(pattern:, resolver:, description: nil, example: nil, if: nil) @pattern = cast_to_pattern(pattern) @resolver = RailsOmnibar.cast_to_proc(resolver) @description = description @example = example + @if = RailsOmnibar.cast_to_condition(binding.local_variable_get(:if)) end def call(input, controller:, omnibar:) @@ -19,6 +20,12 @@ def call(input, controller:, omnibar:) results.map { |e| RailsOmnibar.cast_to_item(e) } end + def handle?(input, controller:, omnibar:) + return false unless pattern.match?(input) + + RailsOmnibar.evaluate_condition(self.if, context: controller, omnibar: omnibar) + end + private def cast_to_pattern(arg) diff --git a/lib/rails_omnibar/command/search.rb b/lib/rails_omnibar/command/search.rb index 48d160d..c90ad44 100644 --- a/lib/rails_omnibar/command/search.rb +++ b/lib/rails_omnibar/command/search.rb @@ -29,7 +29,7 @@ def initialize(finder:, itemizer:, **kwargs) # ActiveRecord-specific search. class RecordSearch < Search - def initialize(model:, columns: :id, pattern: nil, finder: nil, itemizer: nil, example: nil) + def initialize(model:, columns: :id, pattern: nil, finder: nil, itemizer: nil, example: nil, if: nil) # casting and validations model = model.to_s.classify.constantize unless model.is_a?(Class) model < ActiveRecord::Base || raise(ArgumentError, 'model: must be a model') @@ -64,6 +64,7 @@ def initialize(model:, columns: :id, pattern: nil, finder: nil, itemizer: nil, e pattern: pattern, finder: finder, itemizer: itemizer, + if: binding.local_variable_get(:if), ) end end diff --git a/lib/rails_omnibar/commands.rb b/lib/rails_omnibar/commands.rb index 0f3a658..1f014f0 100644 --- a/lib/rails_omnibar/commands.rb +++ b/lib/rails_omnibar/commands.rb @@ -1,17 +1,15 @@ class RailsOmnibar def handle(input, controller) - handler = commands.find { |h| h.pattern.match?(input) } + handler = commands.find do |cmd| + cmd.handle?(input, controller: controller, omnibar: self) + end handler&.call(input, controller: controller, omnibar: self) || [] end - def command_pattern - commands.any? ? Regexp.union(commands.map(&:pattern)) : /$NO_COMMANDS/ - end - def add_command(command) - check_const_and_clear_cache commands << RailsOmnibar.cast_to_command(command) - self + clear_command_pattern_cache + self.class end def self.cast_to_command(arg) @@ -43,6 +41,17 @@ def self.cast_to_proc(arg) private + def command_pattern + @command_pattern ||= begin + re = commands.any? ? Regexp.union(commands.map(&:pattern)) : /$NO_COMMANDS/ + JsRegex.new!(re, target: 'ES2018') + end + end + + def clear_command_pattern_cache + @command_pattern = nil + end + def commands @commands ||= [] end diff --git a/lib/rails_omnibar/conditions.rb b/lib/rails_omnibar/conditions.rb new file mode 100644 index 0000000..c46abb1 --- /dev/null +++ b/lib/rails_omnibar/conditions.rb @@ -0,0 +1,27 @@ +class RailsOmnibar + def self.cast_to_condition(arg) + case arg + when nil, true, false then arg + else + arg.try(:arity) == 0 ? arg : RailsOmnibar.cast_to_proc(arg) + end + end + + def self.evaluate_condition(condition, context:, omnibar:) + case condition + when nil, true then true + when false then false + else + context || raise(<<~EOS) + Missing context for condition, please render the omnibar with `.render(self)` + EOS + if condition.try(:arity) == 0 + context.instance_exec(&condition) + elsif condition.respond_to?(:call) + condition.call(context, controller: context, omnibar: omnibar) + else + raise("unsupported condition type: #{condition.class}") + end + end + end +end diff --git a/lib/rails_omnibar/config.rb b/lib/rails_omnibar/config.rb index 805afd8..746585c 100644 --- a/lib/rails_omnibar/config.rb +++ b/lib/rails_omnibar/config.rb @@ -1,12 +1,14 @@ class RailsOmnibar def configure(&block) - check_const_and_clear_cache tap(&block) + self.class end - attr_reader :auth def auth=(arg) - @auth = arg.try(:arity) == 0 ? arg : RailsOmnibar.cast_to_proc(arg) + config[:auth] = arg.try(:arity) == 0 ? arg : RailsOmnibar.cast_to_proc(arg) + end + def auth + config[:auth] end def authorize(controller) if auth.nil? @@ -20,33 +22,39 @@ def authorize(controller) def max_results=(arg) arg.is_a?(Integer) && arg > 0 || raise(ArgumentError, 'max_results must be > 0') - @max_results = arg + config[:max_results] = arg end def max_results - @max_results || 10 + config[:max_results] || 10 end - attr_writer :modal + def modal=(arg) + config[:modal] = arg + end def modal? - instance_variable_defined?(:@modal) ? !!@modal : false + config.key?(:modal) ? !!config[:modal] : false end - attr_writer :calculator + def calculator=(arg) + config[:calculator] = arg + end def calculator? - instance_variable_defined?(:@calculator) ? !!@calculator : true + config.key?(:calculator) ? !!config[:calculator] : true end - def hotkey - @hotkey || 'k' - end def hotkey=(arg) arg.to_s.size == 1 || raise(ArgumentError, 'hotkey must have length 1') - @hotkey = arg.to_s.downcase + config[:hotkey] = arg.to_s.downcase + end + def hotkey + config[:hotkey] || 'k' end - attr_writer :placeholder + def placeholder=(arg) + config[:placeholder] = arg + end def placeholder - return @placeholder.presence unless @placeholder.nil? + return config[:placeholder].presence unless config[:placeholder].nil? help_item = items.find { |i| i.type == :help } help_item && "Hint: Type `#{help_item.title}` for help" @@ -54,13 +62,15 @@ def placeholder private + def config + @config ||= {} + end + def omnibar_class self.class.name || raise(<<~EOS) - RailsOmnibar subclasses must be assigned to constants - before configuring or rendering them. E.g.: + RailsOmnibar subclasses must be assigned to constants, e.g.: - Foo = Class.new(RailsOmnibar) - Foo.configure { ... } + Foo = Class.new(RailsOmnibar).configure { ... } EOS end end diff --git a/lib/rails_omnibar/inheritance.rb b/lib/rails_omnibar/inheritance.rb new file mode 100644 index 0000000..510f90d --- /dev/null +++ b/lib/rails_omnibar/inheritance.rb @@ -0,0 +1,9 @@ +class RailsOmnibar + def self.inherited(subclass) + bar1 = singleton + bar2 = subclass.send(:singleton) + %i[@commands @config @items].each do |ivar| + bar2.instance_variable_set(ivar, bar1.instance_variable_get(ivar).dup) + end + end +end diff --git a/lib/rails_omnibar/item/base.rb b/lib/rails_omnibar/item/base.rb index bab83e1..adf220e 100644 --- a/lib/rails_omnibar/item/base.rb +++ b/lib/rails_omnibar/item/base.rb @@ -1,9 +1,9 @@ class RailsOmnibar module Item class Base - attr_reader :title, :url, :icon, :modal_html, :suggested, :type + attr_reader :title, :url, :icon, :modal_html, :suggested, :type, :if - def initialize(title:, url: nil, icon: nil, modal_html: nil, suggested: false, type: :default) + def initialize(title:, url: nil, icon: nil, modal_html: nil, suggested: false, type: :default, if: nil) url.present? && modal_html.present? && raise(ArgumentError, 'use EITHER url: OR modal_html:') @title = validate_title(title) @@ -12,10 +12,16 @@ def initialize(title:, url: nil, icon: nil, modal_html: nil, suggested: false, t @modal_html = modal_html @suggested = !!suggested @type = type + @if = RailsOmnibar.cast_to_condition(binding.local_variable_get(:if)) end def as_json(*) - { title: title, url: url, icon: icon, modalHTML: modal_html, suggested: suggested, type: type } + @as_json ||= + { title: title, url: url, icon: icon, modalHTML: modal_html, suggested: suggested, type: type } + end + + def render?(context:, omnibar:) + RailsOmnibar.evaluate_condition(self.if, context: context, omnibar: omnibar) end private diff --git a/lib/rails_omnibar/items.rb b/lib/rails_omnibar/items.rb index f939a39..aac8b78 100644 --- a/lib/rails_omnibar/items.rb +++ b/lib/rails_omnibar/items.rb @@ -1,13 +1,12 @@ class RailsOmnibar def add_item(item) - check_const_and_clear_cache items << RailsOmnibar.cast_to_item(item) - self + self.class end def add_items(*args) args.each { |arg| add_item(arg) } - self + self.class end def self.cast_to_item(arg) diff --git a/lib/rails_omnibar/rendering.rb b/lib/rails_omnibar/rendering.rb index f4054bd..74b1190 100644 --- a/lib/rails_omnibar/rendering.rb +++ b/lib/rails_omnibar/rendering.rb @@ -1,6 +1,7 @@ class RailsOmnibar - def render - @cached_html ||= <<~HTML.html_safe + def render(context = nil) + @context = context + <<~HTML.html_safe
@@ -17,9 +18,9 @@ def html_url def as_json(*) { calculator: calculator?, - commandPattern: JsRegex.new!(command_pattern, target: 'ES2018'), + commandPattern: command_pattern, hotkey: hotkey, - items: items, + items: items.select { |i| i.render?(context: @context, omnibar: self) }, maxResults: max_results, modal: modal?, placeholder: placeholder, @@ -30,11 +31,4 @@ def as_json(*) def urls @urls ||= RailsOmnibar::Engine.routes.url_helpers end - - private - - def check_const_and_clear_cache - omnibar_class # trigger constant assignment check - @cached_html = nil - end end diff --git a/spec/benchmarks/benchmark.rb b/spec/benchmarks/benchmark.rb new file mode 100644 index 0000000..c8460f6 --- /dev/null +++ b/spec/benchmarks/benchmark.rb @@ -0,0 +1,16 @@ +require_relative '../rails_helper' +require 'bundler/inline' + +gemfile do + source 'https://rubygems.org' + gem 'benchmark-ips', require: 'benchmark/ips' +end + +BenchmarkBar = Class.new(RailsOmnibar).configure do |c| + 100.times { c.add_item(title: rand.to_s, url: rand.to_s) } + 10.times { c.add_command(description: rand.to_s, pattern: /#{rand}/, example: rand.to_s, resolver: ->(_){}) } +end + +Benchmark.ips do |x| + x.report('render') { BenchmarkBar.render } # ~3k ips +end diff --git a/spec/lib/rails_omnibar/config_spec.rb b/spec/lib/rails_omnibar/config_spec.rb index bd13b7f..d1f06b8 100644 --- a/spec/lib/rails_omnibar/config_spec.rb +++ b/spec/lib/rails_omnibar/config_spec.rb @@ -53,10 +53,8 @@ expect(subject.placeholder).to eq nil end - it 'raises when trying to configure or render from an anonymous class' do + it 'raises when trying to render from an anonymous class' do klass = Class.new(RailsOmnibar) - expect { klass.configure {} }.to raise_error(/constant/) - expect { klass.add_item(title: 'a', url: 'b') }.to raise_error(/constant/) expect { klass.render }.to raise_error(/constant/) end end diff --git a/spec/lib/rails_omnibar/inheritance_spec.rb b/spec/lib/rails_omnibar/inheritance_spec.rb new file mode 100644 index 0000000..c30feeb --- /dev/null +++ b/spec/lib/rails_omnibar/inheritance_spec.rb @@ -0,0 +1,79 @@ +require 'rails_helper' + +describe RailsOmnibar do + it 'inherits commands from a parent class' do + bar1 = Class.new(RailsOmnibar).configure do |c| + c.add_command(pattern: /foo1/, resolver: ->(_){}) + end + bar2 = Class.new(bar1).configure do |c| + c.add_command(pattern: /foo2/, resolver: ->(_){}) + end + bar3 = Class.new(bar2).configure do |c| + c.add_command(pattern: /foo3/, resolver: ->(_){}) + end + + commands = ->(bar) do + bar.send(:singleton).send(:commands).map { |c| c.pattern.source } + end + + expect(commands.call(bar1)).to eq ['foo1'] + expect(commands.call(bar2)).to eq ['foo1', 'foo2'] + expect(commands.call(bar3)).to eq ['foo1', 'foo2', 'foo3'] + + # later changes to commands should not affect child classes + bar1.add_command(pattern: /foo1B/, resolver: ->(_){}) + expect(commands.call(bar1)).to eq ['foo1', 'foo1B'] + expect(commands.call(bar2)).to eq ['foo1', 'foo2'] + expect(commands.call(bar3)).to eq ['foo1', 'foo2', 'foo3'] + end + + it 'inherits configuration from a parent class' do + bar1 = Class.new(RailsOmnibar).configure do |c| + c.max_results = 7 + c.placeholder = 'hi' + c.hotkey = 'j' + end + bar2 = Class.new(bar1).configure do |c| + c.max_results = 8 + end + bar3 = Class.new(bar2).configure do |c| + c.hotkey = 'k' + end + + config = ->(bar) { bar.send(:singleton).send(:config) } + + expect(config.call(bar1)).to eq(max_results: 7, placeholder: 'hi', hotkey: 'j') + expect(config.call(bar2)).to eq(max_results: 8, placeholder: 'hi', hotkey: 'j') + expect(config.call(bar3)).to eq(max_results: 8, placeholder: 'hi', hotkey: 'k') + + # later changes to config should not affect child classes + bar1.placeholder = 'bye' + expect(bar1.placeholder).to eq 'bye' + expect(bar2.placeholder).to eq 'hi' + expect(bar3.placeholder).to eq 'hi' + end + + it 'inherits items from a parent class' do + bar1 = Class.new(RailsOmnibar).configure do |c| + c.add_item(title: 'foo1', url: 'bar1') + end + bar2 = Class.new(bar1).configure do |c| + c.add_item(title: 'foo2', url: 'bar2') + end + bar3 = Class.new(bar2).configure do |c| + c.add_item(title: 'foo3', url: 'bar3') + end + + items = ->(bar) { bar.send(:singleton).send(:items).map(&:title) } + + expect(items.call(bar1)).to eq ['foo1'] + expect(items.call(bar2)).to eq ['foo1', 'foo2'] + expect(items.call(bar3)).to eq ['foo1', 'foo2', 'foo3'] + + # later changes to items should not affect child classes + bar1.add_item(title: 'foo1B', url: 'bar1B') + expect(items.call(bar1)).to eq ['foo1', 'foo1B'] + expect(items.call(bar2)).to eq ['foo1', 'foo2'] + expect(items.call(bar3)).to eq ['foo1', 'foo2', 'foo3'] + end +end diff --git a/spec/system/rails_omnibar_spec.rb b/spec/system/rails_omnibar_spec.rb index 1387061..dfac2fe 100644 --- a/spec/system/rails_omnibar_spec.rb +++ b/spec/system/rails_omnibar_spec.rb @@ -119,6 +119,33 @@ expect(page).not_to have_content 'fake_result_1' end + it 'can have conditional items and commands' do + visit main_app.root_path + send_keys([:control, 'm']) # custom hotkey, c.f. my_omnibar_template.rb + expect(page).to have_selector 'input' + + type('condi') + expect(page).not_to have_content 'conditional item' + + FactoryBot.create(:user) + expect { type('DELETE users'); sleep 0.3 }.not_to change { User.count } + + # now again with truthy condition + ENV['FAKE_OMNIBAR_IF'] = '1' + refresh # reload page + + send_keys([:control, 'm']) # custom hotkey, c.f. my_omnibar_template.rb + expect(page).to have_selector 'input' + + type('condi') + expect(page).to have_content 'conditional item' + + FactoryBot.create(:user) + expect { type('DELETE users'); sleep 0.3 }.to change { User.count }.to(0) + ensure + ENV['FAKE_OMNIBAR_IF'] = nil + end + def type(str) find('input').set(str) end diff --git a/spec/templates/app_template.rb b/spec/templates/app_template.rb index 0d0641b..b66074d 100644 --- a/spec/templates/app_template.rb +++ b/spec/templates/app_template.rb @@ -29,7 +29,7 @@ inject_into_class 'app/controllers/application_controller.rb', 'ApplicationController', <<-RUBY def index - render html: (MyOmnibar.render + OtherOmnibar.render) + render html: (MyOmnibar.render(self) + OtherOmnibar.render(self)) end RUBY diff --git a/spec/templates/my_omnibar_template.rb b/spec/templates/my_omnibar_template.rb index e9dcdb5..b8772e6 100644 --- a/spec/templates/my_omnibar_template.rb +++ b/spec/templates/my_omnibar_template.rb @@ -3,6 +3,7 @@ c.add_item(title: 'important URL', url: 'https://www.disney.com', suggested: true) c.add_item(title: 'boring URL', url: 'https://www.github.com') + c.add_item(title: 'conditional item', url: '#', if: -> { ENV['FAKE_OMNIBAR_IF'] }) c.add_webadmin_items(prefix: 'Admin:') @@ -43,6 +44,13 @@ end, ) + c.add_command( + description: 'Delete users', + pattern: /DELETE (.+)/i, + resolver: ->(value){ { title: value.classify.constantize.delete_all.to_s } }, + if: ->{ ENV['FAKE_OMNIBAR_IF'] }, + ) + c.add_help # Use a hotkey that is the same in most keyboard layouts to work around