Merge branch...

Merge branch '231373-update-security-configuration-page-to-link-to-sast-configuration-ui' into 'master'

Update Security Configuration page to link to SAST Configuration UI

See merge request gitlab-org/gitlab!38810

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

 <script>
 import { GlAlert, GlLink, GlSprintf, GlTable } from '@gitlab/ui';
-import { s__, __, sprintf } from '~/locale';
+import { s__, __ } from '~/locale';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import AutoFixSettings from './auto_fix_settings.vue';
-import CreateMergeRequestButton from './create_merge_request_button.vue';
+import ManageFeature from './manage_feature.vue';
 
 export default {
   components: {
@@ -12,7 +12,7 @@ export default {
     GlSprintf,
     GlTable,
     AutoFixSettings,
-    CreateMergeRequestButton,
+    ManageFeature,
   },
   mixins: [glFeatureFlagsMixin()],
   props: {
@@ -60,8 +60,7 @@ export default {
     // TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/227575
     createSastMergeRequestPath: {
       type: String,
-      required: false,
-      default: '',
+      required: true,
     },
   },
   computed: {
@@ -114,17 +113,6 @@ export default {
 
       return s__('SecurityConfiguration|Not enabled');
     },
-    getFeatureDocumentationLinkLabel(featureName) {
-      return sprintf(s__('SecurityConfiguration|Feature documentation for %{featureName}'), {
-        featureName,
-      });
-    },
-    // TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/227575
-    canCreateSASTMergeRequest(feature) {
-      return Boolean(
-        feature.type === 'sast' && this.createSastMergeRequestPath && !this.gitlabCiPresent,
-      );
-    },
   },
   autoDevopsAlertMessage: s__(`
     SecurityConfiguration|You can quickly enable all security scanning tools by
@@ -170,20 +158,12 @@ export default {
       </template>
 
       <template #cell(manage)="{ item }">
-        <create-merge-request-button
-          v-if="canCreateSASTMergeRequest(item)"
+        <manage-feature
+          :feature="item"
+          :gitlab-ci-present="gitlabCiPresent"
           :auto-devops-enabled="autoDevopsEnabled"
-          :endpoint="createSastMergeRequestPath"
+          :create-sast-merge-request-path="createSastMergeRequestPath"
         />
-
-        <gl-link
-          v-else
-          target="_blank"
-          :href="item.link"
-          :aria-label="getFeatureDocumentationLinkLabel(item.name)"
-        >
-          {{ s__('SecurityConfiguration|See documentation') }}
-        </gl-link>
       </template>
     </gl-table>
     <auto-fix-settings v-if="glFeatures.securityAutoFix" v-bind="autoFixSettingsProps" />

ee/app/assets/javascripts/security_configuration/components/manage_feature.vue

+<script>
+import { sprintf, s__ } from '~/locale';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
+import { GlButton, GlLink } from '@gitlab/ui';
+import CreateMergeRequestButton from './create_merge_request_button.vue';
+
+export default {
+  components: {
+    GlButton,
+    GlLink,
+    CreateMergeRequestButton,
+  },
+  mixins: [glFeatureFlagsMixin()],
+  props: {
+    feature: {
+      type: Object,
+      required: true,
+    },
+    autoDevopsEnabled: {
+      type: Boolean,
+      required: true,
+    },
+    gitlabCiPresent: {
+      type: Boolean,
+      required: true,
+    },
+    createSastMergeRequestPath: {
+      type: String,
+      required: true,
+    },
+  },
+  computed: {
+    canConfigureFeature() {
+      return Boolean(
+        this.glFeatures.sastConfigurationUi &&
+          this.feature.configuration_path &&
+          !this.gitlabCiPresent,
+      );
+    },
+    // TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/227575
+    canCreateSASTMergeRequest() {
+      return Boolean(
+        this.feature.type === 'sast' && this.createSastMergeRequestPath && !this.gitlabCiPresent,
+      );
+    },
+    getFeatureDocumentationLinkLabel() {
+      return sprintf(s__('SecurityConfiguration|Feature documentation for %{featureName}'), {
+        featureName: this.feature.name,
+      });
+    },
+  },
+};
+</script>
+
+<template>
+  <gl-button
+    v-if="canConfigureFeature && autoDevopsEnabled"
+    :href="feature.configuration_path"
+    data-testid="configureButton"
+    >{{ s__('SecurityConfiguration|Configure') }}</gl-button
+  >
+
+  <gl-button
+    v-else-if="canConfigureFeature"
+    variant="success"
+    category="primary"
+    :href="feature.configuration_path"
+    data-testid="enableButton"
+    >{{ s__('SecurityConfiguration|Enable') }}</gl-button
+  >
+
+  <!-- TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/227575 -->
+  <create-merge-request-button
+    v-else-if="canCreateSASTMergeRequest"
+    :auto-devops-enabled="autoDevopsEnabled"
+    :endpoint="createSastMergeRequestPath"
+  />
+
+  <gl-link
+    v-else
+    target="_blank"
+    :href="feature.link"
+    :aria-label="getFeatureDocumentationLinkLabel"
+    data-testid="docsLink"
+  >
+    {{ s__('SecurityConfiguration|See documentation') }}
+  </gl-link>
+</template>

ee/app/controllers/projects/security/configuration_controller.rb

@@ -9,6 +9,7 @@ module Projects
 
       before_action only: [:show] do
         push_frontend_feature_flag(:security_auto_fix, project, default_enabled: false)
+        push_frontend_feature_flag(:sast_configuration_ui, project, default_enabled: false)
       end
 
       before_action only: [:auto_fix] do

ee/app/presenters/projects/security/configuration_presenter.rb

@@ -134,6 +134,7 @@ module Projects
           configured: configured,
           description: self.class.localized_scan_descriptions[type],
           link: help_page_path(SCAN_DOCS[type]),
+          configuration_path: configuration_path(type),
           name: localized_scan_names[type]
         }
       end
@@ -149,6 +150,12 @@ module Projects
       def project_settings
         ProjectSecuritySetting.safe_find_or_create_for(project)
       end
+
+      def configuration_path(type)
+        {
+          sast: project_security_configuration_sast_path(project)
+        }[type]
+      end
     end
   end
 end

ee/spec/frontend/security_configuration/components/app_spec.js

@@ -2,8 +2,9 @@ import { mount } from '@vue/test-utils';
 import { merge } from 'lodash';
 import { GlAlert, GlLink } from '@gitlab/ui';
 import SecurityConfigurationApp from 'ee/security_configuration/components/app.vue';
-import CreateMergeRequestButton from 'ee/security_configuration/components/create_merge_request_button.vue';
+import ManageFeature from 'ee/security_configuration/components/manage_feature.vue';
 import stubChildren from 'helpers/stub_children';
+import { generateFeatures } from './helpers';
 
 const propsData = {
   features: [],
@@ -12,6 +13,7 @@ const propsData = {
   autoDevopsHelpPagePath: 'http://autoDevopsHelpPagePath',
   autoDevopsPath: 'http://autoDevopsPath',
   helpPagePath: 'http://helpPagePath',
+  gitlabCiPresent: false,
   autoFixSettingsProps: {},
   createSastMergeRequestPath: 'http://createSastMergeRequestPath',
 };
@@ -41,22 +43,10 @@ describe('Security Configuration App', () => {
     wrapper = null;
   });
 
-  const generateFeatures = (n, overrides = {}) => {
-    return [...Array(n).keys()].map(i => ({
-      type: `scan-type-${i}`,
-      name: `name-feature-${i}`,
-      description: `description-feature-${i}`,
-      link: `link-feature-${i}`,
-      configured: i % 2 === 0,
-      ...overrides,
-    }));
-  };
-
   const getPipelinesLink = () => wrapper.find({ ref: 'pipelinesLink' });
   const getFeaturesTable = () => wrapper.find({ ref: 'securityControlTable' });
   const getFeaturesRows = () => getFeaturesTable().findAll('tbody tr');
   const getAlert = () => wrapper.find(GlAlert);
-  const getCreateMergeRequestButton = () => wrapper.find(CreateMergeRequestButton);
   const getRowCells = row => {
     const [feature, status, manage] = row.findAll('td').wrappers;
     return { feature, status, manage };
@@ -142,7 +132,12 @@ describe('Security Configuration App', () => {
         expect(feature.text()).toMatch(features[i].name);
         expect(feature.text()).toMatch(features[i].description);
         expect(status.text()).toMatch(features[i].configured ? 'Enabled' : 'Not enabled');
-        expect(manage.find(GlLink).attributes('href')).toBe(features[i].link);
+        expect(manage.find(ManageFeature).props()).toMatchObject({
+          feature: features[i],
+          autoDevopsEnabled: propsData.autoDevopsEnabled,
+          gitlabCiPresent: propsData.gitlabCiPresent,
+          createSastMergeRequestPath: propsData.createSastMergeRequestPath,
+        });
       }
     });
 
@@ -157,45 +152,4 @@ describe('Security Configuration App', () => {
       });
     });
   });
-
-  describe('enabling SAST by merge request', () => {
-    describe.each`
-      gitlabCiPresent | autoDevopsEnabled | buttonExpected
-      ${false}        | ${false}          | ${true}
-      ${false}        | ${true}           | ${true}
-      ${true}         | ${false}          | ${false}
-    `(
-      'given gitlabCiPresent is $gitlabCiPresent, autoDevopsEnabled is $autoDevopsEnabled',
-      ({ gitlabCiPresent, autoDevopsEnabled, buttonExpected }) => {
-        beforeEach(() => {
-          const features = generateFeatures(1, { type: 'sast', configured: false });
-
-          createComponent({
-            propsData: { features, gitlabCiPresent, autoDevopsEnabled },
-          });
-        });
-
-        if (buttonExpected) {
-          it('renders the CreateMergeRequestButton component', () => {
-            const button = getCreateMergeRequestButton();
-            expect(button.exists()).toBe(true);
-            expect(button.props()).toMatchObject({
-              endpoint: propsData.createSastMergeRequestPath,
-              autoDevopsEnabled,
-            });
-          });
-
-          it('does not render the documentation link', () => {
-            const { manage } = getRowCells(getFeaturesRows().at(0));
-
-            expect(manage.contains(GlLink)).toBe(false);
-          });
-        } else {
-          it('does not render the CreateMergeRequestButton component', () => {
-            expect(getCreateMergeRequestButton().exists()).toBe(false);
-          });
-        }
-      },
-    );
-  });
 });

ee/spec/frontend/security_configuration/components/helpers.js

+// eslint-disable-next-line import/prefer-default-export
+export const generateFeatures = (n, overrides = {}) => {
+  return [...Array(n).keys()].map(i => ({
+    type: `scan-type-${i}`,
+    name: `name-feature-${i}`,
+    description: `description-feature-${i}`,
+    link: `link-feature-${i}`,
+    configuration_path: i % 2 ? `configuration_path-${i}` : null,
+    configured: i % 2 === 0,
+    ...overrides,
+  }));
+};

ee/spec/frontend/security_configuration/components/manage_feature_spec.js

+import { merge } from 'lodash';
+import { shallowMount } from '@vue/test-utils';
+import ManageFeature from 'ee/security_configuration/components/manage_feature.vue';
+import CreateMergeRequestButton from 'ee/security_configuration/components/create_merge_request_button.vue';
+import { generateFeatures } from './helpers';
+
+const createSastMergeRequestPath = '/create_sast_merge_request_path';
+
+describe('ManageFeature component', () => {
+  let wrapper;
+  let feature;
+
+  const createComponent = options => {
+    wrapper = shallowMount(
+      ManageFeature,
+      merge(
+        {
+          propsData: {
+            createSastMergeRequestPath,
+            gitlabCiPresent: false,
+            autoDevopsEnabled: false,
+          },
+        },
+        options,
+      ),
+    );
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+    wrapper = null;
+  });
+
+  const findCreateMergeRequestButton = () => wrapper.find(CreateMergeRequestButton);
+  const findTestId = id => wrapper.find(`[data-testid="${id}"]`);
+
+  describe('given sastConfigurationUi feature flag is enabled', () => {
+    const featureFlagEnabled = {
+      provide: {
+        glFeatures: {
+          sastConfigurationUi: true,
+        },
+      },
+    };
+
+    describe.each`
+      autoDevopsEnabled | expectedTestId
+      ${true}           | ${'configureButton'}
+      ${false}          | ${'enableButton'}
+    `('given autoDevopsEnabled is $autoDevopsEnabled', ({ autoDevopsEnabled, expectedTestId }) => {
+      describe('given no CI file and feature with a configuration path', () => {
+        beforeEach(() => {
+          [feature] = generateFeatures(1, { configuration_path: 'foo' });
+
+          createComponent({
+            ...featureFlagEnabled,
+            propsData: { feature, gitlabCiPresent: false, autoDevopsEnabled },
+          });
+        });
+
+        it('shows a button to configure the feature', () => {
+          const button = findTestId(expectedTestId);
+          expect(button.exists()).toBe(true);
+          expect(button.attributes('href')).toBe(feature.configuration_path);
+        });
+      });
+    });
+  });
+
+  describe('given a feature with type "sast" and no CI file', () => {
+    const autoDevopsEnabled = true;
+
+    beforeEach(() => {
+      [feature] = generateFeatures(1, { type: 'sast' });
+
+      createComponent({
+        propsData: { feature, gitlabCiPresent: false, autoDevopsEnabled },
+      });
+    });
+
+    it('shows the CreateMergeRequestButton component', () => {
+      const button = findCreateMergeRequestButton();
+      expect(button.exists()).toBe(true);
+      expect(button.props()).toMatchObject({
+        endpoint: createSastMergeRequestPath,
+        autoDevopsEnabled,
+      });
+    });
+  });
+
+  describe.each`
+    featureProps        | gitlabCiPresent
+    ${{ type: 'sast' }} | ${true}
+    ${{}}               | ${false}
+  `(
+    'given a featureProps with $featureProps and gitlabCiPresent is $gitlabCiPresent',
+    ({ featureProps, gitlabCiPresent }) => {
+      beforeEach(() => {
+        [feature] = generateFeatures(1, featureProps);
+
+        createComponent({
+          propsData: { feature, gitlabCiPresent },
+        });
+      });
+
+      it('shows docs link for feature', () => {
+        const link = findTestId('docsLink');
+        expect(link.exists()).toBe(true);
+        expect(link.attributes('aria-label')).toContain(feature.name);
+        expect(link.attributes('href')).toBe(feature.link);
+      });
+    },
+  );
+});

ee/spec/presenters/projects/security/configuration_presenter_spec.rb

@@ -223,11 +223,14 @@ RSpec.describe Projects::Security::ConfigurationPresenter do
   end
 
   def security_scan(type, configured:)
+    configuration_path = project_security_configuration_sast_path(project) if type == :sast
+
     {
       "type" => type.to_s,
       "configured" => configured,
       "description" => described_class.localized_scan_descriptions[type],
       "link" => help_page_path(described_class::SCAN_DOCS[type]),
+      "configuration_path" => configuration_path,
       "name" => described_class.localized_scan_names[type]
     }
   end

locale/gitlab.pot

@@ -21420,6 +21420,9 @@ msgstr ""
 msgid "SecurityConfiguration|Customize common SAST settings to suit your requirements. Configuration changes made here override those provided by GitLab and are excluded from updates. For details of more advanced configuration options, see the %{linkStart}GitLab SAST documentation%{linkEnd}."
 msgstr ""
 
+msgid "SecurityConfiguration|Enable"
+msgstr ""
+
 msgid "SecurityConfiguration|Enable via Merge Request"
 msgstr ""