fix(journey-client): add createJourneyStep and baseUrl to support Platform Login#722
fix(journey-client): add createJourneyStep and baseUrl to support Platform Login#722ryanbas21 wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: 611620d 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (18)
🚧 Files skipped from review as they are similar to previous changes (16)
📝 WalkthroughWalkthroughThe journey client now supports direct ChangesJourney client updates
DaVinci API report
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Journey as journey()
participant Wellknown as wellknownApi
participant Config as configSlice
alt serverConfig.baseUrl exists
Journey->>Config: dispatch baseUrl and endpoint paths
else serverConfig.wellknown
Journey->>Wellknown: fetch OpenID configuration
Wellknown-->>Journey: return wellknownResponse
Journey->>Config: dispatch wellknown configuration
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 611620d
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (23.38%) 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 #722 +/- ##
==========================================
+ Coverage 18.07% 23.38% +5.30%
==========================================
Files 155 162 +7
Lines 24398 25628 +1230
Branches 1203 1629 +426
==========================================
+ Hits 4410 5993 +1583
+ Misses 19988 19635 -353
🚀 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 bf5dcfe to https://ForgeRock.github.io/ping-javascript-sdk/pr-722/bf5dcfeba89a0ad0ec998c275bc68ad8642a1453 branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis🆕 New Packages🆕 @forgerock/device-client - 10.0 KB (new) 📊 Minor Changes📈 @forgerock/sdk-types - 9.1 KB (+0.0 KB) ➖ No Changes➖ @forgerock/protect - 144.6 KB 14 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
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 `@packages/sdk-types/src/lib/config.types.ts`:
- Around line 42-46: Update the JourneyServerConfig interface so wellknown is
optional, allowing configurations that provide only baseUrl while preserving the
existing baseUrl and timeout fields.
🪄 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: b724b842-066b-498b-8cff-6ae56478b5f5
📒 Files selected for processing (8)
packages/journey-client/api-report/journey-client.api.mdpackages/journey-client/package.jsonpackages/journey-client/src/_utils.tspackages/journey-client/src/index.tspackages/journey-client/src/lib/client.store.tspackages/journey-client/src/lib/config.slice.test.tspackages/journey-client/src/lib/config.slice.tspackages/sdk-types/src/lib/config.types.ts
💤 Files with no reviewable changes (1)
- packages/journey-client/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/journey-client/api-report/journey-client.api.md
44633fc to
2a1d32f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/journey-app/services/delete-webauthn-device.ts (1)
95-98: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSupport
baseUrlconfiguration directly instead of throwing an error.Since
config.serverConfigcan now containbaseUrlandconfigcontainsrealmPath, you can avoid throwing an error and fully support theLegacyServerConfigbranch here.♻️ Proposed refactor to support `baseUrl`
- const wellknown = 'wellknown' in config.serverConfig ? config.serverConfig.wellknown : undefined; - if (!wellknown) { - throw new Error('serverConfig.wellknown is required to delete a WebAuthn device.'); - } - const baseUrl = getBaseUrlFromWellknown(wellknown); - const realmUrlPath = getRealmUrlPathFromWellknown(wellknown); + const baseUrl = 'baseUrl' in config.serverConfig + ? config.serverConfig.baseUrl + : getBaseUrlFromWellknown(config.serverConfig.wellknown); + + const realmUrlPath = 'wellknown' in config.serverConfig + ? getRealmUrlPathFromWellknown(config.serverConfig.wellknown) + : `realms/${config.realmPath || 'root'}`; +🤖 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/journey-app/services/delete-webauthn-device.ts` around lines 95 - 98, Update the WebAuthn device deletion flow around the wellknown lookup to support the LegacyServerConfig branch: when serverConfig provides baseUrl, combine it with config.realmPath to construct the required endpoint instead of throwing. Preserve the existing wellknown behavior for configurations that provide serverConfig.wellknown, and only fail when neither configuration can produce the endpoint.
🤖 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.
Nitpick comments:
In `@e2e/journey-app/services/delete-webauthn-device.ts`:
- Around line 95-98: Update the WebAuthn device deletion flow around the
wellknown lookup to support the LegacyServerConfig branch: when serverConfig
provides baseUrl, combine it with config.realmPath to construct the required
endpoint instead of throwing. Preserve the existing wellknown behavior for
configurations that provide serverConfig.wellknown, and only fail when neither
configuration can produce the endpoint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5a49c327-21a4-40d5-a33f-a68a76e0c974
📒 Files selected for processing (15)
e2e/journey-app/services/delete-webauthn-device.tspackages/journey-client/api-report/journey-client._utils.api.mdpackages/journey-client/api-report/journey-client.api.mdpackages/journey-client/api-report/journey-client.types.api.mdpackages/journey-client/package.jsonpackages/journey-client/src/_utils.tspackages/journey-client/src/index.tspackages/journey-client/src/lib/client.store.test.tspackages/journey-client/src/lib/client.store.tspackages/journey-client/src/lib/config.slice.test.tspackages/journey-client/src/lib/config.slice.tspackages/journey-client/src/lib/config.types.test-d.tspackages/journey-client/src/lib/config.types.tspackages/sdk-types/src/lib/config.types.tspackages/sdk-utilities/src/lib/config/config.test.ts
💤 Files with no reviewable changes (1)
- packages/journey-client/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/journey-client/package.json
- packages/journey-client/src/_utils.ts
- packages/journey-client/src/lib/client.store.ts
- packages/journey-client/src/lib/config.slice.ts
2a1d32f to
2f7b71f
Compare
Add createJourneyStep to the journey-client api
2f7b71f to
611620d
Compare
Two fixes for Platform Login use cases. These additions are not intended for normal, customer use.
baseUrlas a config optionJIRA Ticket
No Jira
Description
Platform Login has edge cases that need to be support by the Journey Client. One is the ability to store a step outside of the Journey Client and "re-decorate" the Step object with convenience methods upon user return. The other is the ability to support authentication when there is no Wellknown endpoint avialable.
Summary by CodeRabbit
createJourneyStep(and related step/callback types) to simplify recreating steps from serialized data.serverConfig.baseUrlmode in addition to well-known discovery, including correct endpoint path handling and updated warnings behavior.LegacyServerConfigto the public type surface.