Merge branch 'jnnkl-security-config-refactor' into 'master'

Refactor Security Config page after backend has been unified for CE & EE

See merge request gitlab-org/gitlab!79520

app/assets/javascripts/pages/projects/security/configuration/index.js

 import { initSecurityConfiguration } from '~/security_configuration';
 
-initSecurityConfiguration(document.querySelector('#js-security-configuration-static'));
+initSecurityConfiguration(document.querySelector('#js-security-configuration'));

app/assets/javascripts/security_configuration/components/constants.js

@@ -159,15 +159,6 @@ export const securityFeatures = [
     helpPath: SAST_HELP_PATH,
     configurationHelpPath: SAST_CONFIG_HELP_PATH,
     type: REPORT_TYPE_SAST,
-    // This field is currently hardcoded because SAST is always available.
-    // It will eventually come from the Backend, the progress is tracked in
-    // https://gitlab.com/gitlab-org/gitlab/-/issues/331622
-    available: true,
-
-    // This field is currently hardcoded because SAST can always be enabled via MR
-    // It will eventually come from the Backend, the progress is tracked in
-    // https://gitlab.com/gitlab-org/gitlab/-/issues/331621
-    canEnableByMergeRequest: true,
   },
   {
     name: SAST_IAC_NAME,
@@ -176,15 +167,6 @@ export const securityFeatures = [
     helpPath: SAST_IAC_HELP_PATH,
     configurationHelpPath: SAST_IAC_CONFIG_HELP_PATH,
     type: REPORT_TYPE_SAST_IAC,
-
-    // This field is currently hardcoded because SAST IaC is always available.
-    // It will eventually come from the Backend, the progress is tracked in
-    // https://gitlab.com/gitlab-org/gitlab/-/issues/331622
-    available: true,
-
-    // This field will eventually come from the backend, the progress is
-    // tracked in https://gitlab.com/gitlab-org/gitlab/-/issues/331621
-    canEnableByMergeRequest: true,
   },
   {
     name: DAST_NAME,
@@ -206,10 +188,6 @@ export const securityFeatures = [
     helpPath: DEPENDENCY_SCANNING_HELP_PATH,
     configurationHelpPath: DEPENDENCY_SCANNING_CONFIG_HELP_PATH,
     type: REPORT_TYPE_DEPENDENCY_SCANNING,
-
-    // This field will eventually come from the backend, the progress is
-    // tracked in https://gitlab.com/gitlab-org/gitlab/-/issues/331621
-    canEnableByMergeRequest: true,
   },
   {
     name: CONTAINER_SCANNING_NAME,
@@ -217,10 +195,6 @@ export const securityFeatures = [
     helpPath: CONTAINER_SCANNING_HELP_PATH,
     configurationHelpPath: CONTAINER_SCANNING_CONFIG_HELP_PATH,
     type: REPORT_TYPE_CONTAINER_SCANNING,
-
-    // This field will eventually come from the backend, the progress is
-    // tracked in https://gitlab.com/gitlab-org/gitlab/-/issues/331621
-    canEnableByMergeRequest: true,
   },
   {
     name: CLUSTER_IMAGE_SCANNING_NAME,
@@ -235,16 +209,6 @@ export const securityFeatures = [
     helpPath: SECRET_DETECTION_HELP_PATH,
     configurationHelpPath: SECRET_DETECTION_CONFIG_HELP_PATH,
     type: REPORT_TYPE_SECRET_DETECTION,
-
-    // This field is currently hardcoded because Secret Detection is always
-    // available.  It will eventually come from the Backend, the progress is
-    // tracked in https://gitlab.com/gitlab-org/gitlab/-/issues/333113
-    available: true,
-
-    // This field is currently hardcoded because SAST can always be enabled via MR
-    // It will eventually come from the Backend, the progress is tracked in
-    // https://gitlab.com/gitlab-org/gitlab/-/issues/331621
-    canEnableByMergeRequest: true,
   },
   {
     name: API_FUZZING_NAME,

app/assets/javascripts/security_configuration/components/feature_card.vue

@@ -24,9 +24,6 @@ export default {
     enabled() {
       return this.available && this.feature.configured;
     },
-    hasStatus() {
-      return !this.available || typeof this.feature.configured === 'boolean';
-    },
     shortName() {
       return this.feature.shortName ?? this.feature.name;
     },
@@ -93,19 +90,17 @@ export default {
         data-testid="feature-status"
         :data-qa-selector="`${feature.type}_status`"
       >
-        <template v-if="hasStatus">
-          <template v-if="enabled">
-            <gl-icon name="check-circle-filled" />
-            <span class="gl-text-green-700">{{ $options.i18n.enabled }}</span>
-          </template>
+        <template v-if="enabled">
+          <gl-icon name="check-circle-filled" />
+          <span class="gl-text-green-700">{{ $options.i18n.enabled }}</span>
+        </template>
 
-          <template v-else-if="available">
-            {{ $options.i18n.notEnabled }}
-          </template>
+        <template v-else-if="available">
+          {{ $options.i18n.notEnabled }}
+        </template>
 
-          <template v-else>
-            {{ $options.i18n.availableWith }}
-          </template>
+        <template v-else>
+          {{ $options.i18n.availableWith }}
         </template>
       </div>
     </div>

app/assets/javascripts/security_configuration/utils.js

 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import { SCANNER_NAMES_MAP } from '~/security_configuration/components/constants';
 
+/**
+ * This function takes in 3 arrays of objects, securityFeatures, complianceFeatures and features.
+ * securityFeatures and complianceFeatures are static arrays living in the constants.
+ * features is dynamic and coming from the backend.
+ * This function builds a superset of those arrays.
+ * It looks for matching keys within the dynamic and the static arrays
+ * and will enrich the objects with the available static data.
+ * @param [{}] securityFeatures
+ * @param [{}] complianceFeatures
+ * @param [{}] features
+ * @returns {Object} Object with enriched features from constants divided into Security and Compliance Features
+ */
+
 export const augmentFeatures = (securityFeatures, complianceFeatures, features = []) => {
   const featuresByType = features.reduce((acc, feature) => {
     acc[feature.type] = convertObjectPropsToCamelCase(feature, { deep: true });

app/controllers/projects/security/configuration_controller.rb

@@ -10,6 +10,8 @@ module Projects
       def show
         render_403 unless can?(current_user, :read_security_configuration, project)
 
+        @configuration ||= configuration_presenter
+
         respond_to do |format|
           format.html
           format.json do

app/presenters/projects/security/configuration_presenter.rb

@@ -81,7 +81,7 @@ module Projects
           configured: scan.configured?,
           configuration_path: scan.configuration_path,
           available: scan.available?,
-          can_enable_in_merge_request: scan.can_enable_in_merge_request?
+          can_enable_by_merge_request: scan.can_enable_by_merge_request?
         }
       end
 

app/views/projects/security/configuration/show.html.haml

@@ -2,4 +2,6 @@
 - page_title _("Security Configuration")
 - @content_class = "limit-container-width" unless fluid_layout
 
-#js-security-configuration-static{ data: { project_full_path: @project.full_path, upgrade_path: security_upgrade_path } }
+#js-security-configuration{ data: { **@configuration.to_html_data_attribute,
+  upgrade_path: security_upgrade_path,
+  project_full_path: @project.full_path } }

ee/app/assets/javascripts/pages/projects/security/configuration/index.js

-import { initSecurityConfiguration } from '~/security_configuration';
-
-const el =
-  document.querySelector('#js-security-configuration') ||
-  document.querySelector('#js-security-configuration-static');
-
-initSecurityConfiguration(el);

ee/app/views/projects/security/configuration/show.html.haml

@@ -2,12 +2,6 @@
 - page_title _("Security Configuration")
 - @content_class = "limit-container-width" unless fluid_layout
 
-- if @configuration.nil?
-  = render_ce 'projects/security/configuration/show'
-- else
-  #js-security-configuration{ data: { **@configuration.to_html_data_attribute,
-    project_full_path: @project.full_path,
-    auto_fix_help_path: '/',
-    toggle_autofix_setting_endpoint: 'configuration/auto_fix',
-    container_scanning_help_path: help_page_path('user/application_security/container_scanning/index'),
-    dependency_scanning_help_path: help_page_path('user/application_security/dependency_scanning/index') } }
+#js-security-configuration{ data: { **@configuration.to_html_data_attribute,
+  upgrade_path: security_upgrade_path,
+  project_full_path: @project.full_path } }

ee/spec/controllers/projects/security/configuration_controller_spec.rb

@@ -24,7 +24,7 @@ RSpec.describe Projects::Security::ConfigurationController do
     where(:user_role, :security_dashboard_enabled, :status, :selector) do
       :guest     | false | :forbidden | nil
       :guest     | true  | :forbidden | nil
-      :developer | false | :ok        | '#js-security-configuration-static'
+      :developer | false | :ok        | '#js-security-configuration'
       :developer | true  | :ok        | '#js-security-configuration'
     end
 

ee/spec/lib/ee/gitlab/security/scan_configuration_spec.rb

@@ -98,8 +98,8 @@ RSpec.describe ::Gitlab::Security::ScanConfiguration do
     end
   end
 
-  describe '#can_enable_in_merge_request?' do
-    subject { scan.can_enable_in_merge_request? }
+  describe '#can_enable_by_merge_request?' do
+    subject { scan.can_enable_by_merge_request? }
 
     context 'with a scanner that can be enabled in merge request' do
       where(type: %i(sast sast_iac secret_detection dependency_scanning container_scanning))

lib/gitlab/security/scan_configuration.rb

@@ -21,7 +21,7 @@ module Gitlab
         %i[sast sast_iac secret_detection].include?(type)
       end
 
-      def can_enable_in_merge_request?
+      def can_enable_by_merge_request?
         scans_configurable_in_merge_request.include?(type)
       end
 

spec/frontend/security_configuration/components/feature_card_spec.js

@@ -113,7 +113,6 @@ describe('FeatureCard component', () => {
       context                                         | available | configured   | expectedStatus
       ${'a configured feature'}                       | ${true}   | ${true}      | ${'Enabled'}
       ${'an unconfigured feature'}                    | ${true}   | ${false}     | ${'Not enabled'}
-      ${'an available feature with unknown status'}   | ${true}   | ${undefined} | ${''}
       ${'an unavailable feature'}                     | ${false}  | ${false}     | ${'Available with Ultimate'}
       ${'an unavailable feature with unknown status'} | ${false}  | ${undefined} | ${'Available with Ultimate'}
     `('given $context', ({ available, configured, expectedStatus }) => {

spec/lib/gitlab/security/scan_configuration_spec.rb

@@ -47,8 +47,8 @@ RSpec.describe ::Gitlab::Security::ScanConfiguration do
     it { is_expected.to be_nil }
   end
 
-  describe '#can_enable_in_merge_request?' do
-    subject { scan.can_enable_in_merge_request? }
+  describe '#can_enable_by_merge_request?' do
+    subject { scan.can_enable_by_merge_request? }
 
     let(:configured) { true }
 

spec/presenters/projects/security/configuration_presenter_spec.rb

@@ -88,7 +88,7 @@ RSpec.describe Projects::Security::ConfigurationPresenter do
         expect(feature['configured']).to eq(true)
         expect(feature['configuration_path']).to be_nil
         expect(feature['available']).to eq(true)
-        expect(feature['can_enable_in_merge_request']).to eq(true)
+        expect(feature['can_enable_by_merge_request']).to eq(true)
       end
 
       context 'when checking features configured status' do