Merge branch '341932-update-policy-drawer-summary' into 'master'

Update policy summary to be more readable

See merge request gitlab-org/gitlab!81048

ee/app/assets/javascripts/threat_monitoring/components/policy_drawer/constants.js

@@ -11,3 +11,5 @@ export const NOT_ENABLED_LABEL = s__('SecurityOrchestration|Not enabled');
 export const TYPE_TITLE = s__('SecurityOrchestration|Policy Type');
 
 export const STATUS_TITLE = s__('SecurityOrchestration|Status');
+
+export const SUMMARY_TITLE = s__('SecurityOrchestration|Summary');

ee/app/assets/javascripts/threat_monitoring/components/policy_drawer/scan_execution_policy.vue

 <script>
+import { GlSprintf } from '@gitlab/ui';
 import { s__ } from '~/locale';
 import {
   fromYaml,
   humanizeActions,
   humanizeRules,
 } from '../policy_editor/scan_execution_policy/lib';
+import { SUMMARY_TITLE } from './constants';
 import PolicyDrawerLayout from './policy_drawer_layout.vue';
 import PolicyInfoRow from './policy_info_row.vue';
 
 export default {
   i18n: {
-    action: s__('SecurityOrchestration|Action'),
-    rule: s__('SecurityOrchestration|Rule'),
+    multipleActionMessage: s__('SecurityOrchestration|Runs %{actions} and %{lastAction} scans'),
+    noActionMessage: s__('SecurityOrchestration|No actions defined - policy will not run.'),
+    singleActionMessage: s__(`SecurityOrchestration|Runs a %{action} scan`),
     scanExecution: s__('SecurityOrchestration|Scan execution'),
+    summary: SUMMARY_TITLE,
   },
   components: {
+    GlSprintf,
     PolicyDrawerLayout,
     PolicyInfoRow,
   },
@@ -38,6 +43,21 @@ export default {
         return null;
       }
     },
+    hasOnlyOneAction() {
+      return this.humanizedActions.length === 1;
+    },
+    hasMultipleActions() {
+      return this.humanizedActions.length > 1;
+    },
+    firstAction() {
+      return this.hasOnlyOneAction ? this.humanizedActions[0] : '';
+    },
+    allButLastActions() {
+      return this.hasMultipleActions ? this.humanizedActions.slice(0, -1).join(', ') : '';
+    },
+    lastAction() {
+      return this.hasMultipleActions ? [...this.humanizedActions].pop() : '';
+    },
   },
 };
 </script>
