Merge branch '46758-fallout-of-cacheable-attribute' into 'master'

Ensure ApplicationSetting#performance_bar_allowed_group_id is properly set when retrieved from cache

Closes #46758

See merge request gitlab-org/gitlab-ce!19144

1 files changed, 87 insertions, 1 deletions

diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb
index fb07ecc6ae8..6337ee7d724 100644
--- a/spec/services/application_settings/update_service_spec.rb
+++ b/spec/services/application_settings/update_service_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe ApplicationSettings::UpdateService do
-  let(:application_settings) { Gitlab::CurrentSettings.current_application_settings }
+  let(:application_settings) { create(:application_setting) }
   let(:admin) { create(:user, :admin) }
   let(:params) { {} }
 
@@ -54,4 +54,90 @@ describe ApplicationSettings::UpdateService do
       end
     end
   end
+
+  describe 'performance bar settings' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:params_performance_bar_enabled,
+      :params_performance_bar_allowed_group_path,
+      :previous_performance_bar_allowed_group_id,
+      :expected_performance_bar_allowed_group_id) do
+      true | '' | nil | nil
+      true | '' | 42_000_000 | nil
+      true | nil | nil | nil
+      true | nil | 42_000_000 | nil
+      true | 'foo' | nil | nil
+      true | 'foo' | 42_000_000 | nil
+      true | 'group_a' | nil | 42_000_000
+      true | 'group_b' | 42_000_000 | 43_000_000
+      true | 'group_a' | 42_000_000 | 42_000_000
+      false | '' | nil | nil
+      false | '' | 42_000_000 | nil
+      false | nil | nil | nil
+      false | nil | 42_000_000 | nil
+      false | 'foo' | nil | nil
+      false | 'foo' | 42_000_000 | nil
+      false | 'group_a' | nil | nil
+      false | 'group_b' | 42_000_000 | nil
+      false | 'group_a' | 42_000_000 | nil
+    end
+
+    with_them do
+      let(:params) do
+        {
+          performance_bar_enabled: params_performance_bar_enabled,
+          performance_bar_allowed_group_path: params_performance_bar_allowed_group_path
+        }
+      end
+
+      before do
+        if previous_performance_bar_allowed_group_id == 42_000_000 || params_performance_bar_allowed_group_path == 'group_a'
+          create(:group, id: 42_000_000, path: 'group_a')
+        end
+
+        if expected_performance_bar_allowed_group_id == 43_000_000 || params_performance_bar_allowed_group_path == 'group_b'
+          create(:group, id: 43_000_000, path: 'group_b')
+        end
+
+        application_settings.update!(performance_bar_allowed_group_id: previous_performance_bar_allowed_group_id)
+      end
+
+      it 'sets performance_bar_allowed_group_id when present and performance_bar_enabled == true' do
+        expect(application_settings.performance_bar_allowed_group_id).to eq(previous_performance_bar_allowed_group_id)
+
+        if previous_performance_bar_allowed_group_id != expected_performance_bar_allowed_group_id
+          expect { subject.execute }
+            .to change(application_settings, :performance_bar_allowed_group_id)
+            .from(previous_performance_bar_allowed_group_id).to(expected_performance_bar_allowed_group_id)
+        else
+          expect { subject.execute }
+            .not_to change(application_settings, :performance_bar_allowed_group_id)
+        end
+      end
+    end
+
+    context 'when :performance_bar_allowed_group_path is not present' do
+      let(:group) { create(:group) }
+
+      before do
+        application_settings.update!(performance_bar_allowed_group_id: group.id)
+      end
+
+      it 'does not change the performance bar settings' do
+        expect { subject.execute }
+          .not_to change(application_settings, :performance_bar_allowed_group_id)
+      end
+    end
+
+    context 'when :performance_bar_enabled is not present' do
+      let(:group) { create(:group) }
+      let(:params) { { performance_bar_allowed_group_path: group.full_path } }
+
+      it 'implicitely defaults to true' do
+        expect { subject.execute }
+          .to change(application_settings, :performance_bar_allowed_group_id)
+          .from(nil).to(group.id)
+      end
+    end
+  end
 end