From 9aea3e0bbf48cc3c62a092cf1ffe7c3330d94b37 Mon Sep 17 00:00:00 2001 From: Rezart Vata Date: Fri, 29 Oct 2021 17:41:57 +0200 Subject: [PATCH 1/3] [CSTPER-869] validator to check if a group already exists --- .../group-form/group-form.component.ts | 22 ++++++++++-- .../validators/group-exists.validator.ts | 34 +++++++++++++++++++ src/assets/i18n/en.json5 | 2 ++ 3 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 src/app/access-control/group-registry/group-form/validators/group-exists.validator.ts diff --git a/src/app/access-control/group-registry/group-form/group-form.component.ts b/src/app/access-control/group-registry/group-form/group-form.component.ts index b2b9fab58d..072fdda063 100644 --- a/src/app/access-control/group-registry/group-form/group-form.component.ts +++ b/src/app/access-control/group-registry/group-form/group-form.component.ts @@ -1,4 +1,4 @@ -import { Component, EventEmitter, HostListener, OnDestroy, OnInit, Output } from '@angular/core'; +import { Component, EventEmitter, HostListener, OnDestroy, OnInit, Output, ChangeDetectorRef } from '@angular/core'; import { FormGroup } from '@angular/forms'; import { ActivatedRoute, Router } from '@angular/router'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; @@ -16,7 +16,7 @@ import { of as observableOf, Subscription, } from 'rxjs'; -import { catchError, map, switchMap, take, filter } from 'rxjs/operators'; +import { catchError, map, switchMap, take, filter, debounceTime } from 'rxjs/operators'; import { getCollectionEditRolesRoute } from '../../../collection-page/collection-page-routing-paths'; import { getCommunityEditRolesRoute } from '../../../community-page/community-page-routing-paths'; import { DSpaceObjectDataService } from '../../../core/data/dspace-object-data.service'; @@ -45,6 +45,7 @@ import { NotificationsService } from '../../../shared/notifications/notification import { followLink } from '../../../shared/utils/follow-link-config.model'; import { NoContent } from '../../../core/shared/NoContent.model'; import { Operation } from 'fast-json-patch'; +import { ValidateGroupExists } from './validators/group-exists.validator'; @Component({ selector: 'ds-group-form', @@ -126,6 +127,12 @@ export class GroupFormComponent implements OnInit, OnDestroy { */ public AlertTypeEnum = AlertType; + /** + * Subscription to email field value change + */ + groupNameValueChangeSubscribe: Subscription; + + constructor(public groupDataService: GroupDataService, private ePersonDataService: EPersonDataService, private dSpaceObjectDataService: DSpaceObjectDataService, @@ -136,7 +143,8 @@ export class GroupFormComponent implements OnInit, OnDestroy { protected router: Router, private authorizationService: AuthorizationDataService, private modalService: NgbModal, - public requestService: RequestService) { + public requestService: RequestService, + protected changeDetectorRef: ChangeDetectorRef) { } ngOnInit() { @@ -192,6 +200,14 @@ export class GroupFormComponent implements OnInit, OnDestroy { this.groupDescription, ]; this.formGroup = this.formBuilderService.createFormGroup(this.formModel); + + if (!!this.formGroup.controls.groupName && !this.formGroup.controls.groupName.value) { + this.formGroup.controls.groupName.setAsyncValidators(ValidateGroupExists.createValidator(this.groupDataService)); + this.groupNameValueChangeSubscribe = this.groupName.valueChanges.pipe(debounceTime(300)).subscribe(() => { + this.changeDetectorRef.detectChanges(); + }); + } + this.subs.push( observableCombineLatest( this.groupDataService.getActiveGroup(), diff --git a/src/app/access-control/group-registry/group-form/validators/group-exists.validator.ts b/src/app/access-control/group-registry/group-form/validators/group-exists.validator.ts new file mode 100644 index 0000000000..90553fbc76 --- /dev/null +++ b/src/app/access-control/group-registry/group-form/validators/group-exists.validator.ts @@ -0,0 +1,34 @@ +import { AbstractControl, ValidationErrors } from '@angular/forms'; +import { Observable } from 'rxjs'; +import { map, filter } from 'rxjs/operators'; + +import { buildPaginatedList, PaginatedList } from '../../../../core/data/paginated-list.model'; +import { GroupDataService } from '../../../../core/eperson/group-data.service'; +import { getFirstSucceededRemoteData,getFirstSucceededRemoteListPayload } from '../../../../core/shared/operators'; +import { Group } from '../../../../core/eperson/models/group.model'; + +export class ValidateGroupExists { + + /** + * This method will create the validator with the groupDataService requested from component + * @param groupDataService the service with DI in the component that this validator is being utilized. + * @return Observable + */ + static createValidator(groupDataService: GroupDataService) { + return (control: AbstractControl): Promise | Observable => { + return groupDataService.searchGroups(control.value, { + currentPage: 1, + elementsPerPage: 100 + }) + .pipe( + getFirstSucceededRemoteListPayload(), + map( (groups: Group[]) => { + return groups.filter(group => group.name === control.value); + }), + map( (groups: Group[]) => { + return groups.length > 0 ? { groupExists: true } : null; + }), + ); + }; + } +} diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 5cd42bc24c..081f48df3e 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1351,6 +1351,8 @@ "error.validation.emailTaken": "This E-mail is already taken", + "error.validation.groupExists": "This group already exists", + "file-section.error.header": "Error obtaining files for this item", From ce7a5b1499e87c3908cb2917d714d923cbb45e11 Mon Sep 17 00:00:00 2001 From: Rezart Vata Date: Mon, 1 Nov 2021 13:38:26 +0100 Subject: [PATCH 2/3] [CSTPER-869] Fix on edit remove validator, unit testing , lint fixes & report --- .../group-form/group-form.component.spec.ts | 135 +++++++++++++++++- .../group-form/group-form.component.ts | 11 +- 2 files changed, 143 insertions(+), 3 deletions(-) diff --git a/src/app/access-control/group-registry/group-form/group-form.component.spec.ts b/src/app/access-control/group-registry/group-form/group-form.component.spec.ts index 5f0f570044..2307f3c6fa 100644 --- a/src/app/access-control/group-registry/group-form/group-form.component.spec.ts +++ b/src/app/access-control/group-registry/group-form/group-form.component.spec.ts @@ -2,7 +2,7 @@ import { CommonModule } from '@angular/common'; import { HttpClient } from '@angular/common/http'; import { NO_ERRORS_SCHEMA } from '@angular/core'; import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; -import { FormsModule, ReactiveFormsModule } from '@angular/forms'; +import { FormsModule, ReactiveFormsModule, FormArray, FormControl, FormGroup,Validators, NG_VALIDATORS, NG_ASYNC_VALIDATORS } from '@angular/forms'; import { BrowserModule } from '@angular/platform-browser'; import { ActivatedRoute, Router } from '@angular/router'; import { NgbModule } from '@ng-bootstrap/ng-bootstrap'; @@ -34,6 +34,7 @@ import { TranslateLoaderMock } from '../../../shared/testing/translate-loader.mo import { RouterMock } from '../../../shared/mocks/router.mock'; import { NotificationsServiceStub } from '../../../shared/testing/notifications-service.stub'; import { Operation } from 'fast-json-patch'; +import { ValidateGroupExists } from './validators/group-exists.validator'; describe('GroupFormComponent', () => { let component: GroupFormComponent; @@ -117,7 +118,69 @@ describe('GroupFormComponent', () => { return null; } }; - builderService = getMockFormBuilderService(); + builderService = Object.assign(getMockFormBuilderService(),{ + createFormGroup(formModel, options = null) { + const controls = {}; + formModel.forEach( model => { + model.parent = parent; + const controlModel = model; + const controlState = { value: controlModel.value, disabled: controlModel.disabled }; + const controlOptions = this.createAbstractControlOptions(controlModel.validators, controlModel.asyncValidators, controlModel.updateOn); + controls[model.id] = new FormControl(controlState, controlOptions); + }); + return new FormGroup(controls, options); + }, + createAbstractControlOptions(validatorsConfig = null, asyncValidatorsConfig = null, updateOn = null) { + return { + validators: validatorsConfig !== null ? this.getValidators(validatorsConfig) : null, + }; + }, + getValidators(validatorsConfig) { + return this.getValidatorFns(validatorsConfig); + }, + getValidatorFns(validatorsConfig, validatorsToken = this._NG_VALIDATORS) { + let validatorFns = []; + if (this.isObject(validatorsConfig)) { + validatorFns = Object.keys(validatorsConfig).map(validatorConfigKey => { + const validatorConfigValue = validatorsConfig[validatorConfigKey]; + if (this.isValidatorDescriptor(validatorConfigValue)) { + const descriptor = validatorConfigValue; + return this.getValidatorFn(descriptor.name, descriptor.args, validatorsToken); + } + return this.getValidatorFn(validatorConfigKey, validatorConfigValue, validatorsToken); + }); + } + return validatorFns; + }, + getValidatorFn(validatorName, validatorArgs = null, validatorsToken = this._NG_VALIDATORS) { + let validatorFn; + if (Validators.hasOwnProperty(validatorName)) { // Built-in Angular Validators + validatorFn = Validators[validatorName]; + } else { // Custom Validators + if (this._DYNAMIC_VALIDATORS && this._DYNAMIC_VALIDATORS.has(validatorName)) { + validatorFn = this._DYNAMIC_VALIDATORS.get(validatorName); + } else if (validatorsToken) { + validatorFn = validatorsToken.find(validator => validator.name === validatorName); + } + } + if (validatorFn === undefined) { // throw when no validator could be resolved + throw new Error(`validator '${validatorName}' is not provided via NG_VALIDATORS, NG_ASYNC_VALIDATORS or DYNAMIC_FORM_VALIDATORS`); + } + if (validatorArgs !== null) { + return validatorFn(validatorArgs); + } + return validatorFn; + }, + isValidatorDescriptor(value) { + if (this.isObject(value)) { + return value.hasOwnProperty('name') && value.hasOwnProperty('args'); + } + return false; + }, + isObject(value) { + return typeof value === 'object' && value !== null; + } + }); translateService = getMockTranslateService(); router = new RouterMock(); notificationService = new NotificationsServiceStub(); @@ -217,4 +280,72 @@ describe('GroupFormComponent', () => { }); }); + + describe('check form validation', () => { + let groupCommunity; + + beforeEach(() => { + groupName = 'testName'; + groupCommunity = 'testgroupCommunity'; + groupDescription = 'testgroupDescription'; + + expected = Object.assign(new Group(), { + name: groupName, + metadata: { + 'dc.description': [ + { + value: groupDescription + } + ], + }, + }); + spyOn(component.submitForm, 'emit'); + + fixture.detectChanges(); + component.initialisePage(); + fixture.detectChanges(); + }); + describe('groupName, groupCommunity and groupDescription should be required', () => { + it('form should be invalid because the groupName is required', waitForAsync(() => { + fixture.whenStable().then(() => { + expect(component.formGroup.controls.groupName.valid).toBeFalse(); + expect(component.formGroup.controls.groupName.errors.required).toBeTrue(); + }); + })); + }); + + describe('after inserting information groupName,groupCommunity and groupDescription not required', () => { + beforeEach(() => { + component.formGroup.controls.groupName.setValue('test'); + fixture.detectChanges(); + }); + it('groupName should be valid because the groupName is set', waitForAsync(() => { + fixture.whenStable().then(() => { + expect(component.formGroup.controls.groupName.valid).toBeTrue(); + expect(component.formGroup.controls.groupName.errors).toBeNull(); + }); + })); + }); + + describe('after already utilized groupName', () => { + beforeEach(() => { + const groupsDataServiceStubWithGroup = Object.assign(groupsDataServiceStub,{ + searchGroups(query: string): Observable>> { + return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), [expected])); + } + }); + component.formGroup.controls.groupName.setValue('testName'); + component.formGroup.controls.groupName.setAsyncValidators(ValidateGroupExists.createValidator(groupsDataServiceStubWithGroup)); + fixture.detectChanges(); + }); + + it('groupName should not be valid because groupName is already taken', waitForAsync(() => { + fixture.whenStable().then(() => { + expect(component.formGroup.controls.groupName.valid).toBeFalse(); + expect(component.formGroup.controls.groupName.errors.groupExists).toBeTruthy(); + }); + })); + }); + }); + }); diff --git a/src/app/access-control/group-registry/group-form/group-form.component.ts b/src/app/access-control/group-registry/group-form/group-form.component.ts index 072fdda063..826b7dbe69 100644 --- a/src/app/access-control/group-registry/group-form/group-form.component.ts +++ b/src/app/access-control/group-registry/group-form/group-form.component.ts @@ -201,7 +201,7 @@ export class GroupFormComponent implements OnInit, OnDestroy { ]; this.formGroup = this.formBuilderService.createFormGroup(this.formModel); - if (!!this.formGroup.controls.groupName && !this.formGroup.controls.groupName.value) { + if (!!this.formGroup.controls.groupName) { this.formGroup.controls.groupName.setAsyncValidators(ValidateGroupExists.createValidator(this.groupDataService)); this.groupNameValueChangeSubscribe = this.groupName.valueChanges.pipe(debounceTime(300)).subscribe(() => { this.changeDetectorRef.detectChanges(); @@ -217,6 +217,10 @@ export class GroupFormComponent implements OnInit, OnDestroy { ).subscribe(([activeGroup, canEdit, linkedObject]) => { if (activeGroup != null) { + + // Disable group name exists validator + this.formGroup.controls.groupName.clearAsyncValidators(); + this.groupBeingEdited = activeGroup; if (linkedObject?.name) { @@ -452,6 +456,11 @@ export class GroupFormComponent implements OnInit, OnDestroy { ngOnDestroy(): void { this.groupDataService.cancelEditGroup(); this.subs.filter((sub) => hasValue(sub)).forEach((sub) => sub.unsubscribe()); + + if ( hasValue(this.groupNameValueChangeSubscribe) ) { + this.groupNameValueChangeSubscribe.unsubscribe(); + } + } /** From f9c8ac6568b134139efb6f2dfe09b33d296c1447 Mon Sep 17 00:00:00 2001 From: Corrado Lombardi Date: Tue, 16 Nov 2021 22:28:43 +0100 Subject: [PATCH 3/3] [CSTPER-869] removed unused elements --- .../group-form/validators/group-exists.validator.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/app/access-control/group-registry/group-form/validators/group-exists.validator.ts b/src/app/access-control/group-registry/group-form/validators/group-exists.validator.ts index 90553fbc76..88f22413e9 100644 --- a/src/app/access-control/group-registry/group-form/validators/group-exists.validator.ts +++ b/src/app/access-control/group-registry/group-form/validators/group-exists.validator.ts @@ -1,10 +1,9 @@ import { AbstractControl, ValidationErrors } from '@angular/forms'; import { Observable } from 'rxjs'; -import { map, filter } from 'rxjs/operators'; +import { map} from 'rxjs/operators'; -import { buildPaginatedList, PaginatedList } from '../../../../core/data/paginated-list.model'; import { GroupDataService } from '../../../../core/eperson/group-data.service'; -import { getFirstSucceededRemoteData,getFirstSucceededRemoteListPayload } from '../../../../core/shared/operators'; +import { getFirstSucceededRemoteListPayload } from '../../../../core/shared/operators'; import { Group } from '../../../../core/eperson/models/group.model'; export class ValidateGroupExists {