Skip to content

Commit bba71b4

Browse files
UFAL/Fix lost CLARIN license in workflow-item edit: route-aware submission PATCH (#1301) (#1396)
ADAPT port of dtq-dev commit 50a0e30 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/<id> 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/<id> 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 / 50a0e30 (FE-4, Vlna 3).
1 parent 731311b commit bba71b4

2 files changed

Lines changed: 261 additions & 26 deletions

File tree

Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
import { CommonModule } from '@angular/common';
2+
import {
3+
ChangeDetectorRef,
4+
Component,
5+
NO_ERRORS_SCHEMA,
6+
} from '@angular/core';
7+
import {
8+
ComponentFixture,
9+
inject,
10+
TestBed,
11+
waitForAsync,
12+
} from '@angular/core/testing';
13+
import {
14+
FormsModule,
15+
ReactiveFormsModule,
16+
} from '@angular/forms';
17+
import { TranslateModule } from '@ngx-translate/core';
18+
import { of } from 'rxjs';
19+
import { SubmissionFormsConfigDataService } from 'src/app/core/config/submission-forms-config-data.service';
20+
21+
import { RemoteDataBuildService } from '../../../core/cache/builders/remote-data-build.service';
22+
import { ClarinLicenseDataService } from '../../../core/data/clarin/clarin-license-data.service';
23+
import { CollectionDataService } from '../../../core/data/collection-data.service';
24+
import { ConfigurationDataService } from '../../../core/data/configuration-data.service';
25+
import { ItemDataService } from '../../../core/data/item-data.service';
26+
import { PatchRequest } from '../../../core/data/request.models';
27+
import { RequestService } from '../../../core/data/request.service';
28+
import { JsonPatchOperationPathCombiner } from '../../../core/json-patch/builder/json-patch-operation-path-combiner';
29+
import { JsonPatchOperationsBuilder } from '../../../core/json-patch/builder/json-patch-operations-builder';
30+
import { Collection } from '../../../core/shared/collection.model';
31+
import { License } from '../../../core/shared/license.model';
32+
import { FormBuilderService } from '../../../shared/form/builder/form-builder.service';
33+
import { FormComponent } from '../../../shared/form/form.component';
34+
import { FormService } from '../../../shared/form/form.service';
35+
import { getMockFormOperationsService } from '../../../shared/mocks/form-operations-service.mock';
36+
import { getMockFormService } from '../../../shared/mocks/form-service.mock';
37+
import {
38+
mockSubmissionCollectionId,
39+
mockSubmissionId,
40+
} from '../../../shared/mocks/submission.mock';
41+
import { NotificationsService } from '../../../shared/notifications/notifications.service';
42+
import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils';
43+
import { NotificationsServiceStub } from '../../../shared/testing/notifications-service.stub';
44+
import { SectionsServiceStub } from '../../../shared/testing/sections-service.stub';
45+
import { SubmissionServiceStub } from '../../../shared/testing/submission-service.stub';
46+
import { createTestComponent } from '../../../shared/testing/utils.test';
47+
import { SubmissionService } from '../../submission.service';
48+
import { SectionFormOperationsService } from '../form/section-form-operations.service';
49+
import { SectionDataObject } from '../models/section-data.model';
50+
import { SectionsService } from '../sections.service';
51+
import { SectionsType } from '../sections-type';
52+
import { SubmissionSectionClarinLicenseComponent } from './section-license.component';
53+
54+
const collectionId = mockSubmissionCollectionId;
55+
const submissionId = mockSubmissionId;
56+
const licenseText = 'License text';
57+
const helpDeskMail = 'help@desk.mail';
58+
const mockCollection = Object.assign(new Collection(), {
59+
name: 'Community 1-Collection 1',
60+
id: collectionId,
61+
metadata: [
62+
{
63+
key: 'dc.title',
64+
language: 'en_US',
65+
value: 'Community 1-Collection 1',
66+
}],
67+
license: createSuccessfulRemoteDataObject$(Object.assign(new License(), { text: licenseText })),
68+
});
69+
70+
function getMockSubmissionFormsConfigService(): SubmissionFormsConfigDataService {
71+
return jasmine.createSpyObj('FormOperationsService', {
72+
getConfigAll: jasmine.createSpy('getConfigAll'),
73+
getConfigByHref: jasmine.createSpy('getConfigByHref'),
74+
getConfigByName: jasmine.createSpy('getConfigByName'),
75+
getConfigBySearch: jasmine.createSpy('getConfigBySearch'),
76+
});
77+
}
78+
79+
const sectionObject: SectionDataObject = {
80+
config: 'https://dspace7.4science.it/or2018/api/config/submissionforms/license',
81+
mandatory: true,
82+
data: {},
83+
errorsToShow: [],
84+
serverValidationErrors: [],
85+
header: 'submit.progressbar.describe.license',
86+
id: 'license',
87+
sectionType: SectionsType.License,
88+
};
89+
90+
describe('SubmissionSectionClarinLicenseComponent', () => {
91+
92+
const jsonPatchOpBuilder: any = jasmine.createSpyObj('operationsBuilder', {
93+
add: undefined,
94+
replace: undefined,
95+
remove: undefined,
96+
});
97+
98+
const sectionsServiceStub = new SectionsServiceStub();
99+
100+
const mockClarinDataService = jasmine.createSpyObj('ClarinDataService', {
101+
searchBy: jasmine.createSpy('searchBy'),
102+
});
103+
104+
const mockItemDataService = jasmine.createSpyObj('ItemDataService', {
105+
findByHref: jasmine.createSpy('findByHref'),
106+
});
107+
108+
const mockRdbService = jasmine.createSpyObj('RemoteDataBuildService', {
109+
buildFromRequestUUID: jasmine.createSpy('buildFromRequestUUID'),
110+
});
111+
112+
const configurationServiceSpy = jasmine.createSpyObj('configurationService', {
113+
findByPropertyName: of(helpDeskMail),
114+
});
115+
116+
const mockRequestService = jasmine.createSpyObj('RequestService', {
117+
generateRequestId: jasmine.createSpy('generateRequestId'),
118+
send: jasmine.createSpy('send'),
119+
});
120+
121+
const mockCollectionDataService = jasmine.createSpyObj('CollectionDataService', {
122+
findById: jasmine.createSpy('findById'),
123+
findByHref: jasmine.createSpy('findByHref'),
124+
});
125+
126+
beforeEach(waitForAsync(() => {
127+
TestBed.configureTestingModule({
128+
imports: [
129+
CommonModule,
130+
FormsModule,
131+
ReactiveFormsModule,
132+
TranslateModule.forRoot(),
133+
FormComponent,
134+
SubmissionSectionClarinLicenseComponent,
135+
TestComponent,
136+
],
137+
providers: [
138+
{ provide: SectionFormOperationsService, useValue: getMockFormOperationsService() },
139+
{ provide: FormService, useValue: getMockFormService() },
140+
{ provide: JsonPatchOperationsBuilder, useValue: jsonPatchOpBuilder },
141+
{ provide: SubmissionFormsConfigDataService, useValue: getMockSubmissionFormsConfigService() },
142+
{ provide: NotificationsService, useClass: NotificationsServiceStub },
143+
{ provide: SectionsService, useValue: sectionsServiceStub },
144+
{ provide: SubmissionService, useClass: SubmissionServiceStub },
145+
{ provide: CollectionDataService, useValue: mockCollectionDataService },
146+
{ provide: ClarinLicenseDataService, useValue: mockClarinDataService },
147+
{ provide: ItemDataService, useValue: mockItemDataService },
148+
{ provide: RemoteDataBuildService, useValue: mockRdbService },
149+
{ provide: ConfigurationDataService, useValue: configurationServiceSpy },
150+
{ provide: RequestService, useValue: mockRequestService },
151+
{ provide: 'collectionIdProvider', useValue: collectionId },
152+
{ provide: 'sectionDataProvider', useValue: Object.assign({}, sectionObject) },
153+
{ provide: 'submissionIdProvider', useValue: submissionId },
154+
ChangeDetectorRef,
155+
FormBuilderService,
156+
SubmissionSectionClarinLicenseComponent,
157+
],
158+
schemas: [NO_ERRORS_SCHEMA],
159+
}).compileComponents().then();
160+
}));
161+
162+
describe('', () => {
163+
let testComp: TestComponent;
164+
let testFixture: ComponentFixture<TestComponent>;
165+
166+
// synchronous beforeEach
167+
beforeEach(() => {
168+
mockCollectionDataService.findById.and.returnValue(createSuccessfulRemoteDataObject$(mockCollection));
169+
sectionsServiceStub.isSectionReadOnly.and.returnValue(of(false));
170+
sectionsServiceStub.getSectionErrors.and.returnValue(of([]));
171+
172+
const html = `
173+
<ds-submission-section-license></ds-submission-section-license>`;
174+
175+
testFixture = createTestComponent(html, TestComponent) as ComponentFixture<TestComponent>;
176+
testComp = testFixture.componentInstance;
177+
});
178+
179+
afterEach(() => {
180+
testFixture.destroy();
181+
});
182+
183+
it('should create ClarinSubmissionSectionLicenseComponent', inject([SubmissionSectionClarinLicenseComponent], (app: SubmissionSectionClarinLicenseComponent) => {
184+
expect(app).toBeDefined();
185+
}));
186+
187+
it('sendRequest should PATCH the submission object self link (route-aware, works for workflow items)',
188+
inject([SubmissionSectionClarinLicenseComponent], (app: SubmissionSectionClarinLicenseComponent) => {
189+
// Arrange: enable validation flow so sendRequest actually executes
190+
(app as any).couldShowValidationErrors = true;
191+
(app as any).sectionData = { id: 'clarin-license' } as any;
192+
(app as any).pathCombiner = new JsonPatchOperationPathCombiner('sections', 'clarin-license');
193+
194+
const wsiId = 42;
195+
const selfHref = 'http://localhost/api/submission/workspaceitems/' + wsiId;
196+
197+
// The component now resolves the current submission object and PATCHes its
198+
// self link directly, so stub getActualSubmissionItem with a succeeded
199+
// RemoteData exposing _links.self.href.
200+
spyOn(app as any, 'getActualSubmissionItem').and.returnValue(
201+
Promise.resolve({ hasSucceeded: true, payload: { _links: { self: { href: selfHref } } } }),
202+
);
203+
spyOn(app as any, 'updateSectionStatus').and.callFake(() => undefined);
204+
205+
mockRequestService.generateRequestId.and.returnValue('req-id-1');
206+
mockRequestService.send.calls.reset();
207+
mockRdbService.buildFromRequestUUID.and.returnValue(of({ payload: { sections: {}, errors: [] } } as any));
208+
209+
// Act
210+
return (app as any).sendRequest('My CLARIN License').then(() => {
211+
// Assert
212+
expect(mockRequestService.send).toHaveBeenCalledTimes(1);
213+
const sentRequest = mockRequestService.send.calls.mostRecent().args[0] as PatchRequest;
214+
expect(sentRequest.href).toBe(selfHref);
215+
const body: any[] = (sentRequest as any).body;
216+
expect(body.length).toBe(1);
217+
expect(body[0].op).toBe('replace');
218+
expect(body[0].path).toBe('/sections/clarin-license/select');
219+
expect(body[0].value).toBe('My CLARIN License');
220+
});
221+
}));
222+
});
223+
});
224+
225+
// declare a test component
226+
@Component({
227+
selector: 'ds-test-cmp',
228+
template: ``,
229+
imports: [
230+
FormsModule,
231+
ReactiveFormsModule,
232+
],
233+
})
234+
class TestComponent {
235+
236+
}

src/app/submission/sections/clarin-license-resource/section-license.component.ts

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import {
2121
import {
2222
distinctUntilChanged,
2323
filter,
24-
find,
2524
} from 'rxjs/operators';
2625
import { FindListOptions } from 'src/app/core/data/find-list-options.model';
2726
import { hasFailed } from 'src/app/core/data/request-entry-state.model';
@@ -39,16 +38,14 @@ import { JsonPatchOperationPathCombiner } from '../../../core/json-patch/builder
3938
import { JsonPatchOperationsBuilder } from '../../../core/json-patch/builder/json-patch-operations-builder';
4039
import { ClarinLicense } from '../../../core/shared/clarin/clarin-license.model';
4140
import { ConfigurationProperty } from '../../../core/shared/configuration-property.model';
42-
import { HALEndpointService } from '../../../core/shared/hal-endpoint.service';
4341
import { Item } from '../../../core/shared/item.model';
4442
import { MetadataValue } from '../../../core/shared/metadata.models';
4543
import {
4644
getFirstCompletedRemoteData,
4745
getFirstSucceededRemoteListPayload,
4846
} from '../../../core/shared/operators';
49-
import { WorkspaceItem } from '../../../core/submission/models/workspaceitem.model';
47+
import { SubmissionObject } from '../../../core/submission/models/submission-object.model';
5048
import { normalizeSectionData } from '../../../core/submission/submission-response-parsing.service';
51-
import { WorkspaceitemDataService } from '../../../core/submission/workspaceitem-data.service';
5249
import { HELP_DESK_PROPERTY } from '../../../item-page/tombstone/tombstone.constants';
5350
import {
5451
hasValue,
@@ -59,6 +56,7 @@ import {
5956
isUndefined,
6057
} from '../../../shared/empty.util';
6158
import { FormService } from '../../../shared/form/form.service';
59+
import { SubmissionService } from '../../submission.service';
6260
import parseSectionErrors from '../../utils/parseSectionErrors';
6361
import { SectionModelComponent } from '../models/section.model';
6462
import { SectionDataObject } from '../models/section-data.model';
@@ -166,8 +164,7 @@ export class SubmissionSectionClarinLicenseComponent extends SectionModelCompone
166164
* @param clarinLicenseService
167165
* @param translateService
168166
* @param itemService
169-
* @param workspaceItemService
170-
* @param halService
167+
* @param submissionService
171168
* @param rdbService
172169
* @param configurationDataService
173170
* @param requestService
@@ -182,8 +179,7 @@ export class SubmissionSectionClarinLicenseComponent extends SectionModelCompone
182179
protected clarinLicenseService: ClarinLicenseDataService,
183180
protected translateService: TranslateService,
184181
protected itemService: ItemDataService,
185-
protected workspaceItemService: WorkspaceitemDataService,
186-
protected halService: HALEndpointService,
182+
protected submissionService: SubmissionService,
187183
protected rdbService: RemoteDataBuildService,
188184
private configurationDataService: ConfigurationDataService,
189185
protected requestService: RequestService,
@@ -227,11 +223,17 @@ export class SubmissionSectionClarinLicenseComponent extends SectionModelCompone
227223
this.formId = this.formService.getUniqueId(this.sectionData.id);
228224

229225
// Load the accepted license of the item
230-
this.getActualWorkspaceItem()
231-
.then((workspaceItemRD: RemoteData<WorkspaceItem>) => {
232-
this.itemService.findByHref(workspaceItemRD.payload._links.item.href)
226+
this.getActualSubmissionItem()
227+
.then((submissionItemRD: RemoteData<SubmissionObject>) => {
228+
if (!submissionItemRD?.hasSucceeded || !submissionItemRD.payload?._links?.item?.href) {
229+
return;
230+
}
231+
this.itemService.findByHref(submissionItemRD.payload._links.item.href)
233232
.pipe(getFirstCompletedRemoteData())
234233
.subscribe((itemRD: RemoteData<Item>) => {
234+
if (!itemRD?.hasSucceeded || !itemRD.payload) {
235+
return;
236+
}
235237
// Load the metadata where is store clarin license name (`dc.rights`).
236238
const item = itemRD.payload;
237239
const dcRightsMetadata = item.metadata['dc.rights'];
@@ -304,29 +306,25 @@ export class SubmissionSectionClarinLicenseComponent extends SectionModelCompone
304306
}
305307

306308
this.updateSectionStatus();
307-
await this.getActualWorkspaceItem()
308-
.then(workspaceItemRD => {
309+
await this.getActualSubmissionItem()
310+
.then(submissionItemRD => {
311+
if (!submissionItemRD?.hasSucceeded || !submissionItemRD.payload?._links?.self?.href) {
312+
return;
313+
}
309314
const requestId = this.requestService.generateRequestId();
310-
const hrefObs = this.halService.getEndpoint(this.workspaceItemService.getLinkPath());
311-
312315
// Route the PATCH through the `clarin-license` submission step so it
313316
// works for workflow items too and keeps `sections.license` (CC) and
314317
// `sections.clarin-license` payloads separate on subsequent GETs.
315318
const patchOperation2 = {
316319
op: 'replace', path: this.pathCombiner.getPath('select').path, value: licenseNameRest,
317320
} as Operation;
318-
319-
hrefObs.pipe(
320-
find((href: string) => hasValue(href)),
321-
).subscribe((href: string) => {
322-
const request = new PatchRequest(requestId, href + '/' + workspaceItemRD.payload.id, [patchOperation2]);
323-
this.requestService.send(request);
324-
});
321+
const request = new PatchRequest(requestId, submissionItemRD.payload._links.self.href, [patchOperation2]);
322+
this.requestService.send(request);
325323

326324
// process the response
327325
this.rdbService.buildFromRequestUUID(requestId)
328326
.pipe(getFirstCompletedRemoteData())
329-
.subscribe((response: RemoteData<WorkspaceItem>) => {
327+
.subscribe((response: RemoteData<SubmissionObject>) => {
330328

331329
// show validation errors in every section
332330
const workspaceitem = response.payload;
@@ -496,10 +494,11 @@ export class SubmissionSectionClarinLicenseComponent extends SectionModelCompone
496494
}
497495

498496
/**
499-
* Get the current workspace item by the submissionId.
497+
* Get the current submission item (workspace or workflow) by the submissionId.
498+
* This method is route-aware and retrieves from the appropriate endpoint based on the current URL.
500499
*/
501-
private async getActualWorkspaceItem(): Promise<RemoteData<WorkspaceItem>> {
502-
return this.workspaceItemService.findById(this.submissionId)
500+
private async getActualSubmissionItem(): Promise<RemoteData<SubmissionObject>> {
501+
return this.submissionService.retrieveSubmission(this.submissionId)
503502
.pipe(getFirstCompletedRemoteData()).toPromise();
504503
}
505504

0 commit comments

Comments
 (0)