From 8da0312e9d5207a0b0e60db604f0aef8c296a7fb Mon Sep 17 00:00:00 2001 From: Sergey Kozyrenko Date: Tue, 12 May 2026 11:36:11 +0700 Subject: [PATCH] feat(knowledge): allow docType edit, partial UPDATE, and field-clear on save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend now accepts `docType` on `UpdateKnowledgeDocumentInput`, so the edit form was reworked end-to-end: - Form schema mirrors REST length limits (content 65536, question 2048, description 1000, codeLang 100) — GraphQL itself doesn't enforce them. - `docType` is editable on existing docs, not just on create. Switching it clears stale subtype values (`answerType`/`guideType`/`codeLang`) through `setValue` in the Select's `onValueChange`, not via effect, so freshly loaded documents keep their persisted subtype on first render. - UPDATE is now a partial payload built from RHF's `dirtyFields`: untouched optional fields are omitted, cleared fields go out as `""` so the backend wipes them (previously `'' → undefined` silently swallowed clears, leaving stale values on the server). - `KnowledgesProvider` drops the duplicate `Knowledge` interface and uses the GraphQL fragment directly; no more hand-rolled field mapping. - Both submit paths (form button and "Save & Leave" dialog) parse values through `formSchema.safeParse` so trim/normalisation is identical regardless of code path. Co-authored-by: Cursor --- .../knowledges/knowledge-form-controls.tsx | 79 ++++++---- .../features/knowledges/knowledge-form.tsx | 138 +++++++++++++----- frontend/src/graphql/types.ts | 1 + frontend/src/pages/knowledges/knowledge.tsx | 10 +- .../src/providers/knowledges-provider.tsx | 58 ++------ 5 files changed, 178 insertions(+), 108 deletions(-) diff --git a/frontend/src/features/knowledges/knowledge-form-controls.tsx b/frontend/src/features/knowledges/knowledge-form-controls.tsx index 27887db7..fa176a17 100644 --- a/frontend/src/features/knowledges/knowledge-form-controls.tsx +++ b/frontend/src/features/knowledges/knowledge-form-controls.tsx @@ -1,4 +1,4 @@ -import { type Control, useWatch } from 'react-hook-form'; +import { type Control, useFormContext, useWatch } from 'react-hook-form'; import type { KnowledgeAnswerType as KnowledgeAnswerTypeT, @@ -14,6 +14,8 @@ import { KnowledgeAnswerType, KnowledgeDocType, KnowledgeGuideType } from '@/gra import type { FormValues } from './knowledge-form'; +import { KNOWLEDGE_LIMITS } from './knowledge-form'; + // ` - - - - - - - {docTypeValues.map((value) => ( - - {value} - - ))} - - - ) : ( -
- {field.value || '—'} -
- )} + )} @@ -151,6 +171,7 @@ export const KnowledgeMetaFields = ({ control, isNew, isSaving }: KnowledgeMetaF @@ -175,6 +196,7 @@ export const KnowledgeMetaFields = ({ control, isNew, isSaving }: KnowledgeMetaF autoFocus={isNew} className="min-h-0" disabled={isSaving} + maxLength={KNOWLEDGE_LIMITS.question} maxRows={6} minRows={1} placeholder="Short title or question this document answers" @@ -198,6 +220,7 @@ export const KnowledgeMetaFields = ({ control, isNew, isSaving }: KnowledgeMetaF {...field} className="min-h-0" disabled={isSaving} + maxLength={KNOWLEDGE_LIMITS.description} maxRows={8} minRows={1} placeholder="Optional short description" diff --git a/frontend/src/features/knowledges/knowledge-form.tsx b/frontend/src/features/knowledges/knowledge-form.tsx index 6af9334e..3243c245 100644 --- a/frontend/src/features/knowledges/knowledge-form.tsx +++ b/frontend/src/features/knowledges/knowledge-form.tsx @@ -27,15 +27,47 @@ import { KnowledgeHeader } from './knowledge-header'; // Schema, types, pure helpers // --------------------------------------------------------------------------- +// Length limits mirror the REST validation tags on the Go side +// (`backend/pkg/server/models/knowledge.go`). The GraphQL layer itself does +// not enforce them, so without these the user could submit a payload that +// later round-trips through REST and gets rejected. +export const KNOWLEDGE_LIMITS = { + codeLang: 100, + content: 65536, + description: 1000, + question: 2048, +} as const; + +// Optional text fields are trimmed and length-checked but NOT collapsed to +// `undefined` — the partial-update logic in `formValuesToUpdateInput` needs +// to distinguish "user cleared a previously-set value" (send `""` so the +// backend clears it) from "field was empty and untouched" (don't send at +// all so the backend leaves it alone). Mapping `"" → undefined` here would +// erase that signal and break the "clear an existing description" use case. +const optionalTrimmed = (max: number, label: string) => + z + .string() + .trim() + .max(max, { message: `${label} must be ${max} characters or fewer` }) + .optional(); + export const formSchema = z .object({ answerType: z.nativeEnum(KnowledgeAnswerType).optional(), - codeLang: z.string().trim().optional(), - content: z.string().trim().min(1, { message: 'Content is required' }), - description: z.string().trim().optional(), + codeLang: optionalTrimmed(KNOWLEDGE_LIMITS.codeLang, 'Code language'), + content: z + .string() + .trim() + .min(1, { message: 'Content is required' }) + .max(KNOWLEDGE_LIMITS.content, { message: `Content must be ${KNOWLEDGE_LIMITS.content} characters or fewer` }), + description: optionalTrimmed(KNOWLEDGE_LIMITS.description, 'Description'), docType: z.nativeEnum(KnowledgeDocType), guideType: z.nativeEnum(KnowledgeGuideType).optional(), - question: z.string().trim().min(1, { message: 'Question is required' }), + question: z + .string() + .trim() + .min(1, { message: 'Question is required' }) + .max(KNOWLEDGE_LIMITS.question, { message: `Question must be ${KNOWLEDGE_LIMITS.question} characters or fewer` }), }) .superRefine((value, ctx) => { const requiredByDocType: Partial; message: string }>> = @@ -85,38 +117,62 @@ export const documentToFormValues = (k: KnowledgeDocumentFragmentFragment): Form question: k.question, }); -// `description` and `codeLang` are optional and we want empty strings to map -// to `undefined` so the backend treats them as "absent" instead of "set to ''". -const trimmedOrUndefined = (value: null | string | undefined): string | undefined => { - if (!value) { - return undefined; - } +// react-hook-form's `dirtyFields` is a partial map of the same shape as +// `FormValues`, with `true` for fields the user actually changed compared to +// `defaultValues`. We project it onto the (flat) FormValues keys here. +export type DirtyFlags = Partial>; - const trimmed = value.trim(); - - return trimmed.length === 0 ? undefined : trimmed; -}; - -// Shared field projection used by both create and update payloads. The only -// difference between the two GraphQL inputs is that `docType` is required on -// create and immutable on update — see `formValuesTo{Create,Update}Input` -// below. -const formValuesToBasePayload = (values: FormValues) => ({ - answerType: values.docType === KnowledgeDocType.Answer ? values.answerType : undefined, - codeLang: values.docType === KnowledgeDocType.Code ? trimmedOrUndefined(values.codeLang) : undefined, +// CREATE: send all required fields and only non-empty optional fields. There +// is no prior document to "clear", so an empty `description`/`codeLang` just +// means "don't store anything in cmetadata for this field". +export const formValuesToCreateInput = (values: FormValues): CreateKnowledgeDocumentInput => ({ + answerType: values.answerType, + codeLang: values.codeLang ? values.codeLang : undefined, content: values.content, - description: trimmedOrUndefined(values.description), - guideType: values.docType === KnowledgeDocType.Guide ? values.guideType : undefined, + description: values.description ? values.description : undefined, + docType: values.docType, + guideType: values.guideType, question: values.question, }); -export const formValuesToCreateInput = (values: FormValues): CreateKnowledgeDocumentInput => ({ - ...formValuesToBasePayload(values), - docType: values.docType, -}); +// UPDATE: send only fields the user actually edited. `content` is GraphQL- +// required (the backend always re-embeds), so it goes through unconditionally; +// every other field is gated by `dirty`. This way: +// - untouched fields stay `undefined` and the backend keeps the existing value; +// - cleared fields go out as `""` so the backend wipes them; +// - subtype-related fields cleared by `setValue` on docType change are +// marked dirty by the form, so they reach the backend with the right +// "clear me" value (the backend additionally wipes mismatching subtypes +// itself, but we mirror the user-visible state explicitly). +export const formValuesToUpdateInput = (values: FormValues, dirty: DirtyFlags): UpdateKnowledgeDocumentInput => { + const input: UpdateKnowledgeDocumentInput = { content: values.content }; -export const formValuesToUpdateInput = (values: FormValues): UpdateKnowledgeDocumentInput => - formValuesToBasePayload(values); + if (dirty.docType) { + input.docType = values.docType; + } + + if (dirty.question) { + input.question = values.question; + } + + if (dirty.description) { + input.description = values.description ?? ''; + } + + if (dirty.guideType) { + input.guideType = values.guideType; + } + + if (dirty.answerType) { + input.answerType = values.answerType; + } + + if (dirty.codeLang) { + input.codeLang = values.codeLang ?? ''; + } + + return input; +}; // --------------------------------------------------------------------------- // Form component @@ -132,7 +188,7 @@ interface KnowledgeFormProps { isNew: boolean; knowledge?: KnowledgeDocumentFragmentFragment | null; knowledgeName: null | string; - onSubmit: (values: FormValues) => Promise; + onSubmit: (values: FormValues, dirtyFields: DirtyFlags) => Promise; } export const KnowledgeForm = ({ initialValues, isNew, knowledge, knowledgeName, onSubmit }: KnowledgeFormProps) => { @@ -157,7 +213,11 @@ export const KnowledgeForm = ({ initialValues, isNew, knowledge, knowledgeName, const performSave = useCallback( async (values: FormValues): Promise => { try { - const result = await onSubmit(values); + // Snapshot dirty flags from the latest formState. We read it + // here (instead of capturing into deps) so partial-update + // logic in the page sees the same state RHF used to decide + // `isDirty`/`canSubmit` at submit time. + const result = await onSubmit(values, form.formState.dirtyFields as DirtyFlags); // Prefer the server's view of the document — backend may have // trimmed/normalized fields, attached derived data, or filled @@ -183,7 +243,7 @@ export const KnowledgeForm = ({ initialValues, isNew, knowledge, knowledgeName, return false; } }, - [navigate, onSubmit, reset], + [form, navigate, onSubmit, reset], ); // The ref below breaks an otherwise circular hook dependency: @@ -221,10 +281,20 @@ export const KnowledgeForm = ({ initialValues, isNew, knowledge, knowledgeName, return false; } + // `form.getValues()` returns raw field state (no zod transforms applied), + // so we run it through the schema explicitly. This way the dialog path + // produces the same trimmed/normalized values as the form-button path + // (which gets parsed values directly from `handleSubmit`'s callback). + const parsed = formSchema.safeParse(form.getValues()); + + if (!parsed.success) { + return false; + } + setIsSaving(true); try { - return await performSave(form.getValues()); + return await performSave(parsed.data); } finally { setIsSaving(false); } diff --git a/frontend/src/graphql/types.ts b/frontend/src/graphql/types.ts index 969d0385..f348a634 100644 --- a/frontend/src/graphql/types.ts +++ b/frontend/src/graphql/types.ts @@ -1229,6 +1229,7 @@ export type UpdateKnowledgeDocumentInput = { codeLang?: InputMaybe; content: Scalars['String']['input']; description?: InputMaybe; + docType?: InputMaybe; guideType?: InputMaybe; question?: InputMaybe; }; diff --git a/frontend/src/pages/knowledges/knowledge.tsx b/frontend/src/pages/knowledges/knowledge.tsx index b5ceb36b..6d68fce9 100644 --- a/frontend/src/pages/knowledges/knowledge.tsx +++ b/frontend/src/pages/knowledges/knowledge.tsx @@ -5,6 +5,7 @@ import { Button } from '@/components/ui/button'; import { Card, CardContent } from '@/components/ui/card'; import { Spinner } from '@/components/ui/spinner'; import { + type DirtyFlags, documentToFormValues, type FormValues, formValuesToCreateInput, @@ -39,7 +40,12 @@ const Knowledge = () => { ); const handleSubmit = useCallback( - async (values: FormValues): Promise => { + // `values` are the zod-parsed form output (trimmed, length-validated). + // CREATE sends a full payload; UPDATE sends only fields the user + // actually changed (`dirtyFields`) so untouched optional fields stay + // untouched on the backend and explicit clears (e.g. wiping an existing + // description) reach it as `""`. + async (values: FormValues, dirtyFields: DirtyFlags): Promise => { if (isNew) { const created = await createKnowledge(formValuesToCreateInput(values)); @@ -53,7 +59,7 @@ const Knowledge = () => { return {}; } - const updated = await updateKnowledge(knowledgeId, formValuesToUpdateInput(values)); + const updated = await updateKnowledge(knowledgeId, formValuesToUpdateInput(values, dirtyFields)); return { document: updated ?? undefined }; }, diff --git a/frontend/src/providers/knowledges-provider.tsx b/frontend/src/providers/knowledges-provider.tsx index e986541d..b73e3bff 100644 --- a/frontend/src/providers/knowledges-provider.tsx +++ b/frontend/src/providers/knowledges-provider.tsx @@ -1,12 +1,13 @@ import { createContext, type ReactNode, useCallback, useContext, useMemo } from 'react'; import { toast } from 'sonner'; -import type { CreateKnowledgeDocumentInput, UpdateKnowledgeDocumentInput } from '@/graphql/types'; +import type { + CreateKnowledgeDocumentInput, + KnowledgeDocumentFragmentFragment, + UpdateKnowledgeDocumentInput, +} from '@/graphql/types'; import { - KnowledgeAnswerType, - KnowledgeDocType, - KnowledgeGuideType, useCreateKnowledgeDocumentMutation, useDeleteKnowledgeDocumentMutation, useKnowledgeDocumentCreatedSubscription, @@ -18,23 +19,12 @@ import { import { Log } from '@/lib/log'; import { useUser } from '@/providers/user-provider'; -export interface Knowledge { - answerType?: KnowledgeAnswerType | null; - codeLang?: null | string; - content: string; - description?: null | string; - docType: KnowledgeDocType; - flowId?: null | string; - guideType?: KnowledgeGuideType | null; - id: string; - manual: boolean; - partSize: number; - question: string; - subtaskId?: null | string; - taskId?: null | string; - totalSize: number; - userId: string; -} +// The provider operates directly on the GraphQL fragment. Previously we kept a +// hand-rolled `Knowledge` shape that mirrored the fragment field-by-field; that +// duplication forced a manual mapping step and drifted from the schema. The +// alias keeps the public surface (`Knowledge`) for callers while making it +// obvious there is no extra translation layer. +export type Knowledge = KnowledgeDocumentFragmentFragment; interface KnowledgesContextValue { createKnowledge: (input: CreateKnowledgeDocumentInput) => Promise; @@ -70,27 +60,7 @@ export const KnowledgesProvider = ({ children }: KnowledgesProviderProps) => { useKnowledgeDocumentUpdatedSubscription({ skip: !shouldFetch }); useKnowledgeDocumentDeletedSubscription({ skip: !shouldFetch }); - const knowledges = useMemo(() => { - const raw = data?.knowledgeDocuments ?? []; - - return raw.map((d) => ({ - answerType: d.answerType ?? null, - codeLang: d.codeLang ?? null, - content: d.content, - description: d.description ?? null, - docType: d.docType, - flowId: d.flowId ?? null, - guideType: d.guideType ?? null, - id: d.id, - manual: d.manual, - partSize: d.partSize, - question: d.question, - subtaskId: d.subtaskId ?? null, - taskId: d.taskId ?? null, - totalSize: d.totalSize, - userId: d.userId, - })); - }, [data?.knowledgeDocuments]); + const knowledges = useMemo(() => data?.knowledgeDocuments ?? [], [data?.knowledgeDocuments]); const getKnowledge = useCallback( (id: string): Knowledge | undefined => knowledges.find((k) => k.id === id), @@ -102,7 +72,7 @@ export const KnowledgesProvider = ({ children }: KnowledgesProviderProps) => { try { const { data: result } = await createKnowledgeMutation({ variables: { input } }); - return result?.createKnowledgeDocument as Knowledge | undefined; + return result?.createKnowledgeDocument; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Failed to create knowledge document'; toast.error('Failed to create knowledge document', { description: errorMessage }); @@ -118,7 +88,7 @@ export const KnowledgesProvider = ({ children }: KnowledgesProviderProps) => { try { const { data: result } = await updateKnowledgeMutation({ variables: { id, input } }); - return result?.updateKnowledgeDocument as Knowledge | undefined; + return result?.updateKnowledgeDocument; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Failed to update knowledge document'; toast.error('Failed to update knowledge document', { description: errorMessage });