From 55ec542b2ef8dac930bd7fbbf04f73b8bd0fef7b Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Mon, 20 Jul 2026 22:00:11 +0200 Subject: [PATCH] UFAL/Fix lost CLARIN license in workflow-item edit: route-aware submission PATCH (#1301) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADAPT port of dtq-dev commit 50a0e3081e onto dtq-dev-9-base. Problem: the CLARIN license submission section always resolved the in-progress item via WorkspaceitemDataService.findById and PATCHed the hardcoded /api/submission/workspaceitems/ endpoint, so a reviewer editing a WORKFLOW item could not load the accepted license nor save a new one. Fix: replace WorkspaceitemDataService + HALEndpointService with the route-aware SubmissionService.retrieveSubmission (getActualWorkspaceItem -> getActualSubmissionItem returning SubmissionObject), send the sections/clarin-license/select PATCH to the submission object's own _links.self.href (which is /api/workflow/workflowitems/ for workflow items), and add hasSucceeded/null guards on both RemoteData resolutions. v9 adaptations: - File lives at submission/sections/clarin-license-resource/ on v9 (not clarin-license/). - Removed now-unused imports (WorkspaceItem, WorkspaceitemDataService, HALEndpointService, rxjs find); added SubmissionService + SubmissionObject in eslint-sorted positions. .toPromise() kept (compiles on v9-base; diff-minimal). - Spec: v9-base had dropped this spec entirely; rewritten from the dtq-dev spec into v9 standalone TestBed style (imports[] not declarations, standalone TestComponent). Karma: 2/2 green incl. the assertion that sendRequest PATCHes the submission self link (route-aware, works for workflow items). Not pulled in: BE twin 8320889f2c (#1327) — separate BE-4 tranža. Happy path (claimed task) already works on today's v9-base BE via OPERATION_PATH_SECTIONS -> ClarinLicenseResourceStep; full 403/404 error-semantics parity is the BE-4 pass. Fulfils CLARIN_V9_POST_SNAPSHOT_SYNC_ACCEPTANCE.md §5 / 50a0e3081e (FE-4, Vlna 3). --- .../section-license.component.spec.ts | 236 ++++++++++++++++++ .../section-license.component.ts | 51 ++-- 2 files changed, 261 insertions(+), 26 deletions(-) create mode 100644 src/app/submission/sections/clarin-license-resource/section-license.component.spec.ts diff --git a/src/app/submission/sections/clarin-license-resource/section-license.component.spec.ts b/src/app/submission/sections/clarin-license-resource/section-license.component.spec.ts new file mode 100644 index 00000000000..5dfe4b566b6 --- /dev/null +++ b/src/app/submission/sections/clarin-license-resource/section-license.component.spec.ts @@ -0,0 +1,236 @@ +import { CommonModule } from '@angular/common'; +import { + ChangeDetectorRef, + Component, + NO_ERRORS_SCHEMA, +} from '@angular/core'; +import { + ComponentFixture, + inject, + TestBed, + waitForAsync, +} from '@angular/core/testing'; +import { + FormsModule, + ReactiveFormsModule, +} from '@angular/forms'; +import { TranslateModule } from '@ngx-translate/core'; +import { of } from 'rxjs'; +import { SubmissionFormsConfigDataService } from 'src/app/core/config/submission-forms-config-data.service'; + +import { RemoteDataBuildService } from '../../../core/cache/builders/remote-data-build.service'; +import { ClarinLicenseDataService } from '../../../core/data/clarin/clarin-license-data.service'; +import { CollectionDataService } from '../../../core/data/collection-data.service'; +import { ConfigurationDataService } from '../../../core/data/configuration-data.service'; +import { ItemDataService } from '../../../core/data/item-data.service'; +import { PatchRequest } from '../../../core/data/request.models'; +import { RequestService } from '../../../core/data/request.service'; +import { JsonPatchOperationPathCombiner } from '../../../core/json-patch/builder/json-patch-operation-path-combiner'; +import { JsonPatchOperationsBuilder } from '../../../core/json-patch/builder/json-patch-operations-builder'; +import { Collection } from '../../../core/shared/collection.model'; +import { License } from '../../../core/shared/license.model'; +import { FormBuilderService } from '../../../shared/form/builder/form-builder.service'; +import { FormComponent } from '../../../shared/form/form.component'; +import { FormService } from '../../../shared/form/form.service'; +import { getMockFormOperationsService } from '../../../shared/mocks/form-operations-service.mock'; +import { getMockFormService } from '../../../shared/mocks/form-service.mock'; +import { + mockSubmissionCollectionId, + mockSubmissionId, +} from '../../../shared/mocks/submission.mock'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; +import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; +import { NotificationsServiceStub } from '../../../shared/testing/notifications-service.stub'; +import { SectionsServiceStub } from '../../../shared/testing/sections-service.stub'; +import { SubmissionServiceStub } from '../../../shared/testing/submission-service.stub'; +import { createTestComponent } from '../../../shared/testing/utils.test'; +import { SubmissionService } from '../../submission.service'; +import { SectionFormOperationsService } from '../form/section-form-operations.service'; +import { SectionDataObject } from '../models/section-data.model'; +import { SectionsService } from '../sections.service'; +import { SectionsType } from '../sections-type'; +import { SubmissionSectionClarinLicenseComponent } from './section-license.component'; + +const collectionId = mockSubmissionCollectionId; +const submissionId = mockSubmissionId; +const licenseText = 'License text'; +const helpDeskMail = 'help@desk.mail'; +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', + }], + license: createSuccessfulRemoteDataObject$(Object.assign(new License(), { text: licenseText })), +}); + +function getMockSubmissionFormsConfigService(): SubmissionFormsConfigDataService { + return jasmine.createSpyObj('FormOperationsService', { + getConfigAll: jasmine.createSpy('getConfigAll'), + getConfigByHref: jasmine.createSpy('getConfigByHref'), + getConfigByName: jasmine.createSpy('getConfigByName'), + getConfigBySearch: jasmine.createSpy('getConfigBySearch'), + }); +} + +const sectionObject: SectionDataObject = { + config: 'https://dspace7.4science.it/or2018/api/config/submissionforms/license', + mandatory: true, + data: {}, + errorsToShow: [], + serverValidationErrors: [], + header: 'submit.progressbar.describe.license', + id: 'license', + sectionType: SectionsType.License, +}; + +describe('SubmissionSectionClarinLicenseComponent', () => { + + const jsonPatchOpBuilder: any = jasmine.createSpyObj('operationsBuilder', { + add: undefined, + replace: undefined, + remove: undefined, + }); + + const sectionsServiceStub = new SectionsServiceStub(); + + const mockClarinDataService = jasmine.createSpyObj('ClarinDataService', { + searchBy: jasmine.createSpy('searchBy'), + }); + + const mockItemDataService = jasmine.createSpyObj('ItemDataService', { + findByHref: jasmine.createSpy('findByHref'), + }); + + const mockRdbService = jasmine.createSpyObj('RemoteDataBuildService', { + buildFromRequestUUID: jasmine.createSpy('buildFromRequestUUID'), + }); + + const configurationServiceSpy = jasmine.createSpyObj('configurationService', { + findByPropertyName: of(helpDeskMail), + }); + + const mockRequestService = jasmine.createSpyObj('RequestService', { + generateRequestId: jasmine.createSpy('generateRequestId'), + send: jasmine.createSpy('send'), + }); + + const mockCollectionDataService = jasmine.createSpyObj('CollectionDataService', { + findById: jasmine.createSpy('findById'), + findByHref: jasmine.createSpy('findByHref'), + }); + + beforeEach(waitForAsync(() => { + TestBed.configureTestingModule({ + imports: [ + CommonModule, + FormsModule, + ReactiveFormsModule, + TranslateModule.forRoot(), + FormComponent, + SubmissionSectionClarinLicenseComponent, + TestComponent, + ], + providers: [ + { provide: SectionFormOperationsService, useValue: getMockFormOperationsService() }, + { provide: FormService, useValue: getMockFormService() }, + { provide: JsonPatchOperationsBuilder, useValue: jsonPatchOpBuilder }, + { provide: SubmissionFormsConfigDataService, useValue: getMockSubmissionFormsConfigService() }, + { provide: NotificationsService, useClass: NotificationsServiceStub }, + { provide: SectionsService, useValue: sectionsServiceStub }, + { provide: SubmissionService, useClass: SubmissionServiceStub }, + { provide: CollectionDataService, useValue: mockCollectionDataService }, + { provide: ClarinLicenseDataService, useValue: mockClarinDataService }, + { provide: ItemDataService, useValue: mockItemDataService }, + { provide: RemoteDataBuildService, useValue: mockRdbService }, + { provide: ConfigurationDataService, useValue: configurationServiceSpy }, + { provide: RequestService, useValue: mockRequestService }, + { provide: 'collectionIdProvider', useValue: collectionId }, + { provide: 'sectionDataProvider', useValue: Object.assign({}, sectionObject) }, + { provide: 'submissionIdProvider', useValue: submissionId }, + ChangeDetectorRef, + FormBuilderService, + SubmissionSectionClarinLicenseComponent, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents().then(); + })); + + describe('', () => { + let testComp: TestComponent; + let testFixture: ComponentFixture; + + // synchronous beforeEach + beforeEach(() => { + mockCollectionDataService.findById.and.returnValue(createSuccessfulRemoteDataObject$(mockCollection)); + sectionsServiceStub.isSectionReadOnly.and.returnValue(of(false)); + sectionsServiceStub.getSectionErrors.and.returnValue(of([])); + + const html = ` + `; + + testFixture = createTestComponent(html, TestComponent) as ComponentFixture; + testComp = testFixture.componentInstance; + }); + + afterEach(() => { + testFixture.destroy(); + }); + + it('should create ClarinSubmissionSectionLicenseComponent', inject([SubmissionSectionClarinLicenseComponent], (app: SubmissionSectionClarinLicenseComponent) => { + expect(app).toBeDefined(); + })); + + it('sendRequest should PATCH the submission object self link (route-aware, works for workflow items)', + inject([SubmissionSectionClarinLicenseComponent], (app: SubmissionSectionClarinLicenseComponent) => { + // Arrange: enable validation flow so sendRequest actually executes + (app as any).couldShowValidationErrors = true; + (app as any).sectionData = { id: 'clarin-license' } as any; + (app as any).pathCombiner = new JsonPatchOperationPathCombiner('sections', 'clarin-license'); + + const wsiId = 42; + const selfHref = 'http://localhost/api/submission/workspaceitems/' + wsiId; + + // The component now resolves the current submission object and PATCHes its + // self link directly, so stub getActualSubmissionItem with a succeeded + // RemoteData exposing _links.self.href. + spyOn(app as any, 'getActualSubmissionItem').and.returnValue( + Promise.resolve({ hasSucceeded: true, payload: { _links: { self: { href: selfHref } } } }), + ); + spyOn(app as any, 'updateSectionStatus').and.callFake(() => undefined); + + mockRequestService.generateRequestId.and.returnValue('req-id-1'); + mockRequestService.send.calls.reset(); + mockRdbService.buildFromRequestUUID.and.returnValue(of({ payload: { sections: {}, errors: [] } } as any)); + + // Act + return (app as any).sendRequest('My CLARIN License').then(() => { + // Assert + expect(mockRequestService.send).toHaveBeenCalledTimes(1); + const sentRequest = mockRequestService.send.calls.mostRecent().args[0] as PatchRequest; + expect(sentRequest.href).toBe(selfHref); + const body: any[] = (sentRequest as any).body; + expect(body.length).toBe(1); + expect(body[0].op).toBe('replace'); + expect(body[0].path).toBe('/sections/clarin-license/select'); + expect(body[0].value).toBe('My CLARIN License'); + }); + })); + }); +}); + +// declare a test component +@Component({ + selector: 'ds-test-cmp', + template: ``, + imports: [ + FormsModule, + ReactiveFormsModule, + ], +}) +class TestComponent { + +} diff --git a/src/app/submission/sections/clarin-license-resource/section-license.component.ts b/src/app/submission/sections/clarin-license-resource/section-license.component.ts index 563248d3a4f..6c69b362fee 100644 --- a/src/app/submission/sections/clarin-license-resource/section-license.component.ts +++ b/src/app/submission/sections/clarin-license-resource/section-license.component.ts @@ -21,7 +21,6 @@ import { import { distinctUntilChanged, filter, - find, } from 'rxjs/operators'; import { FindListOptions } from 'src/app/core/data/find-list-options.model'; import { hasFailed } from 'src/app/core/data/request-entry-state.model'; @@ -39,16 +38,14 @@ import { JsonPatchOperationPathCombiner } from '../../../core/json-patch/builder import { JsonPatchOperationsBuilder } from '../../../core/json-patch/builder/json-patch-operations-builder'; import { ClarinLicense } from '../../../core/shared/clarin/clarin-license.model'; import { ConfigurationProperty } from '../../../core/shared/configuration-property.model'; -import { HALEndpointService } from '../../../core/shared/hal-endpoint.service'; import { Item } from '../../../core/shared/item.model'; import { MetadataValue } from '../../../core/shared/metadata.models'; import { getFirstCompletedRemoteData, getFirstSucceededRemoteListPayload, } from '../../../core/shared/operators'; -import { WorkspaceItem } from '../../../core/submission/models/workspaceitem.model'; +import { SubmissionObject } from '../../../core/submission/models/submission-object.model'; import { normalizeSectionData } from '../../../core/submission/submission-response-parsing.service'; -import { WorkspaceitemDataService } from '../../../core/submission/workspaceitem-data.service'; import { HELP_DESK_PROPERTY } from '../../../item-page/tombstone/tombstone.constants'; import { hasValue, @@ -59,6 +56,7 @@ import { isUndefined, } from '../../../shared/empty.util'; import { FormService } from '../../../shared/form/form.service'; +import { SubmissionService } from '../../submission.service'; import parseSectionErrors from '../../utils/parseSectionErrors'; import { SectionModelComponent } from '../models/section.model'; import { SectionDataObject } from '../models/section-data.model'; @@ -166,8 +164,7 @@ export class SubmissionSectionClarinLicenseComponent extends SectionModelCompone * @param clarinLicenseService * @param translateService * @param itemService - * @param workspaceItemService - * @param halService + * @param submissionService * @param rdbService * @param configurationDataService * @param requestService @@ -182,8 +179,7 @@ export class SubmissionSectionClarinLicenseComponent extends SectionModelCompone protected clarinLicenseService: ClarinLicenseDataService, protected translateService: TranslateService, protected itemService: ItemDataService, - protected workspaceItemService: WorkspaceitemDataService, - protected halService: HALEndpointService, + protected submissionService: SubmissionService, protected rdbService: RemoteDataBuildService, private configurationDataService: ConfigurationDataService, protected requestService: RequestService, @@ -227,11 +223,17 @@ export class SubmissionSectionClarinLicenseComponent extends SectionModelCompone this.formId = this.formService.getUniqueId(this.sectionData.id); // Load the accepted license of the item - this.getActualWorkspaceItem() - .then((workspaceItemRD: RemoteData) => { - this.itemService.findByHref(workspaceItemRD.payload._links.item.href) + this.getActualSubmissionItem() + .then((submissionItemRD: RemoteData) => { + if (!submissionItemRD?.hasSucceeded || !submissionItemRD.payload?._links?.item?.href) { + return; + } + this.itemService.findByHref(submissionItemRD.payload._links.item.href) .pipe(getFirstCompletedRemoteData()) .subscribe((itemRD: RemoteData) => { + if (!itemRD?.hasSucceeded || !itemRD.payload) { + return; + } // Load the metadata where is store clarin license name (`dc.rights`). const item = itemRD.payload; const dcRightsMetadata = item.metadata['dc.rights']; @@ -304,29 +306,25 @@ export class SubmissionSectionClarinLicenseComponent extends SectionModelCompone } this.updateSectionStatus(); - await this.getActualWorkspaceItem() - .then(workspaceItemRD => { + await this.getActualSubmissionItem() + .then(submissionItemRD => { + if (!submissionItemRD?.hasSucceeded || !submissionItemRD.payload?._links?.self?.href) { + return; + } const requestId = this.requestService.generateRequestId(); - const hrefObs = this.halService.getEndpoint(this.workspaceItemService.getLinkPath()); - // Route the PATCH through the `clarin-license` submission step so it // works for workflow items too and keeps `sections.license` (CC) and // `sections.clarin-license` payloads separate on subsequent GETs. const patchOperation2 = { op: 'replace', path: this.pathCombiner.getPath('select').path, value: licenseNameRest, } as Operation; - - hrefObs.pipe( - find((href: string) => hasValue(href)), - ).subscribe((href: string) => { - const request = new PatchRequest(requestId, href + '/' + workspaceItemRD.payload.id, [patchOperation2]); - this.requestService.send(request); - }); + const request = new PatchRequest(requestId, submissionItemRD.payload._links.self.href, [patchOperation2]); + this.requestService.send(request); // process the response this.rdbService.buildFromRequestUUID(requestId) .pipe(getFirstCompletedRemoteData()) - .subscribe((response: RemoteData) => { + .subscribe((response: RemoteData) => { // show validation errors in every section const workspaceitem = response.payload; @@ -496,10 +494,11 @@ export class SubmissionSectionClarinLicenseComponent extends SectionModelCompone } /** - * Get the current workspace item by the submissionId. + * Get the current submission item (workspace or workflow) by the submissionId. + * This method is route-aware and retrieves from the appropriate endpoint based on the current URL. */ - private async getActualWorkspaceItem(): Promise> { - return this.workspaceItemService.findById(this.submissionId) + private async getActualSubmissionItem(): Promise> { + return this.submissionService.retrieveSubmission(this.submissionId) .pipe(getFirstCompletedRemoteData()).toPromise(); }