Fix shift arrow selections when primary field option wasnt ordered first

changelog/entries/unreleased/bug/1998_fix_shift_arrow_selection_on_grid_view_when_primary_key_fiel.json

@@ -0,0 +1,7 @@
+{
+    "type": "bug",
+    "message": "Fix shift arrow selection on grid view when primary key field option is not first",
+    "issue_number": 1998,
+    "bullet_points": [],
+    "created_at": "2023-09-29"
+}

web-frontend/modules/database/components/view/grid/GridView.vue

@@ -54,6 +54,7 @@
       @edit-modal="openRowEditModal($event)"
       @refresh-row="refreshRow"
       @scroll="scroll($event.pixelY, 0)"
+      @cell-selected="cellSelected"
     >
       <template #foot>
         <div class="grid-view__foot-info">
@@ -122,6 +123,7 @@
       @edit-modal="openRowEditModal($event)"
       @refresh-row="refreshRow"
       @scroll="scroll($event.pixelY, $event.pixelX)"
+      @cell-selected="cellSelected"
     >
     </GridViewSection>
     <GridViewRowDragging
@@ -368,11 +370,16 @@ export default {
     ...mapGetters({
       row: 'rowModalNavigation/getRow',
     }),
+    /**
+     * Returns all visible fields no matter in what section they
+     * belong.
+     */
     allVisibleFields() {
       return this.leftFields.concat(this.visibleFields)
     },
     /**
-     * Returns only the visible fields in the correct order. Primary must always be
+     * Returns only the visible fields in the correct order that are in
+     * the right section of the grid. Primary must always be
      * first if in that list.
      */
     visibleFields() {
@@ -777,7 +784,7 @@ export default {
           rowHeadIndex: rowIndex,
           rowTailIndex: rowIndex,
           fieldHeadIndex: 0,
-          fieldTailIndex: this.visibleFields.length,
+          fieldTailIndex: this.allVisibleFields.length - 1,
         }
       )
     },
