From 75b7fc440a36673bec85bd4022699e2967131773 Mon Sep 17 00:00:00 2001 From: Ryan Hopper-Lowe <46546486+ryanhopperlowe@users.noreply.github.com> Date: Fri, 18 Oct 2024 19:06:49 -0500 Subject: [PATCH] Ui/feat/edit-oauth-apps (#216) * UI feat: allow users to edit oauth apps Signed-off-by: Ryan Hopper-Lowe * UI enhance: improve delete oauth app workflow * UI enhance: update oauth app delete button variant * UI chore: add error messages to oauth app api calls Signed-off-by: Ryan Hopper-Lowe * UI refactor: prepopulate cache with oath data on page load * UI chore: reduce complexity + improve typesafety of OAuth app spec * Add header content for OAuth Apps * UI enhance: improve coloring on delete button confirmation * UI fix: prevent global swr revalidation on focus --------- Signed-off-by: Ryan Hopper-Lowe --- .../composed/ConfirmationDialog.tsx | 49 ++++++++ ui/admin/app/components/header/HeaderNav.tsx | 8 +- .../components/oauth-apps/CreateOauthApp.tsx | 30 ++--- .../components/oauth-apps/DeleteOAuthApp.tsx | 99 ++++++----------- .../components/oauth-apps/EditOAuthApp.tsx | 68 ++++++++++++ .../components/oauth-apps/OAuthAppForm.tsx | 105 +++++++++++++----- .../components/oauth-apps/OAuthAppList.tsx | 49 +++++--- .../app/hooks/oauthApps/useOAuthAppSpec.ts | 14 +++ ui/admin/app/lib/model/oauthApps.ts | 24 ++-- .../app/lib/service/api/oauthAppService.ts | 9 +- ui/admin/app/root.tsx | 17 +-- ui/admin/app/routes/_auth.oauth-apps.tsx | 25 +++-- 12 files changed, 333 insertions(+), 164 deletions(-) create mode 100644 ui/admin/app/components/composed/ConfirmationDialog.tsx create mode 100644 ui/admin/app/components/oauth-apps/EditOAuthApp.tsx create mode 100644 ui/admin/app/hooks/oauthApps/useOAuthAppSpec.ts diff --git a/ui/admin/app/components/composed/ConfirmationDialog.tsx b/ui/admin/app/components/composed/ConfirmationDialog.tsx new file mode 100644 index 000000000..2e96f125d --- /dev/null +++ b/ui/admin/app/components/composed/ConfirmationDialog.tsx @@ -0,0 +1,49 @@ +import { ComponentProps, ReactNode } from "react"; + +import { Button } from "~/components/ui/button"; +import { + Dialog, + DialogClose, + DialogContent, + DialogDescription, + DialogFooter, + DialogTitle, + DialogTrigger, +} from "~/components/ui/dialog"; + +export function ConfirmationDialog({ + children, + title, + description, + onConfirm, + onCancel, + confirmProps, + ...dialogProps +}: ComponentProps & { + children?: ReactNode; + title: ReactNode; + description?: ReactNode; + onConfirm: () => void; + onCancel?: () => void; + confirmProps?: Omit>, "onClick">; +}) { + return ( + + {children && {children}} + + + {title} + {description} + + + + + + + + + + ); +} diff --git a/ui/admin/app/components/header/HeaderNav.tsx b/ui/admin/app/components/header/HeaderNav.tsx index ab316eb6d..b95674073 100644 --- a/ui/admin/app/components/header/HeaderNav.tsx +++ b/ui/admin/app/components/header/HeaderNav.tsx @@ -81,7 +81,7 @@ function getHeaderContent(route: string) { } if (new RegExp($path("/agents")).test(route)) { - return ; + return <>Agents; } if (new RegExp($path("/threads")).test(route)) { @@ -93,14 +93,10 @@ function getHeaderContent(route: string) { } if (new RegExp($path("/users")).test(route)) { - return ; + return <>Users; } } -const UsersContent = () => <>Users; - -const AgentsContent = () => <>Agents; - const AgentEditContent = () => { const params = useParams(); const { agent: agentId } = $params("/agents/:agent", params); diff --git a/ui/admin/app/components/oauth-apps/CreateOauthApp.tsx b/ui/admin/app/components/oauth-apps/CreateOauthApp.tsx index 547f7784b..dccbec2c2 100644 --- a/ui/admin/app/components/oauth-apps/CreateOauthApp.tsx +++ b/ui/admin/app/components/oauth-apps/CreateOauthApp.tsx @@ -3,7 +3,6 @@ import { PlusIcon } from "lucide-react"; import { useState } from "react"; import { mutate } from "swr"; -import { OAuthAppSpec, OAuthAppType } from "~/lib/model/oauthApps"; import { OauthAppService } from "~/lib/service/api/oauthAppService"; import { Button } from "~/components/ui/button"; @@ -20,21 +19,17 @@ import { PopoverContent, PopoverTrigger, } from "~/components/ui/popover"; +import { useOAuthAppSpec } from "~/hooks/oauthApps/useOAuthAppSpec"; import { useAsync } from "~/hooks/useAsync"; import { useDisclosure } from "~/hooks/useDisclosure"; import { OAuthAppForm } from "./OAuthAppForm"; -type CreateOauthAppProps = { - spec: OAuthAppSpec; -}; - -export function CreateOauthApp({ spec }: CreateOauthAppProps) { +export function CreateOauthApp() { const selectModal = useDisclosure(); + const { data: spec } = useOAuthAppSpec(); - const [selectedAppKey, setSelectedAppKey] = useState( - null - ); + const [selectedAppKey, setSelectedAppKey] = useState(null); const createApp = useAsync(OauthAppService.createOauthApp, { onSuccess: () => { @@ -43,6 +38,8 @@ export function CreateOauthApp({ spec }: CreateOauthAppProps) { }, }); + const selectedSpec = selectedAppKey ? spec.get(selectedAppKey) : null; + return ( <> - {Object.entries(spec).map( + {Array.from(spec.entries()).map( ([key, { displayName }]) => ( { - setSelectedAppKey( - key as OAuthAppType - ); + setSelectedAppKey(key); selectModal.onClose(); }} > @@ -89,20 +84,19 @@ export function CreateOauthApp({ spec }: CreateOauthAppProps) { onOpenChange={() => setSelectedAppKey(null)} > - {selectedAppKey && spec[selectedAppKey] && ( + {selectedAppKey && selectedSpec && ( <> - Create {spec[selectedAppKey].displayName} OAuth - App + Create {selectedSpec.displayName} OAuth App Create a new OAuth app for{" "} - {spec[selectedAppKey].displayName} + {selectedSpec.displayName} createApp.execute({ type: selectedAppKey, diff --git a/ui/admin/app/components/oauth-apps/DeleteOAuthApp.tsx b/ui/admin/app/components/oauth-apps/DeleteOAuthApp.tsx index 7e7f06434..0a0fbb34c 100644 --- a/ui/admin/app/components/oauth-apps/DeleteOAuthApp.tsx +++ b/ui/admin/app/components/oauth-apps/DeleteOAuthApp.tsx @@ -1,8 +1,9 @@ -import { Check, TrashIcon, XIcon } from "lucide-react"; +import { TrashIcon } from "lucide-react"; import { mutate } from "swr"; import { OauthAppService } from "~/lib/service/api/oauthAppService"; +import { ConfirmationDialog } from "~/components/composed/ConfirmationDialog"; import { LoadingSpinner } from "~/components/ui/LoadingSpinner"; import { Button } from "~/components/ui/button"; import { @@ -12,81 +13,43 @@ import { TooltipTrigger, } from "~/components/ui/tooltip"; import { useAsync } from "~/hooks/useAsync"; -import { useDisclosure } from "~/hooks/useDisclosure"; export function DeleteOAuthApp({ id }: { id: string }) { - const confirmation = useDisclosure(); - const deleteOAuthApp = useAsync(OauthAppService.deleteOauthApp, { onSuccess: () => mutate(OauthAppService.getOauthApps.key()), }); return (
- {confirmation.isOpen ? ( - <> - - - - - - - Cancel - - - - - - - - - - Confirm Delete - - - - ) : ( - <> - - - - - - - Delete - - - - + + + + Delete + +
); } diff --git a/ui/admin/app/components/oauth-apps/EditOAuthApp.tsx b/ui/admin/app/components/oauth-apps/EditOAuthApp.tsx new file mode 100644 index 000000000..d2cf3ec14 --- /dev/null +++ b/ui/admin/app/components/oauth-apps/EditOAuthApp.tsx @@ -0,0 +1,68 @@ +import { SquarePenIcon } from "lucide-react"; +import { mutate } from "swr"; + +import { OAuthApp } from "~/lib/model/oauthApps"; +import { OauthAppService } from "~/lib/service/api/oauthAppService"; + +import { Button } from "~/components/ui/button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogTitle, + DialogTrigger, +} from "~/components/ui/dialog"; +import { useOAuthAppSpec } from "~/hooks/oauthApps/useOAuthAppSpec"; +import { useAsync } from "~/hooks/useAsync"; +import { useDisclosure } from "~/hooks/useDisclosure"; + +import { OAuthAppForm } from "./OAuthAppForm"; + +type EditOAuthAppProps = { + oauthApp: OAuthApp; +}; + +export function EditOAuthApp({ oauthApp }: EditOAuthAppProps) { + const updateApp = useAsync(OauthAppService.updateOauthApp, { + onSuccess: async () => { + await mutate(OauthAppService.getOauthApps.key()); + modal.onClose(); + }, + }); + + const modal = useDisclosure(); + + const { data: spec } = useOAuthAppSpec(); + + const typeSpec = spec.get(oauthApp.type); + if (!typeSpec) return null; + + return ( + + + + + + + Edit OAuth App ({oauthApp.type}) + + + + + updateApp.execute(oauthApp.id, { + type: oauthApp.type, + ...data, + }) + } + /> + + + ); +} diff --git a/ui/admin/app/components/oauth-apps/OAuthAppForm.tsx b/ui/admin/app/components/oauth-apps/OAuthAppForm.tsx index e0b824ec4..52b438150 100644 --- a/ui/admin/app/components/oauth-apps/OAuthAppForm.tsx +++ b/ui/admin/app/components/oauth-apps/OAuthAppForm.tsx @@ -1,54 +1,80 @@ import { zodResolver } from "@hookform/resolvers/zod"; -import { useMemo } from "react"; +import { useEffect, useMemo } from "react"; import { useForm } from "react-hook-form"; import { z } from "zod"; -import { - OAuthAppParams, - OAuthAppSpec, - OAuthAppType, -} from "~/lib/model/oauthApps"; +import { OAuthApp, OAuthAppInfo, OAuthAppParams } from "~/lib/model/oauthApps"; import { ControlledInput } from "~/components/form/controlledInputs"; import { Button } from "~/components/ui/button"; import { Form } from "~/components/ui/form"; type OAuthAppFormProps = { - appSpec: OAuthAppSpec[OAuthAppType]; + appSpec: OAuthAppInfo; onSubmit: (data: OAuthAppParams) => void; + oauthApp?: OAuthApp; }; -export function OAuthAppForm({ appSpec, onSubmit }: OAuthAppFormProps) { - const fields = Object.entries(appSpec.parameters).map(([key, label]) => ({ - key: key as keyof OAuthAppParams, - label, - })); +export function OAuthAppForm({ + appSpec, + onSubmit, + oauthApp, +}: OAuthAppFormProps) { + const isEdit = !!oauthApp; + + const fields = useMemo(() => { + return Object.entries(appSpec.parameters).map(([key, label]) => ({ + key: key as keyof OAuthAppParams, + label, + })); + }, [appSpec.parameters]); const schema = useMemo(() => { return z.object( - Object.entries(appSpec.parameters).reduce( - (acc, [key]) => { - acc[key as keyof OAuthAppParams] = z.string(); + fields.reduce( + (acc, { key }) => { + acc[key] = z.string(); return acc; }, - {} as Record + {} as Record ) ); - }, [appSpec.parameters]); + }, [fields]); + + const defaultValues = useMemo(() => { + return fields.reduce((acc, { key }) => { + acc[key] = oauthApp?.[key] ?? ""; + + // if editing, use placeholder to show secret value exists + // use a uuid to ensure it never collides with a real secret + if (key === "clientSecret" && isEdit) { + acc.clientSecret = SECRET_PLACEHOLDER; + } + + return acc; + }, {} as OAuthAppParams); + }, [fields, oauthApp, isEdit]); const form = useForm({ - defaultValues: Object.entries(appSpec.parameters).reduce( - (acc, [key]) => { - acc[key as keyof OAuthAppParams] = ""; - return acc; - }, - {} as OAuthAppParams - ), + defaultValues, resolver: zodResolver(schema), }); - const handleSubmit = form.handleSubmit(onSubmit); + useEffect(() => { + form.reset(defaultValues); + }, [defaultValues, form]); + const handleSubmit = form.handleSubmit((data) => { + const { clientSecret, ...rest } = data; + + // if the user skips editing the client secret, we don't want to submit an empty string + // because that will clear it out on the server + if (isEdit && clientSecret === SECRET_PLACEHOLDER) { + onSubmit(rest); + } else { + onSubmit(data); + } + }); return (
@@ -58,6 +84,13 @@ export function OAuthAppForm({ appSpec, onSubmit }: OAuthAppFormProps) { name={key} label={label} control={form.control} + {...(key === "clientSecret" + ? { + onBlur: onBlurClientSecret, + onFocus: onFocusClientSecret, + type: "password", + } + : {})} /> ))} @@ -65,4 +98,26 @@ export function OAuthAppForm({ appSpec, onSubmit }: OAuthAppFormProps) {
); + + function onBlurClientSecret() { + if (!isEdit) return; + + const { clientSecret } = form.getValues(); + + if (!clientSecret) { + form.setValue("clientSecret", SECRET_PLACEHOLDER); + } + } + + function onFocusClientSecret() { + if (!isEdit) return; + + const { clientSecret } = form.getValues(); + + if (clientSecret === SECRET_PLACEHOLDER) { + form.setValue("clientSecret", ""); + } + } } + +const SECRET_PLACEHOLDER = crypto.randomUUID(); diff --git a/ui/admin/app/components/oauth-apps/OAuthAppList.tsx b/ui/admin/app/components/oauth-apps/OAuthAppList.tsx index 750b341c1..ffb07f2f8 100644 --- a/ui/admin/app/components/oauth-apps/OAuthAppList.tsx +++ b/ui/admin/app/components/oauth-apps/OAuthAppList.tsx @@ -1,33 +1,32 @@ import { ColumnDef, createColumnHelper } from "@tanstack/react-table"; +import { KeyIcon } from "lucide-react"; import { useMemo } from "react"; import useSWR from "swr"; -import { OAuthApp, OAuthAppSpec } from "~/lib/model/oauthApps"; +import { OAuthApp } from "~/lib/model/oauthApps"; import { OauthAppService } from "~/lib/service/api/oauthAppService"; import { cn } from "~/lib/utils"; import { DataTable } from "~/components/composed/DataTable"; +import { useOAuthAppSpec } from "~/hooks/oauthApps/useOAuthAppSpec"; import { DeleteOAuthApp } from "./DeleteOAuthApp"; +import { EditOAuthApp } from "./EditOAuthApp"; type Row = OAuthApp & { created?: string; isGateway?: boolean }; const columnHelper = createColumnHelper(); -export function OAuthAppList({ - defaultData, - spec, -}: { - defaultData: OAuthApp[]; - spec: OAuthAppSpec; -}) { +export function OAuthAppList() { + const { data: spec } = useOAuthAppSpec(); + const { data: apps } = useSWR( OauthAppService.getOauthApps.key(), OauthAppService.getOauthApps, - { fallbackData: defaultData } + { fallbackData: [] } ); const rows = useMemo(() => { - const typesWithNoApps = Object.entries(spec) + const typesWithNoApps = Array.from(spec.entries()) .map(([type, { displayName }]) => { if (apps.some((app) => app.type === type)) return null; @@ -61,14 +60,35 @@ export function OAuthAppList({ function getColumns(): ColumnDef[] { return [ + columnHelper.display({ + id: "icon", + cell: ({ row }) => { + const app = row.original; + const { icon } = spec.get(app.type) || {}; + return icon ? ( + {app.type} + ) : ( + + ); + }, + }), + columnHelper.accessor( + (app) => spec.get(app.type)?.displayName ?? app.type, + { + id: "type", + header: "Type", + } + ), columnHelper.accessor((app) => app.name ?? app.id, { id: "name", header: "Name / Id", }), - columnHelper.accessor((app) => spec[app.type].displayName, { - id: "type", - header: "Type", - }), columnHelper.accessor( (app) => app.created ? new Date(app.created).toLocaleString() : "-", @@ -79,6 +99,7 @@ export function OAuthAppList({ cell: ({ row }) => !row.original.isGateway && (
+
), diff --git a/ui/admin/app/hooks/oauthApps/useOAuthAppSpec.ts b/ui/admin/app/hooks/oauthApps/useOAuthAppSpec.ts new file mode 100644 index 000000000..c5fcd7c01 --- /dev/null +++ b/ui/admin/app/hooks/oauthApps/useOAuthAppSpec.ts @@ -0,0 +1,14 @@ +import useSWR from "swr"; + +import { OAuthAppInfo } from "~/lib/model/oauthApps"; +import { OauthAppService } from "~/lib/service/api/oauthAppService"; + +const fallbackData = new Map(); + +export function useOAuthAppSpec() { + return useSWR( + OauthAppService.getSupportedOauthAppTypes.key(), + OauthAppService.getSupportedOauthAppTypes, + { fallbackData } + ); +} diff --git a/ui/admin/app/lib/model/oauthApps.ts b/ui/admin/app/lib/model/oauthApps.ts index 6c68d9bad..f5c04f537 100644 --- a/ui/admin/app/lib/model/oauthApps.ts +++ b/ui/admin/app/lib/model/oauthApps.ts @@ -1,16 +1,5 @@ import { EntityMeta } from "./primitives"; -export const OAuthAppType = { - Microsoft365: "microsoft365", - Slack: "slack", - Notion: "notion", - HubSpot: "hubspot", - GitHub: "github", - Google: "google", - Custom: "custom", -} as const; -export type OAuthAppType = (typeof OAuthAppType)[keyof typeof OAuthAppType]; - export type OAuthAppParams = { refName?: string; name?: string; @@ -31,12 +20,15 @@ export type OAuthAppParams = { }; export type OAuthAppBase = OAuthAppParams & { - type: OAuthAppType; + type: string; }; export type OAuthApp = EntityMeta & OAuthAppBase; -export type OAuthAppSpec = Record< - OAuthAppType, - { displayName: string; parameters: Record } ->; +export type OAuthAppInfo = { + displayName: string; + icon?: string; + parameters: Record; +}; + +export type OAuthAppSpec = Record; diff --git a/ui/admin/app/lib/service/api/oauthAppService.ts b/ui/admin/app/lib/service/api/oauthAppService.ts index a3bff8a56..ef2d43a23 100644 --- a/ui/admin/app/lib/service/api/oauthAppService.ts +++ b/ui/admin/app/lib/service/api/oauthAppService.ts @@ -6,6 +6,7 @@ import { request } from "./primitives"; const getOauthApps = async () => { const res = await request<{ items: OAuthApp[] }>({ url: ApiRoutes.oauthApps.getOauthApps().url, + errorMessage: "Failed to get OAuth apps", }); return res.data.items ?? ([] as OAuthApp[]); @@ -16,6 +17,7 @@ getOauthApps.key = () => const getOauthAppById = async (id: string) => { const res = await request({ url: ApiRoutes.oauthApps.getOauthAppById(id).url, + errorMessage: "Failed to get OAuth app", }); return res.data; @@ -31,6 +33,7 @@ const createOauthApp = async (oauthApp: OAuthAppBase) => { url: ApiRoutes.oauthApps.createOauthApp().url, method: "POST", data: oauthApp, + errorMessage: "Failed to create OAuth app", }); return res.data; @@ -41,6 +44,7 @@ const updateOauthApp = async (id: string, oauthApp: OAuthAppBase) => { url: ApiRoutes.oauthApps.updateOauthApp(id).url, method: "PATCH", data: oauthApp, + errorMessage: "Failed to update OAuth app", }); return res.data; @@ -50,15 +54,17 @@ const deleteOauthApp = async (id: string) => { await request({ url: ApiRoutes.oauthApps.deleteOauthApp(id).url, method: "DELETE", + errorMessage: "Failed to delete OAuth app", }); }; const getSupportedOauthAppTypes = async () => { const res = await request({ url: ApiRoutes.oauthApps.supportedOauthAppTypes().url, + errorMessage: "Failed to get supported OAuth app types", }); - return res.data; + return new Map(Object.entries(res.data)); }; getSupportedOauthAppTypes.key = () => ({ url: ApiRoutes.oauthApps.supportedOauthAppTypes().path }) as const; @@ -66,6 +72,7 @@ getSupportedOauthAppTypes.key = () => const getSupportedAuthTypes = async () => { const res = await request({ url: ApiRoutes.oauthApps.supportedAuthTypes().url, + errorMessage: "Failed to get supported auth types", }); return res.data; diff --git a/ui/admin/app/root.tsx b/ui/admin/app/root.tsx index 48e2b5773..7fa534ead 100644 --- a/ui/admin/app/root.tsx +++ b/ui/admin/app/root.tsx @@ -6,6 +6,7 @@ import { Scripts, ScrollRestoration, } from "@remix-run/react"; +import { SWRConfig } from "swr"; import { AuthProvider } from "~/components/auth/AuthContext"; import { LayoutProvider } from "~/components/layout/LayoutProvider"; @@ -52,13 +53,15 @@ export function Layout({ children }: { children: React.ReactNode }) { export default function App() { return ( - - - - - - - + + + + + + + + + ); } diff --git a/ui/admin/app/routes/_auth.oauth-apps.tsx b/ui/admin/app/routes/_auth.oauth-apps.tsx index dca5046b0..103ca7cbf 100644 --- a/ui/admin/app/routes/_auth.oauth-apps.tsx +++ b/ui/admin/app/routes/_auth.oauth-apps.tsx @@ -1,4 +1,5 @@ import { useLoaderData } from "@remix-run/react"; +import { preload } from "swr"; import { OauthAppService } from "~/lib/service/api/oauthAppService"; @@ -6,24 +7,30 @@ import { CreateOauthApp } from "~/components/oauth-apps/CreateOauthApp"; import { OAuthAppList } from "~/components/oauth-apps/OAuthAppList"; export async function clientLoader() { - const oauthApps = await OauthAppService.getOauthApps(); - const supportedApps = await OauthAppService.getSupportedOauthAppTypes(); - - return { oauthApps, supportedApps }; + await Promise.all([ + preload( + OauthAppService.getSupportedOauthAppTypes.key(), + OauthAppService.getSupportedOauthAppTypes + ), + preload( + OauthAppService.getOauthApps.key(), + OauthAppService.getOauthApps + ), + ]); + + return null; } export default function OauthApps() { - const { supportedApps, oauthApps } = useLoaderData(); - - console.log("oauthApps", supportedApps); + useLoaderData(); return (
- +
- +
); }