fix(drizzle): use loose equality for null check in traverseFields to catch undefined#15977
fix(drizzle): use loose equality for null check in traverseFields to catch undefined#15977xxarupakaxx wants to merge 3 commits into
Conversation
…catch undefined When restoring a version where upload/relationship fields were never set, the field value is `undefined` (not `null`). The strict equality check (`=== null`) missed this case, causing stale relationship rows to persist in the database instead of being deleted. Changed `=== null` to `== null` (loose equality) in two locations: - Line 551: localized relationship/upload fields - Line 572: non-localized relationship/upload fields Fixes payloadcms#15976
| // This bug only affects drizzle-based adapters (postgres, sqlite) where relationships | ||
| // are stored in separate tables and require explicit deletion via relationshipsToDelete. | ||
| // MongoDB stores relationships inline in the document, so this specific code path does not apply. | ||
| if (payload.db.name === 'mongoose') { | ||
| return | ||
| } |
There was a problem hiding this comment.
This looks strange to me, since you don't use any DB-specific APIs in those 2 tests - the output should be the same. Do we need to fix this in MongoDB as well?
There was a problem hiding this comment.
I ran this test with MongoDB and it failed, so I think we need to remove this condition and fix the issue in MongoDB as well, I think you just need to do the same logic in payload/packages/db-mongodb/src/utilities/transform.ts.
There was a problem hiding this comment.
Thanks for catching this! You were right — the test fails on MongoDB as well.
However, fixing it in db-mongodb/src/utilities/transform.ts alone caused unique constraint violations on indexed-fields tests, because setting all falsy relationship/upload fields to null during every write triggers duplicate null errors on unique: true fields.
Instead, I fixed it at the restoreVersion level in the Payload core — missing relationship/upload fields are explicitly set to null in the version data early in the pipeline (right after extracting versionToRestoreWithLocales), so the null values flow through the entire hook chain and reach updateOne/updateGlobal for all adapters.
Removed the mongoose skip condition from both tests.
There was a problem hiding this comment.
@r1tsuu
Friendly ping — any thoughts on the MongoDB unique constraint issue I mentioned?
When restoring a version where relationship/upload fields were never set, the restored data does not include those fields. This causes database adapters (especially MongoDB, which uses $set semantics) to preserve stale values from the current document instead of clearing them. Set missing relationship/upload fields to null in the version data early in the restoreVersion pipeline, so the null values flow through the entire hook chain (afterRead → beforeValidate → beforeChange → updateOne/updateGlobal).
…t upload/relationship fields - Add 'media' (upload) field to DraftPosts test collection - Add test: should clear upload field when restoring a version where the field was never set - Add test: should clear relationship field when restoring a version where the field was never set
58b83bb to
42482d5
Compare
What?
Fixed a bug where restoring a published version does not clear upload/relationship fields when the original version had no value set for those fields.
Why?
In
traverseFields.ts, the null check uses strict equality (=== null), which does not catchundefined. When restoring a version where an upload/relationship field was never set,transform/readdoes not set the property on the result object (leaving it asundefined, notnull). The write transform then fails to add the field torelationshipsToDelete, so stale relationship rows persist in the database.Reproduction steps:
uploadfieldHow?
Changed
=== nullto== null(loose equality) in two locations inpackages/drizzle/src/transform/write/traverseFields.ts:localeData === null→localeData == nullfieldData === null→fieldData == nullThis ensures both
nullandundefinedtrigger deletion of relationship rows.Tests
Added two regression tests in
test/versions/int.spec.ts:should clear upload field when restoring a version where the field was never setshould clear relationship field when restoring a version where the field was never setAlso added a
media(upload) field to theDraftPoststest collection to support the upload test.Fixes #15976