-
Notifications
You must be signed in to change notification settings - Fork 509
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: disable save button when workspace name is empty #2294
Changes from 10 commits
0a496f0
dd0bb12
f4f2944
fcc6a2e
c81f329
5dd0fb9
a0d8e88
66e39d0
da36a76
fa6b514
689cc86
8baaeca
22f00b5
186cc64
b86916d
0dfa31b
c3dab41
537064e
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 | ||||
---|---|---|---|---|---|---|
|
@@ -11,7 +11,7 @@ import { Button, buttonVariants } from "@/components/ui/button"; | |||||
import { Card, CardContent, CardHeader } from "@/components/ui/card"; | ||||||
import { Separator } from "@/components/ui/separator"; | ||||||
import { getTenantId } from "@/lib/auth"; | ||||||
import { and, db, eq, isNull, schema } from "@/lib/db"; | ||||||
import { and, db, eq, isNull, Role, schema } from "@/lib/db"; | ||||||
import { formatNumber } from "@/lib/fmt"; | ||||||
import { | ||||||
getLastUsed, | ||||||
|
@@ -25,6 +25,9 @@ import ms from "ms"; | |||||
import { notFound } from "next/navigation"; | ||||||
import { Chart } from "./chart"; | ||||||
import { VerificationTable } from "./verification-table"; | ||||||
import RolePermissionsTree from "./permission-list"; | ||||||
import { NestedPermissions } from "@/app/(app)/authorization/roles/[roleId]/tree"; | ||||||
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. Use The import of Apply this diff to fix the issue: -import { NestedPermissions } from "@/app/(app)/authorization/roles/[roleId]/tree";
+import type { NestedPermissions } from "@/app/(app)/authorization/roles/[roleId]/tree"; 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome
|
||||||
import PermissionTree from "./permission-list"; | ||||||
|
||||||
export default async function APIKeyDetailPage(props: { | ||||||
params: { | ||||||
|
@@ -71,6 +74,8 @@ export default async function APIKeyDetailPage(props: { | |||||
}, | ||||||
}, | ||||||
}); | ||||||
|
||||||
|
||||||
if (!key || key.workspace.tenantId !== tenantId) { | ||||||
return notFound(); | ||||||
} | ||||||
|
@@ -155,6 +160,41 @@ export default async function APIKeyDetailPage(props: { | |||||
} | ||||||
} | ||||||
|
||||||
const roleTee = key.workspace.roles.map((role) => { | ||||||
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. Correct variable name from The variable Apply this diff to fix the typo: - const roleTee = key.workspace.roles.map((role) => {
+ const roleTree = key.workspace.roles.map((role) => { And update the usage on line 351: -<PermissionTree roles={roleTee} />
+<PermissionTree roles={roleTree} /> Also applies to: 351-351 |
||||||
|
||||||
const nested: NestedPermissions = {}; | ||||||
for (const permission of key.workspace.permissions) { | ||||||
let n = nested; | ||||||
const parts = permission.name.split("."); | ||||||
for (let i = 0; i < parts.length; i++) { | ||||||
const p = parts[i]; | ||||||
if (!(p in n)) { | ||||||
n[p] = { | ||||||
id: permission.id, | ||||||
name: permission.name, | ||||||
description: permission.description, | ||||||
checked: role.permissions.some((p) => p.permissionId === permission.id), | ||||||
part: p, | ||||||
permissions: {}, | ||||||
path: parts.slice(0, i).join("."), | ||||||
}; | ||||||
} | ||||||
n = n[p].permissions; | ||||||
} | ||||||
} | ||||||
let data = { | ||||||
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. Use The variable Apply this diff to fix the issue: - let data = {
+ const data = { 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome
|
||||||
id: role.id, | ||||||
name: role.name, | ||||||
description: role.description, | ||||||
keyId: key.id, | ||||||
active: key.roles.some((keyRole) => keyRole.roleId === role.id), | ||||||
nestedPermissions: nested | ||||||
} | ||||||
return data | ||||||
}) | ||||||
|
||||||
|
||||||
|
||||||
return ( | ||||||
<div className="flex flex-col"> | ||||||
<div className="flex items-center justify-between w-full"> | ||||||
|
@@ -308,7 +348,8 @@ export default async function APIKeyDetailPage(props: { | |||||
</div> | ||||||
</div> | ||||||
|
||||||
<Chart | ||||||
<PermissionTree roles={roleTee} /> | ||||||
{/* <Chart | ||||||
apiId={props.params.apiId} | ||||||
key={JSON.stringify(key)} | ||||||
data={key} | ||||||
|
@@ -320,7 +361,7 @@ export default async function APIKeyDetailPage(props: { | |||||
...p, | ||||||
active: transientPermissionIds.has(p.id), | ||||||
}))} | ||||||
/> | ||||||
/> */} | ||||||
</div> | ||||||
</div> | ||||||
); | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,111 @@ | ||||||||||||||||||
"use client" | ||||||||||||||||||
import React, { useState, useEffect } from 'react'; | ||||||||||||||||||
import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; | ||||||||||||||||||
import { Collapsible, CollapsibleContent, CollapsibleTrigger } from "@/components/ui/collapsible"; | ||||||||||||||||||
import { Label } from "@/components/ui/label"; | ||||||||||||||||||
import { Switch } from "@/components/ui/switch"; | ||||||||||||||||||
import { RecursivePermission } from '@/app/(app)/authorization/roles/[roleId]/tree'; | ||||||||||||||||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; | ||||||||||||||||||
import { CopyButton } from '@/components/dashboard/copy-button'; | ||||||||||||||||||
import { RoleToggle } from './role-toggle'; | ||||||||||||||||||
import { ChevronRight } from 'lucide-react'; | ||||||||||||||||||
|
||||||||||||||||||
export type NestedPermission = { | ||||||||||||||||||
id: string; | ||||||||||||||||||
checked: boolean; | ||||||||||||||||||
description: string | null; | ||||||||||||||||||
name: string; | ||||||||||||||||||
part: string; | ||||||||||||||||||
path: string; | ||||||||||||||||||
permissions: NestedPermissions; | ||||||||||||||||||
}; | ||||||||||||||||||
|
||||||||||||||||||
export type NestedPermissions = Record<string, NestedPermission>; | ||||||||||||||||||
|
||||||||||||||||||
export type Role = { | ||||||||||||||||||
id: string; | ||||||||||||||||||
name: string; | ||||||||||||||||||
keyId: string | ||||||||||||||||||
active: boolean | ||||||||||||||||||
description: string | null; | ||||||||||||||||||
nestedPermissions: NestedPermissions; | ||||||||||||||||||
}; | ||||||||||||||||||
|
||||||||||||||||||
type PermissionTreeProps = { | ||||||||||||||||||
roles: Role[]; | ||||||||||||||||||
}; | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
export default function PermissionTree({ roles }: PermissionTreeProps) { | ||||||||||||||||||
const [openAll, setOpenAll] = useState(false); | ||||||||||||||||||
const [openRoles, setOpenRoles] = useState<string[]>([]); | ||||||||||||||||||
|
||||||||||||||||||
useEffect(() => { | ||||||||||||||||||
setOpenRoles(openAll ? roles.map(role => role.id) : []); | ||||||||||||||||||
}, [openAll, roles]); | ||||||||||||||||||
|
||||||||||||||||||
return ( | ||||||||||||||||||
<Card> | ||||||||||||||||||
<CardHeader className="flex-row items-start justify-between"> | ||||||||||||||||||
<div className="flex flex-col space-y-1.5"> | ||||||||||||||||||
<CardTitle>Permissions</CardTitle> | ||||||||||||||||||
</div> | ||||||||||||||||||
<div className="flex items-center gap-2"> | ||||||||||||||||||
<Label>{openAll ? "Collapse" : "Expand"} All Roles</Label> | ||||||||||||||||||
<Switch checked={openAll} onCheckedChange={setOpenAll} /> | ||||||||||||||||||
</div> | ||||||||||||||||||
</CardHeader> | ||||||||||||||||||
|
||||||||||||||||||
<CardContent className="flex flex-col gap-4"> | ||||||||||||||||||
{roles.map(role => { | ||||||||||||||||||
const isOpen = openRoles.includes(role.id); | ||||||||||||||||||
return ( | ||||||||||||||||||
<Collapsible | ||||||||||||||||||
key={role.id} | ||||||||||||||||||
open={isOpen} | ||||||||||||||||||
onOpenChange={(open) => { | ||||||||||||||||||
setOpenRoles(prev => | ||||||||||||||||||
open | ||||||||||||||||||
? prev.includes(role.id) ? prev : [...prev, role.id] | ||||||||||||||||||
: prev.filter(id => id !== role.id) | ||||||||||||||||||
); | ||||||||||||||||||
}} | ||||||||||||||||||
> | ||||||||||||||||||
<CollapsibleTrigger className="flex items-center gap-1 transition-all [&[data-state=open]>svg]:rotate-90 "> | ||||||||||||||||||
<Tooltip delayDuration={50}> | ||||||||||||||||||
<TooltipTrigger className="flex items-center gap-2"> | ||||||||||||||||||
<ChevronRight className="w-4 h-4 transition-transform duration-200" /> | ||||||||||||||||||
<RoleToggle keyId={role.keyId} roleId={role.id} checked={role.active} /> | ||||||||||||||||||
<span className="text-sm">{role.name}</span> | ||||||||||||||||||
</TooltipTrigger> | ||||||||||||||||||
<TooltipContent side="top" align="start" avoidCollisions={true}> | ||||||||||||||||||
<div className="flex items-center justify-start max-w-sm gap-2 text-content"> | ||||||||||||||||||
<span className="text-ellipsis overflow-hidden hover:overflow-visible">{role.name}</span> | ||||||||||||||||||
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. Ensure The Apply one of the following changes: Option 1: Change -<span className="text-ellipsis overflow-hidden hover:overflow-visible">{role.name}</span>
+<div className="text-ellipsis overflow-hidden hover:overflow-visible">{role.name}</div> Option 2: Add <span
+ className="inline-block text-ellipsis overflow-hidden hover:overflow-visible"
>
{role.name}
</span> 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
<div> | ||||||||||||||||||
<CopyButton value={role.name} /> | ||||||||||||||||||
</div> | ||||||||||||||||||
</div> | ||||||||||||||||||
</TooltipContent> | ||||||||||||||||||
</Tooltip> | ||||||||||||||||||
|
||||||||||||||||||
</CollapsibleTrigger> | ||||||||||||||||||
<CollapsibleContent className="pt-2 pb-4"> | ||||||||||||||||||
<div className="flex flex-col gap-1 ml-4"> | ||||||||||||||||||
{Object.entries(role.nestedPermissions).map(([k, p]) => ( | ||||||||||||||||||
<RecursivePermission | ||||||||||||||||||
key={p.id} | ||||||||||||||||||
k={k} | ||||||||||||||||||
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 Rename prop In the Apply this diff: - k={k}
+ permissionKey={k} Ensure that the
|
||||||||||||||||||
{...p} | ||||||||||||||||||
roleId={role.id} | ||||||||||||||||||
openAll={openAll} | ||||||||||||||||||
/> | ||||||||||||||||||
))} | ||||||||||||||||||
</div> | ||||||||||||||||||
</CollapsibleContent> | ||||||||||||||||||
</Collapsible> | ||||||||||||||||||
); | ||||||||||||||||||
})} | ||||||||||||||||||
</CardContent> | ||||||||||||||||||
</Card> | ||||||||||||||||||
); | ||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,9 +13,15 @@ import { useForm } from "react-hook-form"; | |
import { z } from "zod"; | ||
|
||
export const dynamic = "force-dynamic"; | ||
|
||
const validCharactersRegex = /^[a-zA-Z0-9-_]+$/; | ||
|
||
const formSchema = z.object({ | ||
workspaceId: z.string(), | ||
name: z.string(), | ||
name: z.string().min(3) | ||
.refine((v) => validCharactersRegex.test(v), { | ||
message: "worksapce can only contain letters, numbers, dashes and underscores", | ||
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. typo |
||
}), | ||
}); | ||
|
||
type Props = { | ||
|
@@ -53,7 +59,7 @@ export const UpdateWorkspaceName: React.FC<Props> = ({ workspace }) => { | |
async function onSubmit(values: z.infer<typeof formSchema>) { | ||
await updateName.mutateAsync(values); | ||
} | ||
|
||
const isDisabled = form.formState.isLoading || !form.formState.isValid; | ||
return ( | ||
<Form {...form}> | ||
<form onSubmit={form.handleSubmit(onSubmit)}> | ||
|
@@ -84,7 +90,7 @@ export const UpdateWorkspaceName: React.FC<Props> = ({ workspace }) => { | |
<Button | ||
variant={updateName.isLoading ? "disabled" : "primary"} | ||
type="submit" | ||
disabled={updateName.isLoading} | ||
disabled={updateName.isLoading || isDisabled} | ||
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. put it all in here or all in a separate variable, but please don't do it half and half |
||
> | ||
{updateName.isLoading ? <Loading /> : "Save"} | ||
</Button> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,8 @@ import { ThemeProvider } from "./theme-provider"; | |
const inter = Inter({ | ||
subsets: ["latin"], | ||
variable: "--font-inter", | ||
display: 'swap', | ||
adjustFontFallback: false | ||
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 Remove or correct the The 🔗 Analysis chainApprove font swap, but verify The addition of However, To check if Verify the implementation of The property To ensure it's correctly implemented and intended, please verify where and how [/run_scripts] 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for 'adjustFontFallback' in all TypeScript and JavaScript files
rg --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx}' --type ts --type js 'adjustFontFallback'
Length of output: 332 Script: #!/bin/bash
# Search for the definition or implementation of 'adjustFontFallback'
rg 'adjustFontFallback' --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx}' .
Length of output: 322 |
||
}); | ||
|
||
const pangea = localFont({ | ||
|
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.
Remove unused import
RolePermissionsTree
The import
RolePermissionsTree
on line 28 is not used in this file. Removing it will clean up the code and prevent unnecessary module loading.Apply this diff to fix the issue:
-import RolePermissionsTree from "./permission-list";
📝 Committable suggestion
🧰 Tools
🪛 Biome