Merge branch '323375-move-security-config-status-strings-to-frontend' into 'master'

Move Security Configuration scanner status strings to frontend

See merge request gitlab-org/gitlab!56555

app/assets/javascripts/vue_shared/components/lib/utils/props_utils.js

+/**
+ * Return the union of the given components' props options. Required props take
+ * precendence over non-required props of the same name.
+ *
+ * This makes two assumptions:
+ *  - All given components define their props in verbose object format.
+ *  - The components all agree on the `type` of a common prop.
+ *
+ * @param {object[]} components The components to derive the union from.
+ * @returns {object} The union of the props of the given components.
+ */
+export const propsUnion = (components) =>
+  components.reduce((acc, component) => {
+    Object.entries(component.props ?? {}).forEach(([propName, propOptions]) => {
+      if (process.env.NODE_ENV !== 'production') {
+        if (typeof propOptions !== 'object' || !('type' in propOptions)) {
+          throw new Error(
+            `Cannot create props union: expected verbose prop options for prop "${propName}"`,
+          );
+        }
+
+        if (propName in acc && acc[propName]?.type !== propOptions?.type) {
+          throw new Error(
+            `Cannot create props union: incompatible prop types for prop "${propName}"`,
+          );
+        }
+      }
+
+      if (!(propName in acc) || propOptions.required) {
+        acc[propName] = propOptions;
+      }
+    });
+
+    return acc;
+  }, {});

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

