Merge pull request cylc#1786 from MetRonnie/n-win

markgrahamdawson · web-flow · commit affdd8ab73b3 · 2024-05-09T15:21:47.000+01:00
Fix N-window selector sending mutation upon navigation to stopped workflows
diff --git a/src/components/cylc/workflow/Toolbar.vue b/src/components/cylc/workflow/Toolbar.vue
@@ -89,21 +89,35 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
         :disabled="isStopped"
         link
         size="small"
+        data-cy="n-win-selector"
       >
         N={{ nWindow }}
-        <v-menu activator="parent" :close-on-content-click="false">
+        <v-menu
+          activator="parent"
+          :close-on-content-click="false"
+          max-width="400"
+          data-cy="n-win-popup"
+        >
           <v-card title="Graph Window Depth">
-            <v-card-subtitle>
+            <v-card-text>
               This changes the number of tasks which are displayed.
 
               Higher values may impact performance.
-            </v-card-subtitle>
+            </v-card-text>
             <v-card-text>
               <v-select
-                density="compact"
                 v-model="nWindow"
                 :items="[0,1,2,3]"
-              />
+              >
+                <template #append-inner>
+                  <v-progress-circular
+                    v-if="changingNWindow"
+                    indeterminate
+                    size="20"
+                    width="2"
+                  />
+                </template>
+              </v-select>
             </v-card-text>
           </v-card>
         </v-menu>
@@ -198,6 +212,7 @@ import {
   mdiAccount
 } from '@mdi/js'
 import { startCase } from 'lodash'
+import { until } from '@/utils'
 import { useToolbar, toolbarHeight } from '@/utils/toolbar'
 import WorkflowState from '@/model/WorkflowState.model'
 import graphql from '@/mixins/graphql'
@@ -279,7 +294,7 @@ export default {
       paused: null,
       stop: null
     },
-    nWindow: 1
+    changingNWindow: false,
   }),
 
   computed: {
@@ -357,9 +372,20 @@ export default {
         )
       }
     },
-    nEdgeDistance () {
-      // the graph window distance reported by the scheduler
-      return this.currentWorkflow?.node?.nEdgeDistance
+    nWindow: {
+      get () {
+        // the graph window distance reported by the scheduler
+        return this.currentWorkflow?.node?.nEdgeDistance ?? 1
+      },
+      async set (val) {
+        if (val == null || this.isStopped) return
+
+        this.changingNWindow = true
+        if (await this.setGraphWindow(val)) {
+          await until(() => this.currentWorkflow?.node?.nEdgeDistance === val)
+        }
+        this.changingNWindow = false
+      },
     }
   },
 
@@ -373,16 +399,6 @@ export default {
     isStopped () {
       this.expecting.stop = null
     },
-    nWindow (newVal) {
-      // the user has requested to change the window size
-      this.setGraphWindow(newVal)
-    },
-    nEdgeDistance (newVal) {
-      // the scheduler has reported that the window size has changed
-      if (newVal !== undefined) {
-        this.nWindow = newVal
-      }
-    }
   },
 
   methods: {
@@ -416,12 +432,13 @@ export default {
         }
       })
     },
