Skip to content

Commit

Permalink
Remove unnecessary EpersonDtoModel. Rework code and tests to use EPer…
Browse files Browse the repository at this point in the history
…son instead.
  • Loading branch information
tdonohue committed Sep 25, 2023
1 parent ceb70ff commit 3ab91f1
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 151 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ <h4 id="search" class="border-bottom pb-2">
</div>
</form>

<ds-pagination *ngIf="(ePeopleSearchDtos | async)?.totalElements > 0"
<ds-pagination *ngIf="(ePeopleSearch | async)?.totalElements > 0"
[paginationOptions]="configSearch"
[pageInfoState]="(ePeopleSearchDtos | async)"
[collectionSize]="(ePeopleSearchDtos | async)?.totalElements"
[pageInfoState]="(ePeopleSearch | async)"
[collectionSize]="(ePeopleSearch | async)?.totalElements"
[hideGear]="true"
[hidePagerWhenSinglePage]="true">

Expand All @@ -55,24 +55,24 @@ <h4 id="search" class="border-bottom pb-2">
</tr>
</thead>
<tbody>
<tr *ngFor="let ePerson of (ePeopleSearchDtos | async)?.page">
<td class="align-middle">{{ePerson.eperson.id}}</td>
<tr *ngFor="let eperson of (ePeopleSearch | async)?.page">
<td class="align-middle">{{eperson.id}}</td>
<td class="align-middle">
<a (click)="ePersonDataService.startEditingNewEPerson(ePerson.eperson)"
<a (click)="ePersonDataService.startEditingNewEPerson(eperson)"
[routerLink]="[ePersonDataService.getEPeoplePageRouterLink()]">
{{ dsoNameService.getName(ePerson.eperson) }}
{{ dsoNameService.getName(eperson) }}
</a>
</td>
<td class="align-middle">
{{messagePrefix + '.table.email' | translate}}: {{ ePerson.eperson.email ? ePerson.eperson.email : '-' }}<br/>
{{messagePrefix + '.table.netid' | translate}}: {{ ePerson.eperson.netid ? ePerson.eperson.netid : '-' }}
{{messagePrefix + '.table.email' | translate}}: {{ eperson.email ? eperson.email : '-' }}<br/>
{{messagePrefix + '.table.netid' | translate}}: {{ eperson.netid ? eperson.netid : '-' }}
</td>
<td class="align-middle">
<div class="btn-group edit-field">
<button (click)="addMemberToGroup(ePerson)"
<button (click)="addMemberToGroup(eperson)"
[disabled]="actionConfig.add.disabled"
[ngClass]="['btn btn-sm', actionConfig.add.css]"
title="{{messagePrefix + '.table.edit.buttons.add' | translate: { name: dsoNameService.getName(ePerson.eperson) } }}">
title="{{messagePrefix + '.table.edit.buttons.add' | translate: { name: dsoNameService.getName(eperson) } }}">
<i [ngClass]="actionConfig.add.icon"></i>
</button>
</div>
Expand All @@ -84,18 +84,18 @@ <h4 id="search" class="border-bottom pb-2">

</ds-pagination>

<div *ngIf="(ePeopleSearchDtos | async)?.totalElements == 0 && searchDone"
<div *ngIf="(ePeopleSearch | async)?.totalElements == 0 && searchDone"
class="alert alert-info w-100 mb-2"
role="alert">
{{messagePrefix + '.no-items' | translate}}
</div>

<h4>{{messagePrefix + '.headMembers' | translate}}</h4>

<ds-pagination *ngIf="(ePeopleMembersOfGroupDtos | async)?.totalElements > 0"
<ds-pagination *ngIf="(ePeopleMembersOfGroup | async)?.totalElements > 0"
[paginationOptions]="config"
[pageInfoState]="(ePeopleMembersOfGroupDtos | async)"
[collectionSize]="(ePeopleMembersOfGroupDtos | async)?.totalElements"
[pageInfoState]="(ePeopleMembersOfGroup | async)"
[collectionSize]="(ePeopleMembersOfGroup | async)?.totalElements"
[hideGear]="true"
[hidePagerWhenSinglePage]="true">

