feat: onboarding wizard #1#2719
Merged
comatory merged 5 commits intoondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-topicfrom Apr 23, 2026
Conversation
Contributor
|
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 ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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:
WalkthroughAdds an onboarding feature: new protobuf messages and RPCs (Get/Create/Finish), generated Connect client/server artifacts, a DB migration and Drizzle schema for an Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Router image scan passed✅ No security vulnerabilities found in image: |
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-01
branch
from
9b1427c to
1094f7a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-topic #2719 +/- ##
======================================================================================================
Coverage ? 42.21%
======================================================================================================
Files ? 992
Lines ? 130510
Branches ? 5706
======================================================================================================
Hits ? 55092
Misses ? 73745
Partials ? 1673
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
proto/wg/cosmo/platform/v1/platform.proto (1)
3197-3202: Missing field number 2 inGetOnboardingResponse.Field numbers skip from 1 to 3, leaving field 2 unused. For a new message definition, this is unusual and could indicate an accidental omission or a removed field. If a field was intentionally removed during development, consider adding a
reserved 2;statement to document this and prevent accidental reuse. If this was unintentional, renumber the fields sequentially.Option 1: Add reserved statement if field was intentionally removed
message GetOnboardingResponse { Response response = 1; + reserved 2; optional string finishedAt = 3; int32 federatedGraphsCount = 4; bool enabled = 5; }Option 2: Renumber fields sequentially if this is a new message
message GetOnboardingResponse { Response response = 1; - optional string finishedAt = 3; - int32 federatedGraphsCount = 4; - bool enabled = 5; + optional string finishedAt = 2; + int32 federatedGraphsCount = 3; + bool enabled = 4; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proto/wg/cosmo/platform/v1/platform.proto` around lines 3197 - 3202, GetOnboardingResponse currently skips field number 2 (fields go 1, 3, 4, 5); either explicitly reserve the missing tag or renumber fields—if the field was intentionally removed, add a `reserved 2;` declaration inside the GetOnboardingResponse message to prevent reuse, otherwise renumber fields so they are sequential (e.g., change finishedAt from tag 3 to tag 2 and adjust any dependent code or clients that reference these tags).controlplane/src/core/bufservices/onboarding/createOnboarding.ts (1)
36-36: Track the TODO with a concrete follow-up item.Line 36 documents missing invitation/rename handling in a user-facing onboarding path; please capture this as a tracked issue before rollout.
If you want, I can draft the issue description and acceptance criteria.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controlplane/src/core/bufservices/onboarding/createOnboarding.ts` at line 36, Replace the ambiguous TODO in createOnboarding.ts with a tracked issue reference: create a ticket in your issue tracker describing the missing "invitation flow + organization renaming" work, include acceptance criteria (invite acceptance path, org rename UX, API behavior, edge cases, tests), then update the TODO comment in the createOnboarding function/file to reference the created ticket ID (e.g., TODO(issue-123): handle invitation flow + organization renaming) so the follow-up is discoverable and actionable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controlplane/src/core/repositories/OnboardingRepository.ts`:
- Around line 12-18: The insert into schema.onboarding is passing a non-existent
column "step" which causes insert/type errors; in the call to
this.db.insert(schema.onboarding).values(...) remove the step property from the
payload (and any related typing or interface that includes step for this
operation) so the object only contains valid columns (userId, organizationId,
slack, email); update any tests or callers expecting step to be stored if
necessary.
- Around line 11-19: The create() method can raise a DB unique-constraint error
on duplicate (user_id, organization_id, version); make it idempotent by changing
the insertion logic to perform an upsert / conflict-ignore on schema.onboarding
(or first check for an existing onboarding row for this.organizationId and
userId/version and return success if found) instead of blindly calling
this.db.insert; update the code around create(), this.db.insert and
schema.onboarding to use a "do nothing on conflict" or equivalent upsert
behavior so retries/double-submits simply return success rather than error.
In `@proto/wg/cosmo/platform/v1/platform.proto`:
- Around line 3204-3209: The CreateOnboardingRequest message contains a typo:
rename the field invititationEmails to invitationEmails (keep the same tag
number 2) so generated code uses the correct name; update all references/usages
of CreateOnboardingRequest.invititationEmails to
CreateOnboardingRequest.invitationEmails and regenerate protobuf artifacts and
client code to propagate the change.
---
Nitpick comments:
In `@controlplane/src/core/bufservices/onboarding/createOnboarding.ts`:
- Line 36: Replace the ambiguous TODO in createOnboarding.ts with a tracked
issue reference: create a ticket in your issue tracker describing the missing
"invitation flow + organization renaming" work, include acceptance criteria
(invite acceptance path, org rename UX, API behavior, edge cases, tests), then
update the TODO comment in the createOnboarding function/file to reference the
created ticket ID (e.g., TODO(issue-123): handle invitation flow + organization
renaming) so the follow-up is discoverable and actionable.
In `@proto/wg/cosmo/platform/v1/platform.proto`:
- Around line 3197-3202: GetOnboardingResponse currently skips field number 2
(fields go 1, 3, 4, 5); either explicitly reserve the missing tag or renumber
fields—if the field was intentionally removed, add a `reserved 2;` declaration
inside the GetOnboardingResponse message to prevent reuse, otherwise renumber
fields so they are sequential (e.g., change finishedAt from tag 3 to tag 2 and
adjust any dependent code or clients that reference these tags).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a5f51bdd-8da4-461d-9a11-640537da36fb
⛔ Files ignored due to path filters (2)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.gois excluded by!**/*.pb.go,!**/gen/**connect-go/gen/proto/wg/cosmo/platform/v1/platformv1connect/platform.connect.gois excluded by!**/gen/**
📒 Files selected for processing (12)
connect/src/wg/cosmo/platform/v1/platform-PlatformService_connectquery.tsconnect/src/wg/cosmo/platform/v1/platform_connect.tsconnect/src/wg/cosmo/platform/v1/platform_pb.tscontrolplane/migrations/0137_icy_sasquatch.sqlcontrolplane/migrations/meta/0137_snapshot.jsoncontrolplane/migrations/meta/_journal.jsoncontrolplane/src/core/bufservices/PlatformService.tscontrolplane/src/core/bufservices/onboarding/createOnboarding.tscontrolplane/src/core/bufservices/onboarding/getOnboarding.tscontrolplane/src/core/repositories/OnboardingRepository.tscontrolplane/src/db/schema.tsproto/wg/cosmo/platform/v1/platform.proto
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
proto/wg/cosmo/platform/v1/platform.proto (1)
3204-3207:⚠️ Potential issue | 🟡 MinorFix typo in onboarding request field name before codegen.
invititationEmailscontains a spelling error and will generate awkward API fields across clients. Rename it toinvitationEmailswhile keeping tag2.Proposed fix
message CreateOnboardingRequest { string organizationName = 1; - repeated string invititationEmails = 2; + repeated string invitationEmails = 2; bool slack = 3; bool email = 4; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proto/wg/cosmo/platform/v1/platform.proto` around lines 3204 - 3207, The CreateOnboardingRequest proto has a misspelled field name invititationEmails; rename this field to invitationEmails in the message definition (message CreateOnboardingRequest) but preserve the field number 2 so generated APIs keep wire compatibility; update any references to CreateOnboardingRequest.invititationEmails in the repo (tests, docs, usages) to the new CreateOnboardingRequest.invitationEmails before running codegen.
🧹 Nitpick comments (1)
connect/src/wg/cosmo/platform/v1/platform_pb.ts (1)
25042-25047: Addreserved 2;declaration or correct field numbering inGetOnboardingResponse.Field number 2 is skipped (numbering jumps from 1 to 3), which is inconsistent with the sequential field numbering used throughout the rest of the proto file. If this is intentional backward compatibility, add
reserved 2;to document it. If unintentional, correct the numbering before the API is released.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connect/src/wg/cosmo/platform/v1/platform_pb.ts` around lines 25042 - 25047, The generated proto message for GetOnboardingResponse (the static readonly fields array in its TypeScript class) skips field number 2 — update the proto mapping to either declare `reserved 2;` in the GetOnboardingResponse proto to document the intentional gap or renumber the existing fields so they are sequential (change the field currently using number 3/finishedAt to 2 if that was unintentional); locate the static readonly fields definition for GetOnboardingResponse (the FieldList initializer shown with entries for "response", "finishedAt", "federatedGraphsCount", "enabled") and apply the appropriate change so the TypeScript fields reflect the corrected proto schema.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@connect/src/wg/cosmo/platform/v1/platform_pb.ts`:
- Around line 25075-25078: The generated TypeScript field invititationEmails
contains a typo and must be renamed to invitationEmails in the source .proto and
regenerated; open the proto message that defines the field name (currently
`invititationEmails`), correct the identifier to `invitationEmails`, re-run the
protobuf/TS generation to update the generated symbol in platform_pb.ts (the
field and any accessor names), then update all code references to use
invitationEmails (or add a temporary compatibility alias/mapping from the old
name while you update callers) so the public API uses the correct name.
In `@controlplane/src/core/bufservices/onboarding/createOnboarding.ts`:
- Around line 36-43: The onboarding handler is dropping advertised request
fields (organizationName and invitation emails) when calling
OnboardingRepository.create; update the create call to pass req.organizationName
and req.invitationEmails (or req.invitations) alongside userId/slack/email, and
then update OnboardingRepository.create (and its persisted model/schema) to
accept and persist organizationName and invitations (or return a validation
error if you choose to reject them). Ensure the symbols updated are the
CreateOnboardingRequest usage in createOnboarding.ts, the onboardingRepo.create
invocation, and the OnboardingRepository.create method/DTO/schema so the fields
are carried end-to-end.
- Around line 38-43: The onboarding insertion can throw uniqueness errors on
repeated requests; update the flow so duplicate-create is mapped to a
deterministic domain response instead of bubbling as a generic error: either
implement/use an idempotent repository method (e.g.,
OnboardingRepository.findOrCreate or upsert) that returns the existing row when
one exists, or wrap the call to onboardingRepo.create in a try/catch and detect
the DB unique-constraint error (translate it into a specific domain result like
"AlreadyOnboarded" or return the existing onboarding record), ensuring you
reference onboardingRepo.create and authContext.userId so callers receive a
stable idempotent success rather than a raw DB error.
---
Duplicate comments:
In `@proto/wg/cosmo/platform/v1/platform.proto`:
- Around line 3204-3207: The CreateOnboardingRequest proto has a misspelled
field name invititationEmails; rename this field to invitationEmails in the
message definition (message CreateOnboardingRequest) but preserve the field
number 2 so generated APIs keep wire compatibility; update any references to
CreateOnboardingRequest.invititationEmails in the repo (tests, docs, usages) to
the new CreateOnboardingRequest.invitationEmails before running codegen.
---
Nitpick comments:
In `@connect/src/wg/cosmo/platform/v1/platform_pb.ts`:
- Around line 25042-25047: The generated proto message for GetOnboardingResponse
(the static readonly fields array in its TypeScript class) skips field number 2
— update the proto mapping to either declare `reserved 2;` in the
GetOnboardingResponse proto to document the intentional gap or renumber the
existing fields so they are sequential (change the field currently using number
3/finishedAt to 2 if that was unintentional); locate the static readonly fields
definition for GetOnboardingResponse (the FieldList initializer shown with
entries for "response", "finishedAt", "federatedGraphsCount", "enabled") and
apply the appropriate change so the TypeScript fields reflect the corrected
proto schema.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e187db28-20bf-4d40-b2fd-3d4ccda93887
⛔ Files ignored due to path filters (2)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.gois excluded by!**/*.pb.go,!**/gen/**connect-go/gen/proto/wg/cosmo/platform/v1/platformv1connect/platform.connect.gois excluded by!**/gen/**
📒 Files selected for processing (8)
connect/src/wg/cosmo/platform/v1/platform-PlatformService_connectquery.tsconnect/src/wg/cosmo/platform/v1/platform_connect.tsconnect/src/wg/cosmo/platform/v1/platform_pb.tscontrolplane/src/core/bufservices/PlatformService.tscontrolplane/src/core/bufservices/onboarding/createOnboarding.tscontrolplane/src/core/bufservices/onboarding/getOnboarding.tscontrolplane/src/core/repositories/OnboardingRepository.tsproto/wg/cosmo/platform/v1/platform.proto
🚧 Files skipped from review as they are similar to previous changes (4)
- controlplane/src/core/bufservices/PlatformService.ts
- controlplane/src/core/repositories/OnboardingRepository.ts
- connect/src/wg/cosmo/platform/v1/platform_connect.ts
- controlplane/src/core/bufservices/onboarding/getOnboarding.ts
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-01
branch
from
ec7ea95 to
b6fb36f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
controlplane/src/core/repositories/OnboardingRepository.ts (2)
19-29: Add contextual try/catch around DB operations.Repository methods currently bubble raw DB exceptions directly. Wrapping with contextual errors here improves diagnosability and keeps failure handling consistent.
As per coding guidelines: "Add proper error handling with try-catch blocks".
Also applies to: 32-39, 42-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controlplane/src/core/repositories/OnboardingRepository.ts` around lines 19 - 29, Wrap the DB upsert call in OnboardingRepository (the await this.db.insert(schema.onboarding).values(values).onConflictDoUpdate(...) sequence) in a try/catch so raw DB exceptions are captured and re-thrown or wrapped with contextual information; in the catch, include identifying details (userId, organizationId, version) and the original error when constructing the new error or logging, then rethrow the contextual error so callers get consistent, diagnosable failures; apply the same pattern to the other repository methods noted (the DB calls around lines 32-39 and 42-46).
11-11: Use an interface for input shape and explicit return types on methods.
createOrUpdateuses an inline object type, and all public methods rely on inferred returns. Extracting an interface and adding explicitPromise<...>return types will align this file with TS repo standards.♻️ Proposed typing cleanup
+interface CreateOrUpdateInput { + userId: string; + slack: boolean; + email: boolean; +} + +type OnboardingRow = typeof schema.onboarding.$inferSelect; + export class OnboardingRepository { @@ - public async createOrUpdate({ userId, slack, email }: { userId: string; slack: boolean; email: boolean }) { + public async createOrUpdate({ userId, slack, email }: CreateOrUpdateInput): Promise<void> { @@ - public async finish(userId: string) { + public async finish(userId: string): Promise<OnboardingRow | undefined> { @@ - public async getByUserId(userId: string) { + public async getByUserId(userId: string): Promise<OnboardingRow | undefined> {As per coding guidelines: "Prefer interfaces over type aliases for object shapes in TypeScript" and "Use explicit type annotations for function parameters and return types in TypeScript".
Also applies to: 31-31, 41-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controlplane/src/core/repositories/OnboardingRepository.ts` at line 11, Extract the inline parameter shape into a named interface (e.g. OnboardingCreateOrUpdateInput with userId: string, slack: boolean, email: boolean) and update the createOrUpdate method signature to accept that interface (createOrUpdate(input: OnboardingCreateOrUpdateInput)): add an explicit Promise return type (e.g. createOrUpdate(...): Promise<Onboarding> or the concrete domain type this repository returns). Do the same explicit Promise<...> return-type annotation for the other public methods in this class (the public methods declared at the other two public method locations referenced in the review), replacing inferred returns with concrete Promise types matching their actual return values. Ensure the new interface and explicit return types are exported/visible as needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controlplane/src/core/repositories/OnboardingRepository.ts`:
- Around line 31-36: The schema includes a version column in the unique
constraint but methods finish() and getByUserId() never use or increment it,
causing ambiguity; update either the data operations to be explicit about
versioning (filter by version in getByUserId() and finish(), and increment
version when creating/updating rows via the repository functions such as
finish() and any create/update methods) or simplify the schema by removing the
version column and its unique constraint so the repository only relies on
(userId, organizationId); locate references to schema.onboarding, finish(), and
getByUserId() and apply the chosen approach consistently across all repository
methods.
---
Nitpick comments:
In `@controlplane/src/core/repositories/OnboardingRepository.ts`:
- Around line 19-29: Wrap the DB upsert call in OnboardingRepository (the await
this.db.insert(schema.onboarding).values(values).onConflictDoUpdate(...)
sequence) in a try/catch so raw DB exceptions are captured and re-thrown or
wrapped with contextual information; in the catch, include identifying details
(userId, organizationId, version) and the original error when constructing the
new error or logging, then rethrow the contextual error so callers get
consistent, diagnosable failures; apply the same pattern to the other repository
methods noted (the DB calls around lines 32-39 and 42-46).
- Line 11: Extract the inline parameter shape into a named interface (e.g.
OnboardingCreateOrUpdateInput with userId: string, slack: boolean, email:
boolean) and update the createOrUpdate method signature to accept that interface
(createOrUpdate(input: OnboardingCreateOrUpdateInput)): add an explicit Promise
return type (e.g. createOrUpdate(...): Promise<Onboarding> or the concrete
domain type this repository returns). Do the same explicit Promise<...>
return-type annotation for the other public methods in this class (the public
methods declared at the other two public method locations referenced in the
review), replacing inferred returns with concrete Promise types matching their
actual return values. Ensure the new interface and explicit return types are
exported/visible as needed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1317b5bb-eba4-46be-925f-dd7217887250
⛔ Files ignored due to path filters (2)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.gois excluded by!**/*.pb.go,!**/gen/**connect-go/gen/proto/wg/cosmo/platform/v1/platformv1connect/platform.connect.gois excluded by!**/gen/**
📒 Files selected for processing (9)
connect/src/wg/cosmo/platform/v1/platform-PlatformService_connectquery.tsconnect/src/wg/cosmo/platform/v1/platform_connect.tsconnect/src/wg/cosmo/platform/v1/platform_pb.tscontrolplane/src/core/bufservices/PlatformService.tscontrolplane/src/core/bufservices/onboarding/createOnboarding.tscontrolplane/src/core/bufservices/onboarding/finishOnboarding.tscontrolplane/src/core/bufservices/onboarding/getOnboarding.tscontrolplane/src/core/repositories/OnboardingRepository.tsproto/wg/cosmo/platform/v1/platform.proto
✅ Files skipped from review due to trivial changes (1)
- controlplane/src/core/bufservices/onboarding/createOnboarding.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- controlplane/src/core/bufservices/onboarding/getOnboarding.ts
- controlplane/src/core/bufservices/PlatformService.ts
- controlplane/src/core/bufservices/onboarding/finishOnboarding.ts
- proto/wg/cosmo/platform/v1/platform.proto
- connect/src/wg/cosmo/platform/v1/platform_connect.ts
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-01
branch
from
b6fb36f to
54189f0
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
controlplane/src/core/repositories/OnboardingRepository.ts (1)
42-48: Minor: The?? undefinedis redundant.
findFirstalready returnsundefinedwhen no match is found, so the nullish coalescing is unnecessary.♻️ Simplify the return
public async getByUserId(userId: string) { - const result = await this.db.query.onboarding.findFirst({ + return this.db.query.onboarding.findFirst({ where: and(eq(schema.onboarding.organizationId, this.organizationId), eq(schema.onboarding.userId, userId)), }); - - return result ?? undefined; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controlplane/src/core/repositories/OnboardingRepository.ts` around lines 42 - 48, The getByUserId method unnecessarily uses "return result ?? undefined" after this.db.query.onboarding.findFirst which already yields undefined on no match; simplify by returning the result directly from getByUserId (update the method with "return result;" in the getByUserId function to remove the redundant nullish coalescing).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controlplane/src/core/repositories/OnboardingRepository.ts`:
- Around line 22-29: The onConflictDoUpdate in OnboardingRepository's
createOrUpdate currently resets finishedAt to null on every conflict which can
revert completed onboarding; update the conflict-handling so finishedAt is
preserved by removing finishedAt from the set map in onConflictDoUpdate (i.e.,
stop writing finishedAt there) or change the update to conditionally set
finishedAt = CASE WHEN schema.onboarding.finishedAt IS NULL THEN NULL ELSE
schema.onboarding.finishedAt END (or equivalent ORM conditional) so existing
non-null finishedAt is kept; alternatively, add a guard in the calling handler
to avoid calling OnboardingRepository.createOrUpdate for already-finished
onboarding records.
---
Nitpick comments:
In `@controlplane/src/core/repositories/OnboardingRepository.ts`:
- Around line 42-48: The getByUserId method unnecessarily uses "return result ??
undefined" after this.db.query.onboarding.findFirst which already yields
undefined on no match; simplify by returning the result directly from
getByUserId (update the method with "return result;" in the getByUserId function
to remove the redundant nullish coalescing).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 678bbb39-83b6-4034-9114-a6765cacb9e5
⛔ Files ignored due to path filters (2)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.gois excluded by!**/*.pb.go,!**/gen/**connect-go/gen/proto/wg/cosmo/platform/v1/platformv1connect/platform.connect.gois excluded by!**/gen/**
📒 Files selected for processing (9)
connect/src/wg/cosmo/platform/v1/platform-PlatformService_connectquery.tsconnect/src/wg/cosmo/platform/v1/platform_connect.tsconnect/src/wg/cosmo/platform/v1/platform_pb.tscontrolplane/src/core/bufservices/PlatformService.tscontrolplane/src/core/bufservices/onboarding/createOnboarding.tscontrolplane/src/core/bufservices/onboarding/finishOnboarding.tscontrolplane/src/core/bufservices/onboarding/getOnboarding.tscontrolplane/src/core/repositories/OnboardingRepository.tsproto/wg/cosmo/platform/v1/platform.proto
✅ Files skipped from review due to trivial changes (2)
- controlplane/src/core/bufservices/PlatformService.ts
- connect/src/wg/cosmo/platform/v1/platform_connect.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- controlplane/src/core/bufservices/onboarding/finishOnboarding.ts
- controlplane/src/core/bufservices/onboarding/createOnboarding.ts
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-01
branch
from
54189f0 to
f039f02
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-01
branch
from
bfc365b to
02532a3
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-01
branch
from
02532a3 to
bfc365b
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-topic
branch
from
abc57e2 to
9737099
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-01
branch
from
bfc365b to
d7c192a
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-01
branch
from
d7c192a to
2e761b2
Compare
comatory
requested review from
JivusAyrus,
StarpTech,
endigma,
thisisnithin and
wilsonrivera
as code owners
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-01
branch
2 times, most recently
from
25fa993 to
2e761b2
Compare
comatory
force-pushed
the
ondrej/eng-9192-onboarding-wizard-from-signup-to-live-metrics-topic
branch
from
d86216f to
abc57e2
Compare
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.
How does the onboarding work?
Linear contains most of the information but in a nutshell: onboarding in Studio has companion CLI command
wgc demo. The CLI is needed in order to create the demo federated graph and its subgraph and runs the router.Onboarding is triggered based on two conditions:
This means we do not allow simultaneous onboarding for multiple people in the same organization. The intention of the onboarding is for user to run it in the organization we create for them after the signup.
The onboarding flow is triggered when user signs up. The
wgc democommand can also open the onboarding UI in the web browser but it's there just as a helping guide.User can skip the onboarding at any time on any step in the web wizard. We track the step and whether they've skipped in the browser storage. The user can always resume onboarding by visiting the index page (graph index) where new empty state resides. This empty state links to onboarding web wizard. Once they create federated graph (either demo or any other one), we don't allow to resume the onboarding.
Once the onboarding is finished, user is not able to restart the onboarding (due to condition outlined above).
wgc demooffers you to delete the demo federated graph, so you can use that to drop the federated graph and subgraphs to restart the onboarding.After the onboarding is finished, we show a UI to offer the user to invite other users and read our documentation.
How to test?
If you want to test the whole functionality end to end, it is a bit more involved. The subgraphs are implemented as gRPC plugins, which means we need to connect to staging plugin registry to publish them and pull them to the router.
make infra-downenable-staging-registry.sh(ping me on Slack to get it). This will make changes to compose files and update your.envfiles.make infra-upcosmo-onboarding-v1flag and add your testing email which you will use for sign up:make start-cp&make start-studio). Go tolocalhost:3000, sign up with the email you specified in PostHogpnpm wgc demoto run the demo command. The demo command will display log file once plugins start to get published and when router starts.Release process
The event tracking is still work in progress (#2768) and we need to have dashboards prepared. We will first release it with just the flag enabled on our testing accounts to do further testing in production. After we're sure there are no issues, we can discuss gradual release to subset of users.
PR (1) for onboarding wizard: database changes + RPC
Stack: