-
Notifications
You must be signed in to change notification settings - Fork 499
feat: delete scheduled drafts allows for copy back to draft when appropriate #11143
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
🧪 E2E Preview environment🔑 Environment Variables for Local TestingThis is the preview URL for the E2E tests: https://e2e-studio-ktz5xr8fy.sanity.dev To run the E2E tests locally, you can use the following environment variables, then run 💬 Remember to build the project first with |
🧪 E2E Preview environmentWaiting for preview deployment to finish… |
📊 Playwright Test ReportThis report contains test results, including videos of failing tests. |
📊 Playwright Test ReportWaiting for E2E tests to finish… |
⚡️ Editor Performance ReportDeploying studio and running performance tests… |
74bf037 to
b4f4bc9
Compare
packages/sanity/src/core/singleDocRelease/components/DeleteScheduledDraftDialog.tsx
Show resolved
Hide resolved
| {t('release.dialog.delete-schedule-draft.body')} | ||
| </Text> | ||
| </Box> | ||
| {showCopyCheckbox && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking that maybe it's better to always show this option, it will give security to editors that the scheduled draft document won't be lost.
Even if it's the same, you will need to previously know that and be sure that nothing is changing, if we always show it, then they will be sure that when removing they won't lose it.
wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems odd to show the checkbox when it has no effect though. Perhaps we could just show copy like 'This draft will still be available' or words to that effect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Simplify and improve the copy/showing of the checkbox
| if (draftDoc) { | ||
| await client.discardVersion({publishedId}, false) | ||
| } | ||
|
|
||
| // Create a new draft version from the scheduled draft | ||
| await client.createVersion({ | ||
| baseId: scheduledDraftId, | ||
| ifBaseRevisionId: scheduledDraftDoc._rev, | ||
| publishedId, | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to discard the existing? I think we can do a client.createOrReplace
If we need to discard it, then I think we should do both operations in a transaction, so if for any reason it fails we don't lose the draft
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeh fair
- Try
createOrReplaceor make it transactional
pedrobonamin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this Jordan!
It looks great and it will improve the experience for the users, added some comments to discuss
packages/sanity/src/core/singleDocRelease/components/DeleteScheduledDraftDialog.tsx
Outdated
Show resolved
Hide resolved
b4f4bc9 to
0055f3f
Compare
…s and copy options
3f7c106 to
af3e8a9
Compare
Description
With the launch of Scheduled Drafts there are a few issues when it comes to deleting scheduled drafts.
When a scheduled draft is deleted, we immediately delete the release and the version document within it. This means that if there are changed that are not reflected in another version of the document, they will be lost.
This was flagged as being an obvious issue - there was no reasonable way of copying across the changed, and just deleting the schedule - eg imagining that you wanted to make an amendment to the scheduled draft's content. This would involve API manipulation.
With these changes this issue is resolved.
The default is that the 'copy to draft' is selected. In this case the existing draft will be overridden with the content of the scheduled draft, before the scheduled draft is then deleted. If the user unchecks, then the existing draft will remain and the content in the scheduled draft will be discarded.
What to review
Testing
Notes for release
Deleting Scheduled Drafts will give the option for the content of the scheduled version to be copied to a draft version of the document.