Expand All @@ -110,24 +110,24 @@ <h4>{{messagePrefix + '.headMembers' | translate}}</h4>
</tr>
</thead>
<tbody>
<tr *ngFor="let ePerson of (ePeopleMembersOfGroupDtos | async)?.page">
<td class="align-middle">{{ePerson.eperson.id}}</td>
<tr *ngFor="let eperson of (ePeopleMembersOfGroup | async)?.page">
<td class="align-middle">{{eperson.id}}</td>
<td class="align-middle">
<a (click)="ePersonDataService.startEditingNewEPerson(ePerson.eperson)"
<a (click)="ePersonDataService.startEditingNewEPerson(eperson)"
[routerLink]="[ePersonDataService.getEPeoplePageRouterLink()]">
{{ dsoNameService.getName(ePerson.eperson) }}
{{ dsoNameService.getName(eperson) }}
</a>
</td>
<td class="align-middle">
{{messagePrefix + '.table.email' | translate}}: {{ ePerson.eperson.email ? ePerson.eperson.email : '-' }}<br/>
{{messagePrefix + '.table.netid' | translate}}: {{ ePerson.eperson.netid ? ePerson.eperson.netid : '-' }}
{{messagePrefix + '.table.email' | translate}}: {{ eperson.email ? eperson.email : '-' }}<br/>
{{messagePrefix + '.table.netid' | translate}}: {{ eperson.netid ? eperson.netid : '-' }}
</td>
<td class="align-middle">
<div class="btn-group edit-field">
<button (click)="deleteMemberFromGroup(ePerson)"
<button (click)="deleteMemberFromGroup(eperson)"
[disabled]="actionConfig.remove.disabled"
[ngClass]="['btn btn-sm', actionConfig.remove.css]"
title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(ePerson.eperson) } }}">
title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(eperson) } }}">
<i [ngClass]="actionConfig.remove.icon"></i>
</button>
</div>
Expand All @@ -139,7 +139,7 @@ <h4>{{messagePrefix + '.headMembers' | translate}}</h4>

</ds-pagination>

<div *ngIf="(ePeopleMembersOfGroupDtos | async) == undefined || (ePeopleMembersOfGroupDtos | async)?.totalElements == 0" class="alert alert-info w-100 mb-2"
<div *ngIf="(ePeopleMembersOfGroup | async) == undefined || (ePeopleMembersOfGroup | async)?.totalElements == 0" class="alert alert-info w-100 mb-2"
role="alert">
{{messagePrefix + '.no-members-yet' | translate}}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { Group } from '../../../../core/eperson/models/group.model';
import { PageInfo } from '../../../../core/shared/page-info.model';
import { FormBuilderService } from '../../../../shared/form/builder/form-builder.service';
import { NotificationsService } from '../../../../shared/notifications/notifications.service';
import { GroupMock, GroupMock2 } from '../../../../shared/testing/group-mock';
import { GroupMock } from '../../../../shared/testing/group-mock';
import { MembersListComponent } from './members-list.component';
import { EPersonMock, EPersonMock2 } from '../../../../shared/testing/eperson.mock';
import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils';
Expand All @@ -31,36 +31,34 @@ import { PaginationServiceStub } from '../../../../shared/testing/pagination-ser
import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';
import { DSONameServiceMock } from '../../../../shared/mocks/dso-name.service.mock';

