Do not trigger autosave on workflows that are not persisted (#3400)

Co-authored-by: Benjamin Lu <templu1107@proton.me>

src/components/topbar/WorkflowTab.vue

@@ -61,18 +61,29 @@ const autoSaveDelay = computed(() =>
 )
 
 const shouldShowStatusIndicator = computed(() => {
-  // Return true if:
+  if (workspaceStore.shiftDown) {
-  // 1. The shift key is not pressed (hence no override).
+    // Branch 1: Shift key is held down, do not show the status indicator.
-  // 2. The workflow is either modified or not yet persisted.
+    return false
-  // 3. AutoSave is either turned off, or set to 'after delay'
+  }
-  //    with a delay longer than 3000ms.
+  if (!props.workflowOption.workflow.isPersisted) {
-  return (
+    // Branch 2: Workflow is not persisted, show the status indicator.
-    !workspaceStore.shiftDown &&
+    return true
-    (props.workflowOption.workflow.isModified ||
+  }
-      !props.workflowOption.workflow.isPersisted) &&
+  if (props.workflowOption.workflow.isModified) {
-    (autoSaveSetting.value === 'off' ||
+    // Branch 3: Workflow is modified.
-      (autoSaveSetting.value === 'after delay' && autoSaveDelay.value > 3000))
+    if (autoSaveSetting.value === 'off') {
-  )
+      // Sub-branch 3a: Autosave is off, so show the status indicator.
+      return true
+    }
+    if (autoSaveSetting.value === 'after delay' && autoSaveDelay.value > 3000) {
+      // Sub-branch 3b: Autosave delay is too high, so show the status indicator.
+      return true
+    }
+    // Sub-branch 3c: Workflow is modified but no condition applies, do not show the status indicator.
+    return false
+  }
+  // Default: do not show the status indicator. This should not be reachable.
+  return false
 })
 
 const closeWorkflows = async (options: WorkflowOption[]) => {

src/composables/useWorkflowAutoSave.ts

@@ -39,7 +39,7 @@ export function useWorkflowAutoSave() {
       const delay = autoSaveDelay.value
       autoSaveTimeout = setTimeout(async () => {
         const activeWorkflow = workflowStore.activeWorkflow
-        if (activeWorkflow?.isModified) {
+        if (activeWorkflow?.isModified && activeWorkflow.isPersisted) {
           try {
             isSaving = true
             await workflowService.saveWorkflow(activeWorkflow)

tests-ui/tests/composables/useWorkflowAutoSave.test.ts

@@ -36,7 +36,8 @@ vi.mock('@/stores/workflowStore', () => ({
 
 let mockAutoSaveSetting: string = 'off'
 let mockAutoSaveDelay: number = 1000
-let mockActiveWorkflow: { isModified: boolean } | null = null
+let mockActiveWorkflow: { isModified: boolean; isPersisted?: boolean } | null =
+  null
 
 describe('useWorkflowAutoSave', () => {
   beforeEach(() => {
@@ -51,7 +52,7 @@ describe('useWorkflowAutoSave', () => {
   it('should auto-save workflow after delay when modified and autosave enabled', async () => {
     mockAutoSaveSetting = 'after delay'
     mockAutoSaveDelay = 1000
-    mockActiveWorkflow = { isModified: true }
+    mockActiveWorkflow = { isModified: true, isPersisted: true }
 
     mount({
       template: `<div></div>`,
@@ -72,7 +73,7 @@ describe('useWorkflowAutoSave', () => {
   it('should not auto-save workflow after delay when not modified and autosave enabled', async () => {
     mockAutoSaveSetting = 'after delay'
     mockAutoSaveDelay = 1000
-    mockActiveWorkflow = { isModified: false }
+    mockActiveWorkflow = { isModified: false, isPersisted: true }
 
     mount({
       template: `<div></div>`,
@@ -93,7 +94,7 @@ describe('useWorkflowAutoSave', () => {
   it('should not auto save workflow when autosave is off', async () => {
     mockAutoSaveSetting = 'off'
     mockAutoSaveDelay = 1000
-    mockActiveWorkflow = { isModified: true }
+    mockActiveWorkflow = { isModified: true, isPersisted: true }
 
     mount({
       template: `<div></div>`,
@@ -112,7 +113,7 @@ describe('useWorkflowAutoSave', () => {
   it('should respect the user specified auto save delay', async () => {
     mockAutoSaveSetting = 'after delay'
     mockAutoSaveDelay = 2000
-    mockActiveWorkflow = { isModified: true }
+    mockActiveWorkflow = { isModified: true, isPersisted: true }
 
     mount({
       template: `<div></div>`,
@@ -135,7 +136,7 @@ describe('useWorkflowAutoSave', () => {
   it('should debounce save requests', async () => {
     mockAutoSaveSetting = 'after delay'
     mockAutoSaveDelay = 2000
-    mockActiveWorkflow = { isModified: true }
+    mockActiveWorkflow = { isModified: true, isPersisted: true }
 
     mount({
       template: `<div></div>`,
@@ -164,7 +165,7 @@ describe('useWorkflowAutoSave', () => {
   it('should handle save error gracefully', async () => {
     mockAutoSaveSetting = 'after delay'
     mockAutoSaveDelay = 1000
-    mockActiveWorkflow = { isModified: true }
+    mockActiveWorkflow = { isModified: true, isPersisted: true }
 
     const consoleErrorSpy = vi
       .spyOn(console, 'error')
@@ -197,7 +198,7 @@ describe('useWorkflowAutoSave', () => {
   it('should queue autosave requests during saving and reschedule after save completes', async () => {
     mockAutoSaveSetting = 'after delay'
     mockAutoSaveDelay = 1000
-    mockActiveWorkflow = { isModified: true }
+    mockActiveWorkflow = { isModified: true, isPersisted: true }
 
     mount({
       template: `<div></div>`,
@@ -246,7 +247,7 @@ describe('useWorkflowAutoSave', () => {
   it('should handle edge case delay values properly', async () => {
     mockAutoSaveSetting = 'after delay'
     mockAutoSaveDelay = 0
-    mockActiveWorkflow = { isModified: true }
+    mockActiveWorkflow = { isModified: true, isPersisted: true }
 
     mount({
       template: `<div></div>`,
@@ -271,4 +272,25 @@ describe('useWorkflowAutoSave', () => {
 
     expect(serviceInstance.saveWorkflow).toHaveBeenCalledTimes(1)
   })
+
+  it('should not autosave if workflow is not persisted', async () => {
+    mockAutoSaveSetting = 'after delay'
+    mockAutoSaveDelay = 1000
+    mockActiveWorkflow = { isModified: true, isPersisted: false }
+
+    mount({
+      template: `<div></div>`,
+      setup() {
+        useWorkflowAutoSave()
+        return {}
+      }
+    })
+
+    vi.advanceTimersByTime(1000)
+
+    const serviceInstance = (useWorkflowService as any).mock.results[0].value
+    expect(serviceInstance.saveWorkflow).not.toHaveBeenCalledWith(
+      mockActiveWorkflow
+    )
+  })
 })