From 2ab53138c1670edb8fa2011c1a3ab03738615736 Mon Sep 17 00:00:00 2001 From: Arturo Herrero Date: Fri, 24 Jan 2025 13:12:33 +0100 Subject: [PATCH 1/5] Fix global_search_commits_tab usage in CE --- lib/search/navigation.rb | 4 +--- spec/lib/search/navigation_spec.rb | 19 +++++-------------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/lib/search/navigation.rb b/lib/search/navigation.rb index f7eed0bae14cca..77480c2ff02de7 100644 --- a/lib/search/navigation.rb +++ b/lib/search/navigation.rb @@ -139,9 +139,7 @@ def show_wiki_search_tab? end def show_commits_search_tab? - return true if tab_enabled_for_project?(:commits) - - project.nil? && show_elasticsearch_tabs? && feature_flag_tab_enabled?(:global_search_commits_tab) + tab_enabled_for_project?(:commits) end def show_issues_search_tab? diff --git a/spec/lib/search/navigation_spec.rb b/spec/lib/search/navigation_spec.rb index c20c15077def69..80018b8c992aee 100644 --- a/spec/lib/search/navigation_spec.rb +++ b/spec/lib/search/navigation_spec.rb @@ -160,24 +160,15 @@ end context 'for commits tab' do - where(:feature_flag_enabled, :show_elasticsearch_tabs, :project, :tab_enabled, :condition) do - false | false | nil | true | true - false | false | ref(:project_double) | true | true - false | false | nil | false | false - false | true | ref(:project_double) | false | false - false | true | nil | false | false - true | false | nil | false | false - true | false | ref(:project_double) | false | false - true | true | ref(:project_double) | false | false - true | true | nil | false | true + where(:project, :tab_enabled, :condition) do + nil | true | true + nil | false | false + ref(:project_double) | true | true + ref(:project_double) | false | false end with_them do - let(:options) { { show_elasticsearch_tabs: show_elasticsearch_tabs } } - it 'data item condition is set correctly' do - allow(search_navigation).to receive(:feature_flag_tab_enabled?) - .with(:global_search_commits_tab).and_return(feature_flag_enabled) allow(search_navigation).to receive(:tab_enabled_for_project?).with(:commits).and_return(tab_enabled) expect(tabs[:commits][:condition]).to eq(condition) -- GitLab From 6cea8906e7d6080c3215cf7a8c172e120dd5e679 Mon Sep 17 00:00:00 2001 From: Arturo Herrero Date: Fri, 24 Jan 2025 16:28:14 +0100 Subject: [PATCH 2/5] Show commits search tab if show elasticsearch tabs --- lib/search/navigation.rb | 2 +- spec/lib/search/navigation_spec.rb | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/search/navigation.rb b/lib/search/navigation.rb index 77480c2ff02de7..ad516e7b559253 100644 --- a/lib/search/navigation.rb +++ b/lib/search/navigation.rb @@ -139,7 +139,7 @@ def show_wiki_search_tab? end def show_commits_search_tab? - tab_enabled_for_project?(:commits) + tab_enabled_for_project?(:commits) || show_elasticsearch_tabs? end def show_issues_search_tab? diff --git a/spec/lib/search/navigation_spec.rb b/spec/lib/search/navigation_spec.rb index 80018b8c992aee..ba199fd6532177 100644 --- a/spec/lib/search/navigation_spec.rb +++ b/spec/lib/search/navigation_spec.rb @@ -160,14 +160,18 @@ end context 'for commits tab' do - where(:project, :tab_enabled, :condition) do - nil | true | true - nil | false | false - ref(:project_double) | true | true - ref(:project_double) | false | false + where(:show_elasticsearch_tabs, :project, :tab_enabled, :condition) do + false | nil | true | true + false | nil | false | false + true | nil | false | true + false | ref(:project_double) | true | true + false | ref(:project_double) | false | false + true | ref(:project_double) | false | true end with_them do + let(:options) { { show_elasticsearch_tabs: show_elasticsearch_tabs } } + it 'data item condition is set correctly' do allow(search_navigation).to receive(:tab_enabled_for_project?).with(:commits).and_return(tab_enabled) -- GitLab From 75ee72ee9800a7f32b31089f7fda4c075405e36c Mon Sep 17 00:00:00 2001 From: Arturo Herrero Date: Mon, 27 Jan 2025 12:58:17 +0100 Subject: [PATCH 3/5] Fix global_search_commits_tab usage --- ee/lib/ee/search/navigation.rb | 9 ++++ ee/spec/lib/search/navigation_spec.rb | 72 +++++++++++++++++++++++++++ lib/search/navigation.rb | 2 +- spec/lib/search/navigation_spec.rb | 17 +++---- 4 files changed, 89 insertions(+), 11 deletions(-) diff --git a/ee/lib/ee/search/navigation.rb b/ee/lib/ee/search/navigation.rb index e7d50b4835bb19..52657b3c733e7c 100644 --- a/ee/lib/ee/search/navigation.rb +++ b/ee/lib/ee/search/navigation.rb @@ -83,6 +83,15 @@ def show_epics_search_tab? ::Feature.enabled?(:global_search_epics_tab, user, type: :ops) end + + override :show_commits_search_tab? + def show_commits_search_tab? + return true if super # project search & user can search commits + return false unless show_elasticsearch_tabs? # advanced search enabled + return true if group.present? # group search + + ::Feature.enabled?(:global_search_commits_tab, user, type: :ops) # global search + end end end end diff --git a/ee/spec/lib/search/navigation_spec.rb b/ee/spec/lib/search/navigation_spec.rb index 43606bffec70f4..331fe32467bc1e 100644 --- a/ee/spec/lib/search/navigation_spec.rb +++ b/ee/spec/lib/search/navigation_spec.rb @@ -19,6 +19,78 @@ subject(:tabs) { search_navigation.tabs } + context 'for commits tab' do + context 'when project search' do + let(:project) { project_double } + let(:group) { nil } + + where(:tab_enabled_for_project, :condition) do + true | true + false | false + end + + with_them do + before do + allow(search_navigation).to receive(:tab_enabled_for_project?).and_return(tab_enabled_for_project) + end + + it 'data item condition is set correctly' do + expect(tabs[:commits][:condition]).to eq(condition) + end + end + end + + context 'when group search' do + let(:project) { nil } + let(:group) { group_double } + + where(:feature_flag, :show_elasticsearch_tabs, :condition) do + true | true | true + true | false | false + false | true | true + false | false | false + end + + with_them do + let(:options) { { show_elasticsearch_tabs: show_elasticsearch_tabs } } + + before do + stub_feature_flags(global_search_commits_tab: feature_flag) + end + + it 'data item condition is set correctly' do + expect(tabs[:commits][:condition]).to eq(condition) + end + end + end + + context 'when global search' do + let(:project) { nil } + let(:group) { nil } + + where(:feature_flag, :show_elasticsearch_tabs, :condition) do + true | true | true + false | true | false + false | false | false + true | false | false + false | nil | false + true | nil | false + end + + with_them do + let(:options) { { show_elasticsearch_tabs: show_elasticsearch_tabs } } + + before do + stub_feature_flags(global_search_commits_tab: feature_flag) + end + + it 'data item condition is set correctly' do + expect(tabs[:commits][:condition]).to eq(condition) + end + end + end + end + context 'for epics tab' do context 'when project search' do let(:project) { project_double } diff --git a/lib/search/navigation.rb b/lib/search/navigation.rb index ad516e7b559253..77480c2ff02de7 100644 --- a/lib/search/navigation.rb +++ b/lib/search/navigation.rb @@ -139,7 +139,7 @@ def show_wiki_search_tab? end def show_commits_search_tab? - tab_enabled_for_project?(:commits) || show_elasticsearch_tabs? + tab_enabled_for_project?(:commits) end def show_issues_search_tab? diff --git a/spec/lib/search/navigation_spec.rb b/spec/lib/search/navigation_spec.rb index ba199fd6532177..686965d04479ee 100644 --- a/spec/lib/search/navigation_spec.rb +++ b/spec/lib/search/navigation_spec.rb @@ -56,6 +56,7 @@ before do allow(search_navigation) .to receive_messages(can?: true, tab_enabled_for_project?: false, feature_flag_tab_enabled?: false) + allow(search_navigation).to receive(:tab_enabled_for_project?).and_call_original end subject(:tabs) { search_navigation.tabs } @@ -160,20 +161,16 @@ end context 'for commits tab' do - where(:show_elasticsearch_tabs, :project, :tab_enabled, :condition) do - false | nil | true | true - false | nil | false | false - true | nil | false | true - false | ref(:project_double) | true | true - false | ref(:project_double) | false | false - true | ref(:project_double) | false | true + where(:project, :ability_enabled, :condition) do + nil | true | false + nil | false | false + ref(:project_double) | true | true + ref(:project_double) | false | false end with_them do - let(:options) { { show_elasticsearch_tabs: show_elasticsearch_tabs } } - it 'data item condition is set correctly' do - allow(search_navigation).to receive(:tab_enabled_for_project?).with(:commits).and_return(tab_enabled) + allow(search_navigation).to receive(:can?).with(user, :read_code, project).and_return(ability_enabled) expect(tabs[:commits][:condition]).to eq(condition) end -- GitLab From f87c9de40cb7d3da6353996cfd5b97838adc559d Mon Sep 17 00:00:00 2001 From: Arturo Herrero Date: Tue, 28 Jan 2025 11:28:00 +0100 Subject: [PATCH 4/5] Move feature flag under EE --- .../config}/feature_flags/ops/global_search_commits_tab.yml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {config => ee/config}/feature_flags/ops/global_search_commits_tab.yml (100%) diff --git a/config/feature_flags/ops/global_search_commits_tab.yml b/ee/config/feature_flags/ops/global_search_commits_tab.yml similarity index 100% rename from config/feature_flags/ops/global_search_commits_tab.yml rename to ee/config/feature_flags/ops/global_search_commits_tab.yml -- GitLab From 586187311beb74857aa76f2ab6ec0218642282e3 Mon Sep 17 00:00:00 2001 From: Arturo Herrero Date: Wed, 29 Jan 2025 12:48:25 +0000 Subject: [PATCH 5/5] Improve table formatting --- ee/spec/lib/search/navigation_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/ee/spec/lib/search/navigation_spec.rb b/ee/spec/lib/search/navigation_spec.rb index 331fe32467bc1e..8f3eddb2716e1f 100644 --- a/ee/spec/lib/search/navigation_spec.rb +++ b/ee/spec/lib/search/navigation_spec.rb @@ -25,7 +25,7 @@ let(:group) { nil } where(:tab_enabled_for_project, :condition) do - true | true + true | true false | false end @@ -45,9 +45,9 @@ let(:group) { group_double } where(:feature_flag, :show_elasticsearch_tabs, :condition) do - true | true | true - true | false | false - false | true | true + true | true | true + true | false | false + false | true | true false | false | false end @@ -69,12 +69,12 @@ let(:group) { nil } where(:feature_flag, :show_elasticsearch_tabs, :condition) do - true | true | true - false | true | false + true | true | true + false | true | false false | false | false - true | false | false - false | nil | false - true | nil | false + true | false | false + false | nil | false + true | nil | false end with_them do -- GitLab