Merge branch '345612-split-up-rule-mode' into 'master'

Split up yaml and humanized policy previews

See merge request gitlab-org/gitlab!75386

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/network_policy/network_policy_editor.vue

@@ -7,6 +7,9 @@ import {
   GlToggle,
   GlButton,
   GlAlert,
+  GlIcon,
+  GlCollapse,
+  GlCollapseToggleDirective,
 } from '@gitlab/ui';
 import { mapState, mapActions } from 'vuex';
 import { removeUnnecessaryDashes } from 'ee/threat_monitoring/utils';
@@ -17,7 +20,7 @@ import DimDisableContainer from '../dim_disable_container.vue';
 import PolicyActionPicker from '../policy_action_picker.vue';
 import PolicyAlertPicker from '../policy_alert_picker.vue';
 import PolicyEditorLayout from '../policy_editor_layout.vue';
-import PolicyPreview from '../policy_preview.vue';
+import PolicyPreviewHuman from '../policy_preview_human.vue';
 import {
   DEFAULT_NETWORK_POLICY,
   RuleTypeEndpoint,
@@ -45,12 +48,14 @@ export default {
       'NetworkPolicies|Network Policies can be used to limit which network traffic is allowed between containers inside the cluster.',
     ),
     noEnvironmentButton: __('Learn more'),
+    yamlPreview: s__('SecurityOrchestration|.yaml preview'),
     policyPreview: s__('SecurityOrchestration|Policy preview'),
     rules: s__('SecurityOrchestration|Rules'),
     saveButtonTooltip: s__(
       'NetworkPolicies|Network policy can be created after the environment is loaded successfully.',
     ),
   },
+  policyPreviewHumanCollapseId: 'policy-preview-human-collapse',
   components: {
     GlEmptyState,
     GlFormGroup,
@@ -59,13 +64,18 @@ export default {
     GlToggle,
     GlButton,
     GlAlert,
+    GlIcon,
+    GlCollapse,
     PolicyRuleBuilder,
-    PolicyPreview,
     PolicyActionPicker,
     PolicyAlertPicker,
     PolicyEditorLayout,
+    PolicyPreviewHuman,
     DimDisableContainer,
   },
+  directives: {
+    CollapseToggle: GlCollapseToggleDirective,
+  },
   inject: [
     'networkDocumentationPath',
     'policyEditorEmptyStateSvgPath',
@@ -95,6 +105,7 @@ export default {
       : '';
 
     return {
+      isPolicyPreviewHumanVisible: true,
       yamlEditorValue,
       yamlEditorError: policy.error ? true : null,
       policy,
@@ -107,9 +118,6 @@ export default {
     humanizedPolicy() {
       return this.policy.error ? null : humanizeNetworkPolicy(this.policy);
     },
-    initialTab() {
-      return this.hasParsingError ? 1 : 0;
-    },
     policyAlert() {
       return Boolean(this.policy.annotations);
     },
@@ -219,7 +227,12 @@ export default {
     @update-yaml="updateYaml"
   >
     <template #rule-editor>
-      <gl-alert v-if="hasParsingError" data-testid="parsing-alert" :dismissible="false">
+      <gl-alert
+        v-if="hasParsingError"
+        data-testid="parsing-alert"
+        class="gl-mb-5"
+        :dismissible="false"
+      >
         {{ $options.i18n.PARSING_ERROR_MESSAGE }}
       </gl-alert>
 
@@ -290,17 +303,31 @@ export default {
       </dim-disable-container>
     </template>
     <template #rule-editor-preview>
-      <dim-disable-container data-testid="policy-preview-container" :disabled="hasParsingError">
-        <template #title>
-          <h5>{{ $options.i18n.policyPreview }}</h5>
-        </template>
-
-        <template #disabled>
-          <policy-preview :initial-tab="initialTab" :policy-yaml="yamlEditorValue" />
+      <h5>{{ $options.i18n.yamlPreview }}</h5>
+      <pre
+        data-testid="yaml-preview"
+        class="gl-bg-white gl-border-none gl-p-0"
+        :class="{ 'gl-opacity-5': hasParsingError }"
+        >{{ policyYaml || yamlEditorValue }}</pre
+      >
     </template>
-
-        <policy-preview :policy-yaml="policyYaml" :policy-description="humanizedPolicy" />
-      </dim-disable-container>
+    <template v-if="!hasParsingError" #bottom>
+      <div class="gl-my-6">
+        <gl-button
+          v-collapse-toggle="$options.policyPreviewHumanCollapseId"
+          category="tertiary"
+          class="gl-font-weight-bold gl-bg-transparent! gl-px-0!"
+        >
+          {{ $options.i18n.policyPreview }}
+          <gl-icon :name="isPolicyPreviewHumanVisible ? 'angle-up' : 'angle-down'" :size="12" />
+        </gl-button>
+        <gl-collapse
+          :id="$options.policyPreviewHumanCollapseId"
+          v-model="isPolicyPreviewHumanVisible"
+        >
+          <policy-preview-human class="gl-md-w-70p" :policy-description="humanizedPolicy" />
+        </gl-collapse>
+      </div>
     </template>
   </policy-editor-layout>
   <gl-empty-state

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/policy_editor_layout.vue

@@ -160,6 +160,7 @@ export default {
         </section>
       </div>
     </div>
+    <slot name="bottom"></slot>
     <span
       v-gl-tooltip.hover.focus="{ disabled: disableTooltip }"
       class="gl-pt-2"

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/policy_preview.vue

 <script>
-import { GlAlert, GlTabs, GlTab, GlSafeHtmlDirective } from '@gitlab/ui';
+import { GlTabs, GlTab } from '@gitlab/ui';
 import { PARSING_ERROR_MESSAGE } from './constants';
+import PolicyPreviewHuman from './policy_preview_human.vue';
 
 export default {
   i18n: {
     PARSING_ERROR_MESSAGE,
   },
   components: {
-    GlAlert,
     GlTabs,
     GlTab,
-  },
-  directives: {
-    safeHtml: GlSafeHtmlDirective,
+    PolicyPreviewHuman,
   },
   props: {
     policyYaml: {
@@ -33,23 +31,20 @@ export default {
   data() {
     return { selectedTab: this.initialTab };
   },
-  safeHtmlConfig: { ALLOWED_TAGS: ['strong', 'br'] },
 };
 </script>
 
 <template>
   <gl-tabs v-model="selectedTab" content-class="gl-pt-0">
     <gl-tab :title="s__('NetworkPolicies|Rule')">
-      <div
-        v-if="policyDescription"
-        v-safe-html:[$options.safeHtmlConfig]="policyDescription"
-        class="gl-bg-white gl-rounded-top-left-none gl-rounded-top-right-none gl-rounded-bottom-left-base gl-rounded-bottom-right-base gl-py-3 gl-px-4 gl-border-1 gl-border-solid gl-border-gray-100 gl-border-t-none!"
-      ></div>
-      <div v-else>
-        <gl-alert variant="info" :dismissible="false">
-          {{ $options.i18n.PARSING_ERROR_MESSAGE }}
-        </gl-alert>
-      </div>
+      <policy-preview-human
+        :class="{
+          'gl-border-t-none! gl-rounded-top-left-none gl-rounded-top-right-none': Boolean(
+            policyDescription,
+          ),
+        }"
+        :policy-description="policyDescription"
+      />
     </gl-tab>
     <gl-tab :title="s__('NetworkPolicies|.yaml')">
       <pre class="gl-bg-white gl-rounded-top-left-none gl-rounded-top-right-none gl-border-t-none"

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/policy_preview_human.vue

+<script>
+import { GlAlert, GlSafeHtmlDirective } from '@gitlab/ui';
+import { PARSING_ERROR_MESSAGE } from './constants';
+
+export default {
+  i18n: {
+    PARSING_ERROR_MESSAGE,
+  },
+  components: {
+    GlAlert,
+  },
+  directives: {
+    safeHtml: GlSafeHtmlDirective,
+  },
+  props: {
+    policyDescription: {
+      type: String,
+      required: false,
+      default: '',
+    },
+  },
+  safeHtmlConfig: { ALLOWED_TAGS: ['strong', 'br'] },
+};
+</script>
+
+<template>
+  <div
+    v-if="policyDescription"
+    v-safe-html:[$options.safeHtmlConfig]="policyDescription"
+    class="gl-bg-white gl-rounded-base gl-py-3 gl-px-4 gl-border-1 gl-border-solid gl-border-gray-100"
+  ></div>
+  <div v-else>
+    <gl-alert variant="info" :dismissible="false">
+      {{ $options.i18n.PARSING_ERROR_MESSAGE }}
+    </gl-alert>
+  </div>
+</template>

ee/spec/frontend/threat_monitoring/components/policy_editor/__snapshots__/policy_preview_spec.js.snap

@@ -11,15 +11,10 @@ exports[`PolicyPreview component with policy description renders policy preview
     title="Rule"
     titlelinkclass=""
   >
-    <div
-      class="gl-bg-white gl-rounded-top-left-none gl-rounded-top-right-none gl-rounded-bottom-left-base gl-rounded-bottom-right-base gl-py-3 gl-px-4 gl-border-1 gl-border-solid gl-border-gray-100 gl-border-t-none!"
-    >
-      <strong>
-        bar
-      </strong>
-      <br />
-      test
-    </div>
+    <policy-preview-human-stub
+      class="gl-border-t-none! gl-rounded-top-left-none gl-rounded-top-right-none"
+      policydescription="<strong>bar</strong><br><div>test</div><script></script>"
+    />
   </gl-tab-stub>
    
   <gl-tab-stub

ee/spec/frontend/threat_monitoring/components/policy_editor/network_policy/network_policy_editor_spec.js

@@ -13,9 +13,11 @@ import NetworkPolicyEditor from 'ee/threat_monitoring/components/policy_editor/n
 import PolicyRuleBuilder from 'ee/threat_monitoring/components/policy_editor/network_policy/policy_rule_builder.vue';
 import PolicyAlertPicker from 'ee/threat_monitoring/components/policy_editor/policy_alert_picker.vue';
 import PolicyEditorLayout from 'ee/threat_monitoring/components/policy_editor/policy_editor_layout.vue';
-import PolicyPreview from 'ee/threat_monitoring/components/policy_editor/policy_preview.vue';
+import PolicyPreviewHuman from 'ee/threat_monitoring/components/policy_editor/policy_preview_human.vue';
 import createStore from 'ee/threat_monitoring/store';
-import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
+import { shallowMountExtended, mountExtended } from 'helpers/vue_test_utils_helper';
+import waitForPromises from 'helpers/wait_for_promises';
+import { stubTransition } from 'helpers/stub_transition';
 import { redirectTo } from '~/lib/utils/url_utility';
 import { mockExistingL3Policy, mockExistingL7Policy } from '../../../mocks/mock_data';
 
@@ -29,7 +31,12 @@ describe('NetworkPolicyEditor component', () => {
     threatMonitoring: { environments: [{ id: 1 }], currentEnvironmentId: 1, hasEnvironment: true },
   };
 
-  const factory = ({ propsData, provide = {}, updatedStore = defaultStore } = {}) => {
+  const factory = ({
+    propsData,
+    provide = {},
+    updatedStore = defaultStore,
+    mountFn = shallowMountExtended,
+  } = {}) => {
     store = createStore();
 
     store.replaceState({
@@ -46,7 +53,7 @@ describe('NetworkPolicyEditor component', () => {
 
     jest.spyOn(store, 'dispatch').mockImplementation(() => Promise.resolve());
 
-    wrapper = shallowMountExtended(NetworkPolicyEditor, {
+    wrapper = mountFn(NetworkPolicyEditor, {
       propsData: {
         ...propsData,
       },
@@ -58,12 +65,13 @@ describe('NetworkPolicyEditor component', () => {
         ...provide,
       },
       store,
-      stubs: { DimDisableContainer, PolicyYamlEditor: true },
+      stubs: { DimDisableContainer, PolicyYamlEditor: true, transition: stubTransition() },
     });
   };
 
   const findEmptyState = () => wrapper.findComponent(GlEmptyState);
-  const findPreview = () => wrapper.findComponent(PolicyPreview);
+  const findYamlPreview = () => wrapper.findByTestId('yaml-preview');
+  const findHumanizedPreview = () => wrapper.findComponent(PolicyPreviewHuman);
   const findAddRuleButton = () => wrapper.findByTestId('add-rule');
   const findYAMLParsingAlert = () => wrapper.findByTestId('parsing-alert');
   const findPolicyAlertPicker = () => wrapper.findComponent(PolicyAlertPicker);
@@ -72,6 +80,10 @@ describe('NetworkPolicyEditor component', () => {
   const findPolicyName = () => wrapper.find("[id='policyName']");
   const findPolicyRuleBuilder = () => wrapper.findComponent(PolicyRuleBuilder);
   const findPolicyEditorLayout = () => wrapper.findComponent(PolicyEditorLayout);
+  const findCollapseToggle = () =>
+    wrapper.findByRole('button', {
+      name: NetworkPolicyEditor.i18n.policyPreview,
+    });
 
   const modifyPolicyAlert = async ({ isAlertEnabled }) => {
     const policyAlertPicker = findPolicyAlertPicker();
@@ -80,21 +92,19 @@ describe('NetworkPolicyEditor component', () => {
     await findPolicyEditorLayout().vm.$emit('save-policy');
   };
 
-  beforeEach(() => {
-    factory();
-  });
-
   afterEach(() => {
     wrapper.destroy();
   });
 
   it('renders toggle with label', () => {
+    factory();
     const policyEnableToggle = findPolicyEnableContainer().findComponent(GlToggle);
     expect(policyEnableToggle.exists()).toBe(true);
     expect(policyEnableToggle.props('label')).toBe(NetworkPolicyEditor.i18n.toggleLabel);
   });
 
   it('disables the tooltip and enables the save button', () => {
+    factory();
     expect(findPolicyEditorLayout().props()).toMatchObject({
       disableTooltip: true,
       disableUpdate: false,
@@ -102,6 +112,7 @@ describe('NetworkPolicyEditor component', () => {
   });
 
   it('renders a default rule with label', () => {
+    factory();
     expect(wrapper.findAllComponents(PolicyRuleBuilder)).toHaveLength(1);
     expect(findPolicyRuleBuilder().exists()).toBe(true);
     expect(findPolicyRuleBuilder().attributes()).toMatchObject({
@@ -115,14 +126,16 @@ describe('NetworkPolicyEditor component', () => {
     ${'policy alert picker'}        | ${'does display'}     | ${findPolicyAlertPicker} | ${true}
     ${'policy name input'}          | ${'does display'}     | ${findPolicyName}        | ${true}
     ${'add rule button'}            | ${'does display'}     | ${findAddRuleButton}     | ${true}
-    ${'policy preview'}             | ${'does display'}     | ${findPreview}           | ${true}
+    ${'yaml policy preview'}        | ${'does display'}     | ${findYamlPreview}       | ${true}
     ${'parsing error alert'}        | ${'does not display'} | ${findYAMLParsingAlert}  | ${false}
     ${'no environment empty state'} | ${'does not display'} | ${findEmptyState}        | ${false}
   `('$status the $component', async ({ findComponent, state }) => {
+    factory();
     expect(findComponent().exists()).toBe(state);
   });
 
   it('updates policy on yaml editor value change', async () => {
+    factory();
     findPolicyEditorLayout().vm.$emit('update-yaml', mockExistingL3Policy.manifest);
 
     expect(wrapper.vm.policy).toMatchObject({
@@ -142,18 +155,37 @@ describe('NetworkPolicyEditor component', () => {
   });
 
   it('given there is a name change, updates policy yaml preview', async () => {
-    const initialValue = findPreview().props('policyYaml');
+    factory();
+    const initialValue = findYamlPreview().text();
     await findPolicyName().vm.$emit('input', 'new');
-    expect(findPreview().props('policyYaml')).not.toEqual(initialValue);
+    expect(findYamlPreview().text()).not.toEqual(initialValue);
   });
 
   it('given there is a rule change, updates policy description preview', async () => {
-    const initialValue = findPreview().props('policyDescription');
+    factory();
+    const initialValue = findHumanizedPreview().props('policyDescription');
     await findAddRuleButton().vm.$emit('click');
-    expect(findPreview().props('policyDescription')).not.toEqual(initialValue);
+    expect(findHumanizedPreview().props('policyDescription')).not.toEqual(initialValue);
+  });
+
+  it('toggles the visibility for the humanized policy preview', async () => {
+    factory({ mountFn: mountExtended });
+
+    // Wait for `requestAnimationFrame` in Bootstrap Vue toggle directive
+    // https://github.com/bootstrap-vue/bootstrap-vue/blob/f86c32a7d8a3c0403c9a9421850ce3c97f0ad638/src/directives/toggle/toggle.js#L219
+    await waitForPromises();
+
+    expect(findHumanizedPreview().isVisible()).toBe(true);
+    expect(findCollapseToggle().attributes('aria-expanded')).toBe('true');
+
+    await findCollapseToggle().trigger('click');
+
+    expect(findHumanizedPreview().isVisible()).toBe(false);
+    expect(findCollapseToggle().attributes('aria-expanded')).toBe('false');
   });
 
   it('adds a new rule', async () => {
+    factory();
     expect(wrapper.findAllComponents(PolicyRuleBuilder)).toHaveLength(1);
     const button = findAddRuleButton();
     await button.vm.$emit('click');
@@ -174,6 +206,7 @@ describe('NetworkPolicyEditor component', () => {
   });
 
   it('removes a new rule', async () => {
+    factory();
     await findAddRuleButton().vm.$emit('click');
     expect(wrapper.findAllComponents(PolicyRuleBuilder)).toHaveLength(2);
 
@@ -182,6 +215,7 @@ describe('NetworkPolicyEditor component', () => {
   });
 
   it('updates yaml editor value on switch to yaml editor', async () => {
+    factory();
     const policyEditorLayout = findPolicyEditorLayout();
     findPolicyName().vm.$emit('input', 'test-policy');
     await policyEditorLayout.vm.$emit('update-editor-mode', EDITOR_MODE_YAML);
@@ -207,6 +241,10 @@ describe('NetworkPolicyEditor component', () => {
       expect(findPolicyEnableContainer().attributes().disabled).toBe('true');
     });
 
+    it('does not display the humanized policy preview', () => {
+      expect(findHumanizedPreview().exists()).toBe(false);
+    });
+
     it('renders parsing error alert', () => {
       expect(findYAMLParsingAlert().exists()).toBe(true);
     });
@@ -219,12 +257,10 @@ describe('NetworkPolicyEditor component', () => {
       expect(wrapper.findByTestId('policy-action-container').props().disabled).toBe(true);
     });
 
-    it('disables policy preview and sets initial tab to yaml', () => {
-      expect(wrapper.findByTestId('policy-preview-container').props().disabled).toBe(true);
-      expect(findPreview().props()).toMatchObject({
-        initialTab: 1,
-        policyYaml: mockExistingL7Policy.manifest,
-      });
+    it('displays the manifest text and decreases the opacity to show it is disabled', () => {
+      const preview = findYamlPreview();
+      expect(preview.attributes('class')).toContain('gl-opacity-5');
+      expect(preview.text()).toBe(mockExistingL7Policy.manifest);
     });
 
     it('does not update yaml editor value on switch to yaml editor', async () => {
@@ -257,6 +293,7 @@ describe('NetworkPolicyEditor component', () => {
   });
 
   it('creates policy and redirects to a threat monitoring path', async () => {
+    factory();
     await findPolicyEditorLayout().vm.$emit('save-policy');
     expect(store.dispatch).toHaveBeenCalledWith('networkPolicies/createPolicy', {
       environmentId: 1,
@@ -335,6 +372,10 @@ describe('NetworkPolicyEditor component', () => {
   });
 
   describe('add alert picker', () => {
+    beforeEach(() => {
+      factory();
+    });
+
     it('adds a policy annotation on alert addition', async () => {
       await modifyPolicyAlert({ isAlertEnabled: true });
       expect(store.dispatch).toHaveBeenLastCalledWith('networkPolicies/createPolicy', {

ee/spec/frontend/threat_monitoring/components/policy_editor/policy_preview_human_spec.js

+import { GlAlert } from '@gitlab/ui';
+import { shallowMount } from '@vue/test-utils';
+import PolicyPreviewHuman from 'ee/threat_monitoring/components/policy_editor/policy_preview_human.vue';
+
+describe('PolicyPreviewHuman component', () => {
+  let wrapper;
+
+  const findAlert = () => wrapper.findComponent(GlAlert);
+
+  const factory = ({ propsData } = {}) => {
+    wrapper = shallowMount(PolicyPreviewHuman, {
+      propsData: {
+        ...propsData,
+      },
+    });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('when policy description is not defined', () => {
+    beforeEach(() => {
+      factory();
+    });
+
+    it('renders an alert when policyDescription is not defined', () => {
+      expect(findAlert().exists()).toBe(true);
+    });
+  });
+
+  describe('when policy description is defined', () => {
+    const policyDescription = '<strong>bar</strong><br><div>test</div><script></script>';
+    const policyDescriptionSafe = '<strong>bar</strong><br>test';
+
+    beforeEach(() => {
+      factory({
+        propsData: {
+          policyDescription,
+        },
+      });
+    });
+
+    it('renders the policyDescription when it is defined', () => {
+      expect(wrapper.html()).toContain(policyDescriptionSafe);
+    });
+
+    it('does not render the alert component', () => {
+      expect(findAlert().exists()).toBe(false);
+    });
+  });
+});

ee/spec/frontend/threat_monitoring/components/policy_editor/policy_preview_spec.js

-import { GlAlert, GlTabs } from '@gitlab/ui';
+import { GlTabs } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
+import PolicyPreviewHuman from 'ee/threat_monitoring/components/policy_editor/policy_preview_human.vue';
 import PolicyPreview from 'ee/threat_monitoring/components/policy_editor/policy_preview.vue';
 
 describe('PolicyPreview component', () => {
   let wrapper;
 
-  const findAlert = () => wrapper.findComponent(GlAlert);
   const findTabs = () => wrapper.findComponent(GlTabs);
+  const findPolicyPreviewHuman = () => wrapper.findComponent(PolicyPreviewHuman);
 
   const factory = ({ propsData } = {}) => {
     wrapper = shallowMount(PolicyPreview, {
@@ -21,11 +22,13 @@ describe('PolicyPreview component', () => {
   });
 
   describe('with policy description', () => {
+    const policyDescription = '<strong>bar</strong><br><div>test</div><script></script>';
+
     beforeEach(() => {
       factory({
         propsData: {
           policyYaml: 'foo',
-          policyDescription: '<strong>bar</strong><br><div>test</div><script></script>',
+          policyDescription,
         },
       });
     });
@@ -34,12 +37,12 @@ describe('PolicyPreview component', () => {
       expect(findTabs().element).toMatchSnapshot();
     });
 
-    it('renders the first tab', () => {
-      expect(findTabs().attributes().value).toEqual('0');
+    it('renders the policy preview human', () => {
+      expect(findPolicyPreviewHuman().props('policyDescription')).toBe(policyDescription);
     });
 
-    it('does not render the unsupported attributes alert', () => {
-      expect(findAlert().exists()).toBe(false);
+    it('renders the first tab', () => {
+      expect(findTabs().attributes().value).toEqual('0');
     });
 
     describe('initial tab', () => {
@@ -64,9 +67,5 @@ describe('PolicyPreview component', () => {
         },
       });
     });
-
-    it('does render the unsupported attributes alert', () => {
-      expect(findAlert().exists()).toBe(true);
-    });
   });
 });

locale/gitlab.pot

@@ -31064,6 +31064,9 @@ msgstr ""
 msgid "SecurityOrchestration|%{branches} and %{lastBranch} %{plural}"
 msgstr ""
 
+msgid "SecurityOrchestration|.yaml preview"
+msgstr ""
+
 msgid "SecurityOrchestration|Action"
 msgstr ""