Started confirming automation re-publish#28113
Conversation
closes https://linear.app/ghost/issue/NY-1299 If an automation is active and you go to update it, we now show a confirmation dialog. This change has no effect for inactive automations.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR extends the automation editor's state machine to support republishing active automations through a user confirmation flow. A new Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This comment was marked as low quality.
This comment was marked as low quality.
This comment was marked as low quality.
This comment was marked as low quality.
| @@ -173,27 +233,49 @@ const AutomationEditor: React.FC = () => { | |||
| const onConfirmUnpublishOpenChange = (open: boolean): void => { | |||
There was a problem hiding this comment.
Not strictly in scope, but I simplified this alert open change callback because I was adding a new similar one.
| <AlertDialogHeader> | ||
| <AlertDialogTitle>Update automation?</AlertDialogTitle> | ||
| <AlertDialogDescription> | ||
| This will update the automation for new runs of the automation, as well as any actively-running ones. |
There was a problem hiding this comment.
I'm not 100% sure on this copy, but we can tweak it.
There was a problem hiding this comment.
This diff is a little hard to read because I started adding newlines. Here's the effective change:
export type AutomationEditState =
| 'idle'
| 'saving'
| 'publishing'
+ | 're-publishing'
| 'unpublishing'
| 'confirming unpublish'
+ | 'confirming re-publish'
| 'failed to save'
| 'failed to publish'
+ | 'failed to re-publish'
| 'failed to unpublish';|
|
||
| const oldStatus = draft.status; | ||
| const newStatus = statusToSave ?? oldStatus; | ||
| const statusTransition: `${AutomationStatus} -> ${AutomationStatus}` = `${oldStatus} -> ${newStatus}`; |
There was a problem hiding this comment.
Couldn't tell if this was too clever, but it succinctly described what I wanted...
| case 'confirming re-publish': | ||
| case 'failed to re-publish': |
There was a problem hiding this comment.
not something for this PR, but i'm wondering if we should clean up these states into something other than strings as they continue to grow.
For example, there's sort of a relationship here between these two states, or any two states that're like confirming <whatever> and failed <whatever>.
In all of these functions we don't really have to worry about typos or anything since TypeScript is doing the heavy-lifting, but it kinda feels like the AutomationEditState bucket might be too general now. I'm also finding this harder to read as it grows.
Maybe syntax like state.republish.confirming or something like that? Not urgent, might be a fun one for 2-3 of us to pair on when we're in a QA window just as a chance to talk shop.
closes https://linear.app/ghost/issue/NY-1299
Screencast.mp4
If an automation is active and you go to update it, we now show a confirmation dialog.
This change has no effect for inactive automations.