Skip to content

Commit 67063ab

Browse files
GermanJabloGermanJablo
authored
fix: unpublish updates existing version instead of creating a new one (#15985)
## Summary Unpublishing a document currently creates a new version in the versions table, even though the content hasn't changed. Only `_status` transitions from `published` to `draft`. This clutters the version history with redundant entries and has been a source of confusion (see #10838, #12342, #15125). This PR makes unpublish update the latest existing version in-place instead of creating a new one, for both collections and globals. Closes #10838 (again) ## Related #10838 #12342 #15125 (Discussion) ## Changes - `saveVersion` now accepts an `unpublish` flag. When set, it finds the latest version and updates it in-place (same pattern as the existing autosave overwrite path) instead of creating a new version. - Collection and global update operations pass `unpublish: unpublishAllLocales` to `saveVersion`. No UI or `next` package changes are needed. The admin UI already sends `unpublishAllLocales=true` in the query string when the user clicks "Unpublish." This parameter already flowed through the endpoint handler, the operation, and into `updateDocument`. The only thing missing was that `saveVersion` didn't use it. This PR makes `saveVersion` check for it. For normal updates (saving drafts, publishing, editing content), `unpublishAllLocales` is `undefined`, so `!!undefined` evaluates to `false` and the new code path is skipped entirely. ## Scope and limitations This PR covers the main "Unpublish" action, which sends `unpublishAllLocales=true` even for non-localized collections. This is the case reported in #10838 and #12342. Single-locale unpublish (the "Unpublish in [locale]" button, available when `localizeStatus` is enabled) is **not** covered by this PR. That flow does not send `unpublishAllLocales=true`. Instead, it falls into the single-locale publish/unpublish code path, which involves data merging and snapshots. The current code does not distinguish between "publish locale X" and "unpublish locale X," so fixing it requires a new flag (e.g. `unpublishLocale`) that the UI would need to send. This should be addressed in a follow-up PR. ## Testing Run the new tests with: ``` PAYLOAD_DATABASE=postgres pnpm run test:int versions -- --testNamePattern="Unpublish" ``` Without this PR's changes, 2 of the 3 tests fail: ``` FAIL should not create a new version when unpublishing a collection document AssertionError: expected [ { id: 249, parent: 24, …(6) }, …(1) ] to have a length of 1 but got 2 FAIL should not create a new version when unpublishing a global AssertionError: expected [ …(2) ] to have a length of 1 but got 2 ``` The third test (`should update main table _status to draft when unpublishing`) passes both before and after, confirming the main table was already updated correctly. --------- Co-authored-by: German Jablonski <GermanJablo@users.noreply.github.com>
1 parent 9999083 commit 67063ab

5 files changed

Lines changed: 211 additions & 67 deletions

File tree

‎packages/payload/src/collections/operations/utilities/update.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,7 @@ export const updateDocument = async <
385385
publishSpecificLocale,
386386
req,
387387
snapshot: snapshotToSave,
388+
unpublish: unpublishAllLocales,
388389
})
389390
}
390391

‎packages/payload/src/globals/operations/update.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ export const updateOperation = async <
379379
req,
380380
select,
381381
snapshot: snapshotToSave,
382+
unpublish: unpublishAllLocales,
382383
})
383384

384385
result = {

‎packages/payload/src/versions/saveVersion.ts‎

Lines changed: 18 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { sanitizeInternalFields } from '../utilities/sanitizeInternalFields.js'
99
import { getQueryDraftsSelect } from './drafts/getQueryDraftsSelect.js'
1010
import { enforceMaxVersions } from './enforceMaxVersions.js'
1111
import { saveSnapshot } from './saveSnapshot.js'
12+
import { updateLatestVersion } from './updateLatestVersion.js'
1213

1314
type Args<T extends JsonObject = JsonObject> = {
1415
autosave?: boolean
@@ -24,6 +25,7 @@ type Args<T extends JsonObject = JsonObject> = {
2425
returning?: boolean
2526
select?: SelectType
2627
snapshot?: any
28+
unpublish?: boolean
2729
}
2830

2931
export async function saveVersion<TData extends JsonObject = JsonObject>(
@@ -49,9 +51,10 @@ export async function saveVersion<TData extends JsonObject = JsonObject>({
4951
returning,
5052
select,
5153
snapshot,
54+
unpublish,
5255
}: Args<TData>): Promise<JsonObject | null> {
5356
let result: JsonObject | undefined
54-
let createNewVersion = true
57+
let createdNewVersion = false
5558
const now = new Date().toISOString()
5659
const versionData: {
5760
_status?: 'draft'
@@ -67,74 +70,22 @@ export async function saveVersion<TData extends JsonObject = JsonObject>({
6770
}
6871

6972
try {
70-
if (autosave) {
71-
let docs
72-
const findVersionArgs = {
73-
limit: 1,
74-
pagination: false,
73+
if (unpublish || autosave) {
74+
result = await updateLatestVersion({
75+
id,
76+
collection,
77+
global,
78+
now,
79+
payload,
7580
req,
76-
sort: '-updatedAt',
77-
}
78-
79-
if (collection) {
80-
;({ docs } = await payload.db.findVersions<TData>({
81-
...findVersionArgs,
82-
collection: collection.slug,
83-
limit: 1,
84-
pagination: false,
85-
req,
86-
where: {
87-
parent: {
88-
equals: id,
89-
},
90-
},
91-
}))
92-
} else {
93-
;({ docs } = await payload.db.findGlobalVersions<TData>({
94-
...findVersionArgs,
95-
global: global!.slug,
96-
limit: 1,
97-
pagination: false,
98-
req,
99-
}))
100-
}
101-
const [latestVersion] = docs
102-
103-
// overwrite the latest version if it's set to autosave
104-
if (latestVersion && 'autosave' in latestVersion && latestVersion.autosave === true) {
105-
createNewVersion = false
106-
107-
const updateVersionArgs = {
108-
id: latestVersion.id,
109-
req,
110-
versionData: {
111-
createdAt: new Date(latestVersion.createdAt).toISOString(),
112-
latest: true,
113-
parent: id,
114-
updatedAt: now,
115-
version: {
116-
...versionData,
117-
},
118-
},
119-
}
120-
121-
if (collection) {
122-
result = await payload.db.updateVersion<TData>({
123-
...updateVersionArgs,
124-
collection: collection.slug,
125-
req,
126-
})
127-
} else {
128-
result = await payload.db.updateGlobalVersion<TData>({
129-
...updateVersionArgs,
130-
global: global!.slug,
131-
req,
132-
})
133-
}
134-
}
81+
shouldUpdate: autosave ? (v) => 'autosave' in v && v.autosave === true : undefined,
82+
versionData,
83+
})
13584
}
13685

137-
if (createNewVersion) {
86+
if (!result) {
87+
createdNewVersion = true
88+
13889
const createVersionArgs = {
13990
autosave: Boolean(autosave),
14091
collectionSlug: undefined as string | undefined,
@@ -188,7 +139,7 @@ export async function saveVersion<TData extends JsonObject = JsonObject>({
188139

189140
const max = getVersionsMax(collection || global!)
190141

191-
if (createNewVersion && max > 0) {
142+
if (createdNewVersion && max > 0) {
192143
await enforceMaxVersions({
193144
id,
194145
collection,

‎packages/payload/src/versions/updateLatestVersion.ts‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import type { SanitizedCollectionConfig } from '../collections/config/types.js'
2+
import type { SanitizedGlobalConfig } from '../globals/config/types.js'
3+
import type { Payload } from '../index.js'
4+
import type { JsonObject, PayloadRequest } from '../types/index.js'
5+
6+
type Args<TData extends JsonObject> = {
7+
collection?: SanitizedCollectionConfig
8+
global?: SanitizedGlobalConfig
9+
id?: number | string
10+
now: string
11+
payload: Payload
12+
req?: PayloadRequest
13+
shouldUpdate?: (latestVersion: JsonObject) => boolean
14+
versionData: TData
15+
}
16+
17+
/**
18+
* Finds the latest version and updates it in place if `shouldUpdate` returns true.
19+
* Used by both the unpublish and autosave paths in `saveVersion` to avoid creating
20+
* a redundant new version.
21+
*
22+
* Returns the updated version result, or `undefined` if no update was performed.
23+
*/
24+
export async function updateLatestVersion<TData extends JsonObject>({
25+
id,
26+
collection,
27+
global,
28+
now,
29+
payload,
30+
req,
31+
shouldUpdate = () => true,
32+
versionData,
33+
}: Args<TData>): Promise<JsonObject | undefined> {
34+
let docs
35+
const findVersionArgs = {
36+
limit: 1,
37+
pagination: false,
38+
req,
39+
sort: '-updatedAt',
40+
}
41+
42+
if (collection) {
43+
;({ docs } = await payload.db.findVersions<TData>({
44+
...findVersionArgs,
45+
collection: collection.slug,
46+
where: {
47+
parent: {
48+
equals: id,
49+
},
50+
},
51+
}))
52+
} else {
53+
;({ docs } = await payload.db.findGlobalVersions<TData>({
54+
...findVersionArgs,
55+
global: global!.slug,
56+
}))
57+
}
58+
59+
const [latestVersion] = docs
60+
61+
if (!latestVersion || !shouldUpdate(latestVersion)) {
62+
return undefined
63+
}
64+
65+
const updateVersionArgs = {
66+
id: latestVersion.id,
67+
req,
68+
versionData: {
69+
createdAt: new Date(latestVersion.createdAt).toISOString(),
70+
latest: true,
71+
parent: id,
72+
updatedAt: now,
73+
version: {
74+
...versionData,
75+
},
76+
},
77+
}
78+
79+
if (collection) {
80+
return await payload.db.updateVersion<TData>({
81+
...updateVersionArgs,
82+
collection: collection.slug,
83+
req,
84+
})
85+
}
86+
87+
return await payload.db.updateGlobalVersion<TData>({
88+
...updateVersionArgs,
89+
global: global!.slug,
90+
req,
91+
})
92+
}

‎test/versions/int.spec.ts‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,6 +1367,105 @@ describe('Versions', () => {
13671367
})
13681368
})
13691369

1370+
describe('Unpublish', () => {
1371+
afterEach(async () => {
1372+
await cleanupDocuments({
1373+
collectionSlugs: [draftCollectionSlug],
1374+
payload,
1375+
})
1376+
})
1377+
1378+
it('should not create a new version when unpublishing a collection document', async () => {
1379+
const doc = await payload.create({
1380+
collection: draftCollectionSlug,
1381+
data: {
1382+
_status: 'published',
1383+
description: 'test',
1384+
title: 'unpublish test',
1385+
},
1386+
})
1387+
1388+
const initialVersions = await payload.findVersions({
1389+
collection: draftCollectionSlug,
1390+
where: { parent: { equals: doc.id } },
1391+
})
1392+
1393+
expect(initialVersions.docs).toHaveLength(1)
1394+
expect(initialVersions.docs[0].version._status).toBe('published')
1395+
1396+
const unpublished = await payload.update({
1397+
id: doc.id,
1398+
collection: draftCollectionSlug,
1399+
data: { _status: 'draft' },
1400+
unpublishAllLocales: true,
1401+
})
1402+
1403+
expect(unpublished._status).toBe('draft')
1404+
1405+
const afterVersions = await payload.findVersions({
1406+
collection: draftCollectionSlug,
1407+
where: { parent: { equals: doc.id } },
1408+
})
1409+
1410+
expect(afterVersions.docs).toHaveLength(1)
1411+
expect(afterVersions.docs[0].version._status).toBe('draft')
1412+
})
1413+
1414+
it('should not create a new version when unpublishing a global', async () => {
1415+
await payload.updateGlobal({
1416+
slug: draftGlobalSlug,
1417+
data: { _status: 'published', title: 'unpublish global test' },
1418+
})
1419+
1420+
const initialVersions = await payload.findGlobalVersions({
1421+
slug: draftGlobalSlug,
1422+
})
1423+
1424+
const initialCount = initialVersions.docs.length
1425+
1426+
await payload.updateGlobal({
1427+
slug: draftGlobalSlug,
1428+
data: { _status: 'draft' },
1429+
unpublishAllLocales: true,
1430+
})
1431+
1432+
const afterVersions = await payload.findGlobalVersions({
1433+
slug: draftGlobalSlug,
1434+
})
1435+
1436+
expect(afterVersions.docs).toHaveLength(initialCount)
1437+
expect(afterVersions.docs[0].version._status).toBe('draft')
1438+
1439+
await cleanupGlobal({ payload, globalSlug: draftGlobalSlug })
1440+
})
1441+
1442+
it('should update main table _status to draft when unpublishing', async () => {
1443+
const doc = await payload.create({
1444+
collection: draftCollectionSlug,
1445+
data: {
1446+
_status: 'published',
1447+
description: 'test',
1448+
title: 'main table unpublish test',
1449+
},
1450+
})
1451+
1452+
await payload.update({
1453+
id: doc.id,
1454+
collection: draftCollectionSlug,
1455+
data: { _status: 'draft' },
1456+
unpublishAllLocales: true,
1457+
})
1458+
1459+
const found = await payload.findByID({
1460+
id: doc.id,
1461+
collection: draftCollectionSlug,
1462+
draft: false,
1463+
})
1464+
1465+
expect(found._status).toBe('draft')
1466+
})
1467+
})
1468+
13701469
describe('Draft Types', () => {
13711470
afterEach(async () => {
13721471
await cleanupDocuments({

0 commit comments

Comments
 (0)