Split up yaml and humanized policy previews

The policy preview tabs were confusing for the users. For that reason,
they are split up. The right column now displays only the yaml preview
and the humanized policy preview is displayed at the bottom of the page.

Changelog: changed
EE: true

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 ""