From 68f8411f06eb6564a27d67dea99a54f536babad3 Mon Sep 17 00:00:00 2001
From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
Date: Fri, 21 Oct 2022 17:25:23 +0100
Subject: [PATCH 1/4] Use logical order for task state filter dropdown

Simplify filtering code
---
 src/components/cylc/tree/Tree.vue             |  66 ++++--------
 tests/e2e/specs/tree.cy.js                    |  36 ++-----
 .../components/cylc/tree/tree.vue.spec.js     | 102 +-----------------
 3 files changed, 35 insertions(+), 169 deletions(-)

diff --git a/src/components/cylc/tree/Tree.vue b/src/components/cylc/tree/Tree.vue
index 1b4bd35dc..f63e7cf7b 100644
--- a/src/components/cylc/tree/Tree.vue
+++ b/src/components/cylc/tree/Tree.vue
@@ -70,19 +70,19 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
               @change="filterTasks"
             >
               <template v-slot:item="slotProps">
-                <Task :task="{ state: slotProps.item.value }" />
-                <span class="ml-2">{{ slotProps.item.value }}</span>
+                <Task :task="{ state: slotProps.item }" />
+                <span class="ml-2">{{ slotProps.item }}</span>
               </template>
               <template v-slot:selection="slotProps">
                 <div class="mr-2" v-if="slotProps.index >= 0 && slotProps.index < maximumTasks">
-                  <Task :task="{ state: slotProps.item.value }" />
+                  <Task :task="{ state: slotProps.item }" />
                 </div>
                 <span
                   v-if="slotProps.index === maximumTasks"
                   class="grey--text caption"
                 >
-            (+{{ tasksFilter.states.length - maximumTasks }})
-          </span>
+                  (+{{ tasksFilter.states.length - maximumTasks }})
+                </span>
               </template>
             </v-select>
           </v-col>
@@ -159,9 +159,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 <script>
 import Vue from 'vue'
-import cloneDeep from 'lodash/cloneDeep'
 import { mdiPlus, mdiMinus } from '@mdi/js'
-import TaskState from '@/model/TaskState.model'
+import { TaskStateUserOrder } from '@/model/TaskState.model'
 import TreeItem from '@/components/cylc/tree/TreeItem'
 import Task from '@/components/cylc/Task'
 import { getNodeChildren } from '@/components/cylc/tree/util'
@@ -226,7 +225,7 @@ export default {
         name: '',
         states: []
       },
-      activeFilters: null,
+      taskStates: TaskStateUserOrder.map(ts => ts.name),
       maximumTasks: 4,
       svgPaths: {
         expandIcon: mdiPlus,
@@ -263,27 +262,18 @@ export default {
         return this.workflows
       }
     },
