fix(platform): replace FormDialog isValid with isDirty for proper submit gating#697
Merged
Conversation
…mit gating The isValid prop caused forms to be unsubmittable until async validation resolved, which broke several dialogs. Replace with isDirty to gate submission on user interaction instead. Also remove mode: 'onChange' from useForm calls (unnecessary with zodResolver) and add shouldDirty: true to programmatic setValue calls so the dirty state propagates correctly.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Contributor
📝 WalkthroughWalkthroughThis PR systematically refactors form validation state management across the platform by replacing the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
services/platform/app/features/automations/components/automation-create-dialog.tsx (1)
48-55:⚠️ Potential issue | 🟠 MajorSubmit gating was removed instead of migrated to
isDirty.This dialog no longer provides any dirty-state gate to
FormDialog, so it no longer follows the PR’s interaction-based submit gating pattern.Proposed fix
const { register, handleSubmit, setError, - formState: { isSubmitting, errors }, + formState: { isSubmitting, isDirty, errors }, } = useForm<FormData>({ resolver: zodResolver(formSchema), }); @@ <FormDialog open={open} onOpenChange={onOpenChange} @@ isSubmitting={isSubmitting} + isDirty={isDirty} onSubmit={handleSubmit(onSubmit)} >Also applies to: 97-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/automations/components/automation-create-dialog.tsx` around lines 48 - 55, The form removed the dirty-state gate: include isDirty in the useForm destructure (const { register, handleSubmit, setError, formState: { isSubmitting, isDirty, errors } } = useForm<FormData>({ resolver: zodResolver(formSchema) })) and wire that isDirty into the dialog’s submit gating by passing it to the FormDialog (or the component prop that previously controlled submit gating) so the submit button remains disabled until isDirty === true while still respecting isSubmitting and handleSubmit; update any logic that previously used the old gate (lines around FormDialog render) to use isDirty.services/platform/app/features/automations/triggers/components/schedule-create-dialog.tsx (1)
97-103:⚠️ Potential issue | 🟠 MajorRestore dirty-state submit gating in this dialog.
This migration drops the gating prop entirely. For edit mode, submit becomes available even when nothing changed, which conflicts with the PR’s intended
isDirty-based behavior.Proposed fix
const { handleSubmit, register, reset, - formState: { errors: formErrors }, + formState: { errors: formErrors, isDirty }, setValue, } = form; @@ <FormDialog open={open} onOpenChange={onOpenChange} @@ isSubmitting={isSubmitting} + isDirty={isDirty} onSubmit={handleSubmit(onSubmit)} >Also applies to: 169-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/automations/triggers/components/schedule-create-dialog.tsx` around lines 97 - 103, Restore the dirty-state gating so edits can only be submitted when something changed: read isDirty from the form (e.g., include formState: { errors: formErrors, isDirty } in the destructure) and use it to disable the submit action in the ScheduleCreateDialog edit flow; update the submit handler (the function passed to handleSubmit) to short-circuit (no-op) when in edit mode and !isDirty, and ensure the submit button/UX reflects this disabled state. Reference the existing symbols handleSubmit, formState/isDirty, and setValue/register/reset when reintroducing this gating.services/platform/app/components/ui/dialog/form-dialog.stories.tsx (1)
154-156:⚠️ Potential issue | 🟡 MinorStory copy still describes validity-based behavior
The
InvalidFormstory now usesisDirty={false}(Line 161), but the UI text still says “invalid form” and “form is invalid” (Lines 154-156, 173). Please update wording to dirty-state semantics.Suggested fix
- <Button onClick={() => setOpen(true)}> - Open dialog (invalid form) - </Button> + <Button onClick={() => setOpen(true)}> + Open dialog (pristine form) + </Button> @@ - 'Submit button is disabled when the form is invalid (e.g. required fields not filled).', + 'Submit button is disabled until the form is modified (dirty state).',Also applies to: 161-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/components/ui/dialog/form-dialog.stories.tsx` around lines 154 - 156, The story copy in the InvalidForm story is still about validity but the story now sets isDirty={false}; update all UI text in that story (the Button label that calls setOpen(true) and the message shown inside the dialog) to reflect dirty-state semantics (e.g., "Open dialog (clean form)" or "Open dialog (not dirty)" and "form is not dirty" / "form is clean") instead of "invalid form" / "form is invalid"; locate the text within the InvalidForm story that uses setOpen and the isDirty prop and replace the wording accordingly.services/platform/app/features/automations/triggers/components/event-create-dialog.tsx (1)
62-67:⚠️ Potential issue | 🟠 MajorEdit mode is marked dirty even without user changes
Line 190 (
isDirty={!!selectedEventType}) is true immediately in edit mode becauseselectedEventTypeis preloaded (Lines 64-67). This enables submit before any modification.Suggested fix
+ const isDirty = useMemo(() => { + if (!isEditMode) return !!selectedEventType; + const initialFilters = editing?.eventFilter ?? {}; + const sameFilters = + JSON.stringify(filterValues) === JSON.stringify(initialFilters); + return !sameFilters; + }, [isEditMode, selectedEventType, filterValues, editing?.eventFilter]); @@ - isDirty={!!selectedEventType} + isDirty={isDirty}Also applies to: 190-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/automations/triggers/components/event-create-dialog.tsx` around lines 62 - 67, In edit mode the dialog's isDirty is set to !!selectedEventType which is true immediately because useEffect preloads selectedEventType; fix by capturing initial values when opening for edit (e.g., store initialEventType and initialFilterValues inside the same useEffect that calls setSelectedEventType/setFilterValues) and compute isDirty by comparing current selectedEventType and filterValues to those initial values (use deep equality or JSON.stringify) instead of just !!selectedEventType; update the prop passed to the dialog (isDirty) to use that comparison and leave creation mode behavior (where isDirty can remain !!selectedEventType) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@services/platform/app/features/automations/components/step-create-dialog.tsx`:
- Around line 126-133: The form now marks fields dirty via setValue(..., {
shouldDirty: true }) but the dialog isn't receiving the form's dirty state; pass
the form's current isDirty from useForm into the FormDialog so dirty gating
works. In the component where useForm<FormData> is called (the destructured
register, handleSubmit, formState: { errors, isSubmitting }, reset, setValue,
watch), extract isDirty from formState and forward it as a prop (e.g.,
isDirty={isDirty}) to the FormDialog instance (same places affected around the
126–133 and 217–227 regions) so the dialog can enable/disable close/save based
on the form dirty state.
In
`@services/platform/app/features/custom-agents/components/custom-agent-create-dialog.tsx`:
- Around line 66-69: The formState destructuring for the form is missing
isDirty; update the destructuring where register, handleSubmit, formState: {
isSubmitting, errors } are extracted to also include isDirty, and then pass that
isDirty boolean into the FormDialog props (the FormDialog invocation that
currently receives isSubmitting/errors) so the dialog can gate the submit button
using isDirty; look for the register/handleSubmit/formState block and the
FormDialog component usage to make the two edits.
In
`@services/platform/app/features/customers/components/customer-edit-dialog.test.tsx`:
- Around line 59-88: Update the "submits when customer has no name" test in
CustomerEditDialog to assert the Save button is initially disabled (use the
submitButton variable via getByRole) and then becomes enabled after the field
change (after typing into emailInput with user.type); keep existing assertions
for mutate call and onOpenChange, ensuring you reference submitButton,
emailInput, user, mockMutateAsync and onOpenChange so the test verifies the
dirty-state gating behavior.
- Around line 25-35: The fixture in makeCustomer uses a blanket `as never` which
disables type checking; change the `_id` assertion to `as Id<'customers'>` and
add `satisfies Doc<'customers'>` to the returned object so the entire fixture is
type-checked — update the makeCustomer function to return an object with `_id:
'customer-1' as Id<'customers'>` and append `satisfies Doc<'customers'>` after
the object literal (keeping the existing overrides spread).
In
`@services/platform/app/features/customers/components/customer-edit-dialog.tsx`:
- Line 57: The UI currently converts an emptied name to undefined (name:
trimmedName || undefined) which the server-side updater (update_customer.ts)
strips out, causing silent no-op updates; pick one behavior—here enforce
non-empty names: change the Zod schema in customer-edit-dialog.tsx from name:
z.string().trim() to name: z.string().trim().min(1, "Name is required"), stop
converting empty strings to undefined (pass the actual trimmedName value instead
of trimmedName || undefined), add client-side validation/error display to block
submission when trimmedName is empty, and add matching validation on the server
update_customer.ts to reject empty names so the contract is consistent.
In
`@services/platform/app/features/customers/components/customers-import-dialog.tsx`:
- Around line 103-106: The form no longer exposes isValid, so grab isDirty from
the formMethods' formState (e.g., const { handleSubmit, formState: {
isSubmitting, isDirty } } = formMethods) and pass that isDirty value into the
dialog's dirty gating prop where isValid used to be passed (the Dialog/Modal
component instantiation around the current submit/close handlers). Ensure the
dialog receives isDirty so the existing dirty-based submit gating behavior
(previously tied to isValid) is restored.
In
`@services/platform/app/features/products/components/products-import-dialog.tsx`:
- Around line 64-67: The form's dirty-state gating was removed: re-add isDirty
to the form state destructuring from formMethods (alongside handleSubmit and
isSubmitting) and pass that isDirty value into the dialog's props where the
dialog is rendered (the same place currently passing submit handlers around
lines referenced), so the dialog receives and uses isDirty to enforce
interaction-based submit gating; update any submit-button or dialog gating logic
to check formState.isDirty instead of relying solely on isSubmitting.
In `@services/platform/app/features/vendors/components/vendors-import-dialog.tsx`:
- Around line 100-103: The component removed isValid but never forwards isDirty
to the dialog, breaking dirty-gated submit; update the destructure from
formMethods to include isDirty (and isValid if submit gating needs validation)
from formState, then pass isDirty (and isValid if applicable) into the
FormDialog props where handleSubmit and isSubmitting are passed (refer to
formMethods, formState, isDirty, isValid, handleSubmit, isSubmitting and the
FormDialog render block) so the dialog can correctly enable/disable submission
based on dirty state.
In `@services/platform/app/features/websites/components/website-add-dialog.tsx`:
- Around line 59-67: The form's dirty tracking is set (setValue(..., {
shouldDirty: true })) but FormDialog isn't receiving the isDirty flag; update
the useForm destructuring to include isDirty (e.g., const { register,
handleSubmit, formState: { errors, isDirty }, reset, setValue, watch } =
useForm(...)) and pass that isDirty down to the FormDialog component (where
FormDialog is rendered) so submit gating works; apply the same change at the
other instance noted (the block around the second occurrence).
---
Outside diff comments:
In `@services/platform/app/components/ui/dialog/form-dialog.stories.tsx`:
- Around line 154-156: The story copy in the InvalidForm story is still about
validity but the story now sets isDirty={false}; update all UI text in that
story (the Button label that calls setOpen(true) and the message shown inside
the dialog) to reflect dirty-state semantics (e.g., "Open dialog (clean form)"
or "Open dialog (not dirty)" and "form is not dirty" / "form is clean") instead
of "invalid form" / "form is invalid"; locate the text within the InvalidForm
story that uses setOpen and the isDirty prop and replace the wording
accordingly.
In
`@services/platform/app/features/automations/components/automation-create-dialog.tsx`:
- Around line 48-55: The form removed the dirty-state gate: include isDirty in
the useForm destructure (const { register, handleSubmit, setError, formState: {
isSubmitting, isDirty, errors } } = useForm<FormData>({ resolver:
zodResolver(formSchema) })) and wire that isDirty into the dialog’s submit
gating by passing it to the FormDialog (or the component prop that previously
controlled submit gating) so the submit button remains disabled until isDirty
=== true while still respecting isSubmitting and handleSubmit; update any logic
that previously used the old gate (lines around FormDialog render) to use
isDirty.
In
`@services/platform/app/features/automations/triggers/components/event-create-dialog.tsx`:
- Around line 62-67: In edit mode the dialog's isDirty is set to
!!selectedEventType which is true immediately because useEffect preloads
selectedEventType; fix by capturing initial values when opening for edit (e.g.,
store initialEventType and initialFilterValues inside the same useEffect that
calls setSelectedEventType/setFilterValues) and compute isDirty by comparing
current selectedEventType and filterValues to those initial values (use deep
equality or JSON.stringify) instead of just !!selectedEventType; update the prop
passed to the dialog (isDirty) to use that comparison and leave creation mode
behavior (where isDirty can remain !!selectedEventType) unchanged.
In
`@services/platform/app/features/automations/triggers/components/schedule-create-dialog.tsx`:
- Around line 97-103: Restore the dirty-state gating so edits can only be
submitted when something changed: read isDirty from the form (e.g., include
formState: { errors: formErrors, isDirty } in the destructure) and use it to
disable the submit action in the ScheduleCreateDialog edit flow; update the
submit handler (the function passed to handleSubmit) to short-circuit (no-op)
when in edit mode and !isDirty, and ensure the submit button/UX reflects this
disabled state. Reference the existing symbols handleSubmit, formState/isDirty,
and setValue/register/reset when reintroducing this gating.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 04e67fdd-f435-4508-88c8-f1df53003991
📒 Files selected for processing (27)
services/platform/app/components/ui/dialog/form-dialog.stories.tsxservices/platform/app/components/ui/dialog/form-dialog.tsxservices/platform/app/features/automations/components/automation-create-dialog.tsxservices/platform/app/features/automations/components/automation-rename-dialog.tsxservices/platform/app/features/automations/components/step-create-dialog.tsxservices/platform/app/features/automations/triggers/components/event-create-dialog.tsxservices/platform/app/features/automations/triggers/components/schedule-create-dialog.tsxservices/platform/app/features/custom-agents/components/custom-agent-create-dialog.tsxservices/platform/app/features/customers/components/customer-edit-dialog.test.tsxservices/platform/app/features/customers/components/customer-edit-dialog.tsxservices/platform/app/features/customers/components/customers-import-dialog.tsxservices/platform/app/features/documents/components/document-upload-dialog.tsxservices/platform/app/features/products/components/product-edit-dialog.tsxservices/platform/app/features/products/components/products-import-dialog.tsxservices/platform/app/features/settings/api-keys/components/api-key-create-dialog.tsxservices/platform/app/features/settings/branding/components/branding-form.tsxservices/platform/app/features/settings/integrations/components/circuly-integration-dialog.tsxservices/platform/app/features/settings/integrations/components/protel-integration-dialog.tsxservices/platform/app/features/settings/integrations/components/shopify-integration-dialog.tsxservices/platform/app/features/settings/organization/components/member-add-dialog.tsxservices/platform/app/features/settings/organization/components/member-edit-dialog.tsxservices/platform/app/features/settings/teams/components/team-create-dialog.tsxservices/platform/app/features/settings/teams/components/team-edit-dialog.tsxservices/platform/app/features/vendors/components/vendor-edit-dialog.tsxservices/platform/app/features/vendors/components/vendors-import-dialog.tsxservices/platform/app/features/websites/components/website-add-dialog.tsxservices/platform/app/features/websites/components/website-edit-dialog.tsx
💤 Files with no reviewable changes (1)
- services/platform/app/features/settings/teams/components/team-create-dialog.tsx
Comment on lines
126
to
133
| const { | ||
| register, | ||
| handleSubmit, | ||
| formState: { errors, isSubmitting, isValid }, | ||
| formState: { errors, isSubmitting }, | ||
| reset, | ||
| setValue, | ||
| watch, | ||
| } = useForm<FormData>({ |
Contributor
There was a problem hiding this comment.
Dirty propagation is implemented, but dirty gating is not connected.
setValue(..., { shouldDirty: true }) was added in multiple places, but FormDialog still does not receive isDirty, so the migration is only partial.
Proposed fix
const {
register,
handleSubmit,
- formState: { errors, isSubmitting },
+ formState: { errors, isSubmitting, isDirty },
reset,
setValue,
watch,
} = useForm<FormData>({
@@
<FormDialog
open={open}
onOpenChange={handleClose}
@@
isSubmitting={isSubmitting}
+ isDirty={isDirty}
onSubmit={handleSubmit(onSubmit)}
large
>Also applies to: 217-227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/automations/components/step-create-dialog.tsx`
around lines 126 - 133, The form now marks fields dirty via setValue(..., {
shouldDirty: true }) but the dialog isn't receiving the form's dirty state; pass
the form's current isDirty from useForm into the FormDialog so dirty gating
works. In the component where useForm<FormData> is called (the destructured
register, handleSubmit, formState: { errors, isSubmitting }, reset, setValue,
watch), extract isDirty from formState and forward it as a prop (e.g.,
isDirty={isDirty}) to the FormDialog instance (same places affected around the
126–133 and 217–227 regions) so the dialog can enable/disable close/save based
on the form dirty state.
Comment on lines
66
to
+69
| const { | ||
| register, | ||
| handleSubmit, | ||
| formState: { isSubmitting, isValid, errors }, | ||
| formState: { isSubmitting, errors }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check if file exists and read lines 60-120 =="
if [ -f "services/platform/app/features/custom-agents/components/custom-agent-create-dialog.tsx" ]; then
echo "File found. Reading relevant lines:"
cat -n "services/platform/app/features/custom-agents/components/custom-agent-create-dialog.tsx" | sed -n '60,120p'
else
echo "File not found at that path. Searching for it..."
fd -i "custom-agent-create-dialog" --type f
fiRepository: tale-project/tale
Length of output: 2114
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate FormDialog component =="
fd -i "form-dialog" --type f -e ts -e tsx | head -20Repository: tale-project/tale
Length of output: 220
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for FormDialog definition in components/ui =="
find services/platform/app/components -name "*form-dialog*" -o -name "*dialog*" 2>/dev/null | grep -i form | head -10Repository: tale-project/tale
Length of output: 744
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Read FormDialog component implementation =="
cat -n "services/platform/app/components/ui/dialog/form-dialog.tsx"Repository: tale-project/tale
Length of output: 5692
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for other dialog usages with isDirty in the features directory =="
rg -n "isDirty" services/platform/app/features --type tsx -B2 -A2 | head -50Repository: tale-project/tale
Length of output: 181
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check PR context: search for isDirty introduction or dirty-state gating changes =="
rg -n "isDirty" services/platform/app/components/ui/dialog -B3 -A3Repository: tale-project/tale
Length of output: 4013
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for FormDialog usage patterns with isDirty =="
rg -n "FormDialog" services/platform/app/features -A10 | grep -E "FormDialog|isDirty|formState" | head -40Repository: tale-project/tale
Length of output: 4705
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check if there are other uses of custom-agent-create-dialog or similar dialogs =="
rg -n "formState.*isDirty" services/platform/app/features --type ts -B2 -A2Repository: tale-project/tale
Length of output: 3307
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check team-create-dialog (similar CREATE pattern) =="
head -150 services/platform/app/features/settings/teams/components/team-create-dialog.tsx | tail -60Repository: tale-project/tale
Length of output: 1468
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check if team-create-dialog extracts isDirty or passes it =="
rg -n "formState|isDirty|FormDialog" services/platform/app/features/settings/teams/components/team-create-dialog.tsx -B2 -A2Repository: tale-project/tale
Length of output: 746
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Verify the actual behavior: check if other CREATE dialogs also miss isDirty =="
grep -l "components.*create.*dialog" services/platform/app/features/**/components/*.tsx 2>/dev/null | while read f; do
echo "--- $f ---"
rg -n "isDirty" "$f" || echo " (no isDirty found)"
doneRepository: tale-project/tale
Length of output: 237
Add isDirty to form state destructuring and pass it to FormDialog.
isDirty is missing from the formState destructuring and not passed to FormDialog. This allows the submit button to remain enabled even when the form is untouched, contradicting the intended submit gating migration from isValid to isDirty. The issue also appears in team-create-dialog and differs from all edit dialogs in the codebase that properly pass isDirty.
Extract isDirty from formState at line 69 and pass it to FormDialog at line 115.
Suggested patch
const {
register,
handleSubmit,
- formState: { isSubmitting, errors },
+ formState: { isSubmitting, isDirty, errors },
} = useForm<FormData>({
resolver: zodResolver(formSchema),
defaultValues: {
name: '',
displayName: '',
description: '',
},
});
@@
<FormDialog
open={open}
onOpenChange={onOpenChange}
title={t('customAgents.createAgent')}
submitText={t('customAgents.createDialog.continue')}
submittingText={t('customAgents.createDialog.creating')}
isSubmitting={isSubmitting}
+ isDirty={isDirty}
onSubmit={handleSubmit(onSubmit)}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/custom-agents/components/custom-agent-create-dialog.tsx`
around lines 66 - 69, The formState destructuring for the form is missing
isDirty; update the destructuring where register, handleSubmit, formState: {
isSubmitting, errors } are extracted to also include isDirty, and then pass that
isDirty boolean into the FormDialog props (the FormDialog invocation that
currently receives isSubmitting/errors) so the dialog can gate the submit button
using isDirty; look for the register/handleSubmit/formState block and the
FormDialog component usage to make the two edits.
Comment on lines
+25
to
+35
| function makeCustomer(overrides = {}) { | ||
| return { | ||
| _id: 'customer-1' as never, | ||
| _creationTime: Date.now(), | ||
| organizationId: 'org-1', | ||
| email: 'test@example.com', | ||
| status: 'active' as const, | ||
| source: 'manual_import' as const, | ||
| locale: 'en', | ||
| ...overrides, | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "customer-edit-dialog.test.tsx" services/platform/app/features/customers/components/Repository: tale-project/tale
Length of output: 142
🏁 Script executed:
# Check if file exists and read the relevant section
if [ -f "services/platform/app/features/customers/components/customer-edit-dialog.test.tsx" ]; then
echo "=== File found. Reading lines 20-40 ===";
sed -n '20,40p' "services/platform/app/features/customers/components/customer-edit-dialog.test.tsx";
else
echo "File not found at specified path";
fiRepository: tale-project/tale
Length of output: 544
🏁 Script executed:
# Search for Customer type definition and imports in the codebase
rg "type Customer|interface Customer" services/platform/app/ -A 3 | head -50Repository: tale-project/tale
Length of output: 4234
🏁 Script executed:
# Check what the actual _id type should be in customer definitions
rg "_id.*:" services/platform/convex -t ts -A 1 | grep -A1 "customer" | head -30Repository: tale-project/tale
Length of output: 2997
🏁 Script executed:
# Search for similar fixture/factory functions in test files
rg "function make\w+\(" services/platform/app --type ts -A 5 | head -60Repository: tale-project/tale
Length of output: 3894
🏁 Script executed:
# Check if there are any test files using satisfies for fixtures
rg "satisfies.*Doc<" services/platform/app --type ts -B 2 -A 2 | head -40Repository: tale-project/tale
Length of output: 43
Replace as never with an explicit type cast and add satisfies for fixture validation.
The as never cast completely suppresses type checking. Instead, use as Id<'customers'> for the _id field (necessary for test mocks) and add satisfies Doc<'customers'> on the returned object to ensure the entire fixture shape is validated at compile time.
♻️ Type-safe fixture
function makeCustomer(overrides = {}) {
return {
- _id: 'customer-1' as never,
+ _id: 'customer-1' as Id<'customers'>,
_creationTime: Date.now(),
organizationId: 'org-1',
email: 'test@example.com',
status: 'active' as const,
source: 'manual_import' as const,
locale: 'en',
...overrides,
- };
+ } satisfies Doc<'customers'>;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/customers/components/customer-edit-dialog.test.tsx`
around lines 25 - 35, The fixture in makeCustomer uses a blanket `as never`
which disables type checking; change the `_id` assertion to `as Id<'customers'>`
and add `satisfies Doc<'customers'>` to the returned object so the entire
fixture is type-checked — update the makeCustomer function to return an object
with `_id: 'customer-1' as Id<'customers'>` and append `satisfies
Doc<'customers'>` after the object literal (keeping the existing overrides
spread).
Comment on lines
+59
to
+88
| it('submits when customer has no name', async () => { | ||
| const onOpenChange = vi.fn(); | ||
| const { user } = render( | ||
| <CustomerEditDialog | ||
| customer={makeCustomer({ name: undefined })} | ||
| isOpen={true} | ||
| onOpenChange={onOpenChange} | ||
| />, | ||
| ); | ||
|
|
||
| const emailInput = screen.getByDisplayValue('test@example.com'); | ||
| await user.clear(emailInput); | ||
| await user.type(emailInput, 'new@example.com'); | ||
|
|
||
| const submitButton = screen.getByRole('button', { name: /save/i }); | ||
| await user.click(submitButton); | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockMutateAsync).toHaveBeenCalledWith({ | ||
| customerId: 'customer-1', | ||
| name: undefined, | ||
| email: 'new@example.com', | ||
| locale: 'en', | ||
| }); | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(onOpenChange).toHaveBeenCalledWith(false); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add an explicit regression test for dirty-state submit gating.
Given the PR objective, add one assertion that the Save button starts disabled and only enables after a field change.
Also applies to: 90-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/customers/components/customer-edit-dialog.test.tsx`
around lines 59 - 88, Update the "submits when customer has no name" test in
CustomerEditDialog to assert the Save button is initially disabled (use the
submitButton variable via getByRole) and then becomes enabled after the field
change (after typing into emailInput with user.type); keep existing assertions
for mutate call and onOpenChange, ensuring you reference submitButton,
emailInput, user, mockMutateAsync and onOpenChange so the test verifies the
dirty-state gating behavior.
| 1, | ||
| tCommon('validation.required', { field: tCustomers('name') }), | ||
| ), | ||
| name: z.string().trim(), |
Contributor
There was a problem hiding this comment.
Empty-name edits currently become silent no-ops
With name: z.string().trim() and name: trimmedName || undefined, clearing the name submits undefined. In services/platform/convex/customers/update_customer.ts:28-98, undefined fields are removed before patching, so the existing name is not cleared. This can show a success toast while keeping old data.
Please choose one explicit behavior and enforce it in both UI + mutation contract:
- either disallow empty name again, or
- support explicit clearing (e.g.,
null/'') and handle it server-side.
Suggested fix (disallow empty names)
@@
- name: z.string().trim(),
+ name: z
+ .string()
+ .trim()
+ .min(1, tCommon('validation.required', { field: tCustomers('name') })),
@@
- const trimmedName = data.name.trim();
await updateCustomer.mutateAsync({
customerId: customer._id,
- name: trimmedName || undefined,
+ name: data.name.trim(),
email: data.email.trim(),
locale: data.locale,
});Also applies to: 97-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/customers/components/customer-edit-dialog.tsx`
at line 57, The UI currently converts an emptied name to undefined (name:
trimmedName || undefined) which the server-side updater (update_customer.ts)
strips out, causing silent no-op updates; pick one behavior—here enforce
non-empty names: change the Zod schema in customer-edit-dialog.tsx from name:
z.string().trim() to name: z.string().trim().min(1, "Name is required"), stop
converting empty strings to undefined (pass the actual trimmedName value instead
of trimmedName || undefined), add client-side validation/error display to block
submission when trimmedName is empty, and add matching validation on the server
update_customer.ts to reject empty names so the contract is consistent.
Comment on lines
103
to
106
| const { | ||
| handleSubmit, | ||
| formState: { isSubmitting, isValid }, | ||
| formState: { isSubmitting }, | ||
| } = formMethods; |
Contributor
There was a problem hiding this comment.
Dirty-state gating is missing after isValid removal
After Line 105 drops isValid, the dialog still isn’t passed isDirty (Lines 213-221). That leaves this form outside the PR’s dirty-based submit gating behavior.
Suggested fix
const {
handleSubmit,
- formState: { isSubmitting },
+ formState: { isSubmitting, isDirty },
} = formMethods;
@@
submittingText={tCommon('actions.importing')}
isSubmitting={isSubmitting}
+ isDirty={isDirty}
onSubmit={handleSubmit(onSubmit)}Also applies to: 213-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/customers/components/customers-import-dialog.tsx`
around lines 103 - 106, The form no longer exposes isValid, so grab isDirty from
the formMethods' formState (e.g., const { handleSubmit, formState: {
isSubmitting, isDirty } } = formMethods) and pass that isDirty value into the
dialog's dirty gating prop where isValid used to be passed (the Dialog/Modal
component instantiation around the current submit/close handlers). Ensure the
dialog receives isDirty so the existing dirty-based submit gating behavior
(previously tied to isValid) is restored.
Comment on lines
64
to
67
| const { | ||
| handleSubmit, | ||
| formState: { isSubmitting, isValid }, | ||
| formState: { isSubmitting }, | ||
| } = formMethods; |
Contributor
There was a problem hiding this comment.
Dirty-state submit gating is not wired in this dialog
After removing isValid (Line 66), the dialog is no longer passed isDirty (Lines 227-235). That bypasses the new interaction-based gating model for this form.
Suggested fix
const {
handleSubmit,
- formState: { isSubmitting },
+ formState: { isSubmitting, isDirty },
} = formMethods;
@@
<FormDialog
open={isOpen}
onOpenChange={handleClose}
@@
isSubmitting={isSubmitting}
+ isDirty={isDirty}
onSubmit={handleSubmit(onSubmit)}
>Also applies to: 227-235
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/products/components/products-import-dialog.tsx`
around lines 64 - 67, The form's dirty-state gating was removed: re-add isDirty
to the form state destructuring from formMethods (alongside handleSubmit and
isSubmitting) and pass that isDirty value into the dialog's props where the
dialog is rendered (the same place currently passing submit handlers around
lines referenced), so the dialog receives and uses isDirty to enforce
interaction-based submit gating; update any submit-button or dialog gating logic
to check formState.isDirty instead of relying solely on isSubmitting.
Comment on lines
+51
to
52
| formState: { isSubmitting, errors }, | ||
| reset, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get file size and content
FILE="services/platform/app/features/settings/integrations/components/protel-integration-dialog.tsx"
echo "=== File size ==="
wc -l "$FILE"
echo ""
echo "=== Lines 40-160 (covers 51-52 and 127-129) ==="
sed -n '40,160p' "$FILE"Repository: tale-project/tale
Length of output: 3799
🏁 Script executed:
#!/bin/bash
# Verify schema definition and see the rest of the form
FILE="services/platform/app/features/settings/integrations/components/protel-integration-dialog.tsx"
echo "=== Lines 150-217 (rest of form, including inputs) ==="
sed -n '150,217p' "$FILE"
echo ""
echo "=== Search for protelSchema definition ==="
rg -n "protelSchema|z\.object" "$FILE" -A 10Repository: tale-project/tale
Length of output: 3290
🏁 Script executed:
#!/bin/bash
# Check if there are other integration dialogs that handle connected-mode updates
echo "=== Other integration dialogs ==="
fd "integration.*dialog.tsx" services/platform/app/features/settings/integrations/components/
echo ""
echo "=== Check similar integration patterns (e.g., basic-auth) ==="
rg -l "customFooter.*isConnected" services/platform/app/features/settings/integrations/components/ -t tsx
echo ""
echo "=== Verify form submission behavior when validation fails ==="
echo "Looking for handleSubmit pattern in ProtelIntegrationDialog..."
rg -n "handleSubmit|onSubmit" services/platform/app/features/settings/integrations/components/protel-integration-dialog.tsx -B 2 -A 2Repository: tale-project/tale
Length of output: 1198
🏁 Script executed:
#!/bin/bash
# Check how other integration dialogs handle connected-mode updates
echo "=== integration-manage-dialog.tsx pattern (referenced in learnings) ==="
grep -n "isConnected\|customFooter" services/platform/app/features/settings/integrations/components/integration-manage-dialog.tsx | head -20
echo ""
echo "=== circuly-integration-dialog.tsx pattern ==="
grep -n "isConnected\|customFooter" services/platform/app/features/settings/integrations/components/circuly-integration-dialog.tsx | head -20
echo ""
echo "=== shopify-integration-dialog.tsx pattern ==="
grep -n "isConnected\|customFooter" services/platform/app/features/settings/integrations/components/shopify-integration-dialog.tsx | head -20Repository: tale-project/tale
Length of output: 732
🏁 Script executed:
#!/bin/bash
# Compare how connected-mode footers are implemented in other dialogs
echo "=== circuly-integration-dialog.tsx footer (lines 152-175) ==="
sed -n '152,175p' services/platform/app/features/settings/integrations/components/circuly-integration-dialog.tsx
echo ""
echo "=== shopify-integration-dialog.tsx footer (lines 133-160) ==="
sed -n '133,160p' services/platform/app/features/settings/integrations/components/shopify-integration-dialog.tsx
echo ""
echo "=== protel-integration-dialog.tsx footer (lines 116-135) ==="
sed -n '116,135p' services/platform/app/features/settings/integrations/components/protel-integration-dialog.tsxRepository: tale-project/tale
Length of output: 2219
🏁 Script executed:
#!/bin/bash
# Check shopify's formHandleSubmit - is it different from protel's handleSubmit?
echo "=== shopify-integration-dialog.tsx form setup and formHandleSubmit ==="
sed -n '1,100p' services/platform/app/features/settings/integrations/components/shopify-integration-dialog.tsx | grep -A 5 -B 5 "formHandleSubmit\|handleSubmit\|useForm"
echo ""
echo "=== Check if shopify has similar hidden password field in connected mode ==="
sed -n '180,220p' services/platform/app/features/settings/integrations/components/shopify-integration-dialog.tsxRepository: tale-project/tale
Length of output: 147
🏁 Script executed:
#!/bin/bash
# Simpler verification: check the core issue - password validation in connected mode
echo "=== Verify password field default and validation ==="
sed -n '15,21p' services/platform/app/features/settings/integrations/components/protel-integration-dialog.tsx
echo ""
echo "=== Verify connected-mode form rendering (password hidden) ==="
sed -n '166,217p' services/platform/app/features/settings/integrations/components/protel-integration-dialog.tsxRepository: tale-project/tale
Length of output: 2087
Remove the non-functional Update button from connected-state footer.
When connected, all form inputs are hidden but the Update button remains enabled (only gated by isSubmitting). The password field defaults to an empty string, which violates the schema's .min(1) requirement. Submitting silently fails validation with no user feedback.
The credential form is only shown when disconnected; connected mode should not expose an update CTA that cannot succeed without hidden required fields. Either remove the Update button (as in Circuly) or redesign to allow optional password updates via backend changes.
Suggested fix
const customFooter =
isConnected && onDisconnect ? (
- <>
- <Button
- variant="destructive"
- onClick={handleDisconnect}
- disabled={isSubmitting}
- className="flex-1"
- >
- {isSubmitting
- ? t('integrations.protel.disconnecting')
- : t('integrations.protel.disconnect')}
- </Button>
- <Button
- type="submit"
- disabled={isSubmitting}
- className="flex-1"
- >
- {isSubmitting
- ? t('integrations.protel.updating')
- : t('integrations.protel.update')}
- </Button>
- </>
+ <Button
+ variant="destructive"
+ onClick={handleDisconnect}
+ disabled={isSubmitting}
+ className="flex-1"
+ >
+ {isSubmitting
+ ? t('integrations.protel.disconnecting')
+ : t('integrations.protel.disconnect')}
+ </Button>
) : undefined;
Comment on lines
100
to
103
| const { | ||
| handleSubmit, | ||
| formState: { isSubmitting, isValid }, | ||
| formState: { isSubmitting }, | ||
| } = formMethods; |
Contributor
There was a problem hiding this comment.
Submit gating is no longer tied to dirty state
Line 102 removes isValid, but isDirty is not forwarded to FormDialog (Lines 198-206). This breaks consistency with the dirty-gated submit behavior introduced by this PR.
Suggested fix
const {
handleSubmit,
- formState: { isSubmitting },
+ formState: { isSubmitting, isDirty },
} = formMethods;
@@
submittingText={tCommon('actions.importing')}
isSubmitting={isSubmitting}
+ isDirty={isDirty}
onSubmit={handleSubmit(onSubmit)}Also applies to: 198-206
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/vendors/components/vendors-import-dialog.tsx`
around lines 100 - 103, The component removed isValid but never forwards isDirty
to the dialog, breaking dirty-gated submit; update the destructure from
formMethods to include isDirty (and isValid if submit gating needs validation)
from formState, then pass isDirty (and isValid if applicable) into the
FormDialog props where handleSubmit and isSubmitting are passed (refer to
formMethods, formState, isDirty, isValid, handleSubmit, isSubmitting and the
FormDialog render block) so the dialog can correctly enable/disable submission
based on dirty state.
Comment on lines
59
to
67
| const { | ||
| register, | ||
| handleSubmit, | ||
| formState: { errors, isValid }, | ||
| formState: { errors }, | ||
| reset, | ||
| setValue, | ||
| watch, | ||
| } = useForm<FormData>({ | ||
| resolver: zodResolver(formSchema), |
Contributor
There was a problem hiding this comment.
Dirty tracking is set up but not connected to FormDialog.
setValue(..., { shouldDirty: true }) is in place, but FormDialog is no longer given isDirty, so submit is not gated by user interaction in this dialog.
Proposed fix
const {
register,
handleSubmit,
- formState: { errors },
+ formState: { errors, isDirty },
reset,
setValue,
watch,
} = useForm<FormData>({
@@
<FormDialog
open={isOpen}
onOpenChange={(open) => !open && handleClose()}
title={tWebsites('addWebsite')}
submittingText={tWebsites('adding')}
isSubmitting={isLoading}
+ isDirty={isDirty}
onSubmit={handleSubmit(onSubmit)}
>Also applies to: 109-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/websites/components/website-add-dialog.tsx`
around lines 59 - 67, The form's dirty tracking is set (setValue(..., {
shouldDirty: true })) but FormDialog isn't receiving the isDirty flag; update
the useForm destructuring to include isDirty (e.g., const { register,
handleSubmit, formState: { errors, isDirty }, reset, setValue, watch } =
useForm(...)) and pass that isDirty down to the FormDialog component (where
FormDialog is rendered) so submit gating works; apply the same change at the
other instance noted (the block around the second occurrence).
4 tasks
yannickmonney
pushed a commit
that referenced
this pull request
Apr 8, 2026
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
isValidprop withisDirtyonFormDialogto gate form submission on user interaction instead of async validation state, which caused dialogs to be unsubmittablemode: 'onChange'from alluseFormcalls (unnecessary withzodResolverand caused premature validation)shouldDirty: trueto programmaticsetValuecalls so dirty state propagates correctlyTest plan
customer-edit-dialog.test.tsxtestsSummary by CodeRabbit
Refactor
Tests