From 287d35cb2639f8915c12773ac1c73c2051b52ded Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Mon, 28 Apr 2025 13:04:44 +0200 Subject: [PATCH 1/5] 127655: Fix submission infinite loading --- .../submission/submission-rest.service.ts | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/app/core/submission/submission-rest.service.ts b/src/app/core/submission/submission-rest.service.ts index cf4b741af2..7676bfb141 100644 --- a/src/app/core/submission/submission-rest.service.ts +++ b/src/app/core/submission/submission-rest.service.ts @@ -23,6 +23,23 @@ import { RemoteData } from '../data/remote-data'; import { SubmissionResponse } from './submission-response.model'; import { RestRequest } from '../data/rest-request.model'; +/** + * Retrieve the first emitting payload's dataDefinition, or throw an error if the request failed + */ +export const getFirstDataDefinition = () => + (source: Observable>): Observable => + source.pipe( + getFirstCompletedRemoteData(), + map((response: RemoteData) => { + if (response.hasFailed) { + throw new Error(response.errorMessage); + } else { + return hasValue(response.payload) ? response.payload.dataDefinition : response.payload; + } + }), + distinctUntilChanged(), + ); + /** * The service handling all submission REST requests */ @@ -46,15 +63,7 @@ export class SubmissionRestService { */ protected fetchRequest(requestId: string): Observable { return this.rdbService.buildFromRequestUUID(requestId).pipe( - getFirstCompletedRemoteData(), - map((response: RemoteData) => { - if (response.hasFailed) { - throw new Error(response.errorMessage); - } else { - return hasValue(response.payload) ? response.payload.dataDefinition : response.payload; - } - }), - distinctUntilChanged() + getFirstDataDefinition(), ); } @@ -119,8 +128,9 @@ export class SubmissionRestService { tap((request: RestRequest) => { this.requestService.send(request); }), - mergeMap(() => this.fetchRequest(requestId)), - distinctUntilChanged()); + mergeMap((request) => this.rdbService.buildSingle(request.href)), + getFirstDataDefinition(), + ); } /** From 99e8c1044caa39f3ce85c3d9db2046f2ea286b01 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Mon, 28 Apr 2025 15:15:26 +0200 Subject: [PATCH 2/5] 127655: Submission get data stale re-request --- .../submission-rest.service.spec.ts | 2 +- .../submission/submission-rest.service.ts | 43 ++++++++++++++++--- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/app/core/submission/submission-rest.service.spec.ts b/src/app/core/submission/submission-rest.service.spec.ts index 46e536ff7d..cfa3c34322 100644 --- a/src/app/core/submission/submission-rest.service.spec.ts +++ b/src/app/core/submission/submission-rest.service.spec.ts @@ -62,7 +62,7 @@ describe('SubmissionRestService test suite', () => { scheduler.schedule(() => service.getDataById(resourceEndpoint, resourceScope).subscribe()); scheduler.flush(); - expect(requestService.send).toHaveBeenCalledWith(expected); + expect(requestService.send).toHaveBeenCalledWith(expected, false); }); }); diff --git a/src/app/core/submission/submission-rest.service.ts b/src/app/core/submission/submission-rest.service.ts index 7676bfb141..d9690fea50 100644 --- a/src/app/core/submission/submission-rest.service.ts +++ b/src/app/core/submission/submission-rest.service.ts @@ -1,6 +1,6 @@ import { Injectable } from '@angular/core'; -import { Observable } from 'rxjs'; +import { Observable, skipWhile } from 'rxjs'; import { distinctUntilChanged, filter, map, mergeMap, tap } from 'rxjs/operators'; import { RequestService } from '../data/request.service'; @@ -115,24 +115,53 @@ export class SubmissionRestService { * The endpoint link name * @param id * The submission Object to retrieve + * @param useCachedVersionIfAvailable + * If this is true, the request will only be sent if there's no valid & cached version. Defaults to false * @return Observable * server response */ - public getDataById(linkName: string, id: string): Observable { - const requestId = this.requestService.generateRequestId(); + public getDataById(linkName: string, id: string, useCachedVersionIfAvailable = false): Observable { return this.halService.getEndpoint(linkName).pipe( map((endpointURL: string) => this.getEndpointByIDHref(endpointURL, id)), filter((href: string) => isNotEmpty(href)), distinctUntilChanged(), - map((endpointURL: string) => new SubmissionRequest(requestId, endpointURL)), - tap((request: RestRequest) => { - this.requestService.send(request); + mergeMap((endpointURL: string) => { + const request = this.sendGetDataRequest(endpointURL, useCachedVersionIfAvailable); + const startTime: number = new Date().getTime(); + return this.rdbService.buildSingle(request.href).pipe( + // This skip ensures that if a stale object is present in the cache when you do a + // call it isn't immediately returned, but we wait until the remote data for the new request + // is created. If useCachedVersionIfAvailable is false it also ensures you don't get a + // cached completed object + skipWhile((rd: RemoteData) => rd.isStale || (!useCachedVersionIfAvailable && rd.lastUpdated < startTime)), + tap((rd: RemoteData) => { + if (hasValue(rd) && rd.isStale) { + this.sendGetDataRequest(endpointURL, useCachedVersionIfAvailable); + } + }) + ); }), - mergeMap((request) => this.rdbService.buildSingle(request.href)), getFirstDataDefinition(), ); } + /** + * Send a GET SubmissionRequest + * + * @param href + * Endpoint URL of the submission data + * @param useCachedVersionIfAvailable + * If this is true, the request will only be sent if there's no valid & cached version. Defaults to false + * @return RestRequest + * Request sent + */ + private sendGetDataRequest(href: string, useCachedVersionIfAvailable = false): RestRequest { + const requestId = this.requestService.generateRequestId(); + const request = new SubmissionRequest(requestId, href); + this.requestService.send(request, useCachedVersionIfAvailable); + return request; + } + /** * Make a new post request * From 0dabc8ed8f4c13217913fbbeedeaee8191d74983 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Mon, 28 Apr 2025 17:47:17 +0200 Subject: [PATCH 3/5] 127655: refactor to use buildFromRequestUUID --- .../submission/submission-rest.service.spec.ts | 6 +++++- .../core/submission/submission-rest.service.ts | 15 +++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/app/core/submission/submission-rest.service.spec.ts b/src/app/core/submission/submission-rest.service.spec.ts index cfa3c34322..fdfc7da0af 100644 --- a/src/app/core/submission/submission-rest.service.spec.ts +++ b/src/app/core/submission/submission-rest.service.spec.ts @@ -14,6 +14,8 @@ import { SubmissionRequest } from '../data/request.models'; import { FormFieldMetadataValueObject } from '../../shared/form/builder/models/form-field-metadata-value.model'; +import { of } from 'rxjs'; +import { RequestEntry } from '../data/request-entry.model'; describe('SubmissionRestService test suite', () => { let scheduler: TestScheduler; @@ -38,7 +40,9 @@ describe('SubmissionRestService test suite', () => { } beforeEach(() => { - requestService = getMockRequestService(); + requestService = getMockRequestService(of(Object.assign(new RequestEntry(), { + request: new SubmissionRequest('mock-request-uuid', 'mock-request-href'), + }))); rdbService = getMockRemoteDataBuildService(); scheduler = getTestScheduler(); halService = new HALEndpointServiceStub(resourceEndpointURL); diff --git a/src/app/core/submission/submission-rest.service.ts b/src/app/core/submission/submission-rest.service.ts index d9690fea50..c64d04d1c2 100644 --- a/src/app/core/submission/submission-rest.service.ts +++ b/src/app/core/submission/submission-rest.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@angular/core'; import { Observable, skipWhile } from 'rxjs'; -import { distinctUntilChanged, filter, map, mergeMap, tap } from 'rxjs/operators'; +import { distinctUntilChanged, filter, map, mergeMap, switchMap, tap } from 'rxjs/operators'; import { RequestService } from '../data/request.service'; import { hasValue, isNotEmpty } from '../../shared/empty.util'; @@ -21,7 +21,6 @@ import { getFirstCompletedRemoteData } from '../shared/operators'; import { URLCombiner } from '../url-combiner/url-combiner'; import { RemoteData } from '../data/remote-data'; import { SubmissionResponse } from './submission-response.model'; -import { RestRequest } from '../data/rest-request.model'; /** * Retrieve the first emitting payload's dataDefinition, or throw an error if the request failed @@ -126,9 +125,12 @@ export class SubmissionRestService { filter((href: string) => isNotEmpty(href)), distinctUntilChanged(), mergeMap((endpointURL: string) => { - const request = this.sendGetDataRequest(endpointURL, useCachedVersionIfAvailable); + this.sendGetDataRequest(endpointURL, useCachedVersionIfAvailable); const startTime: number = new Date().getTime(); - return this.rdbService.buildSingle(request.href).pipe( + return this.requestService.getByHref(endpointURL).pipe( + map((requestEntry) => requestEntry.request.uuid), + distinctUntilChanged(), + switchMap((requestId) => this.rdbService.buildFromRequestUUID(requestId)), // This skip ensures that if a stale object is present in the cache when you do a // call it isn't immediately returned, but we wait until the remote data for the new request // is created. If useCachedVersionIfAvailable is false it also ensures you don't get a @@ -152,14 +154,11 @@ export class SubmissionRestService { * Endpoint URL of the submission data * @param useCachedVersionIfAvailable * If this is true, the request will only be sent if there's no valid & cached version. Defaults to false - * @return RestRequest - * Request sent */ - private sendGetDataRequest(href: string, useCachedVersionIfAvailable = false): RestRequest { + private sendGetDataRequest(href: string, useCachedVersionIfAvailable = false) { const requestId = this.requestService.generateRequestId(); const request = new SubmissionRequest(requestId, href); this.requestService.send(request, useCachedVersionIfAvailable); - return request; } /** From c1bd65e8c65f47b423dcfa2d739fe140e6152d0f Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 29 Apr 2025 10:46:45 +0200 Subject: [PATCH 4/5] 127655: avoid nullpointer --- src/app/core/submission/submission-rest.service.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/app/core/submission/submission-rest.service.ts b/src/app/core/submission/submission-rest.service.ts index c64d04d1c2..d83a07387a 100644 --- a/src/app/core/submission/submission-rest.service.ts +++ b/src/app/core/submission/submission-rest.service.ts @@ -4,7 +4,7 @@ import { Observable, skipWhile } from 'rxjs'; import { distinctUntilChanged, filter, map, mergeMap, switchMap, tap } from 'rxjs/operators'; import { RequestService } from '../data/request.service'; -import { hasValue, isNotEmpty } from '../../shared/empty.util'; +import { hasValue, hasValueOperator, isNotEmpty } from '../../shared/empty.util'; import { DeleteRequest, PostRequest, @@ -128,7 +128,8 @@ export class SubmissionRestService { this.sendGetDataRequest(endpointURL, useCachedVersionIfAvailable); const startTime: number = new Date().getTime(); return this.requestService.getByHref(endpointURL).pipe( - map((requestEntry) => requestEntry.request.uuid), + map((requestEntry) => requestEntry?.request?.uuid), + hasValueOperator(), distinctUntilChanged(), switchMap((requestId) => this.rdbService.buildFromRequestUUID(requestId)), // This skip ensures that if a stale object is present in the cache when you do a From 3d32715d252d3b199561ab8cc5a6f2515dd9472d Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Tue, 6 May 2025 13:37:52 +0200 Subject: [PATCH 5/5] 127655: Fixed getFirstDataDefinition not always returning a correct SubmitDataResponseDefinitionObject, leading to an infinite loading screen - Also fixed an issue where the collection switcher could accidentally show the old collection name instead of the new one - Also updated the WorkspaceItemPageResolver & WorkflowItemPageResolver to embed the collection to use fewer requests --- src/app/core/submission/submission-rest.service.ts | 2 +- src/app/submission/form/submission-form.component.spec.ts | 4 +--- src/app/submission/form/submission-form.component.ts | 3 +-- .../workflowitems-edit-page/workflow-item-page.resolver.ts | 1 + .../workspace-item-page.resolver.ts | 7 ++++--- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/app/core/submission/submission-rest.service.ts b/src/app/core/submission/submission-rest.service.ts index d83a07387a..1daf4fcae9 100644 --- a/src/app/core/submission/submission-rest.service.ts +++ b/src/app/core/submission/submission-rest.service.ts @@ -33,7 +33,7 @@ export const getFirstDataDefinition = () => if (response.hasFailed) { throw new Error(response.errorMessage); } else { - return hasValue(response.payload) ? response.payload.dataDefinition : response.payload; + return hasValue(response?.payload?.dataDefinition) ? response.payload.dataDefinition : [response.payload]; } }), distinctUntilChanged(), diff --git a/src/app/submission/form/submission-form.component.spec.ts b/src/app/submission/form/submission-form.component.spec.ts index cc77c44afb..d4499d87ae 100644 --- a/src/app/submission/form/submission-form.component.spec.ts +++ b/src/app/submission/form/submission-form.component.spec.ts @@ -27,7 +27,7 @@ import { TestScheduler } from 'rxjs/testing'; import { SectionsService } from '../sections/sections.service'; import { VisibilityType } from '../sections/visibility-type'; -describe('SubmissionFormComponent Component', () => { +describe('SubmissionFormComponent', () => { let comp: SubmissionFormComponent; let compAsAny: any; @@ -197,7 +197,6 @@ describe('SubmissionFormComponent Component', () => { }); scheduler.flush(); - expect(comp.collectionId).toEqual(submissionObjectNew.collection.id); expect(comp.submissionDefinition).toEqual(submissionObjectNew.submissionDefinition); expect(comp.definitionId).toEqual(submissionObjectNew.submissionDefinition.name); expect(comp.sections).toEqual(submissionObjectNew.sections); @@ -235,7 +234,6 @@ describe('SubmissionFormComponent Component', () => { }); scheduler.flush(); - expect(comp.collectionId).toEqual('45f2f3f1-ba1f-4f36-908a-3f1ea9a557eb'); expect(submissionServiceStub.resetSubmissionObject).not.toHaveBeenCalled(); done(); }); diff --git a/src/app/submission/form/submission-form.component.ts b/src/app/submission/form/submission-form.component.ts index 216aefcfc3..0eb9ad2037 100644 --- a/src/app/submission/form/submission-form.component.ts +++ b/src/app/submission/form/submission-form.component.ts @@ -249,13 +249,12 @@ export class SubmissionFormComponent implements OnChanges, OnDestroy { * new submission object */ onCollectionChange(submissionObject: SubmissionObject) { - this.collectionId = (submissionObject.collection as Collection).id; if (this.definitionId !== (submissionObject.submissionDefinition as SubmissionDefinitionsModel).name) { this.sections = submissionObject.sections; this.submissionDefinition = (submissionObject.submissionDefinition as SubmissionDefinitionsModel); this.definitionId = this.submissionDefinition.name; this.submissionService.resetSubmissionObject( - this.collectionId, + (submissionObject.collection as Collection).id, this.submissionId, submissionObject._links.self.href, this.submissionDefinition, diff --git a/src/app/workflowitems-edit-page/workflow-item-page.resolver.ts b/src/app/workflowitems-edit-page/workflow-item-page.resolver.ts index 4bb3eac513..c5d6ee9520 100644 --- a/src/app/workflowitems-edit-page/workflow-item-page.resolver.ts +++ b/src/app/workflowitems-edit-page/workflow-item-page.resolver.ts @@ -27,6 +27,7 @@ export class WorkflowItemPageResolver implements Resolve> { +export class WorkspaceItemPageResolver implements Resolve> { constructor(private workspaceItemService: WorkspaceitemDataService) { } @@ -22,11 +22,12 @@ export class WorkspaceItemPageResolver implements Resolve> Emits the found workflow item based on the parameters in the current route, * or an error if something went wrong */ - resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable> { + resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable> { return this.workspaceItemService.findById(route.params.id, true, false, followLink('item'), + followLink('collection'), ).pipe( getFirstCompletedRemoteData(), );