-    taskStates: () => {
-      return TaskState.enumValues.map(taskState => {
-        return {
-          text: taskState.name.replace(/_/g, ' '),
-          value: taskState.name
-        }
-      }).sort((left, right) => {
-        return left.text.localeCompare(right.text)
-      })
+    filterByTaskName () {
+      return Boolean(this.tasksFilter.name?.trim())
     },
-    tasksFilterStates: function () {
-      return this.activeFilters.states.map(selectedTaskState => {
-        return selectedTaskState
-      })
+    filterByTaskState () {
+      return Boolean(this.tasksFilter.states?.length)
     }
   },
   watch: {
     workflows: {
       deep: true,
       handler: function () {
-        if (this.activeFilters !== null) {
+        if (this.filterByTaskName || this.filterByTaskState) {
           this.$nextTick(() => {
             this.filterNodes(this.workflows)
           })
@@ -292,29 +282,11 @@ export default {
     }
   },
   methods: {
-    filterByTaskName () {
-      return this.activeFilters.name !== undefined &&
-          this.activeFilters.name !== null &&
-          this.activeFilters.name.trim() !== ''
-    },
-    filterByTaskState () {
-      return this.activeFilters.states !== undefined &&
-          this.activeFilters.states !== null &&
-          this.activeFilters.states.length > 0
-    },
     filterTasks () {
-      const taskNameFilterSet = this.tasksFilter.name !== undefined &&
-          this.tasksFilter.name !== null &&
-          this.tasksFilter.name.trim() !== ''
-      const taskStatesFilterSet = this.tasksFilter.states !== undefined &&
-          this.tasksFilter.states !== null &&
-          this.tasksFilter.states.length > 0
-      if (taskNameFilterSet || taskStatesFilterSet) {
-        this.activeFilters = cloneDeep(this.tasksFilter)
+      if (this.filterByTaskName || this.filterByTaskState) {
         this.filterNodes(this.workflows)
       } else {
         this.removeAllFilters()
-        this.activeFilters = null
       }
     },
     clearInput (event) {
@@ -340,12 +312,12 @@ export default {
           filtered = this.filterNode(child) || filtered
         }
       } else if (node.type === 'task') {
-        if (this.filterByTaskName() && this.filterByTaskState()) {
-          filtered = node.name.includes(this.activeFilters.name) && this.tasksFilterStates.includes(node.node.state)
-        } else if (this.filterByTaskName()) {
-          filtered = node.name.includes(this.activeFilters.name)
-        } else if (this.filterByTaskState()) {
-          filtered = this.tasksFilterStates.includes(node.node.state)
+        if (this.filterByTaskName && this.filterByTaskState) {
+          filtered = node.name.includes(this.tasksFilter.name) && this.tasksFilter.states.includes(node.node.state)
+        } else if (this.filterByTaskName) {
+          filtered = node.name.includes(this.tasksFilter.name)
+        } else if (this.filterByTaskState) {
+          filtered = this.tasksFilter.states.includes(node.node.state)
         }
       }
       if (!this.treeItemCache[node.id]) {
diff --git a/tests/e2e/specs/tree.cy.js b/tests/e2e/specs/tree.cy.js
index b9f8e8fad..2a67254d8 100644
--- a/tests/e2e/specs/tree.cy.js
+++ b/tests/e2e/specs/tree.cy.js
@@ -183,25 +183,12 @@ describe('Tree view', () => {
   })
 
   describe('filters', () => {
-    it('Should not filter by default', () => {
-      cy.visit('/#/tree/one')
-      cy
-        .get('.node-data-task')
-        .contains('sleepy')
-        .should('be.visible')
-      cy
-        .get('.node-data-task')
-        .contains('waiting')
-        .should('be.visible')
-    })
     it('Should filter by task name', () => {
       cy.visit('/#/tree/one')
+      // Should not filter by default
       cy
-        .get('.node-data-task')
-        .contains('sleepy')
-        .should('be.visible')
-      cy
-        .get('.node-data-task')
+        .get('.node-data-task:visible')
+        .should('have.length', 7)
         .contains('waiting')
         .should('be.visible')
       // eep should filter sleepy
@@ -242,23 +229,21 @@ describe('Tree view', () => {
       cy.visit('/#/tree/one')
       cy
         .get('.node-data-task')
-        .contains('sleepy')
+        .contains('failed')
         .should('be.visible')
-      // retry should filter retry
       cy
         .get('#c-tree-filter-task-name')
-        .type('retry')
+        .type('i')
       cy
         .get('#c-tree-filter-task-states')
         .click({ force: true })
-      // click on waiting, the retry is succeeded, but we don't want to see it
-      cy
         .get('.v-list-item')
         .contains(TaskState.WAITING.name)
         .click({ force: true })
       cy
         .get('.node-data-task:visible')
         .should('have.length', 1)
+        .contains('retrying')
     })
 
     it('should show a summary of tasks if the number of selected items is greater than the maximum limit', () => {
@@ -268,13 +253,14 @@ describe('Tree view', () => {
         .click({ force: true })
       // eslint-disable-next-line no-lone-blocks
       TaskState.enumValues.forEach(state => {
-        cy
-          .get('.v-list-item')
+        cy.get('.v-list-item')
           .contains(state.name)
           .click({ force: true })
       })
-      cy
-        .get('.v-select__slot')
+      // Click outside to close dropdown
+      cy.get('noscript')
+        .click({ force: true })
+      cy.get('.v-select__slot')
         .should($select => {
           expect($select).to.contain('(+')
         })
diff --git a/tests/unit/components/cylc/tree/tree.vue.spec.js b/tests/unit/components/cylc/tree/tree.vue.spec.js
index 501d763bf..65c1b8c85 100644
--- a/tests/unit/components/cylc/tree/tree.vue.spec.js
+++ b/tests/unit/components/cylc/tree/tree.vue.spec.js
@@ -22,7 +22,6 @@ import Vue from 'vue'
 import Vuetify from 'vuetify/lib'
 import Tree from '@/components/cylc/tree/Tree'
 import { simpleWorkflowTree4Nodes } from './tree.data'
-import TaskState from '@/model/TaskState.model'
 import CylcObjectPlugin from '@/components/cylc/cylcObject/plugin'
 import cloneDeep from 'lodash/cloneDeep'
 
@@ -119,108 +118,17 @@ describe('Tree component', () => {
     })
   })
   describe('Filter', () => {
-    describe('Filter by name', () => {
-      it('should not filter by name by default', () => {
+    describe('Default', () => {
+      it('should not filter by name or state by default', () => {
         const wrapper = mountFunction({
           propsData: {
             workflows: simpleWorkflowTree4Nodes[0].children
           }
         })
-        expect(wrapper.vm.activeFilters).to.equal(null)
-      })
-      it('should filter by name', () => {
-        const wrapper = mountFunction({
-          propsData: {
-            workflows: simpleWorkflowTree4Nodes[0].children
-          },
-          data () {
-            return {
-              tasksFilter: {
-                name: 'foo'
-              }
-            }
-          }
-        })
-        expect(wrapper.vm.tasksFilter.name).to.equal('foo')
-        expect(wrapper.vm.activeFilters).to.equal(null)
-      })
-    })
-    describe('Filter by state', () => {
-      it('should not filter by state by default', () => {
-        const wrapper = mountFunction({
-          propsData: {
-            workflows: simpleWorkflowTree4Nodes[0].children
-          }
-        })
-        expect(wrapper.vm.activeFilters).to.equal(null)
-      })
-      it('should filter by name', () => {
-        const states = [
-          {
-            value: TaskState.EXPIRED.name
-          },
-          {
-            value: TaskState.SUBMIT_FAILED.name
-          }]
-        const wrapper = mountFunction({
-          propsData: {
-            workflows: simpleWorkflowTree4Nodes[0].children
-          },
-          data () {
-            return {
-              activeFilters: {
-                states
-              }
-            }
-          }
-        })
-        expect(wrapper.vm.activeFilters.states).to.equal(states)
-      })
-    })
-    describe('Enable filters', () => {
-      it('should not have any filters enabled by default', () => {
-        const wrapper = mountFunction({
-          propsData: {
-            workflows: simpleWorkflowTree4Nodes[0].children
-          }
-        })
-        expect(wrapper.vm.activeFilters).to.equal(null)
-      })
-      it('should indicate filters are enabled if filtering by task name', () => {
-        const wrapper = mountFunction({
-          propsData: {
-            workflows: simpleWorkflowTree4Nodes[0].children
-          },
-          data () {
-            return {
-              tasksFilter: {
-                name: 'foo'
-              }
-            }
-          }
-        })
-        wrapper.vm.filterTasks()
-        expect(wrapper.vm.activeFilters.name).to.equal('foo')
-      })
-      it('should indicate filters are enabled if filtering by task states', () => {
-        const states = [
-          TaskState.EXPIRED.name,
-          TaskState.SUBMIT_FAILED.name
-        ]
-        const wrapper = mountFunction({
-          propsData: {
-            workflows: simpleWorkflowTree4Nodes[0].children
-          },
-          data () {
-            return {
-              tasksFilter: {
-                states
-              }
-            }
-          }
+        expect(wrapper.vm.tasksFilter).to.deep.equal({
+          name: '',
+          states: []
         })
-        wrapper.vm.filterTasks()
-        expect(wrapper.vm.activeFilters.states).to.deep.equal(states)
       })
     })
   })

From bd55f9f2f97c0bb1c3e5c270086ad14aea4c344e Mon Sep 17 00:00:00 2001
From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
Date: Mon, 24 Oct 2022 14:45:37 +0100
Subject: [PATCH 2/4] Centralise common Tree/Table view filtering logic

---
 src/components/cylc/common/filter.js | 38 ++++++++++++++++++++++++++++
 src/components/cylc/common/sort.js   |  4 +--
 src/components/cylc/table/Table.vue  | 17 ++-----------
 src/components/cylc/tree/Tree.vue    |  9 ++-----
 4 files changed, 44 insertions(+), 24 deletions(-)
 create mode 100644 src/components/cylc/common/filter.js

diff --git a/src/components/cylc/common/filter.js b/src/components/cylc/common/filter.js
new file mode 100644
index 000000000..e1d76a304
--- /dev/null
+++ b/src/components/cylc/common/filter.js
@@ -0,0 +1,38 @@
+/**
+ * Copyright (C) NIWA & British Crown (Met Office) & Contributors.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/* Logic for filtering tasks. */
+
+/**
+ * Return true if a node has matches the specified name/state filter.
+ *
+ * @export
+ * @param {{ name: string, state: string }} node
+ * @param {?string} name
+ * @param {?string[]} states
+ * @return {boolean}
+ */
+export function matchNode (node, name, states) {
+  let ret = true
+  if (name?.trim()) {
+    ret &&= node.name.includes(name)
+  }
+  if (states?.length) {
+    ret &&= states.includes(node.state)
+  }
+  return ret
+}
diff --git a/src/components/cylc/common/sort.js b/src/components/cylc/common/sort.js
index deb82c014..56584c5e4 100644
--- a/src/components/cylc/common/sort.js
+++ b/src/components/cylc/common/sort.js
@@ -19,9 +19,9 @@
  * Declare function used in sortedIndexBy as a comparator.
  *
  * @callback SortedIndexByComparator
- * @param {object} leftObject - left parameter object
+ * @param {Object} leftObject - left parameter object
  * @param {string} leftValue - left parameter value
- * @param {object} rightObject - right parameter object
+ * @param {Object} rightObject - right parameter object
  * @param {string} rightValue - right parameter value
  * @returns {boolean} - true if leftValue is higher than rightValue
  */
diff --git a/src/components/cylc/table/Table.vue b/src/components/cylc/table/Table.vue
index b3a8709e4..0132cefd6 100644
--- a/src/components/cylc/table/Table.vue
+++ b/src/components/cylc/table/Table.vue
@@ -224,6 +224,7 @@ import cloneDeep from 'lodash/cloneDeep'
 import { mdiChevronDown, mdiArrowDown } from '@mdi/js'
 import { DEFAULT_COMPARATOR } from '@/components/cylc/common/sort'
 import { datetimeComparator } from '@/components/cylc/table/sort'
+import { matchNode } from '@/components/cylc/common/filter'
 
 export default {
   name: 'TableComponent',
@@ -325,21 +326,7 @@ export default {
       return this.activeFilters.states
     },
     filteredTasks () {
-      const filterByName = this.filterByTaskName()
-      const filterByState = this.filterByTaskState()
-      return this.tasks.filter(task => {
-        if (filterByName && filterByState) {
-          return (
-            task.task.name.includes(this.activeFilters.name) &&
-            this.tasksFilterStates.includes(task.task.node.state)
-          )
-        } else if (filterByName) {
-          return task.task.name.includes(this.activeFilters.name)
-        } else if (filterByState) {
-          return this.tasksFilterStates.includes(task.task.node.state)
-        }
-        return true
-      })
+      return this.tasks.filter(({ task }) => matchNode(task.node, this.tasksFilter.name, this.tasksFilter.states))
     }
   },
   methods: {
diff --git a/src/components/cylc/tree/Tree.vue b/src/components/cylc/tree/Tree.vue
index f63e7cf7b..63924f8ab 100644
--- a/src/components/cylc/tree/Tree.vue
+++ b/src/components/cylc/tree/Tree.vue
@@ -163,6 +163,7 @@ import { mdiPlus, mdiMinus } from '@mdi/js'
 import { TaskStateUserOrder } from '@/model/TaskState.model'
 import TreeItem from '@/components/cylc/tree/TreeItem'
 import Task from '@/components/cylc/Task'
+import { matchNode } from '@/components/cylc/common/filter'
 import { getNodeChildren } from '@/components/cylc/tree/util'
 
 export default {
@@ -312,13 +313,7 @@ export default {
           filtered = this.filterNode(child) || filtered
         }
       } else if (node.type === 'task') {
-        if (this.filterByTaskName && this.filterByTaskState) {
-          filtered = node.name.includes(this.tasksFilter.name) && this.tasksFilter.states.includes(node.node.state)
-        } else if (this.filterByTaskName) {
-          filtered = node.name.includes(this.tasksFilter.name)
-        } else if (this.filterByTaskState) {
-          filtered = this.tasksFilter.states.includes(node.node.state)
-        }
+        filtered = matchNode(node.node, this.tasksFilter.name, this.tasksFilter.states)
       }
       if (!this.treeItemCache[node.id]) {
         this.treeItemCache[node.id] = {}

From 6263298ba7d8a22b59acd42ae75e7b228ab07006 Mon Sep 17 00:00:00 2001
From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
Date: Tue, 25 Oct 2022 10:11:06 +0100
Subject: [PATCH 3/4] Move task filter controls into own component

---
 src/components/cylc/TaskFilter.vue            | 106 ++++++++++++++++
 src/components/cylc/table/Table.vue           | 115 +-----------------
 src/components/cylc/tree/Tree.vue             |  82 ++-----------
 tests/e2e/specs/table.cy.js                   |  12 +-
 tests/e2e/specs/tree.cy.js                    |  23 ++--
 .../components/cylc/table/table.vue.spec.js   |   9 --
 .../components/cylc/tree/tree.vue.spec.js     |   5 +-
 7 files changed, 141 insertions(+), 211 deletions(-)
 create mode 100644 src/components/cylc/TaskFilter.vue

diff --git a/src/components/cylc/TaskFilter.vue b/src/components/cylc/TaskFilter.vue
new file mode 100644
index 000000000..13621c492
--- /dev/null
+++ b/src/components/cylc/TaskFilter.vue
@@ -0,0 +1,106 @@
+<!--
+Copyright (C) NIWA & British Crown (Met Office) & Contributors.
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+-->
+
+<!-- Controls for filtering tasks in views. -->
+
+<template>
+  <v-row no-gutters>
+    <v-col
+      cols="12"
+      md="6"
+      class="pr-md-2 mb-2 mb-md-0"
+    >
+      <v-text-field
+        data-cy="filter-task-name"
+        clearable
+        dense
+        flat
+        hide-details
+        outlined
+        placeholder="Filter by task name"
+        v-model="localValue.name"
+        ref="filterNameInput"
+      ></v-text-field>
+    </v-col>
+    <v-col
+      cols="12"
+      md="6"
+      class="mb-2 mb-md-0"
+    >
+      <v-select
+        data-cy="filter-task-states"
+        :items="allStates"
+        clearable
+        dense
+        flat
+        hide-details
+        multiple
+        outlined
+        placeholder="Filter by task state"
+        v-model="localValue.states"
+      >
+        <template v-slot:item="slotProps">
+          <Task :task="{ state: slotProps.item }" />
+          <span class="ml-2">{{ slotProps.item }}</span>
+        </template>
+        <template v-slot:selection="slotProps">
+          <div class="mr-2" v-if="slotProps.index >= 0 && slotProps.index < maxVisibleStates">
+            <Task :task="{ state: slotProps.item }" />
+          </div>
+          <span
+            v-if="slotProps.index === maxVisibleStates"
+            class="grey--text caption"
+          >
+            (+{{ localValue.states.length - maxVisibleStates }})
+          </span>
+        </template>
+      </v-select>
+    </v-col>
+  </v-row>
+</template>
+
+<script>
+import Task from '@/components/cylc/Task'
+import { TaskStateUserOrder } from '@/model/TaskState.model'
+
+export default {
+  name: 'TaskFilter',
+  components: {
+    Task
+  },
+  props: {
+    value: Object // { name, states }
+  },
+  data () {
+    return {
+      maxVisibleStates: 4,
+      allStates: TaskStateUserOrder.map(ts => ts.name)
+    }
+  },
+  computed: {
+    localValue: {
+      get () {
+        return this.value
+      },
+      set (value) {
+        // Update 'value' prop by notifying parent component's v-model for this component
+        this.$emit('input', value)
+      }
+    }
+  }
+}
+</script>
diff --git a/src/components/cylc/table/Table.vue b/src/components/cylc/table/Table.vue
index 0132cefd6..7714e0b86 100644
--- a/src/components/cylc/table/Table.vue
+++ b/src/components/cylc/table/Table.vue
@@ -29,62 +29,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
         v-if="filterable"
         class=""
       >
-        <v-row class="no-gutters">
-          <v-col
-            cols="12"
-            md="6"
-            class="pr-md-2 mb-2 mb-md-0"
-          >
-            <v-text-field
-              id="c-table-filter-task-name"
-              clearable
-              dense
-              flat
-              hide-details
-              outlined
-              placeholder="Filter by task name"
-              v-model.trim="tasksFilter.name"
-              @keyup="filterTasks"
-              @click:clear="clearInput"
-              ref="filterNameInput"
-            ></v-text-field>
-          </v-col>
-          <v-col
-            cols="12"
-            md="6"
-            class="mb-2 mb-md-0"
-          >
-            <v-select
-              id="c-table-filter-task-states"
-              :items="taskStates"
-              clearable
-              dense
-              flat
-              hide-details
-              multiple
-              outlined
-              placeholder="Filter by task state"
-              v-model="tasksFilter.states"
-              @change="filterTasks"
-            >
-              <template v-slot:item="slotProps">
-                <Task :task="{ state: slotProps.item.value }" />
-                <span class="ml-2">{{ slotProps.item.value }}</span>
-              </template>
-              <template v-slot:selection="slotProps">
-                <div class="mr-2" v-if="slotProps.index >= 0 && slotProps.index < maximumTasks">
-                  <Task :task="{ state: slotProps.item.value }" />
-                </div>
-                <span
-                  v-if="slotProps.index === maximumTasks"
-                  class="grey--text caption"
-                >
-                  (+{{ tasksFilter.states.length - maximumTasks }})
-                </span>
-              </template>
-            </v-select>
-          </v-col>
-        </v-row>
+        <TaskFilter v-model="tasksFilter"/>
       </v-col>
     </v-row>
     <v-row
@@ -217,14 +162,13 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 </template>
 
 <script>
-import TaskState from '@/model/TaskState.model'
 import Task from '@/components/cylc/Task'
 import Job from '@/components/cylc/Job'
-import cloneDeep from 'lodash/cloneDeep'
 import { mdiChevronDown, mdiArrowDown } from '@mdi/js'
 import { DEFAULT_COMPARATOR } from '@/components/cylc/common/sort'
 import { datetimeComparator } from '@/components/cylc/table/sort'
 import { matchNode } from '@/components/cylc/common/filter'
+import TaskFilter from '@/components/cylc/TaskFilter.vue'
 
 export default {
   name: 'TableComponent',
@@ -240,7 +184,8 @@ export default {
   },
   components: {
     Task,
-    Job
+    Job,
+    TaskFilter
   },
   data () {
     return {
@@ -303,63 +248,13 @@ export default {
           sort: (a, b) => parseInt(a ?? 0) - parseInt(b ?? 0)
         }
       ],
-      tasksFilter: {
-        name: '',
-        states: []
-      },
-      activeFilters: null,
-      maximumTasks: 4
+      tasksFilter: {}
     }
   },
   computed: {
-    taskStates () {
-      return TaskState.enumValues.map(taskState => {
-        return {
-          text: taskState.name.replace(/_/g, ' '),
-          value: taskState.name
-        }
-      }).sort((left, right) => {
-        return left.text.localeCompare(right.text)
-      })
-    },
-    tasksFilterStates () {
-      return this.activeFilters.states
-    },
     filteredTasks () {
       return this.tasks.filter(({ task }) => matchNode(task.node, this.tasksFilter.name, this.tasksFilter.states))
     }
-  },
-  methods: {
-    filterByTaskName () {
-      return this.activeFilters &&
-        this.activeFilters.name !== undefined &&
-        this.activeFilters.name !== null &&
-        this.activeFilters.name !== ''
-    },
-    filterByTaskState () {
-      return this.activeFilters &&
-        this.activeFilters.states !== undefined &&
-        this.activeFilters.states !== null &&
-        this.activeFilters.states.length > 0
-    },
-    filterTasks () {
-      const taskNameFilterSet = this.tasksFilter.name !== undefined &&
-        this.tasksFilter.name !== null &&
-        this.tasksFilter.name !== ''
-      const taskStatesFilterSet = this.tasksFilter.states !== undefined &&
-        this.tasksFilter.states !== null &&
-        this.tasksFilter.states.length > 0
-      if (taskNameFilterSet || taskStatesFilterSet) {
-        this.activeFilters = cloneDeep(this.tasksFilter)
-      } else {
-        this.activeFilters = null
-      }
-    },
-    clearInput (event) {
-      // I don't really like this, but we need to somehow force the 'change detection' to run again once the clear has taken place
-      this.tasksFilter.name = null
-      this.$refs.filterNameInput.$el.querySelector('input').dispatchEvent(new Event('keyup'))
-    }
   }
 }
 </script>
diff --git a/src/components/cylc/tree/Tree.vue b/src/components/cylc/tree/Tree.vue
index 63924f8ab..871df4852 100644
--- a/src/components/cylc/tree/Tree.vue
+++ b/src/components/cylc/tree/Tree.vue
@@ -29,64 +29,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
         v-if="filterable"
         class="grow"
       >
-        <v-row
-          no-gutters
-        >
-          <v-col
-            cols="12"
-            md="6"
-            class="pr-md-2 mb-2 mb-md-0"
-          >
-            <v-text-field
-              id="c-tree-filter-task-name"
-              clearable
-              dense
-              flat
-              hide-details
-              outlined
-              placeholder="Filter by task name"
-              v-model="tasksFilter.name"
-              @keyup="filterTasks"
-              @click:clear="clearInput"
-              ref="filterNameInput"
-            ></v-text-field>
-          </v-col>
-          <v-col
-            cols="12"
-            md="6"
-            class="mb-2 mb-md-0"
-          >
-            <v-select
-              id="c-tree-filter-task-states"
-              :items="taskStates"
-              clearable
-              dense
-              flat
-              hide-details
-              multiple
-              outlined
-              placeholder="Filter by task state"
-              v-model="tasksFilter.states"
-              @change="filterTasks"
-            >
-              <template v-slot:item="slotProps">
-                <Task :task="{ state: slotProps.item }" />
-                <span class="ml-2">{{ slotProps.item }}</span>
-              </template>
-              <template v-slot:selection="slotProps">
-                <div class="mr-2" v-if="slotProps.index >= 0 && slotProps.index < maximumTasks">
-                  <Task :task="{ state: slotProps.item }" />
-                </div>
-                <span
-                  v-if="slotProps.index === maximumTasks"
-                  class="grey--text caption"
-                >
-                  (+{{ tasksFilter.states.length - maximumTasks }})
-                </span>
-              </template>
-            </v-select>
-          </v-col>
-        </v-row>
+        <TaskFilter v-model="tasksFilter"/>
       </v-col>
       <!-- Expand, collapse all -->
       <v-col
@@ -160,9 +103,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 <script>
 import Vue from 'vue'
 import { mdiPlus, mdiMinus } from '@mdi/js'
-import { TaskStateUserOrder } from '@/model/TaskState.model'
 import TreeItem from '@/components/cylc/tree/TreeItem'
-import Task from '@/components/cylc/Task'
+import TaskFilter from '@/components/cylc/TaskFilter'
 import { matchNode } from '@/components/cylc/common/filter'
 import { getNodeChildren } from '@/components/cylc/tree/util'
 
@@ -211,7 +153,7 @@ export default {
     }
   },
   components: {
-    Task,
+    TaskFilter,
     TreeItem
   },
   data () {
@@ -222,12 +164,7 @@ export default {
       expanded: true,
       expandedFilter: null,
       collapseFilter: null,
-      tasksFilter: {
-        name: '',
-        states: []
-      },
-      taskStates: TaskStateUserOrder.map(ts => ts.name),
-      maximumTasks: 4,
+      tasksFilter: {},
       svgPaths: {
         expandIcon: mdiPlus,
         collapseIcon: mdiMinus
@@ -271,9 +208,13 @@ export default {
     }
   },
   watch: {
+    tasksFilter: {
+      deep: true,
+      handler: 'filterTasks'
+    },
     workflows: {
       deep: true,
-      handler: function () {
+      handler () {
         if (this.filterByTaskName || this.filterByTaskState) {
           this.$nextTick(() => {
             this.filterNodes(this.workflows)
@@ -290,11 +231,6 @@ export default {
         this.removeAllFilters()
       }
     },
-    clearInput (event) {
-      // I don't really like this, but we need to somehow force the 'change detection' to run again once the clear has taken place
-      this.tasksFilter.name = null
-      this.$refs.filterNameInput.$el.querySelector('input').dispatchEvent(new Event('keyup'))
-    },
     filterNodes (nodes) {
       for (const node of nodes) {
         this.filterNode(node)
diff --git a/tests/e2e/specs/table.cy.js b/tests/e2e/specs/table.cy.js
index ea9f6820c..4435c76b5 100644
--- a/tests/e2e/specs/table.cy.js
+++ b/tests/e2e/specs/table.cy.js
@@ -33,10 +33,10 @@ describe('Table view', () => {
         .should('have.length', 7)
         .should('be.visible')
       cy
-        .get('#c-table-filter-task-name')
+        .get('[data-cy=filter-task-name]')
         .should('be.empty')
       cy
-        .get('#c-table-filter-task-states')
+        .get('[data-cy=filter-task-states]')
         .parent()
         .parent()
         .find('input[type="hidden"]')
@@ -54,7 +54,7 @@ describe('Table view', () => {
         .should('be.visible')
       // eep should filter sleepy
       cy
-        .get('#c-table-filter-task-name')
+        .get('[data-cy=filter-task-name]')
         .type('eep')
       cy
         .get('td > div.d-flex > div')
@@ -76,7 +76,7 @@ describe('Table view', () => {
         .contains(TaskState.FAILED.name)
         .should('be.visible')
       cy
-        .get('#c-table-filter-task-states')
+        .get('[data-cy=filter-task-states]')
         .click({ force: true })
       cy
         .get('.v-list-item')
@@ -98,7 +98,7 @@ describe('Table view', () => {
         .should('have.length', 7)
         .should('be.visible')
       cy
-        .get('#c-table-filter-task-states')
+        .get('[data-cy=filter-task-states]')
         .click({ force: true })
       cy
         .get('.v-list-item')
@@ -109,7 +109,7 @@ describe('Table view', () => {
         .should('have.length', 2)
         .should('be.visible')
       cy
-        .get('#c-table-filter-task-name')
+        .get('[data-cy=filter-task-name]')
         .type('eventually')
       cy
         .get('td > div.d-flex > div')
diff --git a/tests/e2e/specs/tree.cy.js b/tests/e2e/specs/tree.cy.js
index 2a67254d8..6d48abd46 100644
--- a/tests/e2e/specs/tree.cy.js
+++ b/tests/e2e/specs/tree.cy.js
@@ -183,26 +183,31 @@ describe('Tree view', () => {
   })
 
   describe('filters', () => {
+    const initialNumTasks = 7
     it('Should filter by task name', () => {
       cy.visit('/#/tree/one')
       // Should not filter by default
       cy
         .get('.node-data-task:visible')
-        .should('have.length', 7)
+        .should('have.length', initialNumTasks)
         .contains('waiting')
-        .should('be.visible')
       // eep should filter sleepy
       cy
-        .get('#c-tree-filter-task-name')
+        .get('[data-cy=filter-task-name]')
         .type('eep')
       cy
-        .get('.node-data-task')
+        .get('.node-data-task:visible')
+        .should('have.length.lessThan', initialNumTasks)
         .contains('sleepy')
-        .should('be.visible')
       cy
         .get('.node-data-task')
         .contains('waiting')
         .should('not.be.visible')
+      // It should stop filtering when input is cleared
+      cy.get('[data-cy=filter-task-name]')
+        .clear()
+        .get('.node-data-task:visible')
+        .should('have.length', initialNumTasks)
     })
     it('Should filter by task states', () => {
       cy.visit('/#/tree/one')
@@ -211,7 +216,7 @@ describe('Tree view', () => {
         .contains(TaskState.FAILED.name)
         .should('be.visible')
       cy
-        .get('#c-tree-filter-task-states')
+        .get('[data-cy=filter-task-states]')
         .click({ force: true })
       cy
         .get('.v-list-item')
@@ -232,10 +237,10 @@ describe('Tree view', () => {
         .contains('failed')
         .should('be.visible')
       cy
-        .get('#c-tree-filter-task-name')
+        .get('[data-cy=filter-task-name]')
         .type('i')
       cy
-        .get('#c-tree-filter-task-states')
+        .get('[data-cy=filter-task-states]')
         .click({ force: true })
         .get('.v-list-item')
         .contains(TaskState.WAITING.name)
@@ -249,7 +254,7 @@ describe('Tree view', () => {
     it('should show a summary of tasks if the number of selected items is greater than the maximum limit', () => {
       cy.visit('/#/tree/one')
       cy
-        .get('#c-tree-filter-task-states')
+        .get('[data-cy=filter-task-states]')
         .click({ force: true })
       // eslint-disable-next-line no-lone-blocks
       TaskState.enumValues.forEach(state => {
diff --git a/tests/unit/components/cylc/table/table.vue.spec.js b/tests/unit/components/cylc/table/table.vue.spec.js
index db3dbb22c..8f8047988 100644
--- a/tests/unit/components/cylc/table/table.vue.spec.js
+++ b/tests/unit/components/cylc/table/table.vue.spec.js
@@ -107,9 +107,6 @@ describe('Table component', () => {
             tasks: simpleTableTasks
           }
         })
-        expect(wrapper.vm.activeFilters).to.equal(null)
-        expect(wrapper.vm.filteredTasks.length).to.equal(3)
-        wrapper.vm.filterTasks()
         expect(wrapper.vm.filteredTasks.length).to.equal(3)
       })
       it('should filter by name', () => {
@@ -125,8 +122,6 @@ describe('Table component', () => {
             }
           }
         })
-        expect(wrapper.vm.filteredTasks.length).to.equal(3)
-        wrapper.vm.filterTasks()
         expect(wrapper.vm.filteredTasks.length).to.equal(1)
       })
       it('should filter by task state', () => {
@@ -145,8 +140,6 @@ describe('Table component', () => {
             }
           }
         })
-        expect(wrapper.vm.filteredTasks.length).to.equal(3)
-        wrapper.vm.filterTasks()
         expect(wrapper.vm.filteredTasks.length).to.equal(1)
       })
       it('should filter by task name and state', () => {
@@ -165,8 +158,6 @@ describe('Table component', () => {
             }
           }
         })
-        expect(wrapper.vm.filteredTasks.length).to.equal(3)
-        wrapper.vm.filterTasks()
         expect(wrapper.vm.filteredTasks.length).to.equal(0)
       })
     })
diff --git a/tests/unit/components/cylc/tree/tree.vue.spec.js b/tests/unit/components/cylc/tree/tree.vue.spec.js
index 65c1b8c85..5542251d5 100644
--- a/tests/unit/components/cylc/tree/tree.vue.spec.js
+++ b/tests/unit/components/cylc/tree/tree.vue.spec.js
@@ -125,10 +125,7 @@ describe('Tree component', () => {
             workflows: simpleWorkflowTree4Nodes[0].children
           }
         })
-        expect(wrapper.vm.tasksFilter).to.deep.equal({
-          name: '',
-          states: []
-        })
+        expect(wrapper.vm.tasksFilter).to.deep.equal({})
       })
     })
   })

From 810075beae1ab175b0cc2dab402777d993eb248f Mon Sep 17 00:00:00 2001
From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
Date: Mon, 19 Dec 2022 13:08:02 +0000
Subject: [PATCH 4/4] Fix bug when expanding/collapsing tree view while filters
 are active

---
 src/components/cylc/tree/Tree.vue |  7 ++--
 tests/e2e/specs/tree.cy.js        | 67 +++++++++++++++++++++++--------
 2 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/src/components/cylc/tree/Tree.vue b/src/components/cylc/tree/Tree.vue
index 871df4852..92dd3633f 100644
--- a/src/components/cylc/tree/Tree.vue
+++ b/src/components/cylc/tree/Tree.vue
@@ -45,6 +45,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
                 v-on="on"
                 @click="expandAll((treeitem) => !['task', 'job', 'job-details'].includes(treeitem.node.type))"
                 icon
+                data-cy="expand-all"
               >
                 <v-icon>{{ svgPaths.expandIcon }}</v-icon>
               </v-btn>
@@ -57,6 +58,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
                 v-on="on"
                 @click="collapseAll()"
                 icon
+                data-cy="collapse-all"
               >
                 <v-icon>{{ svgPaths.collapseIcon }}</v-icon>
               </v-btn>
@@ -251,10 +253,9 @@ export default {
       } else if (node.type === 'task') {
         filtered = matchNode(node.node, this.tasksFilter.name, this.tasksFilter.states)
       }
-      if (!this.treeItemCache[node.id]) {
-        this.treeItemCache[node.id] = {}
+      if (this.treeItemCache[node.id]) {
+        this.treeItemCache[node.id].filtered = filtered
       }
-      this.treeItemCache[node.id].filtered = filtered
       return filtered
     },
     removeAllFilters () {
diff --git a/tests/e2e/specs/tree.cy.js b/tests/e2e/specs/tree.cy.js
index 6d48abd46..4f27be36f 100644
--- a/tests/e2e/specs/tree.cy.js
+++ b/tests/e2e/specs/tree.cy.js
@@ -250,25 +250,60 @@ describe('Tree view', () => {
         .should('have.length', 1)
         .contains('retrying')
     })
+  })
 
-    it('should show a summary of tasks if the number of selected items is greater than the maximum limit', () => {
+  describe('Expand/collapse all buttons', () => {
+    it('Collapses and expands as expected', () => {
       cy.visit('/#/tree/one')
-      cy
-        .get('[data-cy=filter-task-states]')
-        .click({ force: true })
-      // eslint-disable-next-line no-lone-blocks
-      TaskState.enumValues.forEach(state => {
-        cy.get('.v-list-item')
-          .contains(state.name)
-          .click({ force: true })
-      })
-      // Click outside to close dropdown
-      cy.get('noscript')
+      cy.get('.node-data-task')
+        .contains('sleepy')
+        .as('sleepyTask')
+        .should('be.visible')
+      cy.get('[data-cy=collapse-all]')
+        .click()
+        .get('@sleepyTask')
+        .should('not.be.visible')
+        .get('[data-cy=expand-all]')
+        .click()
+        .get('@sleepyTask')
+        .should('be.visible')
+    })
+    it('Works when tasks are being filtered', () => {
+      cy.visit('/#/tree/one')
+      cy.get('.node-data-task')
+        .contains('sleepy')
+        .as('sleepyTask')
+        .should('be.visible')
+      cy.get('[data-cy=filter-task-name]')
+        .type('sleep')
+      cy.get('[data-cy=collapse-all]')
+        .click()
+        .get('@sleepyTask')
+        .should('not.be.visible')
+        .get('[data-cy=expand-all]')
+        .click()
+        .get('@sleepyTask')
+        .should('be.visible')
+    })
+  })
+
+  it('should show a summary of tasks if the number of selected items is greater than the maximum limit', () => {
+    cy.visit('/#/tree/one')
+    cy
+      .get('[data-cy=filter-task-states]')
+      .click({ force: true })
+    // eslint-disable-next-line no-lone-blocks
+    TaskState.enumValues.forEach(state => {
+      cy.get('.v-list-item')
+        .contains(state.name)
         .click({ force: true })
-      cy.get('.v-select__slot')
-        .should($select => {
-          expect($select).to.contain('(+')
-        })
     })
+    // Click outside to close dropdown
+    cy.get('noscript')
+      .click({ force: true })
+    cy.get('.v-select__slot')
+      .should($select => {
+        expect($select).to.contain('(+')
+      })
   })
 })