Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): drafts not using server actions always generate a created at #7503

Merged
merged 5 commits into from
Sep 16, 2024

Conversation

jordanl17
Copy link
Member

@jordanl17 jordanl17 commented Sep 13, 2024

Description

Closes SDX-1519

There are currently instances where (when NOT using server actions), a document would be published with a _createdAt of the publish time, rather than the time at which the draft was created.

This occurred due to _createdAt on the draft not being set on creation, thus on publish the content lake was default setting to now. This issue wouldn't be present if a refresh occurred between creating the draft and then publishing (since again content lake would fabricate a createdAt value for the draft which would then pull through in studio and be used in publish. This issue didn't occur with server actions since the server always reconciled the createdAt and used the local server version rather than the client memory draft.

What to review

The issue seemingly is just that params.timestamp was never set on the mutation execution.

It feels most appropriate to guarantee a valid createdAt be defined when create, createIfNotExists or createOrReplace mutation are called, since these are the instances at which a draft/published doc are conceived. Even if the timestamp is not defined, really this should be defaulted anyway.

Do you feel this makes sense, or does it feel more appropriate to require that the caller define this, otherwise they potentially risk encountering this issue again

Testing

  • Updated SquashedBuffer mock for global date to validate that the createdAt is being set according to the default fallback the first time the draft is createIfNotExists

Notes for release

Fix for an issue where published documents would be assigned a createdAt timestamp of their publish time, rather than the timestamp of when the associated draft was first created.

Copy link

vercel bot commented Sep 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
page-building-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 15, 2024 10:06pm
performance-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 15, 2024 10:06pm
test-compiled-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 15, 2024 10:06pm
test-next-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 15, 2024 10:06pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 15, 2024 10:06pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Sep 15, 2024 10:06pm

Copy link
Contributor

No changes to documentation

Copy link
Contributor

github-actions bot commented Sep 13, 2024

Component Testing Report Updated Sep 15, 2024 10:00 PM (UTC)

✅ All Tests Passed -- expand for details
File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 42s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 9s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 29s 6 0 0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 36s 11 7 0
formBuilder/inputs/PortableText/copyPaste/CopyPasteFields.spec.tsx ✅ Passed (Inspect) 0s 0 12 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 17s 6 0 0
formBuilder/inputs/PortableText/DisableFocusAndUnset.spec.tsx ✅ Passed (Inspect) 10s 3 0 0
formBuilder/inputs/PortableText/DragAndDrop.spec.tsx ✅ Passed (Inspect) 3m 0s 0 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 43s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 43s 21 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 1m 14s 18 0 0
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx ✅ Passed (Inspect) 8s 3 9 0
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx ✅ Passed (Inspect) 25s 9 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 18s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 36s 12 0 0
formBuilder/tree-editing/TreeEditing.spec.tsx ✅ Passed (Inspect) 0s 0 3 0
formBuilder/tree-editing/TreeEditingNestedObjects.spec.tsx ✅ Passed (Inspect) 0s 0 3 0

Copy link
Member

@bjoerge bjoerge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jordanl17! This looks like a very sensible fix to me. Left a couple of comments/suggestions. I think the isString check in particular isn't neccessary, so would be good to remove it :)

packages/@sanity/mutator/src/document/Mutation.ts Outdated Show resolved Hide resolved
Copy link
Member

@bjoerge bjoerge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent! Thank you for taking the time to write tests as well.

@bjoerge bjoerge added this pull request to the merge queue Sep 16, 2024
Merged via the queue into next with commit 728311e Sep 16, 2024
42 checks passed
@bjoerge bjoerge deleted the sdx-1519-createdat-missing branch September 16, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants