Table view: sort empty entries last regardless of sort order (cylc#1762)

MetRonnie · web-flow · commit dac2399f7072 · 2024-05-23T11:56:44.000+01:00
diff --git a/src/components/cylc/common/sort.js b/src/components/cylc/common/sort.js
@@ -1,4 +1,4 @@
-/**
+/*
  * Copyright (C) NIWA & British Crown (Met Office) & Contributors.
  *
  * This program is free software: you can redistribute it and/or modify
@@ -36,15 +36,14 @@
  * @constructor
  */
 export const DEFAULT_COMPARATOR = (left, right) => {
-  return left.toLowerCase()
-    .localeCompare(
-      right.toLowerCase(),
-      undefined,
-      {
-        numeric: true,
-        sensitivity: 'base'
-      }
-    )
+  return left.toLowerCase().localeCompare(
+    right.toLowerCase(),
+    undefined,
+    {
+      numeric: true,
+      sensitivity: 'base',
+    }
+  )
 }
 
 /**
diff --git a/src/components/cylc/table/Table.vue b/src/components/cylc/table/Table.vue
@@ -17,7 +17,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 <template>
   <v-data-table
-    :headers="$options.headers"
+    :headers="headers"
     :items="tasks"
     item-value="task.id"
     multi-sort
@@ -62,7 +62,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
         }"
       >
         <v-icon
-          :icon="$options.icons.mdiChevronDown"
+          :icon="icons.mdiChevronDown"
           size="large"
         />
       </v-btn>
@@ -95,17 +95,21 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
       </tr>
     </template>
     <template v-slot:bottom>
-      <v-data-table-footer :itemsPerPageOptions="$options.itemsPerPageOptions" />
+      <v-data-table-footer :itemsPerPageOptions="itemsPerPageOptions" />
     </template>
   </v-data-table>
 </template>
 
 <script>
+import { ref, watch } from 'vue'
 import Task from '@/components/cylc/Task.vue'
 import Job from '@/components/cylc/Job.vue'
 import { mdiChevronDown } from '@mdi/js'
 import { DEFAULT_COMPARATOR } from '@/components/cylc/common/sort'
-import { datetimeComparator } from '@/components/cylc/table/sort'
+import {
+  datetimeComparator,
+  numberComparator,
+} from '@/components/cylc/table/sort'
 import { dtMean } from '@/utils/tasks'
 import { useCyclePointsOrderDesc } from '@/composables/localStorage'
 import {
@@ -150,87 +154,104 @@ export default {
 
     const itemsPerPage = useInitialOptions('itemsPerPage', { props, emit }, 50)
 
+    const headers = ref([
+      {
+        title: 'Task',
+        key: 'task.name',
+        sortable: true,
+        sortFunc: DEFAULT_COMPARATOR
+      },
+      {
+        title: 'Jobs',
+        key: 'data-table-expand',
+        sortable: false
+      },
+      {
+        title: 'Cycle Point',
+        key: 'task.tokens.cycle',
+        sortable: true,
+        sortFunc: DEFAULT_COMPARATOR,
+      },
+      {
+        title: 'Platform',
+        key: 'latestJob.node.platform',
+        sortable: true,
+        sortFunc: DEFAULT_COMPARATOR,
+      },
+      {
+        title: 'Job Runner',
+        key: 'latestJob.node.jobRunnerName',
+        sortable: true,
+        sortFunc: DEFAULT_COMPARATOR,
+      },
+      {
+        title: 'Job ID',
+        key: 'latestJob.node.jobId',
+        sortable: true,
+        sortFunc: DEFAULT_COMPARATOR,
+      },
+      {
+        title: 'Submit',
+        key: 'latestJob.node.submittedTime',
+        sortable: true,
+        sortFunc: datetimeComparator,
+      },
+      {
+        title: 'Start',
+        key: 'latestJob.node.startedTime',
+        sortable: true,
+        sortFunc: datetimeComparator,
+      },
+      {
+        title: 'Finish',
+        key: 'latestJob.node.finishedTime',
+        sortable: true,
+        sortFunc: datetimeComparator,
+      },
+      {
+        title: 'Run Time',
+        key: 'task.node.task.meanElapsedTime',
+        sortable: true,
+        sortFunc: numberComparator,
+      },
+    ])
+
+    // Ensure that whenever a sort order changes, empty/nullish values are
+    // always sorted last regardless.
+    watch(
+      sortBy,
+      (val) => {
+        for (const { key, order } of val) {
+          const header = headers.value.find((x) => x.key === key)
+          header.sort = (a, b) => {
+            if (!a && !b) return 0
+            if (!a) return order === 'asc' ? 1 : -1
+            if (!b) return order === 'asc' ? -1 : 1
+            return header.sortFunc(a, b)
+          }
+        }
+      },
+      { deep: true, immediate: true }
+    )
+
     return {
       dtMean,
       itemsPerPage,
       page,
       sortBy,
+      headers,
+      icons: {
+        mdiChevronDown
+      },
+      itemsPerPageOptions: [
+        { value: 10, title: '10' },
+        { value: 20, title: '20' },
+        { value: 50, title: '50' },
+        { value: 100, title: '100' },
+        { value: 200, title: '200' },
+        { value: -1, title: 'All' },
+      ],
     }
   },
-
-  headers: [
-    {
-      title: 'Task',
-      key: 'task.name',
-      sortable: true,
-      sort: DEFAULT_COMPARATOR
-    },
-    {
-      title: 'Jobs',
-      key: 'data-table-expand',
-      sortable: false
-    },
-    {
-      title: 'Cycle Point',
-      key: 'task.tokens.cycle',
-      sortable: true,
-      sort: (a, b) => DEFAULT_COMPARATOR(String(a ?? ''), String(b ?? ''))
-    },
-    {
-      title: 'Platform',
-      key: 'latestJob.node.platform',
-      sortable: true,
-      sort: (a, b) => DEFAULT_COMPARATOR(a ?? '', b ?? '')
-    },
-    {
-      title: 'Job Runner',
-      key: 'latestJob.node.jobRunnerName',
-      sortable: true,
-      sort: (a, b) => DEFAULT_COMPARATOR(a ?? '', b ?? '')
-    },
-    {
-      title: 'Job ID',
-      key: 'latestJob.node.jobId',
-      sortable: true,
-      sort: (a, b) => DEFAULT_COMPARATOR(a ?? '', b ?? '')
-    },
-    {
-      title: 'Submit',
-      key: 'latestJob.node.submittedTime',
-      sortable: true,
-      sort: (a, b) => datetimeComparator(a ?? '', b ?? '')
-    },
-    {
-      title: 'Start',
-      key: 'latestJob.node.startedTime',
-      sortable: true,
-      sort: (a, b) => datetimeComparator(a ?? '', b ?? '')
-    },
-    {
-      title: 'Finish',
-      key: 'latestJob.node.finishedTime',
-      sortable: true,
-      sort: (a, b) => datetimeComparator(a ?? '', b ?? '')
-    },
-    {
-      title: 'Run Time',
-      key: 'task.node.task.meanElapsedTime',
-      sortable: true,
-      sort: (a, b) => parseInt(a ?? 0) - parseInt(b ?? 0)
-    },
-  ],
-
-  icons: {
-    mdiChevronDown
-  },
-
-  itemsPerPageOptions: [
-    { value: 10, title: '10' },
-    { value: 20, title: '20' },
-    { value: 50, title: '50' },
-    { value: 100, title: '100' },
-    { value: 200, title: '200' },
-    { value: -1, title: 'All' }
-  ]
 }
 </script>
diff --git a/src/components/cylc/table/sort.js b/src/components/cylc/table/sort.js
@@ -1,4 +1,4 @@
-/**
+/*
  * Copyright (C) NIWA & British Crown (Met Office) & Contributors.
  *
  * This program is free software: you can redistribute it and/or modify
@@ -16,17 +16,23 @@
  */
 
 /**
- * Comparator function for sorting datetime strings. Note: nullish or empty
- * strings are treated as infinity.
+ * Comparator function for sorting datetime strings.
  *
- * @export
- * @param {*} a - The first element for comparison.
- * @param {*} b - The second element for comparison.
+ * @param {string} a - The first element for comparison.
+ * @param {string} b - The second element for comparison.
  * @return {number} A number > 0 if a > b, or < 0 if a < b, or 0 if a === b
  */
 export function datetimeComparator (a, b) {
-  a = (a ?? '') === '' ? Infinity : new Date(a).getTime()
-  b = (b ?? '') === '' ? Infinity : new Date(b).getTime()
-  // Avoid return NaN for a === b === Infinity
-  return a === b ? 0 : a - b
+  return new Date(a) - new Date(b)
+}
+
+/**
+ * Comparator function for sorting numbers.
+ *
+ * @param {number} a
+ * @param {number} b
+ * @returns {number}
+ */
+export function numberComparator (a, b) {
+  return a - b
 }
diff --git a/tests/e2e/specs/table.cy.js b/tests/e2e/specs/table.cy.js
@@ -100,26 +100,29 @@ describe('Table view', () => {
         .should('be.visible')
     })
     it('displays and sorts mean run time', () => {
-      cy
-        // sort dt-mean ascending
-        .get('.c-table')
+      // sort dt-mean ascending
+      cy.get('.c-table')
         .contains('th', 'Run Time').as('dTHeader')
         .click()
-
-        // check 0 is at the top (1st row, 10th column)
+        // (1st row, 10th column)
         .get('tbody > :nth-child(1) > :nth-child(10)')
+        .contains('00:00:04')
+        .get('tbody > :nth-child(2) > :nth-child(10)')
+        .contains('00:00:12')
+        .get('tbody > :nth-child(3) > :nth-child(10)')
         .should(($ele) => {
-          expect($ele.text().trim()).equal('') // no value sorted first
+          expect($ele.text().trim()).equal('') // no value sorted after numbers
         })
-
         // sort dt-mean descending
-        .get('@dTHeader')
+      cy.get('@dTHeader')
         .click()
-
-        // check 7 is at the top (1st row, 10th column)
         .get('tbody > :nth-child(1) > :nth-child(10)')
+        .contains('00:00:12')
+        .get('tbody > :nth-child(2) > :nth-child(10)')
+        .contains('00:00:04')
+        .get('tbody > :nth-child(3) > :nth-child(10)')
         .should(($ele) => {
-          expect($ele.text().trim()).equal('00:00:12')
+          expect($ele.text().trim()).equal('') // no value still sorted after numbers
         })
     })
   })
diff --git a/tests/unit/components/cylc/common/sort.spec.js b/tests/unit/components/cylc/common/sort.spec.js
@@ -0,0 +1,33 @@
+/*
+ * 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/>.
+ */
+
+import { DEFAULT_COMPARATOR } from '@/components/cylc/common/sort'
+
+describe('DEFAULT_COMPARATOR', () => {
+  it.each([
+    ['a', 'b', -1],
+    ['b', 'a', 1],
+    ['a', 'a', 0],
+    ['A', 'a', 0],
+    ['A', 'b', -1],
+    ['1', '2', -1],
+    ['01', '1', 0],
+    ['20000101T0000Z', '20000101T0001Z', -1],
+  ])('(%o, %o) -> %o', (a, b, expected) => {
+    expect(DEFAULT_COMPARATOR(a, b)).toEqual(expected)
+  })
+})
diff --git a/tests/unit/components/cylc/table/sort.spec.js b/tests/unit/components/cylc/table/sort.spec.js
