Commit eda15b06 authored by Lukas 'Eipi' Eipert's avatar Lukas 'Eipi' Eipert Committed by Frédéric Caplette

Fix project permission toggle behavior

Problem:

```
Given that you are on public project
When you disable a permission
  And enable it again
Then the dropdown shows that you selected the last value on the list
While the internal input has another value set.
```

This has to do with a quirky behavior where we replace the `value` and
the `options` provided to the `<select>` inside of the
ProjectFeatureSettings input component. Internally we used the
`selected` attribute of the `<option>` elements to track which option
was selected. That attribute is just for initial rendering though:

> If present, this Boolean attribute indicates that the option is
> initially selected. If the `<option>` element is the descendant of a
> `<select>` element whose multiple attribute is not set, only one
> single `<option>` of this `<select>` element may have the selected
> attribute.

This seems to be non-problematic if one never changes the options that
are availabe for the specific select.

The solution for this is to create a computed property with a getter and
setter and use that computed property as a v-model for the internal
select.

Changelog: fixed
parent 4b34e10d
......@@ -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>
......
......@@ -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: {
......
......@@ -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 defaultProps = {
name: 'Test',
options: [
const defaultOptions = [
[1, 1],
[2, 2],
[3, 3],
[4, 4],
[5, 5],
],
];
const defaultProps = {
name: 'Test',
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);
});
});
});
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', () => {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment