From 375b51d0d4a893fdf75c58d9d23cc2932d7a6e71 Mon Sep 17 00:00:00 2001 From: Ben Bosman Date: Thu, 2 Jan 2020 18:45:09 +0100 Subject: [PATCH 01/11] verify whether file uploads are mandatory --- src/app/shared/mocks/mock-submission.ts | 5 +++- .../upload/section-upload.component.spec.ts | 29 ++++++++++++++++++- .../upload/section-upload.component.ts | 12 +++++++- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/app/shared/mocks/mock-submission.ts b/src/app/shared/mocks/mock-submission.ts index 922e6ad02d..805612c758 100644 --- a/src/app/shared/mocks/mock-submission.ts +++ b/src/app/shared/mocks/mock-submission.ts @@ -1325,7 +1325,7 @@ export const mockUploadConfigResponse = { }, self: 'https://rest.api/dspace-spring-rest/api/config/submissionforms/bitstream-metadata' }, - required: false, + required: true, maxSize: 536870912, name: 'upload', type: 'submissionupload', @@ -1336,6 +1336,9 @@ export const mockUploadConfigResponse = { self: 'https://rest.api/dspace-spring-rest/api/config/submissionuploads/upload' }; +export const mockUploadConfigResponseNotRequired = mockUploadConfigResponse; +mockUploadConfigResponseNotRequired.required = false; + export const mockAccessConditionOptions = [ { name: 'openaccess', diff --git a/src/app/submission/sections/upload/section-upload.component.spec.ts b/src/app/submission/sections/upload/section-upload.component.spec.ts index fd9f88d939..58ad12ddc9 100644 --- a/src/app/submission/sections/upload/section-upload.component.spec.ts +++ b/src/app/submission/sections/upload/section-upload.component.spec.ts @@ -18,6 +18,7 @@ import { mockSubmissionId, mockSubmissionState, mockUploadConfigResponse, + mockUploadConfigResponseNotRequired, mockUploadFiles } from '../../../shared/mocks/mock-submission'; import { BrowserModule } from '@angular/platform-browser'; @@ -215,6 +216,7 @@ describe('SubmissionSectionUploadComponent test suite', () => { expect(comp.collectionName).toBe(mockCollection.name); expect(comp.availableAccessConditionOptions.length).toBe(4); expect(comp.availableAccessConditionOptions).toEqual(mockUploadConfigResponse.accessConditionOptions as any); + expect(comp.required).toBe(true); expect(compAsAny.subs.length).toBe(2); expect(compAsAny.availableGroups.size).toBe(2); expect(compAsAny.availableGroups).toEqual(expectedGroupsMap); @@ -254,6 +256,7 @@ describe('SubmissionSectionUploadComponent test suite', () => { expect(comp.collectionName).toBe(mockCollection.name); expect(comp.availableAccessConditionOptions.length).toBe(4); expect(comp.availableAccessConditionOptions).toEqual(mockUploadConfigResponse.accessConditionOptions as any); + expect(comp.required).toBe(true); expect(compAsAny.subs.length).toBe(2); expect(compAsAny.availableGroups.size).toBe(2); expect(compAsAny.availableGroups).toEqual(expectedGroupsMap); @@ -263,17 +266,41 @@ describe('SubmissionSectionUploadComponent test suite', () => { }); - it('should the properly section status', () => { + it('should properly read the section status', () => { bitstreamService.getUploadedFileList.and.returnValue(hot('-a-b', { a: [], b: mockUploadFiles })); + expect(comp.required).toBe(true); + expect(compAsAny.getSectionStatus()).toBeObservable(cold('-c-d', { c: false, d: true })); }); + + it('should properly read the section status when required is false', () => { + uploadsConfigService.getConfigByHref.and.returnValue(observableOf( + new ConfigData(new PageInfo(), mockUploadConfigResponseNotRequired as any) + )); + + bitstreamService.getUploadedFileList.and.returnValue(observableOf(mockUploadFiles)); + + comp.onSectionInit(); + + bitstreamService.getUploadedFileList.and.returnValue(hot('-a-b', { + a: [], + b: mockUploadFiles + })); + + expect(comp.required).toBe(false); + + expect(compAsAny.getSectionStatus()).toBeObservable(cold('-c-d', { + c: true, + d: true + })); + }); }); }); diff --git a/src/app/submission/sections/upload/section-upload.component.ts b/src/app/submission/sections/upload/section-upload.component.ts index 9dbd1079f4..24bb658d71 100644 --- a/src/app/submission/sections/upload/section-upload.component.ts +++ b/src/app/submission/sections/upload/section-upload.component.ts @@ -104,6 +104,12 @@ export class SubmissionSectionUploadComponent extends SectionModelComponent { */ protected availableGroups: Map; // Groups for any policy + /** + * Is the upload required + * @type {boolean} + */ + public required: boolean; + /** * Array to track all subscriptions and unsubscribe them onDestroy * @type {Array} @@ -172,6 +178,7 @@ export class SubmissionSectionUploadComponent extends SectionModelComponent { }), flatMap(() => config$), flatMap((config: SubmissionUploadsModel) => { + this.required = config.required; this.availableAccessConditionOptions = isNotEmpty(config.accessConditionOptions) ? config.accessConditionOptions : []; this.collectionPolicyType = this.availableAccessConditionOptions.length > 0 @@ -273,8 +280,11 @@ export class SubmissionSectionUploadComponent extends SectionModelComponent { * the section status */ protected getSectionStatus(): Observable { + // if not mandatory, always true + // if mandatory, at least one file is required return this.bitstreamService.getUploadedFileList(this.submissionId, this.sectionData.id).pipe( - map((fileList: any[]) => (isNotUndefined(fileList) && fileList.length > 0))); + map((fileList: any[]) => + (!this.required || (isNotUndefined(fileList) && fileList.length > 0)))); } /** From 9ed2d4321edc2444b105656d8fdc74edd23646fe Mon Sep 17 00:00:00 2001 From: Ben Bosman Date: Fri, 3 Jan 2020 09:32:14 +0100 Subject: [PATCH 02/11] verify whether file uploads are mandatory --- src/app/shared/mocks/mock-submission.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/shared/mocks/mock-submission.ts b/src/app/shared/mocks/mock-submission.ts index 805612c758..a97d2fb31a 100644 --- a/src/app/shared/mocks/mock-submission.ts +++ b/src/app/shared/mocks/mock-submission.ts @@ -1336,7 +1336,8 @@ export const mockUploadConfigResponse = { self: 'https://rest.api/dspace-spring-rest/api/config/submissionuploads/upload' }; -export const mockUploadConfigResponseNotRequired = mockUploadConfigResponse; +// Clone the object and change one property +export const mockUploadConfigResponseNotRequired = JSON.parse(JSON.stringify(mockUploadConfigResponse)); mockUploadConfigResponseNotRequired.required = false; export const mockAccessConditionOptions = [ From fb10c7764b6a9f0e09d67603edd039b2e3ebecab Mon Sep 17 00:00:00 2001 From: Ben Bosman Date: Fri, 3 Jan 2020 10:04:31 +0100 Subject: [PATCH 03/11] case sensitive for Linux compatibility --- .../upload/section-upload.component.spec.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/app/submission/sections/upload/section-upload.component.spec.ts b/src/app/submission/sections/upload/section-upload.component.spec.ts index 58ad12ddc9..8d215d12c7 100644 --- a/src/app/submission/sections/upload/section-upload.component.spec.ts +++ b/src/app/submission/sections/upload/section-upload.component.spec.ts @@ -267,6 +267,12 @@ describe('SubmissionSectionUploadComponent test suite', () => { }); it('should properly read the section status', () => { + uploadsConfigService.getConfigByHref.and.returnValue(observableOf( + new ConfigData(new PageInfo(), mockUploadConfigResponseNotRequired as any) + )); + + comp.onSectionInit(); + bitstreamService.getUploadedFileList.and.returnValue(hot('-a-b', { a: [], b: mockUploadFiles @@ -281,10 +287,21 @@ describe('SubmissionSectionUploadComponent test suite', () => { }); it('should properly read the section status when required is false', () => { + submissionServiceStub.getSubmissionObject.and.returnValue(observableOf(submissionState)); + + collectionDataService.findById.and.returnValue(createSuccessfulRemoteDataObject$(mockCollection)); + + resourcePolicyService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$(mockDefaultAccessCondition)); + uploadsConfigService.getConfigByHref.and.returnValue(observableOf( new ConfigData(new PageInfo(), mockUploadConfigResponseNotRequired as any) )); + groupService.findById.and.returnValues( + createSuccessfulRemoteDataObject$(Object.assign(new Group(), mockGroup)), + createSuccessfulRemoteDataObject$(Object.assign(new Group(), mockGroup)) + ); + bitstreamService.getUploadedFileList.and.returnValue(observableOf(mockUploadFiles)); comp.onSectionInit(); From 1a22f147fb03ed611d3112730effc94b93cefb1b Mon Sep 17 00:00:00 2001 From: Ben Bosman Date: Fri, 3 Jan 2020 10:44:01 +0100 Subject: [PATCH 04/11] verify whether file uploads are mandatory --- .../sections/upload/section-upload.component.spec.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/app/submission/sections/upload/section-upload.component.spec.ts b/src/app/submission/sections/upload/section-upload.component.spec.ts index 8d215d12c7..bf971b2f6f 100644 --- a/src/app/submission/sections/upload/section-upload.component.spec.ts +++ b/src/app/submission/sections/upload/section-upload.component.spec.ts @@ -267,10 +267,21 @@ describe('SubmissionSectionUploadComponent test suite', () => { }); it('should properly read the section status', () => { + submissionServiceStub.getSubmissionObject.and.returnValue(observableOf(submissionState)); + + collectionDataService.findById.and.returnValue(createSuccessfulRemoteDataObject$(mockCollection)); + + resourcePolicyService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$(mockDefaultAccessCondition)); + uploadsConfigService.getConfigByHref.and.returnValue(observableOf( new ConfigData(new PageInfo(), mockUploadConfigResponseNotRequired as any) )); + groupService.findById.and.returnValues( + createSuccessfulRemoteDataObject$(Object.assign(new Group(), mockGroup)), + createSuccessfulRemoteDataObject$(Object.assign(new Group(), mockGroup)) + ); + comp.onSectionInit(); bitstreamService.getUploadedFileList.and.returnValue(hot('-a-b', { From 015a0c76f82b134c49bea70284df857a32945de2 Mon Sep 17 00:00:00 2001 From: Ben Bosman Date: Fri, 3 Jan 2020 11:14:57 +0100 Subject: [PATCH 05/11] verify whether file uploads are mandatory --- .../submission/sections/upload/section-upload.component.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/app/submission/sections/upload/section-upload.component.spec.ts b/src/app/submission/sections/upload/section-upload.component.spec.ts index bf971b2f6f..68f5ec6de4 100644 --- a/src/app/submission/sections/upload/section-upload.component.spec.ts +++ b/src/app/submission/sections/upload/section-upload.component.spec.ts @@ -313,8 +313,6 @@ describe('SubmissionSectionUploadComponent test suite', () => { createSuccessfulRemoteDataObject$(Object.assign(new Group(), mockGroup)) ); - bitstreamService.getUploadedFileList.and.returnValue(observableOf(mockUploadFiles)); - comp.onSectionInit(); bitstreamService.getUploadedFileList.and.returnValue(hot('-a-b', { From a4d9ce2ec9d8141232740facbf9f4422bf01b08c Mon Sep 17 00:00:00 2001 From: Ben Bosman Date: Mon, 6 Jan 2020 09:55:47 +0100 Subject: [PATCH 06/11] verify whether file uploads are mandatory --- .../submission/sections/upload/section-upload.component.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/submission/sections/upload/section-upload.component.spec.ts b/src/app/submission/sections/upload/section-upload.component.spec.ts index 68f5ec6de4..3a512c05c0 100644 --- a/src/app/submission/sections/upload/section-upload.component.spec.ts +++ b/src/app/submission/sections/upload/section-upload.component.spec.ts @@ -274,7 +274,7 @@ describe('SubmissionSectionUploadComponent test suite', () => { resourcePolicyService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$(mockDefaultAccessCondition)); uploadsConfigService.getConfigByHref.and.returnValue(observableOf( - new ConfigData(new PageInfo(), mockUploadConfigResponseNotRequired as any) + new ConfigData(new PageInfo(), mockUploadConfigResponse as any) )); groupService.findById.and.returnValues( From f7d4d89db2d49b1b5b3caa566bacc06fe6577350 Mon Sep 17 00:00:00 2001 From: Ben Bosman Date: Mon, 6 Jan 2020 10:24:19 +0100 Subject: [PATCH 07/11] verify whether file uploads are mandatory --- src/app/shared/mocks/mock-submission.ts | 99 ++++++++++++++++++++++++- 1 file changed, 97 insertions(+), 2 deletions(-) diff --git a/src/app/shared/mocks/mock-submission.ts b/src/app/shared/mocks/mock-submission.ts index a97d2fb31a..31e4c7418e 100644 --- a/src/app/shared/mocks/mock-submission.ts +++ b/src/app/shared/mocks/mock-submission.ts @@ -1336,9 +1336,104 @@ export const mockUploadConfigResponse = { self: 'https://rest.api/dspace-spring-rest/api/config/submissionuploads/upload' }; +export const mockUploadConfigResponseNotRequired = { + accessConditionOptions: [ + { + name: 'openaccess', + groupUUID: '123456-g', + hasStartDate: false, + hasEndDate: false + }, + { + name: 'lease', + groupUUID: '123456-g', + hasStartDate: false, + hasEndDate: true, + maxEndDate: '2019-07-12T14:40:06.308+0000' + }, + { + name: 'embargo', + groupUUID: '123456-g', + hasStartDate: true, + hasEndDate: false, + maxStartDate: '2022-01-12T14:40:06.308+0000' + }, + { + name: 'administrator', + groupUUID: '0f2773dd-1741-475f-80e7-ccdef153d655', + hasStartDate: false, + hasEndDate: false + } + ], + metadata: { + rows: [ + { + fields: [ + { + input: { + type: 'onebox' + }, + label: 'Title', + mandatory: true, + repeatable: false, + mandatoryMessage: 'You must enter a main title for this item.', + hints: 'Enter the name of the file.', + selectableMetadata: [ + { + metadata: 'dc.title', + label: null, + authority: null, + closed: null + } + ], + languageCodes: [] + } + ] + }, + { + fields: [ + { + input: { + type: 'textarea' + }, + label: 'Description', + mandatory: false, + repeatable: true, + hints: 'Enter a description for the file', + selectableMetadata: [ + { + metadata: 'dc.description', + label: null, + authority: null, + closed: null + } + ], + languageCodes: [] + } + ] + } + ], + name: 'bitstream-metadata', + type: 'submissionform', + _links: { + self: 'https://rest.api/dspace-spring-rest/api/config/submissionforms/bitstream-metadata' + }, + self: 'https://rest.api/dspace-spring-rest/api/config/submissionforms/bitstream-metadata' + }, + required: false, + maxSize: 536870912, + name: 'upload', + type: 'submissionupload', + _links: { + metadata: 'https://rest.api/dspace-spring-rest/api/config/submissionuploads/upload/metadata', + self: 'https://rest.api/dspace-spring-rest/api/config/submissionuploads/upload' + }, + self: 'https://rest.api/dspace-spring-rest/api/config/submissionuploads/upload' +}; + // Clone the object and change one property -export const mockUploadConfigResponseNotRequired = JSON.parse(JSON.stringify(mockUploadConfigResponse)); -mockUploadConfigResponseNotRequired.required = false; +// export const mockUploadConfigResponseNotRequired = JSON.parse(JSON.stringify(mockUploadConfigResponse)); +// mockUploadConfigResponseNotRequired.required = false; export const mockAccessConditionOptions = [ { From 3e51fd85980904da61367b9ed373711e62ae6fd7 Mon Sep 17 00:00:00 2001 From: lotte Date: Fri, 10 Jan 2020 16:36:47 +0100 Subject: [PATCH 08/11] fixed test issue --- .../upload/section-upload.component.spec.ts | 91 ++++++++++--------- .../upload/section-upload.component.ts | 4 +- 2 files changed, 51 insertions(+), 44 deletions(-) diff --git a/src/app/submission/sections/upload/section-upload.component.spec.ts b/src/app/submission/sections/upload/section-upload.component.spec.ts index 3a512c05c0..a96dc3b32a 100644 --- a/src/app/submission/sections/upload/section-upload.component.spec.ts +++ b/src/app/submission/sections/upload/section-upload.component.spec.ts @@ -5,6 +5,7 @@ import { TranslateModule } from '@ngx-translate/core'; import { of as observableOf } from 'rxjs'; import { createSuccessfulRemoteDataObject$, createTestComponent } from '../../../shared/testing/utils'; +import { SubmissionObjectState } from '../../objects/submission-objects.reducer'; import { SubmissionService } from '../../submission.service'; import { SubmissionServiceStub } from '../../../shared/testing/submission-service-stub'; import { SectionsService } from '../sections.service'; @@ -18,8 +19,7 @@ import { mockSubmissionId, mockSubmissionState, mockUploadConfigResponse, - mockUploadConfigResponseNotRequired, - mockUploadFiles + mockUploadConfigResponseNotRequired, mockUploadFiles, } from '../../../shared/mocks/mock-submission'; import { BrowserModule } from '@angular/platform-browser'; import { CommonModule } from '@angular/common'; @@ -32,7 +32,6 @@ import { cold, hot } from 'jasmine-marbles'; import { Collection } from '../../../core/shared/collection.model'; import { ResourcePolicy } from '../../../core/shared/resource-policy.model'; import { ResourcePolicyService } from '../../../core/data/resource-policy.service'; -import { RemoteData } from '../../../core/data/remote-data'; import { ConfigData } from '../../../core/config/config-data'; import { PageInfo } from '../../../core/shared/page-info.model'; import { Group } from '../../../core/eperson/models/group.model'; @@ -66,17 +65,7 @@ function getMockResourcePolicyService(): ResourcePolicyService { }); } -const sectionObject: SectionDataObject = { - config: 'https://dspace7.4science.it/or2018/api/config/submissionforms/upload', - mandatory: true, - data: { - files: [] - }, - errors: [], - header: 'submit.progressbar.describe.upload', - id: 'upload', - sectionType: SectionsType.Upload -}; +let sectionObject: SectionDataObject; describe('SubmissionSectionUploadComponent test suite', () => { @@ -91,30 +80,48 @@ describe('SubmissionSectionUploadComponent test suite', () => { let uploadsConfigService: any; let bitstreamService: any; - const submissionId = mockSubmissionId; - const collectionId = mockSubmissionCollectionId; - const submissionState = Object.assign({}, mockSubmissionState[mockSubmissionId]); - const mockCollection = Object.assign(new Collection(), { - name: 'Community 1-Collection 1', - id: collectionId, - metadata: [ - { - key: 'dc.title', - language: 'en_US', - value: 'Community 1-Collection 1' - }], - _links: { - defaultAccessConditions: collectionId + '/defaultAccessConditions' - } - }); - const mockDefaultAccessCondition = Object.assign(new ResourcePolicy(), { - name: null, - groupUUID: '11cc35e5-a11d-4b64-b5b9-0052a5d15509', - id: 20, - uuid: 'resource-policy-20' - }); + let submissionId: string; + let collectionId: string; + let submissionState: SubmissionObjectState; + let mockCollection: Collection; + let mockDefaultAccessCondition: ResourcePolicy; beforeEach(async(() => { + sectionObject = { + config: 'https://dspace7.4science.it/or2018/api/config/submissionforms/upload', + mandatory: true, + data: { + files: [] + }, + errors: [], + header: 'submit.progressbar.describe.upload', + id: 'upload', + sectionType: SectionsType.Upload + }; + submissionId = mockSubmissionId; + collectionId = mockSubmissionCollectionId; + submissionState = Object.assign({}, mockSubmissionState[mockSubmissionId]) as any; + mockCollection = Object.assign(new Collection(), { + name: 'Community 1-Collection 1', + id: collectionId, + metadata: [ + { + key: 'dc.title', + language: 'en_US', + value: 'Community 1-Collection 1' + }], + _links: { + defaultAccessConditions: collectionId + '/defaultAccessConditions' + } + }); + + mockDefaultAccessCondition = Object.assign(new ResourcePolicy(), { + name: null, + groupUUID: '11cc35e5-a11d-4b64-b5b9-0052a5d15509', + id: 20, + uuid: 'resource-policy-20' + }); + TestBed.configureTestingModule({ imports: [ BrowserModule, @@ -207,7 +214,7 @@ describe('SubmissionSectionUploadComponent test suite', () => { comp.onSectionInit(); - const expectedGroupsMap = new Map([ + const expectedGroupsMap = new Map([ [mockUploadConfigResponse.accessConditionOptions[1].name, [mockGroup as any]], [mockUploadConfigResponse.accessConditionOptions[2].name, [mockGroup as any]], ]); @@ -247,7 +254,7 @@ describe('SubmissionSectionUploadComponent test suite', () => { comp.onSectionInit(); - const expectedGroupsMap = new Map([ + const expectedGroupsMap = new Map([ [mockUploadConfigResponse.accessConditionOptions[1].name, [mockGroup as any]], [mockUploadConfigResponse.accessConditionOptions[2].name, [mockGroup as any]], ]); @@ -282,13 +289,13 @@ describe('SubmissionSectionUploadComponent test suite', () => { createSuccessfulRemoteDataObject$(Object.assign(new Group(), mockGroup)) ); - comp.onSectionInit(); - bitstreamService.getUploadedFileList.and.returnValue(hot('-a-b', { a: [], b: mockUploadFiles })); + comp.onSectionInit(); + expect(comp.required).toBe(true); expect(compAsAny.getSectionStatus()).toBeObservable(cold('-c-d', { @@ -313,13 +320,13 @@ describe('SubmissionSectionUploadComponent test suite', () => { createSuccessfulRemoteDataObject$(Object.assign(new Group(), mockGroup)) ); - comp.onSectionInit(); - bitstreamService.getUploadedFileList.and.returnValue(hot('-a-b', { a: [], b: mockUploadFiles })); + comp.onSectionInit(); + expect(comp.required).toBe(false); expect(compAsAny.getSectionStatus()).toBeObservable(cold('-c-d', { diff --git a/src/app/submission/sections/upload/section-upload.component.ts b/src/app/submission/sections/upload/section-upload.component.ts index 24bb658d71..b4c2a9b30b 100644 --- a/src/app/submission/sections/upload/section-upload.component.ts +++ b/src/app/submission/sections/upload/section-upload.component.ts @@ -1,6 +1,6 @@ import { ChangeDetectorRef, Component, Inject } from '@angular/core'; -import { combineLatest, Observable, Subscription } from 'rxjs'; +import { combineLatest as observableCombineLatest, Observable, Subscription } from 'rxjs'; import { distinctUntilChanged, filter, find, flatMap, map, reduce, take, tap } from 'rxjs/operators'; import { SectionModelComponent } from '../models/section.model'; @@ -228,7 +228,7 @@ export class SubmissionSectionUploadComponent extends SectionModelComponent { }), // retrieve submission's bitstreams from state - combineLatest(this.configMetadataForm$, + observableCombineLatest(this.configMetadataForm$, this.bitstreamService.getUploadedFileList(this.submissionId, this.sectionData.id)).pipe( filter(([configMetadataForm, fileList]: [SubmissionFormsModel, any[]]) => { return isNotEmpty(configMetadataForm) && isNotUndefined(fileList) From 70b2c8c6e1cbbfa68398ebce7b9ae8ec6dcdec3b Mon Sep 17 00:00:00 2001 From: Ben Bosman Date: Fri, 10 Jan 2020 18:30:23 +0100 Subject: [PATCH 09/11] go back to the setup where mockUploadConfigResponseNotRequired starts from a clone --- src/app/shared/mocks/mock-submission.ts | 99 +------------------ .../upload/section-upload.component.spec.ts | 2 +- 2 files changed, 3 insertions(+), 98 deletions(-) diff --git a/src/app/shared/mocks/mock-submission.ts b/src/app/shared/mocks/mock-submission.ts index 31e4c7418e..a97d2fb31a 100644 --- a/src/app/shared/mocks/mock-submission.ts +++ b/src/app/shared/mocks/mock-submission.ts @@ -1336,104 +1336,9 @@ export const mockUploadConfigResponse = { self: 'https://rest.api/dspace-spring-rest/api/config/submissionuploads/upload' }; -export const mockUploadConfigResponseNotRequired = { - accessConditionOptions: [ - { - name: 'openaccess', - groupUUID: '123456-g', - hasStartDate: false, - hasEndDate: false - }, - { - name: 'lease', - groupUUID: '123456-g', - hasStartDate: false, - hasEndDate: true, - maxEndDate: '2019-07-12T14:40:06.308+0000' - }, - { - name: 'embargo', - groupUUID: '123456-g', - hasStartDate: true, - hasEndDate: false, - maxStartDate: '2022-01-12T14:40:06.308+0000' - }, - { - name: 'administrator', - groupUUID: '0f2773dd-1741-475f-80e7-ccdef153d655', - hasStartDate: false, - hasEndDate: false - } - ], - metadata: { - rows: [ - { - fields: [ - { - input: { - type: 'onebox' - }, - label: 'Title', - mandatory: true, - repeatable: false, - mandatoryMessage: 'You must enter a main title for this item.', - hints: 'Enter the name of the file.', - selectableMetadata: [ - { - metadata: 'dc.title', - label: null, - authority: null, - closed: null - } - ], - languageCodes: [] - } - ] - }, - { - fields: [ - { - input: { - type: 'textarea' - }, - label: 'Description', - mandatory: false, - repeatable: true, - hints: 'Enter a description for the file', - selectableMetadata: [ - { - metadata: 'dc.description', - label: null, - authority: null, - closed: null - } - ], - languageCodes: [] - } - ] - } - ], - name: 'bitstream-metadata', - type: 'submissionform', - _links: { - self: 'https://rest.api/dspace-spring-rest/api/config/submissionforms/bitstream-metadata' - }, - self: 'https://rest.api/dspace-spring-rest/api/config/submissionforms/bitstream-metadata' - }, - required: false, - maxSize: 536870912, - name: 'upload', - type: 'submissionupload', - _links: { - metadata: 'https://rest.api/dspace-spring-rest/api/config/submissionuploads/upload/metadata', - self: 'https://rest.api/dspace-spring-rest/api/config/submissionuploads/upload' - }, - self: 'https://rest.api/dspace-spring-rest/api/config/submissionuploads/upload' -}; - // Clone the object and change one property -// export const mockUploadConfigResponseNotRequired = JSON.parse(JSON.stringify(mockUploadConfigResponse)); -// mockUploadConfigResponseNotRequired.required = false; +export const mockUploadConfigResponseNotRequired = JSON.parse(JSON.stringify(mockUploadConfigResponse)); +mockUploadConfigResponseNotRequired.required = false; export const mockAccessConditionOptions = [ { diff --git a/src/app/submission/sections/upload/section-upload.component.spec.ts b/src/app/submission/sections/upload/section-upload.component.spec.ts index a96dc3b32a..71433d04f1 100644 --- a/src/app/submission/sections/upload/section-upload.component.spec.ts +++ b/src/app/submission/sections/upload/section-upload.component.spec.ts @@ -273,7 +273,7 @@ describe('SubmissionSectionUploadComponent test suite', () => { }); - it('should properly read the section status', () => { + it('should properly read the section status when required is true', () => { submissionServiceStub.getSubmissionObject.and.returnValue(observableOf(submissionState)); collectionDataService.findById.and.returnValue(createSuccessfulRemoteDataObject$(mockCollection)); From a45273048d882842728a61f59cea95aa3f09afd8 Mon Sep 17 00:00:00 2001 From: Ben Bosman Date: Fri, 31 Jan 2020 09:47:40 +0100 Subject: [PATCH 10/11] verify whether file uploads are mandatory => observe changes to both filelist and required status --- resources/i18n/en.json5 | 2 ++ .../sections/upload/section-upload.component.ts | 14 ++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/resources/i18n/en.json5 b/resources/i18n/en.json5 index 478f183da4..42eee31e7c 100644 --- a/resources/i18n/en.json5 +++ b/resources/i18n/en.json5 @@ -532,6 +532,8 @@ "error.validation.pattern": "This input is restricted by the current pattern: {{ pattern }}.", + "error.validation.filerequired": "The file upload is mandatory", + "footer.copyright": "copyright © 2002-{{ year }}", diff --git a/src/app/submission/sections/upload/section-upload.component.ts b/src/app/submission/sections/upload/section-upload.component.ts index b4c2a9b30b..6c2506b773 100644 --- a/src/app/submission/sections/upload/section-upload.component.ts +++ b/src/app/submission/sections/upload/section-upload.component.ts @@ -1,6 +1,6 @@ import { ChangeDetectorRef, Component, Inject } from '@angular/core'; -import { combineLatest as observableCombineLatest, Observable, Subscription } from 'rxjs'; +import { BehaviorSubject, combineLatest as observableCombineLatest, Observable, Subscription} from 'rxjs'; import { distinctUntilChanged, filter, find, flatMap, map, reduce, take, tap } from 'rxjs/operators'; import { SectionModelComponent } from '../models/section.model'; @@ -108,7 +108,7 @@ export class SubmissionSectionUploadComponent extends SectionModelComponent { * Is the upload required * @type {boolean} */ - public required: boolean; + public required$ = new BehaviorSubject(true); /** * Array to track all subscriptions and unsubscribe them onDestroy @@ -178,7 +178,7 @@ export class SubmissionSectionUploadComponent extends SectionModelComponent { }), flatMap(() => config$), flatMap((config: SubmissionUploadsModel) => { - this.required = config.required; + this.required$.next(config.required); this.availableAccessConditionOptions = isNotEmpty(config.accessConditionOptions) ? config.accessConditionOptions : []; this.collectionPolicyType = this.availableAccessConditionOptions.length > 0 @@ -282,9 +282,11 @@ export class SubmissionSectionUploadComponent extends SectionModelComponent { protected getSectionStatus(): Observable { // if not mandatory, always true // if mandatory, at least one file is required - return this.bitstreamService.getUploadedFileList(this.submissionId, this.sectionData.id).pipe( - map((fileList: any[]) => - (!this.required || (isNotUndefined(fileList) && fileList.length > 0)))); + return observableCombineLatest(this.required$, + this.bitstreamService.getUploadedFileList(this.submissionId, this.sectionData.id), + (required,fileList: any[]) => { + return (!required || (isNotUndefined(fileList) && fileList.length > 0)); + }); } /** From 5585bcbc9b54be5f47939449bbebae556bc3adcb Mon Sep 17 00:00:00 2001 From: Ben Bosman Date: Fri, 31 Jan 2020 15:14:01 +0100 Subject: [PATCH 11/11] updated tests --- .../sections/upload/section-upload.component.spec.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/app/submission/sections/upload/section-upload.component.spec.ts b/src/app/submission/sections/upload/section-upload.component.spec.ts index 71433d04f1..a58de09b8d 100644 --- a/src/app/submission/sections/upload/section-upload.component.spec.ts +++ b/src/app/submission/sections/upload/section-upload.component.spec.ts @@ -223,7 +223,7 @@ describe('SubmissionSectionUploadComponent test suite', () => { expect(comp.collectionName).toBe(mockCollection.name); expect(comp.availableAccessConditionOptions.length).toBe(4); expect(comp.availableAccessConditionOptions).toEqual(mockUploadConfigResponse.accessConditionOptions as any); - expect(comp.required).toBe(true); + expect(comp.required$.getValue()).toBe(true); expect(compAsAny.subs.length).toBe(2); expect(compAsAny.availableGroups.size).toBe(2); expect(compAsAny.availableGroups).toEqual(expectedGroupsMap); @@ -263,7 +263,7 @@ describe('SubmissionSectionUploadComponent test suite', () => { expect(comp.collectionName).toBe(mockCollection.name); expect(comp.availableAccessConditionOptions.length).toBe(4); expect(comp.availableAccessConditionOptions).toEqual(mockUploadConfigResponse.accessConditionOptions as any); - expect(comp.required).toBe(true); + expect(comp.required$.getValue()).toBe(true); expect(compAsAny.subs.length).toBe(2); expect(compAsAny.availableGroups.size).toBe(2); expect(compAsAny.availableGroups).toEqual(expectedGroupsMap); @@ -289,14 +289,14 @@ describe('SubmissionSectionUploadComponent test suite', () => { createSuccessfulRemoteDataObject$(Object.assign(new Group(), mockGroup)) ); - bitstreamService.getUploadedFileList.and.returnValue(hot('-a-b', { + bitstreamService.getUploadedFileList.and.returnValue(cold('-a-b', { a: [], b: mockUploadFiles })); comp.onSectionInit(); - expect(comp.required).toBe(true); + expect(comp.required$.getValue()).toBe(true); expect(compAsAny.getSectionStatus()).toBeObservable(cold('-c-d', { c: false, @@ -320,14 +320,14 @@ describe('SubmissionSectionUploadComponent test suite', () => { createSuccessfulRemoteDataObject$(Object.assign(new Group(), mockGroup)) ); - bitstreamService.getUploadedFileList.and.returnValue(hot('-a-b', { + bitstreamService.getUploadedFileList.and.returnValue(cold('-a-b', { a: [], b: mockUploadFiles })); comp.onSectionInit(); - expect(comp.required).toBe(false); + expect(comp.required$.getValue()).toBe(false); expect(compAsAny.getSectionStatus()).toBeObservable(cold('-c-d', { c: true,