From b7821d5b764a354f78d9fb45ca1b033cf298e37b Mon Sep 17 00:00:00 2001 From: Artur Fedorov <afedorov@gitlab.com> Date: Wed, 7 Feb 2024 00:13:10 +0000 Subject: [PATCH] This MR fixes wrong wording for policy scope Duplicated words projects were removed from UI sentence Changelog: fixed EE: true --- .../components/group_projects_dropdown.vue | 10 ++++---- .../policy_editor/rule_multi_select.vue | 6 ++++- .../scope/compliance_framework_dropdown.vue | 11 ++++---- .../policy_editor/scope/scope_section.vue | 4 +-- .../components/policy_editor/utils.js | 25 ++++++++++++++----- .../compliance_framework_dropdown_spec.js | 2 +- .../policy_editor/scope/scope_section_spec.js | 3 +++ .../components/policy_editor/utils_spec.js | 24 ++++++++++-------- locale/gitlab.pot | 4 +-- 9 files changed, 57 insertions(+), 32 deletions(-) diff --git a/ee/app/assets/javascripts/security_orchestration/components/group_projects_dropdown.vue b/ee/app/assets/javascripts/security_orchestration/components/group_projects_dropdown.vue index 416744e2877f6..8d0ac42061958 100644 --- a/ee/app/assets/javascripts/security_orchestration/components/group_projects_dropdown.vue +++ b/ee/app/assets/javascripts/security_orchestration/components/group_projects_dropdown.vue @@ -80,11 +80,11 @@ export default { return this.selected; }, dropdownPlaceholder() { - return renderMultiSelectText( - this.formattedSelectedProjectsIds, - this.projectItems, - __('projects'), - ); + return renderMultiSelectText({ + selected: this.formattedSelectedProjectsIds, + items: this.projectItems, + itemTypeName: __('projects'), + }); }, loading() { return this.$apollo.queries.projects.loading; diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/rule_multi_select.vue b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/rule_multi_select.vue index 5a709f64a1ec7..fd5807cd2af59 100644 --- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/rule_multi_select.vue +++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/rule_multi_select.vue @@ -47,7 +47,11 @@ export default { return this.includeSelectAll ? this.$options.i18n.selectAllLabel : ''; }, text() { - return renderMultiSelectText(this.selected, this.items, this.itemTypeName); + return renderMultiSelectText({ + selected: this.selected, + items: this.items, + itemTypeName: this.itemTypeName, + }); }, itemsKeys() { return Object.keys(this.items); diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scope/compliance_framework_dropdown.vue b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scope/compliance_framework_dropdown.vue index 7caa8deafa53f..2ca1d807226e7 100644 --- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scope/compliance_framework_dropdown.vue +++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scope/compliance_framework_dropdown.vue @@ -112,11 +112,12 @@ export default { }, {}); }, dropdownPlaceholder() { - return renderMultiSelectText( - this.formattedSelectedFrameworkIds, - this.complianceFrameworkItems, - this.$options.i18n.complianceFrameworkTypeName, - ); + return renderMultiSelectText({ + selected: this.formattedSelectedFrameworkIds, + items: this.complianceFrameworkItems, + itemTypeName: this.$options.i18n.complianceFrameworkTypeName, + useAllSelected: false, + }); }, listBoxItems() { return ( diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scope/scope_section.vue b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scope/scope_section.vue index 6572161ba86de..80db3a5cb7461 100644 --- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scope/scope_section.vue +++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scope/scope_section.vue @@ -29,10 +29,10 @@ export default { EXCEPTION_TYPE_LISTBOX_ITEMS, i18n: { policyScopeFrameworkCopy: s__( - `SecurityOrchestration|Apply this policy to all projects %{projectScopeType} named %{frameworkSelector}`, + `SecurityOrchestration|Apply this policy to %{projectScopeType}named %{frameworkSelector}`, ), policyScopeProjectCopy: s__( - `SecurityOrchestration|Apply this policy to all projects %{projectScopeType} %{exceptionType} %{projectSelector}`, + `SecurityOrchestration|Apply this policy to %{projectScopeType} %{exceptionType} %{projectSelector}`, ), groupProjectErrorDescription: s__('SecurityOrchestration|Failed to load group projects'), complianceFrameworkErrorDescription: s__( diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/utils.js b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/utils.js index a3352096697ec..307d4a7fec375 100644 --- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/utils.js +++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/utils.js @@ -241,9 +241,10 @@ const ONE_ITEM_SELECTED = 1; * @param selected items * @param items items used to render list * @param itemTypeName + * @param useAllSelected all selected option can be disabled * @returns {*} */ -export const renderMultiSelectText = (selected, items, itemTypeName) => { +export const renderMultiSelectText = ({ selected, items, itemTypeName, useAllSelected = true }) => { const itemsKeys = Object.keys(items); const defaultPlaceholder = sprintf( @@ -270,18 +271,30 @@ export const renderMultiSelectText = (selected, items, itemTypeName) => { return defaultPlaceholder; } + const multiSelectLabel = sprintf(MULTIPLE_SELECTED_LABEL, { + firstLabel: items[commonItems[0]], + numberOfAdditionalLabels: commonItems.length - 1, + }); + + const oneItemLabel = items[commonItems[0]] || defaultPlaceholder; + + if (commonItems.length === itemsKeys.length && !useAllSelected) { + if (itemsKeys.length === 1) { + return oneItemLabel; + } + + return multiSelectLabel; + } + switch (commonItems.length) { case itemsKeys.length: return sprintf(ALL_SELECTED_LABEL, { itemTypeName }, false); case NO_ITEM_SELECTED: return defaultPlaceholder; case ONE_ITEM_SELECTED: - return items[commonItems[0]] || defaultPlaceholder; + return oneItemLabel; default: - return sprintf(MULTIPLE_SELECTED_LABEL, { - firstLabel: items[commonItems[0]], - numberOfAdditionalLabels: commonItems.length - 1, - }); + return multiSelectLabel; } }; diff --git a/ee/spec/frontend/security_orchestration/components/policy_editor/scope/compliance_framework_dropdown_spec.js b/ee/spec/frontend/security_orchestration/components/policy_editor/scope/compliance_framework_dropdown_spec.js index a3b191fe304b6..4f3ce91d74297 100644 --- a/ee/spec/frontend/security_orchestration/components/policy_editor/scope/compliance_framework_dropdown_spec.js +++ b/ee/spec/frontend/security_orchestration/components/policy_editor/scope/compliance_framework_dropdown_spec.js @@ -274,7 +274,7 @@ describe('ComplianceFrameworkDropdown', () => { it('renders all frameworks selected text', async () => { await waitForPromises(); - expect(findDropdown().props('toggleText')).toBe('All compliance frameworks'); + expect(findDropdown().props('toggleText')).toBe('A1 +2 more'); }); it('should reset all frameworks', async () => { diff --git a/ee/spec/frontend/security_orchestration/components/policy_editor/scope/scope_section_spec.js b/ee/spec/frontend/security_orchestration/components/policy_editor/scope/scope_section_spec.js index 92daa62242044..118443814fc54 100644 --- a/ee/spec/frontend/security_orchestration/components/policy_editor/scope/scope_section_spec.js +++ b/ee/spec/frontend/security_orchestration/components/policy_editor/scope/scope_section_spec.js @@ -71,6 +71,7 @@ describe('PolicyScope', () => { expect(findExceptionTypeDropdown().exists()).toBe(false); expect(findGroupProjectsDropdown().exists()).toBe(true); + expect(wrapper.text()).toBe('Apply this policy to'); expect(wrapper.emitted('changed')).toEqual([ [ { @@ -170,6 +171,7 @@ describe('PolicyScope', () => { expect(findExceptionTypeDropdown().exists()).toBe(false); expect(findGroupProjectsDropdown().exists()).toBe(false); + expect(wrapper.text()).toBe('Apply this policy to named'); }); it('should render existing excluding projects', () => { @@ -208,6 +210,7 @@ describe('PolicyScope', () => { expect(findComplianceFrameworkDropdown().exists()).toBe(false); expect(findExceptionTypeDropdown().exists()).toBe(false); expect(findGroupProjectsDropdown().exists()).toBe(true); + expect(wrapper.text()).toBe('Apply this policy to'); expect(findGroupProjectsDropdown().props('selected')).toEqual([ convertToGraphQLId(TYPENAME_PROJECT, 'id1'), convertToGraphQLId(TYPENAME_PROJECT, 'id2'), diff --git a/ee/spec/frontend/security_orchestration/components/policy_editor/utils_spec.js b/ee/spec/frontend/security_orchestration/components/policy_editor/utils_spec.js index 3489bb1fbfd86..6e45ce125c9c2 100644 --- a/ee/spec/frontend/security_orchestration/components/policy_editor/utils_spec.js +++ b/ee/spec/frontend/security_orchestration/components/policy_editor/utils_spec.js @@ -250,16 +250,20 @@ describe('slugifyToArray', () => { describe('renderMultiSelectText', () => { it.each` - selected | items | expectedText - ${[]} | ${{}} | ${'Select projects'} - ${['project1']} | ${{ project1: 'project 1', project2: 'project 2' }} | ${'project 1'} - ${['project1', 'project2']} | ${{ project1: 'project 1', project2: 'project 2' }} | ${'All projects'} - ${['project1', 'project2']} | ${{ project1: 'project 1', project2: 'project 2', project3: 'project 3' }} | ${'project 1 +1 more'} - ${[]} | ${{ project1: 'project 1', project2: 'project 2', project3: 'project 3' }} | ${'Select projects'} - ${['project4', 'project5']} | ${{ project1: 'project 1', project2: 'project 2', project3: 'project 3' }} | ${'Select projects'} - ${['project4', 'project5']} | ${{ project2: 'project 2', project3: 'project 3' }} | ${'Select projects'} - `('should render correct selection text', ({ selected, items, expectedText }) => { - expect(renderMultiSelectText(selected, items, 'projects')).toBe(expectedText); + selected | useAllSelected | items | expectedText + ${[]} | ${true} | ${{}} | ${'Select projects'} + ${['project1']} | ${true} | ${{ project1: 'project 1', project2: 'project 2' }} | ${'project 1'} + ${['project1', 'project2']} | ${true} | ${{ project1: 'project 1', project2: 'project 2' }} | ${'All projects'} + ${['project1', 'project2']} | ${false} | ${{ project1: 'project 1', project2: 'project 2' }} | ${'project 1 +1 more'} + ${['project1']} | ${false} | ${{ project1: 'project 1' }} | ${'project 1'} + ${['project1', 'project2']} | ${true} | ${{ project1: 'project 1', project2: 'project 2', project3: 'project 3' }} | ${'project 1 +1 more'} + ${[]} | ${true} | ${{ project1: 'project 1', project2: 'project 2', project3: 'project 3' }} | ${'Select projects'} + ${['project4', 'project5']} | ${true} | ${{ project1: 'project 1', project2: 'project 2', project3: 'project 3' }} | ${'Select projects'} + ${['project4', 'project5']} | ${true} | ${{ project2: 'project 2', project3: 'project 3' }} | ${'Select projects'} + `('should render correct selection text', ({ selected, useAllSelected, items, expectedText }) => { + expect( + renderMultiSelectText({ selected, items, itemTypeName: 'projects', useAllSelected }), + ).toBe(expectedText); }); describe('parseCustomFileConfiguration', () => { diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 46c6ddee6eeae..f54c27b18202f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -44346,10 +44346,10 @@ msgstr "" msgid "SecurityOrchestration|Any merge request" msgstr "" -msgid "SecurityOrchestration|Apply this policy to all projects %{projectScopeType} %{exceptionType} %{projectSelector}" +msgid "SecurityOrchestration|Apply this policy to %{projectScopeType} %{exceptionType} %{projectSelector}" msgstr "" -msgid "SecurityOrchestration|Apply this policy to all projects %{projectScopeType} named %{frameworkSelector}" +msgid "SecurityOrchestration|Apply this policy to %{projectScopeType}named %{frameworkSelector}" msgstr "" msgid "SecurityOrchestration|Are you sure you want to delete this policy? This action cannot be undone." -- GitLab