From 5e17a4e958db858bc7eee8c4806ee51ce46ff00b Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 30 Nov 2021 13:27:16 +0100 Subject: [PATCH 1/7] 85262: Always enable deposit button --- .../submission-form-footer.component.html | 1 - .../submission-form-footer.component.spec.ts | 9 ---- .../submission-objects.effects.spec.ts | 53 ------------------- .../objects/submission-objects.effects.ts | 4 +- 4 files changed, 1 insertion(+), 66 deletions(-) diff --git a/src/app/submission/form/footer/submission-form-footer.component.html b/src/app/submission/form/footer/submission-form-footer.component.html index 4964eb56a2..203f7e2afa 100644 --- a/src/app/submission/form/footer/submission-form-footer.component.html +++ b/src/app/submission/form/footer/submission-form-footer.component.html @@ -41,7 +41,6 @@ diff --git a/src/app/submission/form/footer/submission-form-footer.component.spec.ts b/src/app/submission/form/footer/submission-form-footer.component.spec.ts index dd47dad444..dbeb4ee00d 100644 --- a/src/app/submission/form/footer/submission-form-footer.component.spec.ts +++ b/src/app/submission/form/footer/submission-form-footer.component.spec.ts @@ -201,15 +201,6 @@ describe('SubmissionFormFooterComponent Component', () => { }); }); - it('should have deposit button disabled when submission is not valid', () => { - comp.showDepositAndDiscard = observableOf(true); - compAsAny.submissionIsInvalid = observableOf(true); - fixture.detectChanges(); - const depositBtn: any = fixture.debugElement.query(By.css('.btn-success')); - - expect(depositBtn.nativeElement.disabled).toBeTruthy(); - }); - it('should not have deposit button disabled when submission is valid', () => { comp.showDepositAndDiscard = observableOf(true); compAsAny.submissionIsInvalid = observableOf(false); diff --git a/src/app/submission/objects/submission-objects.effects.spec.ts b/src/app/submission/objects/submission-objects.effects.spec.ts index 122ebd90ac..8e763fd94b 100644 --- a/src/app/submission/objects/submission-objects.effects.spec.ts +++ b/src/app/submission/objects/submission-objects.effects.spec.ts @@ -879,59 +879,6 @@ describe('SubmissionObjectEffects test suite', () => { expect(submissionObjectEffects.saveAndDeposit$).toBeObservable(expected); }); - it('should not allow to deposit when there are errors', () => { - store.nextState({ - submission: { - objects: submissionState - } - } as any); - - actions = hot('--a-', { - a: { - type: SubmissionObjectActionTypes.SAVE_AND_DEPOSIT_SUBMISSION, - payload: { - submissionId: submissionId - } - } - }); - - const response = [Object.assign({}, mockSubmissionRestResponse[0], { - sections: mockSectionsData, - errors: mockSectionsErrors - })]; - - submissionJsonPatchOperationsServiceStub.jsonPatchByResourceType.and.returnValue(observableOf(response)); - - const errorsList = parseSectionErrors(mockSectionsErrors); - const expected = cold('--b-', { - b: [ - new UpdateSectionDataAction( - submissionId, - 'traditionalpageone', - mockSectionsData.traditionalpageone as any, - errorsList.traditionalpageone || [], - errorsList.traditionalpageone || [] - ), - new UpdateSectionDataAction( - submissionId, - 'license', - mockSectionsData.license as any, - errorsList.license || [], - errorsList.license || [] - ), - new UpdateSectionDataAction( - submissionId, - 'upload', - mockSectionsData.upload as any, - errorsList.upload || [], - errorsList.upload || [] - ) - ] - }); - - expect(submissionObjectEffects.saveAndDeposit$).toBeObservable(expected); - }); - it('should catch errors and return a SAVE_SUBMISSION_FORM_ERROR', () => { actions = hot('--a-', { a: { diff --git a/src/app/submission/objects/submission-objects.effects.ts b/src/app/submission/objects/submission-objects.effects.ts index b4ba1c2480..535e662922 100644 --- a/src/app/submission/objects/submission-objects.effects.ts +++ b/src/app/submission/objects/submission-objects.effects.ts @@ -206,9 +206,7 @@ export class SubmissionObjectEffects { if (this.canDeposit(response)) { return new DepositSubmissionAction(action.payload.submissionId); } else { - this.notificationsService.warning(null, this.translate.get('submission.sections.general.sections_not_valid')); - return this.parseSaveResponse((currentState.submission as SubmissionState).objects[action.payload.submissionId], - response, action.payload.submissionId, currentState.forms); + return new SaveSubmissionFormSuccessAction(action.payload.submissionId, response); } }), catchError(() => observableOf(new SaveSubmissionFormErrorAction(action.payload.submissionId)))); From 99af22b6216cfd67fffffc0ab5f7f7883584ab1b Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 30 Nov 2021 13:44:08 +0100 Subject: [PATCH 2/7] 85262: Re-disable deposit button during save/deposit --- .../submission/form/footer/submission-form-footer.component.html | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/submission/form/footer/submission-form-footer.component.html b/src/app/submission/form/footer/submission-form-footer.component.html index 203f7e2afa..e954fab34c 100644 --- a/src/app/submission/form/footer/submission-form-footer.component.html +++ b/src/app/submission/form/footer/submission-form-footer.component.html @@ -41,6 +41,7 @@ From ed2c774d86b71ca09c3e286a448650311bf97aca Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 2 Dec 2021 12:39:07 +0100 Subject: [PATCH 3/7] 85262: Allow empty patch requests --- .../json-patch-operations.service.ts | 16 ++++++---- .../submission-form-footer.component.spec.ts | 9 ++++++ .../submission-objects.effects.spec.ts | 30 +++++++++++++++++++ .../objects/submission-objects.effects.ts | 3 +- 4 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/app/core/json-patch/json-patch-operations.service.ts b/src/app/core/json-patch/json-patch-operations.service.ts index c3363f4db4..f160376837 100644 --- a/src/app/core/json-patch/json-patch-operations.service.ts +++ b/src/app/core/json-patch/json-patch-operations.service.ts @@ -40,13 +40,15 @@ export abstract class JsonPatchOperationsService * observable of response */ - protected submitJsonPatchOperations(hrefObs: Observable, resourceType: string, resourceId?: string): Observable { + protected submitJsonPatchOperations(hrefObs: Observable, resourceType: string, resourceId?: string, allowEmptyRequest = false): Observable { const requestId = this.requestService.generateRequestId(); let startTransactionTime = null; - const [patchRequest$, emptyRequest$] = partition((request: PatchRequestDefinition) => isNotEmpty(request.body))(hrefObs.pipe( + const [patchRequest$, emptyRequest$] = partition((request: PatchRequestDefinition) => allowEmptyRequest || isNotEmpty(request.body))(hrefObs.pipe( mergeMap((endpointURL: string) => { return this.store.select(jsonPatchOperationsByResourceType(resourceType)).pipe( take(1), @@ -79,11 +81,11 @@ export abstract class JsonPatchOperationsService isEmpty(request.body)), + filter((request: PatchRequestDefinition) => !allowEmptyRequest && isEmpty(request.body)), tap(() => startTransactionTime = null), map(() => null)), patchRequest$.pipe( - filter((request: PatchRequestDefinition) => isNotEmpty(request.body)), + filter((request: PatchRequestDefinition) => allowEmptyRequest || isNotEmpty(request.body)), tap(() => this.store.dispatch(new StartTransactionPatchOperationsAction(resourceType, resourceId, startTransactionTime))), tap((request: PatchRequestDefinition) => this.requestService.send(request)), mergeMap(() => { @@ -141,16 +143,18 @@ export abstract class JsonPatchOperationsService * observable of response */ - public jsonPatchByResourceType(linkPath: string, scopeId: string, resourceType: string): Observable { + public jsonPatchByResourceType(linkPath: string, scopeId: string, resourceType: string, allowEmptyRequest = false): Observable { const href$ = this.halService.getEndpoint(linkPath).pipe( filter((href: string) => isNotEmpty(href)), distinctUntilChanged(), map((endpointURL: string) => this.getEndpointByIDHref(endpointURL, scopeId))); - return this.submitJsonPatchOperations(href$, resourceType); + return this.submitJsonPatchOperations(href$, resourceType, undefined, allowEmptyRequest); } /** diff --git a/src/app/submission/form/footer/submission-form-footer.component.spec.ts b/src/app/submission/form/footer/submission-form-footer.component.spec.ts index dbeb4ee00d..072f826deb 100644 --- a/src/app/submission/form/footer/submission-form-footer.component.spec.ts +++ b/src/app/submission/form/footer/submission-form-footer.component.spec.ts @@ -201,6 +201,15 @@ describe('SubmissionFormFooterComponent Component', () => { }); }); + it('should not have deposit button disabled when submission is not valid', () => { + comp.showDepositAndDiscard = observableOf(true); + compAsAny.submissionIsInvalid = observableOf(true); + fixture.detectChanges(); + const depositBtn: any = fixture.debugElement.query(By.css('.btn-success')); + + expect(depositBtn.nativeElement.disabled).toBeFalsy(); + }); + it('should not have deposit button disabled when submission is valid', () => { comp.showDepositAndDiscard = observableOf(true); compAsAny.submissionIsInvalid = observableOf(false); diff --git a/src/app/submission/objects/submission-objects.effects.spec.ts b/src/app/submission/objects/submission-objects.effects.spec.ts index 8e763fd94b..e760f71941 100644 --- a/src/app/submission/objects/submission-objects.effects.spec.ts +++ b/src/app/submission/objects/submission-objects.effects.spec.ts @@ -879,6 +879,36 @@ describe('SubmissionObjectEffects test suite', () => { expect(submissionObjectEffects.saveAndDeposit$).toBeObservable(expected); }); + it('should return a SAVE_SUBMISSION_FORM_SUCCESS action when there are errors', () => { + store.nextState({ + submission: { + objects: submissionState + } + } as any); + + actions = hot('--a-', { + a: { + type: SubmissionObjectActionTypes.SAVE_AND_DEPOSIT_SUBMISSION, + payload: { + submissionId: submissionId + } + } + }); + + const response = [Object.assign({}, mockSubmissionRestResponse[0], { + sections: mockSectionsData, + errors: mockSectionsErrors + })]; + + submissionJsonPatchOperationsServiceStub.jsonPatchByResourceType.and.returnValue(observableOf(response)); + + const expected = cold('--b-', { + b: new SaveSubmissionFormSuccessAction(submissionId, response as any[]) + }); + + expect(submissionObjectEffects.saveAndDeposit$).toBeObservable(expected); + }); + it('should catch errors and return a SAVE_SUBMISSION_FORM_ERROR', () => { actions = hot('--a-', { a: { diff --git a/src/app/submission/objects/submission-objects.effects.ts b/src/app/submission/objects/submission-objects.effects.ts index 535e662922..5432aa7a4c 100644 --- a/src/app/submission/objects/submission-objects.effects.ts +++ b/src/app/submission/objects/submission-objects.effects.ts @@ -201,7 +201,8 @@ export class SubmissionObjectEffects { return this.operationsService.jsonPatchByResourceType( this.submissionService.getSubmissionObjectLinkName(), action.payload.submissionId, - 'sections').pipe( + 'sections', + true).pipe( map((response: SubmissionObject[]) => { if (this.canDeposit(response)) { return new DepositSubmissionAction(action.payload.submissionId); From 768de1a1e73a585dfb50f3f1eb6d09b04dbcde06 Mon Sep 17 00:00:00 2001 From: Nathan Buckingham Date: Fri, 3 Dec 2021 10:54:41 -0500 Subject: [PATCH 4/7] w2p-85346 Allow facets to be authority controlled and add test to verify --- src/app/shared/search/facet-value.model.ts | 6 ++++++ src/app/shared/search/search.utils.spec.ts | 10 ++++++++++ src/app/shared/search/search.utils.ts | 4 ++++ 3 files changed, 20 insertions(+) diff --git a/src/app/shared/search/facet-value.model.ts b/src/app/shared/search/facet-value.model.ts index 969e531bd2..935190743d 100644 --- a/src/app/shared/search/facet-value.model.ts +++ b/src/app/shared/search/facet-value.model.ts @@ -24,6 +24,12 @@ export class FacetValue implements HALResource { @autoserialize count: number; + /** + * The Authority Value for this facet + */ + @autoserialize + authorityKey?: string; + /** * The {@link HALLink}s for this FacetValue */ diff --git a/src/app/shared/search/search.utils.spec.ts b/src/app/shared/search/search.utils.spec.ts index ca4b4894b1..40f1328ffe 100644 --- a/src/app/shared/search/search.utils.spec.ts +++ b/src/app/shared/search/search.utils.spec.ts @@ -12,6 +12,7 @@ describe('Search Utils', () => { let facetValueWithSearchHref: FacetValue; let facetValueWithoutSearchHref: FacetValue; let searchFilterConfig: SearchFilterConfig; + let facetValueWithSearchHrefAuthority: FacetValue; beforeEach(() => { facetValueWithSearchHref = Object.assign(new FacetValue(), { @@ -22,6 +23,11 @@ describe('Search Utils', () => { } } }); + facetValueWithSearchHrefAuthority = Object.assign(new FacetValue(), { + value: 'Value with search href', + authorityKey: 'uuid', + } + ); facetValueWithoutSearchHref = Object.assign(new FacetValue(), { value: 'Value without search href' }); @@ -34,6 +40,10 @@ describe('Search Utils', () => { expect(getFacetValueForType(facetValueWithSearchHref, searchFilterConfig)).toEqual('Value with search href,operator'); }); + it('should retrieve the correct value from the Facet', () => { + expect(getFacetValueForType(facetValueWithSearchHrefAuthority, searchFilterConfig)).toEqual('uuid,authority'); + }); + it('should return the facet value with an equals operator by default', () => { expect(getFacetValueForType(facetValueWithoutSearchHref, searchFilterConfig)).toEqual('Value without search href,equals'); }); diff --git a/src/app/shared/search/search.utils.ts b/src/app/shared/search/search.utils.ts index 38c5f568cb..2b9944562a 100644 --- a/src/app/shared/search/search.utils.ts +++ b/src/app/shared/search/search.utils.ts @@ -16,6 +16,10 @@ export function getFacetValueForType(facetValue: FacetValue, searchFilterConfig: return values[1]; } } + if (facetValue.authorityKey) { + return addOperatorToFilterValue(facetValue.authorityKey, 'authority'); + } + return addOperatorToFilterValue(facetValue.value, 'equals'); } From 549529c8896b5e14e4ce6798fb30d52d35b56f8f Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Fri, 10 Dec 2021 14:18:51 +0100 Subject: [PATCH 5/7] 85262: Get submission object instead of sending empty patch + revert empty patch code --- .../json-patch-operations.service.ts | 16 +++++--------- .../submission-objects.effects.spec.ts | 7 ++++++ .../objects/submission-objects.effects.ts | 22 +++++++++++++------ 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/app/core/json-patch/json-patch-operations.service.ts b/src/app/core/json-patch/json-patch-operations.service.ts index f160376837..c3363f4db4 100644 --- a/src/app/core/json-patch/json-patch-operations.service.ts +++ b/src/app/core/json-patch/json-patch-operations.service.ts @@ -40,15 +40,13 @@ export abstract class JsonPatchOperationsService * observable of response */ - protected submitJsonPatchOperations(hrefObs: Observable, resourceType: string, resourceId?: string, allowEmptyRequest = false): Observable { + protected submitJsonPatchOperations(hrefObs: Observable, resourceType: string, resourceId?: string): Observable { const requestId = this.requestService.generateRequestId(); let startTransactionTime = null; - const [patchRequest$, emptyRequest$] = partition((request: PatchRequestDefinition) => allowEmptyRequest || isNotEmpty(request.body))(hrefObs.pipe( + const [patchRequest$, emptyRequest$] = partition((request: PatchRequestDefinition) => isNotEmpty(request.body))(hrefObs.pipe( mergeMap((endpointURL: string) => { return this.store.select(jsonPatchOperationsByResourceType(resourceType)).pipe( take(1), @@ -81,11 +79,11 @@ export abstract class JsonPatchOperationsService !allowEmptyRequest && isEmpty(request.body)), + filter((request: PatchRequestDefinition) => isEmpty(request.body)), tap(() => startTransactionTime = null), map(() => null)), patchRequest$.pipe( - filter((request: PatchRequestDefinition) => allowEmptyRequest || isNotEmpty(request.body)), + filter((request: PatchRequestDefinition) => isNotEmpty(request.body)), tap(() => this.store.dispatch(new StartTransactionPatchOperationsAction(resourceType, resourceId, startTransactionTime))), tap((request: PatchRequestDefinition) => this.requestService.send(request)), mergeMap(() => { @@ -143,18 +141,16 @@ export abstract class JsonPatchOperationsService * observable of response */ - public jsonPatchByResourceType(linkPath: string, scopeId: string, resourceType: string, allowEmptyRequest = false): Observable { + public jsonPatchByResourceType(linkPath: string, scopeId: string, resourceType: string): Observable { const href$ = this.halService.getEndpoint(linkPath).pipe( filter((href: string) => isNotEmpty(href)), distinctUntilChanged(), map((endpointURL: string) => this.getEndpointByIDHref(endpointURL, scopeId))); - return this.submitJsonPatchOperations(href$, resourceType, undefined, allowEmptyRequest); + return this.submitJsonPatchOperations(href$, resourceType); } /** diff --git a/src/app/submission/objects/submission-objects.effects.spec.ts b/src/app/submission/objects/submission-objects.effects.spec.ts index e760f71941..b2bc054287 100644 --- a/src/app/submission/objects/submission-objects.effects.spec.ts +++ b/src/app/submission/objects/submission-objects.effects.spec.ts @@ -54,6 +54,8 @@ import { Item } from '../../core/shared/item.model'; import { WorkspaceitemDataService } from '../../core/submission/workspaceitem-data.service'; import { WorkflowItemDataService } from '../../core/submission/workflowitem-data.service'; import { HALEndpointService } from '../../core/shared/hal-endpoint.service'; +import { SubmissionObjectDataService } from '../../core/submission/submission-object-data.service'; +import { mockSubmissionObjectDataService } from '../../shared/testing/submission-oject-data-service.mock'; describe('SubmissionObjectEffects test suite', () => { let submissionObjectEffects: SubmissionObjectEffects; @@ -63,6 +65,7 @@ describe('SubmissionObjectEffects test suite', () => { let notificationsServiceStub; let submissionServiceStub; let submissionJsonPatchOperationsServiceStub; + let submissionObjectDataServiceStub; const collectionId: string = mockSubmissionCollectionId; const submissionId: string = mockSubmissionId; const submissionDefinitionResponse: any = mockSubmissionDefinitionResponse; @@ -75,6 +78,9 @@ describe('SubmissionObjectEffects test suite', () => { notificationsServiceStub = new NotificationsServiceStub(); submissionServiceStub = new SubmissionServiceStub(); submissionJsonPatchOperationsServiceStub = new SubmissionJsonPatchOperationsServiceStub(); + submissionObjectDataServiceStub = mockSubmissionObjectDataService; + + submissionServiceStub.hasUnsavedModification.and.returnValue(observableOf(true)); TestBed.configureTestingModule({ imports: [ @@ -99,6 +105,7 @@ describe('SubmissionObjectEffects test suite', () => { { provide: WorkflowItemDataService, useValue: {} }, { provide: WorkflowItemDataService, useValue: {} }, { provide: HALEndpointService, useValue: {} }, + { provide: SubmissionObjectDataService, useValue: submissionObjectDataServiceStub }, ], }); diff --git a/src/app/submission/objects/submission-objects.effects.ts b/src/app/submission/objects/submission-objects.effects.ts index 5432aa7a4c..257854f027 100644 --- a/src/app/submission/objects/submission-objects.effects.ts +++ b/src/app/submission/objects/submission-objects.effects.ts @@ -196,13 +196,21 @@ export class SubmissionObjectEffects { */ @Effect() saveAndDeposit$ = this.actions$.pipe( ofType(SubmissionObjectActionTypes.SAVE_AND_DEPOSIT_SUBMISSION), - withLatestFrom(this.store$), - switchMap(([action, currentState]: [SaveAndDepositSubmissionAction, any]) => { - return this.operationsService.jsonPatchByResourceType( - this.submissionService.getSubmissionObjectLinkName(), - action.payload.submissionId, - 'sections', - true).pipe( + withLatestFrom(this.submissionService.hasUnsavedModification()), + switchMap(([action, hasUnsavedModification]: [SaveAndDepositSubmissionAction, boolean]) => { + let response$: Observable; + if (hasUnsavedModification) { + response$ = this.operationsService.jsonPatchByResourceType( + this.submissionService.getSubmissionObjectLinkName(), + action.payload.submissionId, + 'sections') as Observable; + } else { + response$ = this.submissionObjectService.findById(action.payload.submissionId).pipe( + getFirstSucceededRemoteDataPayload(), + map((submissionObject: SubmissionObject) => [submissionObject]) + ); + } + return response$.pipe( map((response: SubmissionObject[]) => { if (this.canDeposit(response)) { return new DepositSubmissionAction(action.payload.submissionId); From e2614b9dadcd17ce84b809518c783dda3a6a75dc Mon Sep 17 00:00:00 2001 From: Nathan Buckingham Date: Mon, 13 Dec 2021 13:48:09 -0500 Subject: [PATCH 6/7] w2p-85847 Add Authority to browse indexes --- .../browse-by-date-page.component.ts | 2 +- .../browse-by-metadata-page.component.ts | 12 +++++++++--- .../browse-by-title-page.component.ts | 2 +- src/app/core/browse/browse.service.spec.ts | 17 ++++++++++++++++- src/app/core/browse/browse.service.ts | 5 ++++- .../browse-entry-list-element.component.html | 2 +- .../browse-entry-list-element.component.ts | 14 +++++++++++++- 7 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.ts b/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.ts index 3158c3d7cc..2b1cd21f14 100644 --- a/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.ts +++ b/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.ts @@ -63,7 +63,7 @@ export class BrowseByDatePageComponent extends BrowseByMetadataPageComponent { this.browseId = params.id || this.defaultBrowseId; this.startsWith = +params.startsWith || params.startsWith; const searchOptions = browseParamsToOptions(params, currentPage, currentSort, this.browseId); - this.updatePageWithItems(searchOptions, this.value); + this.updatePageWithItems(searchOptions, this.value, undefined); this.updateParent(params.scope); this.updateStartsWithOptions(this.browseId, metadataField, params.scope); })); diff --git a/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.ts b/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.ts index 3573ffb264..c6716afb5f 100644 --- a/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.ts +++ b/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.ts @@ -99,6 +99,11 @@ export class BrowseByMetadataPageComponent implements OnInit { */ value = ''; + /** + * The authority key (may be undefined) associated with {@link #value}. + */ + authority: string; + /** * The current startsWith option (fetched and updated from query-params) */ @@ -123,11 +128,12 @@ export class BrowseByMetadataPageComponent implements OnInit { }) ).subscribe(([params, currentPage, currentSort]: [Params, PaginationComponentOptions, SortOptions]) => { this.browseId = params.id || this.defaultBrowseId; + this.authority = params.authority; this.value = +params.value || params.value || ''; this.startsWith = +params.startsWith || params.startsWith; const searchOptions = browseParamsToOptions(params, currentPage, currentSort, this.browseId); if (isNotEmpty(this.value)) { - this.updatePageWithItems(searchOptions, this.value); + this.updatePageWithItems(searchOptions, this.value, this.authority); } else { this.updatePage(searchOptions); } @@ -166,8 +172,8 @@ export class BrowseByMetadataPageComponent implements OnInit { * scope: string } * @param value The value of the browse-entry to display items for */ - updatePageWithItems(searchOptions: BrowseEntrySearchOptions, value: string) { - this.items$ = this.browseService.getBrowseItemsFor(value, searchOptions); + updatePageWithItems(searchOptions: BrowseEntrySearchOptions, value: string, authority: string) { + this.items$ = this.browseService.getBrowseItemsFor(value, authority, searchOptions); } /** diff --git a/src/app/browse-by/browse-by-title-page/browse-by-title-page.component.ts b/src/app/browse-by/browse-by-title-page/browse-by-title-page.component.ts index b3a2ceed00..a2fcce7bce 100644 --- a/src/app/browse-by/browse-by-title-page/browse-by-title-page.component.ts +++ b/src/app/browse-by/browse-by-title-page/browse-by-title-page.component.ts @@ -46,7 +46,7 @@ export class BrowseByTitlePageComponent extends BrowseByMetadataPageComponent { }) ).subscribe(([params, currentPage, currentSort]: [Params, PaginationComponentOptions, SortOptions]) => { this.browseId = params.id || this.defaultBrowseId; - this.updatePageWithItems(browseParamsToOptions(params, currentPage, currentSort, this.browseId), undefined); + this.updatePageWithItems(browseParamsToOptions(params, currentPage, currentSort, this.browseId), undefined, undefined); this.updateParent(params.scope); })); this.updateStartsWithTextOptions(); diff --git a/src/app/core/browse/browse.service.spec.ts b/src/app/core/browse/browse.service.spec.ts index a28add2e30..ac68fadb31 100644 --- a/src/app/core/browse/browse.service.spec.ts +++ b/src/app/core/browse/browse.service.spec.ts @@ -129,6 +129,7 @@ describe('BrowseService', () => { describe('getBrowseEntriesFor and findList', () => { // should contain special characters such that url encoding can be tested as well const mockAuthorName = 'Donald Smith & Sons'; + const mockAuthorityKey = 'some authority key ?=;'; beforeEach(() => { requestService = getMockRequestService(getRequestEntry$(true)); @@ -155,7 +156,7 @@ describe('BrowseService', () => { it('should call hrefOnlyDataService.findAllByHref with the expected href', () => { const expected = browseDefinitions[1]._links.items.href + '?filterValue=' + encodeURIComponent(mockAuthorName); - scheduler.schedule(() => service.getBrowseItemsFor(mockAuthorName, new BrowseEntrySearchOptions(browseDefinitions[1].id)).subscribe()); + scheduler.schedule(() => service.getBrowseItemsFor(mockAuthorName, undefined, new BrowseEntrySearchOptions(browseDefinitions[1].id)).subscribe()); scheduler.flush(); expect(getFirstUsedArgumentOfSpyMethod(hrefOnlyDataService.findAllByHref)).toBeObservable(cold('(a|)', { @@ -164,6 +165,20 @@ describe('BrowseService', () => { }); }); + describe('when getBrowseItemsFor is called with a valid filter value and authority key', () => { + it('should call hrefOnlyDataService.findAllByHref with the expected href', () => { + const expected = browseDefinitions[1]._links.items.href + + '?filterValue=' + encodeURIComponent(mockAuthorName) + + '&filterAuthority=' + encodeURIComponent(mockAuthorityKey); + + scheduler.schedule(() => service.getBrowseItemsFor(mockAuthorName, mockAuthorityKey, new BrowseEntrySearchOptions(browseDefinitions[1].id)).subscribe()); + scheduler.flush(); + + expect(getFirstUsedArgumentOfSpyMethod(hrefOnlyDataService.findAllByHref)).toBeObservable(cold('(a|)', { + a: expected + })); + }); + }); }); describe('getBrowseURLFor', () => { diff --git a/src/app/core/browse/browse.service.ts b/src/app/core/browse/browse.service.ts index ffc6f313b9..05e625d6c1 100644 --- a/src/app/core/browse/browse.service.ts +++ b/src/app/core/browse/browse.service.ts @@ -105,7 +105,7 @@ export class BrowseService { * @param options Options to narrow down your search * @returns {Observable>>} */ - getBrowseItemsFor(filterValue: string, options: BrowseEntrySearchOptions): Observable>> { + getBrowseItemsFor(filterValue: string, filterAuthority: string, options: BrowseEntrySearchOptions): Observable>> { const href$ = this.getBrowseDefinitions().pipe( getBrowseDefinitionLinks(options.metadataDefinition), hasValueOperator(), @@ -132,6 +132,9 @@ export class BrowseService { if (isNotEmpty(filterValue)) { args.push(`filterValue=${encodeURIComponent(filterValue)}`); } + if (isNotEmpty(filterAuthority)) { + args.push(`filterAuthority=${encodeURIComponent(filterAuthority)}`); + } if (isNotEmpty(args)) { href = new URLCombiner(href, `?${args.join('&')}`).toString(); } diff --git a/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.html b/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.html index e3455bf095..8577ee654c 100644 --- a/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.html +++ b/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.html @@ -1,5 +1,5 @@
- + {{object.value}} diff --git a/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.ts b/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.ts index 1b0f1eef37..cfdf561f99 100644 --- a/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.ts +++ b/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.ts @@ -15,4 +15,16 @@ import { listableObjectComponent } from '../../object-collection/shared/listable * This component is automatically used to create a list view for BrowseEntry objects when used in ObjectCollectionComponent */ @listableObjectComponent(BrowseEntry, ViewMode.ListElement) -export class BrowseEntryListElementComponent extends AbstractListableElementComponent {} +export class BrowseEntryListElementComponent extends AbstractListableElementComponent { + + /** + * Get the query params to access the item page of this browse entry. + */ + public getQueryParams(): {[param: string]: any} { + return { + value: this.object.value, + authority: !!this.object.authority ? this.object.authority : undefined, + startsWith: undefined, + }; + } +} From 8666ae74f6caf172e955220c392746ce79ae1aaa Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 13 Jan 2022 10:48:46 +0100 Subject: [PATCH 7/7] 86367: Process empty parameters fix --- .../process-page/form/process-form.component.spec.ts | 11 +++++++++++ src/app/process-page/form/process-form.component.ts | 6 +++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/app/process-page/form/process-form.component.spec.ts b/src/app/process-page/form/process-form.component.spec.ts index fce7368d54..10bc8294d3 100644 --- a/src/app/process-page/form/process-form.component.spec.ts +++ b/src/app/process-page/form/process-form.component.spec.ts @@ -87,4 +87,15 @@ describe('ProcessFormComponent', () => { component.submitForm({ controls: {} } as any); expect(scriptService.invoke).toHaveBeenCalled(); }); + + describe('when undefined parameters are provided', () => { + beforeEach(() => { + component.parameters = undefined; + }); + + it('should invoke the script with an empty array of parameters', () => { + component.submitForm({ controls: {} } as any); + expect(scriptService.invoke).toHaveBeenCalledWith(script.id, [], jasmine.anything()); + }); + }); }); diff --git a/src/app/process-page/form/process-form.component.ts b/src/app/process-page/form/process-form.component.ts index 1507fe05e9..70eb3160a8 100644 --- a/src/app/process-page/form/process-form.component.ts +++ b/src/app/process-page/form/process-form.component.ts @@ -12,6 +12,7 @@ import { Router } from '@angular/router'; import { getFirstCompletedRemoteData } from '../../core/shared/operators'; import { RemoteData } from '../../core/data/remote-data'; import { getProcessListRoute } from '../process-page-routing.paths'; +import { isEmpty } from '../../shared/empty.util'; /** * Component to create a new script @@ -35,7 +36,7 @@ export class ProcessFormComponent implements OnInit { /** * The parameter values to use to start the process */ - @Input() public parameters: ProcessParameter[]; + @Input() public parameters: ProcessParameter[] = []; /** * Optional files that are used as parameter values @@ -69,6 +70,9 @@ export class ProcessFormComponent implements OnInit { * @param form */ submitForm(form: NgForm) { + if (isEmpty(this.parameters)) { + this.parameters = []; + } if (!this.validateForm(form) || this.isRequiredMissing()) { return; }