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

13 files changed, 183 insertions, 168 deletions

diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index b948e431882..adc423af9e1 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -204,7 +204,7 @@ module ApplicationSettingsHelper
       :pages_domain_verification_enabled,
       :password_authentication_enabled_for_web,
       :password_authentication_enabled_for_git,
-      :performance_bar_allowed_group_id,
+      :performance_bar_allowed_group_path,
       :performance_bar_enabled,
       :plantuml_enabled,
       :plantuml_url,
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index e8ccb320fae..b12f7a2c83f 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -230,6 +230,7 @@ class ApplicationSetting < ActiveRecord::Base
   after_commit do
     reset_memoized_terms
   end
+  after_commit :expire_performance_bar_allowed_user_ids_cache, if: -> { previous_changes.key?('performance_bar_allowed_group_id') }
 
   def self.defaults
     {
@@ -386,31 +387,6 @@ class ApplicationSetting < ActiveRecord::Base
     super(levels.map { |level| Gitlab::VisibilityLevel.level_value(level) })
   end
 
-  def performance_bar_allowed_group_id=(group_full_path)
-    group_full_path = nil if group_full_path.blank?
-
-    if group_full_path.nil?
-      if group_full_path != performance_bar_allowed_group_id
-        super(group_full_path)
-        Gitlab::PerformanceBar.expire_allowed_user_ids_cache
-      end
-
-      return
-    end
-
-    group = Group.find_by_full_path(group_full_path)
-
-    if group
-      if group.id != performance_bar_allowed_group_id
-        super(group.id)
-        Gitlab::PerformanceBar.expire_allowed_user_ids_cache
-      end
-    else
-      super(nil)
-      Gitlab::PerformanceBar.expire_allowed_user_ids_cache
-    end
-  end
-
   def performance_bar_allowed_group
     Group.find_by_id(performance_bar_allowed_group_id)
   end
@@ -420,15 +396,6 @@ class ApplicationSetting < ActiveRecord::Base
     performance_bar_allowed_group_id.present?
   end
 
-  # - If `enable` is true, we early return since the actual attribute that holds
-  #   the enabling/disabling is `performance_bar_allowed_group_id`
-  # - If `enable` is false, we set `performance_bar_allowed_group_id` to `nil`
-  def performance_bar_enabled=(enable)
-    return if Gitlab::Utils.to_boolean(enable)
-
-    self.performance_bar_allowed_group_id = nil
-  end
-
   # Choose one of the available repository storage options. Currently all have
   # equal weighting.
   def pick_repository_storage
@@ -506,4 +473,8 @@ class ApplicationSetting < ActiveRecord::Base
 
     errors.add(:terms, "You need to set terms to be enforced") unless terms.present?
   end
+
+  def expire_performance_bar_allowed_user_ids_cache
+    Gitlab::PerformanceBar.expire_allowed_user_ids_cache
+  end
 end
diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb
index d6d3a661dab..e70445cfb67 100644
--- a/app/services/application_settings/update_service.rb
+++ b/app/services/application_settings/update_service.rb
@@ -3,6 +3,10 @@ module ApplicationSettings
     def execute
       update_terms(@params.delete(:terms))
 
+      if params.key?(:performance_bar_allowed_group_path)
+        params[:performance_bar_allowed_group_id] = performance_bar_allowed_group_id
+      end
+
       @application_setting.update(@params)
     end
 
@@ -18,5 +22,13 @@ module ApplicationSettings
       ApplicationSetting::Term.create(terms: terms)
       @application_setting.reset_memoized_terms
     end
+
+    def performance_bar_allowed_group_id
+      performance_bar_enabled = !params.key?(:performance_bar_enabled) || params.delete(:performance_bar_enabled)
+      group_full_path = params.delete(:performance_bar_allowed_group_path)
+      return nil unless Gitlab::Utils.to_boolean(performance_bar_enabled)
+
+      Group.find_by_full_path(group_full_path)&.id if group_full_path.present?
+    end
   end
 end
diff --git a/app/views/admin/application_settings/_performance_bar.html.haml b/app/views/admin/application_settings/_performance_bar.html.haml
index 8001b42e2f9..030e8610b47 100644
--- a/app/views/admin/application_settings/_performance_bar.html.haml
+++ b/app/views/admin/application_settings/_performance_bar.html.haml
@@ -9,8 +9,8 @@
             = f.check_box :performance_bar_enabled
             Enable the Performance Bar
     .form-group.row
-      = f.label :performance_bar_allowed_group_id, 'Allowed group', class: 'col-form-label col-sm-2'
+      = f.label :performance_bar_allowed_group_path, 'Allowed group', class: 'col-form-label col-sm-2'
       .col-sm-10
-        = f.text_field :performance_bar_allowed_group_id, class: 'form-control', placeholder: 'my-org/my-group', value: @application_setting.performance_bar_allowed_group&.full_path
+        = f.text_field :performance_bar_allowed_group_path, class: 'form-control', placeholder: 'my-org/my-group', value: @application_setting.performance_bar_allowed_group&.full_path
 
   = f.submit 'Save changes', class: "btn btn-success"
diff --git a/doc/api/settings.md b/doc/api/settings.md
index 1ebfe4924b1..36a0782d8f2 100644
--- a/doc/api/settings.md
+++ b/doc/api/settings.md
@@ -55,6 +55,7 @@ Example response:
    "ed25519_key_restriction": 0,
    "enforce_terms": true,
    "terms": "Hello world!",
+   "performance_bar_allowed_group_id": 42
 }
 ```
 
@@ -120,8 +121,9 @@ PUT /application/settings
 | `metrics_timeout`                        | integer          | yes (if `metrics_enabled` is `true`)          | The amount of seconds after which InfluxDB will time out.                                                                                                                                                                                                                                                                                                                                                                                                 |
 | `password_authentication_enabled_for_web` | boolean         | no                                            | Enable authentication for the web interface via a GitLab account password. Default is `true`.                                                                                                                                                                                                                                                                                                                                                                                   |
 | `password_authentication_enabled_for_git` | boolean         | no                                            | Enable authentication for Git over HTTP(S) via a GitLab account password. Default is `true`.                                                                                                                                                                                                                                                                                                                                                                                   |
-| `performance_bar_allowed_group_id`       | string           | no                                            | The group that is allowed to enable the performance bar                                                                                                                                                                                                                                                                                                                                                                                                   |
-| `performance_bar_enabled`                | boolean          | no                                            | Allow enabling the performance bar                                                                                                                                                                                                                                                                                                                                                                                                                        |
+| `performance_bar_allowed_group_path`      | string          | no                                            | Path of the group that is allowed to toggle the performance bar                                                                                                                                                                                                                                                                                                                                                                                                   |
+| `performance_bar_allowed_group_id`       | string           | no                                            | Deprecated: Use `performance_bar_allowed_group_path` instead. Path of the group that is allowed to toggle the performance bar                                                                                                                                                                                                                                                                                                                                                                                                   |
+| `performance_bar_enabled`                | boolean          | no                                            | Deprecated: Pass `performance_bar_allowed_group_path: nil` instead. Allow enabling the performance bar                                                                                                                                                                                                                                                                                                                                                                                                                        |
 | `plantuml_enabled`                       | boolean          | no                                            | Enable PlantUML integration. Default is `false`.                                                                                                                                                                                                                                                                                                                                                                                                          |
 | `plantuml_url`                           | string           | yes (if `plantuml_enabled` is `true`)         | The PlantUML instance URL for integration.                                                                                                                                                                                                                                                                                                                                                                                                                |
 | `polling_interval_multiplier`            | decimal          | no                                            | Interval multiplier used by endpoints that perform polling. Set to 0 to disable polling.                                                                                                                                                                                                                                                                                                                                                                  |
@@ -201,5 +203,6 @@ Example response:
   "ed25519_key_restriction": 0,
   "enforce_terms": true,
   "terms": "Hello world!",
+  "performance_bar_allowed_group_id": 42
 }
 ```
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 3e615f7ac05..3688b44b8df 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -933,8 +933,16 @@ module API
     end
 
     class ApplicationSetting < Grape::Entity
