From e55b70e10a0f3a7d4c0db6610021220dab248cc7 Mon Sep 17 00:00:00 2001
From: Gerardo <gerardo@b310.de>
Date: Mon, 5 Jun 2023 15:41:58 +0200
Subject: [PATCH 1/6] feat: Protected packages: Create protection rules in
 project setting ui

MR includes:
- Add form for creating new package protection rule
- Refetch list of protection rules after submission

Changelog: added
---
 .../packages_protection_rule_form.vue         | 191 ++++++++++
 .../components/packages_protection_rules.vue  |  33 ++
 ..._packages_protection_rule.mutation.graphql |  11 +
 locale/gitlab.pot                             |  12 +
 .../packages_protection_rule_form_spec.js     | 327 ++++++++++++++++++
 .../packages_protection_rules_spec.js         |  81 +++++
 .../settings/project/settings/mock_data.js    |  22 ++
 7 files changed, 677 insertions(+)
 create mode 100644 app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
 create mode 100644 app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/create_packages_protection_rule.mutation.graphql
 create mode 100644 spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js

diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
new file mode 100644
index 00000000000000..3b4a4ed26f1c94
--- /dev/null
+++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
@@ -0,0 +1,191 @@
+<script>
+import { GlAlert, GlButton, GlFormGroup, GlForm, GlFormInput, GlFormSelect } from '@gitlab/ui';
+import createPackagesProtectionRuleMutation from '~/packages_and_registries/settings/project/graphql/mutations/create_packages_protection_rule.mutation.graphql';
+// import packagesProtectionRuleQuery from '~/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql';
+import { s__ } from '~/locale';
+
+const PACKAGES_PROTECTION_RULES_SAVED_SUCCESS_MESSAGE = s__(
+  'PackageRegistry|Rule saved successfully',
+);
+const PACKAGES_PROTECTION_RULES_SAVED_ERROR_MESSAGE = s__(
+  'PackageRegistry|Something went wrong while saving the package protection rule',
+);
+
+export default {
+  components: {
+    GlButton,
+    GlFormInput,
+    GlFormSelect,
+    GlFormGroup,
+    GlAlert,
+    GlForm,
+  },
+  inject: ['projectPath'],
+  props: {
+    value: {
+      type: Object,
+      required: false,
+      default: () => {},
+    },
+    isLoading: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  i18n: {
+    PACKAGES_PROTECTION_RULES_SAVED_SUCCESS_MESSAGE,
+    PACKAGES_PROTECTION_RULES_SAVED_ERROR_MESSAGE,
+  },
+  data() {
+    return {
+      packageProtectionRuleFormData: {
+        packageNamePattern: '',
+        packageType: 'NPM',
+        pushProtectedUpToAccessLevel: 'MAINTAINER',
+        ...this.value,
+      },
+      mutationLoading: false,
+      alertErrorMessage: '',
+    };
+  },
+  computed: {
+    showLoadingIcon() {
+      return this.isLoading || this.mutationLoading;
+    },
+    isSubmitButtonDisabled() {
+      return (
+        this.packageProtectionRuleFormData.packageNamePattern?.trim()?.length <= 0 ||
+        this.showLoadingIcon
+      );
+    },
+    isFieldDisabled() {
+      return this.showLoadingIcon;
+    },
+    createPackagesProtectionRuleMutationInput() {
+      return {
+        projectPath: this.projectPath,
+        packageNamePattern: this.packageProtectionRuleFormData.packageNamePattern,
+        packageType: this.packageProtectionRuleFormData.packageType,
+        pushProtectedUpToAccessLevel: this.packageProtectionRuleFormData
+          .pushProtectedUpToAccessLevel,
+      };
+    },
+    packageTypeOptions() {
+      return [{ value: 'NPM', text: 'NPM' }];
+    },
+    pushProtectedUpToAccessLevelOptions() {
+      return [
+        { value: 'MAINTAINER', text: 'MAINTAINER' },
+        { value: 'OWNER', text: 'OWNER' },
+      ];
+    },
+  },
+  methods: {
+    submit() {
+      this.clearAlertErrorMessage();
+
+      this.mutationLoading = true;
+      return this.$apollo
+        .mutate({
+          mutation: createPackagesProtectionRuleMutation,
+          variables: {
+            input: this.createPackagesProtectionRuleMutationInput,
+          },
+        })
+        .then(({ data }) => {
+          const [errorMessage] = data?.createPackagesProtectionRule?.errors ?? [];
+          if (errorMessage) {
+            this.alertErrorMessage = errorMessage;
+            return;
+          }
+
+          this.clearAlertErrorMessage();
+
+          this.$emit('submit', data.createPackagesProtectionRule.packageProtectionRule);
+        })
+        .catch(() => {
+          this.alertErrorMessage = PACKAGES_PROTECTION_RULES_SAVED_ERROR_MESSAGE;
+        })
+        .finally(() => {
+          this.mutationLoading = false;
+        });
+    },
+    clearAlertErrorMessage() {
+      this.alertErrorMessage = null;
+    },
+    cancelForm() {
+      this.clearAlertErrorMessage();
+      this.$emit('cancel');
+    },
+  },
+};
+</script>
+
+<template>
+  <div class="gl-new-card-add-form gl-m-3">
+    <gl-form @submit.prevent="submit" @reset="cancelForm">
+      <gl-alert v-if="alertErrorMessage" class="mb-3" variant="danger" :dismissible="false">
+        {{ alertErrorMessage }}
+      </gl-alert>
+
+      <gl-form-group
+        id="input-group-package-name-pattern"
+        label="Package Name Pattern:"
+        label-for="input-package-name-pattern"
+      >
+        <gl-form-input
+          id="input-package-name-pattern"
+          v-model="packageProtectionRuleFormData.packageNamePattern"
+          type="text"
+          required
+          :placeholder="__(`Enter package name pattern`)"
+          :disabled="isFieldDisabled"
+          pattern=".{1,}"
+        />
+      </gl-form-group>
+
+      <gl-form-group
+        id="input-group-package-type"
+        label="Package Type:"
+        label-for="input-package-type"
+        :disabled="isFieldDisabled"
+      >
+        <gl-form-select
+          id="input-package-type"
+          v-model="packageProtectionRuleFormData.packageType"
+          :disabled="isFieldDisabled"
+          :options="packageTypeOptions"
+          required
+        />
+      </gl-form-group>
+
+      <gl-form-group
+        id="input-group-push-protected-up-to-access-level"
+        label="Push Protected Up To Access Level:"
+        label-for="input-push-protected-up-to-access-level"
+        :disabled="isFieldDisabled"
+      >
+        <gl-form-select
+          id="input-push-protected-up-to-access-level"
+          v-model="packageProtectionRuleFormData.pushProtectedUpToAccessLevel"
+          :options="pushProtectedUpToAccessLevelOptions"
+          :disabled="isFieldDisabled"
+          required
+        />
+      </gl-form-group>
+
+      <div class="gl-display-flex gl-justify-content-start">
+        <gl-button
+          data-testid="submit-button"
+          variant="confirm"
+          type="submit"
+          :disabled="isSubmitButtonDisabled"
+          :loading="showLoadingIcon"
+          >{{ __('Protect') }}</gl-button
+        >
+        <gl-button class="gl-ml-3" type="reset">{{ __('Cancel') }}</gl-button>
+      </div>
+    </gl-form>
+  </div>
+</template>
diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue
index d917777880332d..c2e6a376367a4d 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue
+++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue
@@ -2,6 +2,7 @@
 import { GlCard, GlTable, GlLoadingIcon } from '@gitlab/ui';
 import packagesProtectionRuleQuery from '~/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql';
 import SettingsBlock from '~/packages_and_registries/shared/components/settings_block.vue';
+import PackagesProtectionRuleForm from '~/packages_and_registries/settings/project/components/packages_protection_rule_form.vue';
 import { s__ } from '~/locale';
 
 const PAGINATION_DEFAULT_PER_PAGE = 10;
@@ -12,6 +13,7 @@ export default {
     GlCard,
     GlTable,
     GlLoadingIcon,
+    PackagesProtectionRuleForm,
   },
   inject: ['projectPath'],
   i18n: {
@@ -24,6 +26,7 @@ export default {
     return {
       fetchSettingsError: false,
       packageProtectionRules: [],
+      protectionRuleFormVisibility: false,
     };
   },
   computed: {
@@ -43,6 +46,9 @@ export default {
     isLoadingPackageProtectionRules() {
       return this.$apollo.queries.packageProtectionRules.loading;
     },
+    isAddProtectionRuleButtonDisabled() {
+      return this.protectionRuleFormVisibility;
+    },
   },
   apollo: {
     packageProtectionRules: {
@@ -72,6 +78,18 @@ export default {
       label: s__('PackageRegistry|Push protected up to access level'),
     },
   ],
+  methods: {
+    showProtectionRuleForm() {
+      this.protectionRuleFormVisibility = true;
+    },
+    hideProtectionRuleForm() {
+      this.protectionRuleFormVisibility = false;
+    },
+    refetchProtectionRules() {
+      this.$apollo.queries.packageProtectionRules.refetch();
+      this.hideProtectionRuleForm();
+    },
+  },
 };
 </script>
 
@@ -92,10 +110,25 @@ export default {
         <template #header>
           <div class="gl-new-card-title-wrapper gl-justify-content-space-between">
             <h3 class="gl-new-card-title">{{ $options.i18n.settingBlockTitle }}</h3>
+            <div class="gl-new-card-actions">
+              <button
+                class="gl-button btn btn-sm btn-default"
+                :disabled="isAddProtectionRuleButtonDisabled"
+                @click="showProtectionRuleForm"
+              >
+                <span class="gl-button-text">{{ __('Add package protection rule') }}</span>
+              </button>
+            </div>
           </div>
         </template>
 
         <template #default>
+          <packages-protection-rule-form
+            v-if="protectionRuleFormVisibility"
+            @cancel="hideProtectionRuleForm"
+            @submit="refetchProtectionRules"
+          />
+
           <gl-table
             :items="tableItems"
             :fields="$options.fields"
diff --git a/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/create_packages_protection_rule.mutation.graphql b/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/create_packages_protection_rule.mutation.graphql
new file mode 100644
index 00000000000000..923b9d2e4b26dc
--- /dev/null
+++ b/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/create_packages_protection_rule.mutation.graphql
@@ -0,0 +1,11 @@
+mutation createPackagesProtectionRule($input: CreatePackagesProtectionRuleInput!) {
+  createPackagesProtectionRule(input: $input) {
+    packageProtectionRule {
+      id
+      packageNamePattern
+      packageType
+      pushProtectedUpToAccessLevel
+    }
+    errors
+  }
+}
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 79c9e718590d74..7f712fe04f4b5d 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3038,6 +3038,9 @@ msgstr ""
 msgid "Add or subtract spent time"
 msgstr ""
 
+msgid "Add package protection rule"
+msgstr ""
+
 msgid "Add people"
 msgstr ""
 
@@ -19034,6 +19037,9 @@ msgstr ""
 msgid "Enter one or more user ID separated by commas"
 msgstr ""
 
+msgid "Enter package name pattern"
+msgstr ""
+
 msgid "Enter the %{name} description"
 msgstr ""
 
@@ -34999,6 +35005,9 @@ msgstr ""
 msgid "PackageRegistry|RubyGems"
 msgstr ""
 
+msgid "PackageRegistry|Rule saved successfully"
+msgstr ""
+
 msgid "PackageRegistry|Show Composer commands"
 msgstr ""
 
@@ -35041,6 +35050,9 @@ msgstr ""
 msgid "PackageRegistry|Something went wrong while fetching the package metadata."
 msgstr ""
 
+msgid "PackageRegistry|Something went wrong while saving the package protection rule"
+msgstr ""
+
 msgid "PackageRegistry|Sorry, your filter produced no results"
 msgstr ""
 
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
new file mode 100644
index 00000000000000..60f2b5ab8f5b8b
--- /dev/null
+++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
@@ -0,0 +1,327 @@
+import VueApollo from 'vue-apollo';
+import Vue from 'vue';
+import { GlForm, GlAlert, GlFormInput } from '@gitlab/ui';
+import { mount } from '@vue/test-utils';
+import createMockApollo from 'helpers/mock_apollo_helper';
+import waitForPromises from 'helpers/wait_for_promises';
+import PackagesProtectionRuleForm from '~/packages_and_registries/settings/project/components/packages_protection_rule_form.vue';
+import createPackagesProtectionRuleMutation from '~/packages_and_registries/settings/project/graphql/mutations/create_packages_protection_rule.mutation.graphql';
+import {
+  createPackagesProtectionRuleMutationPayload,
+  createPackagesProtectionRuleMutationInput,
+  createPackagesProtectionRuleMutationPayloadErrors,
+} from '../mock_data';
+
+Vue.use(VueApollo);
+
+describe('Packages Protection Rule Form', () => {
+  let wrapper;
+  let fakeApollo;
+
+  const defaultProvidedValues = {
+    projectPath: 'path',
+  };
+
+  const defaultProps = { value: { ...createPackagesProtectionRuleMutationInput } };
+
+  const findForm = () => wrapper.findComponent(GlForm);
+  const findAlert = () => wrapper.findComponent(GlAlert);
+  const findPackageNamePatternInput = () => findForm().findComponent(GlFormInput);
+  const findPackageTypeSelect = () => findForm().find('select#input-package-type');
+  const findPushProtectedUpToAccessLevelSelect = () =>
+    findForm().find('select#input-push-protected-up-to-access-level');
+  const findSubmitButton = () => findForm().find('[data-testid="submit-button"]');
+
+  const mountComponent = ({
+    props = defaultProps,
+    data,
+    config,
+    provide = defaultProvidedValues,
+  } = {}) => {
+    wrapper = mount(PackagesProtectionRuleForm, {
+      propsData: { ...props },
+      provide,
+      data() {
+        return { ...data };
+      },
+      ...config,
+    });
+  };
+
+  const mountComponentWithApollo = ({
+    provide = defaultProvidedValues,
+    mutationResolver,
+    props = defaultProps,
+  } = {}) => {
+    const requestHandlers = [[createPackagesProtectionRuleMutation, mutationResolver]];
+
+    fakeApollo = createMockApollo(requestHandlers);
+
+    mountComponent({
+      provide,
+      props,
+      config: {
+        apolloProvider: fakeApollo,
+      },
+    });
+  };
+
+  describe('form fields', () => {
+    describe('form field "packageNamePattern"', () => {
+      it.each`
+        packageNamePattern     | expectedPackageNamePatternInputValue
+        ${'@scope/my-package'} | ${'@scope/my-package'}
+        ${''}                  | ${''}
+        ${' '}                 | ${' '}
+        ${undefined}           | ${''}
+        ${null}                | ${''}
+      `(
+        'handles given props (packageNamePattern: "$packageNamePattern") correctly',
+        ({ packageNamePattern, expectedPackageNamePatternInputValue }) => {
+          const props = { value: { ...defaultProps.value, packageNamePattern } };
+
+          mountComponent({ props });
+
+          expect(findPackageNamePatternInput().element.value).toBe(
+            expectedPackageNamePatternInputValue,
+          );
+        },
+      );
+    });
+
+    describe('form field "packageType"', () => {
+      it('contains only the options for npm', () => {
+        mountComponent();
+
+        expect(findPackageTypeSelect().exists()).toBe(true);
+        const packageTypeSelectOptions = findPackageTypeSelect()
+          .findAll('option')
+          .wrappers.map((option) => option.element.value);
+        expect(packageTypeSelectOptions).toEqual(['NPM']);
+      });
+
+      it.each`
+        packageType               | packageTypeSelectValue
+        ${'NPM'}                  | ${'NPM'}
+        ${'INVALID_PACKAGE_TYPE'} | ${''}
+        ${''}                     | ${''}
+        ${undefined}              | ${''}
+      `(
+        'handles given props (packageType: "$packageType") correctly',
+        ({ packageType, packageTypeSelectValue }) => {
+          const props = { value: { ...defaultProps.value, packageType } };
+
+          mountComponent({ props });
+
+          expect(findPackageTypeSelect().element.value).toBe(packageTypeSelectValue);
+        },
+      );
+    });
+
+    describe('form field "pushProtectedUpToAccessLevelSelect"', () => {
+      it('contains only the options for maintainer and owner', () => {
+        mountComponent();
+
+        expect(findPushProtectedUpToAccessLevelSelect().exists()).toBe(true);
+        const pushProtectedUpToAccessLevelSelectOptions = findPushProtectedUpToAccessLevelSelect()
+          .findAll('option')
+          .wrappers.map((option) => option.element.value);
+        expect(pushProtectedUpToAccessLevelSelectOptions).toEqual(['MAINTAINER', 'OWNER']);
+      });
+
+      it.each`
+        pushProtectedUpToAccessLevel | pushProtectedUpToAccessLevelSelectValue
+        ${'OWNER'}                   | ${'OWNER'}
+        ${'MAINTAINER'}              | ${'MAINTAINER'}
+        ${'INVALID_ACCESS_LEVEL'}    | ${''}
+        ${undefined}                 | ${''}
+        ${''}                        | ${''}
+      `(
+        'handles given props (pushProtectedUpToAccessLevel: "$pushProtectedUpToAccessLevel") correctly',
+        ({ pushProtectedUpToAccessLevel, pushProtectedUpToAccessLevelSelectValue }) => {
+          const props = { value: { ...defaultProps.value, pushProtectedUpToAccessLevel } };
+
+          mountComponent({ props });
+
+          expect(findPushProtectedUpToAccessLevelSelect().element.value).toBe(
+            pushProtectedUpToAccessLevelSelectValue,
+          );
+        },
+      );
+    });
+  });
+
+  describe('form actions', () => {
+    describe('button submit', () => {
+      it.each`
+        packageNamePattern       | submitButtonDisabled
+        ${''}                    | ${true}
+        ${'  '}                  | ${true}
+        ${'@scope/my-package'}   | ${false}
+        ${'@scope/my-package-*'} | ${false}
+      `(
+        'when packageNamePattern is "$packageNamePattern" then the disabled state of the submit button is $submitButtonDisabled',
+        async ({ packageNamePattern, submitButtonDisabled }) => {
+          mountComponent({ props: { value: { packageNamePattern: '' } } });
+
+          expect(findSubmitButton().exists()).toBe(true);
+          expect(findSubmitButton().props().disabled).toBe(true);
+
+          await findPackageNamePatternInput().setValue(packageNamePattern);
+
+          expect(findSubmitButton().exists()).toBe(true);
+          expect(findSubmitButton().props().disabled).toBe(submitButtonDisabled);
+        },
+      );
+
+      it.each`
+        isLoading | mutationLoading | disabled
+        ${true}   | ${true}         | ${true}
+        ${true}   | ${false}        | ${true}
+        ${false}  | ${true}         | ${true}
+        ${false}  | ${false}        | ${false}
+      `(
+        'when isLoading is $isLoading and mutationLoading is $mutationLoading is disabled',
+        ({ isLoading, mutationLoading, disabled }) => {
+          mountComponent({
+            props: { ...defaultProps, isLoading },
+            data: { mutationLoading },
+          });
+
+          expect(findSubmitButton().props().disabled).toBe(disabled);
+
+          expect(findPackageNamePatternInput().attributes().disabled).toBe(
+            disabled ? 'disabled' : undefined,
+          );
+          expect(findPackageTypeSelect().attributes().disabled).toBe(
+            disabled ? 'disabled' : undefined,
+          );
+          expect(findPushProtectedUpToAccessLevelSelect().attributes().disabled).toBe(
+            disabled ? 'disabled' : undefined,
+          );
+        },
+      );
+
+      it.each`
+        isLoading | mutationLoading | showLoading
+        ${true}   | ${true}         | ${true}
+        ${true}   | ${false}        | ${true}
+        ${false}  | ${true}         | ${true}
+        ${false}  | ${false}        | ${false}
+      `(
+        'when isLoading is $isLoading and mutationLoading is $mutationLoading is $showLoading that the loading icon is shown',
+        ({ isLoading, mutationLoading, showLoading }) => {
+          mountComponent({
+            props: { ...defaultProps, isLoading },
+            data: { mutationLoading },
+          });
+
+          expect(findSubmitButton().props('loading')).toBe(showLoading);
+        },
+      );
+    });
+  });
+
+  describe('form events', () => {
+    describe('reset', () => {
+      const mutationResolver = jest
+        .fn()
+        .mockResolvedValue(createPackagesProtectionRuleMutationPayload());
+
+      const resetForm = () => {
+        findForm().trigger('reset');
+        return waitForPromises();
+      };
+
+      beforeEach(async () => {
+        mountComponentWithApollo({ mutationResolver });
+
+        await resetForm();
+      });
+
+      it('emits custom event "cancel"', () => {
+        expect(mutationResolver).not.toHaveBeenCalled();
+
+        expect(wrapper.emitted()).toHaveProperty('cancel');
+        expect(wrapper.emitted().cancel[0]).toEqual([]);
+      });
+
+      it('does not dispatch apollo mutation request', () => {
+        expect(mutationResolver).not.toHaveBeenCalled();
+      });
+
+      it('does not emit custom event "submit"', () => {
+        expect(wrapper.emitted()).not.toHaveProperty('submit');
+      });
+    });
+
+    describe('submit', () => {
+      const submitForm = () => {
+        findForm().trigger('submit');
+        return waitForPromises();
+      };
+
+      it('dispatches correct apollo mutation', async () => {
+        const mutationResolver = jest
+          .fn()
+          .mockResolvedValue(createPackagesProtectionRuleMutationPayload());
+
+        mountComponentWithApollo({
+          mutationResolver,
+          props: { value: { ...defaultProps.value } },
+        });
+
+        await submitForm();
+
+        expect(mutationResolver).toHaveBeenCalledWith({
+          input: { projectPath: 'path', ...defaultProps.value },
+        });
+      });
+
+      it('emits event "submit" when apollo mutation successful', async () => {
+        const mutationResolver = jest
+          .fn()
+          .mockResolvedValue(createPackagesProtectionRuleMutationPayload());
+
+        mountComponentWithApollo({ mutationResolver, props: { value: { ...defaultProps.value } } });
+
+        await submitForm();
+
+        expect(wrapper.emitted()).toHaveProperty('submit');
+        expect(wrapper.emitted().submit[0]).toEqual([
+          { id: expect.any(String), ...defaultProps.value },
+        ]);
+
+        expect(wrapper.emitted()).not.toHaveProperty('cancel');
+      });
+
+      it('shows error alert with general message when apollo mutation request responds with errors', async () => {
+        mountComponentWithApollo({
+          mutationResolver: jest.fn().mockResolvedValue(
+            createPackagesProtectionRuleMutationPayload({
+              errors: createPackagesProtectionRuleMutationPayloadErrors,
+            }),
+          ),
+        });
+
+        await submitForm();
+
+        expect(findAlert().isVisible()).toBe(true);
+        expect(findAlert().text()).toBe(createPackagesProtectionRuleMutationPayloadErrors[0]);
+      });
+
+      it('shows error alert with general message when apollo mutation request fails', async () => {
+        mountComponentWithApollo({
+          mutationResolver: jest.fn().mockRejectedValue(new Error('GraphQL error')),
+        });
+
+        await submitForm();
+
+        expect(findAlert().isVisible()).toBe(true);
+        expect(findAlert().text()).toBe(
+          'Something went wrong while saving the package protection rule',
+        );
+      });
+    });
+  });
+});
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js
index bdb3db7a1b9b11..82db2d63f1ed40 100644
--- a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js
+++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js
@@ -5,6 +5,7 @@ import VueApollo from 'vue-apollo';
 import createMockApollo from 'helpers/mock_apollo_helper';
 import waitForPromises from 'helpers/wait_for_promises';
 import PackagesProtectionRules from '~/packages_and_registries/settings/project/components/packages_protection_rules.vue';
+import PackagesProtectionRuleForm from '~/packages_and_registries/settings/project/components/packages_protection_rule_form.vue';
 import SettingsBlock from '~/vue_shared/components/settings/settings_block.vue';
 import packagesProtectionRuleQuery from '~/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql';
 
@@ -22,6 +23,8 @@ describe('Packages protection rules project settings', () => {
   const findSettingsBlock = () => wrapper.findComponent(SettingsBlock);
   const findTable = () => wrapper.findComponent(GlTable);
   const findTableLoadingIcon = () => wrapper.findComponent(GlLoadingIcon);
+  const findProtectionRuleForm = () => wrapper.findComponent(PackagesProtectionRuleForm);
+  const findAddProtectionRuleButton = () => wrapper.find('button');
   const findTableRows = () => findTable().find('tbody').findAll('tr');
 
   const mountComponent = (mountFn = shallowMount, provide = defaultProvidedValues, config) => {
@@ -94,4 +97,82 @@ describe('Packages protection rules project settings', () => {
       expect(findTable().exists()).toBe(true);
     });
   });
+
+  it('renders table with pagination', async () => {
+    createComponent();
+
+    await waitForPromises();
+
+    expect(findTable().exists()).toBe(true);
+  });
+
+  it('does not initially render package protection form', async () => {
+    createComponent();
+
+    await waitForPromises();
+
+    expect(findAddProtectionRuleButton().exists()).toBe(true);
+    expect(findProtectionRuleForm().exists()).toBe(false);
+  });
+
+  describe('button "add protection rule"', () => {
+    it('button exists', async () => {
+      createComponent();
+
+      await waitForPromises();
+
+      expect(findAddProtectionRuleButton().exists()).toBe(true);
+    });
+
+    describe('when button is clicked', () => {
+      beforeEach(async () => {
+        createComponent();
+
+        await waitForPromises();
+
+        await findAddProtectionRuleButton().trigger('click');
+      });
+
+      it('renders package protection form', () => {
+        expect(findAddProtectionRuleButton().exists()).toBe(true);
+        expect(findProtectionRuleForm().exists()).toBe(true);
+      });
+
+      it('disables the button "add protection rule"', () => {
+        expect(findAddProtectionRuleButton().attributes()).toHaveProperty('disabled');
+      });
+    });
+  });
+
+  describe('form "add protection rule"', () => {
+    let resolver;
+
+    beforeEach(async () => {
+      resolver = jest.fn().mockResolvedValue(packagesProtectionRuleQueryPayload());
+
+      createComponent({ resolver });
+
+      await waitForPromises();
+
+      await findAddProtectionRuleButton().trigger('click');
+    });
+
+    it("handles event 'submit'", async () => {
+      await findProtectionRuleForm().vm.$emit('submit');
+
+      expect(resolver).toHaveBeenCalledTimes(2);
+
+      expect(findProtectionRuleForm().exists()).toBe(false);
+      expect(findAddProtectionRuleButton().attributes()).not.toHaveProperty('disabled');
+    });
+
+    it("handles event 'cancel'", async () => {
+      await findProtectionRuleForm().vm.$emit('cancel');
+
+      expect(resolver).toHaveBeenCalledTimes(1);
+
+      expect(findProtectionRuleForm().exists()).toBe(false);
+      expect(findAddProtectionRuleButton().attributes()).not.toHaveProperty('disabled');
+    });
+  });
 });
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js b/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js
index 5c546289b145e2..e187a31636cf1c 100644
--- a/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js
+++ b/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js
@@ -112,3 +112,25 @@ export const packagesProtectionRuleQueryPayload = ({ override, errors = [] } = {
     },
   },
 });
+
+export const createPackagesProtectionRuleMutationPayload = ({ override, errors = [] } = {}) => ({
+  data: {
+    createPackagesProtectionRule: {
+      packageProtectionRule: {
+        ...packagesProtectionRulesData[0],
+        ...override,
+      },
+      errors,
+    },
+  },
+});
+
+export const createPackagesProtectionRuleMutationInput = {
+  packageNamePattern: `@flight/flight-maintainer-14-*`,
+  packageType: 'NPM',
+  pushProtectedUpToAccessLevel: 'MAINTAINER',
+};
+
+export const createPackagesProtectionRuleMutationPayloadErrors = [
+  'Package name pattern has already been taken',
+];
-- 
GitLab


From b1ee28a2e3e0d339edc4527f4fb087fecaf2cd5f Mon Sep 17 00:00:00 2001
From: Rahul Chanila <rchanila@gitlab.com>
Date: Tue, 9 Jan 2024 07:29:11 +0000
Subject: [PATCH 2/6] refactor: Apply suggestion from @rchanila

Commit includes:
- Suggestions from @rchanila the first review round
- Avoid deprecated components according to danger bot, e.g. `<button`
- Avoid the element attribute `data-testid`
- Adjust test to check to html elements by role
---
 .../packages_protection_rule_form.vue         | 37 ++++++------
 .../components/packages_protection_rules.vue  | 11 ++--
 locale/gitlab.pot                             | 18 +++++-
 .../packages_protection_rule_form_spec.js     | 57 ++++++++++---------
 .../packages_protection_rules_spec.js         | 11 ++--
 5 files changed, 74 insertions(+), 60 deletions(-)

diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
index 3b4a4ed26f1c94..41fe0b64e17e02 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
+++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
@@ -1,8 +1,7 @@
 <script>
 import { GlAlert, GlButton, GlFormGroup, GlForm, GlFormInput, GlFormSelect } from '@gitlab/ui';
 import createPackagesProtectionRuleMutation from '~/packages_and_registries/settings/project/graphql/mutations/create_packages_protection_rule.mutation.graphql';
-// import packagesProtectionRuleQuery from '~/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql';
-import { s__ } from '~/locale';
+import { s__, __ } from '~/locale';
 
 const PACKAGES_PROTECTION_RULES_SAVED_SUCCESS_MESSAGE = s__(
   'PackageRegistry|Rule saved successfully',
@@ -45,13 +44,13 @@ export default {
         pushProtectedUpToAccessLevel: 'MAINTAINER',
         ...this.value,
       },
-      mutationLoading: false,
+      updateInProgress: false,
       alertErrorMessage: '',
     };
   },
   computed: {
     showLoadingIcon() {
-      return this.isLoading || this.mutationLoading;
+      return this.isLoading || this.updateInProgress;
     },
     isSubmitButtonDisabled() {
       return (
@@ -72,12 +71,12 @@ export default {
       };
     },
     packageTypeOptions() {
-      return [{ value: 'NPM', text: 'NPM' }];
+      return [{ value: 'NPM', text: s__('PackageRegistry|Npm') }];
     },
     pushProtectedUpToAccessLevelOptions() {
       return [
-        { value: 'MAINTAINER', text: 'MAINTAINER' },
-        { value: 'OWNER', text: 'OWNER' },
+        { value: 'MAINTAINER', text: __('Maintainer') },
+        { value: 'OWNER', text: __('Owner') },
       ];
     },
   },
@@ -85,7 +84,7 @@ export default {
     submit() {
       this.clearAlertErrorMessage();
 
-      this.mutationLoading = true;
+      this.updateInProgress = true;
       return this.$apollo
         .mutate({
           mutation: createPackagesProtectionRuleMutation,
@@ -100,15 +99,13 @@ export default {
             return;
           }
 
-          this.clearAlertErrorMessage();
-
           this.$emit('submit', data.createPackagesProtectionRule.packageProtectionRule);
         })
         .catch(() => {
           this.alertErrorMessage = PACKAGES_PROTECTION_RULES_SAVED_ERROR_MESSAGE;
         })
         .finally(() => {
-          this.mutationLoading = false;
+          this.updateInProgress = false;
         });
     },
     clearAlertErrorMessage() {
@@ -125,13 +122,17 @@ export default {
 <template>
   <div class="gl-new-card-add-form gl-m-3">
     <gl-form @submit.prevent="submit" @reset="cancelForm">
-      <gl-alert v-if="alertErrorMessage" class="mb-3" variant="danger" :dismissible="false">
+      <gl-alert
+        v-if="alertErrorMessage"
+        class="mb-3"
+        variant="danger"
+        @dismiss="clearAlertErrorMessage"
+      >
         {{ alertErrorMessage }}
       </gl-alert>
 
       <gl-form-group
-        id="input-group-package-name-pattern"
-        label="Package Name Pattern:"
+        :label="s__('PackageRegistry|Package Name Pattern')"
         label-for="input-package-name-pattern"
       >
         <gl-form-input
@@ -141,13 +142,11 @@ export default {
           required
           :placeholder="__(`Enter package name pattern`)"
           :disabled="isFieldDisabled"
-          pattern=".{1,}"
         />
       </gl-form-group>
 
       <gl-form-group
-        id="input-group-package-type"
-        label="Package Type:"
+        :label="s__('PackageRegistry|Package Type')"
         label-for="input-package-type"
         :disabled="isFieldDisabled"
       >
@@ -161,8 +160,7 @@ export default {
       </gl-form-group>
 
       <gl-form-group
-        id="input-group-push-protected-up-to-access-level"
-        label="Push Protected Up To Access Level:"
+        :label="s__('PackageRegistry|Push Protected Up To Access Level')"
         label-for="input-push-protected-up-to-access-level"
         :disabled="isFieldDisabled"
       >
@@ -177,7 +175,6 @@ export default {
 
       <div class="gl-display-flex gl-justify-content-start">
         <gl-button
-          data-testid="submit-button"
           variant="confirm"
           type="submit"
           :disabled="isSubmitButtonDisabled"
diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue
index c2e6a376367a4d..07997c4a8960d5 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue
+++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue
@@ -1,5 +1,5 @@
 <script>
-import { GlCard, GlTable, GlLoadingIcon } from '@gitlab/ui';
+import { GlButton, GlCard, GlTable, GlLoadingIcon } from '@gitlab/ui';
 import packagesProtectionRuleQuery from '~/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql';
 import SettingsBlock from '~/packages_and_registries/shared/components/settings_block.vue';
 import PackagesProtectionRuleForm from '~/packages_and_registries/settings/project/components/packages_protection_rule_form.vue';
@@ -10,6 +10,7 @@ const PAGINATION_DEFAULT_PER_PAGE = 10;
 export default {
   components: {
     SettingsBlock,
+    GlButton,
     GlCard,
     GlTable,
     GlLoadingIcon,
@@ -111,13 +112,13 @@ export default {
           <div class="gl-new-card-title-wrapper gl-justify-content-space-between">
             <h3 class="gl-new-card-title">{{ $options.i18n.settingBlockTitle }}</h3>
             <div class="gl-new-card-actions">
-              <button
-                class="gl-button btn btn-sm btn-default"
+              <gl-button
+                size="small"
                 :disabled="isAddProtectionRuleButtonDisabled"
                 @click="showProtectionRuleForm"
               >
-                <span class="gl-button-text">{{ __('Add package protection rule') }}</span>
-              </button>
+                {{ s__('PackageRegistry|Add package protection rule') }}
+              </gl-button>
             </div>
           </div>
         </template>
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 7f712fe04f4b5d..b96b1ff89a0020 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3038,9 +3038,6 @@ msgstr ""
 msgid "Add or subtract spent time"
 msgstr ""
 
-msgid "Add package protection rule"
-msgstr ""
-
 msgid "Add people"
 msgstr ""
 
@@ -34643,6 +34640,9 @@ msgstr ""
 msgid "PackageRegistry|Add composer registry"
 msgstr ""
 
+msgid "PackageRegistry|Add package protection rule"
+msgstr ""
+
 msgid "PackageRegistry|Additional metadata"
 msgstr ""
 
@@ -34898,6 +34898,9 @@ msgstr ""
 msgid "PackageRegistry|Maven XML"
 msgstr ""
 
+msgid "PackageRegistry|Npm"
+msgstr ""
+
 msgid "PackageRegistry|NuGet"
 msgstr ""
 
@@ -34910,9 +34913,15 @@ msgstr ""
 msgid "PackageRegistry|Other versions"
 msgstr ""
 
+msgid "PackageRegistry|Package Name Pattern"
+msgstr ""
+
 msgid "PackageRegistry|Package Registry"
 msgstr ""
 
+msgid "PackageRegistry|Package Type"
+msgstr ""
+
 msgid "PackageRegistry|Package asset deleted successfully"
 msgstr ""
 
@@ -34984,6 +34993,9 @@ msgstr ""
 msgid "PackageRegistry|Published to the %{project} Package Registry %{datetime}"
 msgstr ""
 
+msgid "PackageRegistry|Push Protected Up To Access Level"
+msgstr ""
+
 msgid "PackageRegistry|Push protected up to access level"
 msgstr ""
 
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
index 60f2b5ab8f5b8b..d2026bdf2487f2 100644
--- a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
+++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
@@ -1,7 +1,7 @@
 import VueApollo from 'vue-apollo';
 import Vue from 'vue';
-import { GlForm, GlAlert, GlFormInput } from '@gitlab/ui';
-import { mount } from '@vue/test-utils';
+import { GlForm } from '@gitlab/ui';
+import { mountExtended } from 'helpers/vue_test_utils_helper';
 import createMockApollo from 'helpers/mock_apollo_helper';
 import waitForPromises from 'helpers/wait_for_promises';
 import PackagesProtectionRuleForm from '~/packages_and_registries/settings/project/components/packages_protection_rule_form.vue';
@@ -24,13 +24,12 @@ describe('Packages Protection Rule Form', () => {
 
   const defaultProps = { value: { ...createPackagesProtectionRuleMutationInput } };
 
-  const findForm = () => wrapper.findComponent(GlForm);
-  const findAlert = () => wrapper.findComponent(GlAlert);
-  const findPackageNamePatternInput = () => findForm().findComponent(GlFormInput);
-  const findPackageTypeSelect = () => findForm().find('select#input-package-type');
+  const findPackageNamePatternInput = () =>
+    wrapper.findByRole('textbox', { name: /package name patter/i });
+  const findPackageTypeSelect = () => wrapper.findByRole('combobox', { name: /package type/i });
   const findPushProtectedUpToAccessLevelSelect = () =>
-    findForm().find('select#input-push-protected-up-to-access-level');
-  const findSubmitButton = () => findForm().find('[data-testid="submit-button"]');
+    wrapper.findByRole('combobox', { name: /push protected up to access level/i });
+  const findSubmitButton = () => wrapper.findByRole('button', { name: /protect/i });
 
   const mountComponent = ({
     props = defaultProps,
@@ -38,7 +37,7 @@ describe('Packages Protection Rule Form', () => {
     config,
     provide = defaultProvidedValues,
   } = {}) => {
-    wrapper = mount(PackagesProtectionRuleForm, {
+    wrapper = mountExtended(PackagesProtectionRuleForm, {
       propsData: { ...props },
       provide,
       data() {
@@ -152,7 +151,7 @@ describe('Packages Protection Rule Form', () => {
   });
 
   describe('form actions', () => {
-    describe('button submit', () => {
+    describe('button "Protect"', () => {
       it.each`
         packageNamePattern       | submitButtonDisabled
         ${''}                    | ${true}
@@ -175,19 +174,21 @@ describe('Packages Protection Rule Form', () => {
       );
 
       it.each`
-        isLoading | mutationLoading | disabled
-        ${true}   | ${true}         | ${true}
-        ${true}   | ${false}        | ${true}
-        ${false}  | ${true}         | ${true}
-        ${false}  | ${false}        | ${false}
+        isLoading | updateInProgress | disabled
+        ${true}   | ${true}          | ${true}
+        ${true}   | ${false}         | ${true}
+        ${false}  | ${true}          | ${true}
+        ${false}  | ${false}         | ${false}
       `(
-        'when isLoading is $isLoading and mutationLoading is $mutationLoading is disabled',
-        ({ isLoading, mutationLoading, disabled }) => {
+        'when isLoading is $isLoading and updateInProgress is $updateInProgress is disabled',
+        async ({ isLoading, updateInProgress, disabled }) => {
           mountComponent({
             props: { ...defaultProps, isLoading },
-            data: { mutationLoading },
+            data: { updateInProgress },
           });
 
+          await waitForPromises;
+
           expect(findSubmitButton().props().disabled).toBe(disabled);
 
           expect(findPackageNamePatternInput().attributes().disabled).toBe(
@@ -203,17 +204,17 @@ describe('Packages Protection Rule Form', () => {
       );
 
       it.each`
-        isLoading | mutationLoading | showLoading
-        ${true}   | ${true}         | ${true}
-        ${true}   | ${false}        | ${true}
-        ${false}  | ${true}         | ${true}
-        ${false}  | ${false}        | ${false}
+        isLoading | updateInProgress | showLoading
+        ${true}   | ${true}          | ${true}
+        ${true}   | ${false}         | ${true}
+        ${false}  | ${true}          | ${true}
+        ${false}  | ${false}         | ${false}
       `(
-        'when isLoading is $isLoading and mutationLoading is $mutationLoading is $showLoading that the loading icon is shown',
-        ({ isLoading, mutationLoading, showLoading }) => {
+        'when isLoading is $isLoading and updateInProgress is $updateInProgress is $showLoading that the loading icon is shown',
+        ({ isLoading, updateInProgress, showLoading }) => {
           mountComponent({
             props: { ...defaultProps, isLoading },
-            data: { mutationLoading },
+            data: { updateInProgress },
           });
 
           expect(findSubmitButton().props('loading')).toBe(showLoading);
@@ -223,6 +224,8 @@ describe('Packages Protection Rule Form', () => {
   });
 
   describe('form events', () => {
+    const findForm = () => wrapper.findComponent(GlForm);
+
     describe('reset', () => {
       const mutationResolver = jest
         .fn()
@@ -256,6 +259,8 @@ describe('Packages Protection Rule Form', () => {
     });
 
     describe('submit', () => {
+      const findAlert = () => wrapper.findByRole('alert');
+
       const submitForm = () => {
         findForm().trigger('submit');
         return waitForPromises();
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js
index 82db2d63f1ed40..4a95e17655fae2 100644
--- a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js
+++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js
@@ -1,4 +1,4 @@
-import { GlTable, GlLoadingIcon } from '@gitlab/ui';
+import { GlButton, GlTable, GlLoadingIcon } from '@gitlab/ui';
 import { shallowMount, mount } from '@vue/test-utils';
 import Vue from 'vue';
 import VueApollo from 'vue-apollo';
@@ -6,7 +6,7 @@ import createMockApollo from 'helpers/mock_apollo_helper';
 import waitForPromises from 'helpers/wait_for_promises';
 import PackagesProtectionRules from '~/packages_and_registries/settings/project/components/packages_protection_rules.vue';
 import PackagesProtectionRuleForm from '~/packages_and_registries/settings/project/components/packages_protection_rule_form.vue';
-import SettingsBlock from '~/vue_shared/components/settings/settings_block.vue';
+import SettingsBlock from '~/packages_and_registries/shared/components/settings_block.vue';
 import packagesProtectionRuleQuery from '~/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql';
 
 import { packagesProtectionRuleQueryPayload, packagesProtectionRulesData } from '../mock_data';
@@ -24,7 +24,7 @@ describe('Packages protection rules project settings', () => {
   const findTable = () => wrapper.findComponent(GlTable);
   const findTableLoadingIcon = () => wrapper.findComponent(GlLoadingIcon);
   const findProtectionRuleForm = () => wrapper.findComponent(PackagesProtectionRuleForm);
-  const findAddProtectionRuleButton = () => wrapper.find('button');
+  const findAddProtectionRuleButton = () => wrapper.findComponent(GlButton);
   const findTableRows = () => findTable().find('tbody').findAll('tr');
 
   const mountComponent = (mountFn = shallowMount, provide = defaultProvidedValues, config) => {
@@ -126,7 +126,7 @@ describe('Packages protection rules project settings', () => {
 
     describe('when button is clicked', () => {
       beforeEach(async () => {
-        createComponent();
+        createComponent({ mountFn: mount });
 
         await waitForPromises();
 
@@ -134,7 +134,6 @@ describe('Packages protection rules project settings', () => {
       });
 
       it('renders package protection form', () => {
-        expect(findAddProtectionRuleButton().exists()).toBe(true);
         expect(findProtectionRuleForm().exists()).toBe(true);
       });
 
@@ -150,7 +149,7 @@ describe('Packages protection rules project settings', () => {
     beforeEach(async () => {
       resolver = jest.fn().mockResolvedValue(packagesProtectionRuleQueryPayload());
 
-      createComponent({ resolver });
+      createComponent({ resolver, mountFn: mount });
 
       await waitForPromises();
 
-- 
GitLab


From daead696432782b1565843c8d175780f47ffb380 Mon Sep 17 00:00:00 2001
From: Rahul Chanila <rchanila@gitlab.com>
Date: Wed, 10 Jan 2024 14:39:29 +0000
Subject: [PATCH 3/6] refactor: Apply suggestion from @rchanila

Commit includes:
- Removing props of the component
- Adjusting tests accordingly
---
 .../packages_protection_rule_form.vue         |  15 +-
 .../packages_protection_rule_form_spec.js     | 162 ++++--------------
 2 files changed, 34 insertions(+), 143 deletions(-)

diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
index 41fe0b64e17e02..046670901e7ae1 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
+++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
@@ -20,18 +20,6 @@ export default {
     GlForm,
   },
   inject: ['projectPath'],
-  props: {
-    value: {
-      type: Object,
-      required: false,
-      default: () => {},
-    },
-    isLoading: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
-  },
   i18n: {
     PACKAGES_PROTECTION_RULES_SAVED_SUCCESS_MESSAGE,
     PACKAGES_PROTECTION_RULES_SAVED_ERROR_MESSAGE,
@@ -42,7 +30,6 @@ export default {
         packageNamePattern: '',
         packageType: 'NPM',
         pushProtectedUpToAccessLevel: 'MAINTAINER',
-        ...this.value,
       },
       updateInProgress: false,
       alertErrorMessage: '',
@@ -50,7 +37,7 @@ export default {
   },
   computed: {
     showLoadingIcon() {
-      return this.isLoading || this.updateInProgress;
+      return this.updateInProgress;
     },
     isSubmitButtonDisabled() {
       return (
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
index d2026bdf2487f2..e07429f5cbfde3 100644
--- a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
+++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
@@ -22,23 +22,15 @@ describe('Packages Protection Rule Form', () => {
     projectPath: 'path',
   };
 
-  const defaultProps = { value: { ...createPackagesProtectionRuleMutationInput } };
-
   const findPackageNamePatternInput = () =>
-    wrapper.findByRole('textbox', { name: /package name patter/i });
+    wrapper.findByRole('textbox', { name: /package name pattern/i });
   const findPackageTypeSelect = () => wrapper.findByRole('combobox', { name: /package type/i });
   const findPushProtectedUpToAccessLevelSelect = () =>
     wrapper.findByRole('combobox', { name: /push protected up to access level/i });
   const findSubmitButton = () => wrapper.findByRole('button', { name: /protect/i });
 
-  const mountComponent = ({
-    props = defaultProps,
-    data,
-    config,
-    provide = defaultProvidedValues,
-  } = {}) => {
+  const mountComponent = ({ data, config, provide = defaultProvidedValues } = {}) => {
     wrapper = mountExtended(PackagesProtectionRuleForm, {
-      propsData: { ...props },
       provide,
       data() {
         return { ...data };
@@ -47,18 +39,13 @@ describe('Packages Protection Rule Form', () => {
     });
   };
 
-  const mountComponentWithApollo = ({
-    provide = defaultProvidedValues,
-    mutationResolver,
-    props = defaultProps,
-  } = {}) => {
+  const mountComponentWithApollo = ({ provide = defaultProvidedValues, mutationResolver } = {}) => {
     const requestHandlers = [[createPackagesProtectionRuleMutation, mutationResolver]];
 
     fakeApollo = createMockApollo(requestHandlers);
 
     mountComponent({
       provide,
-      props,
       config: {
         apolloProvider: fakeApollo,
       },
@@ -66,28 +53,6 @@ describe('Packages Protection Rule Form', () => {
   };
 
   describe('form fields', () => {
-    describe('form field "packageNamePattern"', () => {
-      it.each`
-        packageNamePattern     | expectedPackageNamePatternInputValue
-        ${'@scope/my-package'} | ${'@scope/my-package'}
-        ${''}                  | ${''}
-        ${' '}                 | ${' '}
-        ${undefined}           | ${''}
-        ${null}                | ${''}
-      `(
-        'handles given props (packageNamePattern: "$packageNamePattern") correctly',
-        ({ packageNamePattern, expectedPackageNamePatternInputValue }) => {
-          const props = { value: { ...defaultProps.value, packageNamePattern } };
-
-          mountComponent({ props });
-
-          expect(findPackageNamePatternInput().element.value).toBe(
-            expectedPackageNamePatternInputValue,
-          );
-        },
-      );
-    });
-
     describe('form field "packageType"', () => {
       it('contains only the options for npm', () => {
         mountComponent();
@@ -98,23 +63,6 @@ describe('Packages Protection Rule Form', () => {
           .wrappers.map((option) => option.element.value);
         expect(packageTypeSelectOptions).toEqual(['NPM']);
       });
-
-      it.each`
-        packageType               | packageTypeSelectValue
-        ${'NPM'}                  | ${'NPM'}
-        ${'INVALID_PACKAGE_TYPE'} | ${''}
-        ${''}                     | ${''}
-        ${undefined}              | ${''}
-      `(
-        'handles given props (packageType: "$packageType") correctly',
-        ({ packageType, packageTypeSelectValue }) => {
-          const props = { value: { ...defaultProps.value, packageType } };
-
-          mountComponent({ props });
-
-          expect(findPackageTypeSelect().element.value).toBe(packageTypeSelectValue);
-        },
-      );
     });
 
     describe('form field "pushProtectedUpToAccessLevelSelect"', () => {
@@ -127,41 +75,31 @@ describe('Packages Protection Rule Form', () => {
           .wrappers.map((option) => option.element.value);
         expect(pushProtectedUpToAccessLevelSelectOptions).toEqual(['MAINTAINER', 'OWNER']);
       });
+    });
 
-      it.each`
-        pushProtectedUpToAccessLevel | pushProtectedUpToAccessLevelSelectValue
-        ${'OWNER'}                   | ${'OWNER'}
-        ${'MAINTAINER'}              | ${'MAINTAINER'}
-        ${'INVALID_ACCESS_LEVEL'}    | ${''}
-        ${undefined}                 | ${''}
-        ${''}                        | ${''}
-      `(
-        'handles given props (pushProtectedUpToAccessLevel: "$pushProtectedUpToAccessLevel") correctly',
-        ({ pushProtectedUpToAccessLevel, pushProtectedUpToAccessLevelSelectValue }) => {
-          const props = { value: { ...defaultProps.value, pushProtectedUpToAccessLevel } };
-
-          mountComponent({ props });
+    describe('when graphql mutation is in progress', () => {
+      it('disables all form fields', () => {
+        mountComponent({ data: { updateInProgress: true } });
 
-          expect(findPushProtectedUpToAccessLevelSelect().element.value).toBe(
-            pushProtectedUpToAccessLevelSelectValue,
-          );
-        },
-      );
+        expect(findSubmitButton().props().disabled).toBe(true);
+        expect(findPackageNamePatternInput().attributes().disabled).toBe('disabled');
+        expect(findPackageTypeSelect().attributes().disabled).toBe('disabled');
+        expect(findPushProtectedUpToAccessLevelSelect().attributes().disabled).toBe('disabled');
+      });
     });
   });
 
   describe('form actions', () => {
     describe('button "Protect"', () => {
       it.each`
-        packageNamePattern       | submitButtonDisabled
-        ${''}                    | ${true}
-        ${'  '}                  | ${true}
-        ${'@scope/my-package'}   | ${false}
-        ${'@scope/my-package-*'} | ${false}
+        packageNamePattern                                              | submitButtonDisabled
+        ${''}                                                           | ${true}
+        ${' '}                                                          | ${true}
+        ${createPackagesProtectionRuleMutationInput.packageNamePattern} | ${false}
       `(
         'when packageNamePattern is "$packageNamePattern" then the disabled state of the submit button is $submitButtonDisabled',
         async ({ packageNamePattern, submitButtonDisabled }) => {
-          mountComponent({ props: { value: { packageNamePattern: '' } } });
+          mountComponent();
 
           expect(findSubmitButton().exists()).toBe(true);
           expect(findSubmitButton().props().disabled).toBe(true);
@@ -174,48 +112,13 @@ describe('Packages Protection Rule Form', () => {
       );
 
       it.each`
-        isLoading | updateInProgress | disabled
-        ${true}   | ${true}          | ${true}
-        ${true}   | ${false}         | ${true}
-        ${false}  | ${true}          | ${true}
-        ${false}  | ${false}         | ${false}
+        updateInProgress | showLoading
+        ${true}          | ${true}
+        ${false}         | ${false}
       `(
-        'when isLoading is $isLoading and updateInProgress is $updateInProgress is disabled',
-        async ({ isLoading, updateInProgress, disabled }) => {
-          mountComponent({
-            props: { ...defaultProps, isLoading },
-            data: { updateInProgress },
-          });
-
-          await waitForPromises;
-
-          expect(findSubmitButton().props().disabled).toBe(disabled);
-
-          expect(findPackageNamePatternInput().attributes().disabled).toBe(
-            disabled ? 'disabled' : undefined,
-          );
-          expect(findPackageTypeSelect().attributes().disabled).toBe(
-            disabled ? 'disabled' : undefined,
-          );
-          expect(findPushProtectedUpToAccessLevelSelect().attributes().disabled).toBe(
-            disabled ? 'disabled' : undefined,
-          );
-        },
-      );
-
-      it.each`
-        isLoading | updateInProgress | showLoading
-        ${true}   | ${true}          | ${true}
-        ${true}   | ${false}         | ${true}
-        ${false}  | ${true}          | ${true}
-        ${false}  | ${false}         | ${false}
-      `(
-        'when isLoading is $isLoading and updateInProgress is $updateInProgress is $showLoading that the loading icon is shown',
-        ({ isLoading, updateInProgress, showLoading }) => {
-          mountComponent({
-            props: { ...defaultProps, isLoading },
-            data: { updateInProgress },
-          });
+        'when updateInProgress is $updateInProgress then the button loading icon is $showLoading',
+        ({ updateInProgress, showLoading }) => {
+          mountComponent({ data: { updateInProgress } });
 
           expect(findSubmitButton().props('loading')).toBe(showLoading);
         },
@@ -271,15 +174,16 @@ describe('Packages Protection Rule Form', () => {
           .fn()
           .mockResolvedValue(createPackagesProtectionRuleMutationPayload());
 
-        mountComponentWithApollo({
-          mutationResolver,
-          props: { value: { ...defaultProps.value } },
-        });
+        mountComponentWithApollo({ mutationResolver });
+
+        await findPackageNamePatternInput().setValue(
+          createPackagesProtectionRuleMutationInput.packageNamePattern,
+        );
 
         await submitForm();
 
         expect(mutationResolver).toHaveBeenCalledWith({
-          input: { projectPath: 'path', ...defaultProps.value },
+          input: { projectPath: 'path', ...createPackagesProtectionRuleMutationInput },
         });
       });
 
@@ -288,14 +192,14 @@ describe('Packages Protection Rule Form', () => {
           .fn()
           .mockResolvedValue(createPackagesProtectionRuleMutationPayload());
 
-        mountComponentWithApollo({ mutationResolver, props: { value: { ...defaultProps.value } } });
+        mountComponentWithApollo({ mutationResolver });
 
         await submitForm();
 
         expect(wrapper.emitted()).toHaveProperty('submit');
-        expect(wrapper.emitted().submit[0]).toEqual([
-          { id: expect.any(String), ...defaultProps.value },
-        ]);
+        const expectedEventSubmitPayload = createPackagesProtectionRuleMutationPayload().data
+          .createPackagesProtectionRule.packageProtectionRule;
+        expect(wrapper.emitted().submit[0]).toEqual([expectedEventSubmitPayload]);
 
         expect(wrapper.emitted()).not.toHaveProperty('cancel');
       });
-- 
GitLab


From 0216e42d365972c93b76766097941ede4fd6d97c Mon Sep 17 00:00:00 2001
From: Rahul Chanila <rchanila@gitlab.com>
Date: Mon, 15 Jan 2024 07:39:34 +0000
Subject: [PATCH 4/6] refactor: Apply suggestion from @rchanila

Commit includes:
- Applying the suggestions from the MR review
---
 .../packages_protection_rule_form.vue         | 10 ++--
 .../packages_protection_rule_form_spec.js     | 57 +++++++------------
 .../packages_protection_rules_spec.js         | 12 +---
 3 files changed, 29 insertions(+), 50 deletions(-)

diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
index 046670901e7ae1..6f3d0e7ba148d0 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
+++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
@@ -39,11 +39,11 @@ export default {
     showLoadingIcon() {
       return this.updateInProgress;
     },
+    isEmptyPackageName() {
+      return !this.packageProtectionRuleFormData.packageNamePattern;
+    },
     isSubmitButtonDisabled() {
-      return (
-        this.packageProtectionRuleFormData.packageNamePattern?.trim()?.length <= 0 ||
-        this.showLoadingIcon
-      );
+      return this.isEmptyPackageName || this.showLoadingIcon;
     },
     isFieldDisabled() {
       return this.showLoadingIcon;
@@ -124,7 +124,7 @@ export default {
       >
         <gl-form-input
           id="input-package-name-pattern"
-          v-model="packageProtectionRuleFormData.packageNamePattern"
+          v-model.trim="packageProtectionRuleFormData.packageNamePattern"
           type="text"
           required
           :placeholder="__(`Enter package name pattern`)"
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
index e07429f5cbfde3..6bf00e23bed3d7 100644
--- a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
+++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
@@ -28,6 +28,7 @@ describe('Packages Protection Rule Form', () => {
   const findPushProtectedUpToAccessLevelSelect = () =>
     wrapper.findByRole('combobox', { name: /push protected up to access level/i });
   const findSubmitButton = () => wrapper.findByRole('button', { name: /protect/i });
+  const findForm = () => wrapper.findComponent(GlForm);
 
   const mountComponent = ({ data, config, provide = defaultProvidedValues } = {}) => {
     wrapper = mountExtended(PackagesProtectionRuleForm, {
@@ -78,13 +79,21 @@ describe('Packages Protection Rule Form', () => {
     });
 
     describe('when graphql mutation is in progress', () => {
+      beforeEach(() => {
+        mountComponentWithApollo();
+
+        findForm().trigger('submit');
+      });
+
       it('disables all form fields', () => {
-        mountComponent({ data: { updateInProgress: true } });
+        expect(findSubmitButton().props('disabled')).toBe(true);
+        expect(findPackageNamePatternInput().attributes('disabled')).toBe('disabled');
+        expect(findPackageTypeSelect().attributes('disabled')).toBe('disabled');
+        expect(findPushProtectedUpToAccessLevelSelect().attributes('disabled')).toBe('disabled');
+      });
 
-        expect(findSubmitButton().props().disabled).toBe(true);
-        expect(findPackageNamePatternInput().attributes().disabled).toBe('disabled');
-        expect(findPackageTypeSelect().attributes().disabled).toBe('disabled');
-        expect(findPushProtectedUpToAccessLevelSelect().attributes().disabled).toBe('disabled');
+      it('displays a loading spinner', () => {
+        expect(findSubmitButton().props('loading')).toBe(true);
       });
     });
   });
@@ -101,55 +110,33 @@ describe('Packages Protection Rule Form', () => {
         async ({ packageNamePattern, submitButtonDisabled }) => {
           mountComponent();
 
-          expect(findSubmitButton().exists()).toBe(true);
-          expect(findSubmitButton().props().disabled).toBe(true);
+          expect(findSubmitButton().props('disabled')).toBe(true);
 
           await findPackageNamePatternInput().setValue(packageNamePattern);
 
-          expect(findSubmitButton().exists()).toBe(true);
-          expect(findSubmitButton().props().disabled).toBe(submitButtonDisabled);
-        },
-      );
-
-      it.each`
-        updateInProgress | showLoading
-        ${true}          | ${true}
-        ${false}         | ${false}
-      `(
-        'when updateInProgress is $updateInProgress then the button loading icon is $showLoading',
-        ({ updateInProgress, showLoading }) => {
-          mountComponent({ data: { updateInProgress } });
-
-          expect(findSubmitButton().props('loading')).toBe(showLoading);
+          expect(findSubmitButton().props('disabled')).toBe(submitButtonDisabled);
         },
       );
     });
   });
 
   describe('form events', () => {
-    const findForm = () => wrapper.findComponent(GlForm);
-
     describe('reset', () => {
       const mutationResolver = jest
         .fn()
         .mockResolvedValue(createPackagesProtectionRuleMutationPayload());
 
-      const resetForm = () => {
-        findForm().trigger('reset');
-        return waitForPromises();
-      };
-
-      beforeEach(async () => {
+      beforeEach(() => {
         mountComponentWithApollo({ mutationResolver });
 
-        await resetForm();
+        findForm().trigger('reset');
       });
 
       it('emits custom event "cancel"', () => {
         expect(mutationResolver).not.toHaveBeenCalled();
 
-        expect(wrapper.emitted()).toHaveProperty('cancel');
-        expect(wrapper.emitted().cancel[0]).toEqual([]);
+        expect(wrapper.emitted('cancel')).toBeDefined();
+        expect(wrapper.emitted('cancel')[0]).toEqual([]);
       });
 
       it('does not dispatch apollo mutation request', () => {
@@ -196,10 +183,10 @@ describe('Packages Protection Rule Form', () => {
 
         await submitForm();
 
-        expect(wrapper.emitted()).toHaveProperty('submit');
+        expect(wrapper.emitted('submit')).toBeDefined();
         const expectedEventSubmitPayload = createPackagesProtectionRuleMutationPayload().data
           .createPackagesProtectionRule.packageProtectionRule;
-        expect(wrapper.emitted().submit[0]).toEqual([expectedEventSubmitPayload]);
+        expect(wrapper.emitted('submit')[0]).toEqual([expectedEventSubmitPayload]);
 
         expect(wrapper.emitted()).not.toHaveProperty('cancel');
       });
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js
index 4a95e17655fae2..26ace764a72fcd 100644
--- a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js
+++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js
@@ -98,14 +98,6 @@ describe('Packages protection rules project settings', () => {
     });
   });
 
-  it('renders table with pagination', async () => {
-    createComponent();
-
-    await waitForPromises();
-
-    expect(findTable().exists()).toBe(true);
-  });
-
   it('does not initially render package protection form', async () => {
     createComponent();
 
@@ -138,7 +130,7 @@ describe('Packages protection rules project settings', () => {
       });
 
       it('disables the button "add protection rule"', () => {
-        expect(findAddProtectionRuleButton().attributes()).toHaveProperty('disabled');
+        expect(findAddProtectionRuleButton().attributes('disabled')).toBeDefined();
       });
     });
   });
@@ -162,7 +154,7 @@ describe('Packages protection rules project settings', () => {
       expect(resolver).toHaveBeenCalledTimes(2);
 
       expect(findProtectionRuleForm().exists()).toBe(false);
-      expect(findAddProtectionRuleButton().attributes()).not.toHaveProperty('disabled');
+      expect(findAddProtectionRuleButton().attributes('disabled')).not.toBeDefined();
     });
 
     it("handles event 'cancel'", async () => {
-- 
GitLab


From ec40f514e1d538a378e0228c9804c18d7483762e Mon Sep 17 00:00:00 2001
From: Phillip Wells <pwells@gitlab.com>
Date: Tue, 23 Jan 2024 08:02:35 +0000
Subject: [PATCH 5/6] refactor: Apply suggestion from @philipwells

Commit includes:
- Adding a confirmation modal to
  confirm the deletion of a package protection rule
---
 .../components/packages_protection_rule_form.vue | 13 +++++--------
 locale/gitlab.pot                                | 16 ++--------------
 .../packages_protection_rule_form_spec.js        |  4 ++--
 3 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
index 6f3d0e7ba148d0..9032c9c9c37a21 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
+++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
@@ -3,11 +3,9 @@ import { GlAlert, GlButton, GlFormGroup, GlForm, GlFormInput, GlFormSelect } fro
 import createPackagesProtectionRuleMutation from '~/packages_and_registries/settings/project/graphql/mutations/create_packages_protection_rule.mutation.graphql';
 import { s__, __ } from '~/locale';
 
-const PACKAGES_PROTECTION_RULES_SAVED_SUCCESS_MESSAGE = s__(
-  'PackageRegistry|Rule saved successfully',
-);
+const PACKAGES_PROTECTION_RULES_SAVED_SUCCESS_MESSAGE = s__('PackageRegistry|Rule saved.');
 const PACKAGES_PROTECTION_RULES_SAVED_ERROR_MESSAGE = s__(
-  'PackageRegistry|Something went wrong while saving the package protection rule',
+  'PackageRegistry|Something went wrong while saving the package protection rule.',
 );
 
 export default {
@@ -119,7 +117,7 @@ export default {
       </gl-alert>
 
       <gl-form-group
-        :label="s__('PackageRegistry|Package Name Pattern')"
+        :label="s__('PackageRegistry|Package name pattern')"
         label-for="input-package-name-pattern"
       >
         <gl-form-input
@@ -127,13 +125,12 @@ export default {
           v-model.trim="packageProtectionRuleFormData.packageNamePattern"
           type="text"
           required
-          :placeholder="__(`Enter package name pattern`)"
           :disabled="isFieldDisabled"
         />
       </gl-form-group>
 
       <gl-form-group
-        :label="s__('PackageRegistry|Package Type')"
+        :label="s__('PackageRegistry|Package type')"
         label-for="input-package-type"
         :disabled="isFieldDisabled"
       >
@@ -147,7 +144,7 @@ export default {
       </gl-form-group>
 
       <gl-form-group
-        :label="s__('PackageRegistry|Push Protected Up To Access Level')"
+        :label="s__('PackageRegistry|Push protected up to access level')"
         label-for="input-push-protected-up-to-access-level"
         :disabled="isFieldDisabled"
       >
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index b96b1ff89a0020..49395a7188ce34 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -19034,9 +19034,6 @@ msgstr ""
 msgid "Enter one or more user ID separated by commas"
 msgstr ""
 
-msgid "Enter package name pattern"
-msgstr ""
-
 msgid "Enter the %{name} description"
 msgstr ""
 
@@ -34913,15 +34910,9 @@ msgstr ""
 msgid "PackageRegistry|Other versions"
 msgstr ""
 
-msgid "PackageRegistry|Package Name Pattern"
-msgstr ""
-
 msgid "PackageRegistry|Package Registry"
 msgstr ""
 
-msgid "PackageRegistry|Package Type"
-msgstr ""
-
 msgid "PackageRegistry|Package asset deleted successfully"
 msgstr ""
 
@@ -34993,9 +34984,6 @@ msgstr ""
 msgid "PackageRegistry|Published to the %{project} Package Registry %{datetime}"
 msgstr ""
 
-msgid "PackageRegistry|Push Protected Up To Access Level"
-msgstr ""
-
 msgid "PackageRegistry|Push protected up to access level"
 msgstr ""
 
@@ -35017,7 +35005,7 @@ msgstr ""
 msgid "PackageRegistry|RubyGems"
 msgstr ""
 
-msgid "PackageRegistry|Rule saved successfully"
+msgid "PackageRegistry|Rule saved."
 msgstr ""
 
 msgid "PackageRegistry|Show Composer commands"
@@ -35062,7 +35050,7 @@ msgstr ""
 msgid "PackageRegistry|Something went wrong while fetching the package metadata."
 msgstr ""
 
-msgid "PackageRegistry|Something went wrong while saving the package protection rule"
+msgid "PackageRegistry|Something went wrong while saving the package protection rule."
 msgstr ""
 
 msgid "PackageRegistry|Sorry, your filter produced no results"
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
index 6bf00e23bed3d7..28ea73ab6fee67 100644
--- a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
+++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
@@ -214,8 +214,8 @@ describe('Packages Protection Rule Form', () => {
         await submitForm();
 
         expect(findAlert().isVisible()).toBe(true);
-        expect(findAlert().text()).toBe(
-          'Something went wrong while saving the package protection rule',
+        expect(findAlert().text()).toMatch(
+          /something went wrong while saving the package protection rule/i,
         );
       });
     });
-- 
GitLab


From f431a9ec8339e0e45b19b6e6acacf0f110c61c9f Mon Sep 17 00:00:00 2001
From: Gerardo <gerardo@b310.de>
Date: Tue, 23 Jan 2024 09:40:57 +0100
Subject: [PATCH 6/6] refactor: Apply suggestion from @seggenberger @fguibert

Commit includes:
- Suggestions regarding styling
- Extracting constants from the code
---
 .../components/packages_protection_rule_form.vue    | 13 +++++++++----
 .../components/packages_protection_rules.vue        |  1 -
 .../packages_protection_rule_form_spec.js           |  6 +++++-
 .../settings/project/settings/mock_data.js          |  4 ++--
 4 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
index 9032c9c9c37a21..7c27eacd190ef7 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
+++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue
@@ -8,6 +8,10 @@ const PACKAGES_PROTECTION_RULES_SAVED_ERROR_MESSAGE = s__(
   'PackageRegistry|Something went wrong while saving the package protection rule.',
 );
 
+const GRAPHQL_ACCESS_LEVEL_VALUE_MAINTAINER = 'MAINTAINER';
+const GRAPHQL_ACCESS_LEVEL_VALUE_DEVELOPER = 'DEVELOPER';
+const GRAPHQL_ACCESS_LEVEL_VALUE_OWNER = 'OWNER';
+
 export default {
   components: {
     GlButton,
@@ -27,7 +31,7 @@ export default {
       packageProtectionRuleFormData: {
         packageNamePattern: '',
         packageType: 'NPM',
-        pushProtectedUpToAccessLevel: 'MAINTAINER',
+        pushProtectedUpToAccessLevel: GRAPHQL_ACCESS_LEVEL_VALUE_DEVELOPER,
       },
       updateInProgress: false,
       alertErrorMessage: '',
@@ -60,8 +64,9 @@ export default {
     },
     pushProtectedUpToAccessLevelOptions() {
       return [
-        { value: 'MAINTAINER', text: __('Maintainer') },
-        { value: 'OWNER', text: __('Owner') },
+        { value: GRAPHQL_ACCESS_LEVEL_VALUE_DEVELOPER, text: __('Developer') },
+        { value: GRAPHQL_ACCESS_LEVEL_VALUE_MAINTAINER, text: __('Maintainer') },
+        { value: GRAPHQL_ACCESS_LEVEL_VALUE_OWNER, text: __('Owner') },
       ];
     },
   },
@@ -109,7 +114,7 @@ export default {
     <gl-form @submit.prevent="submit" @reset="cancelForm">
       <gl-alert
         v-if="alertErrorMessage"
-        class="mb-3"
+        class="gl-mb-5"
         variant="danger"
         @dismiss="clearAlertErrorMessage"
       >
diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue
index 07997c4a8960d5..3391524a25d9bd 100644
--- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue
+++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue
@@ -135,7 +135,6 @@ export default {
             :fields="$options.fields"
             show-empty
             stacked="md"
-            class="mb-3"
             :busy="isLoadingPackageProtectionRules"
           >
             <template #table-busy>
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
index 28ea73ab6fee67..7697b7f6bd7892 100644
--- a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
+++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js
@@ -74,7 +74,11 @@ describe('Packages Protection Rule Form', () => {
         const pushProtectedUpToAccessLevelSelectOptions = findPushProtectedUpToAccessLevelSelect()
           .findAll('option')
           .wrappers.map((option) => option.element.value);
-        expect(pushProtectedUpToAccessLevelSelectOptions).toEqual(['MAINTAINER', 'OWNER']);
+        expect(pushProtectedUpToAccessLevelSelectOptions).toEqual([
+          'DEVELOPER',
+          'MAINTAINER',
+          'OWNER',
+        ]);
       });
     });
 
diff --git a/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js b/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js
index e187a31636cf1c..a8133c0ace6a1a 100644
--- a/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js
+++ b/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js
@@ -126,9 +126,9 @@ export const createPackagesProtectionRuleMutationPayload = ({ override, errors =
 });
 
 export const createPackagesProtectionRuleMutationInput = {
-  packageNamePattern: `@flight/flight-maintainer-14-*`,
+  packageNamePattern: `@flight/flight-developer-14-*`,
   packageType: 'NPM',
-  pushProtectedUpToAccessLevel: 'MAINTAINER',
+  pushProtectedUpToAccessLevel: 'DEVELOPER',
 };
 
 export const createPackagesProtectionRuleMutationPayloadErrors = [
-- 
GitLab