-
Notifications
You must be signed in to change notification settings - Fork 2
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
fix: Settings page runbooks #242
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,160 @@ | ||
"use client"; | ||
|
||
import type { UseFormReturn } from "react-hook-form"; | ||
import { z } from "zod"; | ||
|
||
import * as SCHEMA from "@ctrlplane/db/schema"; | ||
import { Button } from "@ctrlplane/ui/button"; | ||
import { | ||
Form, | ||
FormControl, | ||
FormField, | ||
FormItem, | ||
FormLabel, | ||
FormMessage, | ||
FormRootError, | ||
} from "@ctrlplane/ui/form"; | ||
import { Input } from "@ctrlplane/ui/input"; | ||
import { Textarea } from "@ctrlplane/ui/textarea"; | ||
|
||
import { JobAgentConfig } from "~/components/form/job-agent/JobAgentConfig"; | ||
import { JobAgentSelector } from "~/components/form/job-agent/JobAgentSelector"; | ||
import { RunbookVariablesEditor } from "./create/RunbookVariableEditor"; | ||
|
||
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()), | ||
}); | ||
Comment on lines
+24
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Strengthen type safety for jobAgentConfig validation The current schema uses 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
+ ])),
|
||
|
||
export type EditRunbookFormSchema = z.infer<typeof updateRunbookSchema>; | ||
|
||
export type EditRunbookFormProps = { | ||
form: UseFormReturn<EditRunbookFormSchema>; | ||
jobAgents: SCHEMA.JobAgent[]; | ||
jobAgent?: SCHEMA.JobAgent; | ||
workspace: SCHEMA.Workspace; | ||
onSubmit: (data: EditRunbookFormSchema) => void; | ||
}; | ||
|
||
export const EditRunbookForm: React.FC<EditRunbookFormProps> = ({ | ||
form, | ||
jobAgents, | ||
jobAgent, | ||
workspace, | ||
onSubmit, | ||
}) => ( | ||
<Form {...form}> | ||
<form onSubmit={form.handleSubmit(onSubmit)} className="space-y-8"> | ||
<div className="space-y-3"> | ||
<div>General</div> | ||
<FormField | ||
control={form.control} | ||
name="name" | ||
render={({ field }) => ( | ||
<FormItem> | ||
<FormLabel>Name</FormLabel> | ||
<FormControl> | ||
<Input | ||
placeholder="Deploy Hotfix, Rollback Release, Scale Service..." | ||
{...field} | ||
/> | ||
</FormControl> | ||
<FormMessage /> | ||
</FormItem> | ||
)} | ||
/> | ||
<FormField | ||
control={form.control} | ||
name="description" | ||
render={({ field }) => ( | ||
<FormItem> | ||
<FormLabel>Description</FormLabel> | ||
<FormControl> | ||
<Textarea | ||
placeholder="Describe the purpose of this runbook..." | ||
{...field} | ||
/> | ||
</FormControl> | ||
<FormMessage /> | ||
</FormItem> | ||
)} | ||
/> | ||
</div> | ||
|
||
<div className="space-y-3"> | ||
<div>Variables</div> | ||
|
||
<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> | ||
|
||
<FormField | ||
control={form.control} | ||
name="variables" | ||
render={({ field }) => ( | ||
<FormItem> | ||
<FormControl> | ||
<RunbookVariablesEditor | ||
value={field.value} | ||
onChange={field.onChange} | ||
/> | ||
</FormControl> | ||
<FormMessage /> | ||
</FormItem> | ||
)} | ||
/> | ||
</div> | ||
|
||
<div className="space-y-3"> | ||
<div>Agent</div> | ||
|
||
<FormField | ||
control={form.control} | ||
name="jobAgentId" | ||
render={({ field: { value, onChange } }) => ( | ||
<FormItem> | ||
<FormLabel>Job Agent</FormLabel> | ||
<FormControl> | ||
<JobAgentSelector | ||
jobAgents={jobAgents} | ||
workspace={workspace} | ||
value={value} | ||
onChange={onChange} | ||
/> | ||
</FormControl> | ||
<FormMessage /> | ||
</FormItem> | ||
)} | ||
/> | ||
|
||
<FormField | ||
control={form.control} | ||
name="jobAgentConfig" | ||
render={({ field: { value, onChange } }) => ( | ||
<FormItem> | ||
<FormLabel>Config</FormLabel> | ||
<FormControl> | ||
<JobAgentConfig | ||
jobAgent={jobAgent} | ||
workspace={workspace} | ||
value={value} | ||
onChange={onChange} | ||
/> | ||
</FormControl> | ||
<FormMessage /> | ||
</FormItem> | ||
)} | ||
/> | ||
</div> | ||
|
||
<Button type="submit">Save</Button> | ||
<FormRootError /> | ||
</form> | ||
</Form> | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
"use client"; | ||
|
||
import type { RouterOutputs } from "@ctrlplane/api"; | ||
import type React from "react"; | ||
import Link from "next/link"; | ||
import { useParams, usePathname } from "next/navigation"; | ||
|
@@ -18,9 +19,13 @@ import { nFormatter } from "../../_components/nFormatter"; | |
|
||
type RunbookNavBarProps = { | ||
totalJobs: number; | ||
runbook: NonNullable<RouterOutputs["runbook"]["byId"]>; | ||
}; | ||
|
||
export const RunbookNavBar: React.FC<RunbookNavBarProps> = ({ totalJobs }) => { | ||
export const RunbookNavBar: React.FC<RunbookNavBarProps> = ({ | ||
totalJobs, | ||
runbook, | ||
}) => { | ||
const { workspaceSlug, systemSlug, runbookId } = useParams<{ | ||
workspaceSlug: string; | ||
systemSlug: string; | ||
|
@@ -56,16 +61,18 @@ export const RunbookNavBar: React.FC<RunbookNavBarProps> = ({ totalJobs }) => { | |
</NavigationMenuLink> | ||
</Link> | ||
</NavigationMenuItem> | ||
<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> | ||
{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> | ||
)} | ||
Comment on lines
+64
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Settings should remain accessible regardless of runhooks presence The current implementation that hides the Settings menu when runhooks exist is problematic because:
Recommendations:
🔗 Analysis chainVerify 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:
Consider:
Let's check if this behavior is consistent with other similar components: 🏁 Scripts executedThe 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 |
||
</NavigationMenuList> | ||
</NavigationMenu> | ||
</div> | ||
|
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.
🛠️ 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: