Conversation
WalkthroughA comprehensive onboarding module has been introduced with multi-step flows for P2P and NETWORKS intents, including new React components, type definitions, and API integration. Workflow CI trigger has been restricted, and Account/Peer interfaces have been extended with new optional properties. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OnboardingProvider
participant Onboarding
participant API as API/Backend
participant Storage as LocalStorage
User->>OnboardingProvider: Load Dashboard
OnboardingProvider->>Storage: Check onboarding state
OnboardingProvider->>API: Fetch peers, accounts, user
alt showOnboarding true
OnboardingProvider->>Onboarding: Render with initial state
else
OnboardingProvider-->>User: Skip Onboarding
end
alt P2P Intent
Onboarding->>User: Select P2P intent
Onboarding->>Onboarding: Step 1-6: Device setup & testing
else NETWORKS Intent
Onboarding->>User: Select NETWORKS intent
Onboarding->>Onboarding: Step 1-7: Resource & peer setup
end
Onboarding->>API: Create networks, resources, policies
Onboarding->>API: Poll peers for device connectivity (5s interval)
alt Device Connected
API-->>Onboarding: Device detected
Onboarding->>User: Progress to next step
end
User->>Onboarding: Complete final step
Onboarding->>API: updateAccountMeta (clear onboarding flags)
Onboarding->>OnboardingProvider: Call onFinish
OnboardingProvider->>API: Emit analytics events
OnboardingProvider-->>User: Navigate/Close
OnboardingProvider->>Storage: Update onboarding state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Specific areas requiring attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (16)
src/modules/onboarding/p2p/OnboardingSecondDevice.tsx (4)
27-27: Potential SSR issue withnavigatoraccess.Accessing
navigator.sharedirectly in the component body can throw during server-side rendering wherenavigatoris undefined. Consider using a safe check:- const isShareSupported = navigator.share !== undefined; + const isShareSupported = typeof navigator !== "undefined" && navigator.share !== undefined;
36-44: Unhandled promise fromnavigator.share.
navigator.share()returns a promise that can reject (e.g., user cancels the share dialog). Consider handling the rejection to avoid unhandled promise warnings.const openNavigatorShare = () => { if (navigator.share) { - navigator.share({ + navigator.share({ title: "Install NetBird", text: "Install NetBird on another device using this link.", url: getInstallUrl(), - }); + }).catch(() => { + // User cancelled or share failed - no action needed + }); } };
57-71: Mixed async patterns and silent error handling.The function uses
awaitwith.then()chaining, which is inconsistent. Also, sinceuseApiCallis initialized withignoreError=true, API failures are silently ignored—the user won't know if setup key creation failed.Consider using consistent async/await and providing user feedback on failure:
- await setupKeyRequest - .post({ + const result = await setupKeyRequest.post({ name: "Onboarding (Second Device)", type: "one-off", expires_in: 24 * 60 * 60, // 1 day expiration revoked: false, auto_groups: [], usage_limit: 1, ephemeral: false, allow_extra_dns_labels: false, - }) - .then((setupKey) => { - setOpen(true); - setSetupKey(setupKey); }); + if (result) { + setOpen(true); + setSetupKey(result); + }
115-118: Prefer button semantics for click-only actions.Using
href="#"withonClickis an anti-pattern—it can cause unwanted scroll behavior and doesn't convey proper semantics. Since this triggers an action rather than navigation, consider using aButtonwith link styling or addinge.preventDefault().- <InlineLink onClick={installUsingSetupKey} href={"#"}> + <InlineLink + onClick={(e) => { + e.preventDefault(); + installUsingSetupKey(); + }} + href={"#"} + > Install with a setup key <ArrowUpRightIcon size={12} /> </InlineLink>{" "}Or better, replace with a styled button component if available.
src/modules/onboarding/p2p/OnboardingTestP2P.tsx (1)
10-16: Consider makingfirstDevice/secondDevicerequired or guarding the test stepThe copy and ping command assume both devices exist; if either is missing you end up with blank names and
pingwithout an IP. Either:
- make
firstDeviceandsecondDevicerequired inProps, or- guard the paragraph and
Codeblock behind a presence check and show a fallback when a device is missing.Also applies to: 41-51
src/modules/onboarding/OnboardingPolicy.tsx (2)
12-13: Returnnullexplicitly for conditional rendering.React components should return
nullinstead ofundefinedwhen rendering nothing. While React handlesundefinedgracefully, returningnullis the idiomatic pattern.export const OnboardingPolicy = ({ policy, onToggle }: Props) => { - if (!policy) return; + if (!policy) return null;
25-35: Unnecessary optional chaining after guard clause.After the
if (!policy) returnguard on line 13,policyis guaranteed to be defined. The optional chaining (policy?.name,policy?.description,policy?.enabled) is redundant.- <ShieldIcon size={12} className={"shrink-0"} /> - {policy?.name} Policy + <ShieldIcon size={12} className={"shrink-0"} /> + {policy.name} Policy </div> <div className={"text-nb-gray-300 text-[0.8rem] text-left mt-0.5"}> - {policy?.name.includes("Default") + {policy.name.includes("Default") ? "Allows connections between all your devices" - : policy?.description} + : policy.description} </div> </div> <ToggleSwitch onCheckedChange={() => onToggle?.(policy)} - checked={policy?.enabled || false} + checked={policy.enabled} />src/modules/onboarding/OnboardingIntent.tsx (1)
21-56: Consider extracting duplicate matching logic.Both
isNetworksRecommendedandisP2PRecommendeduse the same pattern. A helper function would reduce duplication and make adding new intent categories easier.+const matchesAnyIntent = (useCases: string | undefined, intents: string[]): boolean => { + if (!useCases) return false; + const lowerCases = useCases.toLowerCase(); + return intents.some((intent) => lowerCases.includes(intent.toLowerCase())); +}; + +const NETWORK_INTENTS = [ + "Zero Trust Security", + "Employee Remote Access", + "Business VPN", + "Site-to-Site Connectivity", + "IoT (Internet of Things)", + "MSP (Managed Service Provider)", +]; + +const P2P_INTENTS = [ + "Homelab Automation", + "Home Remote Access", + "File Access", + "Gaming", +]; export const OnboardingIntent = ({onSelect, useCases, isBusiness}: Props) => { - const isNetworksRecommended = useMemo(() => { - if (!useCases) return false; - const intents = [...]; - for (const intent of intents) { - if (useCases.toLowerCase().includes(intent.toLowerCase())) { - return true; - } - } - return false; - }, [useCases]); - - const isP2PRecommended = useMemo(() => { ... }, [useCases]); + const isNetworksRecommended = useMemo( + () => matchesAnyIntent(useCases, NETWORK_INTENTS), + [useCases] + ); + const isP2PRecommended = useMemo( + () => matchesAnyIntent(useCases, P2P_INTENTS), + [useCases] + );src/modules/onboarding/networks/OnboardingAddRoutingPeer.tsx (1)
98-108: Empty catch block silently swallows errors.The clipboard write error is silently ignored. At minimum, log the error for debugging purposes.
const copySetupKey = async (key: string, showMessage = false) => { try { await navigator.clipboard.writeText(key || ""); if (showMessage) { notify({ title: "Setup Key Copied", description: "Successfully copied to clipboard.", }); } - } catch (e) {} + } catch (e) { + console.error("Failed to copy setup key to clipboard:", e); + } };src/modules/onboarding/OnboardingProvider.tsx (2)
48-57: Side effect in render body may cause issues.The localStorage migration logic runs during render, which is a React anti-pattern. While it's guarded by conditions, this could cause unexpected behavior or hydration mismatches in SSR/Next.js contexts.
Move this logic into a
useEffectto ensure it only runs once after mount:+ // Migrate old onboarding state to new key if needed + useEffect(() => { + if (typeof window === "undefined" || !account?.id) return; + const oldKey = "netbird-onboarding-flow"; + const newValue = window.localStorage.getItem(onboardingKey); + const oldValue = window.localStorage.getItem(oldKey); + if (oldValue && !newValue) { + window.localStorage.setItem(onboardingKey, oldValue); + window.localStorage.removeItem(oldKey); + } + }, [account?.id, onboardingKey]); - // Migrate old onboarding state to new key if needed - if (typeof window !== "undefined" && account?.id) { - const oldKey = "netbird-onboarding-flow"; - const oldValue = window.localStorage.getItem(oldKey); - const newValue = window.localStorage.getItem(onboardingKey); - if (oldValue && !newValue) { - window.localStorage.setItem(onboardingKey, oldValue); - window.localStorage.removeItem(oldKey); - } - }
117-119: Remove commented-out code.The commented line suggests an alternative navigation path. If this is intentional for future use, consider adding a TODO comment explaining the rationale. Otherwise, remove the dead code.
if (n) { - // router.push(`/network?id=${n.id}`); router.push("/control-center?tab=networks"); } else {src/modules/onboarding/OnboardingSurvey.tsx (1)
153-155: Use strict equality operator.Using
!=instead of!==can lead to unexpected type coercion. Prefer strict equality for consistency.return [hl, hra, fa, g, zt, ra, bv, st, iot, mp, ou] - .filter((s) => s != "") + .filter((s) => s !== "") .join(", ");src/modules/onboarding/OnboardingDevices.tsx (1)
177-179: Returnnullinstead of implicitundefinedfor React components.When a React component has nothing to render, it should explicitly return
nullrather thanundefined. While this works in practice, returningundefinedis technically incorrect and may cause issues with TypeScript strict mode or future React versions.export const DeviceCard = ({ device, resource }: DeviceCardProps) => { - if (!device && !resource) return; + if (!device && !resource) return null; return (src/modules/onboarding/Onboarding.tsx (3)
219-233: Missing dependencies in useEffect may cause stale closure issues.The effect uses
routerRequest.getandpeersbut neither is listed in the dependency array. This could cause:
- The effect to not re-run when
peersupdates- Stale
peersreference being used in the callbackConsider adding the missing dependencies or extracting
routerRequestoutside the component if it's stable:useEffect(() => { if (firstNetwork && intent === Intent.NETWORKS && !firstRoutingPeer) { const firstRouterId = firstNetwork?.routers?.[0]; if (firstRouterId) { routerRequest .get(`/${firstNetwork?.id}/routers/${firstRouterId}`) .then((r) => { const routingPeer = peers?.find((p) => p.id === r.peer) ?? undefined; if (!routingPeer) return; setFirstRoutingPeer(routingPeer); }); } } - }, [intent, firstNetwork, peers]); + }, [intent, firstNetwork, peers, routerRequest]);Note: If
routerRequestis a new object on each render (fromuseApiCall), you may need to memoize it or restructure the API call approach.
606-608: Returnnullinstead of implicitundefined.Same issue as noted in
DeviceCard- React components should explicitly returnnullwhen rendering nothing.const Stepper = ({ step, maxSteps }: { step: number; maxSteps: number }) => { - if (step <= 0) return; + if (step <= 0) return null;
235-246: MissingresourceRequestdependency in useEffect.Similar to the previous effect,
resourceRequestis used but not listed in dependencies.useEffect(() => { if (firstNetwork && intent === Intent.NETWORKS) { const firstResourceId = firstNetwork?.resources?.[0]; if (firstResourceId) { resourceRequest .get(`/${firstNetwork?.id}/resources/${firstResourceId}`) .then((r) => { setResource(r); }); } } - }, [intent, firstNetwork]); + }, [intent, firstNetwork, resourceRequest]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
src/assets/onboarding/acl.pngis excluded by!**/*.pngsrc/assets/onboarding/activity.pngis excluded by!**/*.pngsrc/assets/onboarding/posture.pngis excluded by!**/*.png
📒 Files selected for processing (21)
.github/workflows/build_and_push.yml(0 hunks)src/contexts/AnalyticsProvider.tsx(1 hunks)src/interfaces/Account.ts(1 hunks)src/interfaces/Peer.ts(1 hunks)src/layouts/DashboardLayout.tsx(3 hunks)src/modules/onboarding/Onboarding.tsx(1 hunks)src/modules/onboarding/OnboardingDevices.tsx(1 hunks)src/modules/onboarding/OnboardingEnd.tsx(1 hunks)src/modules/onboarding/OnboardingIntent.tsx(1 hunks)src/modules/onboarding/OnboardingPolicy.tsx(1 hunks)src/modules/onboarding/OnboardingProvider.tsx(1 hunks)src/modules/onboarding/OnboardingSurvey.tsx(1 hunks)src/modules/onboarding/networks/OnboardingAddResource.tsx(1 hunks)src/modules/onboarding/networks/OnboardingAddRoutingPeer.tsx(1 hunks)src/modules/onboarding/networks/OnboardingAddUserDevice.tsx(1 hunks)src/modules/onboarding/networks/OnboardingExplainPolicy.tsx(1 hunks)src/modules/onboarding/networks/OnboardingTestResource.tsx(1 hunks)src/modules/onboarding/p2p/OnboardingExplainDefaultPolicy.tsx(1 hunks)src/modules/onboarding/p2p/OnboardingFirstDevice.tsx(1 hunks)src/modules/onboarding/p2p/OnboardingSecondDevice.tsx(1 hunks)src/modules/onboarding/p2p/OnboardingTestP2P.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/build_and_push.yml
🧰 Additional context used
🧬 Code graph analysis (12)
src/modules/onboarding/networks/OnboardingAddResource.tsx (8)
src/interfaces/Network.ts (2)
Network(3-11)NetworkResource(22-30)src/contexts/GroupsProvider.tsx (1)
useGroups(163-163)src/utils/api.tsx (1)
useApiCall(169-212)src/interfaces/Policy.ts (1)
Policy(4-12)src/interfaces/Group.ts (1)
Group(1-12)src/components/Notification.tsx (1)
notify(151-155)src/components/RadioCard.tsx (2)
RadioCardGroup(50-67)RadioCard(13-40)src/modules/networks/resources/ResourceSingleAddressInput.tsx (1)
ResourceSingleAddressInput(19-82)
src/modules/onboarding/OnboardingSurvey.tsx (5)
src/contexts/AnalyticsProvider.tsx (1)
HubspotFormField(20-24)src/contexts/UsersProvider.tsx (1)
useLoggedInUser(87-111)src/components/SegmentedTabs.tsx (1)
SegmentedTabs(99-99)src/components/Checkbox.tsx (1)
Checkbox(54-54)src/utils/helpers.ts (1)
cn(6-8)
src/modules/onboarding/networks/OnboardingAddUserDevice.tsx (6)
src/interfaces/Peer.ts (1)
Peer(4-33)src/interfaces/Policy.ts (1)
Policy(4-12)src/utils/api.tsx (1)
useApiCall(169-212)src/interfaces/Group.ts (2)
Group(1-12)GroupPeer(14-17)src/components/modal/Modal.tsx (2)
Modal(209-209)ModalContent(211-211)src/modules/setup-netbird-modal/SetupModal.tsx (1)
SetupModalContent(70-237)
src/modules/onboarding/p2p/OnboardingExplainDefaultPolicy.tsx (2)
src/interfaces/Policy.ts (1)
Policy(4-12)src/modules/onboarding/OnboardingPolicy.tsx (1)
OnboardingPolicy(12-39)
src/modules/onboarding/OnboardingDevices.tsx (6)
src/interfaces/Network.ts (1)
NetworkResource(22-30)src/interfaces/Peer.ts (1)
Peer(4-33)src/utils/helpers.ts (1)
cn(6-8)src/components/ui/TruncatedText.tsx (1)
TruncatedText(13-92)src/hooks/useOperatingSystem.ts (1)
getOperatingSystem(25-40)src/modules/peers/PeerOSCell.tsx (1)
OSLogo(82-99)
src/modules/onboarding/p2p/OnboardingTestP2P.tsx (4)
src/interfaces/Peer.ts (1)
Peer(4-33)src/interfaces/Policy.ts (1)
Policy(4-12)src/components/Steps.tsx (1)
Steps(9-19)src/components/InlineLink.tsx (1)
InlineLink(35-41)
src/modules/onboarding/p2p/OnboardingSecondDevice.tsx (9)
src/interfaces/Peer.ts (1)
Peer(4-33)src/utils/api.tsx (1)
useApiCall(169-212)src/interfaces/SetupKey.ts (1)
SetupKey(3-20)src/contexts/DialogProvider.tsx (1)
useDialog(132-132)src/utils/netbird.ts (1)
getInstallUrl(14-16)src/utils/helpers.ts (1)
cn(6-8)src/components/InlineLink.tsx (1)
InlineLink(35-41)src/components/modal/Modal.tsx (2)
Modal(209-209)ModalContent(211-211)src/modules/setup-netbird-modal/SetupModal.tsx (1)
SetupModalContent(70-237)
src/modules/onboarding/OnboardingProvider.tsx (8)
src/contexts/AnalyticsProvider.tsx (2)
HubspotFormField(20-24)useAnalytics(154-154)src/interfaces/Account.ts (1)
Account(1-27)src/modules/account/useAccount.tsx (1)
useAccount(6-21)src/contexts/UsersProvider.tsx (1)
useLoggedInUser(87-111)src/hooks/useLocalStorage.tsx (1)
useLocalStorage(20-124)src/modules/onboarding/Onboarding.tsx (2)
OnboardingState(33-39)Onboarding(89-604)src/utils/netbird.ts (2)
isNetBirdHosted(18-23)isLocalDev(25-27)src/interfaces/Network.ts (1)
Network(3-11)
src/modules/onboarding/OnboardingPolicy.tsx (3)
src/interfaces/Policy.ts (1)
Policy(4-12)src/utils/helpers.ts (1)
cn(6-8)src/components/ToggleSwitch.tsx (1)
ToggleSwitch(71-71)
src/modules/onboarding/networks/OnboardingTestResource.tsx (7)
src/interfaces/Network.ts (1)
NetworkResource(22-30)src/interfaces/Peer.ts (1)
Peer(4-33)src/components/Steps.tsx (1)
Steps(9-19)src/utils/helpers.ts (1)
cn(6-8)src/components/InlineLink.tsx (1)
InlineLink(35-41)src/components/modal/Modal.tsx (2)
Modal(209-209)ModalContent(211-211)src/modules/setup-netbird-modal/SetupModal.tsx (1)
SetupModalContent(70-237)
src/modules/onboarding/Onboarding.tsx (8)
src/interfaces/Peer.ts (1)
Peer(4-33)src/contexts/AnalyticsProvider.tsx (1)
HubspotFormField(20-24)src/interfaces/Network.ts (3)
Network(3-11)NetworkResource(22-30)NetworkRouter(13-20)src/interfaces/Policy.ts (1)
Policy(4-12)src/components/modal/Modal.tsx (2)
Modal(209-209)ModalPortal(215-215)src/utils/helpers.ts (1)
cn(6-8)src/utils/netbird.ts (1)
isNetBirdHosted(18-23)src/components/InlineLink.tsx (1)
InlineLink(35-41)
src/layouts/DashboardLayout.tsx (2)
src/utils/netbird.ts (1)
isNetBirdHosted(18-23)src/modules/onboarding/OnboardingProvider.tsx (1)
OnboardingProvider(30-170)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_n_push
🔇 Additional comments (13)
src/modules/onboarding/p2p/OnboardingSecondDevice.tsx (1)
121-130: LGTM!The modal implementation correctly conditionally renders based on
setupKeystate and properly passessetupKey.keytoSetupModalContent.src/modules/onboarding/OnboardingEnd.tsx (1)
15-37: Onboarding completion copy and OIDC personalization look solidTitle derivation from the OIDC user with a neutral fallback is correct, and the copy/layout align with existing onboarding patterns.
src/modules/onboarding/p2p/OnboardingTestP2P.tsx (1)
24-39: P2P test flow and copy are clear and consistentStep layout, messaging, and wiring of the primary CTA to
onNextfit well with the rest of the onboarding flow.Also applies to: 53-75
src/interfaces/Peer.ts (1)
26-32: Optionaldisapproval_reasonaddition looks safeMaking
disapproval_reasonoptional preserves compatibility with existing Peer payloads while enabling richer approval/disapproval UX.src/layouts/DashboardLayout.tsx (1)
8-9: Confirm intent of gatingOnboardingProviderwith!isNetBirdHosted()This moves onboarding rendering behind an environment check, so NetBird‑hosted tenants will never mount
OnboardingProviderat all (even if onboarding flags are set). If that’s the intended product behavior, the wiring is fine; otherwise consider relying solely onOnboardingProvider’s internal logic for hosted vs self‑hosted handling.Also applies to: 24-25, 33-45
src/contexts/AnalyticsProvider.tsx (1)
20-24: Form field type addition looks goodThe
HubspotFormFieldshape is minimal and correctly requiresnameandvalue, which should keep onboarding form payloads well-typed.src/interfaces/Account.ts (1)
26-32: Account onboarding metadata model looks reasonableAdding an optional
onboardingblock withonboarding_flow_pendingandsignup_form_pendingflags cleanly separates onboarding state from core account fields.src/modules/onboarding/networks/OnboardingExplainPolicy.tsx (1)
12-50: Networks policy explanation component is consistent and focusedConditional extra guidance when
policyis present plus delegation toOnboardingPolicykeeps this step lightweight and aligned with the overall onboarding UX.src/modules/onboarding/p2p/OnboardingExplainDefaultPolicy.tsx (1)
12-50: P2P default-policy explanation mirrors the networks variant nicelyShared layout with tailored copy and reuse of
OnboardingPolicykeeps the P2P path consistent with the networks onboarding experience.src/modules/onboarding/p2p/OnboardingFirstDevice.tsx (1)
15-61: LGTM!The component structure is clean with clear separation of state management, side effects, and rendering. The modal integration with
SetupModalContentfollows the existing patterns in the codebase.src/modules/onboarding/networks/OnboardingTestResource.tsx (1)
33-40: LGTM!The
pingAddresscomputation correctly handles the different resource types (host with/32suffix, wildcard domains, and subnets) with appropriate user-friendly messages.src/modules/onboarding/OnboardingIntent.tsx (1)
109-175: LGTM!The
IntentCardcomponent is well-structured with clear props, accessible button semantics, and good hover/transition styling. The tooltip for recommendations provides helpful context.src/modules/onboarding/networks/OnboardingAddRoutingPeer.tsx (1)
37-67: LGTM with a note on the setup key generation flow.The workflow for creating/finding the "Routing Peers" group and generating the setup key is well-structured. The
notifywrapper with promise handling provides good UX feedback.
2 participants
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.