@@ -88,6 +88,7 @@ export default {
         :feature="item"
         :gitlab-ci-present="gitlabCiPresent"
         :gitlab-ci-history-path="gitlabCiHistoryPath"
+        :auto-devops-enabled="autoDevopsEnabled"
       />
     </template>
 

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

 <script>
-import { GlLink } from '@gitlab/ui';
+import { propsUnion } from '~/vue_shared/components/lib/utils/props_utils';
+import {
+  REPORT_TYPE_SAST,
+  REPORT_TYPE_DAST_PROFILES,
+} from '~/vue_shared/security_reports/constants';
+import StatusDastProfiles from './status_dast_profiles.vue';
+import StatusGeneric from './status_generic.vue';
+import StatusSast from './status_sast.vue';
+
+const scannerComponentMap = {
+  [REPORT_TYPE_SAST]: StatusSast,
+  [REPORT_TYPE_DAST_PROFILES]: StatusDastProfiles,
+};
 
 export default {
-  components: {
-    GlLink,
-  },
-  props: {
-    feature: {
-      type: Object,
-      required: true,
-    },
-    gitlabCiPresent: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
-    gitlabCiHistoryPath: {
-      type: String,
-      required: false,
-      default: '',
-    },
-  },
+  inheritAttrs: false,
+  props: propsUnion([StatusGeneric, ...Object.values(scannerComponentMap)]),
   computed: {
-    canViewCiHistory() {
-      const { type, configured } = this.feature;
-      return type === 'sast' && configured && this.gitlabCiPresent;
+    statusComponent() {
+      return scannerComponentMap[this.feature.type] ?? StatusGeneric;
     },
   },
 };
 </script>
 
 <template>
-  <div>
-    {{ feature.status }}
-    <template v-if="canViewCiHistory">
-      <br />
-      <gl-link :href="gitlabCiHistoryPath">{{ s__('SecurityConfiguration|View history') }}</gl-link>
-    </template>
-  </div>
+  <component :is="statusComponent" v-bind="$props" />
 </template>

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

+<script>
+import { s__ } from '~/locale';
+
+export default {
+  inheritAttrs: false,
+  i18n: {
+    availableForOnDemand: s__('SecurityConfiguration|Available for on-demand DAST'),
+  },
+};
+</script>
+
+<template>
+  <div>{{ $options.i18n.availableForOnDemand }}</div>
+</template>

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

+<script>
+import { s__ } from '~/locale';
+
+export default {
+  inheritAttrs: false,
+  props: {
+    feature: {
+      type: Object,
+      required: true,
+    },
+    autoDevopsEnabled: {
+      type: Boolean,
+      required: true,
+    },
+  },
+  computed: {
+    status() {
+      if (this.feature.configured) {
+        return this.autoDevopsEnabled
+          ? this.$options.i18n.enabledWithAutoDevOps
+          : this.$options.i18n.enabled;
+      }
+
+      return this.$options.i18n.notEnabled;
+    },
+  },
+  i18n: {
+    enabled: s__('SecurityConfiguration|Enabled'),
+    enabledWithAutoDevOps: s__('SecurityConfiguration|Enabled with Auto DevOps'),
+    notEnabled: s__('SecurityConfiguration|Not enabled'),
+  },
+};
+</script>
+
+<template>
+  <div>{{ status }}</div>
+</template>

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

+<script>
+import { GlLink } from '@gitlab/ui';
+import StatusGeneric from './status_generic.vue';
+
+export default {
+  components: {
+    GlLink,
+    StatusGeneric,
+  },
+  inheritAttrs: false,
+  props: {
+    feature: {
+      type: Object,
+      required: true,
+    },
+    autoDevopsEnabled: {
+      type: Boolean,
+      required: true,
+    },
+    gitlabCiPresent: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    gitlabCiHistoryPath: {
+      type: String,
+      required: false,
+      default: '',
+    },
+  },
+  computed: {
+    canViewCiHistory() {
+      return this.feature.configured && this.gitlabCiPresent;
+    },
+  },
+};
+</script>
+
+<template>
+  <div>
+    <status-generic :feature="feature" :auto-devops-enabled="autoDevopsEnabled" />
+
+    <gl-link v-if="canViewCiHistory" :href="gitlabCiHistoryPath">{{
+      s__('SecurityConfiguration|View history')
+    }}</gl-link>
+  </div>
+</template>

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

@@ -58,14 +58,11 @@ module Projects
 
       def features
         scans = scan_types.map do |scan_type|
-          if scanner_enabled?(scan_type)
-            scan(scan_type, configured: true, status: auto_devops_source? ? s_('SecurityConfiguration|Enabled with Auto DevOps') : s_('SecurityConfiguration|Enabled'))
-          else
-            scan(scan_type, configured: false, status: s_('SecurityConfiguration|Not enabled'))
-          end
+          scan(scan_type, configured: scanner_enabled?(scan_type))
         end
 
-        dast_profiles_insert(scans)
+        # DAST On-demand scans is a static (non job) entry.  Add it manually.
+        scans << scan(:dast_profiles, configured: true)
       end
 
       def latest_pipeline_path
@@ -74,23 +71,10 @@ module Projects
         project_pipeline_path(self, latest_default_branch_pipeline)
       end
 
-      # DAST On-demand scans is a static (non job) entry.  Add it manually following DAST
-      # TODO: remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/323375
-      def dast_profiles_insert(scans)
-        index = scans.index { |scan| scan[:type] == :dast }
-
-        unless index.nil?
-          scans.insert(index + 1, scan(:dast_profiles, configured: true, status: s_('SecurityConfiguration|Available for on-demand DAST')))
-        end
-
-        scans
-      end
-
-      def scan(type, configured: false, status:)
+      def scan(type, configured: false)
         {
           type: type,
           configured: configured,
-          status: status,
           configuration_path: configuration_path(type)
         }
       end

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

@@ -65,6 +65,7 @@ describe('ConfigurationTable component', () => {
       feature,
       gitlabCiPresent: propsData.gitlabCiPresent,
       gitlabCiHistoryPath: propsData.gitlabCiHistoryPath,
+      autoDevopsEnabled: propsData.autoDevopsEnabled,
     });
     expect(manage.find(ManageFeature).props()).toEqual({
       feature,

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

-import { GlLink } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
+import { pick } from 'lodash';
 import FeatureStatus from 'ee/security_configuration/components/feature_status.vue';
+import StatusDastProfiles from 'ee/security_configuration/components/status_dast_profiles.vue';
+import StatusGeneric from 'ee/security_configuration/components/status_generic.vue';
+import StatusSast from 'ee/security_configuration/components/status_sast.vue';
+import {
+  REPORT_TYPE_SAST,
+  REPORT_TYPE_DAST_PROFILES,
+} from '~/vue_shared/security_reports/constants';
 import { generateFeatures } from './helpers';
 
-const gitlabCiHistoryPath = '/ci/history';
+const props = {
+  gitlabCiPresent: true,
+  gitlabCiHistoryPath: '/ci-history',
+  autoDevopsEnabled: false,
+};
+
+const attrs = {
+  'data-foo': 'bar',
+};
 
 describe('FeatureStatus component', () => {
   let wrapper;
-  let feature;
 
   const createComponent = (options) => {
     wrapper = shallowMount(FeatureStatus, options);
@@ -15,39 +29,37 @@ describe('FeatureStatus component', () => {
 
   afterEach(() => {
     wrapper.destroy();
-    feature = undefined;
   });
 
-  const findHistoryLink = () => wrapper.find(GlLink);
-
   describe.each`
-    context                       | type      | configured | gitlabCiPresent | shouldShowHistory
-    ${'no CI with sast disabled'} | ${'sast'} | ${false}   | ${false}        | ${false}
-    ${'CI with sast disabled'}    | ${'sast'} | ${false}   | ${true}         | ${false}
-    ${'no CI with sast enabled'}  | ${'sast'} | ${true}    | ${false}        | ${false}
-    ${'CI with foo enabled'}      | ${'foo'}  | ${true}    | ${true}         | ${false}
-    ${'CI with sast enabled'}     | ${'sast'} | ${true}    | ${true}         | ${true}
-  `('given $context', ({ type, configured, gitlabCiPresent, shouldShowHistory }) => {
+    type                         | expectedComponent
+    ${REPORT_TYPE_SAST}          | ${StatusSast}
+    ${REPORT_TYPE_DAST_PROFILES} | ${StatusDastProfiles}
+    ${'foo'}                     | ${StatusGeneric}
+  `('given a $type feature', ({ type, expectedComponent }) => {
+    let feature;
+    let component;
+
     beforeEach(() => {
-      [feature] = generateFeatures(1, { type, configured });
+      [feature] = generateFeatures(1, { type });
 
       createComponent({
-        propsData: { feature, gitlabCiPresent, gitlabCiHistoryPath },
+        propsData: { feature, ...props },
+        attrs,
       });
-    });
 
-    it('shows feature status text', () => {
-      expect(wrapper.text()).toContain(feature.status);
+      component = wrapper.findComponent(expectedComponent);
     });
 
-    it(`${shouldShowHistory ? 'shows' : 'does not show'} the history link`, () => {
-      expect(findHistoryLink().exists()).toBe(shouldShowHistory);
+    it('renders expected component', () => {
+      expect(component.exists()).toBe(true);
     });
 
-    if (shouldShowHistory) {
-      it("sets the link's href correctly", () => {
-        expect(findHistoryLink().attributes('href')).toBe(gitlabCiHistoryPath);
-      });
-    }
+    it('passes through props to expected component', () => {
+      // Exclude props not defined on the expected component, since
+      // @vue/test-utils won't include them in `Wrapper#props`.
+      const expectedProps = pick({ feature, ...props }, Object.keys(expectedComponent.props ?? {}));
+      expect(component.props()).toEqual(expectedProps);
+    });
   });
 });

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

+import { shallowMount } from '@vue/test-utils';
+import StatusDastProfiles from 'ee/security_configuration/components/status_dast_profiles.vue';
+
+describe('StatusDastProfiles component', () => {
+  let wrapper;
+
+  const createComponent = () => {
+    wrapper = shallowMount(StatusDastProfiles);
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  it('renders the fixed DAST Profiles status', () => {
+    createComponent();
+
+    expect(wrapper.element).toMatchInlineSnapshot(`
+      <div>
+        Available for on-demand DAST
+      </div>
+    `);
+  });
+});

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

+import { shallowMount } from '@vue/test-utils';
+import StatusGeneric from 'ee/security_configuration/components/status_generic.vue';
+import { generateFeatures } from './helpers';
+
+describe('StatusGeneric component', () => {
+  let wrapper;
+
+  const createComponent = (options) => {
+    wrapper = shallowMount(StatusGeneric, options);
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe.each`
+    context                                         | configured | autoDevopsEnabled | status
+    ${'not configured'}                             | ${false}   | ${false}          | ${StatusGeneric.i18n.notEnabled}
+    ${'not configured, but Auto DevOps is enabled'} | ${false}   | ${true}           | ${StatusGeneric.i18n.notEnabled}
+    ${'configured'}                                 | ${true}    | ${false}          | ${StatusGeneric.i18n.enabled}
+    ${'configured with Auto DevOps'}                | ${true}    | ${true}           | ${StatusGeneric.i18n.enabledWithAutoDevOps}
+  `('given the feature is $context', ({ configured, autoDevopsEnabled, status }) => {
+    let feature;
+
+    beforeEach(() => {
+      [feature] = generateFeatures(1, { configured });
+
+      createComponent({
+        propsData: { feature, autoDevopsEnabled },
+      });
+    });
+
+    it(`shows the status "${status}"`, () => {
+      expect(wrapper.text()).toBe(status);
+    });
+  });
+});

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

+import { GlLink } from '@gitlab/ui';
+import { shallowMount } from '@vue/test-utils';
+import StatusGeneric from 'ee/security_configuration/components/status_generic.vue';
+import StatusSast from 'ee/security_configuration/components/status_sast.vue';
+import { REPORT_TYPE_SAST } from '~/vue_shared/security_reports/constants';
+import { generateFeatures } from './helpers';
+
+const gitlabCiHistoryPath = '/ci/history';
+const autoDevopsEnabled = true;
+
+describe('StatusSast component', () => {
+  let wrapper;
+
+  const createComponent = (options) => {
+    wrapper = shallowMount(StatusSast, options);
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  const findHistoryLink = () => wrapper.find(GlLink);
+
+  describe.each`
+    context                       | configured | gitlabCiPresent | shouldShowHistory
+    ${'no CI with sast disabled'} | ${false}   | ${false}        | ${false}
+    ${'CI with sast disabled'}    | ${false}   | ${true}         | ${false}
+    ${'no CI with sast enabled'}  | ${true}    | ${false}        | ${false}
+    ${'CI with sast enabled'}     | ${true}    | ${true}         | ${true}
+  `('given $context', ({ configured, gitlabCiPresent, shouldShowHistory }) => {
+    let feature;
+
+    beforeEach(() => {
+      [feature] = generateFeatures(1, { type: REPORT_TYPE_SAST, configured });
+
+      createComponent({
+        propsData: { feature, gitlabCiPresent, gitlabCiHistoryPath, autoDevopsEnabled },
+      });
+    });
+
+    it('shows the generic status', () => {
+      const genericComponent = wrapper.findComponent(StatusGeneric);
+      expect(genericComponent.exists()).toBe(true);
+      expect(genericComponent.props()).toEqual({
+        feature,
+        autoDevopsEnabled,
+      });
+    });
+
+    it(`${shouldShowHistory ? 'shows' : 'does not show'} the history link`, () => {
+      expect(findHistoryLink().exists()).toBe(shouldShowHistory);
+    });
+
+    if (shouldShowHistory) {
+      it("sets the link's href correctly", () => {
+        expect(findHistoryLink().attributes('href')).toBe(gitlabCiHistoryPath);
+      });
+    }
+  });
+});

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

@@ -80,15 +80,15 @@ RSpec.describe Projects::Security::ConfigurationPresenter do
 
       it 'reports that all scanners are configured for which latest pipeline has builds' do
         expect(Gitlab::Json.parse(subject[:features])).to contain_exactly(
-          security_scan(:dast, configured: true, auto_dev_ops_enabled: true),
-          security_scan(:dast_profiles, configured: true, auto_dev_ops_enabled: true),
-          security_scan(:sast, configured: true, auto_dev_ops_enabled: true),
-          security_scan(:container_scanning, configured: false, auto_dev_ops_enabled: true),
-          security_scan(:dependency_scanning, configured: false, auto_dev_ops_enabled: true),
-          security_scan(:license_scanning, configured: false, auto_dev_ops_enabled: true),
-          security_scan(:secret_detection, configured: true, auto_dev_ops_enabled: true),
-          security_scan(:coverage_fuzzing, configured: false, auto_dev_ops_enabled: true),
-          security_scan(:api_fuzzing, configured: false, auto_dev_ops_enabled: true)
+          security_scan(:dast, configured: true),
+          security_scan(:sast, configured: true),
+          security_scan(:container_scanning, configured: false),
+          security_scan(:dependency_scanning, configured: false),
+          security_scan(:license_scanning, configured: false),
+          security_scan(:secret_detection, configured: true),
+          security_scan(:coverage_fuzzing, configured: false),
+          security_scan(:api_fuzzing, configured: false),
+          security_scan(:dast_profiles, configured: true)
         )
       end
     end
@@ -105,14 +105,14 @@ RSpec.describe Projects::Security::ConfigurationPresenter do
       it 'reports all security jobs as unconfigured' do
         expect(Gitlab::Json.parse(subject[:features])).to contain_exactly(
           security_scan(:dast, configured: false),
-          security_scan(:dast_profiles, configured: true),
           security_scan(:sast, configured: false),
           security_scan(:container_scanning, configured: false),
           security_scan(:dependency_scanning, configured: false),
           security_scan(:license_scanning, configured: false),
           security_scan(:secret_detection, configured: false),
           security_scan(:coverage_fuzzing, configured: false),
-          security_scan(:api_fuzzing, configured: false)
+          security_scan(:api_fuzzing, configured: false),
+          security_scan(:dast_profiles, configured: true)
         )
       end
     end
@@ -254,15 +254,12 @@ RSpec.describe Projects::Security::ConfigurationPresenter do
     end
   end
 
-  def security_scan(type, configured:, auto_dev_ops_enabled: false)
+  def security_scan(type, configured:)
     configuration_path = configuration_path(type)
 
-    status_str = scan_status(type, configured, auto_dev_ops_enabled)
-
     {
       "type" => type.to_s,
       "configured" => configured,
-      "status" => status_str,
       "configuration_path" => configuration_path
     }
   end
@@ -274,16 +271,4 @@ RSpec.describe Projects::Security::ConfigurationPresenter do
       api_fuzzing: project_security_configuration_api_fuzzing_path(project)
     }[type]
   end
-
-  def scan_status(type, configured, auto_dev_ops_enabled)
-    if type == :dast_profiles
-      "Available for on-demand DAST"
-    elsif configured && auto_dev_ops_enabled
-      "Enabled with Auto DevOps"
-    elsif configured
-      "Enabled"
-    else
-      "Not enabled"
-    end
-  end
 end

spec/frontend/vue_shared/components/lib/utils/props_utils_spec.js

+import { propsUnion } from '~/vue_shared/components/lib/utils/props_utils';
+
+describe('propsUnion', () => {
+  const stringRequired = {
+    type: String,
+    required: true,
+  };
+
+  const stringOptional = {
+    type: String,
+    required: false,
+  };
+
+  const numberOptional = {
+    type: Number,
+    required: false,
+  };
+
+  const booleanRequired = {
+    type: Boolean,
+    required: true,
+  };
+
+  const FooComponent = {
+    props: { foo: stringRequired },
+  };
+
+  const BarComponent = {
+    props: { bar: numberOptional },
+  };
+
+  const FooBarComponent = {
+    props: {
+      foo: stringRequired,
+      bar: numberOptional,
+    },
+  };
+
+  const FooOptionalComponent = {
+    props: {
+      foo: stringOptional,
+    },
+  };
+
+  const QuxComponent = {
+    props: {
+      foo: booleanRequired,
+      qux: stringRequired,
+    },
+  };
+
+  it('returns an empty object given no components', () => {
+    expect(propsUnion([])).toEqual({});
+  });
+
+  it('merges non-overlapping props', () => {
+    expect(propsUnion([FooComponent, BarComponent])).toEqual({
+      ...FooComponent.props,
+      ...BarComponent.props,
+    });
+  });
+
+  it('merges overlapping props', () => {
+    expect(propsUnion([FooComponent, BarComponent, FooBarComponent])).toEqual({
+      ...FooComponent.props,
+      ...BarComponent.props,
+      ...FooBarComponent.props,
+    });
+  });
+
+  it.each`
+    components
+    ${[FooComponent, FooOptionalComponent]}
+    ${[FooOptionalComponent, FooComponent]}
+  `('prefers required props over non-required props', ({ components }) => {
+    expect(propsUnion(components)).toEqual(FooComponent.props);
+  });
+
+  it('throws if given props with conflicting types', () => {
+    expect(() => propsUnion([FooComponent, QuxComponent])).toThrow(/incompatible prop types/);
+  });
+
+  it.each`
+    components
+    ${[{ props: ['foo', 'bar'] }]}
+    ${[{ props: { foo: String, bar: Number } }]}
+    ${[{ props: { foo: {}, bar: {} } }]}
+  `('throw if given a non-verbose props object', ({ components }) => {
+    expect(() => propsUnion(components)).toThrow(/expected verbose prop/);
+  });
+});