-      expose :id
-      expose(*::ApplicationSettingsHelper.visible_attributes)
+      def self.exposed_attributes
+        attributes = ::ApplicationSettingsHelper.visible_attributes
+        attributes.delete(:performance_bar_allowed_group_path)
+        attributes.delete(:performance_bar_enabled)
+
+        attributes
+      end
+
+      expose :id, :performance_bar_allowed_group_id
+      expose(*exposed_attributes)
       expose(:restricted_visibility_levels) do |setting, _options|
         setting.restricted_visibility_levels.map { |level| Gitlab::VisibilityLevel.string_level(level) }
       end
diff --git a/lib/api/settings.rb b/lib/api/settings.rb
index d727ad59367..4b506127c0e 100644
--- a/lib/api/settings.rb
+++ b/lib/api/settings.rb
@@ -49,6 +49,9 @@ module API
       optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5
       mutually_exclusive :password_authentication_enabled_for_web, :password_authentication_enabled, :signin_enabled
       optional :password_authentication_enabled_for_git, type: Boolean, desc: 'Flag indicating if password authentication is enabled for Git over HTTP(S)'
+      optional :performance_bar_allowed_group_path, type: String, desc: 'Path of the group that is allowed to toggle the performance bar.'
+      optional :performance_bar_allowed_group_id, type: String, desc: 'Depreated: Use :performance_bar_allowed_group_path instead. Path of the group that is allowed to toggle the performance bar.' # support legacy names, can be removed in v6
+      optional :performance_bar_enabled, type: String, desc: 'Deprecated: Pass `performance_bar_allowed_group_path: nil` instead. Allow enabling the performance.' # support legacy names, can be removed in v6
       optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication'
       given require_two_factor_authentication: ->(val) { val } do
         requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication'
