Merge branch '351200-leipert-fix-toggle-logic' into 'master'

Fix project permission toggle behavior

See merge request gitlab-org/gitlab!83503

app/assets/javascripts/pages/projects/shared/permissions/components/project_feature_setting.vue

@@ -44,6 +44,15 @@ export default {
     },
   },
   computed: {
+    internalValue: {
+      get() {
+        return this.value;
+      },
+      set(value) {
+        this.$emit('change', value);
+      },
+    },
+
     featureEnabled() {
       return this.value !== 0;
     },
@@ -68,10 +77,6 @@ export default {
         this.$emit('change', firstOptionValue);
       }
     },
-
-    selectOption(e) {
-      this.$emit('change', Number(e.target.value));
-    },
   },
 };
 </script>
@@ -93,15 +98,14 @@ export default {
     />
     <div class="select-wrapper gl-flex-grow-1">
       <select
+        v-model="internalValue"
         :disabled="displaySelectInput"
         class="form-control project-repo-select select-control"
-        @change="selectOption"
       >
         <option
           v-for="[optionValue, optionName] in displayOptions"
           :key="optionValue"
           :value="optionValue"
-          :selected="optionValue === value"
         >
           {{ optionName }}
         </option>

app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue

@@ -9,7 +9,6 @@ import {
   featureAccessLevelMembers,
   featureAccessLevelEveryone,
   featureAccessLevel,
-  featureAccessLevelNone,
   CVE_ID_REQUEST_BUTTON_I18N,
   featureAccessLevelDescriptions,
 } from '../constants';
@@ -225,8 +224,6 @@ export default {
     },
 
     operationsFeatureAccessLevelOptions() {
-      if (!this.operationsEnabled) return [featureAccessLevelNone];
-
       return this.featureAccessLevelOptions.filter(
         ([value]) => value <= this.operationsAccessLevel,
       );
@@ -251,10 +248,6 @@ export default {
       return options;
     },
 
-    metricsOptionsDropdownDisabled() {
-      return this.operationsFeatureAccessLevelOptions.length < 2 || !this.operationsEnabled;
-    },
-
     operationsEnabled() {
       return this.operationsAccessLevel > featureAccessLevel.NOT_ENABLED;
     },
@@ -392,6 +385,15 @@ export default {
       else if (oldValue === featureAccessLevel.NOT_ENABLED)
         toggleHiddenClassBySelector('.merge-requests-feature', false);
     },
+
+    operationsAccessLevel(value, oldValue) {
+      if (value < oldValue) {
+        // sub-features cannot have more permissive access level
+        this.metricsDashboardAccessLevel = Math.min(this.metricsDashboardAccessLevel, value);
+      } else if (oldValue === 0) {
+        this.metricsDashboardAccessLevel = value;
+      }
+    },
   },
 
   methods: {

spec/frontend/pages/projects/shared/permissions/components/project_feature_settings_spec.js

@@ -3,15 +3,17 @@ import { shallowMount } from '@vue/test-utils';
 import ProjectFeatureSetting from '~/pages/projects/shared/permissions/components/project_feature_setting.vue';
 
 describe('Project Feature Settings', () => {
+  const defaultOptions = [
+    [1, 1],
+    [2, 2],
+    [3, 3],
+    [4, 4],
+    [5, 5],
+  ];
+
   const defaultProps = {
     name: 'Test',
-    options: [
-      [1, 1],
-      [2, 2],
-      [3, 3],
-      [4, 4],
-      [5, 5],
-    ],
+    options: defaultOptions,
     value: 1,
     disabledInput: false,
     showToggle: true,
@@ -110,15 +112,25 @@ describe('Project Feature Settings', () => {
       },
     );
 
-    it('should emit the change when a new option is selected', () => {
+    it('should emit the change when a new option is selected', async () => {
       wrapper = mountComponent();
 
       expect(wrapper.emitted('change')).toBeUndefined();
 
-      wrapper.findAll('option').at(1).trigger('change');
+      await wrapper.findAll('option').at(1).setSelected();
 
       expect(wrapper.emitted('change')).toHaveLength(1);
       expect(wrapper.emitted('change')[0]).toEqual([2]);
     });
+
+    it('value of select matches prop `value` if options are modified', async () => {
+      wrapper = mountComponent();
+
+      await wrapper.setProps({ value: 0, options: [[0, 0]] });
+      expect(wrapper.find('select').element.selectedIndex).toBe(0);
+
+      await wrapper.setProps({ value: 2, options: defaultOptions });
+      expect(wrapper.find('select').element.selectedIndex).toBe(1);
+    });
   });
 });

spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js

 import { GlSprintf, GlToggle } from '@gitlab/ui';
 import { shallowMount, mount } from '@vue/test-utils';
-import projectFeatureSetting from '~/pages/projects/shared/permissions/components/project_feature_setting.vue';
+import ProjectFeatureSetting from '~/pages/projects/shared/permissions/components/project_feature_setting.vue';
 import settingsPanel from '~/pages/projects/shared/permissions/components/settings_panel.vue';
 import {
   featureAccessLevel,
@@ -21,6 +21,7 @@ const defaultProps = {
     wikiAccessLevel: 20,
     snippetsAccessLevel: 20,
     operationsAccessLevel: 20,
+    metricsDashboardAccessLevel: 20,
     pagesAccessLevel: 10,
     analyticsAccessLevel: 20,
     containerRegistryAccessLevel: 20,
@@ -75,7 +76,7 @@ describe('Settings Panel', () => {
   const findLFSFeatureToggle = () => findLFSSettingsRow().find(GlToggle);
   const findRepositoryFeatureProjectRow = () => wrapper.find({ ref: 'repository-settings' });
   const findRepositoryFeatureSetting = () =>
-    findRepositoryFeatureProjectRow().find(projectFeatureSetting);
+    findRepositoryFeatureProjectRow().find(ProjectFeatureSetting);
   const findProjectVisibilitySettings = () => wrapper.find({ ref: 'project-visibility-settings' });
   const findIssuesSettingsRow = () => wrapper.find({ ref: 'issues-settings' });
   const findAnalyticsRow = () => wrapper.find({ ref: 'analytics-settings' });
@@ -106,7 +107,11 @@ describe('Settings Panel', () => {
       'input[name="project[project_setting_attributes][warn_about_potentially_unwanted_characters]"]',
     );
   const findMetricsVisibilitySettings = () => wrapper.find({ ref: 'metrics-visibility-settings' });
+  const findMetricsVisibilityInput = () =>
+    findMetricsVisibilitySettings().findComponent(ProjectFeatureSetting);
   const findOperationsSettings = () => wrapper.find({ ref: 'operations-settings' });
+  const findOperationsVisibilityInput = () =>
+    findOperationsSettings().findComponent(ProjectFeatureSetting);
   const findConfirmDangerButton = () => wrapper.findComponent(ConfirmDanger);
 
   afterEach(() => {
@@ -595,7 +600,7 @@ describe('Settings Panel', () => {
   });
 
   describe('Metrics dashboard', () => {
-    it('should show the metrics dashboard access toggle', () => {
+    it('should show the metrics dashboard access select', () => {
       wrapper = mountComponent();
 
       expect(findMetricsVisibilitySettings().exists()).toBe(true);
@@ -610,23 +615,51 @@ describe('Settings Panel', () => {
     });
 
     it.each`
-      scenario                                                                          | selectedOption                                | selectedOptionLabel
-      ${{ currentSettings: { visibilityLevel: visibilityOptions.PRIVATE } }}            | ${String(featureAccessLevel.PROJECT_MEMBERS)} | ${'Only Project Members'}
-      ${{ currentSettings: { operationsAccessLevel: featureAccessLevel.NOT_ENABLED } }} | ${String(featureAccessLevel.NOT_ENABLED)}     | ${'Enable feature to choose access level'}
+      before                                | after
+      ${featureAccessLevel.NOT_ENABLED}     | ${featureAccessLevel.EVERYONE}
+      ${featureAccessLevel.NOT_ENABLED}     | ${featureAccessLevel.PROJECT_MEMBERS}
+      ${featureAccessLevel.EVERYONE}        | ${featureAccessLevel.PROJECT_MEMBERS}
+      ${featureAccessLevel.EVERYONE}        | ${featureAccessLevel.NOT_ENABLED}
+      ${featureAccessLevel.PROJECT_MEMBERS} | ${featureAccessLevel.NOT_ENABLED}
     `(
-      'should disable the metrics visibility dropdown when #scenario',
-      ({ scenario, selectedOption, selectedOptionLabel }) => {
-        wrapper = mountComponent(scenario, mount);
+      'when updating Operations Settings access level from `$before` to `$after`, Metric Dashboard access is updated to `$after` as well',
+      async ({ before, after }) => {
+        wrapper = mountComponent({
+          currentSettings: { operationsAccessLevel: before, metricsDashboardAccessLevel: before },
+        });
 
-        const select = findMetricsVisibilitySettings().find('select');
-        const option = select.find('option');
+        await findOperationsVisibilityInput().vm.$emit('change', after);
 
-        expect(select.attributes('disabled')).toBe('disabled');
-        expect(select.element.value).toBe(selectedOption);
-        expect(option.attributes('value')).toBe(selectedOption);
-        expect(option.text()).toBe(selectedOptionLabel);
+        expect(findMetricsVisibilityInput().props('value')).toBe(after);
       },
     );
+
+    it('when updating Operations Settings access level from `10` to `20`, Metric Dashboard access is not increased', async () => {
+      wrapper = mountComponent({
+        currentSettings: {
+          operationsAccessLevel: featureAccessLevel.PROJECT_MEMBERS,
+          metricsDashboardAccessLevel: featureAccessLevel.PROJECT_MEMBERS,
+        },
+      });
+
+      await findOperationsVisibilityInput().vm.$emit('change', featureAccessLevel.EVERYONE);
+
+      expect(findMetricsVisibilityInput().props('value')).toBe(featureAccessLevel.PROJECT_MEMBERS);
+    });
+
+    it('should reduce Metrics visibility level when visibility is set to private', async () => {
+      wrapper = mountComponent({
+        currentSettings: {
+          visibilityLevel: visibilityOptions.PUBLIC,
+          operationsAccessLevel: featureAccessLevel.EVERYONE,
+          metricsDashboardAccessLevel: featureAccessLevel.EVERYONE,
+        },
+      });
+
+      await findProjectVisibilityLevelInput().setValue(visibilityOptions.PRIVATE);
+
+      expect(findMetricsVisibilityInput().props('value')).toBe(featureAccessLevel.PROJECT_MEMBERS);
+    });
   });
 
   describe('Analytics', () => {