From 325728de8fd401d072a8dd1ec90fc4748aa09892 Mon Sep 17 00:00:00 2001 From: Nona Luypaert <nona.luypaert@atmire.com> Date: Mon, 19 Feb 2024 11:09:05 +0100 Subject: [PATCH] 112198: Ensure relationship requests are sent one by one - remove (de)select all buttons in DynamicLookupRelationSearchTab - add requestQueue to RelationshipEffects --- .../relationship.effects.spec.ts | 6 +- .../relationship.effects.ts | 90 ++++++++++++++++--- ...-lookup-relation-search-tab.component.html | 24 +---- ...-lookup-relation-search-tab.component.scss | 4 + 4 files changed, 86 insertions(+), 38 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts index a5b1b00d75e..ef28b3ec2f8 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts @@ -182,6 +182,8 @@ describe('RelationshipEffects', () => { let action; describe('When the last value in the debounceMap is also an ADD_RELATIONSHIP action', () => { beforeEach(() => { + jasmine.getEnv().allowRespy(true); + spyOn((relationEffects as any), 'addRelationship').and.returnValue(createSuccessfulRemoteDataObject$(relationship)); (relationEffects as any).initialActionMap[identifier] = RelationshipActionTypes.ADD_RELATIONSHIP; ((relationEffects as any).debounceTime as jasmine.Spy).and.returnValue((v) => v); }); @@ -247,6 +249,8 @@ describe('RelationshipEffects', () => { let action; describe('When the last value in the debounceMap is also an REMOVE_RELATIONSHIP action', () => { beforeEach(() => { + jasmine.getEnv().allowRespy(true); + spyOn((relationEffects as any), 'removeRelationship').and.returnValue(createSuccessfulRemoteDataObject$(undefined)); ((relationEffects as any).debounceTime as jasmine.Spy).and.returnValue((v) => v); (relationEffects as any).initialActionMap[identifier] = RelationshipActionTypes.REMOVE_RELATIONSHIP; }); @@ -256,7 +260,7 @@ describe('RelationshipEffects', () => { actions = hot('--a-|', { a: action }); const expected = cold('--b-|', { b: undefined }); expect(relationEffects.mapLastActions$).toBeObservable(expected); - expect((relationEffects as any).removeRelationship).toHaveBeenCalledWith(leftItem, rightItem, relationshipType.leftwardType, '1234',); + expect((relationEffects as any).removeRelationship).toHaveBeenCalledWith(leftItem, rightItem, relationshipType.leftwardType); }); }); diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts index 2b9c1c29737..cb9cdace07c 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts @@ -1,7 +1,7 @@ import { Inject, Injectable } from '@angular/core'; import { Actions, createEffect, ofType } from '@ngrx/effects'; -import { filter, map, mergeMap, switchMap, take } from 'rxjs/operators'; -import { BehaviorSubject, Observable } from 'rxjs'; +import { filter, map, mergeMap, switchMap, take, tap, concatMap } from 'rxjs/operators'; +import { BehaviorSubject, Observable, Subject } from 'rxjs'; import { RelationshipDataService } from '../../../../../core/data/relationship-data.service'; import { getRemoteDataPayload, @@ -36,11 +36,31 @@ import { TranslateService } from '@ngx-translate/core'; const DEBOUNCE_TIME = 500; +enum RelationOperationType { + Add, + Remove, +} + +interface RelationOperation { + type: RelationOperationType + item1: Item + item2: Item + relationshipType: string + submissionId: string + nameVariant?: string +} + /** * NGRX effects for RelationshipEffects */ @Injectable() export class RelationshipEffects { + + /** + * Queue to hold all requests, so we can ensure they get sent one at a time + */ + private requestQueue: Subject<RelationOperation> = new Subject(); + /** * Map that keeps track of the latest RelationshipEffects for each relationship's composed identifier */ @@ -82,9 +102,22 @@ export class RelationshipEffects { nameVariant = this.nameVariantUpdates[identifier]; delete this.nameVariantUpdates[identifier]; } - this.addRelationship(item1, item2, relationshipType, submissionId, nameVariant); + this.requestQueue.next({ + type: RelationOperationType.Add, + item1, + item2, + relationshipType, + submissionId, + nameVariant + }); } else { - this.removeRelationship(item1, item2, relationshipType, submissionId); + this.requestQueue.next({ + type: RelationOperationType.Remove, + item1, + item2, + relationshipType, + submissionId, + }); } } delete this.debounceMap[identifier]; @@ -161,8 +194,41 @@ export class RelationshipEffects { private selectableListService: SelectableListService, @Inject(DEBOUNCE_TIME_OPERATOR) private debounceTime: <T>(dueTime: number) => (source: Observable<T>) => Observable<T>, ) { + this.executeRequestsInQueue(); } + /** + * Subscribe to the request queue, execute the requests inside. Wait for each request to complete + * before sending the next one + * @private + */ + private executeRequestsInQueue() { + this.requestQueue.pipe( + // concatMap ensures the next request in the queue will only start after the previous one has emitted + concatMap((next: RelationOperation) => { + switch (next.type) { + case RelationOperationType.Add: + return this.addRelationship(next.item1, next.item2, next.relationshipType, next.submissionId, next.nameVariant).pipe( + map(() => next) + ); + case RelationOperationType.Remove: + return this.removeRelationship(next.item1, next.item2, next.relationshipType).pipe( + map(() => next) + ); + default: + return [next]; + } + }), + // refresh the workspaceitem after each request. It would be great if we could find a way to + // optimize this so it only happens when the queue is empty. + switchMap((next: RelationOperation) => this.refreshWorkspaceItemInCache(next.submissionId)) + // update the form after the workspaceitem is refreshed + ).subscribe((next: SubmissionObject) => { + this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(next.id, [next], false)); + }); + } + + private createIdentifier(item1: Item, item2: Item, relationshipType: string): string { return `${item1.uuid}-${item2.uuid}-${relationshipType}`; } @@ -185,7 +251,7 @@ export class RelationshipEffects { } }), take(1), - switchMap((rd: RemoteData<Relationship>) => { + tap((rd: RemoteData<Relationship>) => { if (hasNoValue(rd) || rd.hasFailed) { // An error occurred, deselect the object from the selectable list and display an error notification const listId = `list-${submissionId}-${relationshipType}`; @@ -203,19 +269,15 @@ export class RelationshipEffects { } this.notificationsService.error(this.translateService.instant('relationships.add.error.title'), errorContent); } - return this.refreshWorkspaceItemInCache(submissionId); - }), - ).subscribe((submissionObject: SubmissionObject) => this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(submissionId, [submissionObject], false))); + }) + ); } - private removeRelationship(item1: Item, item2: Item, relationshipType: string, submissionId: string) { - this.relationshipService.getRelationshipByItemsAndLabel(item1, item2, relationshipType).pipe( + private removeRelationship(item1: Item, item2: Item, relationshipType: string) { + return this.relationshipService.getRelationshipByItemsAndLabel(item1, item2, relationshipType).pipe( mergeMap((relationship: Relationship) => this.relationshipService.deleteRelationship(relationship.id, 'none')), take(1), - switchMap(() => this.refreshWorkspaceItemInCache(submissionId)), - ).subscribe((submissionObject: SubmissionObject) => { - this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(submissionId, [submissionObject], false)); - }); + ); } /** diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.html index 17aafae5eb9..00bfa7981aa 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.html @@ -15,27 +15,7 @@ (selectObject)="selectObject.emit($event)"> <div additionalSearchOptions *ngIf="repeatable" class="position-absolute"> <div class="input-group mb-3"> - <div class="input-group-prepend"> - <div class="input-group-text"> - <!-- In theory we don't need separate checkboxes for this, - but I wasn't able to get this to work correctly without them. - Checkboxes that are in the indeterminate state always switch to checked when clicked - This seemed like the cleanest and clearest solution to solve this issue for now. --> - - <input *ngIf="!allSelected && !(someSelected$ | async)" - type="checkbox" - [indeterminate]="false" - (change)="selectAll()"> - <input *ngIf="!allSelected && (someSelected$ | async)" - type="checkbox" - [indeterminate]="true" - (change)="deselectAll()"> - <input *ngIf="allSelected" type="checkbox" - [checked]="true" - (change)="deselectAll()"> - </div> - </div> - <div ngbDropdown class="input-group-append"> + <div ngbDropdown class="input-group dropdown-button"> <button *ngIf="selectAllLoading" type="button" class="btn btn-outline-secondary rounded-right"> <span class="spinner-border spinner-border-sm" role="status" @@ -55,8 +35,6 @@ (click)="selectPage(resultsRD?.page)">{{ ('submission.sections.describe.relationship-lookup.search-tab.select-page' | translate) }}</button> <button class="dropdown-item" (click)="deselectPage(resultsRD?.page)">{{ ('submission.sections.describe.relationship-lookup.search-tab.deselect-page' | translate) }}</button> - <button class="dropdown-item" (click)="selectAll()">{{ ('submission.sections.describe.relationship-lookup.search-tab.select-all' | translate) }}</button> - <button class="dropdown-item" (click)="deselectAll()">{{ ('submission.sections.describe.relationship-lookup.search-tab.deselect-all' | translate) }}</button> </div> </div> </div> diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.scss b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.scss index f73f8b8c34b..b4bda36e164 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.scss +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.scss @@ -1,3 +1,7 @@ .position-absolute { right: var(--bs-spacer); } + +.dropdown-button { + z-index: 3; +}