Skip to content

Commit cef7cae

Browse files
authored
Merge pull request DSpace#4060 from alexandrevryghem/w2p-127655_fix-submission-form-getting-stuck-in-loop_contribute-main
Fix infinite loading submission forms
2 parents daaa810 + 81fb2e1 commit cef7cae

7 files changed

Lines changed: 75 additions & 29 deletions

File tree

src/app/core/submission/submission-rest.service.spec.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { getTestScheduler } from 'jasmine-marbles';
2+
import { of } from 'rxjs';
23
import { TestScheduler } from 'rxjs/testing';
34

45
import { FormFieldMetadataValueObject } from '../../shared/form/builder/models/form-field-metadata-value.model';
@@ -13,6 +14,7 @@ import {
1314
SubmissionRequest,
1415
} from '../data/request.models';
1516
import { RequestService } from '../data/request.service';
17+
import { RequestEntry } from '../data/request-entry.model';
1618
import { SubmissionRestService } from './submission-rest.service';
1719

1820
describe('SubmissionRestService test suite', () => {
@@ -38,7 +40,9 @@ describe('SubmissionRestService test suite', () => {
3840
}
3941

4042
beforeEach(() => {
41-
requestService = getMockRequestService();
43+
requestService = getMockRequestService(of(Object.assign(new RequestEntry(), {
44+
request: new SubmissionRequest('mock-request-uuid', 'mock-request-href'),
45+
})));
4246
rdbService = getMockRemoteDataBuildService();
4347
scheduler = getTestScheduler();
4448
halService = new HALEndpointServiceStub(resourceEndpointURL);
@@ -62,7 +66,7 @@ describe('SubmissionRestService test suite', () => {
6266
scheduler.schedule(() => service.getDataById(resourceEndpoint, resourceScope).subscribe());
6367
scheduler.flush();
6468

65-
expect(requestService.send).toHaveBeenCalledWith(expected);
69+
expect(requestService.send).toHaveBeenCalledWith(expected, false);
6670
});
6771
});
6872

src/app/core/submission/submission-rest.service.ts

Lines changed: 62 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
import { Injectable } from '@angular/core';
2-
import { Observable } from 'rxjs';
2+
import {
3+
Observable,
4+
skipWhile,
5+
} from 'rxjs';
36
import {
47
distinctUntilChanged,
58
filter,
69
map,
710
mergeMap,
11+
switchMap,
812
tap,
913
} from 'rxjs/operators';
1014

1115
import {
1216
hasValue,
17+
hasValueOperator,
1318
isNotEmpty,
1419
} from '../../shared/empty.util';
1520
import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service';
@@ -25,14 +30,30 @@ import {
2530
} from '../data/request.models';
2631
import { RequestService } from '../data/request.service';
2732
import { RequestError } from '../data/request-error.model';
28-
import { RestRequest } from '../data/rest-request.model';
2933
import { HttpOptions } from '../dspace-rest/dspace-rest.service';
3034
import { HALEndpointService } from '../shared/hal-endpoint.service';
3135
import { getFirstCompletedRemoteData } from '../shared/operators';
3236
import { SubmitDataResponseDefinitionObject } from '../shared/submit-data-response-definition.model';
3337
import { URLCombiner } from '../url-combiner/url-combiner';
3438
import { SubmissionResponse } from './submission-response.model';
3539

40+
/**
41+
* Retrieve the first emitting payload's dataDefinition, or throw an error if the request failed
42+
*/
43+
export const getFirstDataDefinition = () =>
44+
(source: Observable<RemoteData<SubmissionResponse>>): Observable<SubmitDataResponseDefinitionObject> =>
45+
source.pipe(
46+
getFirstCompletedRemoteData(),
47+
map((response: RemoteData<SubmissionResponse>) => {
48+
if (response.hasFailed) {
49+
throw new ErrorResponse({ statusText: response.errorMessage, statusCode: response.statusCode } as RequestError);
50+
} else {
51+
return hasValue(response?.payload?.dataDefinition) ? response.payload.dataDefinition : [response.payload];
52+
}
53+
}),
54+
distinctUntilChanged(),
55+
);
56+
3657
/**
3758
* The service handling all submission REST requests
3859
*/
@@ -56,15 +77,7 @@ export class SubmissionRestService {
5677
*/
5778
protected fetchRequest(requestId: string): Observable<SubmitDataResponseDefinitionObject> {
5879
return this.rdbService.buildFromRequestUUID<SubmissionResponse>(requestId).pipe(
59-
getFirstCompletedRemoteData(),
60-
map((response: RemoteData<SubmissionResponse>) => {
61-
if (response.hasFailed) {
62-
throw new ErrorResponse({ statusText: response.errorMessage, statusCode: response.statusCode } as RequestError);
63-
} else {
64-
return hasValue(response.payload) ? response.payload.dataDefinition : response.payload;
65-
}
66-
}),
67-
distinctUntilChanged(),
80+
getFirstDataDefinition(),
6881
);
6982
}
7083

@@ -116,21 +129,52 @@ export class SubmissionRestService {
116129
* The endpoint link name
117130
* @param id
118131
* The submission Object to retrieve
132+
* @param useCachedVersionIfAvailable
133+
* If this is true, the request will only be sent if there's no valid & cached version. Defaults to false
119134
* @return Observable<SubmitDataResponseDefinitionObject>
120135
* server response
121136
*/
122-
public getDataById(linkName: string, id: string): Observable<SubmitDataResponseDefinitionObject> {
123-
const requestId = this.requestService.generateRequestId();
137+
public getDataById(linkName: string, id: string, useCachedVersionIfAvailable = false): Observable<SubmitDataResponseDefinitionObject> {
124138
return this.halService.getEndpoint(linkName).pipe(
125139
map((endpointURL: string) => this.getEndpointByIDHref(endpointURL, id)),
126140
filter((href: string) => isNotEmpty(href)),
127141
distinctUntilChanged(),
128-
map((endpointURL: string) => new SubmissionRequest(requestId, endpointURL)),
129-
tap((request: RestRequest) => {
130-
this.requestService.send(request);
142+
mergeMap((endpointURL: string) => {
143+
this.sendGetDataRequest(endpointURL, useCachedVersionIfAvailable);
144+
const startTime: number = new Date().getTime();
145+
return this.requestService.getByHref(endpointURL).pipe(
146+
map((requestEntry) => requestEntry?.request?.uuid),
147+
hasValueOperator(),
148+
distinctUntilChanged(),
149+
switchMap((requestId) => this.rdbService.buildFromRequestUUID<SubmissionResponse>(requestId)),
150+
// This skip ensures that if a stale object is present in the cache when you do a
151+
// call it isn't immediately returned, but we wait until the remote data for the new request
152+
// is created. If useCachedVersionIfAvailable is false it also ensures you don't get a
153+
// cached completed object
154+
skipWhile((rd: RemoteData<SubmissionResponse>) => rd.isStale || (!useCachedVersionIfAvailable && rd.lastUpdated < startTime)),
155+
tap((rd: RemoteData<SubmissionResponse>) => {
156+
if (hasValue(rd) && rd.isStale) {
157+
this.sendGetDataRequest(endpointURL, useCachedVersionIfAvailable);
158+
}
159+
}),
160+
);
131161
}),
132-
mergeMap(() => this.fetchRequest(requestId)),
133-
distinctUntilChanged());
162+
getFirstDataDefinition(),
163+
);
164+
}
165+
166+
/**
167+
* Send a GET SubmissionRequest
168+
*
169+
* @param href
170+
* Endpoint URL of the submission data
171+
* @param useCachedVersionIfAvailable
172+
* If this is true, the request will only be sent if there's no valid & cached version. Defaults to false
173+
*/
174+
private sendGetDataRequest(href: string, useCachedVersionIfAvailable = false) {
175+
const requestId = this.requestService.generateRequestId();
176+
const request = new SubmissionRequest(requestId, href);
177+
this.requestService.send(request, useCachedVersionIfAvailable);
134178
}
135179

136180
/**

src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ import {
9696
getRemoteDataPayload,
9797
} from '../../../../core/shared/operators';
9898
import { SubmissionObject } from '../../../../core/submission/models/submission-object.model';
99+
import { SUBMISSION_LINKS_TO_FOLLOW } from '../../../../core/submission/resolver/submission-links-to-follow';
99100
import { SubmissionObjectDataService } from '../../../../core/submission/submission-object-data.service';
100101
import { paginatedRelationsToItems } from '../../../../item-page/simple/item-types/shared/item-relationships-utils';
101102
import { SubmissionService } from '../../../../submission/submission.service';
@@ -450,7 +451,7 @@ export class DsDynamicFormControlContainerComponent extends DynamicFormControlCo
450451
*/
451452
private setItem() {
452453
const submissionObject$ = this.submissionObjectService
453-
.findById(this.model.submissionId, true, true, followLink('item'), followLink('collection')).pipe(
454+
.findById(this.model.submissionId, true, true, ...SUBMISSION_LINKS_TO_FOLLOW).pipe(
454455
getAllSucceededRemoteData(),
455456
getRemoteDataPayload(),
456457
);

src/app/submission/form/submission-form.component.spec.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,6 @@ describe('SubmissionFormComponent', () => {
226226
});
227227
scheduler.flush();
228228

229-
expect(comp.collectionId).toEqual(submissionObjectNew.collection.id);
230229
expect(comp.submissionDefinition).toEqual(submissionObjectNew.submissionDefinition);
231230
expect(comp.definitionId).toEqual(submissionObjectNew.submissionDefinition.name);
232231
expect(comp.sections).toEqual(submissionObjectNew.sections);
@@ -264,7 +263,6 @@ describe('SubmissionFormComponent', () => {
264263
});
265264
scheduler.flush();
266265

267-
expect(comp.collectionId).toEqual('45f2f3f1-ba1f-4f36-908a-3f1ea9a557eb');
268266
expect(submissionServiceStub.resetSubmissionObject).not.toHaveBeenCalled();
269267
done();
270268
});

src/app/submission/form/submission-form.component.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,13 +288,12 @@ export class SubmissionFormComponent implements OnChanges, OnDestroy {
288288
* new submission object
289289
*/
290290
onCollectionChange(submissionObject: SubmissionObject) {
291-
this.collectionId = (submissionObject.collection as Collection).id;
292291
if (this.definitionId !== (submissionObject.submissionDefinition as SubmissionDefinitionsModel).name) {
293292
this.sections = submissionObject.sections;
294293
this.submissionDefinition = (submissionObject.submissionDefinition as SubmissionDefinitionsModel);
295294
this.definitionId = this.submissionDefinition.name;
296295
this.submissionService.resetSubmissionObject(
297-
this.collectionId,
296+
(submissionObject.collection as Collection).id,
298297
this.submissionId,
299298
submissionObject._links.self.href,
300299
this.submissionDefinition,

src/app/workflowitems-edit-page/workflow-item-page.resolver.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import { Observable } from 'rxjs';
99
import { RemoteData } from '../core/data/remote-data';
1010
import { getFirstCompletedRemoteData } from '../core/shared/operators';
1111
import { WorkflowItem } from '../core/submission/models/workflowitem.model';
12+
import { SUBMISSION_LINKS_TO_FOLLOW } from '../core/submission/resolver/submission-links-to-follow';
1213
import { WorkflowItemDataService } from '../core/submission/workflowitem-data.service';
13-
import { followLink } from '../shared/utils/follow-link-config.model';
1414

1515
export const workflowItemPageResolver: ResolveFn<RemoteData<WorkflowItem>> = (
1616
route: ActivatedRouteSnapshot,
@@ -21,7 +21,7 @@ export const workflowItemPageResolver: ResolveFn<RemoteData<WorkflowItem>> = (
2121
route.params.id,
2222
true,
2323
false,
24-
followLink('item'),
24+
...SUBMISSION_LINKS_TO_FOLLOW,
2525
).pipe(
2626
getFirstCompletedRemoteData(),
2727
);

src/app/workspaceitems-edit-page/workspace-item-page.resolver.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import { Observable } from 'rxjs';
99
import { RemoteData } from '../core/data/remote-data';
1010
import { getFirstCompletedRemoteData } from '../core/shared/operators';
1111
import { WorkspaceItem } from '../core/submission/models/workspaceitem.model';
12+
import { SUBMISSION_LINKS_TO_FOLLOW } from '../core/submission/resolver/submission-links-to-follow';
1213
import { WorkspaceitemDataService } from '../core/submission/workspaceitem-data.service';
13-
import { followLink } from '../shared/utils/follow-link-config.model';
1414

1515
/**
1616
* Method for resolving a workflow item based on the parameters in the current route
@@ -28,7 +28,7 @@ export const workspaceItemPageResolver: ResolveFn<RemoteData<WorkspaceItem>> = (
2828
return workspaceItemService.findById(route.params.id,
2929
true,
3030
false,
31-
followLink('item'),
31+
...SUBMISSION_LINKS_TO_FOLLOW,
3232
).pipe(
3333
getFirstCompletedRemoteData(),
3434
);

0 commit comments

Comments
 (0)