Skip to content

Commit 325728d

Browse files
committed
112198: Ensure relationship requests are sent one by one
- remove (de)select all buttons in DynamicLookupRelationSearchTab - add requestQueue to RelationshipEffects
1 parent 404ccd9 commit 325728d

4 files changed

Lines changed: 86 additions & 38 deletions

File tree

src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,8 @@ describe('RelationshipEffects', () => {
182182
let action;
183183
describe('When the last value in the debounceMap is also an ADD_RELATIONSHIP action', () => {
184184
beforeEach(() => {
185+
jasmine.getEnv().allowRespy(true);
186+
spyOn((relationEffects as any), 'addRelationship').and.returnValue(createSuccessfulRemoteDataObject$(relationship));
185187
(relationEffects as any).initialActionMap[identifier] = RelationshipActionTypes.ADD_RELATIONSHIP;
186188
((relationEffects as any).debounceTime as jasmine.Spy).and.returnValue((v) => v);
187189
});
@@ -247,6 +249,8 @@ describe('RelationshipEffects', () => {
247249
let action;
248250
describe('When the last value in the debounceMap is also an REMOVE_RELATIONSHIP action', () => {
249251
beforeEach(() => {
252+
jasmine.getEnv().allowRespy(true);
253+
spyOn((relationEffects as any), 'removeRelationship').and.returnValue(createSuccessfulRemoteDataObject$(undefined));
250254
((relationEffects as any).debounceTime as jasmine.Spy).and.returnValue((v) => v);
251255
(relationEffects as any).initialActionMap[identifier] = RelationshipActionTypes.REMOVE_RELATIONSHIP;
252256
});
@@ -256,7 +260,7 @@ describe('RelationshipEffects', () => {
256260
actions = hot('--a-|', { a: action });
257261
const expected = cold('--b-|', { b: undefined });
258262
expect(relationEffects.mapLastActions$).toBeObservable(expected);
259-
expect((relationEffects as any).removeRelationship).toHaveBeenCalledWith(leftItem, rightItem, relationshipType.leftwardType, '1234',);
263+
expect((relationEffects as any).removeRelationship).toHaveBeenCalledWith(leftItem, rightItem, relationshipType.leftwardType);
260264
});
261265
});
262266

src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts

Lines changed: 76 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { Inject, Injectable } from '@angular/core';
22
import { Actions, createEffect, ofType } from '@ngrx/effects';
3-
import { filter, map, mergeMap, switchMap, take } from 'rxjs/operators';
4-
import { BehaviorSubject, Observable } from 'rxjs';
3+
import { filter, map, mergeMap, switchMap, take, tap, concatMap } from 'rxjs/operators';
4+
import { BehaviorSubject, Observable, Subject } from 'rxjs';
55
import { RelationshipDataService } from '../../../../../core/data/relationship-data.service';
66
import {
77
getRemoteDataPayload,
@@ -36,11 +36,31 @@ import { TranslateService } from '@ngx-translate/core';
3636

3737
const DEBOUNCE_TIME = 500;
3838

39+
enum RelationOperationType {
40+
Add,
41+
Remove,
42+
}
43+
44+
interface RelationOperation {
45+
type: RelationOperationType
46+
item1: Item
47+
item2: Item
48+
relationshipType: string
49+
submissionId: string
50+
nameVariant?: string
51+
}
52+
3953
/**
4054
* NGRX effects for RelationshipEffects
4155
*/
4256
@Injectable()
4357
export class RelationshipEffects {
58+
59+
/**
60+
* Queue to hold all requests, so we can ensure they get sent one at a time
61+
*/
62+
private requestQueue: Subject<RelationOperation> = new Subject();
63+
4464
/**
4565
* Map that keeps track of the latest RelationshipEffects for each relationship's composed identifier
4666
*/
@@ -82,9 +102,22 @@ export class RelationshipEffects {
82102
nameVariant = this.nameVariantUpdates[identifier];
83103
delete this.nameVariantUpdates[identifier];
84104
}
85-
this.addRelationship(item1, item2, relationshipType, submissionId, nameVariant);
105+
this.requestQueue.next({
106+
type: RelationOperationType.Add,
107+
item1,
108+
item2,
109+
relationshipType,
110+
submissionId,
111+
nameVariant
112+
});
86113
} else {
87-
this.removeRelationship(item1, item2, relationshipType, submissionId);
114+
this.requestQueue.next({
115+
type: RelationOperationType.Remove,
116+
item1,
117+
item2,
118+
relationshipType,
119+
submissionId,
120+
});
88121
}
89122
}
90123
delete this.debounceMap[identifier];
@@ -161,8 +194,41 @@ export class RelationshipEffects {
161194
private selectableListService: SelectableListService,
162195
@Inject(DEBOUNCE_TIME_OPERATOR) private debounceTime: <T>(dueTime: number) => (source: Observable<T>) => Observable<T>,
163196
) {
197+
this.executeRequestsInQueue();
164198
}
165199

200+
/**
201+
* Subscribe to the request queue, execute the requests inside. Wait for each request to complete
202+
* before sending the next one
203+
* @private
204+
*/
205+
private executeRequestsInQueue() {
206+
this.requestQueue.pipe(
207+
// concatMap ensures the next request in the queue will only start after the previous one has emitted
208+
concatMap((next: RelationOperation) => {
209+
switch (next.type) {
210+
case RelationOperationType.Add:
211+
return this.addRelationship(next.item1, next.item2, next.relationshipType, next.submissionId, next.nameVariant).pipe(
212+
map(() => next)
213+
);
214+
case RelationOperationType.Remove:
215+
return this.removeRelationship(next.item1, next.item2, next.relationshipType).pipe(
216+
map(() => next)
217+
);
218+
default:
219+
return [next];
220+
}
221+
}),
222+
// refresh the workspaceitem after each request. It would be great if we could find a way to
223+
// optimize this so it only happens when the queue is empty.
224+
switchMap((next: RelationOperation) => this.refreshWorkspaceItemInCache(next.submissionId))
225+
// update the form after the workspaceitem is refreshed
226+
).subscribe((next: SubmissionObject) => {
227+
this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(next.id, [next], false));
228+
});
229+
}
230+
231+
166232
private createIdentifier(item1: Item, item2: Item, relationshipType: string): string {
167233
return `${item1.uuid}-${item2.uuid}-${relationshipType}`;
168234
}
@@ -185,7 +251,7 @@ export class RelationshipEffects {
185251
}
186252
}),
187253
take(1),
188-
switchMap((rd: RemoteData<Relationship>) => {
254+
tap((rd: RemoteData<Relationship>) => {
189255
if (hasNoValue(rd) || rd.hasFailed) {
190256
// An error occurred, deselect the object from the selectable list and display an error notification
191257
const listId = `list-${submissionId}-${relationshipType}`;
@@ -203,19 +269,15 @@ export class RelationshipEffects {
203269
}
204270
this.notificationsService.error(this.translateService.instant('relationships.add.error.title'), errorContent);
205271
}
206-
return this.refreshWorkspaceItemInCache(submissionId);
207-
}),
208-
).subscribe((submissionObject: SubmissionObject) => this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(submissionId, [submissionObject], false)));
272+
})
273+
);
209274
}
210275

211-
private removeRelationship(item1: Item, item2: Item, relationshipType: string, submissionId: string) {
212-
this.relationshipService.getRelationshipByItemsAndLabel(item1, item2, relationshipType).pipe(
276+
private removeRelationship(item1: Item, item2: Item, relationshipType: string) {
277+
return this.relationshipService.getRelationshipByItemsAndLabel(item1, item2, relationshipType).pipe(
213278
mergeMap((relationship: Relationship) => this.relationshipService.deleteRelationship(relationship.id, 'none')),
214279
take(1),
215-
switchMap(() => this.refreshWorkspaceItemInCache(submissionId)),
216-
).subscribe((submissionObject: SubmissionObject) => {
217-
this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(submissionId, [submissionObject], false));
218-
});
280+
);
219281
}
220282

221283
/**

src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.html

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,7 @@
1515
(selectObject)="selectObject.emit($event)">
1616
<div additionalSearchOptions *ngIf="repeatable" class="position-absolute">
1717
<div class="input-group mb-3">
18-
<div class="input-group-prepend">
19-
<div class="input-group-text">
20-
<!-- In theory we don't need separate checkboxes for this,
21-
but I wasn't able to get this to work correctly without them.
22-
Checkboxes that are in the indeterminate state always switch to checked when clicked
23-
This seemed like the cleanest and clearest solution to solve this issue for now. -->
24-
25-
<input *ngIf="!allSelected && !(someSelected$ | async)"
26-
type="checkbox"
27-
[indeterminate]="false"
28-
(change)="selectAll()">
29-
<input *ngIf="!allSelected && (someSelected$ | async)"
30-
type="checkbox"
31-
[indeterminate]="true"
32-
(change)="deselectAll()">
33-
<input *ngIf="allSelected" type="checkbox"
34-
[checked]="true"
35-
(change)="deselectAll()">
36-
</div>
37-
</div>
38-
<div ngbDropdown class="input-group-append">
18+
<div ngbDropdown class="input-group dropdown-button">
3919
<button *ngIf="selectAllLoading" type="button"
4020
class="btn btn-outline-secondary rounded-right">
4121
<span class="spinner-border spinner-border-sm" role="status"
@@ -55,8 +35,6 @@
5535
(click)="selectPage(resultsRD?.page)">{{ ('submission.sections.describe.relationship-lookup.search-tab.select-page' | translate) }}</button>
5636
<button class="dropdown-item"
5737
(click)="deselectPage(resultsRD?.page)">{{ ('submission.sections.describe.relationship-lookup.search-tab.deselect-page' | translate) }}</button>
58-
<button class="dropdown-item" (click)="selectAll()">{{ ('submission.sections.describe.relationship-lookup.search-tab.select-all' | translate) }}</button>
59-
<button class="dropdown-item" (click)="deselectAll()">{{ ('submission.sections.describe.relationship-lookup.search-tab.deselect-all' | translate) }}</button>
6038
</div>
6139
</div>
6240
</div>
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
11
.position-absolute {
22
right: var(--bs-spacer);
33
}
4+
5+
.dropdown-button {
6+
z-index: 3;
7+
}

0 commit comments

Comments
 (0)