Merge branch '340339-policy-editor-performance' into 'master'

Fix policy editor performance

See merge request gitlab-org/gitlab!70024

ee/app/assets/javascripts/threat_monitoring/components/policies/policies_app.vue

 <script>
 import { mapActions } from 'vuex';
+import { isValidEnvironmentId } from '../../utils';
 import PoliciesHeader from './policies_header.vue';
 import PoliciesList from './policies_list.vue';
 
@@ -15,7 +16,7 @@ export default {
       // An invalid default environment id means there there are no available
       // environments, therefore infrastructure cannot be set up. A valid default
       // environment id only means that infrastructure *might* be set up.
-      shouldFetchEnvironment: this.isValidEnvironmentId(this.defaultEnvironmentId),
+      shouldFetchEnvironment: isValidEnvironmentId(this.defaultEnvironmentId),
       shouldUpdatePolicyList: false,
     };
   },
@@ -27,10 +28,6 @@ export default {
   },
   methods: {
     ...mapActions('threatMonitoring', ['fetchEnvironments', 'setCurrentEnvironmentId']),
-
-    isValidEnvironmentId(id) {
-      return Number.isInteger(id) && id >= 0;
-    },
     handleUpdatePolicyList(val) {
       this.shouldUpdatePolicyList = val;
     },

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

@@ -4,7 +4,6 @@ import {
   GlFormGroup,
   GlFormInput,
   GlFormTextarea,
-  GlLoadingIcon,
   GlToggle,
   GlButton,
   GlAlert,
@@ -48,13 +47,15 @@ export default {
     noEnvironmentButton: __('Learn more'),
     policyPreview: s__('SecurityOrchestration|Policy preview'),
     rules: s__('SecurityOrchestration|Rules'),
+    saveButtonTooltip: s__(
+      'NetworkPolicies|Network policy can be created after the environment is loaded successfully.',
+    ),
   },
   components: {
     GlEmptyState,
     GlFormGroup,
     GlFormInput,
     GlFormTextarea,
-    GlLoadingIcon,
     GlToggle,
     GlButton,
     GlAlert,
@@ -65,7 +66,13 @@ export default {
     PolicyEditorLayout,
     DimDisableContainer,
   },
-  inject: ['networkDocumentationPath', 'noEnvironmentSvgPath', 'projectId', 'threatMonitoringPath'],
+  inject: [
+    'hasEnvironment',
+    'networkDocumentationPath',
+    'noEnvironmentSvgPath',
+    'projectId',
+    'threatMonitoringPath',
+  ],
   props: {
     existingPolicy: {
       type: Object,
@@ -95,8 +102,8 @@ export default {
     };
   },
   computed: {
-    hasEnvironment() {
-      return Boolean(this.environments.length);
+    customSaveTooltipText() {
+      return !this.retrievedEnvironments ? this.$options.i18n.saveButtonTooltip : '';
     },
     humanizedPolicy() {
       return this.policy.error ? null : humanizeNetworkPolicy(this.policy);
@@ -110,6 +117,9 @@ export default {
     policyYaml() {
       return this.hasParsingError ? '' : toYaml(this.policy);
     },
+    retrievedEnvironments() {
+      return !this.isLoadingEnvironments && Boolean(this.environments.length);
+    },
     ...mapState('threatMonitoring', [
       'currentEnvironmentId',
       'environments',
@@ -193,9 +203,11 @@ export default {
 </script>
 
 <template>
-  <gl-loading-icon v-if="isLoadingEnvironments" size="lg" />
   <policy-editor-layout
-    v-else-if="hasEnvironment"
+    v-if="hasEnvironment"
+    :custom-save-tooltip-text="customSaveTooltipText"
+    :disable-tooltip="retrievedEnvironments"
+    :disable-update="!retrievedEnvironments"
     :is-editing="isEditing"
     :is-removing-policy="isRemovingPolicy"
     :is-updating-policy="isUpdatingPolicy"

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

 <script>
-import { GlButton, GlFormGroup, GlModal, GlModalDirective, GlSegmentedControl } from '@gitlab/ui';
+import {
+  GlButton,
+  GlFormGroup,
+  GlModal,
+  GlModalDirective,
+  GlSegmentedControl,
+  GlTooltipDirective,
+} from '@gitlab/ui';
 import { s__, sprintf } from '~/locale';
 import { DELETE_MODAL_CONFIG, EDITOR_MODES, EDITOR_MODE_RULE, EDITOR_MODE_YAML } from './constants';
 
@@ -15,14 +22,29 @@ export default {
     PolicyYamlEditor: () =>
       import(/* webpackChunkName: 'policy_yaml_editor' */ '../policy_yaml_editor.vue'),
   },
-  directives: { GlModal: GlModalDirective },
+  directives: { GlModal: GlModalDirective, GlTooltip: GlTooltipDirective },
   inject: ['threatMonitoringPath'],
   props: {
+    customSaveButtonText: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    customSaveTooltipText: {
+      type: String,
+      required: false,
+      default: '',
+    },
     defaultEditorMode: {
       type: String,
       required: false,
       default: EDITOR_MODE_RULE,
     },
+    disableTooltip: {
+      type: Boolean,
+      required: false,
+      default: true,
+    },
     disableUpdate: {
       type: Boolean,
       required: false,
@@ -68,10 +90,16 @@ export default {
     deleteModalTitle() {
       return sprintf(s__('NetworkPolicies|Delete policy: %{policy}'), { policy: this.policyName });
     },
+    saveTooltipText() {
+      return this.customSaveTooltipText || this.saveButtonText;
+    },
     saveButtonText() {
-      return this.isEditing
-        ? s__('NetworkPolicies|Save changes')
-        : s__('NetworkPolicies|Create policy');
+      return (
+        this.customSaveButtonText ||
+        (this.isEditing
+          ? s__('NetworkPolicies|Save changes')
+          : s__('NetworkPolicies|Create policy'))
+      );
     },
     shouldShowRuleEditor() {
       return this.selectedEditorMode === EDITOR_MODE_RULE;
@@ -132,18 +160,23 @@ export default {
         </section>
       </div>
     </div>
-    <gl-button
-      type="submit"
-      variant="success"
-      data-testid="save-policy"
-      :loading="isUpdatingPolicy"
-      :disabled="disableUpdate"
-      @click="savePolicy"
+    <span
+      v-gl-tooltip.hover.focus="{ disabled: disableTooltip }"
+      class="gl-pt-2"
+      :title="saveTooltipText"
+      data-testid="save-policy-tooltip"
     >
-      <slot name="save-button-text">
+      <gl-button
+        type="submit"
+        variant="success"
+        data-testid="save-policy"
+        :loading="isUpdatingPolicy"
+        :disabled="disableUpdate"
+        @click="savePolicy"
+      >
         {{ saveButtonText }}
-      </slot>
-    </gl-button>
+      </gl-button>
+    </span>
     <gl-button
       v-if="isEditing"
       v-gl-modal="'delete-modal'"

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/scan_execution_policy/scan_execution_policy_editor.vue

@@ -110,6 +110,7 @@ export default {
 
 <template>
   <policy-editor-layout
+    :custom-save-button-text="$options.i18n.createMergeRequest"
     :default-editor-mode="$options.DEFAULT_EDITOR_MODE"
     :disable-update="disableScanExecutionUpdate"
     :editor-modes="$options.EDITOR_MODES"
@@ -121,9 +122,5 @@ export default {
     @remove-policy="handleModifyPolicy($options.SECURITY_POLICY_ACTIONS.REMOVE)"
     @save-policy="handleModifyPolicy()"
     @update-yaml="updateYaml"
-  >
-    <template #save-button-text>
-      {{ $options.i18n.createMergeRequest }}
-    </template>
-  </policy-editor-layout>
+  />
 </template>

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

@@ -4,7 +4,7 @@ import { convertObjectPropsToCamelCase, parseBoolean } from '~/lib/utils/common_
 import PolicyEditorApp from './components/policy_editor/policy_editor.vue';
 import { DEFAULT_ASSIGNED_POLICY_PROJECT } from './constants';
 import createStore from './store';
-import { gqClient } from './utils';
+import { gqClient, isValidEnvironmentId } from './utils';
 
 Vue.use(VueApollo);
 
@@ -16,6 +16,7 @@ export default () => {
   const el = document.querySelector('#js-policy-builder-app');
   const {
     assignedPolicyProject,
+    defaultEnvironmentId,
     disableScanExecutionUpdate,
     environmentsEndpoint,
     configureAgentHelpPath,
@@ -64,6 +65,7 @@ export default () => {
       noEnvironmentSvgPath,
       projectId,
       projectPath,
+      hasEnvironment: isValidEnvironmentId(parseInt(defaultEnvironmentId, 10)),
       threatMonitoringPath,
     },
     store,

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

@@ -28,6 +28,15 @@ export const getPolicyType = (typeName = '') => {
   return null;
 };
 
+/**
+ * Checks if an environment id is valid
+ * @param {Number} id environment id
+ * @returns {Boolean}
+ */
+export const isValidEnvironmentId = (id) => {
+  return Number.isInteger(id) && id >= 0;
+};
+
 /**
  * Removes inital line dashes from a policy YAML that is received from the API, which
  * is not required for the user.

ee/app/helpers/projects/security/policies_helper.rb

@@ -22,6 +22,7 @@ module Projects::Security::PoliciesHelper
 
     {
       assigned_policy_project: assigned_policy_project(project).to_json,
+      default_environment_id: project.default_environment&.id || -1,
       disable_scan_execution_update: disable_scan_execution_update.to_s,
       network_policies_endpoint: project_security_network_policies_path(project),
       configure_agent_help_path: help_page_url('user/clusters/agent/repository.html'),

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

-import { GlEmptyState, GlLoadingIcon, GlToggle } from '@gitlab/ui';
+import { GlEmptyState, GlToggle } from '@gitlab/ui';
 import { EDITOR_MODE_YAML } from 'ee/threat_monitoring/components/policy_editor/constants';
 import DimDisableContainer from 'ee/threat_monitoring/components/policy_editor/dim_disable_container.vue';
 import {
@@ -46,10 +46,10 @@ describe('NetworkPolicyEditor component', () => {
 
     wrapper = shallowMountExtended(NetworkPolicyEditor, {
       propsData: {
-        hasEnvironment: true,
         ...propsData,
       },
       provide: {
+        hasEnvironment: true,
         networkDocumentationPath: 'path/to/docs',
         noEnvironmentSvgPath: 'path/to/svg',
         threatMonitoringPath: '/threat-monitoring',
@@ -62,7 +62,6 @@ describe('NetworkPolicyEditor component', () => {
   };
 
   const findEmptyState = () => wrapper.findComponent(GlEmptyState);
-  const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon);
   const findPreview = () => wrapper.findComponent(PolicyPreview);
   const findAddRuleButton = () => wrapper.findByTestId('add-rule');
   const findYAMLParsingAlert = () => wrapper.findByTestId('parsing-alert');
@@ -94,6 +93,13 @@ describe('NetworkPolicyEditor component', () => {
     expect(policyEnableToggle.props('label')).toBe(NetworkPolicyEditor.i18n.toggleLabel);
   });
 
+  it('disables the tooltip and enables the save button', () => {
+    expect(findPolicyEditorLayout().props()).toMatchObject({
+      disableTooltip: true,
+      disableUpdate: false,
+    });
+  });
+
   it('renders a default rule with label', () => {
     expect(wrapper.findAllComponents(PolicyRuleBuilder)).toHaveLength(1);
     expect(findPolicyRuleBuilder().exists()).toBe(true);
@@ -110,7 +116,6 @@ describe('NetworkPolicyEditor component', () => {
     ${'add rule button'}            | ${'does display'}     | ${findAddRuleButton}     | ${true}
     ${'policy preview'}             | ${'does display'}     | ${findPreview}           | ${true}
     ${'parsing error alert'}        | ${'does not display'} | ${findYAMLParsingAlert}  | ${false}
-    ${'loading icon'}               | ${'does not display'} | ${findLoadingIcon}       | ${false}
     ${'no environment empty state'} | ${'does not display'} | ${findEmptyState}        | ${false}
   `('$status the $component', async ({ findComponent, state }) => {
     expect(findComponent().exists()).toBe(state);
@@ -356,24 +361,29 @@ describe('NetworkPolicyEditor component', () => {
         updatedStore: { threatMonitoring: { environments: [], isLoadingEnvironments: true } },
       });
     });
-    it.each`
-      component                       | status                | findComponent             | state
-      ${'loading icon'}               | ${'does display'}     | ${findLoadingIcon}        | ${true}
-      ${'policy editor layout'}       | ${'does not display'} | ${findPolicyEditorLayout} | ${false}
-      ${'no environment empty state'} | ${'does not display'} | ${findEmptyState}         | ${false}
-    `('$status the $component', ({ findComponent, state }) => {
-      expect(findComponent().exists()).toBe(state);
+
+    it('does not display the "no environment" empty state', () => {
+      expect(findEmptyState().exists()).toBe(false);
+    });
+
+    it('displays the "PolicyEditorLayout" component enables the tooltip and disables the save button', () => {
+      expect(findPolicyEditorLayout().props()).toMatchObject({
+        disableTooltip: false,
+        disableUpdate: true,
+      });
     });
   });
 
   describe('when no environments are configured', () => {
     beforeEach(() => {
-      factory({ updatedStore: { threatMonitoring: { environments: [] } } });
+      factory({
+        provide: { hasEnvironment: false },
+        updatedStore: { threatMonitoring: { environments: [] } },
+      });
     });
 
     it.each`
       component                       | status                | findComponent             | state
-      ${'loading icon'}               | ${'does display'}     | ${findLoadingIcon}        | ${false}
       ${'policy editor layout'}       | ${'does not display'} | ${findPolicyEditorLayout} | ${false}
       ${'no environment empty state'} | ${'does not display'} | ${findEmptyState}         | ${true}
     `('$status the $component', ({ findComponent, state }) => {

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

@@ -5,10 +5,15 @@ import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
 
 describe('PolicyEditorLayout component', () => {
   let wrapper;
+  let glTooltipDirectiveMock;
   const threatMonitoringPath = '/threat-monitoring';
 
   const factory = ({ propsData = {} } = {}) => {
+    glTooltipDirectiveMock = jest.fn();
     wrapper = shallowMountExtended(PolicyEditorLayout, {
+      directives: {
+        GlTooltip: glTooltipDirectiveMock,
+      },
       propsData: {
         ...propsData,
       },
@@ -44,6 +49,10 @@ describe('PolicyEditorLayout component', () => {
       expect(findComponent().exists()).toBe(state);
     });
 
+    it('disables the save button tooltip', async () => {
+      expect(glTooltipDirectiveMock.mock.calls[0][1].value.disabled).toBe(true);
+    });
+
     it('does display the correct save button text when creating a new policy', () => {
       const saveButton = findSavePolicyButton();
       expect(saveButton.exists()).toBe(true);
@@ -124,11 +133,25 @@ describe('PolicyEditorLayout component', () => {
     });
   });
 
-  describe('disabled actions', () => {
-    it('disables the save button', async () => {
+  describe('custom behavior', () => {
+    it('displays the custom save button text when it is passed in', async () => {
+      const customSaveButtonText = 'Custom Text';
+      factory({ propsData: { customSaveButtonText } });
+      expect(findSavePolicyButton().exists()).toBe(true);
+      expect(findSavePolicyButton().text()).toBe(customSaveButtonText);
+    });
+
+    it('disables the save button when "disableUpdate" is true', async () => {
       factory({ propsData: { disableUpdate: true } });
       expect(findSavePolicyButton().exists()).toBe(true);
       expect(findSavePolicyButton().attributes('disabled')).toBe('true');
     });
+
+    it('enables the save button tooltip when "disableTooltip" is false', async () => {
+      const customSaveTooltipText = 'Custom Test';
+      factory({ propsData: { customSaveTooltipText, disableTooltip: false } });
+      expect(glTooltipDirectiveMock.mock.calls[0][1].value.disabled).toBe(false);
+      expect(glTooltipDirectiveMock.mock.calls[0][0].title).toBe(customSaveTooltipText);
+    });
   });
 });

ee/spec/frontend/threat_monitoring/utils_spec.js

@@ -3,6 +3,7 @@ import { POLICY_TYPE_COMPONENT_OPTIONS } from 'ee/threat_monitoring/components/c
 import {
   getContentWrapperHeight,
   getPolicyType,
+  isValidEnvironmentId,
   removeUnnecessaryDashes,
 } from 'ee/threat_monitoring/utils';
 import { setHTMLFixture } from 'helpers/fixtures';
@@ -44,6 +45,19 @@ describe('Threat Monitoring Utils', () => {
     });
   });
 
+  describe('isValidEnvironmentId', () => {
+    it.each`
+      input        | output
+      ${-1}        | ${false}
+      ${undefined} | ${false}
+      ${'0'}       | ${false}
+      ${0}         | ${true}
+      ${1}         | ${true}
+    `('returns $output when used on $input', ({ input, output }) => {
+      expect(isValidEnvironmentId(input)).toBe(output);
+    });
+  });
+
   describe('removeUnnecessaryDashes', () => {
     it.each`
       input          | output

ee/spec/helpers/projects/security/policies_helper_spec.rb

@@ -39,6 +39,7 @@ RSpec.describe Projects::Security::PoliciesHelper do
     let(:base_data) do
       {
         assigned_policy_project: "null",
+        default_environment_id: -1,
         disable_scan_execution_update: "false",
         network_policies_endpoint: kind_of(String),
         configure_agent_help_path: kind_of(String),
@@ -84,7 +85,7 @@ RSpec.describe Projects::Security::PoliciesHelper do
         )
       end
 
-      it { is_expected.to match(base_data) }
+      it { is_expected.to match(base_data.merge(default_environment_id: project.default_environment.id)) }
     end
   end
 end

locale/gitlab.pot

@@ -22337,6 +22337,9 @@ msgstr ""
 msgid "NetworkPolicies|Network Policies can be used to limit which network traffic is allowed between containers inside the cluster."
 msgstr ""
 
+msgid "NetworkPolicies|Network policy can be created after the environment is loaded successfully."
+msgstr ""
+
 msgid "NetworkPolicies|Network traffic"
 msgstr ""