@@ -1049,6 +1056,14 @@ export default {
       this.$store.dispatch(this.storePrefix + 'view/grid/setSelectedCell', {
         rowId: nextRowId,
         fieldId: nextFieldId,
+        fields: this.fields,
+      })
+    },
+    cellSelected({ fieldId, rowId }) {
+      this.$store.dispatch(this.storePrefix + 'view/grid/setSelectedCell', {
+        rowId,
+        fieldId,
+        fields: this.fields,
       })
     },
     /**
@@ -1070,8 +1085,7 @@ export default {
         this.storePrefix + 'view/grid/multiSelectShiftClick',
         {
           rowId: row.id,
-          fieldIndex:
+          fieldIndex: this.allVisibleFields.findIndex((f) => f.id === field.id),
-            this.visibleFields.findIndex((f) => f.id === field.id) + 1,
         }
       )
     },
@@ -1081,8 +1095,9 @@ export default {
      * selected cell.
      */
     multiSelectStart({ event, row, field }) {
-      let fieldIndex = this.visibleFields.findIndex((f) => f.id === field.id)
+      const fieldIndex = this.allVisibleFields.findIndex(
-      if (this.canFitInTwoColumns) fieldIndex += 1
+        (f) => f.id === field.id
+      )
 
       this.$store.dispatch(this.storePrefix + 'view/grid/multiSelectStart', {
         rowId: row.id,
@@ -1095,8 +1110,9 @@ export default {
      * with the last cell hovered over.
      */
     multiSelectHold({ event, row, field }) {
-      let fieldIndex = this.visibleFields.findIndex((f) => f.id === field.id)
+      const fieldIndex = this.allVisibleFields.findIndex(
-      if (this.canFitInTwoColumns) fieldIndex += 1
+        (f) => f.id === field.id
+      )
 
       this.$store.dispatch(this.storePrefix + 'view/grid/multiSelectHold', {
         rowId: row.id,
@@ -1186,20 +1202,18 @@ export default {
           scrollDirection = 'vertical'
         }
 
-        const fieldId =
+        const fieldId = this.$store.getters[
-          this.$store.getters[this.storePrefix + 'view/grid/getFieldIdByIndex'](
+          this.storePrefix + 'view/grid/getFieldIdByIndex'
-            fieldIndex
+        ](fieldIndex, this.fields)
-          )
         if (fieldId === -1) {
           return
         }
         const field = this.$store.getters['field/get'](fieldId)
         const verticalContainer = this.$refs.right.$refs.body
         const horizontalContainer = this.$refs.right.$el
-        const visibleFieldOptions =
+        const visibleFieldOptions = this.$store.getters[
-          this.$store.getters[
+          this.storePrefix + 'view/grid/getOrderedVisibleFieldOptions'
-            this.storePrefix + 'view/grid/getOrderedVisibleFieldOptions'
+        ](this.fields)
-          ]
         let elementRight = -horizontalContainer.scrollLeft
         for (let i = 0; i < visibleFieldOptions.length; i++) {
           const fieldOption = visibleFieldOptions[i]
@@ -1238,6 +1252,7 @@ export default {
             this.storePrefix + 'view/grid/setSelectedCellCancelledMultiSelect',
             {
               direction: arrowShiftKeysMapping[key],
+              fields: this.fields,
             }
           )
         }
@@ -1280,8 +1295,9 @@ export default {
       const rowIndex = this.$store.getters[
         this.storePrefix + 'view/grid/getRowIndexById'
       ](row.id)
-      let fieldIndex = this.visibleFields.findIndex((f) => f.id === field.id)
+      const fieldIndex = this.allVisibleFields.findIndex(
-      if (this.canFitInTwoColumns) fieldIndex += 1
+        (f) => f.id === field.id
+      )
       await this.pasteData(textData, jsonData, rowIndex, fieldIndex)
     },
     /**

web-frontend/modules/database/components/view/grid/GridViewRow.vue

@@ -265,10 +265,7 @@ export default {
       return this.row._.selected && this.row._.selectedFieldId === fieldId
     },
     selectCell(fieldId, rowId = this.row.id) {
-      this.$store.dispatch(this.storePrefix + 'view/grid/setSelectedCell', {
+      this.$emit('cell-selected', { fieldId, rowId })
-        rowId,
-        fieldId,
-      })
     },
     // Return an object that represents if a cell is selected,
     // and it's current position in the selection grid

web-frontend/modules/database/store/view/grid.js

@@ -1222,24 +1222,26 @@ export const actions = {
   setAddRowHover({ commit }, value) {
     commit('SET_ADD_ROW_HOVER', value)
   },
-  setSelectedCell({ commit, getters, rootGetters }, { rowId, fieldId }) {
+  setSelectedCell(
+    { commit, getters, rootGetters },
+    { rowId, fieldId, fields }
+  ) {
     commit('SET_SELECTED_CELL', { rowId, fieldId })
 
     const rowIndex = getters.getRowIndexById(rowId)
 
     if (rowIndex !== -1) {
       commit('SET_MULTISELECT_START_ROW_INDEX', rowIndex)
-
+      const visibleFieldOptions = getters.getOrderedVisibleFieldOptions(fields)
-      const visibleFieldEntries = getters.getOrderedVisibleFieldOptions
       commit(
         'SET_MULTISELECT_START_FIELD_INDEX',
-        visibleFieldEntries.findIndex((f) => parseInt(f[0]) === fieldId)
+        visibleFieldOptions.findIndex((f) => parseInt(f[0]) === fieldId)
       )
     }
   },
   setSelectedCellCancelledMultiSelect(
     { commit, getters, rootGetters, dispatch },
-    { direction }
+    { direction, fields }
   ) {
     const rowIndex = getters.getMultiSelectStartRowIndex
     const fieldIndex = getters.getMultiSelectStartFieldIndex
@@ -1249,7 +1251,7 @@ export const actions = {
     )
 
     const rows = getters.getAllRows
-    const visibleFieldEntries = getters.getOrderedVisibleFieldOptions
+    const visibleFieldEntries = getters.getOrderedVisibleFieldOptions(fields)
     const row = rows[newRowIndex - getters.getBufferStartIndex]
     const field = visibleFieldEntries[newFieldIndex]
 
@@ -1257,6 +1259,7 @@ export const actions = {
       dispatch('setSelectedCell', {
         rowId: row.id,
         fieldId: parseInt(field[0]),
+        fields,
       })
     } else {
       const oldRow = rows[rowIndex - getters.getBufferStartIndex]
@@ -1266,6 +1269,7 @@ export const actions = {
         dispatch('setSelectedCell', {
           rowId: oldRow.id,
           fieldId: parseInt(oldField[0]),
+          fields,
         })
       }
     }
@@ -1415,7 +1419,6 @@ export const actions = {
     const rowIndex = getters.getRowIndexById(rowId)
     const startRowIndex = getters.getMultiSelectStartRowIndex
     const startFieldIndex = getters.getMultiSelectStartFieldIndex
-
     const newHeadRowIndex = Math.min(startRowIndex, rowIndex)
     const newHeadFieldIndex = Math.min(startFieldIndex, fieldIndex)
     const newTailRowIndex = Math.max(startRowIndex, rowIndex)
@@ -1700,6 +1703,7 @@ export const actions = {
       await dispatch('setSelectedCell', {
         rowId: rowsPopulated[0].id,
         fieldId: primaryField.id,
+        fields,
       })
     }
 
@@ -2738,24 +2742,38 @@ export const getters = {
   getAllFieldOptions(state) {
     return state.fieldOptions
   },
-  getOrderedFieldOptions(state, getters) {
+  getOrderedFieldOptions: (state, getters) => (fields) => {
+    const primaryField = fields.find((f) => f.primary === true)
+    const primaryFieldId = primaryField?.id || -1
+
     return Object.entries(getters.getAllFieldOptions)
       .map(([fieldIdStr, options]) => [parseInt(fieldIdStr), options])
       .sort(([a, { order: orderA }], [b, { order: orderB }]) => {
-        // First by order.
+        const isAPrimary = a === primaryFieldId
+        const isBPrimary = b === primaryFieldId
+
+        // Place primary field first
+        if (isAPrimary === true && !isBPrimary) {
+          return -1
+        } else if (isBPrimary === true && !isAPrimary) {
+          return 1
+        }
+
+        // Then by order
         if (orderA > orderB) {
           return 1
         } else if (orderA < orderB) {
           return -1
         }
 
+        // Finally by id if order is the same
         return a - b
       })
   },
-  getOrderedVisibleFieldOptions(state, getters) {
+  getOrderedVisibleFieldOptions: (state, getters) => (fields) => {
-    return getters.getOrderedFieldOptions.filter(
+    return getters
-      ([fieldId, options]) => options.hidden === false
+      .getOrderedFieldOptions(fields)
-    )
+      .filter(([fieldId, options]) => options.hidden === false)
   },
   getNumberOfVisibleFields(state) {
     return Object.values(state.fieldOptions).filter((fo) => fo.hidden === false)
@@ -2849,8 +2867,8 @@ export const getters = {
     }
     return -1
   },
-  getFieldIdByIndex: (state, getters) => (fieldIndex) => {
+  getFieldIdByIndex: (state, getters) => (fieldIndex, fields) => {
-    const orderedFieldOptions = getters.getOrderedVisibleFieldOptions
+    const orderedFieldOptions = getters.getOrderedVisibleFieldOptions(fields)
     if (orderedFieldOptions[fieldIndex]) {
       return orderedFieldOptions[fieldIndex][0]
     }

web-frontend/test/unit/database/store/view/grid.spec.js

@@ -878,40 +878,7 @@ describe('Grid view store', () => {
   })
 
   test('getOrderedFieldOptions', () => {
-    const state = Object.assign(gridStore.state(), {
+    const fields = []
-      fieldOptions: {
-        1: {
-          order: 0,
-          hidden: false,
-        },
-        2: {
-          order: 1,
-          hidden: true,
-        },
-        3: {
-          order: 3,
-          hidden: false,
-        },
-        4: {
-          order: 2,
-          hidden: false,
-        },
-      },
-    })
-    gridStore.state = () => state
-    store.registerModule('grid', gridStore)
-
-    expect(
-      JSON.parse(JSON.stringify(store.getters['grid/getOrderedFieldOptions']))
-    ).toEqual([
-      [1, { hidden: false, order: 0 }],
-      [2, { hidden: true, order: 1 }],
-      [4, { hidden: false, order: 2 }],
-      [3, { hidden: false, order: 3 }],
-    ])
-  })
-
-  test('getOrderedVisibleFieldOptions', () => {
     const state = Object.assign(gridStore.state(), {
       fieldOptions: {
         1: {
@@ -937,7 +904,86 @@ describe('Grid view store', () => {
 
     expect(
       JSON.parse(
-        JSON.stringify(store.getters['grid/getOrderedVisibleFieldOptions'])
+        JSON.stringify(store.getters['grid/getOrderedFieldOptions'](fields))
+      )
+    ).toEqual([
+      [1, { hidden: false, order: 0 }],
+      [2, { hidden: true, order: 1 }],
+      [4, { hidden: false, order: 2 }],
+      [3, { hidden: false, order: 3 }],
+    ])
+  })
+
+  test('getOrderedFieldOptions places primary field first', () => {
+    const fields = [
+      { id: 2, primary: false },
+      { id: 3, primary: true },
+    ]
+    const state = Object.assign(gridStore.state(), {
+      fieldOptions: {
+        1: {
+          order: 0,
+          hidden: false,
+        },
+        2: {
+          order: 1,
+          hidden: true,
+        },
+        3: {
+          order: 3,
+          hidden: false,
+        },
+        4: {
+          order: 2,
+          hidden: false,
+        },
+      },
+    })
+    gridStore.state = () => state
+    store.registerModule('grid', gridStore)
+
+    expect(
+      JSON.parse(
+        JSON.stringify(store.getters['grid/getOrderedFieldOptions'](fields))
+      )
+    ).toEqual([
+      [3, { hidden: false, order: 3 }],
+      [1, { hidden: false, order: 0 }],
+      [2, { hidden: true, order: 1 }],
+      [4, { hidden: false, order: 2 }],
+    ])
+  })
+
+  test('getOrderedVisibleFieldOptions', () => {
+    const fields = []
+    const state = Object.assign(gridStore.state(), {
+      fieldOptions: {
+        1: {
+          order: 0,
+          hidden: false,
+        },
+        2: {
+          order: 1,
+          hidden: true,
+        },
+        3: {
+          order: 3,
+          hidden: false,
+        },
+        4: {
+          order: 2,
+          hidden: false,
+        },
+      },
+    })
+    gridStore.state = () => state
+    store.registerModule('grid', gridStore)
+
+    expect(
+      JSON.parse(
+        JSON.stringify(
+          store.getters['grid/getOrderedVisibleFieldOptions'](fields)
+        )
       )
     ).toEqual([
       [1, { hidden: false, order: 0 }],
@@ -979,6 +1025,7 @@ describe('Grid view store', () => {
   })
 
   test('getFieldIdByIndex', () => {
+    const fields = []
     const state = Object.assign(gridStore.state(), {
       fieldOptions: {
         1: {
@@ -1002,6 +1049,6 @@ describe('Grid view store', () => {
     gridStore.state = () => state
     store.registerModule('grid', gridStore)
 
-    expect(store.getters['grid/getFieldIdByIndex'](2)).toBe(3)
+    expect(store.getters['grid/getFieldIdByIndex'](2, fields)).toBe(3)
   })
 })

web-frontend/test/unit/database/store/view/gridMultiSelect.spec.js

@@ -557,6 +557,7 @@ describe('Grid view multiple select', () => {
     })
 
     test('previous direction', async () => {
+      const fields = []
       state.multiSelectActive = false
       state.multiSelectHeadRowIndex = 14
       state.multiSelectHeadFieldIndex = 1
@@ -567,6 +568,7 @@ describe('Grid view multiple select', () => {
 
       await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
         direction: 'previous',
+        fields,
       })
 
       expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -582,6 +584,7 @@ describe('Grid view multiple select', () => {
     })
 
     test('next direction', async () => {
+      const fields = []
       state.multiSelectActive = false
       state.multiSelectHeadRowIndex = 14
       state.multiSelectHeadFieldIndex = 1
@@ -592,6 +595,7 @@ describe('Grid view multiple select', () => {
 
       await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
         direction: 'next',
+        fields,
       })
 
       expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -607,6 +611,7 @@ describe('Grid view multiple select', () => {
     })
 
     test('above direction', async () => {
+      const fields = []
       state.multiSelectActive = false
       state.multiSelectHeadRowIndex = 14
       state.multiSelectHeadFieldIndex = 1
@@ -617,6 +622,7 @@ describe('Grid view multiple select', () => {
 
       await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
         direction: 'above',
+        fields,
       })
 
       expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -632,6 +638,7 @@ describe('Grid view multiple select', () => {
     })
 
     test('below direction', async () => {
+      const fields = []
       state.multiSelectActive = false
       state.multiSelectHeadRowIndex = 14
       state.multiSelectHeadFieldIndex = 1
@@ -642,6 +649,7 @@ describe('Grid view multiple select', () => {
 
       await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
         direction: 'below',
+        fields,
       })
 
       expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -657,6 +665,7 @@ describe('Grid view multiple select', () => {
     })
 
     test('previous direction at the edge', async () => {
+      const fields = []
       state.multiSelectActive = false
       state.multiSelectHeadRowIndex = 14
       state.multiSelectHeadFieldIndex = 0
@@ -667,6 +676,7 @@ describe('Grid view multiple select', () => {
 
       await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
         direction: 'previous',
+        fields,
       })
 
       expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -682,6 +692,7 @@ describe('Grid view multiple select', () => {
     })
 
     test('next direction at the edge', async () => {
+      const fields = []
       state.multiSelectActive = false
       state.multiSelectHeadRowIndex = 14
       state.multiSelectHeadFieldIndex = 2
@@ -692,6 +703,7 @@ describe('Grid view multiple select', () => {
 
       await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
         direction: 'next',
+        fields,
       })
 
       expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -707,6 +719,7 @@ describe('Grid view multiple select', () => {
     })
 
     test('above direction at the edge', async () => {
+      const fields = []
       state.multiSelectActive = false
       state.multiSelectHeadRowIndex = 9
       state.multiSelectHeadFieldIndex = 1
@@ -717,6 +730,7 @@ describe('Grid view multiple select', () => {
 
       await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
         direction: 'above',
+        fields,
       })
 
       expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -732,6 +746,7 @@ describe('Grid view multiple select', () => {
     })
 
     test('below direction at the edge', async () => {
+      const fields = []
       state.multiSelectActive = false
       state.multiSelectHeadRowIndex = 17
       state.multiSelectHeadFieldIndex = 1
@@ -742,6 +757,7 @@ describe('Grid view multiple select', () => {
 
       await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
         direction: 'below',
+        fields,
       })
 
       expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)