final tests (group-form & member&subgroup lists) & notification

for group creation fail, with specific notification for group name in use

resources/i18n/en.json5

@@ -277,9 +277,11 @@
 
   "admin.access-control.groups.form.groupDescription": "Description",
 
-  "admin.access-control.groups.form.notification.created.success": "Successfully created group \"{{name}}\"",
+  "admin.access-control.groups.form.notification.created.success": "Successfully created Group \"{{name}}\"",
 
-  "admin.access-control.groups.form.notification.created.failure": "Failed to create group \"{{name}}\"",
+  "admin.access-control.groups.form.notification.created.failure": "Failed to create Group \"{{name}}\"",
+
+  "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",
 

src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts

@@ -139,13 +139,21 @@ describe('EPersonFormComponent', () => {
   }));
 
   describe('when submitting the form', () => {
-    const firstName = 'testName';
+    let firstName;
-    const lastName = 'testLastName';
+    let lastName;
-    const email = 'testEmail@test.com';
+    let email;
-    const canLogIn = false;
+    let canLogIn;
-    const requireCertificate = false;
+    let requireCertificate;
 
-    const expected = Object.assign(new EPerson(), {
+    let expected;
+    beforeEach(() => {
+      firstName = 'testName';
+      lastName = 'testLastName';
+      email = 'testEmail@test.com';
+      canLogIn = false;
+      requireCertificate = false;
+
+      expected = Object.assign(new EPerson(), {
         metadata: {
           'eperson.firstname': [
             {
@@ -162,7 +170,6 @@ describe('EPersonFormComponent', () => {
         canLogIn: canLogIn,
         requireCertificate: requireCertificate,
       });
-    beforeEach(() => {
       spyOn(component.submitForm, 'emit');
       component.firstName.value = firstName;
       component.lastName.value = lastName;
@@ -185,7 +192,10 @@ describe('EPersonFormComponent', () => {
     });
 
     describe('with an active eperson', () => {
-      const expectedWithId = Object.assign(new EPerson(), {
+      let expectedWithId;
+
+      beforeEach(() => {
+        expectedWithId = Object.assign(new EPerson(), {
           metadata: {
             'eperson.firstname': [
               {
@@ -202,8 +212,6 @@ describe('EPersonFormComponent', () => {
           canLogIn: canLogIn,
           requireCertificate: requireCertificate,
         });
-
-      beforeEach(() => {
         spyOn(ePersonDataServiceStub, 'getActiveEPerson').and.returnValue(observableOf(expectedWithId));
         component.onSubmit();
         fixture.detectChanges();

src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.spec.ts

@@ -0,0 +1,153 @@
+import { CommonModule } from '@angular/common';
+import { HttpClient } from '@angular/common/http';
+import { NO_ERRORS_SCHEMA } from '@angular/core';
+import { async, ComponentFixture, inject, TestBed } from '@angular/core/testing';
+import { FormsModule, ReactiveFormsModule } from '@angular/forms';
+import { BrowserModule } from '@angular/platform-browser';
+import { ActivatedRoute, Router } from '@angular/router';
+import { NgbModule } from '@ng-bootstrap/ng-bootstrap';
+import { Store } from '@ngrx/store';
+import { TranslateLoader, TranslateModule, TranslateService } from '@ngx-translate/core';
+import { of as observableOf } from 'rxjs';
+import { Observable } from 'rxjs/internal/Observable';
+import { RemoteDataBuildService } from '../../../../core/cache/builders/remote-data-build.service';
+import { ObjectCacheService } from '../../../../core/cache/object-cache.service';
+import { RestResponse } from '../../../../core/cache/response.models';
+import { DSOChangeAnalyzer } from '../../../../core/data/dso-change-analyzer.service';
+import { PaginatedList } from '../../../../core/data/paginated-list';
+import { RemoteData } from '../../../../core/data/remote-data';
+import { EPersonDataService } from '../../../../core/eperson/eperson-data.service';
+import { GroupDataService } from '../../../../core/eperson/group-data.service';
+import { Group } from '../../../../core/eperson/models/group.model';
+import { HALEndpointService } from '../../../../core/shared/hal-endpoint.service';
+import { PageInfo } from '../../../../core/shared/page-info.model';
+import { UUIDService } from '../../../../core/shared/uuid.service';
+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';
+import { MockTranslateLoader } from '../../../../shared/testing/mock-translate-loader';
+import { NotificationsServiceStub } from '../../../../shared/testing/notifications-service-stub';
+import { createSuccessfulRemoteDataObject$ } from '../../../../shared/testing/utils';
+import { GroupFormComponent } from './group-form.component';
+
+describe('GroupFormComponent', () => {
+  let component: GroupFormComponent;
+  let fixture: ComponentFixture<GroupFormComponent>;
+  let translateService: TranslateService;
+  let builderService: FormBuilderService;
+  let ePersonDataServiceStub: any;
+  let groupsDataServiceStub: any;
+  let router;
+
+  let groups;
+  let groupName;
+  let groupDescription;
+  let expected;
+
+  beforeEach(async(() => {
+    groups = [GroupMock, GroupMock2]
+    groupName = 'testGroupName';
+    groupDescription = 'testDescription';
+    expected = Object.assign(new Group(), {
+      name: groupName,
+      metadata: {
+        'dc.description': [
+          {
+            value: groupDescription
+          }
+        ],
+      },
+    });
+    ePersonDataServiceStub = {};
+    groupsDataServiceStub = {
+      allGroups: groups,
+      activeGroup: null,
+      getActiveGroup(): Observable<Group> {
+        return observableOf(this.activeGroup);
+      },
+      getGroupRegistryRouterLink(): string {
+        return '/admin/access-control/groups';
+      },
+      editGroup(group: Group) {
+        this.activeGroup = group
+      },
+      cancelEditGroup(): void {
+        this.activeGroup = null;
+      },
+      findById(id: string) {
+        return observableOf({ payload: null, hasSucceeded: true });
+      },
+      tryToCreate(group: Group): Observable<RestResponse> {
+        this.allGroups = [...this.allGroups, group]
+        return observableOf(new RestResponse(true, 200, 'Success'));
+      },
+      searchGroups(query: string): Observable<RemoteData<PaginatedList<Group>>> {
+        return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), []))
+      }
+    };
+    builderService = getMockFormBuilderService();
+    translateService = getMockTranslateService();
+    router = new MockRouter();
+    TestBed.configureTestingModule({
+      imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule,
+        TranslateModule.forRoot({
+          loader: {
+            provide: TranslateLoader,
+            useClass: MockTranslateLoader
+          }
+        }),
+      ],
+      declarations: [GroupFormComponent],
+      providers: [GroupFormComponent,
+        { provide: EPersonDataService, useValue: ePersonDataServiceStub },
+        { provide: GroupDataService, useValue: groupsDataServiceStub },
+        { provide: NotificationsService, useValue: new NotificationsServiceStub() },
+        { provide: FormBuilderService, useValue: builderService },
+        { provide: DSOChangeAnalyzer, useValue: {} },
+        { provide: HttpClient, useValue: {} },
+        { provide: ObjectCacheService, useValue: {} },
+        { provide: UUIDService, useValue: {} },
+        { provide: Store, useValue: {} },
+        { provide: RemoteDataBuildService, useValue: {} },
+        { provide: HALEndpointService, useValue: {} },
+        { provide: ActivatedRoute, useValue: { data: observableOf({ dso: { payload: {} } }), params: observableOf({}) } },
+        { provide: Router, useValue: router },
+      ],
+      schemas: [NO_ERRORS_SCHEMA]
+    }).compileComponents();
+  }));
+
+  beforeEach(() => {
+    fixture = TestBed.createComponent(GroupFormComponent);
+    component = fixture.componentInstance;
+    fixture.detectChanges();
+  });
+
+  it('should create GroupFormComponent', inject([GroupFormComponent], (comp: GroupFormComponent) => {
+    expect(comp).toBeDefined();
+  }));
+
+  describe('when submitting the form', () => {
+    beforeEach(() => {
+      spyOn(component.submitForm, 'emit');
+      component.groupName.value = groupName;
+      component.groupDescription.value = groupDescription;
+    });
+    describe('without active Group', () => {
+      beforeEach(() => {
+        component.onSubmit();
+        fixture.detectChanges();
+      });
+
+      it('should emit a new group using the correct values', async(() => {
+        fixture.whenStable().then(() => {
+          expect(component.submitForm.emit).toHaveBeenCalledWith(expected);
+        });
+      }));
+    });
+  });
+
+});

src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.ts

@@ -11,6 +11,8 @@ import { TranslateService } from '@ngx-translate/core';
 import { combineLatest } from 'rxjs/internal/observable/combineLatest';
 import { Subscription } from 'rxjs/internal/Subscription';
 import { take } from 'rxjs/operators';
+import { RestResponse } from '../../../../core/cache/response.models';
+import { PaginatedList } from '../../../../core/data/paginated-list';
 import { EPersonDataService } from '../../../../core/eperson/eperson-data.service';
 import { GroupDataService } from '../../../../core/eperson/group-data.service';
 import { Group } from '../../../../core/eperson/models/group.model';
@@ -92,9 +94,9 @@ export class GroupFormComponent implements OnInit, OnDestroy {
   }
 
   ngOnInit() {
-    this.route.params.subscribe((params) => {
+    this.subs.push(this.route.params.subscribe((params) => {
       this.setActiveGroup(params.groupId)
-    });
+    }));
     combineLatest(
       this.translateService.get(`${this.messagePrefix}.groupName`),
       this.translateService.get(`${this.messagePrefix}.groupDescription`),
@@ -155,7 +157,7 @@ export class GroupFormComponent implements OnInit, OnDestroy {
             ],
           },
         };
-        if (group == null) {
+        if (group === null) {
           this.createNewGroup(values);
         } else {
           this.editGroup(group, values);
@@ -169,14 +171,38 @@ export class GroupFormComponent implements OnInit, OnDestroy {
    * @param values
    */
   createNewGroup(values) {
-    this.subs.push(this.groupDataService.createOrUpdateGroup(Object.assign(new Group(), values))
+    const groupToCreate = Object.assign(new Group(), values);
-      .pipe(
+    const response = this.groupDataService.tryToCreate(groupToCreate);
-        getSucceededRemoteData(),
+    response.pipe(take(1)).subscribe((restResponse: RestResponse) => {
-        getRemoteDataPayload())
+      if (restResponse.isSuccessful) {
-      .subscribe((group: Group) => {
+        this.notificationsService.success(this.translateService.get(this.messagePrefix  + '.notification.created.success', { name: groupToCreate.name }));
-        this.notificationsService.success(this.translateService.get(this.messagePrefix + '.notification.created.success', { name: group.name }));
+        this.submitForm.emit(groupToCreate);
-        this.setActiveGroup(group.id);
+      } else {
-        this.submitForm.emit(group);
+        this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.created.failure', { name: groupToCreate.name }));
+        this.showNotificationIfNameInUse(groupToCreate, 'created');
+        this.cancelForm.emit();
+      }
+    });
+  }
+
+  /**
+   * Checks for the given group if there is already a group in the system with that group name and shows error if that
+   * is the case
+   * @param group                 group to check
+   * @param notificationSection   whether in create or edit
+   */
+  private showNotificationIfNameInUse(group: Group, notificationSection: string) {
+    // Relevant message for group name in use
+    this.subs.push(this.groupDataService.searchGroups(group.name, {
+      currentPage: 1,
+      elementsPerPage: 0
+    }).pipe(getSucceededRemoteData(), getRemoteDataPayload())
+      .subscribe((list: PaginatedList<Group>) => {
+        if (list.totalElements > 0) {
+          this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.' + notificationSection + '.failure.groupNameInUse', {
+            name: group.name
+          }));
+        }
       }));
   }
 

src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.html

@@ -30,7 +30,7 @@
                  (pageChange)="onPageChange($event)">
 
     <div class="table-responsive">
-      <table id="groups" class="table table-striped table-hover table-bordered">
+      <table id="epersons" class="table table-striped table-hover table-bordered">
         <thead>
         <tr>
           <th scope="col">{{messagePrefix + '.table.id' | translate}}</th>

src/app/+admin/admin-access-control/group-registry/group-form/members-list/members-list.component.spec.ts

@@ -0,0 +1,193 @@
+import { CommonModule } from '@angular/common';
+import { NO_ERRORS_SCHEMA } from '@angular/core';
+import { async, ComponentFixture, fakeAsync, inject, TestBed, tick } from '@angular/core/testing';
+import { FormsModule, ReactiveFormsModule } from '@angular/forms';
+import { BrowserModule, By } from '@angular/platform-browser';
+import { NgbModule } from '@ng-bootstrap/ng-bootstrap';
+import { TranslateLoader, TranslateModule, TranslateService } from '@ngx-translate/core';
+import { Observable } from 'rxjs/internal/Observable';
+import { RestResponse } from '../../../../../core/cache/response.models';
+import { PaginatedList } from '../../../../../core/data/paginated-list';
+import { RemoteData } from '../../../../../core/data/remote-data';
+import { EPersonDataService } from '../../../../../core/eperson/eperson-data.service';
+import { GroupDataService } from '../../../../../core/eperson/group-data.service';
+import { EPerson } from '../../../../../core/eperson/models/eperson.model';
+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 { getMockTranslateService } from '../../../../../shared/mocks/mock-translate.service';
+import { NotificationsService } from '../../../../../shared/notifications/notifications.service';
+import { EPersonMock, EPersonMock2 } from '../../../../../shared/testing/eperson-mock';
+import { GroupMock, GroupMock2 } from '../../../../../shared/testing/group-mock';
+import { MockTranslateLoader } from '../../../../../shared/testing/mock-translate-loader';
+import { NotificationsServiceStub } from '../../../../../shared/testing/notifications-service-stub';
+import { of as observableOf } from 'rxjs';
+import { createSuccessfulRemoteDataObject$ } from '../../../../../shared/testing/utils';
+import { MembersListComponent } from './members-list.component';
+
+describe('MembersListComponent', () => {
+  let component: MembersListComponent;
+  let fixture: ComponentFixture<MembersListComponent>;
+  let translateService: TranslateService;
+  let builderService: FormBuilderService;
+  let ePersonDataServiceStub: any;
+  let groupsDataServiceStub: any;
+  let activeGroup;
+  let allEPersons;
+  let allGroups;
+
+  beforeEach(async(() => {
+    activeGroup = GroupMock;
+    activeGroup.epersons = [EPersonMock2];
+    allEPersons = [EPersonMock, EPersonMock2];
+    allGroups = [GroupMock, GroupMock2]
+    ePersonDataServiceStub = {
+      findAllByHref(href: string): Observable<RemoteData<PaginatedList<EPerson>>> {
+        return createSuccessfulRemoteDataObject$(new PaginatedList<EPerson>(new PageInfo(), activeGroup.epersons))
+      },
+      searchByScope(scope: string, query: string): Observable<RemoteData<PaginatedList<EPerson>>> {
+        if (query === '') {
+          return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), allEPersons))
+        }
+        return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), []))
+      },
+      clearEPersonRequests() {
+        // empty
+      },
+      getEPeoplePageRouterLink(): string {
+        return '/admin/access-control/epeople';
+      }
+    };
+    groupsDataServiceStub = {
+      getActiveGroup(): Observable<Group> {
+        return observableOf(activeGroup);
+      },
+      searchGroups(query: string): Observable<RemoteData<PaginatedList<Group>>> {
+        if (query === '') {
+          return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), allGroups))
+        }
+        return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), []))
+      },
+      addMemberToGroup(parentGroup, eperson: EPerson): Observable<RestResponse> {
+        activeGroup.epersons = [...activeGroup.epersons, eperson];
+        return observableOf(new RestResponse(true, 200, 'Success'));
+      },
+      clearGroupsRequests() {
+        // empty
+      },
+      deleteMemberFromGroup(parentGroup, epersonToDelete: EPerson): Observable<RestResponse> {
+        activeGroup.epersons = activeGroup.epersons.find((eperson: EPerson) => {
+          if (eperson.id !== epersonToDelete.id) {
+            return eperson;
+          }
+        });
+        return observableOf(new RestResponse(true, 200, 'Success'));
+      }
+    };
+    builderService = getMockFormBuilderService();
+    translateService = getMockTranslateService();
+    TestBed.configureTestingModule({
+      imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule,
+        TranslateModule.forRoot({
+          loader: {
+            provide: TranslateLoader,
+            useClass: MockTranslateLoader
+          }
+        }),
+      ],
+      declarations: [MembersListComponent],
+      providers: [MembersListComponent,
+        { provide: EPersonDataService, useValue: ePersonDataServiceStub },
+        { provide: GroupDataService, useValue: groupsDataServiceStub },
+        { provide: NotificationsService, useValue: new NotificationsServiceStub() },
+        { provide: FormBuilderService, useValue: builderService },
+      ],
+      schemas: [NO_ERRORS_SCHEMA]
+    }).compileComponents();
+  }));
+
+  beforeEach(() => {
+    fixture = TestBed.createComponent(MembersListComponent);
+    component = fixture.componentInstance;
+    fixture.detectChanges();
+  });
+
+  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'));
+    expect(epersonIdsFound.length).toEqual(1);
+    activeGroup.epersons.map((eperson: EPerson) => {
+      expect(epersonIdsFound.find((foundEl) => {
+        return (foundEl.nativeElement.textContent.trim() === eperson.uuid);
+      })).toBeTruthy();
+    });
+  });
+
+  describe('search', () => {
+    describe('when searching without query', () => {
+      let epersonsFound;
+      beforeEach(fakeAsync(() => {
+        component.search({ scope: 'metadata', query: '' });
+        tick();
+        fixture.detectChanges();
+        epersonsFound = fixture.debugElement.queryAll(By.css('#epersons tbody tr'));
+      }));
+
+      it('should display all epersons', () => {
+        expect(epersonsFound.length).toEqual(2);
+      });
+
+      describe('if eperson is already a subeperson', () => {
+        it('should have delete button, else it should have add button', () => {
+          activeGroup.epersons.map((eperson: EPerson) => {
+            epersonsFound.map((foundEPersonRowElement) => {
+              if (foundEPersonRowElement.debugElement !== undefined) {
+                const epersonId = foundEPersonRowElement.debugElement.query(By.css('td:first-child'));
+                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'));
+                if (epersonId.nativeElement.textContent === eperson.id) {
+                  expect(addButton).toBeUndefined();
+                  expect(deleteButton).toBeDefined();
+                } else {
+                  expect(deleteButton).toBeUndefined();
+                  expect(addButton).toBeDefined();
+                }
+              }
+            })
+          })
+        });
+      });
+
+      describe('if first add button is pressed', () => {
+        beforeEach(fakeAsync(() => {
+          const addButton = fixture.debugElement.query(By.css('#epersons 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'));
+          expect(epersonsFound.length).toEqual(2);
+        });
+      });
+
+      describe('if first delete button is pressed', () => {
+        beforeEach(fakeAsync(() => {
+          const addButton = fixture.debugElement.query(By.css('#epersons 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);
+        });
+      });
+    });
+  });
+
+});

src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.spec.ts

@@ -0,0 +1,177 @@
+import { CommonModule } from '@angular/common';
+import { NO_ERRORS_SCHEMA } from '@angular/core';
+import { async, ComponentFixture, fakeAsync, inject, TestBed, tick } from '@angular/core/testing';
+import { FormsModule, ReactiveFormsModule } from '@angular/forms';
+import { BrowserModule, By } from '@angular/platform-browser';
+import { NgbModule } from '@ng-bootstrap/ng-bootstrap';
+import { TranslateLoader, TranslateModule, TranslateService } from '@ngx-translate/core';
+import { Observable } from 'rxjs/internal/Observable';
+import { RestResponse } from '../../../../../core/cache/response.models';
+import { PaginatedList } from '../../../../../core/data/paginated-list';
+import { RemoteData } from '../../../../../core/data/remote-data';
+import { GroupDataService } from '../../../../../core/eperson/group-data.service';
+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 { getMockTranslateService } from '../../../../../shared/mocks/mock-translate.service';
+import { NotificationsService } from '../../../../../shared/notifications/notifications.service';
+import { GroupMock, GroupMock2 } from '../../../../../shared/testing/group-mock';
+import { MockTranslateLoader } from '../../../../../shared/testing/mock-translate-loader';
+import { NotificationsServiceStub } from '../../../../../shared/testing/notifications-service-stub';
+import { of as observableOf } from 'rxjs';
+import { createSuccessfulRemoteDataObject$ } from '../../../../../shared/testing/utils';
+import { SubgroupsListComponent } from './subgroups-list.component';
+
+describe('SubgroupsListComponent', () => {
+  let component: SubgroupsListComponent;
+  let fixture: ComponentFixture<SubgroupsListComponent>;
+  let translateService: TranslateService;
+  let builderService: FormBuilderService;
+  let ePersonDataServiceStub: any;
+  let groupsDataServiceStub: any;
+  let activeGroup;
+  let allGroups;
+
+  beforeEach(async(() => {
+    activeGroup = GroupMock;
+    allGroups = [GroupMock, GroupMock2]
+    ePersonDataServiceStub = {};
+    groupsDataServiceStub = {
+      activeGroup: activeGroup,
+      getActiveGroup(): Observable<Group> {
+        return observableOf(this.activeGroup);
+      },
+      findAllByHref(href: string): Observable<RemoteData<PaginatedList<Group>>> {
+        return createSuccessfulRemoteDataObject$(new PaginatedList<Group>(new PageInfo(), this.activeGroup.subgroups))
+      },
+      getGroupEditPageRouterLink(group: Group): string {
+        return '/admin/access-control/groups/' + group.id;
+      },
+      searchGroups(query: string): Observable<RemoteData<PaginatedList<Group>>> {
+        if (query === '') {
+          return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), allGroups))
+        }
+        return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), []))
+      },
+      addSubGroupToGroup(parentGroup, subgroup: Group): Observable<RestResponse> {
+        this.activeGroup.subgroups = [...this.activeGroup.subgroups, subgroup];
+        return observableOf(new RestResponse(true, 200, 'Success'));
+      },
+      clearGroupsRequests() {
+        // empty
+      },
+      deleteSubGroupFromGroup(parentGroup, subgroup: Group): Observable<RestResponse>  {
+        this.activeGroup.subgroups = this.activeGroup.subgroups.find((group: Group) => {
+          if (group.id !== subgroup.id) {
+            return group;
+          }
+        });
+        return observableOf(new RestResponse(true, 200, 'Success'));
+      }
+    };
+    builderService = getMockFormBuilderService();
+    translateService = getMockTranslateService();
+    TestBed.configureTestingModule({
+      imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule,
+        TranslateModule.forRoot({
+          loader: {
+            provide: TranslateLoader,
+            useClass: MockTranslateLoader
+          }
+        }),
+      ],
+      declarations: [SubgroupsListComponent],
+      providers: [SubgroupsListComponent,
+        { provide: GroupDataService, useValue: groupsDataServiceStub },
+        { provide: NotificationsService, useValue: new NotificationsServiceStub() },
+        { provide: FormBuilderService, useValue: builderService },
+      ],
+      schemas: [NO_ERRORS_SCHEMA]
+    }).compileComponents();
+  }));
+
+  beforeEach(() => {
+    fixture = TestBed.createComponent(SubgroupsListComponent);
+    component = fixture.componentInstance;
+    fixture.detectChanges();
+  });
+
+  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'));
+    expect(groupIdsFound.length).toEqual(1);
+    activeGroup.subgroups.map((group: Group) => {
+      expect(groupIdsFound.find((foundEl) => {
+        return (foundEl.nativeElement.textContent.trim() === group.uuid);
+      })).toBeTruthy();
+    })
+  });
+
+  describe('search', () => {
+    describe('when searching without query', () => {
+      let groupsFound;
+      beforeEach(fakeAsync(() => {
+        component.search({ query: '' });
+        tick();
+        fixture.detectChanges();
+        groupsFound = fixture.debugElement.queryAll(By.css('#groups tbody tr'));
+      }));
+
+      it('should display all groups', () => {
+        expect(groupsFound.length).toEqual(2);
+      });
+
+      describe('if group is already a subgroup', () => {
+        it('should have delete button, else it should have add button', () => {
+          activeGroup.subgroups.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();
+                }
+              }
+            })
+          })
+        });
+      });
+
+      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);
+        });
+      });
+    });
+  });
+
+});

src/app/+admin/admin-access-control/group-registry/group-form/subgroup-list/subgroups-list.component.ts

@@ -169,7 +169,7 @@ export class SubgroupsListComponent implements OnInit, OnDestroy {
     this.groups = this.groupDataService.findAllByHref(activeGroup._links.subgroups.href, {
       currentPage: 1,
       elementsPerPage: this.config.pageSize
-    })
+    });
   }
 
   /**

src/app/shared/testing/group-mock.ts

@@ -3,7 +3,7 @@ import { EPersonMock } from './eperson-mock';
 
 export const GroupMock2: Group = Object.assign(new Group(), {
   handle: null,
-  groups: [],
+  subgroups: [],
   epersons: [],
   selfRegistered: false,
   _links: {
@@ -20,7 +20,7 @@ export const GroupMock2: Group = Object.assign(new Group(), {
 
 export const GroupMock: Group = Object.assign(new Group(), {
   handle: null,
-  groups: [GroupMock2],
+  subgroups: [GroupMock2],
   epersons: [EPersonMock],
   selfRegistered: false,
   _links: {