@@ -50,12 +70,28 @@ export default {
     :type="$options.i18n.scanExecution"
   >
     <template v-if="parsedYaml" #summary>
-      <policy-info-row data-testid="policy-rules" :label="$options.i18n.rule">
-        <p v-for="rule in humanizedRules" :key="rule">{{ rule }}</p>
-      </policy-info-row>
-
-      <policy-info-row data-testid="policy-actions" :label="$options.i18n.action">
-        <p v-for="action in humanizedActions" :key="action">{{ action }}</p>
+      <policy-info-row data-testid="policy-summary" :label="$options.i18n.summary">
+        <p>
+          <template v-if="!humanizedActions.length">{{ $options.i18n.noActionMessage }}</template>
+          <gl-sprintf v-else-if="hasOnlyOneAction" :message="$options.i18n.singleActionMessage">
+            <template #action>
+              <strong>{{ firstAction }}</strong>
+            </template>
+          </gl-sprintf>
+          <gl-sprintf v-else :message="$options.i18n.multipleActionMessage">
+            <template #actions>
+              <strong>{{ allButLastActions }}</strong>
+            </template>
+            <template #lastAction>
+              <strong>{{ lastAction }}</strong>
+            </template>
+          </gl-sprintf>
+        </p>
+        <ul>
+          <li v-for="(rule, idx) in humanizedRules" :key="idx">
+            {{ rule }}
+          </li>
+        </ul>
       </policy-info-row>
     </template>
   </policy-drawer-layout>

ee/app/assets/javascripts/threat_monitoring/components/policy_drawer/scan_result_policy.vue

 <script>
 import { s__ } from '~/locale';
 import { fromYaml, humanizeRules, humanizeAction } from '../policy_editor/scan_result_policy/lib';
+import { SUMMARY_TITLE } from './constants';
 import PolicyDrawerLayout from './policy_drawer_layout.vue';
 import PolicyInfoRow from './policy_info_row.vue';
 
 export default {
   i18n: {
-    action: s__('SecurityOrchestration|Action'),
-    rule: s__('SecurityOrchestration|Rule'),
+    summary: SUMMARY_TITLE,
     scanResult: s__('SecurityOrchestration|Scan result'),
   },
   components: {
@@ -51,7 +51,7 @@ export default {
     :type="$options.i18n.scanResult"
   >
     <template v-if="parsedYaml" #summary>
-      <policy-info-row data-testid="policy-rules" :label="$options.i18n.rule">
+      <policy-info-row data-testid="policy-summary" :label="$options.i18n.summary">
         <p>{{ humanizedAction }}</p>
         <ul>
           <li v-for="(rule, idx) in humanizedRules" :key="idx">

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/scan_execution_policy/lib/humanize.js

 import cronstrue from 'cronstrue/i18n';
-import { convertToTitleCase, humanize } from '~/lib/utils/text_utility';
 import { getPreferredLocales, sprintf, s__, n__ } from '~/locale';
 import { NO_RULE_MESSAGE } from '../../constants';
-
-const getActionText = (scanType) =>
-  sprintf(s__('SecurityOrchestration|Executes a %{scanType} scan'), {
-    scanType: convertToTitleCase(humanize(scanType)),
-  });
+import { convertScannersToTitleCase } from '../../utils';
 
 /**
  * Create a human-readable list of strings, adding the necessary punctuation and conjunctions
@@ -67,10 +62,11 @@ const HUMANIZE_RULES_METHODS = {
 /**
  * Create a human-readable version of the actions
  * @param {Array} actions [{"scan":"dast","scanner_profile":"Scanner Profile","site_profile":"Site Profile"},{"type":"secret_detection"}]
- * @returns {Set}
+ * @returns {Array}
  */
 export const humanizeActions = (actions) => {
-  return new Set(actions.map((action) => getActionText(action.scan)));
+  const scanners = actions.map((a) => a.scan);
+  return [...new Set(convertScannersToTitleCase(scanners))];
 };
 
 /**

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/scan_result_policy/lib/humanize.js

 import { sprintf, s__, n__ } from '~/locale';
 import { NO_RULE_MESSAGE } from '../../constants';
+import { convertScannersToTitleCase } from '../../utils';
 
 /**
  * Simple logic for indefinite articles which does not include the exceptions
@@ -171,7 +172,7 @@ const humanizeRule = (rule) => {
     ),
     {
       scanners: humanizeItems({
-        items: rule.scanners,
+        items: convertScannersToTitleCase(rule.scanners),
         singular: s__('SecurityOrchestration|scanner finds'),
         plural: s__('SecurityOrchestration|scanners find'),
       }),

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/utils.js

+import { convertToTitleCase, humanize } from '~/lib/utils/text_utility';
 import createPolicyProject from 'ee/threat_monitoring/graphql/mutations/create_policy_project.mutation.graphql';
 import createScanExecutionPolicy from 'ee/threat_monitoring/graphql/mutations/create_scan_execution_policy.mutation.graphql';
 import { gqClient } from 'ee/threat_monitoring/utils';
@@ -123,3 +124,11 @@ export const assignSecurityPolicyProject = async (projectPath) => {
 
   return { ...project, branch: project?.branch?.rootRef, errors };
 };
+
+/**
+ * Converts scanner strings to title case
+ * @param {Array} scanners (e.g. 'container_scanning', `dast`, etcetera)
+ * @returns {Array} (e.g. 'Container Scanning', `Dast`, etcetera)
+ */
+export const convertScannersToTitleCase = (scanners = []) =>
+  scanners.map((scanner) => convertToTitleCase(humanize(scanner)));

ee/spec/frontend/threat_monitoring/components/policy_drawer/__snapshots__/scan_execution_policy_spec.js.snap

+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`ScanExecutionPolicy component default policy renders the correct policy action message 1`] = `
+<div
+  data-testid="policy-summary"
+>
+  <div
+    data-testid="policy-summary"
+  >
+    <h5
+      class="gl-mt-6"
+      data-testid="label"
+    >
+      Summary
+    </h5>
+     
+    <p
+      data-testid="content"
+    >
+      <p>
+        Runs a 
+        <strong>
+          Dast
+        </strong>
+         scan
+      </p>
+       
+      <ul>
+        <li>
+          
+          Scan to be performed on every pipeline on the main branch
+        
+        </li>
+      </ul>
+    </p>
+  </div>
+</div>
+`;
+
+exports[`ScanExecutionPolicy component multiple action policy renders the correct policy action message 1`] = `
+<div
+  data-testid="policy-summary"
+>
+  <div
+    data-testid="policy-summary"
+  >
+    <h5
+      class="gl-mt-6"
+      data-testid="label"
+    >
+      Summary
+    </h5>
+     
+    <p
+      data-testid="content"
+    >
+      <p>
+        Runs 
+        <strong>
+          Container Scanning, Secret Detection
+        </strong>
+         and 
+        <strong>
+          Sast
+        </strong>
+         scans
+      </p>
+       
+      <ul>
+        <li>
+          
+          Scan to be performed on every pipeline on the main branch
+        
+        </li>
+      </ul>
+    </p>
+  </div>
+</div>
+`;
+
+exports[`ScanExecutionPolicy component no action policy renders the correct policy action message 1`] = `
+<div
+  data-testid="policy-summary"
+>
+  <div
+    data-testid="policy-summary"
+  >
+    <h5
+      class="gl-mt-6"
+      data-testid="label"
+    >
+      Summary
+    </h5>
+     
+    <p
+      data-testid="content"
+    >
+      <p>
+        No actions defined - policy will not run.
+      </p>
+       
+      <ul>
+        <li>
+          
+          Scan to be performed on every pipeline on the main branch
+        
+        </li>
+      </ul>
+    </p>
+  </div>
+</div>
+`;

ee/spec/frontend/threat_monitoring/components/policy_drawer/scan_execution_policy_spec.js

-import PolicyDrawerLayout from 'ee/threat_monitoring/components/policy_drawer/policy_drawer_layout.vue';
 import ScanExecutionPolicy from 'ee/threat_monitoring/components/policy_drawer/scan_execution_policy.vue';
-import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
-import { mockScanExecutionPolicy } from '../../mocks/mock_data';
+import { mountExtended } from 'helpers/vue_test_utils_helper';
+import {
+  mockScanExecutionPolicy,
+  mockScanExecutionManifestNoActions,
+  mockScanExecutionManifestMultipleActions,
+} from '../../mocks/mock_data';
 
 describe('ScanExecutionPolicy component', () => {
   let wrapper;
 
-  const findActions = () => wrapper.findByTestId('policy-actions');
-  const findRules = () => wrapper.findByTestId('policy-rules');
+  const findSummary = () => wrapper.findByTestId('policy-summary');
 
   const factory = ({ propsData } = {}) => {
-    wrapper = shallowMountExtended(ScanExecutionPolicy, {
+    wrapper = mountExtended(ScanExecutionPolicy, {
       propsData,
-      stubs: {
-        PolicyDrawerLayout,
-      },
     });
   };
 
@@ -22,21 +21,18 @@ describe('ScanExecutionPolicy component', () => {
     wrapper.destroy();
   });
 
-  describe('default policy', () => {
+  describe.each`
+    title                       | propsData
+    ${'default policy'}         | ${{ policy: mockScanExecutionPolicy }}
+    ${'no action policy'}       | ${{ policy: { ...mockScanExecutionPolicy, yaml: mockScanExecutionManifestNoActions } }}
+    ${'multiple action policy'} | ${{ policy: { ...mockScanExecutionPolicy, yaml: mockScanExecutionManifestMultipleActions } }}
+  `('$title', ({ propsData }) => {
     beforeEach(() => {
-      factory({ propsData: { policy: mockScanExecutionPolicy } });
+      factory({ propsData });
     });
 
-    it.each`
-      component    | finder         | text
-      ${'actions'} | ${findActions} | ${''}
-      ${'rules'}   | ${findRules}   | ${''}
-    `('does render the policy $component', ({ finder, text }) => {
-      const component = finder();
-      expect(component.exists()).toBe(true);
-      if (text) {
-        expect(component.text()).toBe(text);
-      }
+    it('renders the correct policy action message', () => {
+      expect(findSummary().element).toMatchSnapshot();
     });
   });
 });

ee/spec/frontend/threat_monitoring/components/policy_drawer/scan_result_policy_spec.js

@@ -6,7 +6,7 @@ import { mockScanResultPolicy } from '../../mocks/mock_data';
 describe('ScanResultPolicy component', () => {
   let wrapper;
 
-  const findRules = () => wrapper.findByTestId('policy-rules');
+  const findSummary = () => wrapper.findByTestId('policy-summary');
 
   const factory = ({ propsData } = {}) => {
     wrapper = shallowMountExtended(ScanResultPolicy, {
@@ -26,8 +26,8 @@ describe('ScanResultPolicy component', () => {
       factory({ propsData: { policy: mockScanResultPolicy } });
     });
 
-    it('does render the policy rules', () => {
-      expect(findRules().exists()).toBe(true);
+    it('does render the policy summary', () => {
+      expect(findSummary().exists()).toBe(true);
     });
   });
 });

ee/spec/frontend/threat_monitoring/components/policy_editor/scan_execution_policy/lib/humanize_spec.js

@@ -17,6 +17,7 @@ const mockActions = [
   { scan: 'dast', scanner_profile: 'Scanner Profile', site_profile: 'Site Profile' },
   { scan: 'dast', scanner_profile: 'Scanner Profile 01', site_profile: 'Site Profile 01' },
   { scan: 'secret_detection' },
+  { scan: 'container_image_scanning' },
 ];
 
 const mockRules = [
@@ -33,17 +34,19 @@ const mockRules = [
 
 describe('humanizeActions', () => {
   it('returns an empty Array of actions as an empty Set', () => {
-    expect(humanizeActions([])).toStrictEqual(new Set());
+    expect(humanizeActions([])).toStrictEqual([]);
   });
 
   it('returns a single action as human-readable string', () => {
-    expect(humanizeActions([mockActions[0]])).toStrictEqual(new Set(['Executes a Dast scan']));
+    expect(humanizeActions([mockActions[0]])).toStrictEqual(['Dast']);
   });
 
   it('returns multiple actions as human-readable strings', () => {
-    expect(humanizeActions(mockActions)).toStrictEqual(
-      new Set(['Executes a Dast scan', 'Executes a Secret Detection scan']),
-    );
+    expect(humanizeActions(mockActions)).toStrictEqual([
+      'Dast',
+      'Secret Detection',
+      'Container Image Scanning',
+    ]);
   });
 });
 

ee/spec/frontend/threat_monitoring/components/policy_editor/scan_result_policy/lib/humanize_spec.js

@@ -59,6 +59,11 @@ const mockRules = [
   },
 ];
 
+const mockRulesHumanized = [
+  'The Sast scanner finds a critical vulnerability in an open merge request targeting the main branch.',
+  'The Dast or Sast scanners find info or critical vulnerabilities in an open merge request targeting the master or main branches.',
+];
+
 const mockRulesEmptyBranch = {
   type: 'scan_finding',
   branches: [],
@@ -74,21 +79,16 @@ describe('humanizeRules', () => {
   });
 
   it('returns a single rule as a human-readable string for user approvers only', () => {
-    expect(humanizeRules([mockRules[0]])).toStrictEqual([
-      'The sast scanner finds a critical vulnerability in an open merge request targeting the main branch.',
-    ]);
+    expect(humanizeRules([mockRules[0]])).toStrictEqual([mockRulesHumanized[0]]);
   });
 
   it('returns multiple rules with different number of branches as human-readable strings', () => {
-    expect(humanizeRules(mockRules)).toStrictEqual([
-      'The sast scanner finds a critical vulnerability in an open merge request targeting the main branch.',
-      'The dast or sast scanners find info or critical vulnerabilities in an open merge request targeting the master or main branches.',
-    ]);
+    expect(humanizeRules(mockRules)).toStrictEqual(mockRulesHumanized);
   });
 
   it('returns a single rule as a human-readable string for all branches', () => {
     expect(humanizeRules([mockRulesEmptyBranch])).toStrictEqual([
-      'The sast scanner finds a critical vulnerability in an open merge request targeting all branches.',
+      'The Sast scanner finds a critical vulnerability in an open merge request targeting all branches.',
     ]);
   });
 });

ee/spec/frontend/threat_monitoring/components/policy_editor/scan_execution_policy/lib/utils_spec.js → ee/spec/frontend/threat_monitoring/components/policy_editor/utils_spec.js

 import {
   assignSecurityPolicyProject,
   modifyPolicy,
+  convertScannersToTitleCase,
 } from 'ee/threat_monitoring/components/policy_editor/utils';
 import { DEFAULT_ASSIGNED_POLICY_PROJECT } from 'ee/threat_monitoring/constants';
 import createPolicyProject from 'ee/threat_monitoring/graphql/mutations/create_policy_project.mutation.graphql';
@@ -102,3 +103,14 @@ describe('modifyPolicy', () => {
     await expect(modifyPolicy(createSavePolicyInput())).rejects.toThrowError(error);
   });
 });
+
+describe('convertScannersToTitleCase', () => {
+  it.each`
+    title                                            | input                                                 | output
+    ${'returns empty array if no imput is provided'} | ${undefined}                                          | ${[]}
+    ${'returns empty array for an empty array'}      | ${[]}                                                 | ${[]}
+    ${'returns converted array'}                     | ${['dast', 'container_scanning', 'secret_detection']} | ${['Dast', 'Container Scanning', 'Secret Detection']}
+  `('$title', ({ input, output }) => {
+    expect(convertScannersToTitleCase(input)).toStrictEqual(output);
+  });
+});

ee/spec/frontend/threat_monitoring/mocks/mock_data.js

@@ -49,6 +49,17 @@ export const mockEnvironmentsResponse = {
   stopped_count: 5,
 };
 
+export const mockScanExecutionManifestNoActions = `type: scan_execution_policy
+name: Test Dast
+description: This policy enforces pipeline configuration to have a job with DAST scan
+enabled: false
+rules:
+  - type: pipeline
+    branches:
+      - main
+actions: []
+`;
+
 export const mockDastScanExecutionManifest = `type: scan_execution_policy
 name: Test Dast
 description: This policy enforces pipeline configuration to have a job with DAST scan
@@ -63,6 +74,20 @@ actions:
     scanner_profile: required_scanner_profile
 `;
 
+export const mockScanExecutionManifestMultipleActions = `type: scan_execution_policy
+name: Test Dast
+description: This policy enforces pipeline configuration to have a job with DAST scan
+enabled: false
+rules:
+  - type: pipeline
+    branches:
+      - main
+actions:
+  - scan: container_scanning
+  - scan: secret_detection
+  - scan: sast
+`;
+
 export const mockDastScanExecutionObject = {
   type: 'scan_execution_policy',
   name: 'Test Dast',

locale/gitlab.pot

@@ -32488,9 +32488,6 @@ msgstr ""
 msgid "SecurityOrchestration|.yaml preview"
 msgstr ""
 
-msgid "SecurityOrchestration|Action"
-msgstr ""
-
 msgid "SecurityOrchestration|Actions"
 msgstr ""
 
@@ -32533,9 +32530,6 @@ msgstr ""
 msgid "SecurityOrchestration|Enforce security for this project. %{linkStart}More information.%{linkEnd}"
 msgstr ""
 
-msgid "SecurityOrchestration|Executes a %{scanType} scan"
-msgstr ""
-
 msgid "SecurityOrchestration|If you are using Auto DevOps, your %{monospacedStart}auto-deploy-values.yaml%{monospacedEnd} file will not be updated if you change a policy in this section. Auto DevOps users should make changes by following the %{linkStart}Container Network Policy documentation%{linkEnd}."
 msgstr ""
 
@@ -32551,6 +32545,9 @@ msgstr ""
 msgid "SecurityOrchestration|New policy"
 msgstr ""
 
+msgid "SecurityOrchestration|No actions defined - policy will not run."
+msgstr ""
+
 msgid "SecurityOrchestration|No description"
 msgstr ""
 
@@ -32593,10 +32590,13 @@ msgstr ""
 msgid "SecurityOrchestration|Require %{approvals} %{plural} from %{approvers} if any of the following occur:"
 msgstr ""
 
-msgid "SecurityOrchestration|Rule"
+msgid "SecurityOrchestration|Rules"
 msgstr ""
 
-msgid "SecurityOrchestration|Rules"
+msgid "SecurityOrchestration|Runs %{actions} and %{lastAction} scans"
+msgstr ""
+
+msgid "SecurityOrchestration|Runs a %{action} scan"
 msgstr ""
 
 msgid "SecurityOrchestration|Scan Execution"