Skip to content

feat(reactjs-todo-davinci): add ValidatedPasswordCollector support#116

Open
ryanbas21 wants to merge 2 commits into
mainfrom
feat/validated-password-collector
Open

feat(reactjs-todo-davinci): add ValidatedPasswordCollector support#116
ryanbas21 wants to merge 2 commits into
mainfrom
feat/validated-password-collector

Conversation

@ryanbas21
Copy link
Copy Markdown
Contributor

@ryanbas21 ryanbas21 commented Jun 3, 2026

Summary

  • Upgrades @forgerock/davinci-client to PR#638 preview build (adds ValidatedPasswordCollector support)
  • Extends the Password component to render a static requirements list, inline validation errors on keystroke, and an optional confirm field (when verify: true) with real-time mismatch feedback
  • Wires ValidatedPasswordCollector into form.js via a new validator function exposed from useDavinci
  • Adds 2 Playwright e2e tests against the 356a254c PingOne tenant (which returns a passwordPolicy on the registration password field)

Changes

File Change
package.json @forgerock/davinci-clientpkg.pr.new PR#638 build
hooks/davinci.hook.js Expose validator(collector) with try/catch safety fallback
password.js Requirements list, inline errors, split visibility state, real-time confirm mismatch
form.js ValidatedPasswordCollector case in mapCollectorsToComponents
e2e/davinci-validated-password.spec.js 2 e2e tests: requirements list visible, inline errors appear/clear
playwright.config.ts 8443 webServer points to 356a254c tenant for ValidatedPassword tests

Notes

  • The pkg.pr.new URL should be replaced with the published npm tag once PR#638 is merged to the SDK
  • The verify: true confirm field is implemented but not reachable in the current PingOne environment (collector.output.verify is false) — the code is correct and will activate automatically when the environment is configured
  • The 356a254c tenant's pingOneSSOConnector/createUser step is returning "requestTimedOut" so a happy-path registration e2e test is not included — covered by the existing davinci-register-user.spec.js against 02fb4743

Test plan

  • npm run e2e -- --grep "ValidatedPasswordCollector" — 4 passing (Chromium + Firefox)
  • npm run e2e -- --grep "Login" — existing login tests still pass
  • Navigate to registration form manually and confirm requirements list and inline errors render

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added password validation with inline error messaging
    • Display password requirements (uppercase, lowercase, digits)
    • Added password confirmation field for verified password inputs
    • Enabled dynamic client configuration via URL parameters
  • Tests

    • Added end-to-end tests for password validation workflows

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 49904456-e9ed-421f-8757-b859dfd687cf

📥 Commits

Reviewing files that changed from the base of the PR and between 2745e9a and aaf99e7.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (5)
  • javascript/reactjs-todo-davinci/client/components/davinci-client/form.js
  • javascript/reactjs-todo-davinci/client/components/davinci-client/hooks/create-client.utils.js
  • javascript/reactjs-todo-davinci/client/components/davinci-client/hooks/davinci.hook.js
  • javascript/reactjs-todo-davinci/client/components/davinci-client/password.js
  • javascript/reactjs-todo-davinci/e2e/davinci-validated-password.spec.js
🚧 Files skipped from review as they are similar to previous changes (3)
  • javascript/reactjs-todo-davinci/e2e/davinci-validated-password.spec.js
  • javascript/reactjs-todo-davinci/client/components/davinci-client/form.js
  • javascript/reactjs-todo-davinci/client/components/davinci-client/password.js

📝 Walkthrough

Walkthrough

This PR adds password validation support to the DaVinci form system. A new validator function is exposed through the useDavinci() hook, the Form component wires it to a new ValidatedPasswordCollector case, the Password component is enhanced with requirements display and confirmation logic, and E2E tests verify the complete flow.

Changes

Password Validation in DaVinci Forms

Layer / File(s) Summary
Validation hook API
client/components/davinci-client/hooks/davinci.hook.js
The useDavinci() hook now exports a validator(collector) function that wraps DaVinci client validation, catches errors, and returns a fallback empty-array validator on failure.
Form integration for ValidatedPasswordCollector
client/components/davinci-client/form.js
Form component destructures validator from the hook and adds a switch case for ValidatedPasswordCollector that passes validator={validator(collector)} and verify={collector.output.verify} to the Password component.
Password component enhancement
client/components/davinci-client/password.js
Password component is refactored with new props (validator, verify), state for visibility toggles and validation errors, character-set regex patterns and buildRequirements() helper to render inline validation messages, and a conditional confirm-password section with mismatch detection.
Test environment and E2E coverage
client/components/davinci-client/hooks/create-client.utils.js, e2e/davinci-validated-password.spec.js
Adds CLIENT_CONFIGS map for clientId-based test configuration, and new Playwright spec with tests verifying password requirements are displayed, validation errors appear on policy violation, and errors clear on compliant input.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A password grows strong with each validation check,
Requirements displayed, no need to deck,
Confirm it twice to match just right,
The form now shines with validation light!
🔐✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding ValidatedPasswordCollector support, which is reflected across all modified files including form mapping, password component enhancements, hook exposure, and e2e tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/validated-password-collector

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@javascript/reactjs-todo-davinci/package.json`:
- Line 42: The package.json currently pins `@forgerock/davinci-client` to a
preview pkg.pr.new URL; replace that entry in
javascript/reactjs-todo-davinci/package.json so the dependency references the
published npm version string (e.g. "`@forgerock/davinci-client`":
">=<stable-version>" or a specific semver) instead of the pkg.pr.new URL, then
regenerate the lockfile (run npm install/npm ci locally) so package-lock.json no
longer contains a resolved pkg.pr.new URL; ensure the final package-lock.json
committed to the repo points to the stable registry artifact so CI installs
reliably.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 518e875a-ed2f-4793-b3ca-eb263fe249c5

📥 Commits

Reviewing files that changed from the base of the PR and between bc9ebef and 2745e9a.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (6)
  • javascript/reactjs-todo-davinci/client/components/davinci-client/form.js
  • javascript/reactjs-todo-davinci/client/components/davinci-client/hooks/davinci.hook.js
  • javascript/reactjs-todo-davinci/client/components/davinci-client/password.js
  • javascript/reactjs-todo-davinci/e2e/davinci-validated-password.spec.js
  • javascript/reactjs-todo-davinci/package.json
  • javascript/reactjs-todo-davinci/playwright.config.ts

Comment thread javascript/reactjs-todo-davinci/package.json Outdated
- Upgrades @forgerock/davinci-client to latest (2.0.0) which ships
  ValidatedPasswordCollector with password policy validation support
- Extends the Password component to render a static requirements list
  from the password policy, inline validation errors on keystroke,
  and an optional confirm field with real-time mismatch feedback
- Wires ValidatedPasswordCollector into form.js via a new validator
  function exposed from useDavinci with proper error handling
- Adds a second webServer (port 8444, 356a254c tenant) in playwright
  config for ValidatedPasswordCollector e2e tests; skipped in CI
  where only the 02fb4743 tenant is available
@ryanbas21 ryanbas21 force-pushed the feat/validated-password-collector branch from 41d7602 to cf50b48 Compare June 3, 2026 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant