fix: only allow save after form is modified (#6189)

Author: JarrodMFlesch

packages/ui/src/elements/DocumentDrawer/DrawerContent.tsx

@@ -21,7 +21,7 @@ import { RenderTitle } from '../RenderTitle/index.js'
 import { baseClass } from './index.js'
 
 export const DocumentDrawerContent: React.FC<DocumentDrawerProps> = ({
-  id,
+  id: existingDocID,
   Header,
   collectionSlug,
   drawerSlug,
@@ -39,7 +39,7 @@ export const DocumentDrawerContent: React.FC<DocumentDrawerProps> = ({
   const { closeModal, modalState, toggleModal } = useModal()
   const locale = useLocale()
   const { t } = useTranslation()
-  const [createdID, setCreatedID] = useState()
+  const [docID, setDocID] = useState(existingDocID)
   const [isOpen, setIsOpen] = useState(false)
   const [collectionConfig] = useRelatedCollections(collectionSlug)
   const { formQueryParams } = useFormQueryParams()
@@ -48,10 +48,12 @@ export const DocumentDrawerContent: React.FC<DocumentDrawerProps> = ({
   const { componentMap } = useComponentMap()
 
   const { Edit } = componentMap[`${collectionSlug ? 'collections' : 'globals'}`][collectionSlug]
-  const isEditing = Boolean(id)
-  const apiURL = id ? `${serverURL}${apiRoute}/${collectionSlug}/${id}?locale=${locale.code}` : null
+  const isEditing = Boolean(docID)
+  const apiURL = docID
+    ? `${serverURL}${apiRoute}/${collectionSlug}/${docID}?locale=${locale.code}`
+    : null
   const action = `${serverURL}${apiRoute}/${collectionSlug}${
-    isEditing ? `/${id}` : ''
+    isEditing ? `/${docID}` : ''
   }?${formattedQueryParams}`
 
   useEffect(() => {
@@ -70,7 +72,7 @@ export const DocumentDrawerContent: React.FC<DocumentDrawerProps> = ({
 
   const onSave = useCallback<DocumentDrawerProps['onSave']>(
     (args) => {
-      setCreatedID(args.doc.id)
+      setDocID(args.doc.id)
       if (typeof onSaveFromProps === 'function') {
         void onSaveFromProps({
           ...args,
@@ -115,7 +117,7 @@ export const DocumentDrawerContent: React.FC<DocumentDrawerProps> = ({
       // Same reason as above. We need to fully-fetch the docPreferences from the server. This is done in DocumentInfoProvider if we set it to null here.
       hasSavePermission={null}
       // isLoading,
-      id={id || createdID}
+      id={docID}
       isEditing={isEditing}
       onLoadError={onLoadError}
       onSave={onSave}

packages/ui/src/elements/Save/index.tsx

@@ -2,20 +2,27 @@
 
 import React, { useRef } from 'react'
 
-import { useForm } from '../../forms/Form/context.js'
+import { useForm, useFormModified } from '../../forms/Form/context.js'
 import { FormSubmit } from '../../forms/Submit/index.js'
 import { useHotkey } from '../../hooks/useHotkey.js'
 import { useEditDepth } from '../../providers/EditDepth/index.js'
+import { useOperation } from '../../providers/Operation/index.js'
 import { useTranslation } from '../../providers/Translation/index.js'
 
 export const DefaultSaveButton: React.FC<{ label?: string }> = ({ label: labelProp }) => {
   const { t } = useTranslation()
   const { submit } = useForm()
+  const modified = useFormModified()
   const label = labelProp || t('general:save')
   const ref = useRef<HTMLButtonElement>(null)
   const editDepth = useEditDepth()
+  const operation = useOperation()
+
+  const forceDisable = operation === 'update' && !modified
 
   useHotkey({ cmdCtrlKey: true, editDepth, keyCodes: ['s'] }, (e) => {
+    if (forceDisable) return
+
     e.preventDefault()
     e.stopPropagation()
     if (ref?.current) {
@@ -26,6 +33,7 @@ export const DefaultSaveButton: React.FC<{ label?: string }> = ({ label: labelPr
   return (
     <FormSubmit
       buttonId="action-save"
+      disabled={forceDisable}
       onClick={() => submit()}
       ref={ref}
       size="small"

packages/ui/src/elements/SaveDraft/index.tsx

@@ -9,6 +9,7 @@ import { useConfig } from '../../providers/Config/index.js'
 import { useDocumentInfo } from '../../providers/DocumentInfo/index.js'
 import { useEditDepth } from '../../providers/EditDepth/index.js'
 import { useLocale } from '../../providers/Locale/index.js'
+import { useOperation } from '../../providers/Operation/index.js'
 import { useTranslation } from '../../providers/Translation/index.js'
 
 const baseClass = 'save-draft'
@@ -25,8 +26,13 @@ const DefaultSaveDraftButton: React.FC = () => {
   const editDepth = useEditDepth()
   const { t } = useTranslation()
   const { submit } = useForm()
+  const operation = useOperation()
+
+  const forceDisable = operation === 'update' && !modified
 
   const saveDraft = useCallback(async () => {
+    if (forceDisable) return
+
     const search = `?locale=${locale}&depth=0&fallback-locale=null&draft=true`
     let action
     let method = 'POST'
@@ -48,12 +54,10 @@ const DefaultSaveDraftButton: React.FC = () => {
       },
       skipValidation: true,
     })
-  }, [submit, collectionSlug, globalSlug, serverURL, api, locale, id])
+  }, [submit, collectionSlug, globalSlug, serverURL, api, locale, id, forceDisable])
 
   useHotkey({ cmdCtrlKey: true, editDepth, keyCodes: ['s'] }, (e) => {
-    if (!modified) {
-      return
-    }
+    if (forceDisable) return
 
     e.preventDefault()
     e.stopPropagation()
@@ -67,7 +71,7 @@ const DefaultSaveDraftButton: React.FC = () => {
       buttonId="action-save-draft"
       buttonStyle="secondary"
       className={baseClass}
-      disabled={!modified}
+      disabled={forceDisable}
       onClick={saveDraft}
       ref={ref}
       size="small"

test/admin/e2e.spec.ts

@@ -465,7 +465,6 @@ describe('admin', () => {
       await globalLabel.click()
       await checkPageTitle(page, label)
       await checkBreadcrumb(page, label)
-      await saveDocAndAssert(page)
     })
 
     test('global — should render slug in sentence case as fallback', async () => {
@@ -478,7 +477,6 @@ describe('admin', () => {
       await globalLabel.click()
       await checkPageTitle(page, label)
       await checkBreadcrumb(page, label)
-      await saveDocAndAssert(page)
     })
   })
 

test/fields-relationship/e2e.spec.ts

@@ -438,8 +438,10 @@ describe('fields - relationship', () => {
     ).toHaveCount(1)
 
     // save the same document again to ensure the relationship field doesn't receive duplicative values
+    await drawerField.fill('Updated document')
     await saveButton.click()
-    await expect(page.locator('.Toastify')).toContainText('successfully')
+    await expect(page.locator('.Toastify')).toContainText('Updated successfully')
+    await page.locator('.doc-drawer__header-close').click()
     await expect(
       page.locator('#field-relationshipHasMany .value-container .rs__multi-value'),
     ).toHaveCount(1)

test/localization/e2e.spec.ts

@@ -116,7 +116,6 @@ describe('Localization', () => {
 
       await fillValues({ description, title })
       await saveDocAndAssert(page)
-      await saveDocAndAssert(page)
 
       await expect(page.locator('#field-title')).toHaveValue(title)
       await expect(page.locator('#field-description')).toHaveValue(description)
@@ -241,7 +240,6 @@ describe('Localization', () => {
       await openDocControls(page)
       await page.locator('#action-duplicate').click()
       await expect(page.locator('.id-label')).not.toContainText(originalID)
-      await page.locator('#action-save').click()
 
       // verify that the locale did copy
       await expect(page.locator('#field-title')).toHaveValue(englishTitle)