From bd74cb779152b9cbe5b9469ef373065a6d1ad2c6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonas=20W=C3=A4lter?= <jonas.waelter@noser.com>
Date: Thu, 1 Dec 2022 10:43:59 +0000
Subject: [PATCH] Redesign Package Registry project setting

Changelog: changed
---
 .../permissions/components/settings_panel.vue | 88 ++++++++++++++-----
 locale/gitlab.pot                             | 12 ++-
 .../settings/packages_settings_spec.rb        |  4 +
 .../components/settings_panel_spec.js         | 74 ++++++++++++----
 4 files changed, 137 insertions(+), 41 deletions(-)

diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
index b344c4f18910d..2070f942e2d8c 100644
--- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
+++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
@@ -23,6 +23,12 @@ import ProjectSettingRow from './project_setting_row.vue';
 
 const FEATURE_ACCESS_LEVEL_ANONYMOUS = [30, s__('ProjectSettings|Everyone')];
 
+const PACKAGE_REGISTRY_ACCESS_LEVEL_DEFAULT_BY_PROJECT_VISIBILITY = {
+  [VISIBILITY_LEVEL_PRIVATE_INTEGER]: featureAccessLevel.PROJECT_MEMBERS,
+  [VISIBILITY_LEVEL_INTERNAL_INTEGER]: featureAccessLevel.EVERYONE,
+  [VISIBILITY_LEVEL_PUBLIC_INTEGER]: FEATURE_ACCESS_LEVEL_ANONYMOUS[0],
+};
+
 export default {
   i18n: {
     ...CVE_ID_REQUEST_BUTTON_I18N,
@@ -47,11 +53,15 @@ export default {
     packagesHelpText: s__(
       'ProjectSettings|Every project can have its own space to store its packages. Note: The Package Registry is always visible when a project is public.',
     ),
-    packageRegistryHelpText: s__(
-      'ProjectSettings|Every project can have its own space to store its packages.',
+    packageRegistryHelpText: s__('ProjectSettings|Publish, store, and view packages in a project.'),
+    packageRegistryForEveryoneHelpText: s__(
+      'ProjectSettings|Anyone can pull packages with a package manager API.',
     ),
     packagesLabel: s__('ProjectSettings|Packages'),
     packageRegistryLabel: s__('ProjectSettings|Package registry'),
+    packageRegistryForEveryoneLabel: s__(
+      'ProjectSettings|Allow anyone to pull from Package Registry',
+    ),
     pagesLabel: s__('ProjectSettings|Pages'),
     ciCdLabel: __('CI/CD'),
     repositoryLabel: s__('ProjectSettings|Repository'),
@@ -287,18 +297,6 @@ export default {
       );
     },
 
-    packageRegistryFeatureAccessLevelOptions() {
-      const options = [FEATURE_ACCESS_LEVEL_ANONYMOUS];
-
-      if (this.visibilityLevel === VISIBILITY_LEVEL_PRIVATE_INTEGER) {
-        options.unshift(featureAccessLevelMembers);
-      } else if (this.visibilityLevel === VISIBILITY_LEVEL_INTERNAL_INTEGER) {
-        options.unshift(featureAccessLevelEveryone);
-      }
-
-      return options;
-    },
-
     pagesFeatureAccessLevelOptions() {
       const options = [featureAccessLevelMembers];
 
@@ -366,6 +364,15 @@ export default {
     packageRegistryAccessLevelEnabled() {
       return this.glFeatures.packageRegistryAccessLevel;
     },
+    packageRegistryEnabled() {
+      return this.packageRegistryAccessLevel > featureAccessLevel.NOT_ENABLED;
+    },
+    packageRegistryApiForEveryoneEnabled() {
+      return this.packageRegistryAccessLevel === FEATURE_ACCESS_LEVEL_ANONYMOUS[0];
+    },
+    packageRegistryApiForEveryoneEnabledShown() {
+      return this.visibilityLevel !== VISIBILITY_LEVEL_PUBLIC_INTEGER;
+    },
     splitOperationsEnabled() {
       return this.glFeatures.splitOperationsVisibilityPermissions;
     },
@@ -474,9 +481,8 @@ export default {
           this.packageRegistryAccessLevelEnabled &&
           this.packageRegistryAccessLevel === featureAccessLevel.PROJECT_MEMBERS
         ) {
-          this.packageRegistryAccessLevel = Math.min(
-            ...this.packageRegistryFeatureAccessLevelOptions.map((option) => option[0]),
-          );
+          this.packageRegistryAccessLevel =
+            PACKAGE_REGISTRY_ACCESS_LEVEL_DEFAULT_BY_PROJECT_VISIBILITY[value];
         }
         if (this.buildsAccessLevel > featureAccessLevel.NOT_ENABLED)
           this.buildsAccessLevel = featureAccessLevel.EVERYONE;
@@ -561,6 +567,22 @@ export default {
     visibilityAllowed(option) {
       return this.allowedVisibilityOptions.includes(option);
     },
+    onPackageRegistryEnabledToggle(value) {
+      this.packageRegistryAccessLevel = value
+        ? this.packageRegistryAccessLevelDefault()
+        : featureAccessLevel.NOT_ENABLED;
+    },
+    onPackageRegistryApiForEveryoneEnabledToggle(value) {
+      this.packageRegistryAccessLevel = value
+        ? FEATURE_ACCESS_LEVEL_ANONYMOUS[0]
+        : this.packageRegistryAccessLevelDefault();
+    },
+    packageRegistryAccessLevelDefault() {
+      return (
+        PACKAGE_REGISTRY_ACCESS_LEVEL_DEFAULT_BY_PROJECT_VISIBILITY[this.visibilityLevel] ??
+        featureAccessLevel.NOT_ENABLED
+      );
+    },
   },
 };
 </script>
@@ -897,10 +919,36 @@ export default {
         :help-text="$options.i18n.packageRegistryHelpText"
         data-testid="package-registry-access-level"
       >
-        <project-feature-setting
-          v-model="packageRegistryAccessLevel"
+        <gl-toggle
+          class="gl-my-2"
+          :value="packageRegistryEnabled"
           :label="$options.i18n.packageRegistryLabel"
-          :options="packageRegistryFeatureAccessLevelOptions"
+          label-position="hidden"
+          name="package_registry_enabled"
+          @change="onPackageRegistryEnabledToggle"
+        />
+        <div
+          v-if="packageRegistryApiForEveryoneEnabledShown"
+          class="project-feature-setting-group gl-pl-7 gl-sm-pl-5 gl-my-3"
+        >
+          <project-setting-row
+            :label="$options.i18n.packageRegistryForEveryoneLabel"
+            :help-text="$options.i18n.packageRegistryForEveryoneHelpText"
+          >
+            <gl-toggle
+              class="gl-my-2"
+              :value="packageRegistryApiForEveryoneEnabled"
+              :disabled="!packageRegistryEnabled"
+              :label="$options.i18n.packageRegistryForEveryoneLabel"
+              label-position="hidden"
+              name="package_registry_api_for_everyone_enabled"
+              @change="onPackageRegistryApiForEveryoneEnabledToggle"
+            />
+          </project-setting-row>
+        </div>
+        <input
+          :value="packageRegistryAccessLevel"
+          type="hidden"
           name="project[project_feature_attributes][package_registry_access_level]"
         />
       </project-setting-row>
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 2317d81bc6377..1f00acba2ea77 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -32176,12 +32176,18 @@ msgstr ""
 msgid "ProjectSettings|Allow"
 msgstr ""
 
+msgid "ProjectSettings|Allow anyone to pull from Package Registry"
+msgstr ""
+
 msgid "ProjectSettings|Always show thumbs-up and thumbs-down award emoji buttons on issues, merge requests, and snippets."
 msgstr ""
 
 msgid "ProjectSettings|Analytics"
 msgstr ""
 
+msgid "ProjectSettings|Anyone can pull packages with a package manager API."
+msgstr ""
+
 msgid "ProjectSettings|Auto-close referenced issues on default branch"
 msgstr ""
 
@@ -32263,9 +32269,6 @@ msgstr ""
 msgid "ProjectSettings|Every project can have its own space to store its Docker images"
 msgstr ""
 
-msgid "ProjectSettings|Every project can have its own space to store its packages."
-msgstr ""
-
 msgid "ProjectSettings|Every project can have its own space to store its packages. Note: The Package Registry is always visible when a project is public."
 msgstr ""
 
@@ -32425,6 +32428,9 @@ msgstr ""
 msgid "ProjectSettings|Public"
 msgstr ""
 
+msgid "ProjectSettings|Publish, store, and view packages in a project."
+msgstr ""
+
 msgid "ProjectSettings|Releases"
 msgstr ""
 
diff --git a/spec/features/projects/settings/packages_settings_spec.rb b/spec/features/projects/settings/packages_settings_spec.rb
index 1c2b0faa21505..57e9816ddd170 100644
--- a/spec/features/projects/settings/packages_settings_spec.rb
+++ b/spec/features/projects/settings/packages_settings_spec.rb
@@ -33,6 +33,10 @@
 
       it 'displays the packages access level setting' do
         expect(page).to have_selector('[data-testid="package-registry-access-level"] > label', text: 'Package registry')
+        expect(page).to have_selector('input[name="package_registry_enabled"]', visible: false)
+        expect(page).to have_selector('input[name="package_registry_enabled"] + button', visible: true)
+        expect(page).to have_selector('input[name="package_registry_api_for_everyone_enabled"]', visible: false)
+        expect(page).to have_selector('input[name="package_registry_api_for_everyone_enabled"] + button', visible: true)
         expect(page).to have_selector(
           'input[name="project[project_feature_attributes][package_registry_access_level]"]',
           visible: false
diff --git a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
index 155f1ff4ef3e3..8e0ca544a1c62 100644
--- a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
+++ b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
@@ -114,9 +114,14 @@ describe('Settings Panel', () => {
   const findPackageSettings = () => wrapper.findComponent({ ref: 'package-settings' });
   const findPackageAccessLevel = () =>
     wrapper.find('[data-testid="package-registry-access-level"]');
-  const findPackageAccessLevels = () =>
-    wrapper.find('[name="project[project_feature_attributes][package_registry_access_level]"]');
   const findPackagesEnabledInput = () => wrapper.find('[name="project[packages_enabled]"]');
+  const findPackageRegistryEnabledInput = () => wrapper.find('[name="package_registry_enabled"]');
+  const findPackageRegistryAccessLevelHiddenInput = () =>
+    wrapper.find(
+      'input[name="project[project_feature_attributes][package_registry_access_level]"]',
+    );
+  const findPackageRegistryApiForEveryoneEnabledInput = () =>
+    wrapper.find('[name="package_registry_api_for_everyone_enabled"]');
   const findPagesSettings = () => wrapper.findComponent({ ref: 'pages-settings' });
   const findPagesAccessLevels = () =>
     wrapper.find('[name="project[project_feature_attributes][pages_access_level]"]');
@@ -587,28 +592,63 @@ describe('Settings Panel', () => {
         expect(findPackageAccessLevel().exists()).toBe(true);
       });
 
+      it('has hidden input field for package registry access level', () => {
+        wrapper = mountComponent({
+          glFeatures: { packageRegistryAccessLevel: true },
+          packagesAvailable: true,
+        });
+
+        expect(findPackageRegistryAccessLevelHiddenInput().exists()).toBe(true);
+      });
+
       it.each`
-        visibilityLevel                      | output
-        ${VISIBILITY_LEVEL_PRIVATE_INTEGER}  | ${[[featureAccessLevel.PROJECT_MEMBERS, 'Only Project Members'], [30, 'Everyone']]}
-        ${VISIBILITY_LEVEL_INTERNAL_INTEGER} | ${[[featureAccessLevel.EVERYONE, 'Everyone With Access'], [30, 'Everyone']]}
-        ${VISIBILITY_LEVEL_PUBLIC_INTEGER}   | ${[[30, 'Everyone']]}
+        projectVisibilityLevel               | packageRegistryEnabled | packageRegistryApiForEveryoneEnabled | expectedAccessLevel
+        ${VISIBILITY_LEVEL_PRIVATE_INTEGER}  | ${false}               | ${'disabled'}                        | ${featureAccessLevel.NOT_ENABLED}
+        ${VISIBILITY_LEVEL_PRIVATE_INTEGER}  | ${true}                | ${false}                             | ${featureAccessLevel.PROJECT_MEMBERS}
+        ${VISIBILITY_LEVEL_PRIVATE_INTEGER}  | ${true}                | ${true}                              | ${FEATURE_ACCESS_LEVEL_ANONYMOUS}
+        ${VISIBILITY_LEVEL_INTERNAL_INTEGER} | ${false}               | ${'disabled'}                        | ${featureAccessLevel.NOT_ENABLED}
+        ${VISIBILITY_LEVEL_INTERNAL_INTEGER} | ${true}                | ${false}                             | ${featureAccessLevel.EVERYONE}
+        ${VISIBILITY_LEVEL_INTERNAL_INTEGER} | ${true}                | ${true}                              | ${FEATURE_ACCESS_LEVEL_ANONYMOUS}
+        ${VISIBILITY_LEVEL_PUBLIC_INTEGER}   | ${false}               | ${'hidden'}                          | ${featureAccessLevel.NOT_ENABLED}
+        ${VISIBILITY_LEVEL_PUBLIC_INTEGER}   | ${true}                | ${'hidden'}                          | ${FEATURE_ACCESS_LEVEL_ANONYMOUS}
       `(
-        'renders correct options when visibilityLevel is $visibilityLevel',
-        async ({ visibilityLevel, output }) => {
+        'sets correct access level',
+        async ({
+          projectVisibilityLevel,
+          packageRegistryEnabled,
+          packageRegistryApiForEveryoneEnabled,
+          expectedAccessLevel,
+        }) => {
           wrapper = mountComponent({
             glFeatures: { packageRegistryAccessLevel: true },
             packagesAvailable: true,
             currentSettings: {
-              visibilityLevel,
+              visibilityLevel: projectVisibilityLevel,
             },
           });
 
-          expect(findPackageAccessLevels().props('options')).toStrictEqual(output);
+          await findPackageRegistryEnabledInput().vm.$emit('change', packageRegistryEnabled);
+
+          const packageRegistryApiForEveryoneEnabledInput = findPackageRegistryApiForEveryoneEnabledInput();
+
+          if (packageRegistryApiForEveryoneEnabled === 'hidden') {
+            expect(packageRegistryApiForEveryoneEnabledInput.exists()).toBe(false);
+          } else if (packageRegistryApiForEveryoneEnabled === 'disabled') {
+            expect(packageRegistryApiForEveryoneEnabledInput.props('disabled')).toBe(true);
+          } else {
+            expect(packageRegistryApiForEveryoneEnabledInput.props('disabled')).toBe(false);
+            await packageRegistryApiForEveryoneEnabledInput.vm.$emit(
+              'change',
+              packageRegistryApiForEveryoneEnabled,
+            );
+          }
+
+          expect(wrapper.vm.packageRegistryAccessLevel).toBe(expectedAccessLevel);
         },
       );
 
       it.each`
-        initialProjectVisibilityLevel        | newProjectVisibilityLevel            | initialPackageRegistryOption          | expectedPackageRegistryOption
+        initialProjectVisibilityLevel        | newProjectVisibilityLevel            | initialAccessLevel                    | expectedAccessLevel
         ${VISIBILITY_LEVEL_PRIVATE_INTEGER}  | ${VISIBILITY_LEVEL_INTERNAL_INTEGER} | ${featureAccessLevel.NOT_ENABLED}     | ${featureAccessLevel.NOT_ENABLED}
         ${VISIBILITY_LEVEL_PRIVATE_INTEGER}  | ${VISIBILITY_LEVEL_INTERNAL_INTEGER} | ${featureAccessLevel.PROJECT_MEMBERS} | ${featureAccessLevel.EVERYONE}
         ${VISIBILITY_LEVEL_PRIVATE_INTEGER}  | ${VISIBILITY_LEVEL_INTERNAL_INTEGER} | ${FEATURE_ACCESS_LEVEL_ANONYMOUS}     | ${FEATURE_ACCESS_LEVEL_ANONYMOUS}
@@ -626,27 +666,25 @@ describe('Settings Panel', () => {
         ${VISIBILITY_LEVEL_PUBLIC_INTEGER}   | ${VISIBILITY_LEVEL_INTERNAL_INTEGER} | ${featureAccessLevel.NOT_ENABLED}     | ${featureAccessLevel.NOT_ENABLED}
         ${VISIBILITY_LEVEL_PUBLIC_INTEGER}   | ${VISIBILITY_LEVEL_INTERNAL_INTEGER} | ${FEATURE_ACCESS_LEVEL_ANONYMOUS}     | ${featureAccessLevel.EVERYONE}
       `(
-        'changes option from $initialPackageRegistryOption to $expectedPackageRegistryOption when visibilityLevel changed from $initialProjectVisibilityLevel to $newProjectVisibilityLevel',
+        'changes access level when project visibility level changed',
         async ({
           initialProjectVisibilityLevel,
           newProjectVisibilityLevel,
-          initialPackageRegistryOption,
-          expectedPackageRegistryOption,
+          initialAccessLevel,
+          expectedAccessLevel,
         }) => {
           wrapper = mountComponent({
             glFeatures: { packageRegistryAccessLevel: true },
             packagesAvailable: true,
             currentSettings: {
               visibilityLevel: initialProjectVisibilityLevel,
-              packageRegistryAccessLevel: initialPackageRegistryOption,
+              packageRegistryAccessLevel: initialAccessLevel,
             },
           });
 
           await findProjectVisibilityLevelInput().setValue(newProjectVisibilityLevel);
 
-          expect(findPackageAccessLevels().props('value')).toStrictEqual(
-            expectedPackageRegistryOption,
-          );
+          expect(wrapper.vm.packageRegistryAccessLevel).toBe(expectedAccessLevel);
         },
       );
     });
-- 
GitLab