add list support for numeric fields (#1789, #2033)

bihealth · Dec 19, 2024 · a782c60 · a782c60
1 parent c314d86
commit a782c60
Show file tree

Hide file tree

Showing 7 changed files with 135 additions and 72 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -35,6 +35,7 @@ Added
     - Token auth support in study plugin IGV XML serving views (#1999, #2021)
     - Support for newlines in altamISA error messages (#2033)
     - Support for comment, performer and contact field values as list (#1789, #2033)
+    - Support for numeric field values as list (#1789, #2033)
 - **Taskflowbackend**
     - ``TaskflowAPI.raise_submit_api_exception()`` helper (#1847)
 

diff --git a/docs_manual/source/sodar_release_notes.rst b/docs_manual/source/sodar_release_notes.rst
@@ -20,6 +20,7 @@ Release for SODAR Core v1.0 upgrade, iRODS v4.3 upgrade and feature updates.
 - Add session control in Django settings and environment variables
 - Add token-based iRODS/IGV basic auth support for OIDC users
 - Add support for comment, performer and contact field values as list
+- Add support for numeric field values as list
 - Update minimum supported iRODS version to v4.3.3
 - Update REST API versioning
 - Update REST API views for OpenAPI support

diff --git a/samplesheets/rendering.py b/samplesheets/rendering.py
@@ -483,14 +483,17 @@ def _get_length(value, col_type=None):
             return round(len(value) - nc - wc + 0.6 * nc + 1.3 * wc)
 
         def _is_num(value):
-            """Return whether a value contains an integer/double"""
-            if isinstance(value, str) and '_' in value:
-                return False  # HACK because float() accepts underscore
-            try:
-                float(value)
-                return True
-            except (ValueError, TypeError):
-                return False
+            """Return whether a value contains only integers/doubles"""
+            # Supports lists
+            values = value if isinstance(value, list) else [value]
+            for v in values:
+                if isinstance(v, str) and '_' in v:
+                    return False  # HACK because float() accepts underscore
+                try:
+                    float(v)
+                except (ValueError, TypeError):
+                    return False
+            return True
 
         top_idx = 0  # Top header index
         grp_idx = 0  # Index within current top header group
@@ -504,7 +507,6 @@ def _is_num(value):
                 and header_name not in th.PROCESS_NAME_HEADERS
                 and not self._field_configs[i]
                 and self._field_header[i]['col_type'] not in ['NUMERIC', 'UNIT']
-                and any(_is_num(x[i]['value']) for x in self._table_data)
                 and all(
                     (_is_num(x[i]['value']) or not x[i]['value'])
                     for x in self._table_data
@@ -535,31 +537,26 @@ def _is_num(value):
                         contact_vals.append('; '.join(x[i]['value']))
                     else:
                         contact_vals.append(x[i]['value'])
-                max_cell_len = max(
-                    [
-                        (
-                            _get_length(re.findall(link_re, x)[0][0])
-                            if re.findall(link_re, x)
-                            else len(x or '')
-                        )
-                        for x in contact_vals
-                    ]
-                )
+                cell_lengths = [
+                    (
+                        _get_length(re.findall(link_re, x)[0][0])
+                        if re.findall(link_re, x)
+                        else len(x or '')
+                    )
+                    for x in contact_vals
+                ]
+                max_cell_len = max(cell_lengths)
             elif col_type == 'EXTERNAL_LINKS':  # Special case, count elements
                 header_len = 0  # Header length is not comparable
-                max_cell_len = max(
-                    [
-                        (
-                            len(x[i]['value'])
-                            if (
-                                x[i]['value']
-                                and isinstance(x[i]['value'], list)
-                            )
-                            else 0
-                        )
-                        for x in self._table_data
-                    ]
-                )
+                cell_lengths = [
+                    (
+                        len(x[i]['value'])
+                        if x[i]['value'] and isinstance(x[i]['value'], list)
+                        else 0
+                    )
+                    for x in self._table_data
+                ]
+                max_cell_len = max(cell_lengths)
             else:  # Generic type
                 max_cell_len = max(
                     [

diff --git a/samplesheets/vueapp/src/components/editors/DataCellEditor.vue b/samplesheets/vueapp/src/components/editors/DataCellEditor.vue
@@ -138,6 +138,15 @@ export default Vue.extend({
       if (this.isPopup() && this.editUnitEnabled) return text
       return ''
     },
+    testListRegex () { // Test regex with semicolon-separated list support
+      // Don't need to add this to multiple regexes this way..
+      if (this.editValue.slice(-1) === ';') return false
+      const valSplit = this.editValue.split(';')
+      for (let i = 0; i < valSplit.length; i++) {
+        if (!this.regex.test(valSplit[i].trim())) return false
+      }
+      return true
+    },
     getValidState () {
       if (this.nameColumn) { // Name is a special case
         // TODO: Cleanup/simplify
@@ -158,18 +167,20 @@ export default Vue.extend({
           return false
         }
       } else if (this.editValue !== '') {
-        // TODO: Enable validation of listed values separately
         // Test range
         if (['integer', 'double'].includes(this.editConfig.format) &&
             'range' in this.editConfig &&
             this.editConfig.range.length === 2) {
           const range = this.editConfig.range
-          const valNum = parseFloat(this.editValue)
-          if (valNum < parseFloat(range[0]) ||
-              valNum > parseFloat(range[1])) {
-            this.invalidMsg = 'Not in Range (' +
-              parseInt(range[0]) + '-' + parseInt(range[1]) + ')'
-            return false
+          const valSplit = this.editValue.split(';')
+          for (let i = 0; i < valSplit.length; i++) {
+            const valNum = parseFloat(valSplit[i].trim())
+            if (valNum < parseFloat(range[0]) ||
+                valNum > parseFloat(range[1])) {
+              this.invalidMsg = 'Not in Range (' +
+                  parseInt(range[0]) + '-' + parseInt(range[1]) + ')'
+              return false
+            }
           }
         } else if (this.editConfig.format === 'date') {
           if (!dateRegex.test(this.editValue)) {
@@ -188,10 +199,7 @@ export default Vue.extend({
         }
       }
       // Test Regex
-      return !(this.editValue !== '' &&
-          this.regex &&
-          !this.regex.test(this.editValue)
-      )
+      return !(this.editValue !== '' && this.regex && !this.testListRegex())
     },
     getUpdateData () {
       return Object.assign(
@@ -207,7 +215,6 @@ export default Vue.extend({
       for (let i = 0; i < gridUuids.length; i++) {
         const gridOptions = this.app.getGridOptionsByUuid(gridUuids[i])
         const gridApi = gridOptions.api
-
         if (!gridOptions.columnApi.getColumn(this.params.colDef.field)) {
           continue // Skip this grid if the column is not present
         }
@@ -314,12 +321,10 @@ export default Vue.extend({
       this.regex = /^[\w\-.]+$/
     } else if (this.headerInfo.header_type === 'name') { // Other names
       this.regex = /^([A-Za-z0-9-_/]*)$/
-    } else { // Default regex for certain fields
-      if (this.editConfig.format === 'integer') {
-        this.regex = /^(([1-9][0-9]*)|([0]?))$/ // TODO: TBD: Allow negative?
-      } else if (this.editConfig.format === 'double') {
-        this.regex = /^-?[0-9]+\.[0-9]+?$/
-      }
+    } else if (this.editConfig.format === 'integer') {
+      this.regex = /^(([1-9][0-9]*)|([0]?))$/ // TODO: TBD: Allow negative?
+    } else if (this.editConfig.format === 'double') {
+      this.regex = /^-?[0-9]+\.[0-9]+?$/
     }
 
     // Special setup for the name column
@@ -351,11 +356,7 @@ export default Vue.extend({
     if (!this.destroyCalled) {
       this.destroyCalled = true // HACK for issue #869
       // Convert to list value if applicable
-      // TODO: Allow for integer, double and unit-having fields
-      if (this.editValue.includes(';') && (
-        this.isValueArray || (
-          !['integer', 'double'].includes(this.editConfig.format) &&
-          !this.value.unit))) {
+      if (this.editValue.includes(';')) {
         this.value.value = this.editValue.split(';')
         for (let i = 0; i < this.value.value.length; i++) {
           this.value.value[i] = this.value.value[i].trim()

diff --git a/samplesheets/vueapp/src/components/renderers/DataCellRenderer.vue b/samplesheets/vueapp/src/components/renderers/DataCellRenderer.vue
@@ -4,12 +4,8 @@
        :data-row-id="params.node.id"
        @mouseover="onMouseOver"
        @mouseout="onMouseOut">
-    <!-- Value with unit -->
-    <span v-if="'unit' in value && value.unit">
-      {{ value.value }} <span class="text-muted">{{ value.unit }}</span>
-    </span>
     <!-- Ontology term(s) -->
-    <span v-else-if="colType === 'ONTOLOGY' && value.value.length > 0">
+    <span v-if="colType === 'ONTOLOGY' && value.value.length > 0">
       <span v-if="!params.app.editMode && headerName === 'hpo terms'">
         <b-button
             class="btn sodar-list-btn mr-1 sodar-ss-hpo-copy-btn"
@@ -76,6 +72,10 @@
     <span v-else>
       {{ value.value }}
     </span>
+    <!-- Unit -->
+    <span v-if="value && value.value && 'unit' in value && value.unit">
+      <span class="text-muted">{{ value.unit }}</span>
+    </span>
   </div>
 </template>
 

diff --git a/samplesheets/vueapp/tests/unit/DataCellEditor.spec.js b/samplesheets/vueapp/tests/unit/DataCellEditor.spec.js
@@ -306,7 +306,7 @@ describe('DataCellEditor.vue', () => {
   })
 
   it('updates value in unit column', async () => {
-    const oldValue = '90 day'
+    const oldValue = '90'
     const newValue = '100'
     const table = copy(studyTablesOneCol).tables.study
     table.field_header[0] = studyTablesEdit.tables.study.field_header[2]
@@ -320,18 +320,50 @@ describe('DataCellEditor.vue', () => {
     await waitAG(wrapper)
     await waitRAF()
 
-    expect(wrapper.find('.sodar-ss-data').text()).toBe(oldValue)
+    // NOTE: Vue inserts line breaks and spaces between words in spans, so we
+    //       have to test these with toContain() (or modify the text)
+    expect(wrapper.find('.sodar-ss-data').text()).toContain(oldValue)
+    expect(wrapper.find('.sodar-ss-data').text()).toContain('day')
     await wrapper.find('.sodar-ss-data-cell').trigger('dblclick')
     const input = wrapper.find('.ag-cell-edit-input')
     input.element.value = newValue
     await input.trigger('input')
     await gridOptions.api.stopEditing()
     expect(wrapper.find('.ag-cell-edit-input').exists()).toBe(false)
-    expect(wrapper.find('.sodar-ss-data').text()).toBe(newValue + ' day')
+    expect(wrapper.find('.sodar-ss-data').text()).toContain(newValue)
+    expect(wrapper.find('.sodar-ss-data').text()).toContain('day')
+  })
+
+  it('updates value in unit column to a list', async () => {
+    const oldValue = '90'
+    const newValue = '100;101'
+    const table = copy(studyTablesOneCol).tables.study
+    table.field_header[0] = studyTablesEdit.tables.study.field_header[2]
+    table.table_data[0][0] = copy(studyTablesEdit).tables.study.table_data[0][2]
+    const wrapper = mountSheetTable({
+      table: table,
+      editStudyConfig: {
+        nodes: [{ fields: [studyEditConfig.nodes[0].fields[2]] }]
+      }
+    })
+    await waitAG(wrapper)
+    await waitRAF()
+
+    expect(wrapper.find('.sodar-ss-data').text()).toContain(oldValue)
+    expect(wrapper.find('.sodar-ss-data').text()).not.toContain('100; 101')
+    expect(wrapper.find('.sodar-ss-data').text()).toContain('day')
+    await wrapper.find('.sodar-ss-data-cell').trigger('dblclick')
+    const input = wrapper.find('.ag-cell-edit-input')
+    input.element.value = newValue
+    await input.trigger('input')
+    await gridOptions.api.stopEditing()
+    expect(wrapper.find('.ag-cell-edit-input').exists()).toBe(false)
+    expect(wrapper.find('.sodar-ss-data').text()).toContain('100; 101')
+    expect(wrapper.find('.sodar-ss-data').text()).toContain('day')
   })
 
   it('updates value in unit column outside range (should fail)', async () => {
-    const oldValue = '90 day'
+    const oldValue = '90'
     const newValue = '170' // max = 150
     const table = copy(studyTablesOneCol).tables.study
     table.field_header[0] = studyTablesEdit.tables.study.field_header[2]
@@ -345,7 +377,7 @@ describe('DataCellEditor.vue', () => {
     await waitAG(wrapper)
     await waitRAF()
 
-    expect(wrapper.find('.sodar-ss-data').text()).toBe(oldValue)
+    expect(wrapper.find('.sodar-ss-data').text()).toContain(oldValue)
     await wrapper.find('.sodar-ss-data-cell').trigger('dblclick')
     const input = wrapper.find('.ag-cell-edit-input')
     input.element.value = newValue
@@ -354,11 +386,36 @@ describe('DataCellEditor.vue', () => {
     expect(wrapper.find('.ag-cell-edit-input').classes()).toContain('text-danger')
     await gridOptions.api.stopEditing()
     expect(wrapper.find('.ag-cell-edit-input').exists()).toBe(false)
-    expect(wrapper.find('.sodar-ss-data').text()).toBe(oldValue)
+    expect(wrapper.find('.sodar-ss-data').text()).toContain(oldValue)
+  })
+
+  it('updates list value in unit column outside range (should fail)', async () => {
+    const oldValue = '90'
+    const newValue = '100;170'
+    const table = copy(studyTablesOneCol).tables.study
+    table.field_header[0] = studyTablesEdit.tables.study.field_header[2]
+    table.table_data[0][0] = copy(studyTablesEdit).tables.study.table_data[0][2]
+    const wrapper = mountSheetTable({
+      table: table,
+      editStudyConfig: {
+        nodes: [{ fields: [studyEditConfig.nodes[0].fields[2]] }]
+      }
+    })
+    await waitAG(wrapper)
+    await waitRAF()
+
+    expect(wrapper.find('.sodar-ss-data').text()).toContain(oldValue)
+    await wrapper.find('.sodar-ss-data-cell').trigger('dblclick')
+    const input = wrapper.find('.ag-cell-edit-input')
+    input.element.value = newValue
+    await input.trigger('input')
+    await gridOptions.api.stopEditing()
+    expect(wrapper.find('.ag-cell-edit-input').exists()).toBe(false)
+    expect(wrapper.find('.sodar-ss-data').text()).toContain(oldValue)
   })
 
   it('updates unit in unit column', async () => {
-    const oldValue = '90 day'
+    const oldValue = '90'
     const table = copy(studyTablesOneCol).tables.study
     table.field_header[0] = studyTablesEdit.tables.study.field_header[2]
     table.table_data[0][0] = copy(studyTablesEdit).tables.study.table_data[0][2]
@@ -371,13 +428,17 @@ describe('DataCellEditor.vue', () => {
     await waitAG(wrapper)
     await waitRAF()
 
-    expect(wrapper.find('.sodar-ss-data').text()).toBe(oldValue)
+    expect(wrapper.find('.sodar-ss-data').text()).toContain(oldValue)
+    expect(wrapper.find('.sodar-ss-data').text()).toContain('day')
+    expect(wrapper.find('.sodar-ss-data').text()).not.toContain('year')
     await wrapper.find('.sodar-ss-data-cell').trigger('dblclick')
     const unitInput = wrapper.find('#sodar-ss-data-cell-unit')
     await unitInput.findAll('option').at(2).setSelected()
     await gridOptions.api.stopEditing()
     expect(wrapper.find('.ag-cell-edit-input').exists()).toBe(false)
-    expect(wrapper.find('.sodar-ss-data').text()).toBe('90 year')
+    expect(wrapper.find('.sodar-ss-data').text()).toContain('90')
+    expect(wrapper.find('.sodar-ss-data').text()).toContain('year')
+    expect(wrapper.find('.sodar-ss-data').text()).not.toContain('day')
   })
 
   it('updates value in unit column with empty value', async () => {
@@ -403,7 +464,7 @@ describe('DataCellEditor.vue', () => {
   })
 
   it('updates unit in unit column to empty unit', async () => {
-    const oldValue = '90 day'
+    const oldValue = '90'
     const table = copy(studyTablesOneCol).tables.study
     table.field_header[0] = studyTablesEdit.tables.study.field_header[2]
     table.table_data[0][0] = copy(studyTablesEdit).tables.study.table_data[0][2]
@@ -416,7 +477,8 @@ describe('DataCellEditor.vue', () => {
     await waitAG(wrapper)
     await waitRAF()
 
-    expect(wrapper.find('.sodar-ss-data').text()).toBe(oldValue)
+    expect(wrapper.find('.sodar-ss-data').text()).toContain(oldValue)
+    expect(wrapper.find('.sodar-ss-data').text()).toContain('day')
     await wrapper.find('.sodar-ss-data-cell').trigger('dblclick')
     const unitInput = wrapper.find('#sodar-ss-data-cell-unit')
     await unitInput.findAll('option').at(0).setSelected()

diff --git a/samplesheets/vueapp/tests/unit/DataCellRenderer.spec.js b/samplesheets/vueapp/tests/unit/DataCellRenderer.spec.js
@@ -157,7 +157,8 @@ describe('DataCellRenderer.vue', () => {
 
     const cell = wrapper.find('.sodar-ss-data-cell')
     const cellData = cell.find('.sodar-ss-data')
-    expect(cellData.text()).toBe('90 day')
+    expect(cellData.text()).toContain('90')
+    expect(cellData.text()).toContain('day')
     expect(cell.classes()).toContain('text-right')
     expect(cell.classes()).not.toContain('text-muted')
     expect(cellData.find('span.text-muted').exists()).toBe(true)