test(davinci-client): e2e tests for agreements, single checkbox, rich text#638
test(davinci-client): e2e tests for agreements, single checkbox, rich text#638ancheetah wants to merge 1 commit into
Conversation
|
📝 WalkthroughWalkthroughSingleCheckboxField error representation moved from nested ChangesCheckbox Validation Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 89d04ee ☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
We updated the test fixture in node.reducer.test.ts to use the new flattened errorMessage field on SingleCheckboxField, which fixes the assertion failure caused by the PR's intentional type change. The implementation in collector.utils.ts was already reading field.errorMessage directly, but the test was still supplying the old nested validation: { errorMessage: '...' } shape, causing the message to fall back to 'Value cannot be empty'.
Tip
✅ We verified this fix by re-running @forgerock/davinci-client:test.
diff --git a/packages/davinci-client/src/lib/node.reducer.test.ts b/packages/davinci-client/src/lib/node.reducer.test.ts
index 3db2cbd..75d66e7 100644
--- a/packages/davinci-client/src/lib/node.reducer.test.ts
+++ b/packages/davinci-client/src/lib/node.reducer.test.ts
@@ -1699,7 +1699,7 @@ describe('The node collector reducer with ValidatedBooleanCollector', () => {
key: 'accept-terms',
label: 'Accept Terms',
required: true,
- validation: { errorMessage: 'You must accept the terms' },
+ errorMessage: 'You must accept the terms',
},
],
formData: {},
Or Apply changes locally with:
npx nx-cloud apply-locally 2XKE-yVlk
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/davinci-app/components/boolean.ts`:
- Around line 51-64: The checkbox handler currently only calls updater when
validation passes, causing collector state to lag when validation fails; always
invoke updater((event.target as HTMLInputElement).checked) (the updater call in
this block) regardless of the validation result so the collector state stays in
sync, then run the existing validation logic that creates/removes the
`${collectorKey}-error` element based on result; keep the existing error logging
path (checking 'error' in the updater return) and ensure you do not
remove/create error DOM nodes prematurely (use updater first, then handle
Array.isArray(result) and error DOM updates using result, collectorKey, formEl).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59f71200-eefc-4560-b9b1-eea5176a5604
📒 Files selected for processing (10)
.gitignoree2e/davinci-app/components/boolean.tse2e/davinci-app/components/text.tse2e/davinci-app/main.tse2e/davinci-app/server-configs.tse2e/davinci-suites/src/form-fields.test.tspackages/davinci-client/api-report/davinci-client.api.mdpackages/davinci-client/api-report/davinci-client.types.api.mdpackages/davinci-client/src/lib/collector.utils.tspackages/davinci-client/src/lib/davinci.types.ts
128e211 to
89d04ee
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
e2e/davinci-suites/src/form-fields.test.ts (1)
92-94: ⚡ Quick winUse consistent navigation pattern with
asyncEventshelper.The second test uses
page.goto()with a hardcoded URL, while the first test uses theasyncEventshelper with a relative URL. Using the helper provides consistent event handling and avoids hardcoding the port/protocol/host.♻️ Refactor to use asyncEvents helper
test('should render form validation fields', async ({ page }) => { - await page.goto('http://localhost:5829/?clientId=e4ef2896-8d90-4abd-bf0f-7b8034995927'); + const { navigate } = asyncEvents(page); + await navigate('/?clientId=e4ef2896-8d90-4abd-bf0f-7b8034995927'); await expect(page.getByText('Select Form Fields Test Form')).toBeVisible();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/davinci-suites/src/form-fields.test.ts` around lines 92 - 94, The test "should render form validation fields" currently calls page.goto with a hardcoded full URL; update it to use the existing asyncEvents helper and a relative path like the other test to keep event handling consistent and avoid embedding host/port. Replace the page.goto('http://localhost:5829/?clientId=...') call inside the test body with the asyncEvents call used elsewhere (refer to the asyncEvents helper and the test name "should render form validation fields") so the test navigates to '/?clientId=e4ef2896-8d90-4abd-bf0f-7b8034995927' via asyncEvents instead of a hardcoded URL.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/davinci-app/components/boolean.ts`:
- Line 25: The code constructs DOM selectors using the raw collectorKey (const
collectorKey = collector.output.key) and then passes it to
document.querySelector for id/class selectors, which breaks when keys contain
CSS-special characters (e.g., dots); update each querySelector call that uses
collectorKey (the selectors around where collectorKey is interpolated) to wrap
the key with CSS.escape(collectorKey) when building id or class selectors so the
selectors are valid for any key value.
---
Nitpick comments:
In `@e2e/davinci-suites/src/form-fields.test.ts`:
- Around line 92-94: The test "should render form validation fields" currently
calls page.goto with a hardcoded full URL; update it to use the existing
asyncEvents helper and a relative path like the other test to keep event
handling consistent and avoid embedding host/port. Replace the
page.goto('http://localhost:5829/?clientId=...') call inside the test body with
the asyncEvents call used elsewhere (refer to the asyncEvents helper and the
test name "should render form validation fields") so the test navigates to
'/?clientId=e4ef2896-8d90-4abd-bf0f-7b8034995927' via asyncEvents instead of a
hardcoded URL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c29145f7-cabe-4a6a-b6d0-2690b2d520dc
📒 Files selected for processing (11)
.gitignoree2e/davinci-app/components/boolean.tse2e/davinci-app/components/text.tse2e/davinci-app/main.tse2e/davinci-app/server-configs.tse2e/davinci-suites/src/form-fields.test.tspackages/davinci-client/api-report/davinci-client.api.mdpackages/davinci-client/api-report/davinci-client.types.api.mdpackages/davinci-client/src/lib/collector.utils.tspackages/davinci-client/src/lib/davinci.types.tspackages/davinci-client/src/lib/node.reducer.test.ts
✅ Files skipped from review due to trivial changes (1)
- .gitignore
| validator: Validator<ValidatedBooleanCollector>, | ||
| ) { | ||
| // Create a container for the checkboxes | ||
| const collectorKey = collector.output.key; |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
find . -type f -name "boolean.ts" -path "*davinci-app*"Repository: ForgeRock/ping-javascript-sdk
Repository: ForgeRock/ping-javascript-sdk
Exit code: 0
stdout:
./e2e/davinci-app/components/boolean.ts
Script executed:
cat -n ./e2e/davinci-app/components/boolean.tsRepository: ForgeRock/ping-javascript-sdk
Repository: ForgeRock/ping-javascript-sdk
Exit code: 0
stdout:
1 /*
2 * Copyright (c) 2026 Ping Identity Corporation. All rights reserved.
3 *
4 * This software may be modified and distributed under the terms
5 * of the MIT license. See the LICENSE file for details.
6 */
7 import type {
8 ValidatedBooleanCollector,
9 Updater,
10 Validator,
11 } from '`@forgerock/davinci-client/types`';
12
13 /**
14 * Creates a single checkbox and attaches it to the form
15 * `@param` {HTMLFormElement} formEl - The form element to attach the checkbox to
16 * `@param` {ValidatedBooleanCollector} collector - Contains the configuration
17 * `@param` {Updater} updater - Function to call when selection changes
18 */
19 export default function booleanComponent(
20 formEl: HTMLFormElement,
21 collector: ValidatedBooleanCollector,
22 updater: Updater<ValidatedBooleanCollector>,
23 validator: Validator<ValidatedBooleanCollector>,
24 ) {
25 const collectorKey = collector.output.key;
26
27 // Create a container for the checkbox
28 const containerDiv = document.createElement('div');
29 containerDiv.className = 'single-checkbox-container';
30
31 // Create a single checkbox
32 const wrapper = document.createElement('div');
33 wrapper.className = 'checkbox-wrapper';
34
35 const checkbox = document.createElement('input');
36 checkbox.type = 'checkbox';
37 checkbox.id = collectorKey;
38 checkbox.name = collectorKey || 'single-checkbox-field';
39 checkbox.checked = collector.output.value;
40 checkbox.value = 'checked';
41
42 const label = document.createElement('label');
43 label.htmlFor = checkbox.id;
44 label.textContent = collector.output.label;
45
46 // Add event listener to handle single-select behavior
47 checkbox.addEventListener('change', (event) => {
48 const checked = (event.target as HTMLInputElement).checked;
49 const result = validator(checked);
50 const errorEl = formEl?.querySelector(`.${collectorKey}-error`);
51
52 // Keep collector state aligned with the current UI value
53 const updateError = updater(checked);
54 if (updateError && 'error' in updateError) {
55 console.error(updateError.error.message);
56 }
57
58 // Validate the input
59 if (Array.isArray(result) && result.length && !errorEl) {
60 const errorEl = document.createElement('div');
61 errorEl.className = `${collectorKey}-error`;
62 errorEl.innerText = result.join(', ');
63 formEl?.querySelector(`#${collectorKey}`)?.after(errorEl);
64 } else {
65 formEl.querySelector(`.${collectorKey}-error`)?.remove();
66 }
67 });
68
69 wrapper.appendChild(checkbox);
70 wrapper.appendChild(label);
71 containerDiv.appendChild(wrapper);
72
73 // Append the container to the form
74 formEl.appendChild(containerDiv);
75 }
Script executed:
# Search for how collector.output.key is generated or what format it follows
rg "collector\.output\.key" -B 3 -A 3 --type ts --type jsRepository: ForgeRock/ping-javascript-sdk
Repository: ForgeRock/ping-javascript-sdk
Exit code: 0
stdout:
e2e/davinci-app/main.ts- collector, // This is the plain object of the collector
e2e/davinci-app/main.ts- davinciClient.flow({
e2e/davinci-app/main.ts- // Returns a function to call the flow from within component
e2e/davinci-app/main.ts: action: collector.output.key,
e2e/davinci-app/main.ts- }),
e2e/davinci-app/main.ts- renderForm, // Ignore this; it's just for re-rendering the form
e2e/davinci-app/main.ts- );
--
e2e/davinci-app/components/boolean.ts- updater: Updater<ValidatedBooleanCollector>,
e2e/davinci-app/components/boolean.ts- validator: Validator<ValidatedBooleanCollector>,
e2e/davinci-app/components/boolean.ts-) {
e2e/davinci-app/components/boolean.ts: const collectorKey = collector.output.key;
e2e/davinci-app/components/boolean.ts-
e2e/davinci-app/components/boolean.ts- // Create a container for the checkbox
e2e/davinci-app/components/boolean.ts- const containerDiv = document.createElement('div');
--
e2e/davinci-app/components/fido.ts- if (collector.type === 'FidoRegistrationCollector') {
e2e/davinci-app/components/fido.ts- const button = document.createElement('button');
e2e/davinci-app/components/fido.ts- button.type = 'button';
e2e/davinci-app/components/fido.ts: button.value = collector.output.key;
e2e/davinci-app/components/fido.ts- button.innerHTML = 'FIDO Register';
e2e/davinci-app/components/fido.ts- formEl.appendChild(button);
e2e/davinci-app/components/fido.ts-
--
e2e/davinci-app/components/fido.ts- } else if (collector.type === 'FidoAuthenticationCollector') {
e2e/davinci-app/components/fido.ts- const button = document.createElement('button');
e2e/davinci-app/components/fido.ts- button.type = 'button';
e2e/davinci-app/components/fido.ts: button.value = collector.output.key;
e2e/davinci-app/components/fido.ts- button.innerHTML = 'FIDO Authenticate';
e2e/davinci-app/components/fido.ts- formEl.appendChild(button);
e2e/davinci-app/components/fido.ts-
--
e2e/davinci-app/components/password.ts- updater: Updater<PasswordCollector | ValidatedPasswordCollector>,
e2e/davinci-app/components/password.ts- validator?: Validator,
e2e/davinci-app/components/password.ts-) {
e2e/davinci-app/components/password.ts: const collectorKey = dotToCamelCase(collector.output.key);
e2e/davinci-app/components/password.ts- const label = document.createElement('label');
e2e/davinci-app/components/password.ts- const input = document.createElement('input');
e2e/davinci-app/components/password.ts-
--
e2e/davinci-app/components/polling.ts-) {
e2e/davinci-app/components/polling.ts- const button = document.createElement('button');
e2e/davinci-app/components/polling.ts- button.type = 'button';
e2e/davinci-app/components/polling.ts: button.value = collector.output.key;
e2e/davinci-app/components/polling.ts- button.innerHTML = 'Start polling';
e2e/davinci-app/components/polling.ts- formEl.appendChild(button);
e2e/davinci-app/components/polling.ts-
--
e2e/davinci-app/components/multi-value.ts-
e2e/davinci-app/components/multi-value.ts- const checkbox = document.createElement('input');
e2e/davinci-app/components/multi-value.ts- checkbox.type = 'checkbox';
e2e/davinci-app/components/multi-value.ts: checkbox.id = `${collector.output.key}-${index}`;
e2e/davinci-app/components/multi-value.ts: checkbox.name = collector.output.key || 'checkbox-field';
e2e/davinci-app/components/multi-value.ts- checkbox.value = option.value;
e2e/davinci-app/components/multi-value.ts-
e2e/davinci-app/components/multi-value.ts- const label = document.createElement('label');
--
e2e/davinci-app/components/text.ts- updater: Updater<TextCollector | ValidatedTextCollector>,
e2e/davinci-app/components/text.ts- validator: Validator<ValidatedTextCollector>,
e2e/davinci-app/components/text.ts-) {
e2e/davinci-app/components/text.ts: const collectorKey = dotToCamelCase(collector.output.key);
e2e/davinci-app/components/text.ts- const label = document.createElement('label');
e2e/davinci-app/components/text.ts- const input = document.createElement('input');
e2e/davinci-app/components/text.ts-
--
e2e/davinci-app/components/single-value.ts- // Create the label element
e2e/davinci-app/components/single-value.ts- const labelEl = document.createElement('label');
e2e/davinci-app/components/single-value.ts- labelEl.textContent = collector.output.label || 'Select an option';
e2e/davinci-app/components/single-value.ts: labelEl.setAttribute('for', collector.output.key || 'dropdown-field');
e2e/davinci-app/components/single-value.ts- labelEl.className = 'dropdown-label';
e2e/davinci-app/components/single-value.ts: labelEl.setAttribute('for', collector.output.key || 'dropdown-field');
e2e/davinci-app/components/single-value.ts-
e2e/davinci-app/components/single-value.ts- // Create the select element
e2e/davinci-app/components/single-value.ts- const selectEl = document.createElement('select');
e2e/davinci-app/components/single-value.ts: selectEl.name = collector.output.key || 'dropdown-field';
e2e/davinci-app/components/single-value.ts: selectEl.id = collector.output.key || 'dropdown-field';
e2e/davinci-app/components/single-value.ts-
e2e/davinci-app/components/single-value.ts- // Add all options from the data
e2e/davinci-app/components/single-value.ts- for (const option of collector.output.options) {
Script executed:
# Search for dotToCamelCase definition and usage
rg "dotToCamelCase" -B 2 -A 5 --type ts --type jsRepository: ForgeRock/ping-javascript-sdk
Repository: ForgeRock/ping-javascript-sdk
Exit code: 0
stdout:
e2e/journey-app/helper.ts- * of the MIT license. See the LICENSE file for details.
e2e/journey-app/helper.ts- */
e2e/journey-app/helper.ts:export function dotToCamelCase(str: string) {
e2e/journey-app/helper.ts- return str
e2e/journey-app/helper.ts- .split('.')
e2e/journey-app/helper.ts- .map((part: string, index: number) =>
e2e/journey-app/helper.ts- index === 0 ? part.toLowerCase() : part.charAt(0).toUpperCase() + part.slice(1).toLowerCase(),
e2e/journey-app/helper.ts- )
--
e2e/davinci-app/helper.ts- * of the MIT license. See the LICENSE file for details.
e2e/davinci-app/helper.ts- */
e2e/davinci-app/helper.ts:export function dotToCamelCase(str: string) {
e2e/davinci-app/helper.ts- return str
e2e/davinci-app/helper.ts- .split('.')
e2e/davinci-app/helper.ts- .map((part: string, index: number) =>
e2e/davinci-app/helper.ts- index === 0 ? part.toLowerCase() : part.charAt(0).toUpperCase() + part.slice(1).toLowerCase(),
e2e/davinci-app/helper.ts- )
--
e2e/davinci-app/components/text.ts- Validator,
e2e/davinci-app/components/text.ts-} from '`@forgerock/davinci-client/types`';
e2e/davinci-app/components/text.ts:import { dotToCamelCase } from '../helper.js';
e2e/davinci-app/components/text.ts-
e2e/davinci-app/components/text.ts-export default function textComponent(
e2e/davinci-app/components/text.ts- formEl: HTMLFormElement,
e2e/davinci-app/components/text.ts- collector: TextCollector | ValidatedTextCollector,
e2e/davinci-app/components/text.ts- updater: Updater<TextCollector | ValidatedTextCollector>,
e2e/davinci-app/components/text.ts- validator: Validator<ValidatedTextCollector>,
e2e/davinci-app/components/text.ts-) {
e2e/davinci-app/components/text.ts: const collectorKey = dotToCamelCase(collector.output.key);
e2e/davinci-app/components/text.ts- const label = document.createElement('label');
e2e/davinci-app/components/text.ts- const input = document.createElement('input');
e2e/davinci-app/components/text.ts-
e2e/davinci-app/components/text.ts- label.htmlFor = collectorKey;
e2e/davinci-app/components/text.ts- label.innerText = collector.output.label;
--
e2e/davinci-app/components/password.ts- Validator,
e2e/davinci-app/components/password.ts-} from '`@forgerock/davinci-client/types`';
e2e/davinci-app/components/password.ts:import { dotToCamelCase } from '../helper.js';
e2e/davinci-app/components/password.ts-
e2e/davinci-app/components/password.ts-const UPPERCASE_RE = /^[A-Z]+$/;
e2e/davinci-app/components/password.ts-const LOWERCASE_RE = /^[a-z]+$/;
e2e/davinci-app/components/password.ts-const DIGIT_RE = /^[0-9]+$/;
e2e/davinci-app/components/password.ts-
--
e2e/davinci-app/components/password.ts- validator?: Validator,
e2e/davinci-app/components/password.ts-) {
e2e/davinci-app/components/password.ts: const collectorKey = dotToCamelCase(collector.output.key);
e2e/davinci-app/components/password.ts- const label = document.createElement('label');
e2e/davinci-app/components/password.ts- const input = document.createElement('input');
e2e/davinci-app/components/password.ts-
e2e/davinci-app/components/password.ts- label.htmlFor = collectorKey;
e2e/davinci-app/components/password.ts- label.innerText = collector.output.label;
Script executed:
# Verify CSS.escape is available and check for any other selector uses
rg "querySelector" e2e/davinci-app/components/boolean.ts -nRepository: ForgeRock/ping-javascript-sdk
Repository: ForgeRock/ping-javascript-sdk
Exit code: 0
stdout:
50: const errorEl = formEl?.querySelector(`.${collectorKey}-error`);
63: formEl?.querySelector(`#${collectorKey}`)?.after(errorEl);
65: formEl.querySelector(`.${collectorKey}-error`)?.remove();
Escape collectorKey in querySelector selectors to handle keys with CSS-special characters.
Keys containing dots (e.g., from dotToCamelCase in other components) will break unescaped selectors. Use CSS.escape() for all querySelector id and class selectors at lines 50, 63, and 65.
🔧 Proposed fix
const collectorKey = collector.output.key;
+ const selectorKey = CSS.escape(collectorKey);
@@
- const errorEl = formEl?.querySelector(`.${collectorKey}-error`);
+ const errorEl = formEl?.querySelector(`.${selectorKey}-error`);
@@
- formEl?.querySelector(`#${collectorKey}`)?.after(errorEl);
+ formEl?.querySelector(`#${selectorKey}`)?.after(errorEl);
@@
- formEl.querySelector(`.${collectorKey}-error`)?.remove();
+ formEl.querySelector(`.${selectorKey}-error`)?.remove();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/davinci-app/components/boolean.ts` at line 25, The code constructs DOM
selectors using the raw collectorKey (const collectorKey = collector.output.key)
and then passes it to document.querySelector for id/class selectors, which
breaks when keys contain CSS-special characters (e.g., dots); update each
querySelector call that uses collectorKey (the selectors around where
collectorKey is interpolated) to wrap the key with CSS.escape(collectorKey) when
building id or class selectors so the selectors are valid for any key value.
JIRA Ticket
https://pingidentity.atlassian.net/browse/SDKS-5000
Description
Extends e2e coverage for agreements, rich text, and single checkbox fields; fixes
SingleCheckboxFieldtype to match the actual server payload. Moves Form Fields Test flow to new web tenant.Key Changes
SingleCheckboxFieldtype — flattenedvalidation.errorMessageinto a top-levelerrorMessage?: stringto match the server shape. Updatedcollector.utils.tsand regenerated API reports.booleanComponent— wired inValidator<ValidatedBooleanCollector>so the checkbox runs client-side validation and renders inline errors on change.server-configs.ts— swapped client ID to a PingOne environment that includes the updated form fields flow.form-fields.test.ts— added assertions for rich text link rendering, agreement visibility, single checkbox default state, and required-field error on uncheck.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores