From 48d7f6987e7eef36a78d1b3f3233597e4664733a Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Mon, 30 Mar 2020 11:10:02 +0200 Subject: [PATCH] [groups/epeople admin pages] Changes/Fixes: * Added check with Group.permanent on whether or not to show delete button * Disabled groupName input field if group that is being edited is permanent * Cancel edit group button sends you back to group registry now * In group edit, cache only gets cleared of that groups subgroups and epersons cache (at member delete/add) instead of cleared of all groups info * Separated search groups result list and list of subgroups in edit group page and eperson members of edit group page * Fixed pagination after search issues in groups registry, group edit page (groups search, subgroups, epeople search and members) * Applied same fixes to epeople registry * Browse All button added to all group/eperson searches to browse all groups/epeople * Fixed immediately being able to add members after group creation in edit group page * Fixed hover i18n tags on groups & epeople registry page => Missing tags * Fixed tests after changes --- resources/i18n/en.json5 | 30 +++-- .../epeople-registry.component.html | 9 +- .../epeople-registry.component.spec.ts | 11 +- .../epeople-registry.component.ts | 62 ++++++--- .../eperson-form.component.spec.ts | 3 + .../eperson-form/eperson-form.component.ts | 14 -- .../group-form/group-form.component.html | 5 +- .../group-form/group-form.component.ts | 45 +++++-- .../members-list/members-list.component.html | 74 ++++++++--- .../members-list.component.spec.ts | 84 +++++++++--- .../members-list/members-list.component.ts | 117 +++++++++++----- .../subgroups-list.component.html | 77 ++++++++--- .../subgroups-list.component.spec.ts | 125 +++++++++++------- .../subgroup-list/subgroups-list.component.ts | 101 +++++++++----- .../groups-registry.component.html | 19 +-- .../groups-registry.component.spec.ts | 15 ++- .../groups-registry.component.ts | 52 +++++--- src/app/core/cache/object-cache.service.ts | 2 - src/app/core/eperson/eperson-data.service.ts | 9 +- src/app/core/eperson/group-data.service.ts | 9 +- src/app/shared/testing/group-mock.ts | 54 ++++---- 21 files changed, 626 insertions(+), 291 deletions(-) diff --git a/resources/i18n/en.json5 b/resources/i18n/en.json5 index d0bc0e6cae..e099f475b4 100644 --- a/resources/i18n/en.json5 +++ b/resources/i18n/en.json5 @@ -176,6 +176,8 @@ "admin.access-control.epeople.search.head": "Search", + "admin.access-control.epeople.button.see-all": "Browse All", + "admin.access-control.epeople.search.scope.metadata": "Metadata", "admin.access-control.epeople.search.scope.email": "E-mail (exact)", @@ -192,9 +194,9 @@ "admin.access-control.epeople.table.edit": "Edit", - "admin.access-control.epeople.table.edit.buttons.edit": "Edit", + "admin.access-control.epeople.table.edit.buttons.edit": "Edit \"{{name}}\"", - "admin.access-control.epeople.table.edit.buttons.remove": "Remove", + "admin.access-control.epeople.table.edit.buttons.remove": "Delete \"{{name}}\"", "admin.access-control.epeople.no-items": "No EPeople to show.", @@ -250,6 +252,8 @@ "admin.access-control.groups.search.head": "Search groups", + "admin.access-control.groups.button.see-all": "Browse all", + "admin.access-control.groups.search.button": "Search", "admin.access-control.groups.table.id": "ID", @@ -262,6 +266,10 @@ "admin.access-control.groups.table.edit": "Edit", + "admin.access-control.groups.table.edit.buttons.edit": "Edit \"{{name}}\"", + + "admin.access-control.groups.table.edit.buttons.remove": "Delete \"{{name}}\"", + "admin.access-control.groups.no-items": "No groups found with this in their name or this as UUID", "admin.access-control.groups.notification.deleted.success": "Successfully deleted group \"{{name}}\"", @@ -283,10 +291,14 @@ "admin.access-control.groups.form.notification.created.failure.groupNameInUse": "Failed to create Group with name: \"{{name}}\", make sure the name is not already in use.", - "admin.access-control.groups.form.members-list.head": "Members", + "admin.access-control.groups.form.members-list.head": "EPeople", "admin.access-control.groups.form.members-list.search.head": "Search EPeople", + "admin.access-control.groups.form.members-list.button.see-all": "Browse All", + + "admin.access-control.groups.form.members-list.headMembers": "Browse Members", + "admin.access-control.groups.form.members-list.search.scope.metadata": "Metadata", "admin.access-control.groups.form.members-list.search.scope.email": "E-mail (exact)", @@ -315,14 +327,16 @@ "admin.access-control.groups.form.members-list.no-members-yet": "No members in group yet, search and add.", - "admin.access-control.groups.form.members-list.button.see-all": "Search all", - "admin.access-control.groups.form.members-list.no-items": "No EPeople found in that search", - "admin.access-control.groups.form.subgroups-list.head": "Subgroups", + "admin.access-control.groups.form.subgroups-list.head": "Groups", "admin.access-control.groups.form.subgroups-list.search.head": "Search Groups", + "admin.access-control.groups.form.subgroups-list.button.see-all": "Browse All", + + "admin.access-control.groups.form.subgroups-list.headSubgroups": "Browse Subgroups", + "admin.access-control.groups.form.subgroups-list.search.button": "Search", "admin.access-control.groups.form.subgroups-list.table.id": "ID", @@ -347,9 +361,7 @@ "admin.access-control.groups.form.subgroups-list.no-items": "No groups found with this in their name or this as UUID", - "admin.access-control.groups.form.subgroups-list.no-subgroups-yet": "No subgroups in group yet, search and add.", - - "admin.access-control.groups.form.subgroups-list.button.see-all": "Search all", + "admin.access-control.groups.form.subgroups-list.no-subgroups-yet": "No subgroups in group yet.", "admin.access-control.groups.form.return": "Return to groups", diff --git a/src/app/+admin/admin-access-control/epeople-registry/epeople-registry.component.html b/src/app/+admin/admin-access-control/epeople-registry/epeople-registry.component.html index dd1e8bb62c..20593756c1 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/epeople-registry.component.html +++ b/src/app/+admin/admin-access-control/epeople-registry/epeople-registry.component.html @@ -15,7 +15,10 @@ - +
@@ -21,16 +24,16 @@
- + (pageChange)="onPageChangeSearch($event)">
- +
@@ -39,7 +42,7 @@ - + @@ -67,16 +70,55 @@ - - - +