-    setGraphWindow (nWindow) {
-      this.$workflowService.mutate(
+    async setGraphWindow (nWindow) {
+      const { status } = await this.$workflowService.mutate(
         'setGraphWindowExtent',
         this.currentWorkflow.id,
         { nEdgeDistance: nWindow }
       )
+      return status === mutationStatus.SUCCEEDED
     },
     startCase,
   },
diff --git a/src/utils/index.js b/src/utils/index.js
@@ -38,15 +38,30 @@ export const getPageTitle = (key, params = {}) => {
  * @param {import('vue').WatchCallback} callback
  * @param {import('vue').WatchOptions?} options
  */
-export const when = (source, callback, options = {}) => {
+export function when (source, callback, options = {}) {
   const unwatch = watch(
     source,
     (ready) => {
       if (ready) {
-        callback()
         unwatch()
+        callback()
       }
     },
     { immediate: true, ...options }
   )
 }
+
+/**
+ * Return a promise that resolves when the source becomes truthy.
+ *
+ * Awaitable version of when().
+ *
+ * @param {import('vue').WatchSource} source
+ * @param {import('vue').WatchOptions?} options
+ * @returns {Promise<void>}
+ */
+export function until (source, options = {}) {
+  return new Promise((resolve) => {
+    when(source, resolve, options)
+  })
+}
diff --git a/tests/e2e/specs/toolbar.cy.js b/tests/e2e/specs/toolbar.cy.js
@@ -15,6 +15,9 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+import { mutationStatus } from '@/utils/aotf'
+import { Deferred } from '$tests/util'
+
 describe('Toolbar component', () => {
   it('Is displayed when we are looking at a workflow', () => {
     cy.visit('/#/workspace/one')
@@ -63,3 +66,68 @@ describe('Toolbar Component authenticated user', () => {
       .should('have.text', 'U')
   })
 })
+
+describe('N-window selector', () => {
+  /** Capture mutations. */
+  function captureGraphWinMutation () {
+    const mutations = []
+    const deferred = new Deferred()
+    cy.window().its('app.$workflowService').then((service) => {
+      // mock the workflow service's mutate method to catch high-level calls
+      service.mutate = async (name, id, args) => {
+        switch (name) {
+          case 'setGraphWindowExtent':
+            mutations.push(args.nEdgeDistance)
+            await deferred.promise
+            return [mutationStatus.SUCCEEDED, '']
+          default:
+            throw new Error('This mutation is not mocked by the tests')
+        }
+      }
+    })
+    return { mutations, deferred }
+  }
+
+  it('Is enabled for workflows that are not stopped', () => {
+    // Disabled for stopped workflow:
+    cy.visit('/#/workspace/multi/level/run1')
+      .get('#core-app-bar')
+      .find('[data-cy=n-win-selector]')
+      .should('have.attr', 'disabled')
+    // Enabled for running workflow:
+    cy.visit('/#/workspace/one')
+      .get('[data-cy=n-win-selector]')
+      .click()
+      .get('[data-cy=n-win-popup]')
+      .find('input')
+      .invoke('val')
+      .should('eq', '1')
+  })
+
+  it('Sets the n-window', () => {
+    cy.visit('/#/workspace/one')
+    const { mutations, deferred } = captureGraphWinMutation()
+    cy.get('[data-cy=n-win-selector]')
+      .click()
+      .get('[data-cy=n-win-popup]')
+      .find('[role=combobox]')
+      .click()
+      .invoke('attr', 'aria-owns').then((dropdownID) => {
+        cy.get(`#${dropdownID}`)
+          .contains('3')
+          .click()
+          .then(() => {
+            expect(mutations).to.deep.equal([3])
+            // Should show loading spinner while waiting for response
+            cy.get('[data-cy=n-win-popup] .v-progress-circular').as('loadingSpinner')
+              .should('be.visible').then(() => {
+                // Allow the "response" to complete
+                deferred.resolve()
+                cy.get('@loadingSpinner')
+                  .should('not.exist')
+                // Note we cannot mock websockets in Cypress so we cannot test the actual update of the n-window in the GraphQL subscription
+              })
+          })
+      })
+  })
+})
diff --git a/tests/unit/utils/index.spec.js b/tests/unit/utils/index.spec.js
@@ -16,19 +16,27 @@
  */
 
 import { nextTick, ref } from 'vue'
-import { getPageTitle, when } from '@/utils/index'
+import { getPageTitle, when, until } from '@/utils/index'
 
 describe('getPageTitle()', () => {
   it('displays the application title correctly', () => {
     expect(getPageTitle('App.dashboard')).to.equal('Cylc UI | Dashboard')
   })
 })
 
-describe('when()', () => {
-  it('watches source until true and then stops watching', async () => {
+describe.each([
+  { func: when, description: 'watches source until true and then stops watching' },
+  { func: until, description: 'returns a promise that resolves when source becomes true' },
+])('$func', ({ func, description }) => {
+  it(description, async () => {
     const source = ref(false)
     let counter = 0
-    when(source, () => counter++)
+    switch (func) {
+      case when:
+        when(source, () => counter++); break
+      case until:
+        until(source).then(() => counter++); break
+    }
     expect(counter).toEqual(0)
     source.value = true
     await nextTick()
