fix: Settings page runbooks #242
Conversation
WalkthroughThe changes in this pull request involve significant refactoring and enhancements to the runbook management components within the application. The Changes
Possibly related PRs
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (10)
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/settings/page.tsx (2)
1-4: Consider adding loading state handlingSince this is a Next.js page with async data fetching, consider importing and implementing loading UI components for a better user experience.
import { notFound } from "next/navigation"; +import { Skeleton } from "~/components/ui/skeleton"; import { api } from "~/trpc/server"; import { EditRunbook } from "./EditRunbook";
20-31: Enhance accessibility with semantic HTMLWhile the layout is functional, it could benefit from semantic HTML and accessibility improvements.
- <div className="flex justify-center py-6"> - <div className="max-w-2xl"> + <main className="flex justify-center py-6" role="main"> + <section className="max-w-2xl" aria-label="Runbook Settings"> <EditRunbook runbook={runbook} jobAgents={jobAgents} jobAgent={jobAgent} workspace={workspace} /> - </div> - </div> + </section> + </main>apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/settings/EditRunbook.tsx (2)
9-9: Consider restructuring the import pathsThe relative import path
../../EditRunbookFormsuggests this component might be deeply nested. Consider moving shared components to a more accessible location to improve maintainability.Also applies to: 11-11
26-33: Consider adding type safety for form default valuesThe spread operator
...runbookmight include properties not defined in the form schema. Consider explicitly selecting only the required fields to prevent potential runtime issues.const defaultValues = { - ...runbook, + name: runbook.name, + enabled: runbook.enabled, description: runbook.description ?? "", jobAgentId: jobAgent.id, jobAgentConfig: jobAgent.config, };apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/EditRunbookDialog.tsx (1)
66-72: Consider adding prop types validationWhile the EditRunbookForm component receives all necessary props, consider adding TypeScript interface definitions for better type safety and documentation.
Add an interface definition:
interface EditRunbookFormProps { form: ReturnType<typeof useForm>; jobAgents: JobAgent[]; jobAgent: JobAgent | undefined; workspace: Workspace; onSubmit: (data: EditRunbookFormSchema) => Promise<void>; }apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/RunbookNavBar.tsx (1)
Line range hint
78-81: Consider enhancing the Trigger button based on runbook stateSince we now have access to the runbook prop, consider using it to:
- Disable the trigger button if the runbook is not in a triggerable state
- Add a tooltip explaining why the button might be disabled
- Show relevant runbook metadata in the button or tooltip
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/EditRunbookForm.tsx (3)
42-48: Consider adding loading state handlingThe form lacks loading state management during submission. This could lead to multiple submissions if users click the save button repeatedly.
Example improvement:
export type EditRunbookFormProps = { form: UseFormReturn<EditRunbookFormSchema>; jobAgents: SCHEMA.JobAgent[]; jobAgent?: SCHEMA.JobAgent; workspace: SCHEMA.Workspace; onSubmit: (data: EditRunbookFormSchema) => void; + isSubmitting?: boolean; };Then update the submit button:
- <Button type="submit">Save</Button> + <Button type="submit" disabled={isSubmitting}> + {isSubmitting ? "Saving..." : "Save"} + </Button>
90-95: Consider extracting static text for internationalizationThe description text should be moved to a translation file to support multiple languages.
Consider using your i18n solution to handle the text:
- <div className="text-sm text-muted-foreground"> - Variables in runbooks make automation flexible and reusable. They let - you customize runbooks with user inputs and use environment-specific - values without hardcoding. This allows runbooks to adapt to different - scenarios without changing their core logic. - </div> + <div className="text-sm text-muted-foreground"> + {t('runbook.variables.description')} + </div>
49-159: Consider breaking down the form into smaller componentsThe form component is quite large and handles multiple concerns. Consider splitting it into smaller, focused components for better maintainability.
Suggested structure:
const RunbookGeneralSection = ({ control }: { control: Control<EditRunbookFormSchema> }) => { // Name and description fields }; const RunbookVariablesSection = ({ control }: { control: Control<EditRunbookFormSchema> }) => { // Variables section }; const RunbookAgentSection = ({ control, jobAgents, jobAgent, workspace }: Props) => { // Agent configuration }; // Main component becomes more readable export const EditRunbookForm: React.FC<EditRunbookFormProps> = (props) => ( <Form {...props.form}> <form onSubmit={props.form.handleSubmit(props.onSubmit)} className="space-y-8"> <RunbookGeneralSection control={props.form.control} /> <RunbookVariablesSection control={props.form.control} /> <RunbookAgentSection {...props} /> <Button type="submit">Save</Button> <FormRootError /> </form> </Form> );packages/api/src/router/runbook.ts (1)
24-27: Enhance error handling for missing relations.While the query enhancement is good, consider adding explicit error handling for cases where relations might be null:
- Add runtime type checking for the returned data
- Maintain consistency with error handling in other procedures
Consider adding type validation:
ctx.db.query.runbook.findFirst({ where: eq(SCHEMA.runbook.id, input), with: { variables: true, runhooks: true }, -}) +}).then((result) => { + if (!result) { + throw new TRPCError({ + code: 'NOT_FOUND', + message: 'Runbook not found', + }); + } + return result; +})
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (7)
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/EditRunbookDialog.tsx(3 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/EditRunbookForm.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/RunbookNavBar.tsx(3 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/layout.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/settings/EditRunbook.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/settings/page.tsx(1 hunks)packages/api/src/router/runbook.ts(1 hunks)
🔇 Additional comments (11)
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/settings/page.tsx (1)
23-28: Verify EditRunbook component interface
Let's ensure the EditRunbook component exists and accepts all the required props.
✅ Verification successful
EditRunbook component props are correctly defined and used
The verification shows that the EditRunbook component in apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/settings/EditRunbook.tsx has the exact interface matching the usage:
type EditRunbookProps = {
runbook: NonNullable<RouterOutputs["runbook"]["byId"]>;
jobAgents: SCHEMA.JobAgent[];
jobAgent: SCHEMA.JobAgent;
workspace: SCHEMA.Workspace;
};The component implementation also correctly accepts and uses these props.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for EditRunbook component definition
ast-grep --pattern 'interface $props {
$$$
runbook: $_
jobAgents: $_
jobAgent: $_
workspace: $_
$$$
}'
# Verify the component implementation
ast-grep --pattern 'function EditRunbook({ runbook, jobAgents, jobAgent, workspace }: $props) {
$$$
}'
Length of output: 223
Script:
#!/bin/bash
# Let's try a broader search for EditRunbook component and its props
ast-grep --pattern 'EditRunbook = ({ $$ }: $_) => {
$$$
}'
# Also search for any interface or type definition related to EditRunbook
ast-grep --pattern 'type EditRunbookProps = {
$$$
}'
# Search for the component file itself
fd EditRunbook
# As a fallback, let's use ripgrep to find any EditRunbook definitions
rg "EditRunbook" -A 5
Length of output: 14403
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/layout.tsx (1)
46-46: LGTM! Efficient reuse of fetched runbook data.
The addition of the runbook prop to RunbookNavBar is a good improvement, making the runbook data directly available to the navigation component without requiring additional API calls.
Let's verify that the RunbookNavBar component properly types the new prop:
✅ Verification successful
Props are properly typed and match the usage
The RunbookNavBar component correctly defines and uses the runbook prop with proper typing:
- Type
RunbookNavBarPropsexplicitly definesrunbook: NonNullable<RouterOutputs["runbook"]["byId"]> - The prop is used within the component to check
runbook.runhooks.length - The type is imported from the API router outputs, ensuring type safety
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify RunbookNavBar component prop types
# Expected: Find the RunbookNavBar component definition with proper typing for the runbook prop
# Search for the RunbookNavBar component definition
ast-grep --pattern 'interface $interface {
$$$
runbook: $type
$$$
}'
# Alternative: Look for type definitions in the component
ast-grep --pattern 'function RunbookNavBar({ $$$, runbook }: { $$$, runbook: $type })'
Length of output: 165
Script:
#!/bin/bash
# Let's try a different approach to find the RunbookNavBar component and its type definitions
# First, let's locate the RunbookNavBar component file
fd RunbookNavBar
# Then let's examine its content and type definitions
rg -A 5 "RunbookNavBar" --type tsx
Length of output: 187
Script:
#!/bin/bash
# Let's examine the RunbookNavBar component file directly
cat "apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/RunbookNavBar.tsx"
Length of output: 3215
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/settings/EditRunbook.tsx (2)
20-25: LGTM! Well-structured component declaration
The component follows React best practices with proper TypeScript typing and clean props destructuring.
40-48: LGTM! Clean component rendering
The component renders the form with proper prop passing and follows React best practices.
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/EditRunbookDialog.tsx (2)
17-17: LGTM! Clean import organization
The imports are well-organized and properly aligned with the component's refactoring goals.
Also applies to: 19-19, 21-21
19-19: Verify the EditRunbookForm implementation
Let's ensure the new EditRunbookForm component properly implements all the functionality from the previous version.
Also applies to: 21-21, 66-72
✅ Verification successful
Based on the shell script results, I can now generate the final response since I have sufficient information about the EditRunbookForm implementation.
EditRunbookForm implementation is complete and well-structured
The new EditRunbookForm component properly implements all essential functionality:
- Form fields for name and description
- Variables editor with proper documentation
- Job agent selection and configuration
- Comprehensive form validation using zod schema
- Proper error handling with FormRootError
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify EditRunbookForm implementation and compare with git history
# Check if EditRunbookForm exists and contains necessary form fields
fd --glob "*EditRunbookForm.tsx" | xargs cat
# Look for any TODO or FIXME comments that might indicate incomplete refactoring
rg -i "TODO|FIXME" --glob "*EditRunbookForm.tsx"
# Check git history for removed form fields to ensure nothing was missed
git log -p -- "$(fd --glob "*EditRunbookDialog.tsx")" | rg "FormField|Input|Select"
Length of output: 4659
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/RunbookNavBar.tsx (2)
3-3: LGTM! Type safety is well handled
Good use of NonNullable to ensure type safety with the runbook prop.
Also applies to: 22-22
25-28: LGTM! Props are properly typed and destructured
The component signature correctly implements the updated RunbookNavBarProps type.
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/EditRunbookForm.tsx (2)
32-40: LGTM! Well-structured type definitions
The type definitions are clear, properly exported, and provide good type safety through zod inference.
6-22: Verify component dependencies
Let's ensure all required components are properly exported from their respective packages.
✅ Verification successful
Let me verify the exports more precisely by checking the actual export statements.
All component dependencies are properly exported and available
All UI components from @ctrlplane/ui packages (Button, Form components, Input, Textarea) and custom components (JobAgentConfig, JobAgentSelector, RunbookVariablesEditor) are properly exported from their respective source files and available for import.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all required components are properly exported
# Check UI component exports
rg -l "export.*(?:Button|Form|FormControl|FormField|FormItem|FormLabel|FormMessage|FormRootError|Input|Textarea)" --type ts
# Check custom component exports
rg -l "export.*(?:JobAgentConfig|JobAgentSelector|RunbookVariablesEditor)" --type ts
Length of output: 2975
Script:
#!/bin/bash
# Check UI component exports in their source files
echo "=== Button exports ==="
rg "export.*Button" packages/ui/src/button.tsx
echo -e "\n=== Form component exports ==="
rg "export.*Form" packages/ui/src/form.tsx
echo -e "\n=== Input exports ==="
rg "export.*Input" packages/ui/src/input.tsx
echo -e "\n=== Textarea exports ==="
rg "export.*Textarea" packages/ui/src/textarea.tsx
echo -e "\n=== Custom component exports ==="
echo "JobAgentConfig:"
rg "export.*JobAgentConfig" apps/webservice/src/components/form/job-agent/JobAgentConfig.tsx
echo -e "\nJobAgentSelector:"
rg "export.*JobAgentSelector" apps/webservice/src/components/form/job-agent/JobAgentSelector.tsx
echo -e "\nRunbookVariablesEditor:"
rg "export.*RunbookVariablesEditor" apps/webservice/src/app/[workspaceSlug]/\(app\)/systems/[systemSlug]/runbooks/create/RunbookVariableEditor.tsx
Length of output: 1409
packages/api/src/router/runbook.ts (1)
24-27: Consider performance implications of eager loading.
The addition of variables and runhooks relations to the query is good for data completeness, but consider the following recommendations:
- For large runbooks with many variables/hooks, this could impact query performance.
- Consider implementing selective loading based on client needs.
Let's verify the usage of these relations in the codebase:
✅ Verification successful
Eager loading of relations is justified by frontend usage
The search results confirm that both variables and runhooks relations are actively used in multiple frontend components:
- Variables are used in
TriggerRunbook.tsxfor rendering - Runhooks are used in
RunbookNavBar.tsxandRunbookRow.tsxfor conditional rendering
Since these relations are consistently used in the UI components that display runbook details, eager loading them in the byId query is appropriate and prevents N+1 query issues. The performance impact is justified by the actual usage patterns in the frontend.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all loaded relations are actually used in the frontend
# and if there are any missing relations that should be included
# Search for usage of runbook.variables
rg -t typescript "runbook\.(variables|runhooks)" --glob "packages/**/{components,pages}/**"
# Search for other places loading runbook by ID to ensure consistency
ast-grep --pattern 'findFirst({
where: eq($_, $_),
with: $$$
})'
Length of output: 199
Script:
#!/bin/bash
# Let's try a different approach with more generic search patterns
# Search for usage of variables and runhooks in TypeScript/JavaScript files
rg "runbook\.(variables|runhooks)" --type-add 'ts:*.{ts,tsx}' --type ts
# Look for similar findFirst queries in the codebase
rg "findFirst.*with.*variables.*runhooks" --type-add 'ts:*.{ts,tsx}' --type ts -A 2
# Check if there are any performance-related patterns or selective loading
rg "select.*runbook" --type-add 'ts:*.{ts,tsx}' --type ts
# Search for other runbook queries to compare patterns
rg "from.*runbook.*where" --type-add 'ts:*.{ts,tsx}' --type ts
Length of output: 844
| const workspace = await api.workspace.bySlug(params.workspaceSlug); | ||
| if (workspace == null) return notFound(); | ||
| const jobAgents = await api.job.agent.byWorkspaceId(workspace.id); | ||
| const runbook = await api.runbook.byId(params.runbookId); | ||
| if (runbook == null) return notFound(); | ||
|
|
||
| const jobAgent = jobAgents.find((ja) => ja.id === runbook.jobAgentId); | ||
| if (jobAgent == null) return notFound(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and optimize API calls
The current implementation has two potential improvements:
- API calls could be parallelized using Promise.all for better performance
- Missing error handling for API calls
Consider refactoring to:
- const workspace = await api.workspace.bySlug(params.workspaceSlug);
- if (workspace == null) return notFound();
- const jobAgents = await api.job.agent.byWorkspaceId(workspace.id);
- const runbook = await api.runbook.byId(params.runbookId);
- if (runbook == null) return notFound();
+ try {
+ const [workspace, runbook] = await Promise.all([
+ api.workspace.bySlug(params.workspaceSlug),
+ api.runbook.byId(params.runbookId)
+ ]);
+
+ if (workspace == null || runbook == null) return notFound();
+
+ const jobAgents = await api.job.agent.byWorkspaceId(workspace.id);
+ const jobAgent = jobAgents.find((ja) => ja.id === runbook.jobAgentId);
+ if (jobAgent == null) return notFound();
+ } catch (error) {
+ console.error('Failed to fetch runbook settings:', error);
+ throw error; // Let Next.js error boundary handle it
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const workspace = await api.workspace.bySlug(params.workspaceSlug); | |
| if (workspace == null) return notFound(); | |
| const jobAgents = await api.job.agent.byWorkspaceId(workspace.id); | |
| const runbook = await api.runbook.byId(params.runbookId); | |
| if (runbook == null) return notFound(); | |
| const jobAgent = jobAgents.find((ja) => ja.id === runbook.jobAgentId); | |
| if (jobAgent == null) return notFound(); | |
| try { | |
| const [workspace, runbook] = await Promise.all([ | |
| api.workspace.bySlug(params.workspaceSlug), | |
| api.runbook.byId(params.runbookId) | |
| ]); | |
| if (workspace == null || runbook == null) return notFound(); | |
| const jobAgents = await api.job.agent.byWorkspaceId(workspace.id); | |
| const jobAgent = jobAgents.find((ja) => ja.id === runbook.jobAgentId); | |
| if (jobAgent == null) return notFound(); | |
| } catch (error) { | |
| console.error('Failed to fetch runbook settings:', error); | |
| throw error; // Let Next.js error boundary handle it | |
| } |
| const update = api.runbook.update.useMutation(); | ||
| const router = useRouter(); | ||
|
|
||
| const onSubmit = (data: EditRunbookFormSchema) => | ||
| update.mutateAsync({ id: runbook.id, data }).then(() => router.refresh()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and loading states
The mutation handling could be improved by:
- Adding error handling for failed mutations
- Showing loading states during the mutation
- Providing user feedback on success/failure
const update = api.runbook.update.useMutation();
const router = useRouter();
-const onSubmit = (data: EditRunbookFormSchema) =>
- update.mutateAsync({ id: runbook.id, data }).then(() => router.refresh());
+const onSubmit = async (data: EditRunbookFormSchema) => {
+ try {
+ await update.mutateAsync({ id: runbook.id, data });
+ // TODO: Add success toast notification
+ router.refresh();
+ } catch (error) {
+ // TODO: Add error toast notification
+ console.error('Failed to update runbook:', error);
+ }
+};Committable suggestion skipped: line range outside the PR's diff.
| const onSubmit = (data: EditRunbookFormSchema) => | ||
| update | ||
| .mutateAsync({ id: runbook.id, data }) | ||
| .then(() => router.refresh()) | ||
| .then(() => setOpen(false)), | ||
| ); | ||
| .then(() => setOpen(false)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to the form submission
The promise chain should include error handling and user feedback for a better user experience.
Consider this improvement:
const onSubmit = (data: EditRunbookFormSchema) =>
update
.mutateAsync({ id: runbook.id, data })
.then(() => router.refresh())
- .then(() => setOpen(false));
+ .then(() => {
+ setOpen(false);
+ // Consider adding a success toast/notification here
+ })
+ .catch((error) => {
+ console.error('Failed to update runbook:', error);
+ // Consider adding an error toast/notification here
+ });Committable suggestion skipped: line range outside the PR's diff.
| {runbook.runhooks.length === 0 && ( | ||
| <NavigationMenuItem> | ||
| <Link href={settingsUrl} legacyBehavior passHref> | ||
| <NavigationMenuLink | ||
| className="group inline-flex h-9 w-max items-center justify-center rounded-md px-4 py-2 text-sm font-medium transition-colors hover:bg-accent/50 hover:text-accent-foreground focus:outline-none disabled:pointer-events-none disabled:opacity-50 data-[active]:bg-accent/50 data-[state=open]:bg-accent/50" | ||
| active={isSettingsActive} | ||
| > | ||
| Settings | ||
| </NavigationMenuLink> | ||
| </Link> | ||
| </NavigationMenuItem> | ||
| )} |
There was a problem hiding this comment.
💡 Codebase verification
Settings should remain accessible regardless of runhooks presence
The current implementation that hides the Settings menu when runhooks exist is problematic because:
- The settings page (
RunbookSettingsPage) exists and is fully implemented without any conditions related to runhooks, suggesting it's meant to be accessible at all times - The page contains important functionality like
EditRunbookthat users would need regardless of runhook status - There's no documented reason or technical limitation that justifies hiding settings access when runhooks are present
Recommendations:
- Remove the conditional rendering (
runbook.runhooks.length === 0) and always show the Settings menu item - If there are specific settings that shouldn't be modified with runhooks, handle those restrictions within the settings page itself rather than hiding the entire navigation item
🔗 Analysis chain
Verify the UX decision to hide settings when runhooks exist
The current implementation hides the Settings menu item when runhooks are present. This might lead to:
- Reduced accessibility to settings when they might still be needed
- Confusion when the settings option disappears after adding runhooks
- Potential workflow disruption for users managing runbook configurations
Consider:
- Keeping settings always accessible
- Adding a tooltip explaining why settings are hidden (if this behavior must be maintained)
- Documenting this behavior in user guides
Let's check if this behavior is consistent with other similar components:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar navigation patterns in other components
rg -t tsx "NavigationMenu.*Settings" --glob "!**/RunbookNavBar.tsx"
Length of output: 97
Script:
#!/bin/bash
# Search for similar navigation patterns with settings in other components
rg -t typescript -t javascriptreact -t typescriptreact "NavigationMenu.*Settings" --glob "!**/RunbookNavBar.tsx"
# Also search for other navigation components that might show/hide settings conditionally
rg -t typescript -t javascriptreact -t typescriptreact "Settings.*className.*(\{.*\})" -B 2 -A 2
Length of output: 283
Script:
#!/bin/bash
# Search for similar navigation patterns with settings
rg "NavigationMenu.*Settings" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' -t tsx -t ts --glob "!**/RunbookNavBar.tsx"
# Look for conditional rendering of settings in navigation
rg "Settings.*className.*\{.*\}" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' -t tsx -t ts -B 3 -A 3
# Search for other instances of runbook settings navigation
fd -e tsx -e ts . | xargs rg "runbook.*settings" -i
Length of output: 433
Script:
#!/bin/bash
# Check the settings page implementation to understand if it's meant to be accessible with runhooks
cat "apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/[runbookId]/settings/page.tsx"
# Look for any other navigation bars in the runbook context to understand the pattern
rg -B 3 -A 3 "NavigationMenu" --type-add 'tsx:*.tsx' -t tsx -g "**/runbooks/**"
# Search for any documentation or comments about runbook settings access
rg "runbook.*settings?.*access" -i --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type-add 'md:*.md' -t tsx -t ts -t md
Length of output: 7948
| export const updateRunbookSchema = z.object({ | ||
| name: z.string().min(1), | ||
| description: z.string(), | ||
| variables: z.array(SCHEMA.createRunbookVariable), | ||
| jobAgentId: z.string().uuid({ message: "Must be a valid job agent ID" }), | ||
| jobAgentConfig: z.record(z.any()), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strengthen type safety for jobAgentConfig validation
The current schema uses z.record(z.any()) for jobAgentConfig which is too permissive and could lead to runtime errors. Consider creating a more specific schema based on the possible agent configuration types.
Example improvement:
- jobAgentConfig: z.record(z.any()),
+ jobAgentConfig: z.record(z.union([
+ z.string(),
+ z.number(),
+ z.boolean(),
+ z.array(z.string()),
+ // Add other specific types based on your agent configurations
+ ])),Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
New Features
EditRunbookFormcomponent for improved runbook editing experience.EditRunbookcomponent for managing runbook settings.RunbookNavBarto conditionally render settings based on runbook state.RunbookSettingsPagefor displaying runbook settings.Bug Fixes
EditRunbookDialog.Documentation