Skip to content

Commit 245187a

Browse files
committed
Merge remote-tracking branch 'atmire-github/w2p-112198_add-relationship-effects-queue' into w2p-112198_add-relationship-effects-queue-main
Conflicts: src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.html
2 parents 3ab9f6a + 325728d commit 245187a

4 files changed

Lines changed: 87 additions & 35 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
@@ -197,6 +197,8 @@ describe('RelationshipEffects', () => {
197197
let action;
198198
describe('When the last value in the debounceMap is also an ADD_RELATIONSHIP action', () => {
199199
beforeEach(() => {
200+
jasmine.getEnv().allowRespy(true);
201+
spyOn((relationEffects as any), 'addRelationship').and.returnValue(createSuccessfulRemoteDataObject$(relationship));
200202
(relationEffects as any).initialActionMap[identifier] = RelationshipActionTypes.ADD_RELATIONSHIP;
201203
((relationEffects as any).debounceTime as jasmine.Spy).and.returnValue((v) => v);
202204
});
@@ -262,6 +264,8 @@ describe('RelationshipEffects', () => {
262264
let action;
263265
describe('When the last value in the debounceMap is also an REMOVE_RELATIONSHIP action', () => {
264266
beforeEach(() => {
267+
jasmine.getEnv().allowRespy(true);
268+
spyOn((relationEffects as any), 'removeRelationship').and.returnValue(createSuccessfulRemoteDataObject$(undefined));
265269
((relationEffects as any).debounceTime as jasmine.Spy).and.returnValue((v) => v);
266270
(relationEffects as any).initialActionMap[identifier] = RelationshipActionTypes.REMOVE_RELATIONSHIP;
267271
});
@@ -271,7 +275,7 @@ describe('RelationshipEffects', () => {
271275
actions = hot('--a-|', { a: action });
272276
const expected = cold('--b-|', { b: undefined });
273277
expect(relationEffects.mapLastActions$).toBeObservable(expected);
274-
expect((relationEffects as any).removeRelationship).toHaveBeenCalledWith(leftItem, rightItem, relationshipType.leftwardType, '1234');
278+
expect((relationEffects as any).removeRelationship).toHaveBeenCalledWith(leftItem, rightItem, relationshipType.leftwardType);
275279
});
276280
});
277281

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

Lines changed: 77 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
12
import {
23
Inject,
34
Injectable,
@@ -12,13 +13,16 @@ import { TranslateService } from '@ngx-translate/core';
1213
import {
1314
BehaviorSubject,
1415
Observable,
16+
Subject,
1517
} from 'rxjs';
1618
import {
19+
concatMap,
1720
filter,
1821
map,
1922
mergeMap,
2023
switchMap,
2124
take,
25+
tap,
2226
} from 'rxjs/operators';
2327

2428
import { ObjectCacheService } from '../../../../../core/cache/object-cache.service';
@@ -58,11 +62,31 @@ import {
5862

5963
const DEBOUNCE_TIME = 500;
6064

65+
enum RelationOperationType {
66+
Add,
67+
Remove,
68+
}
69+
70+
interface RelationOperation {
71+
type: RelationOperationType
72+
item1: Item
73+
item2: Item
74+
relationshipType: string
75+
submissionId: string
76+
nameVariant?: string
77+
}
78+
6179
/**
6280
* NGRX effects for RelationshipEffects
6381
*/
6482
@Injectable()
6583
export class RelationshipEffects {
84+
85+
/**
86+
* Queue to hold all requests, so we can ensure they get sent one at a time
87+
*/
88+
private requestQueue: Subject<RelationOperation> = new Subject();
89+
6690
/**
6791
* Map that keeps track of the latest RelationshipEffects for each relationship's composed identifier
6892
*/
@@ -104,9 +128,22 @@ export class RelationshipEffects {
104128
nameVariant = this.nameVariantUpdates[identifier];
105129
delete this.nameVariantUpdates[identifier];
106130
}
107-
this.addRelationship(item1, item2, relationshipType, submissionId, nameVariant);
131+
this.requestQueue.next({
132+
type: RelationOperationType.Add,
133+
item1,
134+
item2,
135+
relationshipType,
136+
submissionId,
137+
nameVariant,
138+
});
108139
} else {
109-
this.removeRelationship(item1, item2, relationshipType, submissionId);
140+
this.requestQueue.next({
141+
type: RelationOperationType.Remove,
142+
item1,
143+
item2,
144+
relationshipType,
145+
submissionId,
146+
});
110147
}
111148
}
112149
delete this.debounceMap[identifier];
@@ -183,8 +220,41 @@ export class RelationshipEffects {
183220
private selectableListService: SelectableListService,
184221
@Inject(DEBOUNCE_TIME_OPERATOR) private debounceTime: <T>(dueTime: number) => (source: Observable<T>) => Observable<T>,
185222
) {
223+
this.executeRequestsInQueue();
186224
}
187225

226+
/**
227+
* Subscribe to the request queue, execute the requests inside. Wait for each request to complete
228+
* before sending the next one
229+
* @private
230+
*/
231+
private executeRequestsInQueue() {
232+
this.requestQueue.pipe(
233+
// concatMap ensures the next request in the queue will only start after the previous one has emitted
234+
concatMap((next: RelationOperation) => {
235+
switch (next.type) {
236+
case RelationOperationType.Add:
237+
return this.addRelationship(next.item1, next.item2, next.relationshipType, next.submissionId, next.nameVariant).pipe(
238+
map(() => next),
239+
);
240+
case RelationOperationType.Remove:
241+
return this.removeRelationship(next.item1, next.item2, next.relationshipType).pipe(
242+
map(() => next),
243+
);
244+
default:
245+
return [next];
246+
}
247+
}),
248+
// refresh the workspaceitem after each request. It would be great if we could find a way to
249+
// optimize this so it only happens when the queue is empty.
250+
switchMap((next: RelationOperation) => this.refreshWorkspaceItemInCache(next.submissionId)),
251+
// update the form after the workspaceitem is refreshed
252+
).subscribe((next: SubmissionObject) => {
253+
this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(next.id, [next], false));
254+
});
255+
}
256+
257+
188258
private createIdentifier(item1: Item, item2: Item, relationshipType: string): string {
189259
return `${item1.uuid}-${item2.uuid}-${relationshipType}`;
190260
}
@@ -207,7 +277,7 @@ export class RelationshipEffects {
207277
}
208278
}),
209279
take(1),
210-
switchMap((rd: RemoteData<Relationship>) => {
280+
tap((rd: RemoteData<Relationship>) => {
211281
if (hasNoValue(rd) || rd.hasFailed) {
212282
// An error occurred, deselect the object from the selectable list and display an error notification
213283
const listId = `list-${submissionId}-${relationshipType}`;
@@ -225,19 +295,15 @@ export class RelationshipEffects {
225295
}
226296
this.notificationsService.error(this.translateService.instant('relationships.add.error.title'), errorContent);
227297
}
228-
return this.refreshWorkspaceItemInCache(submissionId);
229298
}),
230-
).subscribe((submissionObject: SubmissionObject) => this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(submissionId, [submissionObject], false)));
299+
);
231300
}
232301

233-
private removeRelationship(item1: Item, item2: Item, relationshipType: string, submissionId: string) {
234-
this.relationshipService.getRelationshipByItemsAndLabel(item1, item2, relationshipType).pipe(
302+
private removeRelationship(item1: Item, item2: Item, relationshipType: string) {
303+
return this.relationshipService.getRelationshipByItemsAndLabel(item1, item2, relationshipType).pipe(
235304
mergeMap((relationship: Relationship) => this.relationshipService.deleteRelationship(relationship.id, 'none')),
236305
take(1),
237-
switchMap(() => this.refreshWorkspaceItemInCache(submissionId)),
238-
).subscribe((submissionObject: SubmissionObject) => {
239-
this.store.dispatch(new SaveSubmissionSectionFormSuccessAction(submissionId, [submissionObject], false));
240-
});
306+
);
241307
}
242308

243309
/**

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) !== true"
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)