-
Notifications
You must be signed in to change notification settings - Fork 8
feat: add invitation url params #193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: add invitation url params #193
Conversation
WalkthroughAdds optional invitation support to login types and URL mapping: two new fields ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/types.ts (1)
224-233: Document new public invitation fields onLoginOptions
invitationCodeandisInvitationare part of the public login API but currently lack JSDoc, unlike most other fields here. Consider adding brief comments explaining:
- what
invitationCoderepresents (e.g. single‑use org/user invitation token), and- when
isInvitationshould be set (and whether presence ofinvitationCodealone is sufficient).This keeps the options contract self‑describing for consumers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/types.ts(2 hunks)lib/utils/mapLoginMethodParamsForUrl.test.ts(1 hunks)lib/utils/mapLoginMethodParamsForUrl.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-01-16T21:47:40.307Z
Learnt from: DanielRivers
Repo: kinde-oss/js-utils PR: 61
File: lib/sessionManager/types.ts:20-20
Timestamp: 2025-01-16T21:47:40.307Z
Learning: Security documentation for configuration options in this codebase should be placed in the implementation file (e.g., index.ts) rather than the types file, as demonstrated with `useInsecureForRefreshToken` in `lib/sessionManager/index.ts`.
Applied to files:
lib/types.ts
📚 Learning: 2024-10-25T23:53:26.124Z
Learnt from: DanielRivers
Repo: kinde-oss/js-utils PR: 15
File: lib/utils/generateAuthUrl.test.ts:104-104
Timestamp: 2024-10-25T23:53:26.124Z
Learning: In `lib/utils/generateAuthUrl.test.ts`, the code is test code and may not require strict adherence to PKCE specifications.
Applied to files:
lib/utils/mapLoginMethodParamsForUrl.test.ts
🧬 Code graph analysis (1)
lib/utils/mapLoginMethodParamsForUrl.test.ts (4)
lib/types.ts (1)
LoginMethodParams(18-41)lib/utils/mapLoginMethodParamsForUrl.ts (1)
mapLoginMethodParamsForUrl(4-41)lib/main.ts (1)
mapLoginMethodParamsForUrl(11-11)lib/utils/index.ts (1)
mapLoginMethodParamsForUrl(6-6)
🔇 Additional comments (3)
lib/types.ts (1)
18-41: LoginMethodParams now correctly exposesinvitationCodeIncluding
"invitationCode"inLoginMethodParamskeeps the type in sync with the URL-mapping utility and looks correct.lib/utils/mapLoginMethodParamsForUrl.test.ts (1)
143-157: Invitation mapping test aligns with implementationThe new test covers the invitation flow well: it asserts
invitation_code,is_invitation: "true", and preserves defaultscope/audiencebehavior. This is a good addition to prevent regressions around invitation handling.lib/utils/mapLoginMethodParamsForUrl.ts (1)
33-35: Correct invitation parameter mappingMapping
options.invitationCodetoinvitation_codeand derivingis_invitationas"true"when it’s present fits the existing pattern and will be filtered out cleanly when absent. Looks good.
There was a problem hiding this 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/types.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-16T21:47:40.307Z
Learnt from: DanielRivers
Repo: kinde-oss/js-utils PR: 61
File: lib/sessionManager/types.ts:20-20
Timestamp: 2025-01-16T21:47:40.307Z
Learning: Security documentation for configuration options in this codebase should be placed in the implementation file (e.g., index.ts) rather than the types file, as demonstrated with `useInsecureForRefreshToken` in `lib/sessionManager/index.ts`.
Applied to files:
lib/types.ts
🔇 Additional comments (1)
lib/types.ts (1)
39-39: Verify thatisInvitationshould not be included inLoginMethodParams.The addition of
invitationCodetoLoginMethodParamsis present, butisInvitationis not. If the mapping logic automatically derivesis_invitationfrom the presence ofinvitationCode, this design is correct. However, theLoginOptionstype definesisInvitationwith documentation stating it is "required when code is provided," yet the field is optional in the type definition. Please clarify whetherisInvitationis auto-derived in the mapping layer or if it should be explicitly provided by callers.
Explain your changes
Adds required url params to work with invitaitions
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.