describe('MembersListComponent', () => {
fdescribe('MembersListComponent', () => {
let component: MembersListComponent;
let fixture: ComponentFixture<MembersListComponent>;
let translateService: TranslateService;
let builderService: FormBuilderService;
let ePersonDataServiceStub: any;
let groupsDataServiceStub: any;
let activeGroup;
let allEPersons: EPerson[];
let allGroups: Group[];
let epersonMembers: EPerson[];
let subgroupMembers: Group[];
let epersonNonMembers: EPerson[];
let paginationService;

beforeEach(waitForAsync(() => {
activeGroup = GroupMock;
epersonMembers = [EPersonMock2];
subgroupMembers = [GroupMock2];
allEPersons = [EPersonMock, EPersonMock2];
allGroups = [GroupMock, GroupMock2];
epersonNonMembers = [EPersonMock];
ePersonDataServiceStub = {
activeGroup: activeGroup,
epersonMembers: epersonMembers,
subgroupMembers: subgroupMembers,
epersonNonMembers: epersonNonMembers,
// This method is used to get all the current members
findListByHref(_href: string): Observable<RemoteData<PaginatedList<EPerson>>> {
return createSuccessfulRemoteDataObject$(buildPaginatedList<EPerson>(new PageInfo(), groupsDataServiceStub.getEPersonMembers()));
},
// This method is used to search across *non-members*
searchByScope(scope: string, query: string): Observable<RemoteData<PaginatedList<EPerson>>> {
if (query === '') {
return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), allEPersons));
return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), epersonNonMembers));
}
return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), []));
},
Expand All @@ -77,22 +75,22 @@ describe('MembersListComponent', () => {
groupsDataServiceStub = {
activeGroup: activeGroup,
epersonMembers: epersonMembers,
subgroupMembers: subgroupMembers,
allGroups: allGroups,
epersonNonMembers: epersonNonMembers,
getActiveGroup(): Observable<Group> {
return observableOf(activeGroup);
},
getEPersonMembers() {
return this.epersonMembers;
},
searchGroups(query: string): Observable<RemoteData<PaginatedList<Group>>> {
if (query === '') {
return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), this.allGroups));
}
return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), []));
},
addMemberToGroup(parentGroup, eperson: EPerson): Observable<RestResponse> {
this.epersonMembers = [...this.epersonMembers, eperson];
addMemberToGroup(parentGroup, epersonToAdd: EPerson): Observable<RestResponse> {
// Add eperson to list of members
this.epersonMembers = [...this.epersonMembers, epersonToAdd];
// Remove eperson from list of non-members
this.epersonNonMembers.forEach( (eperson: EPerson, index: number) => {
if (eperson.id === epersonToAdd.id) {
this.epersonNonMembers.splice(index, 1);
}
});
return observableOf(new RestResponse(true, 200, 'Success'));
},
clearGroupsRequests() {
Expand All @@ -105,14 +103,14 @@ describe('MembersListComponent', () => {
return '/access-control/groups/' + group.id;
},
deleteMemberFromGroup(parentGroup, epersonToDelete: EPerson): Observable<RestResponse> {
this.epersonMembers = this.epersonMembers.find((eperson: EPerson) => {
if (eperson.id !== epersonToDelete.id) {
return eperson;
// Remove eperson from list of members
this.epersonMembers.forEach( (eperson: EPerson, index: number) => {
if (eperson.id === epersonToDelete.id) {
this.epersonMembers.splice(index, 1);
}
});
if (this.epersonMembers === undefined) {
this.epersonMembers = [];
}
// Add eperson to list of non-members
this.epersonNonMembers = [...this.epersonNonMembers, epersonToDelete];
return observableOf(new RestResponse(true, 200, 'Success'));
}
};
Expand Down Expand Up @@ -160,90 +158,78 @@ describe('MembersListComponent', () => {
expect(comp).toBeDefined();
}));

it('should show list of eperson members of current active group', () => {
const epersonIdsFound = fixture.debugElement.queryAll(By.css('#ePeopleMembersOfGroup tr td:first-child'));
expect(epersonIdsFound.length).toEqual(1);
epersonMembers.map((eperson: EPerson) => {
expect(epersonIdsFound.find((foundEl) => {
return (foundEl.nativeElement.textContent.trim() === eperson.uuid);
})).toBeTruthy();
describe('current members list', () => {
it('should show list of eperson members of current active group', () => {
const epersonIdsFound = fixture.debugElement.queryAll(By.css('#ePeopleMembersOfGroup tr td:first-child'));
expect(epersonIdsFound.length).toEqual(1);
epersonMembers.map((eperson: EPerson) => {
epersonIdsFound.find((foundEl) => {
return (foundEl.nativeElement.textContent.trim() === eperson.uuid);
});
});
});

it('should show a delete button next to each member', () => {
const epersonsFound = fixture.debugElement.queryAll(By.css('#ePeopleMembersOfGroup tbody tr'));
epersonsFound.map((foundEPersonRowElement: DebugElement) => {
const addButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-plus'));
const deleteButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-trash-alt'));
expect(addButton).toBeNull();
expect(deleteButton).not.toBeNull();
});
});

describe('if first delete button is pressed', () => {
beforeEach(() => {
const deleteButton: DebugElement = fixture.debugElement.query(By.css('#ePeopleMembersOfGroup tbody .fa-trash-alt'));
deleteButton.nativeElement.click();
fixture.detectChanges();
});
it('then no ePerson remains as a member of the active group.', () => {
const epersonsFound = fixture.debugElement.queryAll(By.css('#ePeopleMembersOfGroup tbody tr'));
expect(epersonsFound.length).toEqual(0);
});
});
});

describe('search', () => {
describe('when searching without query', () => {
let epersonsFound: DebugElement[];
beforeEach(fakeAsync(() => {
spyOn(component, 'isMemberOfGroup').and.callFake((ePerson: EPerson) => {
return observableOf(activeGroup.epersons.includes(ePerson));
});
component.search({ scope: 'metadata', query: '' });
tick();
fixture.detectChanges();
epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr'));
// Stop using the fake spy function (because otherwise the clicking on the buttons will not change anything
// because they don't change the value of activeGroup.epersons)
jasmine.getEnv().allowRespy(true);
spyOn(component, 'isMemberOfGroup').and.callThrough();
}));

it('should display all epersons', () => {
expect(epersonsFound.length).toEqual(2);
it('should display only non-members of the group', () => {
const epersonIdsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr td:first-child'));
expect(epersonIdsFound.length).toEqual(1);
epersonNonMembers.map((eperson: EPerson) => {
epersonIdsFound.find((foundEl) => {
return (foundEl.nativeElement.textContent.trim() === eperson.uuid);
});
});
});

describe('if eperson is already a eperson', () => {
it('should have delete button, else it should have add button', () => {
const memberIds: string[] = activeGroup.epersons.map((ePerson: EPerson) => ePerson.id);
epersonsFound.map((foundEPersonRowElement: DebugElement) => {
const epersonId: DebugElement = foundEPersonRowElement.query(By.css('td:first-child'));
const addButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-plus'));
const deleteButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-trash-alt'));
if (memberIds.includes(epersonId.nativeElement.textContent)) {
expect(addButton).toBeNull();
expect(deleteButton).not.toBeNull();
} else {
expect(deleteButton).toBeNull();
expect(addButton).not.toBeNull();
}
});
it('should display an add button next to non-members, not a delete button', () => {
epersonsFound.map((foundEPersonRowElement: DebugElement) => {
const addButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-plus'));
const deleteButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-trash-alt'));
expect(addButton).not.toBeNull();
expect(deleteButton).toBeNull();
});
});

describe('if first add button is pressed', () => {
beforeEach(fakeAsync(() => {
beforeEach(() => {
const addButton: DebugElement = fixture.debugElement.query(By.css('#epersonsSearch tbody .fa-plus'));
addButton.nativeElement.click();
tick();
fixture.detectChanges();
}));
it('then all the ePersons are member of the active group', () => {
epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr'));
expect(epersonsFound.length).toEqual(2);
epersonsFound.map((foundEPersonRowElement: DebugElement) => {
const addButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-plus'));
const deleteButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-trash-alt'));
expect(addButton).toBeNull();
expect(deleteButton).not.toBeNull();
});
});
});

describe('if first delete button is pressed', () => {
beforeEach(fakeAsync(() => {
const deleteButton: DebugElement = fixture.debugElement.query(By.css('#epersonsSearch tbody .fa-trash-alt'));
deleteButton.nativeElement.click();
tick();
fixture.detectChanges();
}));
it('then no ePerson is member of the active group', () => {
it('then all (two) ePersons are member of the active group. No non-members left', () => {
epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr'));
expect(epersonsFound.length).toEqual(2);
epersonsFound.map((foundEPersonRowElement: DebugElement) => {
const addButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-plus'));
const deleteButton: DebugElement = foundEPersonRowElement.query(By.css('td:last-child .fa-trash-alt'));
expect(deleteButton).toBeNull();
expect(addButton).not.toBeNull();
});
expect(epersonsFound.length).toEqual(0);
});
});
});
Expand Down
Loading

0 comments on commit 3ab91f1

Please sign in to comment.