{{messagePrefix + '.headMembers' | translate}}

+ + + +
+
{{messagePrefix + '.table.id' | translate}}
{{ePerson.id}} {{ePerson.name}}
+ + + + + + + + + + + + + + +
{{messagePrefix + '.table.id' | translate}}{{messagePrefix + '.table.name' | translate}}{{messagePrefix + '.table.edit' | translate}}
{{ePerson.id}}{{ePerson.name}} +
+ +
+
+
+ + + + + diff --git a/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.spec.ts b/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.spec.ts index 92f31aa09d..07fab49ca5 100644 --- a/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.spec.ts +++ b/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.spec.ts @@ -1,8 +1,9 @@ import { CommonModule } from '@angular/common'; import { NO_ERRORS_SCHEMA } from '@angular/core'; -import { async, ComponentFixture, fakeAsync, inject, TestBed, tick } from '@angular/core/testing'; +import { async, ComponentFixture, fakeAsync, flush, inject, TestBed, tick } from '@angular/core/testing'; import { FormsModule, ReactiveFormsModule } from '@angular/forms'; import { BrowserModule, By } from '@angular/platform-browser'; +import { Router } from '@angular/router'; import { NgbModule } from '@ng-bootstrap/ng-bootstrap'; import { TranslateLoader, TranslateModule, TranslateService } from '@ngx-translate/core'; import { Observable } from 'rxjs/internal/Observable'; @@ -16,6 +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 { getMockFormBuilderService } from '../../../../../shared/mocks/mock-form-builder-service'; +import { MockRouter } from '../../../../../shared/mocks/mock-router'; import { getMockTranslateService } from '../../../../../shared/mocks/mock-translate.service'; import { NotificationsService } from '../../../../../shared/notifications/notifications.service'; import { EPersonMock, EPersonMock2 } from '../../../../../shared/testing/eperson-mock'; @@ -36,15 +38,21 @@ describe('MembersListComponent', () => { let activeGroup; let allEPersons; let allGroups; + let epersonMembers; + let subgroupMembers; beforeEach(async(() => { activeGroup = GroupMock; - activeGroup.epersons = [EPersonMock2]; + epersonMembers = [EPersonMock2]; + subgroupMembers = [GroupMock2]; allEPersons = [EPersonMock, EPersonMock2]; - allGroups = [GroupMock, GroupMock2] + allGroups = [GroupMock, GroupMock2]; ePersonDataServiceStub = { + activeGroup: activeGroup, + epersonMembers: epersonMembers, + subgroupMembers: subgroupMembers, findAllByHref(href: string): Observable>> { - return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), activeGroup.epersons)) + return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), groupsDataServiceStub.getEPersonMembers())) }, searchByScope(scope: string, query: string): Observable>> { if (query === '') { @@ -55,33 +63,52 @@ describe('MembersListComponent', () => { clearEPersonRequests() { // empty }, + clearLinkRequests() { + // empty + }, getEPeoplePageRouterLink(): string { return '/admin/access-control/epeople'; } }; groupsDataServiceStub = { + activeGroup: activeGroup, + epersonMembers: epersonMembers, + subgroupMembers: subgroupMembers, + allGroups: allGroups, getActiveGroup(): Observable { return observableOf(activeGroup); }, + getEPersonMembers() { + return this.epersonMembers; + }, searchGroups(query: string): Observable>> { if (query === '') { - return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), allGroups)) + return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), this.allGroups)) } return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), [])) }, addMemberToGroup(parentGroup, eperson: EPerson): Observable { - activeGroup.epersons = [...activeGroup.epersons, eperson]; + this.epersonMembers = [...this.epersonMembers, eperson]; return observableOf(new RestResponse(true, 200, 'Success')); }, clearGroupsRequests() { // empty }, + clearGroupLinkRequests() { + // empty + }, + getGroupEditPageRouterLink(group: Group): string { + return '/admin/access-control/groups/' + group.id; + }, deleteMemberFromGroup(parentGroup, epersonToDelete: EPerson): Observable { - activeGroup.epersons = activeGroup.epersons.find((eperson: EPerson) => { + this.epersonMembers = this.epersonMembers.find((eperson: EPerson) => { if (eperson.id !== epersonToDelete.id) { return eperson; } }); + if (this.epersonMembers === undefined) { + this.epersonMembers = [] + } return observableOf(new RestResponse(true, 200, 'Success')); } }; @@ -102,6 +129,7 @@ describe('MembersListComponent', () => { { provide: GroupDataService, useValue: groupsDataServiceStub }, { provide: NotificationsService, useValue: new NotificationsServiceStub() }, { provide: FormBuilderService, useValue: builderService }, + { provide: Router, useValue: new MockRouter() }, ], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); @@ -112,15 +140,20 @@ describe('MembersListComponent', () => { component = fixture.componentInstance; fixture.detectChanges(); }); + afterEach(fakeAsync(() => { + fixture.destroy(); + flush(); + component = null; + })); it('should create MembersListComponent', inject([MembersListComponent], (comp: MembersListComponent) => { expect(comp).toBeDefined(); })); it('should show list of eperson members of current active group', () => { - const epersonIdsFound = fixture.debugElement.queryAll(By.css('#epersons tr td:first-child')); + const epersonIdsFound = fixture.debugElement.queryAll(By.css('#ePeopleMembersOfGroup tr td:first-child')); expect(epersonIdsFound.length).toEqual(1); - activeGroup.epersons.map((eperson: EPerson) => { + epersonMembers.map((eperson: EPerson) => { expect(epersonIdsFound.find((foundEl) => { return (foundEl.nativeElement.textContent.trim() === eperson.uuid); })).toBeTruthy(); @@ -134,14 +167,14 @@ describe('MembersListComponent', () => { component.search({ scope: 'metadata', query: '' }); tick(); fixture.detectChanges(); - epersonsFound = fixture.debugElement.queryAll(By.css('#epersons tbody tr')); + epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr')); })); it('should display all epersons', () => { expect(epersonsFound.length).toEqual(2); }); - describe('if eperson is already a subeperson', () => { + describe('if eperson is already a eperson', () => { it('should have delete button, else it should have add button', () => { activeGroup.epersons.map((eperson: EPerson) => { epersonsFound.map((foundEPersonRowElement) => { @@ -164,27 +197,42 @@ describe('MembersListComponent', () => { describe('if first add button is pressed', () => { beforeEach(fakeAsync(() => { - const addButton = fixture.debugElement.query(By.css('#epersons tbody .fa-plus')); + const addButton = fixture.debugElement.query(By.css('#epersonsSearch tbody .fa-plus')); addButton.nativeElement.click(); tick(); fixture.detectChanges(); })); - it('one more subeperson in list (from 1 to 2 total epersons)', () => { - epersonsFound = fixture.debugElement.queryAll(By.css('#epersons tbody tr')); + it('all groups in search member of selected group', () => { + epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr')); expect(epersonsFound.length).toEqual(2); + epersonsFound.map((foundEPersonRowElement) => { + if (foundEPersonRowElement.debugElement !== undefined) { + const addButton = foundEPersonRowElement.debugElement.query(By.css('td:last-child .fa-plus')); + const deleteButton = foundEPersonRowElement.debugElement.query(By.css('td:last-child .fa-trash-alt')); + expect(addButton).toBeUndefined(); + expect(deleteButton).toBeDefined(); + } + }) }); }); describe('if first delete button is pressed', () => { beforeEach(fakeAsync(() => { - const addButton = fixture.debugElement.query(By.css('#epersons tbody .fa-trash-alt')); + const addButton = fixture.debugElement.query(By.css('#epersonsSearch tbody .fa-trash-alt')); addButton.nativeElement.click(); tick(); fixture.detectChanges(); })); - it('one less subeperson in list from 1 to 0 (of 2 total epersons)', () => { - epersonsFound = fixture.debugElement.queryAll(By.css('#epersons tbody tr')); - expect(epersonsFound.length).toEqual(0); + it('first eperson in search delete button, because now member', () => { + epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr')); + epersonsFound.map((foundEPersonRowElement) => { + if (foundEPersonRowElement.debugElement !== undefined) { + const addButton = foundEPersonRowElement.debugElement.query(By.css('td:last-child .fa-plus')); + const deleteButton = foundEPersonRowElement.debugElement.query(By.css('td:last-child .fa-trash-alt')); + expect(deleteButton).toBeUndefined(); + expect(addButton).toBeDefined(); + } + }) }); }); }); diff --git a/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts b/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts index 116b4d74e1..9529d099f6 100644 --- a/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts +++ b/src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.ts @@ -1,5 +1,6 @@ import { Component, Input, OnDestroy, OnInit } from '@angular/core'; import { FormBuilder } from '@angular/forms'; +import { Router } from '@angular/router'; import { TranslateService } from '@ngx-translate/core'; import { Observable, of as observableOf, Subscription } from 'rxjs'; import { map, mergeMap, take } from 'rxjs/operators'; @@ -28,12 +29,24 @@ export class MembersListComponent implements OnInit, OnDestroy { messagePrefix: string; /** - * EPeople being displayed, initially all members, after search result of search + * EPeople being displayed in search result, initially all members, after search result of search */ - ePeople: Observable>>; + ePeopleSearch: Observable>>; + /** + * List of EPeople members of currently active group being edited + */ + ePeopleMembersOfGroup: Observable>>; /** - * Pagination config used to display the list of EPeople + * Pagination config used to display the list of EPeople that are result of EPeople search + */ + configSearch: PaginationComponentOptions = Object.assign(new PaginationComponentOptions(), { + id: 'search-members-list-pagination', + pageSize: 5, + currentPage: 1 + }); + /** + * Pagination config used to display the list of EPerson Membes of active group being edited */ config: PaginationComponentOptions = Object.assign(new PaginationComponentOptions(), { id: 'members-list-pagination', @@ -49,50 +62,57 @@ export class MembersListComponent implements OnInit, OnDestroy { // The search form searchForm; - /** - * Whether or not user has done a search yet - */ + // Current search in edit group - epeople search form + currentSearchQuery: string; + currentSearchScope: string; + + // Whether or not user has done a EPeople search yet searchDone: boolean; + // current active group being edited + groupBeingEdited: Group; + constructor(private groupDataService: GroupDataService, public ePersonDataService: EPersonDataService, private translateService: TranslateService, private notificationsService: NotificationsService, - private formBuilder: FormBuilder) { + private formBuilder: FormBuilder, + private router: Router) { + this.currentSearchQuery = ''; + this.currentSearchScope = 'metadata'; } ngOnInit() { - this.subs.push(this.groupDataService.getActiveGroup().subscribe((group: Group) => { - if (group != null) { - this.ePeople = this.ePersonDataService.findAllByHref(group._links.epersons.href, { - currentPage: 1, - elementsPerPage: this.config.pageSize - }) - } - })); this.searchForm = this.formBuilder.group(({ scope: 'metadata', query: '', })); - this.searchDone = false; + this.subs.push(this.groupDataService.getActiveGroup().subscribe((activeGroup: Group) => { + if (activeGroup != null) { + this.groupBeingEdited = activeGroup; + this.forceUpdateEPeople(activeGroup); + } + })); } /** - * Event triggered when the user changes page + * Event triggered when the user changes page on search result + * @param event + */ + onPageChangeSearch(event) { + this.configSearch.currentPage = event; + this.search({ scope: this.currentSearchScope, query: this.currentSearchQuery }); + } + + /** + * Event triggered when the user changes page on EPerson embers of active group * @param event */ onPageChange(event) { - this.updateMembers({ + this.ePeopleMembersOfGroup = this.ePersonDataService.findAllByHref(this.groupBeingEdited._links.epersons.href, { currentPage: event, elementsPerPage: this.config.pageSize - }); - } - - /** - * Update the list of members by fetching it from the rest api or cache - */ - private updateMembers(options) { - this.ePeople = this.ePersonDataService.getEPeople(options); + }) } /** @@ -120,7 +140,7 @@ export class MembersListComponent implements OnInit, OnDestroy { if (activeGroup != null) { const response = this.groupDataService.addMemberToGroup(activeGroup, ePerson); this.showNotifications('addMember', response, ePerson.name, activeGroup); - this.forceUpdateEPeople(activeGroup); + this.forceUpdateEPeople(activeGroup, ePerson); } else { this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure.noActiveGroup')); } @@ -155,10 +175,22 @@ export class MembersListComponent implements OnInit, OnDestroy { * @param data Contains scope and query param */ search(data: any) { + const query: string = data.query; + const scope: string = data.scope; + if (query != null && this.currentSearchQuery !== query && this.groupBeingEdited) { + this.router.navigateByUrl(this.groupDataService.getGroupEditPageRouterLink(this.groupBeingEdited)); + this.currentSearchQuery = query; + this.configSearch.currentPage = 1; + } + if (scope != null && this.currentSearchScope !== scope && this.groupBeingEdited) { + this.router.navigateByUrl(this.groupDataService.getGroupEditPageRouterLink(this.groupBeingEdited)); + this.currentSearchScope = scope; + this.configSearch.currentPage = 1; + } this.searchDone = true; - this.ePeople = this.ePersonDataService.searchByScope(data.scope, data.query, { - currentPage: 1, - elementsPerPage: this.config.pageSize + this.ePeopleSearch = this.ePersonDataService.searchByScope(this.currentSearchScope, this.currentSearchQuery, { + currentPage: this.configSearch.currentPage, + elementsPerPage: this.configSearch.pageSize }); } @@ -167,12 +199,15 @@ export class MembersListComponent implements OnInit, OnDestroy { * a new REST call * @param activeGroup Group currently being edited */ - public forceUpdateEPeople(activeGroup: Group) { - this.groupDataService.clearGroupsRequests(); - this.ePersonDataService.clearEPersonRequests(); - this.ePeople = this.ePersonDataService.findAllByHref(activeGroup._links.epersons.href, { - currentPage: 1, - elementsPerPage: this.config.pageSize + public forceUpdateEPeople(activeGroup: Group, ePersonToUpdate?: EPerson) { + if (ePersonToUpdate != null) { + this.ePersonDataService.clearLinkRequests(ePersonToUpdate._links.groups.href); + } + this.ePersonDataService.clearLinkRequests(activeGroup._links.epersons.href); + this.router.navigateByUrl(this.groupDataService.getGroupEditPageRouterLink(activeGroup)); + this.ePeopleMembersOfGroup = this.ePersonDataService.findAllByHref(activeGroup._links.epersons.href, { + currentPage: this.configSearch.currentPage, + elementsPerPage: this.configSearch.pageSize }) } @@ -199,4 +234,14 @@ export class MembersListComponent implements OnInit, OnDestroy { } }) } + + /** + * Reset all input-fields to be empty and search all search + */ + clearFormAndResetResult() { + this.searchForm.patchValue({ + query: '', + }); + this.search({ query: '' }); + } } diff --git a/src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html b/src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html index 72f17fc25c..4901e4cfe0 100644 --- a/src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html +++ b/src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html @@ -1,7 +1,10 @@

{{messagePrefix + '.head' | translate}}

- +
@@ -15,16 +18,16 @@
- + (pageChange)="onPageChangeSearch($event)">
- +
@@ -33,7 +36,7 @@ - + @@ -41,14 +44,14 @@
@@ -58,19 +61,55 @@
{{messagePrefix + '.table.id' | translate}}
{{group.id}} {{group.name}}
- - - - +

{{messagePrefix + '.headSubgroups' | translate}}

+ + + +
+ + + + + + + + + + + + + + + +
{{messagePrefix + '.table.id' | translate}}{{messagePrefix + '.table.name' | translate}}{{messagePrefix + '.table.edit' | translate}}
{{group.id}}{{group.name}} +
+ +
+
+
+ + + + diff --git a/src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.spec.ts b/src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.spec.ts index 367f3c050f..4bd088b513 100644 --- a/src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.spec.ts +++ b/src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.spec.ts @@ -1,8 +1,9 @@ import { CommonModule } from '@angular/common'; import { NO_ERRORS_SCHEMA } from '@angular/core'; -import { async, ComponentFixture, fakeAsync, inject, TestBed, tick } from '@angular/core/testing'; +import { async, ComponentFixture, fakeAsync, flush, inject, TestBed, tick } from '@angular/core/testing'; import { FormsModule, ReactiveFormsModule } from '@angular/forms'; import { BrowserModule, By } from '@angular/platform-browser'; +import { Router } from '@angular/router'; import { NgbModule } from '@ng-bootstrap/ng-bootstrap'; import { TranslateLoader, TranslateModule, TranslateService } from '@ngx-translate/core'; import { Observable } from 'rxjs/internal/Observable'; @@ -14,6 +15,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 { getMockFormBuilderService } from '../../../../../shared/mocks/mock-form-builder-service'; +import { MockRouter } from '../../../../../shared/mocks/mock-router'; import { getMockTranslateService } from '../../../../../shared/mocks/mock-translate.service'; import { NotificationsService } from '../../../../../shared/notifications/notifications.service'; import { GroupMock, GroupMock2 } from '../../../../../shared/testing/group-mock'; @@ -31,19 +33,26 @@ describe('SubgroupsListComponent', () => { let ePersonDataServiceStub: any; let groupsDataServiceStub: any; let activeGroup; + let subgroups; let allGroups; + let routerStub; beforeEach(async(() => { activeGroup = GroupMock; - allGroups = [GroupMock, GroupMock2] + subgroups = [GroupMock2]; + allGroups = [GroupMock, GroupMock2]; ePersonDataServiceStub = {}; groupsDataServiceStub = { activeGroup: activeGroup, + subgroups: subgroups, getActiveGroup(): Observable { return observableOf(this.activeGroup); }, + getSubgroups(): Group { + return this.activeGroup; + }, findAllByHref(href: string): Observable>> { - return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), this.activeGroup.subgroups)) + return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), this.subgroups)) }, getGroupEditPageRouterLink(group: Group): string { return '/admin/access-control/groups/' + group.id; @@ -55,14 +64,17 @@ describe('SubgroupsListComponent', () => { return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), [])) }, addSubGroupToGroup(parentGroup, subgroup: Group): Observable { - this.activeGroup.subgroups = [...this.activeGroup.subgroups, subgroup]; + this.subgroups = [...this.subgroups, subgroup]; return observableOf(new RestResponse(true, 200, 'Success')); }, clearGroupsRequests() { // empty }, - deleteSubGroupFromGroup(parentGroup, subgroup: Group): Observable { - this.activeGroup.subgroups = this.activeGroup.subgroups.find((group: Group) => { + clearGroupLinkRequests() { + // empty + }, + deleteSubGroupFromGroup(parentGroup, subgroup: Group): Observable { + this.subgroups = this.subgroups.find((group: Group) => { if (group.id !== subgroup.id) { return group; } @@ -70,6 +82,7 @@ describe('SubgroupsListComponent', () => { return observableOf(new RestResponse(true, 200, 'Success')); } }; + routerStub = new MockRouter(); builderService = getMockFormBuilderService(); translateService = getMockTranslateService(); TestBed.configureTestingModule({ @@ -86,6 +99,7 @@ describe('SubgroupsListComponent', () => { { provide: GroupDataService, useValue: groupsDataServiceStub }, { provide: NotificationsService, useValue: new NotificationsServiceStub() }, { provide: FormBuilderService, useValue: builderService }, + { provide: Router, useValue: routerStub }, ], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); @@ -96,13 +110,18 @@ describe('SubgroupsListComponent', () => { component = fixture.componentInstance; fixture.detectChanges(); }); + afterEach(fakeAsync(() => { + fixture.destroy(); + flush(); + component = null; + })); it('should create SubgroupsListComponent', inject([SubgroupsListComponent], (comp: SubgroupsListComponent) => { expect(comp).toBeDefined(); })); it('should show list of subgroups of current active group', () => { - const groupIdsFound = fixture.debugElement.queryAll(By.css('#groups tr td:first-child')); + const groupIdsFound = fixture.debugElement.queryAll(By.css('#subgroupsOfGroup tr td:first-child')); expect(groupIdsFound.length).toEqual(1); activeGroup.subgroups.map((group: Group) => { expect(groupIdsFound.find((foundEl) => { @@ -111,64 +130,76 @@ describe('SubgroupsListComponent', () => { }) }); + describe('if first group delete button is pressed', () => { + let groupsFound; + beforeEach(fakeAsync(() => { + const addButton = fixture.debugElement.query(By.css('#subgroupsOfGroup tbody .deleteButton')); + addButton.triggerEventHandler('click', { + preventDefault: () => {/**/ + } + }); + tick(); + fixture.detectChanges(); + })); + it('one less subgroup in list from 1 to 0 (of 2 total groups)', () => { + groupsFound = fixture.debugElement.queryAll(By.css('#subgroupsOfGroup tbody tr')); + expect(groupsFound.length).toEqual(0); + }); + }); + describe('search', () => { - describe('when searching without query', () => { + describe('when searching with empty query', () => { let groupsFound; beforeEach(fakeAsync(() => { component.search({ query: '' }); - tick(); - fixture.detectChanges(); - groupsFound = fixture.debugElement.queryAll(By.css('#groups tbody tr')); + groupsFound = fixture.debugElement.queryAll(By.css('#groupsSearch tbody tr')); })); it('should display all groups', () => { + fixture.detectChanges(); + groupsFound = fixture.debugElement.queryAll(By.css('#groupsSearch tbody tr')); expect(groupsFound.length).toEqual(2); + groupsFound = fixture.debugElement.queryAll(By.css('#groupsSearch tbody tr')); + const groupIdsFound = fixture.debugElement.queryAll(By.css('#groupsSearch tbody tr td:first-child')); + allGroups.map((group: Group) => { + expect(groupIdsFound.find((foundEl) => { + return (foundEl.nativeElement.textContent.trim() === group.uuid); + })).toBeTruthy(); + }) }); describe('if group is already a subgroup', () => { it('should have delete button, else it should have add button', () => { - activeGroup.subgroups.map((group: Group) => { + fixture.detectChanges(); + groupsFound = fixture.debugElement.queryAll(By.css('#groupsSearch tbody tr')); + const getSubgroups = groupsDataServiceStub.getSubgroups().subgroups; + if (getSubgroups !== undefined && getSubgroups.length > 0) { groupsFound.map((foundGroupRowElement) => { if (foundGroupRowElement.debugElement !== undefined) { - const groupId = foundGroupRowElement.debugElement.query(By.css('td:first-child')); const addButton = foundGroupRowElement.debugElement.query(By.css('td:last-child .fa-plus')); const deleteButton = foundGroupRowElement.debugElement.query(By.css('td:last-child .fa-trash-alt')); - if (groupId.nativeElement.textContent === group.id) { - expect(addButton).toBeUndefined(); - expect(deleteButton).toBeDefined(); - } else { - expect(deleteButton).toBeUndefined(); - expect(addButton).toBeDefined(); - } + expect(addButton).toBeUndefined(); + expect(deleteButton).toBeDefined(); } }) - }) - }); - }); - - describe('if first add button is pressed', () => { - beforeEach(fakeAsync(() => { - const addButton = fixture.debugElement.query(By.css('#groups tbody .fa-plus')); - addButton.nativeElement.click(); - tick(); - fixture.detectChanges(); - })); - it('one more subgroup in list (from 1 to 2 total groups)', () => { - groupsFound = fixture.debugElement.queryAll(By.css('#groups tbody tr')); - expect(groupsFound.length).toEqual(2); - }); - }); - - describe('if first delete button is pressed', () => { - beforeEach(fakeAsync(() => { - const addButton = fixture.debugElement.query(By.css('#groups tbody .fa-trash-alt')); - addButton.nativeElement.click(); - tick(); - fixture.detectChanges(); - })); - it('one less subgroup in list from 1 to 0 (of 2 total groups)', () => { - groupsFound = fixture.debugElement.queryAll(By.css('#groups tbody tr')); - expect(groupsFound.length).toEqual(0); + } else { + getSubgroups.map((group: Group) => { + groupsFound.map((foundGroupRowElement) => { + if (foundGroupRowElement.debugElement !== undefined) { + const groupId = foundGroupRowElement.debugElement.query(By.css('td:first-child')); + const addButton = foundGroupRowElement.debugElement.query(By.css('td:last-child .fa-plus')); + const deleteButton = foundGroupRowElement.debugElement.query(By.css('td:last-child .fa-trash-alt')); + if (groupId.nativeElement.textContent === group.id) { + expect(addButton).toBeUndefined(); + expect(deleteButton).toBeDefined(); + } else { + expect(deleteButton).toBeUndefined(); + expect(addButton).toBeDefined(); + } + } + }) + }) + } }); }); }); diff --git a/src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts b/src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts index da23061219..09d38c7c3e 100644 --- a/src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts +++ b/src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts @@ -1,5 +1,6 @@ import { Component, Input, OnDestroy, OnInit } from '@angular/core'; import { FormBuilder } from '@angular/forms'; +import { Router } from '@angular/router'; import { TranslateService } from '@ngx-translate/core'; import { Observable, of as observableOf, Subscription } from 'rxjs'; import { map, mergeMap, take } from 'rxjs/operators'; @@ -12,7 +13,6 @@ import { getRemoteDataPayload, getSucceededRemoteData } from '../../../../../cor import { hasValue } from '../../../../../shared/empty.util'; import { NotificationsService } from '../../../../../shared/notifications/notifications.service'; import { PaginationComponentOptions } from '../../../../../shared/pagination/pagination-component-options.model'; -import { followLink } from '../../../../../shared/utils/follow-link-config.model'; @Component({ selector: 'ds-subgroups-list', @@ -27,9 +27,13 @@ export class SubgroupsListComponent implements OnInit, OnDestroy { messagePrefix: string; /** - * Groups being displayed, initially all subgroups, after search result of search + * Result of search groups, initially all groups */ - groups: Observable>>; + groupsSearch: Observable>>; + /** + * List of all subgroups of group being edited + */ + subgroupsOfGroup: Observable>>; /** * List of subscriptions @@ -37,7 +41,15 @@ export class SubgroupsListComponent implements OnInit, OnDestroy { subs: Subscription[] = []; /** - * Pagination config used to display the list of subgroups + * Pagination config used to display the list of groups that are result of groups search + */ + configSearch: PaginationComponentOptions = Object.assign(new PaginationComponentOptions(), { + id: 'search-subgroups-list-pagination', + pageSize: 5, + currentPage: 1 + }); + /** + * Pagination config used to display the list of subgroups of currently active group being edited */ config: PaginationComponentOptions = Object.assign(new PaginationComponentOptions(), { id: 'subgroups-list-pagination', @@ -48,50 +60,55 @@ export class SubgroupsListComponent implements OnInit, OnDestroy { // The search form searchForm; - /** - * Whether or not user has done a search yet - */ + // Current search in edit group - groups search form + currentSearchQuery: string; + + // Whether or not user has done a Groups search yet searchDone: boolean; + // current active group being edited + groupBeingEdited: Group; + constructor(public groupDataService: GroupDataService, private translateService: TranslateService, private notificationsService: NotificationsService, - private formBuilder: FormBuilder) { + private formBuilder: FormBuilder, + private router: Router) { + this.currentSearchQuery = ''; } ngOnInit() { - this.subs.push(this.groupDataService.getActiveGroup().subscribe((group: Group) => { - if (group != null) { - this.groups = this.groupDataService.findAllByHref(group._links.subgroups.href, { - currentPage: 1, - elementsPerPage: this.config.pageSize - }) - } - })); this.searchForm = this.formBuilder.group(({ query: '', })); - this.searchDone = false; + this.subs.push(this.groupDataService.getActiveGroup().subscribe((activeGroup: Group) => { + if (activeGroup != null) { + this.groupBeingEdited = activeGroup; + this.forceUpdateGroups(activeGroup); + } + })); } /** - * Event triggered when the user changes page + * Event triggered when the user changes page on search result + * @param event + */ + onPageChangeSearch(event) { + this.configSearch.currentPage = event; + this.search({ query: this.currentSearchQuery }); + } + + /** + * Event triggered when the user changes page on subgroups of active group * @param event */ onPageChange(event) { - this.updateSubgroups({ + this.subgroupsOfGroup = this.groupDataService.findAllByHref(this.groupBeingEdited._links.subgroups.href, { currentPage: event, elementsPerPage: this.config.pageSize }); } - /** - * Update the list of subgroups by fetching it from the rest api or cache - */ - private updateSubgroups(options) { - this.groups = this.groupDataService.getGroups(options); - } - /** * Whether or not the given group is a subgroup of the group currently being edited * @param possibleSubgroup Group that is a possible subgroup (being tested) of the group currently being edited @@ -152,22 +169,28 @@ export class SubgroupsListComponent implements OnInit, OnDestroy { * @param data Contains query param */ search(data: any) { - this.searchDone = true; const query: string = data.query; - this.groups = this.groupDataService.searchGroups(query.trim(), { - currentPage: 1, - elementsPerPage: this.config.pageSize + if (query != null && this.currentSearchQuery !== query) { + this.router.navigateByUrl(this.groupDataService.getGroupEditPageRouterLink(this.groupBeingEdited)); + this.currentSearchQuery = query; + this.configSearch.currentPage = 1; + } + this.searchDone = true; + this.groupsSearch = this.groupDataService.searchGroups(this.currentSearchQuery, { + currentPage: this.configSearch.currentPage, + elementsPerPage: this.configSearch.pageSize }); } /** - * Force-update the list of groups by first clearing the cache related to groups, then performing a new REST call + * Force-update the list of groups by first clearing the cache of results of this active groups' subgroups, then performing a new REST call * @param activeGroup Group currently being edited */ public forceUpdateGroups(activeGroup: Group) { - this.groupDataService.clearGroupsRequests(); - this.groups = this.groupDataService.findAllByHref(activeGroup._links.subgroups.href, { - currentPage: 1, + this.groupDataService.clearGroupLinkRequests(activeGroup._links.subgroups.href); + this.router.navigateByUrl(this.groupDataService.getGroupEditPageRouterLink(activeGroup)); + this.subgroupsOfGroup = this.groupDataService.findAllByHref(activeGroup._links.subgroups.href, { + currentPage: this.config.currentPage, elementsPerPage: this.config.pageSize }); } @@ -195,4 +218,14 @@ export class SubgroupsListComponent implements OnInit, OnDestroy { } }) } + + /** + * Reset all input-fields to be empty and search all search + */ + clearFormAndResetResult() { + this.searchForm.patchValue({ + query: '', + }); + this.search({ query: '' }); + } } diff --git a/src/app/+admin/admin-access-control/group-registry/groups-registry.component.html b/src/app/+admin/admin-access-control/group-registry/groups-registry.component.html index d0cde805c0..3bd7d7ac4f 100644 --- a/src/app/+admin/admin-access-control/group-registry/groups-registry.component.html +++ b/src/app/+admin/admin-access-control/group-registry/groups-registry.component.html @@ -12,7 +12,10 @@
- +
@@ -42,7 +45,7 @@ {{messagePrefix + 'table.id' | translate}} {{messagePrefix + 'table.name' | translate}} {{messagePrefix + 'table.members' | translate}} - + {{messagePrefix + 'table.edit' | translate}} @@ -51,16 +54,17 @@ {{group.id}} {{group.name}} {{(getMembers(group) | async)?.payload?.totalElements + (getSubgroups(group) | async)?.payload?.totalElements}} - +
- -
@@ -69,7 +73,6 @@
-