From 9ff1f658411f5d8c0a71cecfe0d63cf496412314 Mon Sep 17 00:00:00 2001 From: Maurizio Vitale Date: Tue, 26 Apr 2022 15:29:06 +0100 Subject: [PATCH] [AAE-8061] Fix the group restriction for people widget (#7601) * FIx the restrinction * Fix unit test about restricted groups * Add a small sleep to make it green * Add sleep to other checks * Remove sleeps, improve assertions Co-authored-by: MichalFidor --- .../people-group-cloud-component.page.ts | 7 ++- ...people-group-cloud-filter-component.e2e.ts | 30 +++++------- .../components/people-cloud.component.spec.ts | 46 ++++++++++++++----- .../components/people-cloud.component.ts | 9 ++-- .../src/lib/people/mock/user-cloud.mock.ts | 6 +-- .../pages/group-cloud-component.page.ts | 15 ++++-- .../pages/people-cloud-component.page.ts | 22 ++++++--- 7 files changed, 81 insertions(+), 54 deletions(-) diff --git a/e2e/process-services-cloud/pages/people-group-cloud-component.page.ts b/e2e/process-services-cloud/pages/people-group-cloud-component.page.ts index c8d6910a09..6cf95e3a50 100644 --- a/e2e/process-services-cloud/pages/people-group-cloud-component.page.ts +++ b/e2e/process-services-cloud/pages/people-group-cloud-component.page.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { by, element, $, $$ } from 'protractor'; +import { by, element, $, $$, browser } from 'protractor'; import { BrowserVisibility, BrowserActions } from '@alfresco/adf-testing'; export class PeopleGroupCloudComponentPage { @@ -40,6 +40,11 @@ export class PeopleGroupCloudComponentPage { peopleFilterByAppName = $('.app-people-control-options mat-radio-button[value="appName"]'); groupFilterByAppName = $('.app-groups-control-options mat-radio-button[value="appName"]'); + async navigateTo() { + await browser.get('#/cloud/people-group-cloud'); + await browser.waitForAngular(); + } + async checkPeopleCloudComponentTitleIsDisplayed(): Promise { await BrowserVisibility.waitUntilElementIsVisible(this.peopleCloudComponentTitle); } diff --git a/e2e/process-services-cloud/people/people-group-cloud-filter-component.e2e.ts b/e2e/process-services-cloud/people/people-group-cloud-filter-component.e2e.ts index dd7c264468..81e1cff889 100644 --- a/e2e/process-services-cloud/people/people-group-cloud-filter-component.e2e.ts +++ b/e2e/process-services-cloud/people/people-group-cloud-filter-component.e2e.ts @@ -18,14 +18,12 @@ import { createApiService, GroupCloudComponentPage, GroupIdentityService, IdentityService, LoginPage, PeopleCloudComponentPage } from '@alfresco/adf-testing'; import { browser } from 'protractor'; import { PeopleGroupCloudComponentPage } from './../pages/people-group-cloud-component.page'; -import { NavigationBarPage } from '../../core/pages/navigation-bar.page'; describe('People Groups Cloud Component', () => { describe('People Groups Cloud Component', () => { const loginSSOPage = new LoginPage(); - const navigationBarPage = new NavigationBarPage(); const peopleGroupCloudComponentPage = new PeopleGroupCloudComponentPage(); const peopleCloudComponent = new PeopleCloudComponentPage(); const groupCloudComponentPage = new GroupCloudComponentPage(); @@ -46,13 +44,13 @@ describe('People Groups Cloud Component', () => { hrGroup = await groupIdentityService.getGroupInfoByGroupName('hr'); testGroup = await groupIdentityService.getGroupInfoByGroupName('testgroup'); - testUser = await identityService.createIdentityUserWithRole([identityService.ROLES.ACTIVITI_USER]); apsUser = await identityService.createIdentityUserWithRole([identityService.ROLES.ACTIVITI_USER]); + await identityService.addUserToGroup(testUser.idIdentityService, testGroup.id); await identityService.addUserToGroup(apsUser.idIdentityService, hrGroup.id); - noRoleUser = await identityService.createIdentityUser(); + noRoleUser = await identityService.createIdentityUser(); groupNoRole = await groupIdentityService.createIdentityGroup(); users = [apsUser.idIdentityService, noRoleUser.idIdentityService, testUser.idIdentityService]; @@ -62,21 +60,15 @@ describe('People Groups Cloud Component', () => { afterAll(async () => { await apiService.loginWithProfile('identityAdmin'); - for (let i = 0; i < users.length; i++) { - await identityService.deleteIdentityUser(users[i]); + for (const user of users) { + await identityService.deleteIdentityUser(user); } await groupIdentityService.deleteIdentityGroup(groupNoRole.id); }); beforeEach(async () => { - await navigationBarPage.navigateToPeopleGroupCloudPage(); - await peopleGroupCloudComponentPage.checkGroupsCloudComponentTitleIsDisplayed(); - await peopleGroupCloudComponentPage.checkPeopleCloudComponentTitleIsDisplayed(); - }); - - afterEach(async () => { - await browser.refresh(); + await peopleGroupCloudComponentPage.navigateTo(); }); it('[C305041] Should filter the People Single Selection with the Application name filter', async () => { @@ -88,8 +80,7 @@ describe('People Groups Cloud Component', () => { await peopleCloudComponent.checkUserIsDisplayed(`${testUser.firstName} ${testUser.lastName}`); await peopleCloudComponent.selectAssigneeFromList(`${testUser.firstName} ${testUser.lastName}`); - await browser.sleep(100); - await expect(await peopleCloudComponent.checkSelectedPeople(`${testUser.firstName} ${testUser.lastName}`)); + await expect(await peopleCloudComponent.checkSelectedPeople(`${testUser.firstName} ${testUser.lastName}`)).toBeTruthy(`${testUser.firstName} ${testUser.lastName} is not visible here!`); }); it('[C305041] Should filter the People Multiple Selection with the Application name filter', async () => { @@ -99,15 +90,14 @@ describe('People Groups Cloud Component', () => { await peopleCloudComponent.searchAssignee(testUser.firstName); await peopleCloudComponent.checkUserIsDisplayed(`${testUser.firstName} ${testUser.lastName}`); await peopleCloudComponent.selectAssigneeFromList(`${testUser.firstName} ${testUser.lastName}`); - await peopleCloudComponent.checkSelectedPeople(`${testUser.firstName} ${testUser.lastName}`); await peopleCloudComponent.searchAssignee(apsUser.firstName); await peopleCloudComponent.checkUserIsDisplayed(`${apsUser.firstName} ${apsUser.lastName}`); await peopleCloudComponent.selectAssigneeFromList(`${apsUser.firstName} ${apsUser.lastName}`); - await peopleCloudComponent.checkSelectedPeople(`${apsUser.firstName} ${apsUser.lastName}`); + await expect(await peopleCloudComponent.checkSelectedPeople(`${apsUser.firstName} ${apsUser.lastName}`)).toBeTruthy(`${apsUser.firstName} ${apsUser.lastName} is not visible here!`); await peopleCloudComponent.searchAssignee(noRoleUser.firstName); - await peopleCloudComponent.checkNoResultsFoundError(); + await expect(await peopleCloudComponent.checkNoResultsFoundError()).toBeTruthy('There is something in the list!'); }); it('[C305041] Should filter the Groups Single Selection with the Application name filter', async () => { @@ -117,7 +107,7 @@ describe('People Groups Cloud Component', () => { await groupCloudComponentPage.searchGroups(hrGroup.name); await groupCloudComponentPage.checkGroupIsDisplayed(hrGroup.name); await groupCloudComponentPage.selectGroupFromList(hrGroup.name); - await expect(await groupCloudComponentPage.checkSelectedGroup(hrGroup.name)); + await expect(await groupCloudComponentPage.checkSelectedGroup(hrGroup.name)).toBeTruthy(`${hrGroup.name} is not visible here!`); }); it('[C305041] Should filter the Groups Multiple Selection with the Application name filter', async () => { @@ -128,11 +118,13 @@ describe('People Groups Cloud Component', () => { await groupCloudComponentPage.checkGroupIsDisplayed(testGroup.name); await groupCloudComponentPage.selectGroupFromList(testGroup.name); await groupCloudComponentPage.checkSelectedGroup(testGroup.name); + await expect(await groupCloudComponentPage.checkSelectedGroup(testGroup.name)).toBeTruthy(`${testGroup.name} is not visible here!`); await groupCloudComponentPage.searchGroupsToExisting(hrGroup.name); await groupCloudComponentPage.checkGroupIsDisplayed(hrGroup.name); await groupCloudComponentPage.selectGroupFromList(hrGroup.name); await groupCloudComponentPage.checkSelectedGroup(hrGroup.name); + await expect(await groupCloudComponentPage.checkSelectedGroup(hrGroup.name)).toBeTruthy(`${hrGroup.name} is not visible here!`); await groupCloudComponentPage.searchGroupsToExisting(groupNoRole.name); await groupCloudComponentPage.checkGroupIsNotDisplayed(groupNoRole.name); diff --git a/lib/process-services-cloud/src/lib/people/components/people-cloud.component.spec.ts b/lib/process-services-cloud/src/lib/people/components/people-cloud.component.spec.ts index d99ec3981b..9fd5094061 100644 --- a/lib/process-services-cloud/src/lib/people/components/people-cloud.component.spec.ts +++ b/lib/process-services-cloud/src/lib/people/components/people-cloud.component.spec.ts @@ -824,6 +824,8 @@ describe('PeopleCloudComponent', () => { }); describe('Groups restriction', () => { + const GROUP_1 = 0; + const GROUP_2 = 1; beforeEach(() => { fixture.detectChanges(); @@ -831,8 +833,8 @@ describe('PeopleCloudComponent', () => { }); it('Shoud display all users if groups restriction is empty', async () => { - getInvolvedGroupsSpy = spyOn(identityService, 'getInvolvedGroups').and.returnValue(of(mockInvolvedGroups)); component.groupsRestriction = []; + getInvolvedGroupsSpy = spyOn(identityService, 'getInvolvedGroups').and.returnValue(of({name: 'fire'})); typeInputValue('M'); await fixture.whenStable(); @@ -842,21 +844,41 @@ describe('PeopleCloudComponent', () => { expect(fixture.debugElement.queryAll(By.css('[data-automation-id="adf-people-cloud-row"]')).length).toEqual(mockUsers.length); }); - it('Should display users that belongs to restricted groups', async () => { - getInvolvedGroupsSpy = spyOn(identityService, 'getInvolvedGroups').and.returnValue(of(mockInvolvedGroups)); - component.groupsRestriction = [mockInvolvedGroups[0].name, mockInvolvedGroups[1].name]; + it('Should display users that belongs to every restricted groups', async () => { + component.groupsRestriction = ['water', 'fire', 'air']; + getInvolvedGroupsSpy = spyOn(identityService, 'getInvolvedGroups').and.returnValue(of([{name: 'fire'}, {name: 'air'}, {name: 'water'}])); typeInputValue('M'); await fixture.whenStable(); fixture.detectChanges(); - - expect(getInvolvedGroupsSpy).toHaveBeenCalledTimes(3); - expect(fixture.debugElement.queryAll(By.css('[data-automation-id="adf-people-cloud-row"]')).length).toEqual(mockUsers.length); + const userList = fixture.debugElement.queryAll(By.css('[data-automation-id="adf-people-cloud-row"]')); + expect(userList.length).toEqual(3); }); - it('Should not display users that not belongs to restricted groups', async () => { - getInvolvedGroupsSpy = spyOn(identityService, 'getInvolvedGroups').and.returnValue(of([mockInvolvedGroups[0]])); - component.groupsRestriction = [mockInvolvedGroups[0].name, mockInvolvedGroups[1].name]; + it('Should not display user if belong to partial restricted groups', async () => { + component.groupsRestriction = ['water', 'fire', 'air']; + getInvolvedGroupsSpy = spyOn(identityService, 'getInvolvedGroups').and.returnValue(of([{name: 'fire'}, {name: 'water'}])); + typeInputValue('M'); + + await fixture.whenStable(); + fixture.detectChanges(); + const userList = fixture.debugElement.queryAll(By.css('[data-automation-id="adf-people-cloud-row"]')); + expect(userList.length).toEqual(0); + }); + + it('Should not display user if belong to none of the restricted groups', async () => { + component.groupsRestriction = ['water', 'fire', 'air']; + getInvolvedGroupsSpy = spyOn(identityService, 'getInvolvedGroups').and.returnValue(of([])); + typeInputValue('M'); + + await fixture.whenStable(); + fixture.detectChanges(); + expect(fixture.debugElement.queryAll(By.css('[data-automation-id="adf-people-cloud-row"]')).length).toEqual(0); + }); + + it('Should not be able to see a user that does not belong to the restricted group', async () => { + component.groupsRestriction = ['water']; + getInvolvedGroupsSpy = spyOn(identityService, 'getInvolvedGroups').and.returnValue(of([{name: 'fire'}])); typeInputValue('M'); await fixture.whenStable(); @@ -867,7 +889,7 @@ describe('PeopleCloudComponent', () => { it('Should mark as invalid preselected user if is not belongs to restricted groups', (done) => { spyOn(identityService, 'findUserById').and.returnValue(of(mockPreselectedUsers[0])); - getInvolvedGroupsSpy = spyOn(identityService, 'getInvolvedGroups').and.returnValue(of([mockInvolvedGroups[0]])); + getInvolvedGroupsSpy = spyOn(identityService, 'getInvolvedGroups').and.returnValue(of([mockInvolvedGroups[GROUP_1]])); const expectedWarning = { message: 'INVALID_PRESELECTED_USERS', @@ -882,7 +904,7 @@ describe('PeopleCloudComponent', () => { done(); }); - component.groupsRestriction = [mockInvolvedGroups[0].name, mockInvolvedGroups[1].name]; + component.groupsRestriction = [mockInvolvedGroups[GROUP_1].name, mockInvolvedGroups[GROUP_2].name]; component.mode = 'single'; component.validate = true; component.preSelectUsers = [mockPreselectedUsers[0]]; diff --git a/lib/process-services-cloud/src/lib/people/components/people-cloud.component.ts b/lib/process-services-cloud/src/lib/people/components/people-cloud.component.ts index d1c810213f..81447f8d9f 100644 --- a/lib/process-services-cloud/src/lib/people/components/people-cloud.component.ts +++ b/lib/process-services-cloud/src/lib/people/components/people-cloud.component.ts @@ -33,7 +33,6 @@ import { Observable, of, BehaviorSubject, Subject } from 'rxjs'; import { switchMap, debounceTime, distinctUntilChanged, mergeMap, tap, filter, map, takeUntil } from 'rxjs/operators'; import { FullNamePipe, - IdentityGroupModel, IdentityUserModel, IdentityUserService, LogService @@ -563,15 +562,13 @@ export class PeopleCloudComponent implements OnInit, OnChanges, OnDestroy { private isUserPartOfAllRestrictedGroups(user: IdentityUserModel): Observable { return this.getUserGroups(user.id).pipe( - map(userGroups => userGroups.filter( - restrictedGroup => userGroups.includes(restrictedGroup) - ).length >= this.groupsRestriction.length) + map(userGroups => this.groupsRestriction.every(restricted => userGroups.includes(restricted))) ); } - private getUserGroups(userId: string): Observable { + private getUserGroups(userId: string): Observable { return this.identityUserService.getInvolvedGroups(userId).pipe( - map(groups => groups.map(({id, name}) => ({id, name}))) + map(groups => groups.map((group) => group.name)) ); } diff --git a/lib/process-services-cloud/src/lib/people/mock/user-cloud.mock.ts b/lib/process-services-cloud/src/lib/people/mock/user-cloud.mock.ts index 6a9c5535c5..9c5046b3c5 100644 --- a/lib/process-services-cloud/src/lib/people/mock/user-cloud.mock.ts +++ b/lib/process-services-cloud/src/lib/people/mock/user-cloud.mock.ts @@ -15,8 +15,6 @@ * limitations under the License. */ -import { IdentityGroupModel } from '@alfresco/adf-core'; - export const mockUsers = [ { id: 'fake-id-1', username: 'first-name-1 last-name-1', firstName: 'first-name-1', lastName: 'last-name-1', email: 'abc@xyz.com' }, { id: 'fake-id-2', username: 'first-name-2 last-name-2', firstName: 'first-name-2', lastName: 'last-name-2', email: 'abcd@xyz.com'}, @@ -49,6 +47,6 @@ export const mockPreselectedUsers = [ ]; export const mockInvolvedGroups = [ - { id: 'mock-group-id-1', name: 'Mock Group 1', path: '/mock', subGroups: [] } as IdentityGroupModel, - { id: 'mock-group-id-2', name: 'Mock Group 2', path: '', subGroups: [] } as IdentityGroupModel + { id: 'mock-group-id-1', name: 'Mock Group 1', path: '/mock', subGroups: [] }, + { id: 'mock-group-id-2', name: 'Mock Group 2', path: '', subGroups: [] } ]; diff --git a/lib/testing/src/lib/protractor/process-services-cloud/pages/group-cloud-component.page.ts b/lib/testing/src/lib/protractor/process-services-cloud/pages/group-cloud-component.page.ts index afbae47434..739f123298 100644 --- a/lib/testing/src/lib/protractor/process-services-cloud/pages/group-cloud-component.page.ts +++ b/lib/testing/src/lib/protractor/process-services-cloud/pages/group-cloud-component.page.ts @@ -19,9 +19,9 @@ import { by, element, $, ElementFinder, $$ } from 'protractor'; import { BrowserVisibility } from '../../core/utils/browser-visibility'; import { BrowserActions } from '../../core/utils/browser-actions'; import { FormFields } from '../../core/pages/form/form-fields'; +import { TestElement } from '../../core/test-element'; export class GroupCloudComponentPage { - groupCloudSearch = $('input[data-automation-id="adf-cloud-group-search-input"]'); groupField = $('group-cloud-widget .adf-readonly'); formFields = new FormFields(); @@ -29,11 +29,11 @@ export class GroupCloudComponentPage { getGroupRowLocatorByName = async (name: string): Promise => $$(`mat-option[data-automation-id="adf-cloud-group-chip-${name}"]`).first(); async searchGroups(name: string): Promise { - await BrowserActions.clearSendKeys(this.groupCloudSearch, name); + await BrowserActions.clearSendKeys(this.groupCloudSearch, name, 100); } async searchGroupsToExisting(name: string) { - await BrowserActions.clearSendKeys(this.groupCloudSearch, name); + await BrowserActions.clearSendKeys(this.groupCloudSearch, name, 100); } async getGroupsFieldContent(): Promise { @@ -57,8 +57,13 @@ export class GroupCloudComponentPage { await BrowserVisibility.waitUntilElementIsNotVisible(groupRow); } - async checkSelectedGroup(group: string): Promise { - await BrowserVisibility.waitUntilElementIsVisible(element(by.cssContainingText('mat-chip[data-automation-id*="adf-cloud-group-chip-"]', group))); + async checkSelectedGroup(group: string): Promise { + try { + await TestElement.byText('mat-chip[data-automation-id*="adf-cloud-group-chip-"]', group).waitVisible(); + return true; + } catch (e) { + return false; + }; } async checkGroupNotSelected(group: string): Promise { diff --git a/lib/testing/src/lib/protractor/process-services-cloud/pages/people-cloud-component.page.ts b/lib/testing/src/lib/protractor/process-services-cloud/pages/people-cloud-component.page.ts index b0a7fe57bf..c20b8b4898 100644 --- a/lib/testing/src/lib/protractor/process-services-cloud/pages/people-cloud-component.page.ts +++ b/lib/testing/src/lib/protractor/process-services-cloud/pages/people-cloud-component.page.ts @@ -22,7 +22,6 @@ import { FormFields } from '../../core/pages/form/form-fields'; import { TestElement } from '../../core/test-element'; export class PeopleCloudComponentPage { - peopleCloudSearch = $('input[data-automation-id="adf-people-cloud-search-input"]'); assigneeField = $('input[data-automation-id="adf-people-cloud-search-input"]'); selectionReady = $('div[data-automation-id="adf-people-cloud-row"]'); @@ -50,7 +49,7 @@ export class PeopleCloudComponentPage { } async searchAssignee(name: string): Promise { - await BrowserActions.clearSendKeys(this.peopleCloudSearch, name); + await BrowserActions.clearSendKeys(this.peopleCloudSearch, name, 100); } async selectAssigneeFromList(name: string): Promise { @@ -103,8 +102,13 @@ export class PeopleCloudComponentPage { await BrowserVisibility.waitUntilElementIsNotVisible(optionList); } - async checkSelectedPeople(person: string): Promise { - await BrowserVisibility.waitUntilElementIsVisible(element(by.cssContainingText('mat-chip-list mat-chip', person))); + async checkSelectedPeople(person: string): Promise { + try { + await TestElement.byText('mat-chip-list mat-chip', person).waitVisible(); + return true; + } catch (e) { + return false; + } } async getAssigneeFieldContent(): Promise { @@ -163,9 +167,13 @@ export class PeopleCloudComponentPage { } } - async checkNoResultsFoundError(): Promise { - const errorLocator = $('[data-automation-id="adf-people-cloud-no-results"]'); - await BrowserVisibility.waitUntilElementIsVisible(errorLocator); + async checkNoResultsFoundError(): Promise { + try { + await TestElement.byCss('[data-automation-id="adf-people-cloud-no-results"]').waitVisible(); + return true; + } catch (e) { + return false; + } } }