Conversation
🦋 Changeset detectedLatest commit: 65a6b01 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughAdds QR code collector support: new field and collector types, factory and reducer integration, tests, an e2e UI component to render QR codes (with fallback text and error handling), and an updated e2e server config entry. Changes
Sequence Diagram(s)mermaid Field->>Reducer: dispatch node/next with field Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 65a6b01
☁️ Nx Cloud last updated this comment at |
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (14.86%) is below the target coverage (40.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #562 +/- ##
===========================================
- Coverage 70.90% 14.86% -56.05%
===========================================
Files 53 153 +100
Lines 2021 26292 +24271
Branches 377 1067 +690
===========================================
+ Hits 1433 3907 +2474
- Misses 588 22385 +21797
🚀 New features to boost your workflow:
|
@forgerock/davinci-client
@forgerock/device-client
@forgerock/journey-client
@forgerock/oidc-client
@forgerock/protect
@forgerock/sdk-types
@forgerock/sdk-utilities
@forgerock/iframe-manager
@forgerock/sdk-logger
@forgerock/sdk-oidc
@forgerock/sdk-request-middleware
@forgerock/storage
commit: |
|
Deployed 10852f2 to https://ForgeRock.github.io/ping-javascript-sdk/pr-562/10852f274efb8f49119ab33422db4ea7490e178d branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis🚨 Significant Changes🔻 @forgerock/device-client - 0.0 KB (-9.7 KB, -100.0%) 📊 Minor Changes📉 @forgerock/device-client - 9.7 KB (-0.0 KB) ➖ No Changes➖ @forgerock/sdk-utilities - 11.2 KB 14 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
Add QR_CODE field type support to the DaVinci client SDK, enabling applications to render QR codes returned by DaVinci flows. - Add QrCodeField type and QrCodeCollectorBase interface - Add returnQrCodeCollector factory with defensive fallbacks - Wire QR_CODE dispatch in node reducer (early return, same as LABEL) - Export QrCodeCollector in public API types - Add QR code component and config to sample davinci-app - Add unit tests for factory and reducer integration
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/davinci-app/components/qr-code.ts (1)
10-14: Consider adding adata-testidto the error element for e2e test coverage.This would enable testing error scenarios in automated tests.
🧪 Proposed improvement
if (collector.error) { const errorEl = document.createElement('p'); errorEl.innerText = `QR Code error: ${collector.error}`; + errorEl.setAttribute('data-testid', 'qr-code-error'); formEl.appendChild(errorEl); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/davinci-app/components/qr-code.ts` around lines 10 - 14, Add a data-testid attribute to the error paragraph so e2e tests can select it: when handling collector.error in the QR code component (the branch that creates errorEl and appends to formEl), set errorEl.dataset.testid = 'qr-code-error' (or use errorEl.setAttribute('data-testid', 'qr-code-error')) before appending; keep the existing innerText and return behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@e2e/davinci-app/components/qr-code.ts`:
- Around line 10-14: Add a data-testid attribute to the error paragraph so e2e
tests can select it: when handling collector.error in the QR code component (the
branch that creates errorEl and appends to formEl), set errorEl.dataset.testid =
'qr-code-error' (or use errorEl.setAttribute('data-testid', 'qr-code-error'))
before appending; keep the existing innerText and return behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 554b1372-8058-424c-9dda-d435ac4bab4a
📒 Files selected for processing (12)
e2e/davinci-app/components/qr-code.tse2e/davinci-app/main.tse2e/davinci-app/server-configs.tspackages/davinci-client/src/lib/collector.types.tspackages/davinci-client/src/lib/collector.utils.test.tspackages/davinci-client/src/lib/collector.utils.tspackages/davinci-client/src/lib/davinci.types.tspackages/davinci-client/src/lib/node.reducer.test.tspackages/davinci-client/src/lib/node.reducer.tspackages/davinci-client/src/lib/node.types.test-d.tspackages/davinci-client/src/lib/node.types.tspackages/davinci-client/src/types.ts
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Nx Cloud has identified a flaky task in your failed CI:
🔂 Since the failure was identified as flaky, we triggered a CI rerun by adding an empty commit to this branch.
🔔 Heads up, your workspace has pending recommendations ↗ to auto-apply fixes for similar failures.
🎓 Learn more about Self-Healing CI on nx.dev
2 participants
Jira
https://pingidentity.atlassian.net/browse/SDKS-4686
Summary
QR_CODEfield type support to the DaVinci client SDK, enabling applications to render QR codes returned by DaVinci flowsQrCodeCollector(NoValueCollector family) withoutput.src(base64 data URI) andoutput.fallbackText(manual pairing key)QR_CODEfield dispatch in the node reducer with defensive fallbacks for malformed server responsesChanges
davinci-client
davinci.types.ts— NewQrCodeFieldtype (type: 'QR_CODE',key,content,fallbackText?)collector.types.ts— NewQrCodeCollectorBaseinterface withsrcandfallbackTextin outputcollector.utils.ts—returnQrCodeCollectorfactory with defensive fallbacks matchingreturnNoValueCollectorpatternnode.reducer.ts—QR_CODEearly return before switch (same pattern asLABEL)node.types.ts/types.ts—QrCodeCollectorregistered inCollectorsunion and public APIdavinci-app (sample)
components/qr-code.ts— Renders QR code<img>with fallback text, checkscollector.errormain.ts— WiresQrCodeCollectorbranch into render loopserver-configs.ts— Adds QR code flow test configTests
returnQrCodeCollector(happy path, missing fallbackText, missing content, missing key, accumulated errors)QR_CODEfield dispatchCollectorsunionTest plan
nx run @forgerock/davinci-client:nxTest)http://localhost:5829/?clientId=c12743f9-08e8-4420-a624-71bbb08e9fe1and verify QR code rendersSummary by CodeRabbit
New Features
Tests
Chores