Skip to content

Commit 6fda71a

Browse files
authored
fix(ui): save button becomes disabled after failed save with draft validation (#14584)
### What? Fixed an issue where the Save Draft button becomes disabled after a failed save operation when draft validation is enabled, preventing users from retrying without making additional form changes. ### Why? When `versions.drafts.validate` is enabled, if a draft save fails (due to validation errors, network issues, or server errors), the Save Draft button would become disabled. This is problematic because: 1. Users cannot retry the save without making arbitrary changes to the form 2. The behavior is inconsistent with unvalidated drafts, which always allow saving 3. Transient failures (network errors, temporary third-party service issues) become permanent blocks The root cause: `setModified(false)` is called early in the submit flow for ALL saves (line 365), which marks the form as "unmodified". When a draft save subsequently fails, the form remains marked as unmodified, which triggers the disabled state for the Save Draft button on update operations (since `disabled = operation === 'update' && !modified`). ### How - Added targeted logic in the error handler that explicitly restores the modified state for failed draft saves - When `overridesFromArgs['_status'] === 'draft'` and the save fails, call `setModified(true)` to mark the form as modified again - This ensures the Save Draft button remains enabled for retry on update operations - Also added `validateDrafts` to the submit callback's dependency array to prevent stale closure issues Fixes #14227
1 parent 565680d commit 6fda71a

File tree

5 files changed

+179
-3
lines changed

5 files changed

+179
-3
lines changed

packages/ui/src/forms/Form/index.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,8 +411,12 @@ export const Form: React.FC<FormProps> = (props) => {
411411

412412
// When there was an error submitting a draft,
413413
// set the form state to unsubmitted, to not trigger visible form validation on changes after the failed submit.
414-
if (!validateDrafts && overridesFromArgs['_status'] === 'draft') {
415-
setSubmitted(false)
414+
// Also keep the form as modified so the save button remains enabled for retry.
415+
if (overridesFromArgs['_status'] === 'draft') {
416+
setModified(true)
417+
if (!validateDrafts) {
418+
setSubmitted(false)
419+
}
416420
}
417421

418422
contextRef.current = { ...contextRef.current } // triggers rerender of all components that subscribe to form
@@ -494,6 +498,7 @@ export const Form: React.FC<FormProps> = (props) => {
494498
router,
495499
t,
496500
i18n,
501+
validateDrafts,
497502
waitForAutocomplete,
498503
],
499504
)
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import type { CollectionConfig } from 'payload'
2+
3+
export const draftValidationSlug = 'draft-validation'
4+
5+
/**
6+
* This collection tests the behavior of draft validation and save button state.
7+
* Specifically, it tests that the save button remains enabled after a failed save,
8+
* allowing users to retry without making additional form changes.
9+
*/
10+
export const DraftValidationCollection: CollectionConfig = {
11+
slug: draftValidationSlug,
12+
admin: {
13+
useAsTitle: 'title',
14+
},
15+
fields: [
16+
{
17+
name: 'title',
18+
type: 'text',
19+
required: true,
20+
},
21+
{
22+
name: 'failValidation',
23+
type: 'checkbox',
24+
admin: {
25+
description:
26+
'Check this box to simulate a validation failure. The save button should remain enabled after the failure.',
27+
},
28+
defaultValue: false,
29+
},
30+
{
31+
name: 'validatedField',
32+
type: 'text',
33+
admin: {
34+
description:
35+
'This field will fail validation if "Fail Validation" checkbox is checked. This simulates validation failures from business logic, network errors, or third-party validation.',
36+
},
37+
validate: (value, { data }) => {
38+
// Simulate a validation failure based on the checkbox state
39+
if (data?.failValidation) {
40+
return 'Validation failed: This is a simulated validation error to test save button behavior.'
41+
}
42+
return true
43+
},
44+
},
45+
{
46+
name: 'description',
47+
type: 'textarea',
48+
},
49+
],
50+
versions: {
51+
drafts: {
52+
validate: true,
53+
},
54+
},
55+
}

test/form-state/config.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@ import path from 'path'
44
import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
55
import { devUser } from '../credentials.js'
66
import { AutosavePostsCollection } from './collections/Autosave/index.js'
7+
import { DraftValidationCollection } from './collections/DraftValidation/index.js'
78
import { PostsCollection, postsSlug } from './collections/Posts/index.js'
89

910
const filename = fileURLToPath(import.meta.url)
1011
const dirname = path.dirname(filename)
1112

1213
export default buildConfigWithDefaults({
13-
collections: [PostsCollection, AutosavePostsCollection],
14+
collections: [PostsCollection, AutosavePostsCollection, DraftValidationCollection],
1415
admin: {
1516
importMap: {
1617
baseDir: path.resolve(dirname),

test/form-state/e2e.spec.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import { AdminUrlUtil } from '../helpers/adminUrlUtil.js'
3030
import { initPayloadE2ENoConfig } from '../helpers/initPayloadE2ENoConfig.js'
3131
import { TEST_TIMEOUT, TEST_TIMEOUT_LONG } from '../playwright.config.js'
3232
import { autosavePostsSlug } from './collections/Autosave/index.js'
33+
import { draftValidationSlug } from './collections/DraftValidation/index.js'
3334
import { postsSlug } from './collections/Posts/index.js'
3435

3536
const { describe, beforeEach, afterEach } = test
@@ -649,6 +650,54 @@ test.describe('Form State', () => {
649650
)
650651
})
651652
})
653+
654+
test.describe('Draft Validation', () => {
655+
let draftValidationUrl: AdminUrlUtil
656+
657+
test.beforeAll(() => {
658+
draftValidationUrl = new AdminUrlUtil(serverURL, draftValidationSlug)
659+
})
660+
661+
test('should keep save draft button enabled after validation failure on update', async () => {
662+
// Create a document successfully
663+
await page.goto(draftValidationUrl.create)
664+
await page.locator('#field-title').fill('Test Document')
665+
await page.locator('#field-validatedField').fill('Valid data')
666+
await page.click('#action-save-draft')
667+
await expect(page.locator('.payload-toast-container .toast-success')).toBeVisible()
668+
669+
await page.waitForURL(/\/admin\/collections\/draft-validation\/[a-zA-Z0-9]+/)
670+
671+
// Modify document to trigger validation failure
672+
await page.locator('#field-title').fill('Modified Document')
673+
await page.locator('#field-failValidation').check()
674+
await page.locator('#field-validatedField').fill('This will fail validation')
675+
676+
await page.click('#action-save-draft')
677+
await expect(page.locator('.payload-toast-container .toast-error')).toBeVisible()
678+
679+
// Save Draft button should remain enabled after failed save
680+
const saveDraftButton = page.locator('#action-save-draft')
681+
await expect(saveDraftButton).toBeEnabled()
682+
683+
// Verify we can retry without additional changes
684+
await page.click('#action-save-draft')
685+
await expect(page.locator('.payload-toast-container .toast-error')).toBeVisible()
686+
})
687+
688+
test('should keep save draft button enabled after successful save when form is modified again', async () => {
689+
await page.goto(draftValidationUrl.create)
690+
await page.locator('#field-title').fill('Test Document')
691+
await page.locator('#field-validatedField').fill('Valid data')
692+
await page.click('#action-save-draft')
693+
await expect(page.locator('.payload-toast-container .toast-success')).toBeVisible()
694+
695+
await page.locator('#field-title').fill('Modified Document')
696+
697+
const saveDraftButton = page.locator('#action-save-draft')
698+
await expect(saveDraftButton).toBeEnabled()
699+
})
700+
})
652701
})
653702

654703
async function createPost(overrides?: Partial<Post>): Promise<Post> {

test/form-state/payload-types.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ export interface Config {
6969
collections: {
7070
posts: Post;
7171
'autosave-posts': AutosavePost;
72+
'draft-validation': DraftValidation;
73+
'payload-kv': PayloadKv;
7274
users: User;
7375
'payload-locked-documents': PayloadLockedDocument;
7476
'payload-preferences': PayloadPreference;
@@ -78,6 +80,8 @@ export interface Config {
7880
collectionsSelect: {
7981
posts: PostsSelect<false> | PostsSelect<true>;
8082
'autosave-posts': AutosavePostsSelect<false> | AutosavePostsSelect<true>;
83+
'draft-validation': DraftValidationSelect<false> | DraftValidationSelect<true>;
84+
'payload-kv': PayloadKvSelect<false> | PayloadKvSelect<true>;
8185
users: UsersSelect<false> | UsersSelect<true>;
8286
'payload-locked-documents': PayloadLockedDocumentsSelect<false> | PayloadLockedDocumentsSelect<true>;
8387
'payload-preferences': PayloadPreferencesSelect<false> | PayloadPreferencesSelect<true>;
@@ -175,6 +179,43 @@ export interface AutosavePost {
175179
createdAt: string;
176180
_status?: ('draft' | 'published') | null;
177181
}
182+
/**
183+
* This interface was referenced by `Config`'s JSON-Schema
184+
* via the `definition` "draft-validation".
185+
*/
186+
export interface DraftValidation {
187+
id: string;
188+
title: string;
189+
/**
190+
* Check this box to simulate a validation failure. The save button should remain enabled after the failure.
191+
*/
192+
failValidation?: boolean | null;
193+
/**
194+
* This field will fail validation if "Fail Validation" checkbox is checked. This simulates validation failures from business logic, network errors, or third-party validation.
195+
*/
196+
validatedField?: string | null;
197+
description?: string | null;
198+
updatedAt: string;
199+
createdAt: string;
200+
_status?: ('draft' | 'published') | null;
201+
}
202+
/**
203+
* This interface was referenced by `Config`'s JSON-Schema
204+
* via the `definition` "payload-kv".
205+
*/
206+
export interface PayloadKv {
207+
id: string;
208+
key: string;
209+
data:
210+
| {
211+
[k: string]: unknown;
212+
}
213+
| unknown[]
214+
| string
215+
| number
216+
| boolean
217+
| null;
218+
}
178219
/**
179220
* This interface was referenced by `Config`'s JSON-Schema
180221
* via the `definition` "users".
@@ -214,6 +255,10 @@ export interface PayloadLockedDocument {
214255
relationTo: 'autosave-posts';
215256
value: string | AutosavePost;
216257
} | null)
258+
| ({
259+
relationTo: 'draft-validation';
260+
value: string | DraftValidation;
261+
} | null)
217262
| ({
218263
relationTo: 'users';
219264
value: string | User;
@@ -314,6 +359,27 @@ export interface AutosavePostsSelect<T extends boolean = true> {
314359
createdAt?: T;
315360
_status?: T;
316361
}
362+
/**
363+
* This interface was referenced by `Config`'s JSON-Schema
364+
* via the `definition` "draft-validation_select".
365+
*/
366+
export interface DraftValidationSelect<T extends boolean = true> {
367+
title?: T;
368+
failValidation?: T;
369+
validatedField?: T;
370+
description?: T;
371+
updatedAt?: T;
372+
createdAt?: T;
373+
_status?: T;
374+
}
375+
/**
376+
* This interface was referenced by `Config`'s JSON-Schema
377+
* via the `definition` "payload-kv_select".
378+
*/
379+
export interface PayloadKvSelect<T extends boolean = true> {
380+
key?: T;
381+
data?: T;
382+
}
317383
/**
318384
* This interface was referenced by `Config`'s JSON-Schema
319385
* via the `definition` "users_select".

0 commit comments

Comments
 (0)