From 11f70e0f08e9ca9383fef5ca8acc377fbbb3c279 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 18 Sep 2018 16:12:37 +0000 Subject: Improve TabHelper to clarify the use of Namespaces for the nav_link --- app/helpers/application_helper.rb | 7 ++- app/helpers/tab_helper.rb | 16 ++++++- spec/helpers/application_helper_spec.rb | 63 +++++++++++++++++---------- spec/helpers/tab_helper_spec.rb | 76 +++++++++++++++++++++++++-------- 4 files changed, 120 insertions(+), 42 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index bb401b03709..ef4560023af 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -13,7 +13,7 @@ module ApplicationHelper # Check if a particular controller is the current one # - # args - One or more controller names to check + # args - One or more controller names to check (using path notation when inside namespaces) # # Examples # @@ -21,6 +21,11 @@ module ApplicationHelper # current_controller?(:tree) # => true # current_controller?(:commits) # => false # current_controller?(:commits, :tree) # => true + # + # # On Admin::ApplicationController + # current_controller?(:application) # => true + # current_controller?('admin/application') # => true + # current_controller?('gitlab/application') # => false def current_controller?(*args) args.any? do |v| v.to_s.downcase == controller.controller_name || v.to_s.downcase == controller.controller_path diff --git a/app/helpers/tab_helper.rb b/app/helpers/tab_helper.rb index e310fda51d7..d91f0f78db7 100644 --- a/app/helpers/tab_helper.rb +++ b/app/helpers/tab_helper.rb @@ -8,7 +8,7 @@ module TabHelper # element is the value passed to the block. # # options - The options hash used to determine if the element is "active" (default: {}) - # :controller - One or more controller names to check (optional). + # :controller - One or more controller names to check, use path notation when namespaced (optional). # :action - One or more action names to check (optional). # :path - A shorthand path, such as 'dashboard#index', to check (optional). # :html_options - Extra options to be passed to the list element (optional). @@ -42,6 +42,20 @@ module TabHelper # nav_link(controller: :tree, html_options: {class: 'home'}) { "Hello" } # # => '
  • Hello
    • ' # + # # For namespaced controllers like Admin::AppearancesController#show + # + # # Controller and namespace matches + # nav_link(controller: 'admin/appearances') { "Hello" } + # # => '
  • Hello
    • ' + # + # # Controller and namespace matches but action doesn't + # nav_link(controller: 'admin/appearances', action: :edit) { "Hello" } + # # => '
  • Hello
    • ' + # + # # Shorthand path with namespace + # nav_link(path: 'admin/appearances#show') { "Hello"} + # # => '
  • Hello
    • ' + # # Returns a list item element String def nav_link(options = {}, &block) klass = active_nav_link?(options) ? 'active' : '' diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 14297a1a544..1238cfbd1e7 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -3,48 +3,66 @@ require 'spec_helper' describe ApplicationHelper do describe 'current_controller?' do - it 'returns true when controller matches argument' do + before do stub_controller_name('foo') + end - expect(helper.current_controller?(:foo)).to eq true + it 'returns true when controller matches argument' do + expect(helper.current_controller?(:foo)).to be_truthy end it 'returns false when controller does not match argument' do - stub_controller_name('foo') - - expect(helper.current_controller?(:bar)).to eq false + expect(helper.current_controller?(:bar)).to be_falsey end it 'takes any number of arguments' do - stub_controller_name('foo') + expect(helper.current_controller?(:baz, :bar)).to be_falsey + expect(helper.current_controller?(:baz, :bar, :foo)).to be_truthy + end + + context 'when namespaced' do + before do + stub_controller_path('bar/foo') + end + + it 'returns true when controller matches argument' do + expect(helper.current_controller?(:foo)).to be_truthy + end - expect(helper.current_controller?(:baz, :bar)).to eq false - expect(helper.current_controller?(:baz, :bar, :foo)).to eq true + it 'returns true when controller and namespace matches argument in path notation' do + expect(helper.current_controller?('bar/foo')).to be_truthy + end + + it 'returns false when namespace doesnt match' do + expect(helper.current_controller?('foo/foo')).to be_falsey + end end def stub_controller_name(value) allow(helper.controller).to receive(:controller_name).and_return(value) end + + def stub_controller_path(value) + allow(helper.controller).to receive(:controller_path).and_return(value) + end end describe 'current_action?' do - it 'returns true when action matches' do + before do stub_action_name('foo') + end - expect(helper.current_action?(:foo)).to eq true + it 'returns true when action matches' do + expect(helper.current_action?(:foo)).to be_truthy end it 'returns false when action does not match' do - stub_action_name('foo') - - expect(helper.current_action?(:bar)).to eq false + expect(helper.current_action?(:bar)).to be_falsey end it 'takes any number of arguments' do - stub_action_name('foo') - - expect(helper.current_action?(:baz, :bar)).to eq false - expect(helper.current_action?(:baz, :bar, :foo)).to eq true + expect(helper.current_action?(:baz, :bar)).to be_falsey + expect(helper.current_action?(:baz, :bar, :foo)).to be_truthy end def stub_action_name(value) @@ -100,8 +118,7 @@ describe ApplicationHelper do end it 'accepts a custom html_class' do - expect(element(html_class: 'custom_class').attr('class')) - .to eq 'js-timeago custom_class' + expect(element(html_class: 'custom_class').attr('class')).to eq 'js-timeago custom_class' end it 'accepts a custom tooltip placement' do @@ -114,6 +131,7 @@ describe ApplicationHelper do it 'add class for the short format' do timeago_element = element(short_format: 'short') + expect(timeago_element.attr('class')).to eq 'js-short-timeago' expect(timeago_element.next_element).to eq nil end @@ -128,11 +146,9 @@ describe ApplicationHelper do context 'when alternate support url is specified' do let(:alternate_url) { 'http://company.example.com/getting-help' } - before do + it 'returns the alternate support url' do stub_application_setting(help_page_support_url: alternate_url) - end - it 'returns the alternate support url' do expect(helper.support_url).to eq(alternate_url) end end @@ -155,9 +171,12 @@ describe ApplicationHelper do describe '#autocomplete_data_sources' do let(:project) { create(:project) } let(:noteable_type) { Issue } + it 'returns paths for autocomplete_sources_controller' do sources = helper.autocomplete_data_sources(project, noteable_type) + expect(sources.keys).to match_array([:members, :issues, :mergeRequests, :labels, :milestones, :commands]) + sources.keys.each do |key| expect(sources[key]).not_to be_nil end diff --git a/spec/helpers/tab_helper_spec.rb b/spec/helpers/tab_helper_spec.rb index b473c0a7416..9abf63d4bd4 100644 --- a/spec/helpers/tab_helper_spec.rb +++ b/spec/helpers/tab_helper_spec.rb @@ -9,31 +9,71 @@ describe TabHelper do allow(self).to receive(:action_name).and_return('foo') end - it "captures block output" do - expect(nav_link { "Testing Blocks" }).to match(/Testing Blocks/) + context 'with the content of the li' do + it "captures block output" do + expect(nav_link { "Testing Blocks" }).to match(/Testing Blocks/) + end end - it "performs checks on the current controller" do - expect(nav_link(controller: :foo)).to match(/
  • /) - expect(nav_link(controller: :bar)).not_to match(/active/) - expect(nav_link(controller: [:foo, :bar])).to match(/active/) - end + context 'with controller param' do + it "performs checks on the current controller" do + expect(nav_link(controller: :foo)).to match(/
  • /) + expect(nav_link(controller: :bar)).not_to match(/active/) + expect(nav_link(controller: [:foo, :bar])).to match(/active/) + end + + context 'with action param' do + it "performs checks on both controller and action when both are present" do + expect(nav_link(controller: :bar, action: :foo)).not_to match(/active/) + expect(nav_link(controller: :foo, action: :bar)).not_to match(/active/) + expect(nav_link(controller: :foo, action: :foo)).to match(/active/) + end + end + + context 'with namespace in path notation' do + before do + allow(controller).to receive(:controller_path).and_return('bar/foo') + end - it "performs checks on the current action" do - expect(nav_link(action: :foo)).to match(/
  • /) - expect(nav_link(action: :bar)).not_to match(/active/) - expect(nav_link(action: [:foo, :bar])).to match(/active/) + it 'performs checks on both controller and namespace' do + expect(nav_link(controller: 'foo/foo')).not_to match(/active/) + expect(nav_link(controller: 'bar/foo')).to match(/active/) + end + + context 'with action param' do + it "performs checks on both namespace, controller and action when they are all present" do + expect(nav_link(controller: 'foo/foo', action: :foo)).not_to match(/active/) + expect(nav_link(controller: 'bar/foo', action: :bar)).not_to match(/active/) + expect(nav_link(controller: 'bar/foo', action: :foo)).to match(/active/) + end + end + end end - it "performs checks on both controller and action when both are present" do - expect(nav_link(controller: :bar, action: :foo)).not_to match(/active/) - expect(nav_link(controller: :foo, action: :bar)).not_to match(/active/) - expect(nav_link(controller: :foo, action: :foo)).to match(/active/) + context 'with action param' do + it "performs checks on the current action" do + expect(nav_link(action: :foo)).to match(/
  • /) + expect(nav_link(action: :bar)).not_to match(/active/) + expect(nav_link(action: [:foo, :bar])).to match(/active/) + end end - it "accepts a path shorthand" do - expect(nav_link(path: 'foo#bar')).not_to match(/active/) - expect(nav_link(path: 'foo#foo')).to match(/active/) + context 'with path param' do + it "accepts a path shorthand" do + expect(nav_link(path: 'foo#bar')).not_to match(/active/) + expect(nav_link(path: 'foo#foo')).to match(/active/) + end + + context 'with namespace' do + before do + allow(controller).to receive(:controller_path).and_return('bar/foo') + end + + it 'accepts a path shorthand with namespace' do + expect(nav_link(path: 'bar/foo#foo')).to match(/active/) + expect(nav_link(path: 'foo/foo#foo')).not_to match(/active/) + end + end end it "passes extra html options to the list element" do -- cgit v1.2.1