refactor(journey-client): rename fr-webauthn to journey-webauthn and … #415
Conversation
…other fr prefixes to journey
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 dba4466
☁️ Nx Cloud last updated this comment at |
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (18.45%) 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 @@
## create-journey-package #415 +/- ##
===========================================================
- Coverage 80.55% 18.45% -62.10%
===========================================================
Files 44 139 +95
Lines 3646 27348 +23702
Branches 342 943 +601
===========================================================
+ Hits 2937 5047 +2110
- Misses 709 22301 +21592
🚀 New features to boost your workflow:
|
|
Deployed 6fa17c9 to https://ForgeRock.github.io/ping-javascript-sdk/pr-415/6fa17c961eafad25878fd941273a87e2ff3f72a0 branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis🆕 New Packages🆕 @forgerock/device-client - 9.2 KB (new) 13 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
![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.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
cerebrl
left a comment
cerebrl
left a comment
There was a problem hiding this comment.
This looks good. Do we want to convert the abstract classes to factory functions to keep consistent with our patterns? Would that be too much work?
| import { logger as loggerFn } from '@forgerock/sdk-logger'; | ||
|
|
||
| const FRLogger = logger({ level: 'info' }); | ||
| const JourneyLogger = loggerFn({ level: 'info' }); |
There was a problem hiding this comment.
I think this might be an issue. We are kind of hard-coding the logger level here, but I don't think we can do this.
ryanbas21
force-pushed
the
fix-login-failure-success
branch
from
107a322 to
acb3df7
Compare
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
We identified that the e2e test failures were caused by changing the storage key prefix from "FR-SDK" to "Journey-SDK". This change breaks the authentication flow since existing stored state/PKCE data uses the old prefix. We've reverted this specific change while keeping all the other refactoring changes intact.
We verified this fix by re-running @forgerock/davinci-suites:e2e-ci--src/basic.test.ts.
diff --git a/packages/sdk-effects/oidc/src/lib/state-pkce.effects.ts b/packages/sdk-effects/oidc/src/lib/state-pkce.effects.ts
index 23d3e90..d29657c 100644
--- a/packages/sdk-effects/oidc/src/lib/state-pkce.effects.ts
+++ b/packages/sdk-effects/oidc/src/lib/state-pkce.effects.ts
@@ -10,7 +10,7 @@ import { createVerifier, createState } from '@forgerock/sdk-utilities';
import type { GetAuthorizationUrlOptions } from '@forgerock/sdk-types';
function getStorageKey(clientId: string, prefix?: string) {
- return `${prefix || 'Journey-SDK'}-authflow-${clientId}`;
+ return `${prefix || 'FR-SDK'}-authflow-${clientId}`;
}
/**
⚙️ An Nx Cloud workspace admin can disable these reviews in workspace settings.
cerebrl
left a comment
cerebrl
left a comment
There was a problem hiding this comment.
We can do this conversion later since we are running out of time.
| * }); | ||
| * ``` | ||
| */ | ||
| export function journeyDevice( |
There was a problem hiding this comment.
This is the one module that we should keep as a class so that the customer can easily override the methods with their own. That's why I mention abstract classes in my previous comment for conversion. This one non-abstract needs to remain a class.
refactor(journey-client): rename fr-webauthn to journey-webauthn and …
3 participants
…other fr prefixes to journey
JIRA Ticket
see #414
Description
Further renaming, still more to do.