Skip to content

Commit 24381ed

Browse files
vatsalparikhvatsalparikh
committed
fix(davinci-client): address PR review comments on ImageCollector
1 parent c1803d9 commit 24381ed

10 files changed

Lines changed: 119 additions & 61 deletions

File tree

‎.changeset/curly-wolves-swim.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
'@forgerock/davinci-client': minor
33
---
44

5-
Add `ImageCollector` for rendering `IMAGE` form fields from DaVinci Forms.
5+
Add `ImageCollector` for rendering `IMAGE` form fields from PingOne Forms.

‎e2e/davinci-app/components/form-image.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2025 - 2026 Ping Identity Corporation. All rights reserved.
2+
* Copyright (c) 2026 Ping Identity Corporation. All rights reserved.
33
*
44
* This software may be modified and distributed under the terms
55
* of the MIT license. See the LICENSE file for details.

‎e2e/davinci-suites/src/form-image.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2025 - 2026 Ping Identity Corporation. All rights reserved.
2+
* Copyright (c) 2026 Ping Identity Corporation. All rights reserved.
33
*
44
* This software may be modified and distributed under the terms
55
* of the MIT license. See the LICENSE file for details.

‎packages/davinci-client/api-report/davinci-client.api.md‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -283,16 +283,12 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
283283
resume: (input: {
284284
continueToken: string;
285285
}) => Promise<InternalErrorResponse | NodeStates>;
286-
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<ContinueNode | ErrorNode | FailureNode | StartNode | SuccessNode>;
286+
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<StartNode | ErrorNode | FailureNode | ContinueNode | SuccessNode>;
287287
update: <T extends SingleValueCollectors | MultiSelectCollector | ObjectValueCollectors | AutoCollectors>(collector: T) => Updater<T>;
288288
validate: (collector: SingleValueCollectors | ObjectValueCollectors | MultiValueCollectors | AutoCollectors) => Validator;
289289
pollStatus: (collector: PollingCollector) => Poller;
290290
getClient: () => {
291-
action: string;
292-
collectors: Collectors[];
293-
description?: string;
294-
name?: string;
295-
status: "continue";
291+
status: "start";
296292
} | {
297293
action: string;
298294
collectors: Collectors[];
@@ -302,7 +298,11 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
302298
} | {
303299
status: "failure";
304300
} | {
305-
status: "start";
301+
action: string;
302+
collectors: Collectors[];
303+
description?: string;
304+
name?: string;
305+
status: "continue";
306306
} | {
307307
authorization?: {
308308
code?: string;
@@ -313,15 +313,9 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
313313
getCollectors: () => Collectors[];
314314
getError: () => DaVinciError | null;
315315
getErrorCollectors: () => CollectorErrors[];
316-
getNode: () => ContinueNode | ErrorNode | FailureNode | StartNode | SuccessNode;
316+
getNode: () => StartNode | ErrorNode | FailureNode | ContinueNode | SuccessNode;
317317
getServer: () => {
318-
_links?: Links;
319-
id?: string;
320-
interactionId?: string;
321-
interactionToken?: string;
322-
href?: string;
323-
eventName?: string;
324-
status: "continue";
318+
status: "start";
325319
} | {
326320
_links?: Links;
327321
eventName?: string;
@@ -338,7 +332,13 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
338332
interactionToken?: string;
339333
status: "failure";
340334
} | {
341-
status: "start";
335+
_links?: Links;
336+
id?: string;
337+
interactionId?: string;
338+
interactionToken?: string;
339+
href?: string;
340+
eventName?: string;
341+
status: "continue";
342342
} | {
343343
_links?: Links;
344344
eventName?: string;
@@ -355,14 +355,14 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
355355
} & Omit<{
356356
requestId: string;
357357
data?: unknown;
358-
error?: FetchBaseQueryError | SerializedError | undefined;
358+
error?: SerializedError | FetchBaseQueryError | undefined;
359359
endpointName: string;
360360
startedTimeStamp: number;
361361
fulfilledTimeStamp?: number;
362362
}, "data" | "fulfilledTimeStamp"> & Required<Pick<{
363363
requestId: string;
364364
data?: unknown;
365-
error?: FetchBaseQueryError | SerializedError | undefined;
365+
error?: SerializedError | FetchBaseQueryError | undefined;
366366
endpointName: string;
367367
startedTimeStamp: number;
368368
fulfilledTimeStamp?: number;
@@ -379,14 +379,14 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
379379
} & Omit<{
380380
requestId: string;
381381
data?: unknown;
382-
error?: FetchBaseQueryError | SerializedError | undefined;
382+
error?: SerializedError | FetchBaseQueryError | undefined;
383383
endpointName: string;
384384
startedTimeStamp: number;
385385
fulfilledTimeStamp?: number;
386386
}, "error"> & Required<Pick<{
387387
requestId: string;
388388
data?: unknown;
389-
error?: FetchBaseQueryError | SerializedError | undefined;
389+
error?: SerializedError | FetchBaseQueryError | undefined;
390390
endpointName: string;
391391
startedTimeStamp: number;
392392
fulfilledTimeStamp?: number;
@@ -407,14 +407,14 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
407407
} & Omit<{
408408
requestId: string;
409409
data?: unknown;
410-
error?: FetchBaseQueryError | SerializedError | undefined;
410+
error?: SerializedError | FetchBaseQueryError | undefined;
411411
endpointName: string;
412412
startedTimeStamp: number;
413413
fulfilledTimeStamp?: number;
414414
}, "data" | "fulfilledTimeStamp"> & Required<Pick<{
415415
requestId: string;
416416
data?: unknown;
417-
error?: FetchBaseQueryError | SerializedError | undefined;
417+
error?: SerializedError | FetchBaseQueryError | undefined;
418418
endpointName: string;
419419
startedTimeStamp: number;
420420
fulfilledTimeStamp?: number;
@@ -431,14 +431,14 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
431431
} & Omit<{
432432
requestId: string;
433433
data?: unknown;
434-
error?: FetchBaseQueryError | SerializedError | undefined;
434+
error?: SerializedError | FetchBaseQueryError | undefined;
435435
endpointName: string;
436436
startedTimeStamp: number;
437437
fulfilledTimeStamp?: number;
438438
}, "error"> & Required<Pick<{
439439
requestId: string;
440440
data?: unknown;
441-
error?: FetchBaseQueryError | SerializedError | undefined;
441+
error?: SerializedError | FetchBaseQueryError | undefined;
442442
endpointName: string;
443443
startedTimeStamp: number;
444444
fulfilledTimeStamp?: number;

‎packages/davinci-client/api-report/davinci-client.types.api.md‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -283,16 +283,12 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
283283
resume: (input: {
284284
continueToken: string;
285285
}) => Promise<InternalErrorResponse | NodeStates>;
286-
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<ContinueNode | ErrorNode | FailureNode | StartNode | SuccessNode>;
286+
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<StartNode | ErrorNode | FailureNode | ContinueNode | SuccessNode>;
287287
update: <T extends SingleValueCollectors | MultiSelectCollector | ObjectValueCollectors | AutoCollectors>(collector: T) => Updater<T>;
288288
validate: (collector: SingleValueCollectors | ObjectValueCollectors | MultiValueCollectors | AutoCollectors) => Validator;
289289
pollStatus: (collector: PollingCollector) => Poller;
290290
getClient: () => {
291-
action: string;
292-
collectors: Collectors[];
293-
description?: string;
294-
name?: string;
295-
status: "continue";
291+
status: "start";
296292
} | {
297293
action: string;
298294
collectors: Collectors[];
@@ -302,7 +298,11 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
302298
} | {
303299
status: "failure";
304300
} | {
305-
status: "start";
301+
action: string;
302+
collectors: Collectors[];
303+
description?: string;
304+
name?: string;
305+
status: "continue";
306306
} | {
307307
authorization?: {
308308
code?: string;
@@ -313,15 +313,9 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
313313
getCollectors: () => Collectors[];
314314
getError: () => DaVinciError | null;
315315
getErrorCollectors: () => CollectorErrors[];
316-
getNode: () => ContinueNode | ErrorNode | FailureNode | StartNode | SuccessNode;
316+
getNode: () => StartNode | ErrorNode | FailureNode | ContinueNode | SuccessNode;
317317
getServer: () => {
318-
_links?: Links;
319-
id?: string;
320-
interactionId?: string;
321-
interactionToken?: string;
322-
href?: string;
323-
eventName?: string;
324-
status: "continue";
318+
status: "start";
325319
} | {
326320
_links?: Links;
327321
eventName?: string;
@@ -338,7 +332,13 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
338332
interactionToken?: string;
339333
status: "failure";
340334
} | {
341-
status: "start";
335+
_links?: Links;
336+
id?: string;
337+
interactionId?: string;
338+
interactionToken?: string;
339+
href?: string;
340+
eventName?: string;
341+
status: "continue";
342342
} | {
343343
_links?: Links;
344344
eventName?: string;
@@ -355,14 +355,14 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
355355
} & Omit<{
356356
requestId: string;
357357
data?: unknown;
358-
error?: FetchBaseQueryError | SerializedError | undefined;
358+
error?: SerializedError | FetchBaseQueryError | undefined;
359359
endpointName: string;
360360
startedTimeStamp: number;
361361
fulfilledTimeStamp?: number;
362362
}, "data" | "fulfilledTimeStamp"> & Required<Pick<{
363363
requestId: string;
364364
data?: unknown;
365-
error?: FetchBaseQueryError | SerializedError | undefined;
365+
error?: SerializedError | FetchBaseQueryError | undefined;
366366
endpointName: string;
367367
startedTimeStamp: number;
368368
fulfilledTimeStamp?: number;
@@ -379,14 +379,14 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
379379
} & Omit<{
380380
requestId: string;
381381
data?: unknown;
382-
error?: FetchBaseQueryError | SerializedError | undefined;
382+
error?: SerializedError | FetchBaseQueryError | undefined;
383383
endpointName: string;
384384
startedTimeStamp: number;
385385
fulfilledTimeStamp?: number;
386386
}, "error"> & Required<Pick<{
387387
requestId: string;
388388
data?: unknown;
389-
error?: FetchBaseQueryError | SerializedError | undefined;
389+
error?: SerializedError | FetchBaseQueryError | undefined;
390390
endpointName: string;
391391
startedTimeStamp: number;
392392
fulfilledTimeStamp?: number;
@@ -407,14 +407,14 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
407407
} & Omit<{
408408
requestId: string;
409409
data?: unknown;
410-
error?: FetchBaseQueryError | SerializedError | undefined;
410+
error?: SerializedError | FetchBaseQueryError | undefined;
411411
endpointName: string;
412412
startedTimeStamp: number;
413413
fulfilledTimeStamp?: number;
414414
}, "data" | "fulfilledTimeStamp"> & Required<Pick<{
415415
requestId: string;
416416
data?: unknown;
417-
error?: FetchBaseQueryError | SerializedError | undefined;
417+
error?: SerializedError | FetchBaseQueryError | undefined;
418418
endpointName: string;
419419
startedTimeStamp: number;
420420
fulfilledTimeStamp?: number;
@@ -431,14 +431,14 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
431431
} & Omit<{
432432
requestId: string;
433433
data?: unknown;
434-
error?: FetchBaseQueryError | SerializedError | undefined;
434+
error?: SerializedError | FetchBaseQueryError | undefined;
435435
endpointName: string;
436436
startedTimeStamp: number;
437437
fulfilledTimeStamp?: number;
438438
}, "error"> & Required<Pick<{
439439
requestId: string;
440440
data?: unknown;
441-
error?: FetchBaseQueryError | SerializedError | undefined;
441+
error?: SerializedError | FetchBaseQueryError | undefined;
442442
endpointName: string;
443443
startedTimeStamp: number;
444444
fulfilledTimeStamp?: number;

‎packages/davinci-client/src/lib/collector.utils.test.ts‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,17 @@ describe('No Value Collectors', () => {
932932
},
933933
});
934934
});
935+
936+
it('should not set a content error for IMAGE fields', () => {
937+
const imageField: ImageField = {
938+
type: 'IMAGE',
939+
key: 'image',
940+
imageUrl: 'https://example.com/image.png',
941+
description: 'Alt text',
942+
};
943+
const result = returnNoValueCollector(imageField, 0, 'ImageCollector');
944+
expect(result.error).toBeNull();
945+
});
935946
});
936947

937948
describe('returnReadOnlyCollector', () => {
@@ -1177,8 +1188,8 @@ describe('returnImageCollector', () => {
11771188
expect(result.output.label).toBe('Friendly alt text');
11781189
});
11791190

1180-
// (e) imageUrl absent — src defaults to empty string
1181-
it('should default src to empty string when imageUrl is absent', () => {
1191+
// (e) imageUrl empty — src defaults to empty string, error is set
1192+
it('should default src to empty string and set an error when imageUrl is empty', () => {
11821193
const mockField: ImageField = {
11831194
type: 'IMAGE',
11841195
key: 'image',
@@ -1187,6 +1198,46 @@ describe('returnImageCollector', () => {
11871198
};
11881199
const result = returnImageCollector(mockField, 0);
11891200
expect(result.output.src).toBe('');
1201+
expect(result.error).toBe('imageUrl is not found in the field object. ');
1202+
});
1203+
1204+
// (e2) description empty — label/alt default to empty string, error is set
1205+
it('should set an error when description is empty', () => {
1206+
const mockField: ImageField = {
1207+
type: 'IMAGE',
1208+
key: 'image',
1209+
description: '',
1210+
imageUrl: 'https://example.com/image.png',
1211+
};
1212+
const result = returnImageCollector(mockField, 0);
1213+
expect(result.error).toBe('description is not found in the field object. ');
1214+
});
1215+
1216+
// (e3) both imageUrl and description empty — error lists both
1217+
it('should set an error for both missing fields when both are empty', () => {
1218+
const mockField: ImageField = {
1219+
type: 'IMAGE',
1220+
key: 'image',
1221+
description: '',
1222+
imageUrl: '',
1223+
};
1224+
const result = returnImageCollector(mockField, 0);
1225+
expect(result.error).toBe(
1226+
'imageUrl is not found in the field object. description is not found in the field object. ',
1227+
);
1228+
});
1229+
1230+
// (e4) imageUrl present — src passes through verbatim
1231+
it('should pass imageUrl through as src', () => {
1232+
const mockField: ImageField = {
1233+
type: 'IMAGE',
1234+
key: 'image',
1235+
description: 'Alt text',
1236+
imageUrl: 'https://example.com/image.png',
1237+
};
1238+
const result = returnImageCollector(mockField, 0);
1239+
expect(result.output.src).toBe('https://example.com/image.png');
1240+
expect(result.error).toBeNull();
11901241
});
11911242

11921243
// (f) output.label mirrors alt

‎packages/davinci-client/src/lib/collector.utils.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -946,7 +946,7 @@ export function returnNoValueCollector<
946946
CollectorType extends NoValueCollectorTypes = 'NoValueCollector',
947947
>(field: Field, idx: number, collectorType: CollectorType) {
948948
let error = '';
949-
if (!('content' in field)) {
949+
if (field.type !== 'IMAGE' && !('content' in field)) {
950950
error = `${error}Content is not found in the field object. `;
951951
}
952952
if (!('type' in field)) {
@@ -1038,15 +1038,22 @@ export function returnQrCodeCollector(field: QrCodeField, idx: number): QrCodeCo
10381038
*/
10391039
export function returnImageCollector(field: ImageField, idx: number): ImageCollector {
10401040
const base = returnNoValueCollector(field, idx, 'ImageCollector');
1041+
let error = base.error || '';
1042+
if (!field.imageUrl) {
1043+
error = `${error}imageUrl is not found in the field object. `;
1044+
}
1045+
if (!field.description) {
1046+
error = `${error}description is not found in the field object. `;
1047+
}
10411048

10421049
return {
10431050
...base,
1044-
error: null,
1051+
error: error || null,
10451052
output: {
10461053
...base.output,
1047-
label: field.description,
1048-
src: field.imageUrl,
1049-
alt: field.description,
1054+
label: field.description || '',
1055+
src: field.imageUrl || '',
1056+
alt: field.description || '',
10501057
...(field.hyperlinkUrl ? { href: field.hyperlinkUrl } : {}),
10511058
},
10521059
};

0 commit comments

Comments
 (0)