Merge branch '209014-follow-up-refactor-specs-and-streamline-status-page-settings' into 'master'

Resolve "Follow-up: Refactor specs and streamline status page settings"

Closes #209014

See merge request gitlab-org/gitlab!29462

ee/app/controllers/ee/projects/settings/operations_controller.rb

@@ -8,7 +8,7 @@ module EE
         extend ActiveSupport::Concern
 
         prepended do
-          helper_method :tracing_setting
+          helper_method :tracing_setting, :status_page_setting
 
           private
 
@@ -16,6 +16,10 @@ module EE
             @tracing_setting ||= project.tracing_setting || project.build_tracing_setting
           end
 
+          def status_page_setting
+            @status_page_setting ||= project.status_page_setting || project.build_status_page_setting
+          end
+
           def has_tracing_license?
             project.feature_available?(:tracing, current_user)
           end

ee/app/helpers/operations_helper.rb

@@ -20,14 +20,14 @@ module OperationsHelper
     }
   end
 
-  def status_page_settings_data(status_page_setting)
+  def status_page_settings_data
     {
-      'user-can-enable-status-page' => can?(current_user, :admin_operations, @project).to_s,
-      'setting-enabled' => status_page_setting&.enabled?&.to_s,
-      'setting-aws-access-key' => status_page_setting&.aws_access_key,
-      'setting-masked-aws-secret-key' => status_page_setting&.masked_aws_secret_key,
-      'setting-aws-region' => status_page_setting&.aws_region,
-      'setting-aws-s3-bucket-name' => status_page_setting&.aws_s3_bucket_name
+      'operations-settings-endpoint' => project_settings_operations_path(@project),
+      'enabled' => status_page_setting.enabled?.to_s,
+      'bucket-name' => status_page_setting.aws_s3_bucket_name,
+      'region' => status_page_setting.aws_region,
+      'aws-access-key' => status_page_setting.aws_access_key,
+      'aws-secret-key' => status_page_setting.masked_aws_secret_key
     }
   end
 end

ee/app/models/status_page_setting.rb

@@ -32,6 +32,8 @@ class StatusPageSetting < ApplicationRecord
   scope :enabled, -> { where(enabled: true) }
 
   def masked_aws_secret_key
+    return if aws_secret_key.blank?
+
     '*' * 40
   end
 

ee/app/views/projects/settings/operations/_status_page.html.haml

 - return unless @project.feature_available?(:status_page, current_user) && @project.beta_feature_available?(:status_page)
 
-- setting = status_page_settings_data(@project.status_page_setting)
-
-.js-status-page-settings{ data: { operations_settings_endpoint: project_settings_operations_path(@project),
-    enabled: setting['setting-enabled'],
-    bucket_name: setting['setting-aws-s3-bucket-name'],
-    region: setting['setting-aws-region'],
-    aws_access_key: setting['setting-aws-access-key'],
-    aws_secret_key: setting['setting-masked-aws-secret-key'] } }
+.js-status-page-settings{ data: status_page_settings_data }

ee/spec/helpers/ee/operations_helper_spec.rb

@@ -6,39 +6,27 @@ describe OperationsHelper do
   describe '#status_page_settings_data' do
     let_it_be(:user) { create(:user) }
     let_it_be(:project) { create(:project, :private) }
+    let_it_be(:status_page_setting) { project.build_status_page_setting }
 
-    subject { helper.status_page_settings_data(status_page_setting) }
+    subject { helper.status_page_settings_data }
 
     before do
       helper.instance_variable_set(:@project, project)
+      allow(helper).to receive(:status_page_setting) { status_page_setting }
       allow(helper).to receive(:current_user) { user }
       allow(helper)
         .to receive(:can?).with(user, :admin_operations, project) { true }
     end
 
     context 'setting does not exist' do
-      let(:status_page_setting) { nil }
-
-      it 'returns the correct values' do
-        expect(subject.keys)
-          .to contain_exactly(
-            'user-can-enable-status-page',
-            'setting-enabled',
-            'setting-aws-access-key',
-            'setting-masked-aws-secret-key',
-            'setting-aws-region',
-            'setting-aws-s3-bucket-name'
-          )
-      end
-
       it 'returns the correct values' do
         expect(subject).to eq(
-          'user-can-enable-status-page' => 'true',
-          'setting-enabled' => nil,
-          'setting-aws-access-key' => nil,
-          'setting-masked-aws-secret-key' => nil,
-          'setting-aws-region' => nil,
-          'setting-aws-s3-bucket-name' => nil
+          'operations-settings-endpoint' => project_settings_operations_path(project),
+          'enabled' => 'false',
+          'aws-access-key' => nil,
+          'aws-secret-key' => nil,
+          'region' => nil,
+          'bucket-name' => nil
         )
       end
 
@@ -50,12 +38,12 @@ describe OperationsHelper do
 
         it 'returns the correct values' do
           expect(subject).to eq(
-            'user-can-enable-status-page' => 'false',
-            'setting-enabled' => nil,
-            'setting-aws-access-key' => nil,
-            'setting-masked-aws-secret-key' => nil,
-            'setting-aws-region' => nil,
-            'setting-aws-s3-bucket-name' => nil
+            'operations-settings-endpoint' => project_settings_operations_path(project),
+            'enabled' => 'false',
+            'aws-access-key' => nil,
+            'aws-secret-key' => nil,
+            'region' => nil,
+            'bucket-name' => nil
           )
         end
       end
@@ -65,14 +53,14 @@ describe OperationsHelper do
       let(:status_page_setting) { create(:status_page_setting) }
 
       it 'returns the correct values' do
-        aggregate_failures do
-          expect(subject['user-can-enable-status-page']).to eq('true')
-          expect(subject['setting-enabled']).to eq(status_page_setting.enabled.to_s)
-          expect(subject['setting-aws-access-key']).to eq(status_page_setting.aws_access_key)
-          expect(subject['setting-masked-aws-secret-key']).to eq(status_page_setting.masked_aws_secret_key)
-          expect(subject['setting-aws-region']).to eq(status_page_setting.aws_region)
-          expect(subject['setting-aws-s3-bucket-name']).to eq(status_page_setting.aws_s3_bucket_name)
-        end
+        expect(subject).to eq(
+          'operations-settings-endpoint' => project_settings_operations_path(project),
+          'enabled' => status_page_setting.enabled.to_s,
+          'aws-access-key' => status_page_setting.aws_access_key,
+          'aws-secret-key' => status_page_setting.masked_aws_secret_key,
+          'region' => status_page_setting.aws_region,
+          'bucket-name' => status_page_setting.aws_s3_bucket_name
+        )
       end
     end
   end

ee/spec/models/status_page_setting_spec.rb

@@ -74,6 +74,20 @@ describe StatusPageSetting do
     end
   end
 
+  describe '#masked_aws_secret_key' do
+    let(:status_page_setting) { build(:status_page_setting) }
+
+    subject { status_page_setting.masked_aws_secret_key }
+
+    it { is_expected.to eq('*' * 40) }
+
+    context 'when no secret saved' do
+      let(:status_page_setting) { build(:status_page_setting, aws_secret_key: nil) }
+
+      it { is_expected.to eq(nil) }
+    end
+  end
+
   describe '#enabled?' do
     let(:status_page_setting) { build(:status_page_setting, :enabled) }