diff --git a/src/app/core/submission/submission-response-parsing.service.ts b/src/app/core/submission/submission-response-parsing.service.ts index a87f788c1d..b7517a8248 100644 --- a/src/app/core/submission/submission-response-parsing.service.ts +++ b/src/app/core/submission/submission-response-parsing.service.ts @@ -5,7 +5,6 @@ import { RestRequest } from '../data/request.models'; import { DSpaceRESTV2Response } from '../dspace-rest-v2/dspace-rest-v2-response.model'; import { ErrorResponse, RestResponse, SubmissionSuccessResponse } from '../cache/response-cache.models'; import { isEmpty, isNotEmpty, isNotNull } from '../../shared/empty.util'; - import { ConfigObject } from '../config/models/config.model'; import { BaseResponseParsingService } from '../data/base-response-parsing.service'; import { GLOBAL_CONFIG } from '../../../config'; @@ -18,9 +17,6 @@ import { NormalizedWorkspaceItem } from './models/normalized-workspaceitem.model import { normalizeSectionData } from './models/workspaceitem-sections.model'; import { NormalizedWorkflowItem } from './models/normalized-workflowitem.model'; import { NormalizedEditItem } from './models/normalized-edititem.model'; -import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; -import { NormalizedSubmissionObject } from './models/normalized-submission-object.model'; -import { DSpaceRESTv2Serializer } from '../dspace-rest-v2/dspace-rest-v2.serializer'; @Injectable() export class SubmissionResponseParsingService extends BaseResponseParsingService implements ResponseParsingService { @@ -57,7 +53,7 @@ export class SubmissionResponseParsingService extends BaseResponseParsingService const normalizedDefinition = Array.of(); const processedList = Array.isArray(dataDefinition) ? dataDefinition : Array.of(dataDefinition); - processedList.forEach((item, index) => { + processedList.forEach((item) => { let normalizedItem = Object.assign({}, item); // In case data is an Instance of NormalizedWorkspaceItem normalize field value of all the section of type form @@ -92,10 +88,10 @@ export class SubmissionResponseParsingService extends BaseResponseParsingService precessedSection[sectionId] = normalizedSectionData; } }); - normalizedItem = Object.assign({}, item, {sections: precessedSection}); + normalizedItem = Object.assign({}, item, { sections: precessedSection }); } } - normalizedDefinition.push( normalizedItem); + normalizedDefinition.push(normalizedItem); }); return normalizedDefinition; diff --git a/src/app/submission/edit/submission-edit.component.spec.ts b/src/app/submission/edit/submission-edit.component.spec.ts index d64193501d..5c9a247aa2 100644 --- a/src/app/submission/edit/submission-edit.component.spec.ts +++ b/src/app/submission/edit/submission-edit.component.spec.ts @@ -16,6 +16,7 @@ import { getMockTranslateService } from '../../shared/mocks/mock-translate.servi import { RouterStub } from '../../shared/testing/router-stub'; import { ActivatedRouteStub } from '../../shared/testing/active-router-stub'; import { mockSubmissionObject } from '../../shared/mocks/mock-submission'; +import { RemoteData } from '../../core/data/remote-data'; describe('SubmissionEditComponent Component', () => { @@ -65,7 +66,14 @@ describe('SubmissionEditComponent Component', () => { it('should init properly when a valid SubmissionObject has been retrieved', fakeAsync(() => { route.testParams = { id: submissionId }; - submissionServiceStub.retrieveSubmission.and.returnValue(observableOf(submissionObject)); + submissionServiceStub.retrieveSubmission.and.returnValue(observableOf( + new RemoteData( + false, + false, + true, + null, + submissionObject) + )); fixture.detectChanges(); @@ -80,7 +88,14 @@ describe('SubmissionEditComponent Component', () => { it('should redirect to mydspace when an empty SubmissionObject has been retrieved', fakeAsync(() => { route.testParams = { id: submissionId }; - submissionServiceStub.retrieveSubmission.and.returnValue(observableOf({})); + submissionServiceStub.retrieveSubmission.and.returnValue(observableOf( + new RemoteData( + false, + false, + true, + null, + {}) + )); fixture.detectChanges(); diff --git a/src/app/submission/edit/submission-edit.component.ts b/src/app/submission/edit/submission-edit.component.ts index db8408159e..d128191d79 100644 --- a/src/app/submission/edit/submission-edit.component.ts +++ b/src/app/submission/edit/submission-edit.component.ts @@ -2,7 +2,7 @@ import { ChangeDetectorRef, Component, OnDestroy, OnInit } from '@angular/core'; import { ActivatedRoute, ParamMap, Router } from '@angular/router'; import { Subscription } from 'rxjs'; -import { flatMap, tap } from 'rxjs/operators'; +import { filter, switchMap } from 'rxjs/operators'; import { TranslateService } from '@ngx-translate/core'; import { WorkspaceitemSectionsObject } from '../../core/submission/models/workspaceitem-sections.model'; @@ -12,6 +12,7 @@ import { SubmissionService } from '../submission.service'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { SubmissionObject } from '../../core/submission/models/submission-object.model'; import { Collection } from '../../core/shared/collection.model'; +import { RemoteData } from '../../core/data/remote-data'; @Component({ selector: 'ds-submission-edit', @@ -42,21 +43,28 @@ export class SubmissionEditComponent implements OnDestroy, OnInit { ngOnInit() { this.subs.push(this.route.paramMap.pipe( - flatMap((params: ParamMap) => this.submissionService.retrieveSubmission(params.get('id'))) - ).subscribe((submissionObject: SubmissionObject) => { - // NOTE new submission is retrieved on the browser side only - if (isNotNull(submissionObject)) { - if (isEmpty(submissionObject)) { + switchMap((params: ParamMap) => this.submissionService.retrieveSubmission(params.get('id'))), + // NOTE new submission is retrieved on the browser side only, so get null on server side rendering + filter((submissionObjectRD: RemoteData) => isNotNull(submissionObjectRD)) + ).subscribe((submissionObjectRD: RemoteData) => { + if (submissionObjectRD.hasSucceeded) { + if (isEmpty(submissionObjectRD.payload)) { this.notificationsService.info(null, this.translate.get('submission.general.cannot_submit')); this.router.navigate(['/mydspace']); } else { - this.submissionId = submissionObject.id; - this.collectionId = (submissionObject.collection as Collection).id; - this.selfUrl = submissionObject.self; - this.sections = submissionObject.sections; - this.submissionDefinition = (submissionObject.submissionDefinition as SubmissionDefinitionsModel); + this.submissionId = submissionObjectRD.payload.id.toString(); + this.collectionId = (submissionObjectRD.payload.collection as Collection).id; + this.selfUrl = submissionObjectRD.payload.self; + this.sections = submissionObjectRD.payload.sections; + this.submissionDefinition = (submissionObjectRD.payload.submissionDefinition as SubmissionDefinitionsModel); this.changeDetectorRef.detectChanges(); } + } else { + if (submissionObjectRD.error.statusCode === 404) { + // redirect to not found page + this.router.navigate(['/404'], { skipLocationChange: true }); + } + // TODO handle generic error } })); } diff --git a/src/app/submission/objects/submission-objects.reducer.ts b/src/app/submission/objects/submission-objects.reducer.ts index e538a1a789..c02b2fd0f1 100644 --- a/src/app/submission/objects/submission-objects.reducer.ts +++ b/src/app/submission/objects/submission-objects.reducer.ts @@ -1,38 +1,38 @@ import { hasValue, isNotEmpty, isNotNull, isUndefined } from '../../shared/empty.util'; -import { findKey, uniqWith, isEqual, differenceWith } from 'lodash'; +import { differenceWith, findKey, isEqual, uniqWith } from 'lodash'; import { + ChangeSubmissionCollectionAction, CompleteInitSubmissionFormAction, + DeleteSectionErrorsAction, DeleteUploadedFileAction, + DepositSubmissionAction, + DepositSubmissionErrorAction, + DepositSubmissionSuccessAction, DisableSectionAction, EditFileDataAction, EnableSectionAction, - NewUploadedFileAction, + InertSectionErrorsAction, + InitSectionAction, InitSubmissionFormAction, + NewUploadedFileAction, + RemoveSectionErrorsAction, + ResetSubmissionFormAction, + SaveAndDepositSubmissionAction, + SaveForLaterSubmissionFormAction, + SaveForLaterSubmissionFormErrorAction, + SaveForLaterSubmissionFormSuccessAction, + SaveSubmissionFormAction, + SaveSubmissionFormErrorAction, + SaveSubmissionFormSuccessAction, + SaveSubmissionSectionFormAction, + SaveSubmissionSectionFormErrorAction, + SaveSubmissionSectionFormSuccessAction, SectionStatusChangeAction, + SetActiveSectionAction, SubmissionObjectAction, SubmissionObjectActionTypes, - InertSectionErrorsAction, - DeleteSectionErrorsAction, - ResetSubmissionFormAction, - UpdateSectionDataAction, - SaveSubmissionFormAction, - SetActiveSectionAction, - SaveSubmissionSectionFormAction, - DepositSubmissionAction, - DepositSubmissionSuccessAction, - DepositSubmissionErrorAction, - ChangeSubmissionCollectionAction, - SaveSubmissionFormSuccessAction, - SaveSubmissionFormErrorAction, - SaveSubmissionSectionFormSuccessAction, - SaveSubmissionSectionFormErrorAction, - InitSectionAction, - RemoveSectionErrorsAction, - SaveForLaterSubmissionFormAction, - SaveAndDepositSubmissionAction, - SaveForLaterSubmissionFormSuccessAction, - SaveForLaterSubmissionFormErrorAction + UpdateSectionDataAction } from './submission-objects.actions'; import { WorkspaceitemSectionDataType } from '../../core/submission/models/workspaceitem-sections.model'; import { WorkspaceitemSectionUploadObject } from '../../core/submission/models/workspaceitem-section-upload.model'; @@ -117,7 +117,6 @@ export function submissionObjectReducer(state = initialState, action: Submission } case SubmissionObjectActionTypes.SAVE_SUBMISSION_FORM_SUCCESS: - case SubmissionObjectActionTypes.SAVE_FOR_LATER_SUBMISSION_FORM_SUCCESS: case SubmissionObjectActionTypes.SAVE_SUBMISSION_SECTION_FORM_SUCCESS: case SubmissionObjectActionTypes.SAVE_SUBMISSION_FORM_ERROR: case SubmissionObjectActionTypes.SAVE_FOR_LATER_SUBMISSION_FORM_ERROR: diff --git a/src/app/submission/server-submission.service.ts b/src/app/submission/server-submission.service.ts index bfb46f985b..f9382af8d0 100644 --- a/src/app/submission/server-submission.service.ts +++ b/src/app/submission/server-submission.service.ts @@ -4,6 +4,7 @@ import { Observable, of as observableOf } from 'rxjs'; import { SubmissionService } from './submission.service'; import { SubmissionObject } from '../core/submission/models/submission-object.model'; +import { RemoteData } from '../core/data/remote-data'; @Injectable() export class ServerSubmissionService extends SubmissionService { @@ -12,7 +13,7 @@ export class ServerSubmissionService extends SubmissionService { return observableOf(null); } - retrieveSubmission(submissionId): Observable { + retrieveSubmission(submissionId): Observable> { return observableOf(null); } diff --git a/src/app/submission/submission-rest.service.ts b/src/app/submission/submission-rest.service.ts index b364611ae8..83c8e5f594 100644 --- a/src/app/submission/submission-rest.service.ts +++ b/src/app/submission/submission-rest.service.ts @@ -6,7 +6,7 @@ import { distinctUntilChanged, filter, flatMap, map, mergeMap, tap } from 'rxjs/ import { ResponseCacheService } from '../core/cache/response-cache.service'; import { RequestService } from '../core/data/request.service'; import { ResponseCacheEntry } from '../core/cache/response-cache.reducer'; -import { SubmissionSuccessResponse } from '../core/cache/response-cache.models'; +import { ErrorResponse, RestResponse, SubmissionSuccessResponse } from '../core/cache/response-cache.models'; import { isNotEmpty } from '../shared/empty.util'; import { DeleteRequest, @@ -36,25 +36,15 @@ export class SubmissionRestService { protected submitData(request: RestRequest): Observable { const responses = this.responseCache.get(request.href).pipe(map((entry: ResponseCacheEntry) => entry.response)); const errorResponses = responses.pipe( - filter((response: SubmissionSuccessResponse) => !response.isSuccessful), - mergeMap(() => observableThrowError(new Error(`Couldn't send data to server`))) + filter((response: RestResponse) => !response.isSuccessful), + mergeMap((error: ErrorResponse) => observableThrowError(error)) ); const successResponses = responses.pipe( - filter((response: SubmissionSuccessResponse) => response.isSuccessful), + filter((response: RestResponse) => response.isSuccessful), map((response: SubmissionSuccessResponse) => response.dataDefinition as any), distinctUntilChanged() ); return observableMerge(errorResponses, successResponses); -/* const [successResponse, errorResponse] = this.responseCache.get(request.href) - .map((entry: ResponseCacheEntry) => entry.response) - .partition((response: RestResponse) => response.isSuccessful); - return Observable.merge( - errorResponse.flatMap((response: ErrorResponse) => - observableThrowError(new Error(`Couldn't send data to server`))), - successResponse - .filter((response: SubmissionSuccessResponse) => isNotEmpty(response)) - .map((response: SubmissionSuccessResponse) => response.dataDefinition) - .distinctUntilChanged());*/ } protected fetchRequest(request: RestRequest): Observable { @@ -62,8 +52,8 @@ export class SubmissionRestService { map((entry: ResponseCacheEntry) => entry.response), tap(() => this.responseCache.remove(request.href))); const errorResponses = responses.pipe( - filter((response: SubmissionSuccessResponse) => !response.isSuccessful), - mergeMap(() => observableThrowError(new Error(`Couldn't retrieve the data`))) + filter((response: RestResponse) => !response.isSuccessful), + mergeMap((error: ErrorResponse) => observableThrowError(error)) ); const successResponses = responses.pipe( filter((response: SubmissionSuccessResponse) => response.isSuccessful && isNotEmpty(response)), @@ -71,18 +61,6 @@ export class SubmissionRestService { distinctUntilChanged() ); return observableMerge(errorResponses, successResponses); - -/* const [successResponse, errorResponse] = this.responseCache.get(request.href) - .map((entry: ResponseCacheEntry) => entry.response) - .do(() => this.responseCache.remove(request.href)) - .partition((response: RestResponse) => response.isSuccessful); - return Observable.merge( - errorResponse.flatMap((response: ErrorResponse) => - observableThrowError(new Error(`Couldn't retrieve the data`))), - successResponse - .filter((response: SubmissionSuccessResponse) => isNotEmpty(response)) - .map((response: SubmissionSuccessResponse) => response.dataDefinition) - .distinctUntilChanged());*/ } protected getEndpointByIDHref(endpoint, resourceID): string { diff --git a/src/app/submission/submission.service.spec.ts b/src/app/submission/submission.service.spec.ts index 5783df2ef8..f892a554e9 100644 --- a/src/app/submission/submission.service.spec.ts +++ b/src/app/submission/submission.service.spec.ts @@ -30,9 +30,13 @@ import { ResetSubmissionFormAction, SaveAndDepositSubmissionAction, SaveForLaterSubmissionFormAction, - SaveSubmissionFormAction, SaveSubmissionSectionFormAction, + SaveSubmissionFormAction, + SaveSubmissionSectionFormAction, SetActiveSectionAction } from './objects/submission-objects.actions'; +import { RemoteData } from '../core/data/remote-data'; +import { RemoteDataError } from '../core/data/remote-data-error'; +import { throwError as observableThrowError } from 'rxjs/internal/observable/throwError'; describe('SubmissionService test suite', () => { const config = MOCK_SUBMISSION_CONFIG; @@ -363,7 +367,7 @@ describe('SubmissionService test suite', () => { beforeEach(() => { service = TestBed.get(SubmissionService); - spyOn((service as any).store, 'dispatch').and.callThrough() + spyOn((service as any).store, 'dispatch').and.callThrough(); }); describe('changeSubmissionCollection', () => { @@ -840,11 +844,36 @@ describe('SubmissionService test suite', () => { const result = service.retrieveSubmission('826'); const expected = cold('(b|)', { - b: mockSubmissionRestResponse[0] + b: new RemoteData( + false, + false, + true, + null, + mockSubmissionRestResponse[0]) }); expect(result).toBeObservable(expected); }); + + it('should catch error from REST endpoint', () => { + (service as any).restService.getDataById.and.callFake( + () => observableThrowError({ + statusCode: 500, + statusText: 'Internal Server Error', + errorMessage: 'Error message' + }) + ); + + const result = service.retrieveSubmission('826').subscribe((r) => { + expect(r).toEqual(new RemoteData( + false, + false, + false, + new RemoteDataError(500, 'Internal Server Error', 'Error message'), + null + )) + }); + }); }); describe('setActiveSection', () => { diff --git a/src/app/submission/submission.service.ts b/src/app/submission/submission.service.ts index 1a531ff8bc..debcc53722 100644 --- a/src/app/submission/submission.service.ts +++ b/src/app/submission/submission.service.ts @@ -17,7 +17,8 @@ import { ResetSubmissionFormAction, SaveAndDepositSubmissionAction, SaveForLaterSubmissionFormAction, - SaveSubmissionFormAction, SaveSubmissionSectionFormAction, + SaveSubmissionFormAction, + SaveSubmissionSectionFormAction, SetActiveSectionAction } from './objects/submission-objects.actions'; import { @@ -39,6 +40,9 @@ import { SectionsType } from './sections/sections-type'; import { NotificationsService } from '../shared/notifications/notifications.service'; import { SubmissionDefinitionsModel } from '../core/config/models/config-submission-definitions.model'; import { WorkspaceitemSectionsObject } from '../core/submission/models/workspaceitem-sections.model'; +import { RemoteData } from '../core/data/remote-data'; +import { ErrorResponse } from '../core/cache/response-cache.models'; +import { RemoteDataError } from '../core/data/remote-data-error'; @Injectable() export class SubmissionService { @@ -244,7 +248,7 @@ export class SubmissionService { } notifyNewSection(submissionId: string, sectionId: string, sectionType?: SectionsType) { - this.translate.get('submission.sections.general.metadata-extracted-new-section', {sectionId}).pipe( + this.translate.get('submission.sections.general.metadata-extracted-new-section', { sectionId }).pipe( first()) .subscribe((m) => { this.notificationsService.info(null, m, null, true); @@ -252,7 +256,9 @@ export class SubmissionService { } redirectToMyDSpace() { - this.routeService.getPreviousUrl().subscribe((previousUrl: string) => { + this.routeService.getPreviousUrl().pipe( + first() + ).subscribe((previousUrl: string) => { if (isEmpty(previousUrl) || !previousUrl.startsWith('/mydspace')) { this.router.navigate(['/mydspace']); } else { @@ -276,11 +282,26 @@ export class SubmissionService { this.store.dispatch(new ResetSubmissionFormAction(collectionId, submissionId, selfUrl, sections, submissionDefinition)); } - retrieveSubmission(submissionId): Observable { + retrieveSubmission(submissionId): Observable> { return this.restService.getDataById(this.getSubmissionObjectLinkName(), submissionId).pipe( filter((submissionObjects: SubmissionObject[]) => isNotUndefined(submissionObjects)), first(), - map((submissionObjects: SubmissionObject[]) => submissionObjects[0])); + map((submissionObjects: SubmissionObject[]) => new RemoteData( + false, + false, + true, + null, + submissionObjects[0])), + catchError((errorResponse: ErrorResponse) => { + return observableOf(new RemoteData( + false, + false, + false, + new RemoteDataError(errorResponse.statusCode, errorResponse.statusText, errorResponse.errorMessage), + null + )) + }) + ); } setActiveSection(submissionId, sectionId) {