fix(ui): prevent form validation after draft submit error (#12918)
Follow-up to #12893. ### What? Removes cases where submitting a document as a draft in a collection with versioning enabled would cause publishing validation errors to be displayed on further document form changes. An example case is when saving the draft failed due to a `beforeChange` hook throwing an `APIError`. ### How The behavior change is that the form state is marked as un-submitted post a submit failure as a draft. The form not being considered as submitted results in `packages/ui/src/views/Edit/index.tsx` to use `skipValidation: true`. --------- Co-authored-by: Jarrod Flesch <jarrodmflesch@gmail.com>
This commit is contained in:
committed by
GitHub
parent
faed3aaf26
commit
a2c31fa44a
48
test/versions/collections/DraftsWithChangeHook.ts
Normal file
48
test/versions/collections/DraftsWithChangeHook.ts
Normal file
@@ -0,0 +1,48 @@
|
||||
import type { CollectionConfig } from 'payload'
|
||||
|
||||
import { APIError } from 'payload'
|
||||
|
||||
import { draftWithChangeHookCollectionSlug } from '../slugs.js'
|
||||
|
||||
const DraftWithChangeHookPosts: CollectionConfig = {
|
||||
slug: draftWithChangeHookCollectionSlug,
|
||||
admin: {
|
||||
defaultColumns: ['title', 'description', 'createdAt', '_status'],
|
||||
useAsTitle: 'title',
|
||||
},
|
||||
fields: [
|
||||
{
|
||||
name: 'title',
|
||||
type: 'text',
|
||||
label: 'Title',
|
||||
localized: true,
|
||||
required: true,
|
||||
unique: true,
|
||||
hooks: {
|
||||
beforeChange: [
|
||||
(args) => {
|
||||
if (args?.data?.title?.includes('Invalid')) {
|
||||
throw new APIError('beforeChange hook threw APIError 422', 422)
|
||||
}
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
{
|
||||
name: 'description',
|
||||
type: 'textarea',
|
||||
label: 'Description',
|
||||
required: true,
|
||||
},
|
||||
],
|
||||
versions: {
|
||||
drafts: {
|
||||
schedulePublish: {
|
||||
timeFormat: 'HH:mm',
|
||||
},
|
||||
},
|
||||
maxPerDoc: 0,
|
||||
},
|
||||
}
|
||||
|
||||
export default DraftWithChangeHookPosts
|
||||
@@ -11,6 +11,7 @@ import CustomIDs from './collections/CustomIDs.js'
|
||||
import { Diff } from './collections/Diff/index.js'
|
||||
import DisablePublish from './collections/DisablePublish.js'
|
||||
import DraftPosts from './collections/Drafts.js'
|
||||
import DraftWithChangeHook from './collections/DraftsWithChangeHook.js'
|
||||
import DraftWithMax from './collections/DraftsWithMax.js'
|
||||
import DraftsWithValidate from './collections/DraftsWithValidate.js'
|
||||
import ErrorOnUnpublish from './collections/ErrorOnUnpublish.js'
|
||||
@@ -46,6 +47,7 @@ export default buildConfigWithDefaults({
|
||||
AutosaveWithDraftValidate,
|
||||
DraftPosts,
|
||||
DraftWithMax,
|
||||
DraftWithChangeHook,
|
||||
DraftsWithValidate,
|
||||
ErrorOnUnpublish,
|
||||
LocalizedPosts,
|
||||
|
||||
@@ -60,6 +60,7 @@ import {
|
||||
disablePublishSlug,
|
||||
draftCollectionSlug,
|
||||
draftGlobalSlug,
|
||||
draftWithChangeHookCollectionSlug,
|
||||
draftWithMaxCollectionSlug,
|
||||
draftWithMaxGlobalSlug,
|
||||
draftWithValidateCollectionSlug,
|
||||
@@ -87,6 +88,7 @@ describe('Versions', () => {
|
||||
let autosaveWithDraftButtonURL: AdminUrlUtil
|
||||
let autosaveWithDraftValidateURL: AdminUrlUtil
|
||||
let draftWithValidateURL: AdminUrlUtil
|
||||
let draftWithChangeHookURL: AdminUrlUtil
|
||||
let disablePublishURL: AdminUrlUtil
|
||||
let customIDURL: AdminUrlUtil
|
||||
let postURL: AdminUrlUtil
|
||||
@@ -1139,6 +1141,7 @@ describe('Versions', () => {
|
||||
beforeAll(() => {
|
||||
autosaveWithDraftValidateURL = new AdminUrlUtil(serverURL, autosaveWithDraftValidateSlug)
|
||||
draftWithValidateURL = new AdminUrlUtil(serverURL, draftWithValidateCollectionSlug)
|
||||
draftWithChangeHookURL = new AdminUrlUtil(serverURL, draftWithChangeHookCollectionSlug)
|
||||
})
|
||||
|
||||
test('- can save', async () => {
|
||||
@@ -1158,6 +1161,50 @@ describe('Versions', () => {
|
||||
await expect(page.locator('#field-title')).toHaveValue('New title')
|
||||
})
|
||||
|
||||
test('- can save draft with error thrown in beforeChange hook and continue editing without being shown publishing validation', async () => {
|
||||
await page.goto(draftWithChangeHookURL.create)
|
||||
|
||||
const titleField = page.locator('#field-title')
|
||||
const descriptionField = page.locator('#field-description')
|
||||
|
||||
await titleField.fill('Initial title')
|
||||
await descriptionField.fill('Initial description')
|
||||
await saveDocAndAssert(page, '#action-save-draft')
|
||||
|
||||
// Provoke an error being thrown in the beforeChange hook
|
||||
await titleField.fill('Invalid title')
|
||||
await saveDocAndAssert(page, '#action-save-draft')
|
||||
|
||||
// Verify that the form retains its client state and that no validation errors are shown
|
||||
await expect(page.locator('#field-title')).toHaveValue('Invalid title')
|
||||
await expect(page.locator('#field-description')).toHaveValue('Initial description')
|
||||
await expect(page.locator('.field-error')).toHaveCount(0)
|
||||
|
||||
// Make another field invalid
|
||||
await descriptionField.fill('')
|
||||
|
||||
// Verify that no validation errors are shown even after the debounced validation would have been triggered and processed.
|
||||
await wait(2000)
|
||||
await expect(page.locator('.field-error')).toHaveCount(0)
|
||||
|
||||
// Make the form valid again (`beforeChange` hook not throwing, `required` validation passing)
|
||||
await titleField.fill('New valid title')
|
||||
await descriptionField.fill('New valid description')
|
||||
await saveDocAndAssert(page, '#action-save-draft')
|
||||
|
||||
// Verify that valid draft submissions can be saved
|
||||
await expect(page.locator('#field-title')).toHaveValue('New valid title')
|
||||
await expect(page.locator('#field-description')).toHaveValue('New valid description')
|
||||
await expect(page.locator('.field-error')).toHaveCount(0)
|
||||
|
||||
await page.reload()
|
||||
|
||||
// Verify that valid draft submissions are persisted
|
||||
await expect(page.locator('#field-title')).toHaveValue('New valid title')
|
||||
await expect(page.locator('#field-description')).toHaveValue('New valid description')
|
||||
await expect(page.locator('.field-error')).toHaveCount(0)
|
||||
})
|
||||
|
||||
test('- can safely trigger validation errors and then continue editing', async () => {
|
||||
await page.goto(draftWithValidateURL.create)
|
||||
|
||||
@@ -1170,6 +1217,10 @@ describe('Versions', () => {
|
||||
await titleField.fill('')
|
||||
await saveDocAndAssert(page, '#action-save-draft', 'error')
|
||||
|
||||
const parentFieldType = page.locator('.field-type:has(#field-title)')
|
||||
await expect(parentFieldType.locator('.tooltip--show')).toBeVisible()
|
||||
await expect(parentFieldType).toHaveClass(/error/)
|
||||
|
||||
await titleField.fill('New title')
|
||||
|
||||
await saveDocAndAssert(page, '#action-save-draft')
|
||||
|
||||
@@ -75,6 +75,7 @@ export interface Config {
|
||||
'autosave-with-validate-posts': AutosaveWithValidatePost;
|
||||
'draft-posts': DraftPost;
|
||||
'draft-with-max-posts': DraftWithMaxPost;
|
||||
'draft-posts-with-change-hook': DraftPostsWithChangeHook;
|
||||
'draft-with-validate-posts': DraftWithValidatePost;
|
||||
'error-on-unpublish': ErrorOnUnpublish;
|
||||
'localized-posts': LocalizedPost;
|
||||
@@ -100,6 +101,7 @@ export interface Config {
|
||||
'autosave-with-validate-posts': AutosaveWithValidatePostsSelect<false> | AutosaveWithValidatePostsSelect<true>;
|
||||
'draft-posts': DraftPostsSelect<false> | DraftPostsSelect<true>;
|
||||
'draft-with-max-posts': DraftWithMaxPostsSelect<false> | DraftWithMaxPostsSelect<true>;
|
||||
'draft-posts-with-change-hook': DraftPostsWithChangeHookSelect<false> | DraftPostsWithChangeHookSelect<true>;
|
||||
'draft-with-validate-posts': DraftWithValidatePostsSelect<false> | DraftWithValidatePostsSelect<true>;
|
||||
'error-on-unpublish': ErrorOnUnpublishSelect<false> | ErrorOnUnpublishSelect<true>;
|
||||
'localized-posts': LocalizedPostsSelect<false> | LocalizedPostsSelect<true>;
|
||||
@@ -330,6 +332,18 @@ export interface DraftWithMaxPost {
|
||||
createdAt: string;
|
||||
_status?: ('draft' | 'published') | null;
|
||||
}
|
||||
/**
|
||||
* This interface was referenced by `Config`'s JSON-Schema
|
||||
* via the `definition` "draft-posts-with-change-hook".
|
||||
*/
|
||||
export interface DraftPostsWithChangeHook {
|
||||
id: string;
|
||||
title: string;
|
||||
description: string;
|
||||
updatedAt: string;
|
||||
createdAt: string;
|
||||
_status?: ('draft' | 'published') | null;
|
||||
}
|
||||
/**
|
||||
* This interface was referenced by `Config`'s JSON-Schema
|
||||
* via the `definition` "draft-with-validate-posts".
|
||||
@@ -742,6 +756,10 @@ export interface PayloadLockedDocument {
|
||||
relationTo: 'draft-with-max-posts';
|
||||
value: string | DraftWithMaxPost;
|
||||
} | null)
|
||||
| ({
|
||||
relationTo: 'draft-posts-with-change-hook';
|
||||
value: string | DraftPostsWithChangeHook;
|
||||
} | null)
|
||||
| ({
|
||||
relationTo: 'draft-with-validate-posts';
|
||||
value: string | DraftWithValidatePost;
|
||||
@@ -954,6 +972,17 @@ export interface DraftWithMaxPostsSelect<T extends boolean = true> {
|
||||
createdAt?: T;
|
||||
_status?: T;
|
||||
}
|
||||
/**
|
||||
* This interface was referenced by `Config`'s JSON-Schema
|
||||
* via the `definition` "draft-posts-with-change-hook_select".
|
||||
*/
|
||||
export interface DraftPostsWithChangeHookSelect<T extends boolean = true> {
|
||||
title?: T;
|
||||
description?: T;
|
||||
updatedAt?: T;
|
||||
createdAt?: T;
|
||||
_status?: T;
|
||||
}
|
||||
/**
|
||||
* This interface was referenced by `Config`'s JSON-Schema
|
||||
* via the `definition` "draft-with-validate-posts_select".
|
||||
@@ -1413,6 +1442,10 @@ export interface TaskSchedulePublish {
|
||||
| ({
|
||||
relationTo: 'draft-posts';
|
||||
value: string | DraftPost;
|
||||
} | null)
|
||||
| ({
|
||||
relationTo: 'draft-posts-with-change-hook';
|
||||
value: string | DraftPostsWithChangeHook;
|
||||
} | null);
|
||||
global?: 'draft-global' | null;
|
||||
user?: (string | null) | User;
|
||||
|
||||
@@ -12,6 +12,8 @@ export const draftCollectionSlug = 'draft-posts'
|
||||
export const draftWithValidateCollectionSlug = 'draft-with-validate-posts'
|
||||
export const draftWithMaxCollectionSlug = 'draft-with-max-posts'
|
||||
|
||||
export const draftWithChangeHookCollectionSlug = 'draft-posts-with-change-hook'
|
||||
|
||||
export const postCollectionSlug = 'posts'
|
||||
|
||||
export const diffCollectionSlug = 'diff'
|
||||
@@ -31,6 +33,7 @@ export const collectionSlugs = [
|
||||
autosaveCollectionSlug,
|
||||
autosaveWithMultiSelectCollectionSlug,
|
||||
draftCollectionSlug,
|
||||
draftWithChangeHookCollectionSlug,
|
||||
postCollectionSlug,
|
||||
diffCollectionSlug,
|
||||
mediaCollectionSlug,
|
||||
|
||||
Reference in New Issue
Block a user