-
Notifications
You must be signed in to change notification settings - Fork 10
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
Automatic Onboarding #615
Automatic Onboarding #615
Conversation
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 WalkthroughRecent changes across multiple files significantly improve the onboarding process by updating types, refining functions, and introducing new features for better state management and checklist handling. These include renaming checklist item names, incorporating new downstream step checks, and replacing Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User Interface
participant API as Onboarding API
participant Server as Server State
participant DB as Database
UI->>API: Fetch Onboarding State
API->>Server: Retrieve State Data
Server->>DB: Query Onboarding State
DB-->>Server: Onboarding Data Response
Server-->>API: Forward Onboarding Data
API-->>UI: Return Onboarding State
UI->>API: Update Onboarding Progress
API->>Server: Send Progress Update
Server->>DB: Update Onboarding State
DB-->>Server: Confirmation
Server-->>API: Update Success
API-->>UI: Confirm Update
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@coderabbitai review |
Actions performedReview triggered.
|
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: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- src/app/onboarding/ProductionSetup/Items.tsx (4 hunks)
- src/app/onboarding/ProductionSetup/index.tsx (3 hunks)
- src/app/onboarding/StarterSetup/Items.tsx (2 hunks)
- src/app/onboarding/StarterSetup/index.tsx (3 hunks)
- src/components/onboarding/ChecklistItem.tsx (2 hunks)
- src/components/onboarding/SkippedStep.tsx (1 hunks)
- src/data/api.ts (1 hunks)
- src/data/server/onboarding-state.ts (1 hunks)
- src/layouts/AuthenticatedLayout/OnboardingItem.tsx (2 hunks)
- src/layouts/AuthenticatedLayout/Sidebar.tsx (2 hunks)
- src/lib/constants.ts (2 hunks)
- src/lib/onboarding.spec.ts (1 hunks)
- src/lib/onboarding.ts (1 hunks)
- src/types/core.ts (5 hunks)
- src/types/onboarding.ts (1 hunks)
Files not reviewed due to errors (2)
- src/components/onboarding/ChecklistItem.tsx (no review received)
- src/lib/onboarding.ts (no review received)
Files skipped from review due to trivial changes (3)
- src/components/onboarding/SkippedStep.tsx
- src/data/api.ts
- src/lib/constants.ts
Additional context used
Biome
src/data/server/onboarding-state.ts
[error] 24-24: Use Array.isArray() instead of instanceof Array. (lint/suspicious/useIsArray)
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
Additional comments not posted (24)
src/types/onboarding.ts (2)
2-12
: Enum values updated to reflect new onboarding structure.The
OnboardingChecklistItemName
enum has been updated with new values that better describe the onboarding steps, aligning with the PR's enhancements.
14-14
: UpdatedOnboardingResponse
type for flexible onboarding step handling.The
OnboardingResponse
type definition has been updated to be an array ofOnboardingChecklistItemName
, enhancing flexibility in managing onboarding steps.src/data/server/onboarding-state.ts (2)
1-5
: Correct imports for onboarding functionalities.The imports are correctly set up to support the new functionalities related to onboarding state management.
41-49
: Proper setup ofuseOnboarding
hook using React Query.The
useOnboarding
hook is well-implemented, making effective use ofuseQuery
for fetching onboarding data, with appropriate query key and function setup.src/layouts/AuthenticatedLayout/OnboardingItem.tsx (3)
3-3
: Refactor: Update import to use new onboarding state management.The import of
useOnboarding
from@/data/server/onboarding-state
replaces the previous state management method, aligning with the new onboarding process.
14-14
: Error and Pending State Handling: Ensure graceful handling of API states.The implementation correctly handles error and pending states by returning
null
or a skeleton loader, respectively. This maintains a smooth user experience during data fetching delays or errors.Also applies to: 15-15
27-27
: Refactor: Utilize new onboarding functions and manage setup item progress effectively.The usage of
getProductionSetupItems
andgetProgress
functions aligns with the new onboarding state management. The logic to determine the completion of setup stages is clear and correctly implements the new state management approach.Also applies to: 30-31, 34-35, 38-40
src/app/onboarding/StarterSetup/index.tsx (5)
4-4
: Refactor: Update import to use new onboarding state management.The import of
useOnboarding
from@/data/server/onboarding-state
replaces the previous state management method, aligning with the new onboarding process.
23-25
: Error and Pending State Handling: Ensure graceful handling of API states.The implementation correctly handles error and pending states by returning a skeleton loader or
null
, respectively. This maintains a smooth user experience during data fetching delays or errors.
30-34
: Refactor: Utilize new onboarding functions to manage setup item progress effectively.The usage of
getOnboardingProgress
function aligns with the new onboarding state management. The logic to determine the completion of setup stages is clear and correctly implements the new state management approach.
80-80
: Ensure correct component props and onboarding state management.The
ConnectZenMLStep
andRunFirstPipeline
components are correctly passed theorder
andonboardingState
props. This ensures that the components can function with the new onboarding logic and maintain the correct state throughout the user's interaction.Also applies to: 84-84
19-19
: Ensure proper use of refetch intervals in hooks.Setting a refetch interval of 5000 milliseconds (5 seconds) in
useOnboarding
might be too frequent depending on the expected frequency of onboarding state changes. Consider adjusting this value based on user activity patterns or system requirements.Verification successful
The search results show that the
useOnboarding
hook with arefetchInterval
of 5000 milliseconds is used consistently in bothProductionSetup
andStarterSetup
components. This suggests a deliberate choice for this specific interval.
src/app/onboarding/ProductionSetup/index.tsx
src/app/onboarding/StarterSetup/index.tsx
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the refetch interval setting for useOnboarding hook across other components for consistency. # Test: Search for useOnboarding hook usage with refetch intervals. Expect: Consistent usage or justified differences. rg --type python $'useOnboarding\({ refetchInterval: \d+ \}\)'Length of output: 205
Script:
#!/bin/bash # Description: Verify the refetch interval setting for useOnboarding hook across other components for consistency. # Test: Search for useOnboarding hook usage with refetch intervals. Expect: Consistent usage or justified differences. rg --type ts --type tsx 'useOnboarding\(\{ refetchInterval: \d+ \}\)'Length of output: 99
Script:
#!/bin/bash # Description: Verify the refetch interval setting for useOnboarding hook across other components for consistency. # Test: Search for useOnboarding hook usage with refetch intervals. Expect: Consistent usage or justified differences. rg --glob '*.ts' --glob '*.tsx' 'useOnboarding\(\{ refetchInterval: \d+ \}\)'Length of output: 290
src/lib/onboarding.spec.ts (4)
1-1
: Ensure updated imports and function usage in tests.The imports of
OnboardingChecklistItemName
and the functionscheckDownstreamStep
,getProgress
,getStarterSetupItems
are aligned with the new onboarding logic, ensuring that the tests will validate the updated logic correctly.Also applies to: 3-3
9-9
: Test cases update: Validate new onboarding item handling.The test cases have been updated to reflect the new onboarding logic, ensuring that different deployment scenarios are correctly handled. This is crucial for maintaining the integrity of the onboarding process.
Also applies to: 15-16
19-19
: Comprehensive testing of onboarding progress calculation.The tests comprehensively cover various scenarios for onboarding progress calculation using the new logic. This ensures that the onboarding process is robust and behaves as expected under different conditions.
Also applies to: 21-30, 32-38, 40-50, 52-58
62-62
: Testing downstream step checks: Validate new logic for handling onboarding dependencies.The tests for
checkDownstreamStep
function validate the new logic for handling dependencies between onboarding steps. This is essential for ensuring that the onboarding process correctly handles step dependencies and progresses appropriately.Also applies to: 63-76, 78-83
src/layouts/AuthenticatedLayout/Sidebar.tsx (2)
25-25
: Refactor: Integration of the OnboardingItem component into the Sidebar.The addition of the
OnboardingItem
component into the sidebar ensures that the onboarding process is visible and accessible from the main UI, aligning with the goals of enhancing user onboarding.
51-51
: Ensure correct placement and functionality of the OnboardingItem in the Sidebar.The placement of the
OnboardingItem
above other essential links in the sidebar ensures it is immediately noticeable to new users, potentially improving engagement with the onboarding process.src/app/onboarding/StarterSetup/Items.tsx (1)
14-26
: Verification needed for theConnectZenMLStep
component.Ensure that the
ConnectZenMLStep
component integrates correctly with the rest of the onboarding process and that thehasDownStreamFinished
variable is calculated correctly.src/app/onboarding/ProductionSetup/index.tsx (1)
107-131
: Ensure correct propagation ofactive
prop in checklist components.Verify that the
active
prop is correctly used in the child components (CreateServiceConnector
,CreateArtifactStore
,CreateNewStack
,RunNewPipeline
) to control their behavior based on the completion of the starter setup.Verification successful
The
active
prop is indeed used in the child components (CreateServiceConnector
,CreateArtifactStore
,CreateNewStack
,RunNewPipeline
) as part of their props. This confirms that theactive
prop is being propagated correctly to these components.
src/app/onboarding/ProductionSetup/Items.tsx
:
CreateServiceConnector
CreateArtifactStore
CreateNewStack
RunNewPipeline
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of `active` prop in child components. # Test: Search for the `active` prop usage in the JSX files. rg --type js --type jsx $'active='Length of output: 61
Script:
#!/bin/bash # Locate files containing the component names. fd -e tsx -e jsx -e js -e ts 'CreateServiceConnector|CreateArtifactStore|CreateNewStack|RunNewPipeline'Length of output: 105
Script:
#!/bin/bash # Search for the component definitions using ast-grep. ast-grep --lang typescript --pattern 'function CreateServiceConnector($_) { $$$ }' ast-grep --lang typescript --pattern 'function CreateArtifactStore($_) { $$$ }' ast-grep --lang typescript --pattern 'function CreateNewStack($_) { $$$ }' ast-grep --lang typescript --pattern 'function RunNewPipeline($_) { $$$ }'Length of output: 320
Script:
#!/bin/bash # Search for component definitions using arrow functions and class components. ast-grep --lang typescript --pattern 'const CreateServiceConnector = ($_) => { $$$ }' ast-grep --lang typescript --pattern 'const CreateArtifactStore = ($_) => { $$$ }' ast-grep --lang typescript --pattern 'const CreateNewStack = ($_) => { $$$ }' ast-grep --lang typescript --pattern 'const RunNewPipeline = ($_) => { $$$ }' ast-grep --lang typescript --pattern 'class CreateServiceConnector { $$$ }' ast-grep --lang typescript --pattern 'class CreateArtifactStore { $$$ }' ast-grep --lang typescript --pattern 'class CreateNewStack { $$$ }' ast-grep --lang typescript --pattern 'class RunNewPipeline { $$$ }'Length of output: 625
Script:
#!/bin/bash # Search for import statements for the components. rg 'import.*(CreateServiceConnector|CreateArtifactStore|CreateNewStack|RunNewPipeline)' --glob '*.tsx' --glob '*.js' --glob '*.ts'Length of output: 132
Script:
#!/bin/bash # Search for the usage of the components in the codebase. rg 'CreateServiceConnector|CreateArtifactStore|CreateNewStack|RunNewPipeline' --glob '*.tsx' --glob '*.js' --glob '*.ts'Length of output: 1164
src/app/onboarding/ProductionSetup/Items.tsx (1)
58-67
: Logic verification needed forCreateArtifactStore
.Ensure that the
hasDownStreamFinished
calculation is correct and that the component behaves as expected whenitem
is true or false.Verification successful
The command to search for
CreateArtifactStore
did not produce any output, which means we couldn't locate its usage in the codebase. This might indicate that the component is either not used or the search command needs adjustment.To ensure thorough verification of the logic within
CreateArtifactStore
, I will generate a new shell script to search for the function definition and its usage throughout the codebase, including both JavaScript and TypeScript files, as it might be defined or used in TypeScript files as well.
The
CreateArtifactStore
function is defined insrc/app/onboarding/ProductionSetup/Items.tsx
and used insrc/app/onboarding/ProductionSetup/index.tsx
. To verify the logic, I will review the relevant sections of these files to ensure thehasDownStreamFinished
calculation is correct and the component behaves as expected whenitem
is true or false.
The
CreateArtifactStore
component is defined and used as follows:
Definition in
src/app/onboarding/ProductionSetup/Items.tsx
:export function CreateArtifactStore({ onboardingState, active, order }: Props) { const [selectedProvider, setSelectedProvider] = useState<CloudProvider>("aws"); const itemName: OnboardingChecklistItemName = "remote_artifact_store_created"; const hasDownStreamFinishded = checkDownstreamStep(itemName, onboardingState || [], order); const item = hasOnboardingItem(itemName, onboardingState || []); return ( <ChecklistItem hasDownstream={hasDownStreamFinishded} itemName={itemName} completed={!!item} title="Create an artifact store" active={active} > <p className="mb-3"> Configuring a remote artifact store will version your pipeline's data directly in your cloud provider{" "} <LearnMoreLink href="https://docs.zenml.io/user-guide/production-guide/remote-storage" /> </p> <div className="space-y-5"> {/* Additional content */} </div> </ChecklistItem> ); }Usage in
src/app/onboarding/ProductionSetup/index.tsx
:import { CreateArtifactStore, CreateNewStack, CreateServiceConnector, RunNewPipeline } from "./Items"; export function ProductionSetupChecklist() { const onboarding = useOnboarding({ refetchInterval: 5000 }); const serverInfo = useServerInfo(); const [open, setOpen] = useState(true); const isStarterSetupFinished = // logic to determine if starter setup is finished return ( <ul> <li className="py-5 first:pt-0 last:pb-0"> <CreateArtifactStore order={order} onboardingState={onboarding.data} active={isStarterSetupFinished} /> </li> <li className="py-5 first:pt-0 last:pb-0"> <CreateNewStack order={order} onboardingState={onboarding.data} active={isStarterSetupFinished} /> </li> {/* Additional items */} </ul> ); }Verification
hasDownStreamFinished
Calculation:
- The variable
hasDownStreamFinishded
is calculated using the functioncheckDownstreamStep(itemName, onboardingState || [], order)
. This logic appears to be correctly implemented, assumingcheckDownstreamStep
is a well-defined function that returns a boolean indicating whether downstream steps are finished.Component Behavior:
- The
CreateArtifactStore
component renders aChecklistItem
with properties based on thehasDownStreamFinishded
anditem
variables. Thecompleted
prop is set to!!item
, which correctly converts the item to a boolean.Conclusion
The logic for
hasDownStreamFinishded
and the behavior of theCreateArtifactStore
component based on theitem
state appear to be correctly implemented. TheChecklistItem
component is given the correct props to reflect the completion status and downstream state.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify logic in `CreateArtifactStore`. # Test: Search for the usage of `CreateArtifactStore` in JSX files and verify integration. rg --type js --type jsx $'CreateArtifactStore'Length of output: 73
Script:
#!/bin/bash # Description: Correct the search command to include both JavaScript and JSX files. # Test: Search for the usage of `CreateArtifactStore` in JavaScript and JSX files. rg --type-add 'jsx:*.jsx' --type js --type jsx 'CreateArtifactStore'Length of output: 68
Script:
#!/bin/bash # Description: Correct the search command to include both JavaScript and JSX files. # Test: Search for the usage of `CreateArtifactStore` in JavaScript and JSX files. rg --type-add 'jsx:*.jsx' --type js --type jsx 'CreateArtifactStore'Length of output: 68
Script:
#!/bin/bash # Description: Correct the search command to include both JavaScript and JSX files. # Test: Search for the usage of `CreateArtifactStore` in JavaScript and JSX files. rg --type-add 'jsx:*.jsx' --type js --type jsx 'CreateArtifactStore'Length of output: 68
Script:
#!/bin/bash # Description: Correct the search command to include both JavaScript and JSX files. # Test: Search for the usage of `CreateArtifactStore` in JavaScript and JSX files. rg --type-add 'jsx:*.jsx' --type js --type jsx 'CreateArtifactStore'Length of output: 68
Script:
#!/bin/bash # Description: Search for the definition and usage of `CreateArtifactStore` in JavaScript, TypeScript, and JSX files. # Test: Search for the definition of `CreateArtifactStore`. rg --type-add 'jsx:*.jsx' --type-add 'tsx:*.tsx' --type js --type jsx --type ts --type tsx 'function CreateArtifactStore' # Test: Search for the usage of `CreateArtifactStore`. rg --type-add 'jsx:*.jsx' --type-add 'tsx:*.tsx' --type js --type jsx --type ts --type tsx 'CreateArtifactStore'Length of output: 628
Script:
#!/bin/bash # Extract the relevant sections of the files for review. # Display the definition of `CreateArtifactStore`. rg --type-add 'jsx:*.jsx' --type-add 'tsx:*.tsx' --type js --type jsx --type ts --type tsx -A 20 'function CreateArtifactStore' src/app/onboarding/ProductionSetup/Items.tsx # Display the usage of `CreateArtifactStore`. rg --type-add 'jsx:*.jsx' --type-add 'tsx:*.tsx' --type js --type jsx --type ts --type tsx -A 10 'CreateArtifactStore' src/app/onboarding/ProductionSetup/index.tsxLength of output: 1804
src/types/core.ts (3)
1229-1238
: LGTM!The new API endpoint for fetching the onboarding state is well-defined and includes a clear description. This is crucial for the automatic onboarding feature.
6766-6766
: Good simplification!Changing
ServerSettingsResponseMetadata
toRecord<string, never>
effectively indicates no additional metadata is expected, which simplifies handling server settings responses.
13399-13432
: Well-defined API operation!The operation
get_onboarding_state_api_v1_onboarding_state_get
is defined with comprehensive response handling, covering successful and error scenarios. This is essential for robust API design.
Summary by CodeRabbit
New Features
Refactor
useServerSettings
withuseOnboarding
for improved onboarding state management.Bug Fixes
Documentation