@@ -134,12 +137,25 @@ module API
                  desc: "Restrictions on the complexity of uploaded #{type.upcase} keys. A value of #{ApplicationSetting::FORBIDDEN_KEY_VALUE} disables all #{type.upcase} keys."
       end
 
-      optional(*::ApplicationSettingsHelper.visible_attributes)
-      at_least_one_of(*::ApplicationSettingsHelper.visible_attributes)
+      optional_attributes = ::ApplicationSettingsHelper.visible_attributes << :performance_bar_allowed_group_id
+
+      optional(*optional_attributes)
+      at_least_one_of(*optional_attributes)
     end
     put "application/settings" do
       attrs = declared_params(include_missing: false)
 
+      # support legacy names, can be removed in v6
+      if attrs.has_key?(:performance_bar_allowed_group_id)
+        attrs[:performance_bar_allowed_group_path] = attrs.delete(:performance_bar_allowed_group_id)
+      end
+
+      # support legacy names, can be removed in v6
+      if attrs.has_key?(:performance_bar_enabled)
+        performance_bar_enabled = attrs.delete(:performance_bar_allowed_group_id)
+        attrs[:performance_bar_allowed_group_path] = nil unless performance_bar_enabled
+      end
+
       # support legacy names, can be removed in v5
       if attrs.has_key?(:signin_enabled)
         attrs[:password_authentication_enabled_for_web] = attrs.delete(:signin_enabled)
diff --git a/rubocop/cop/line_break_around_conditional_block.rb b/rubocop/cop/line_break_around_conditional_block.rb
index 3e7021e724e..8b6052fee1b 100644
--- a/rubocop/cop/line_break_around_conditional_block.rb
+++ b/rubocop/cop/line_break_around_conditional_block.rb
@@ -95,7 +95,7 @@ module RuboCop
       end
 
       def end_clause_line?(line)
-        line =~ /^\s*(rescue|else|elsif|when)/
+        line =~ /^\s*(#|rescue|else|elsif|when)/
       end
 
       def begin_line?(line)
diff --git a/spec/factories/application_settings.rb b/spec/factories/application_settings.rb
index 3ecc90b6573..00c063c49f8 100644
--- a/spec/factories/application_settings.rb
+++ b/spec/factories/application_settings.rb
@@ -1,4 +1,5 @@
 FactoryBot.define do
   factory :application_setting do
+    default_projects_limit 42
   end
 end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 7e47043a1cb..f8f07205623 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -391,68 +391,6 @@ describe ApplicationSetting do
   end
 
   describe 'performance bar settings' do
-    describe 'performance_bar_allowed_group_id=' do
-      context 'with a blank path' do
-        before do
-          setting.performance_bar_allowed_group_id = create(:group).full_path
-        end
-
-        it 'persists nil for a "" path and clears allowed user IDs cache' do
-          expect(Gitlab::PerformanceBar).to receive(:expire_allowed_user_ids_cache)
-
-          setting.performance_bar_allowed_group_id = ''
-
-          expect(setting.performance_bar_allowed_group_id).to be_nil
-        end
-      end
-
-      context 'with an invalid path' do
-        it 'does not persist an invalid group path' do
-          setting.performance_bar_allowed_group_id = 'foo'
-
-          expect(setting.performance_bar_allowed_group_id).to be_nil
-        end
-      end
-
-      context 'with a path to an existing group' do
-        let(:group) { create(:group) }
-
-        it 'persists a valid group path and clears allowed user IDs cache' do
-          expect(Gitlab::PerformanceBar).to receive(:expire_allowed_user_ids_cache)
-
-          setting.performance_bar_allowed_group_id = group.full_path
-
-          expect(setting.performance_bar_allowed_group_id).to eq(group.id)
-        end
-
-        context 'when the given path is the same' do
-          context 'with a blank path' do
-            before do
-              setting.performance_bar_allowed_group_id = nil
-            end
-
-            it 'clears the cached allowed user IDs' do
-              expect(Gitlab::PerformanceBar).not_to receive(:expire_allowed_user_ids_cache)
-
-              setting.performance_bar_allowed_group_id = ''
-            end
-          end
-
-          context 'with a valid path' do
-            before do
-              setting.performance_bar_allowed_group_id = group.full_path
-            end
-
-            it 'clears the cached allowed user IDs' do
-              expect(Gitlab::PerformanceBar).not_to receive(:expire_allowed_user_ids_cache)
-
-              setting.performance_bar_allowed_group_id = group.full_path
-            end
-          end
-        end
-      end
-    end
-
     describe 'performance_bar_allowed_group' do
       context 'with no performance_bar_allowed_group_id saved' do
         it 'returns nil' do
@@ -464,11 +402,11 @@ describe ApplicationSetting do
         let(:group) { create(:group) }
 
         before do
-          setting.performance_bar_allowed_group_id = group.full_path
+          setting.update!(performance_bar_allowed_group_id: group.id)
         end
 
         it 'returns the group' do
-          expect(setting.performance_bar_allowed_group).to eq(group)
+          expect(setting.reload.performance_bar_allowed_group).to eq(group)
         end
       end
     end
@@ -478,67 +416,11 @@ describe ApplicationSetting do
         let(:group) { create(:group) }
 
         before do
-          setting.performance_bar_allowed_group_id = group.full_path
+          setting.update!(performance_bar_allowed_group_id: group.id)
         end
 
         it 'returns true' do
-          expect(setting.performance_bar_enabled).to be_truthy
-        end
-      end
-    end
-
-    describe 'performance_bar_enabled=' do
-      context 'when the performance bar is enabled' do
-        let(:group) { create(:group) }
-
-        before do
-          setting.performance_bar_allowed_group_id = group.full_path
-        end
-
-        context 'when passing true' do
-          it 'does not clear allowed user IDs cache' do
-            expect(Gitlab::PerformanceBar).not_to receive(:expire_allowed_user_ids_cache)
-
-            setting.performance_bar_enabled = true
-
-            expect(setting.performance_bar_allowed_group_id).to eq(group.id)
-            expect(setting.performance_bar_enabled).to be_truthy
-          end
-        end
-
-        context 'when passing false' do
-          it 'disables the performance bar and clears allowed user IDs cache' do
-            expect(Gitlab::PerformanceBar).to receive(:expire_allowed_user_ids_cache)
-
-            setting.performance_bar_enabled = false
-
-            expect(setting.performance_bar_allowed_group_id).to be_nil
-            expect(setting.performance_bar_enabled).to be_falsey
-          end
-        end
-      end
-
-      context 'when the performance bar is disabled' do
-        context 'when passing true' do
-          it 'does nothing and does not clear allowed user IDs cache' do
-            expect(Gitlab::PerformanceBar).not_to receive(:expire_allowed_user_ids_cache)
-
-            setting.performance_bar_enabled = true
-
-            expect(setting.performance_bar_allowed_group_id).to be_nil
-            expect(setting.performance_bar_enabled).to be_falsey
-          end
-        end
-
-        context 'when passing false' do
-          it 'does nothing and does not clear allowed user IDs cache' do
-            expect(Gitlab::PerformanceBar).not_to receive(:expire_allowed_user_ids_cache)
-
-            setting.performance_bar_enabled = false
-
-            expect(setting.performance_bar_allowed_group_id).to be_nil
-            expect(setting.performance_bar_enabled).to be_falsey
-          end
+          expect(setting.reload.performance_bar_enabled).to be_truthy
         end
       end
     end
diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb
index 8b22d1e72f3..aead8978dd4 100644
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -24,10 +24,15 @@ describe API::Settings, 'Settings' do
       expect(json_response['ecdsa_key_restriction']).to eq(0)
       expect(json_response['ed25519_key_restriction']).to eq(0)
       expect(json_response['circuitbreaker_failure_count_threshold']).not_to be_nil
+      expect(json_response['performance_bar_allowed_group_id']).to be_nil
+      expect(json_response).not_to have_key('performance_bar_allowed_group_path')
+      expect(json_response).not_to have_key('performance_bar_enabled')
     end
   end
 
   describe "PUT /application/settings" do
+    let(:group) { create(:group) }
+
     context "custom repository storage type set in the config" do
       before do
         storages = { 'custom' => 'tmp/tests/custom_repositories' }
@@ -56,7 +61,8 @@ describe API::Settings, 'Settings' do
           ed25519_key_restriction: 256,
           circuitbreaker_check_interval: 2,
           enforce_terms: true,
-          terms: 'Hello world!'
+          terms: 'Hello world!',
+          performance_bar_allowed_group_path: group.full_path
 
         expect(response).to have_gitlab_http_status(200)
         expect(json_response['default_projects_limit']).to eq(3)
@@ -80,9 +86,27 @@ describe API::Settings, 'Settings' do
         expect(json_response['circuitbreaker_check_interval']).to eq(2)
         expect(json_response['enforce_terms']).to be(true)
         expect(json_response['terms']).to eq('Hello world!')
+        expect(json_response['performance_bar_allowed_group_id']).to eq(group.id)
       end
     end
 
+    it "supports legacy performance_bar_allowed_group_id" do
+      put api("/application/settings", admin),
+        performance_bar_allowed_group_id: group.full_path
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(json_response['performance_bar_allowed_group_id']).to eq(group.id)
+    end
+
+    it "supports legacy performance_bar_enabled" do
+      put api("/application/settings", admin),
+        performance_bar_enabled: false,
+        performance_bar_allowed_group_id: group.full_path
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(json_response['performance_bar_allowed_group_id']).to be_nil
+    end
+
     context "missing koding_url value when koding_enabled is true" do
       it "returns a blank parameter error message" do
         put api("/application/settings", admin), koding_enabled: true
diff --git a/spec/rubocop/cop/line_break_around_conditional_block_spec.rb b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb
index 7ddf9141fcd..03eeffe6483 100644
--- a/spec/rubocop/cop/line_break_around_conditional_block_spec.rb
+++ b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb
@@ -256,6 +256,18 @@ describe RuboCop::Cop::LineBreakAroundConditionalBlock do
       expect(cop.offenses).to be_empty
     end
 
+    it "doesn't flag violation for #{conditional} followed by a comment" do
+      source = <<~RUBY
+          #{conditional} condition
+            do_something
+          end
+          # a short comment
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
     it "doesn't flag violation for #{conditional} followed by an end" do
       source = <<~RUBY
           class Foo
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