Merge pull request 'fix(filters): source the picker variables from public/getcols when the table is not returned' (#338) from feat/filter-vars-fallback into feat/mock-config-and-ddl

Reviewed-on: #338

client/cypress/e2e/row-cell-limits.cy.ts

@@ -118,6 +118,58 @@ context('EDIT row and cell limits, VIEW row limit: ', function () {
       }).should('exist')
     })
   })
+
+  it('5 | the filter picker still lists the variables after the abort', () => {
+    openTableFromTree(libraryToOpenIncludes, 'mpe_x_search')
+    openEdit()
+
+    cy.get('.abortMsg', { timeout: longerCommandTimeout }).should(
+      'contain.text',
+      'Table is too big (1000 rows) - please filter and try again!'
+    )
+
+    closeAbortModal()
+
+    // The abort returns no table, and the picker's variable list normally comes
+    // from the table's own columns - so without the public/getcols fallback the
+    // "please filter and try again" advice has nothing to pick from.  Every
+    // column of MPE_X_SEARCH must be on offer.
+    openFilterPopup()
+
+    filterVariables().should(($options) => {
+      const names = $options
+        .toArray()
+        .map((o) => (o.textContent || '').trim())
+        .filter((n) => n.length > 0)
+
+      expect(names, 'the picker offers the table columns').to.include.members([
+        'PRIMARY_KEY_FIELD',
+        'SITE_NAME',
+        'VESSEL',
+        'SAMPLE_COUNT',
+        'DEPTH_M',
+        'SPECIES',
+        'CRUISE_DATE',
+        'EXPEDITION_ID',
+        'NOTES'
+      ])
+    })
+
+    // and the suggestion list is usable, not just populated
+    setFilterVariable('SITE_NAME')
+
+    // Submitting is what exercises TYPE.  The picker keys its operator set and
+    // its value quoting on it, and the fallback list never passes through
+    // mergeColsRules, so without TYPE on this payload a character value would be
+    // submitted unquoted and the backend's %mp_filtercheck would reject it.
+    setFilterValue('Bristol')
+    submitFilter()
+
+    cy.get('.infoBar-text', { timeout: longerCommandTimeout }).should(
+      'contain.text',
+      "SITE_NAME = 'Bristol'"
+    )
+  })
 })
 
 const visitPage = (url: string) => {
@@ -133,6 +185,53 @@ const openEdit = () => {
   }).click()
 }
 
+/** The abort modal offers Close, and the filter button sits behind it. */
+const closeAbortModal = () => {
+  cy.get('app-info-modal', { timeout: longerCommandTimeout })
+    .contains('button', 'Close')
+    .click()
+}
+
+/**
+ * The EDIT screen's Filter button opens the same .filter-modal the viewer uses.
+ * It only renders once the editor is in its read-only (non-editing) state,
+ * which is where a failed EDIT load leaves it.
+ */
+const openFilterPopup = () => {
+  cy.contains('button.btnView', 'Filter', {
+    timeout: longerCommandTimeout
+  }).click()
+  cy.get('.filter-modal', { timeout: longerCommandTimeout }).should(
+    'be.visible'
+  )
+}
+
+/** The picker's variable suggestion list. */
+const filterVariables = () =>
+  cy.get('#datalist_vals_var_id0_0 option', { timeout: longerCommandTimeout })
+
+/** The soft-select drops a transparent overlay over the modal, so it needs force. */
+const setFilterVariable = (column: string) => {
+  cy.get('#vals_var_id0_0').clear({ force: true }).type(column, { force: true })
+  cy.get('#datalist_vals_var_id0_0 option').contains(column).click({
+    force: true
+  })
+  cy.get('#vals_var_id0_0').trigger('keyup', { key: 'Escape', force: true })
+}
+
+/** The value input of the same soft-select row. */
+const setFilterValue = (value: string) => {
+  cy.get('#vals_0_0').clear({ force: true }).type(value, { force: true })
+  cy.get('#vals_0_0').trigger('keyup', { key: 'Escape', force: true })
+  cy.wait(400)
+}
+
+const submitFilter = () => {
+  cy.contains('.filter-modal button', 'Ok').click()
+  cy.get('.app-loading', { timeout: longerCommandTimeout }).should('not.exist')
+  cy.wait(2500)
+}
+
 /**
  * Sets a DC option in MPE_CONFIG through the editor and approves it, then hands
  * back to the callback.  The same route licensing.cy.ts uses for the licence

client/src/app/editor/editor.component.ts

@@ -1206,7 +1206,16 @@ export class EditorComponent
       this.cdf.detectChanges()
 
       this.submitLoading = false
-      this.sasStoreService.setQueryVariables(this.libds, this.cols)
+      // The columns that came with the table are the cheapest source for the
+      // picker. When the table was not returned - the row/cell limits abort the
+      // request before the data arrives - that list is empty, so the store
+      // falls back to public/getcols rather than leaving the picker with
+      // nothing to pick from.
+      this.sasStoreService
+        .setQueryVariablesFromTable(this.libds, this.cols)
+        .catch((err: any) =>
+          this.eventService.catchResponseError('public/getcols', err)
+        )
     }
   }
 

client/src/app/services/sas-store.service.ts

@@ -295,6 +295,40 @@ export class SasStoreService {
     this.columns.next(columnsData)
   }
 
+  /**
+   * Column metadata for the filter picker, read from the service.
+   *
+   * The picker's variable list normally comes from the columns that arrived
+   * with the table (see setQueryVariablesFromTable). This is the fallback for
+   * when there is no table - public/getcols reads the metadata only.
+   */
+  public async getCols(libds: string) {
+    const tables = { iwant: [{ libds: libds }] }
+
+    const res = await this.sasService.request('public/getcols', tables)
+
+    return res.adapterResponse.cols || []
+  }
+
+  /**
+   * Feeds the filter picker its variable list.
+   *
+   * `cols` is the column metadata that came back with the table, which costs
+   * nothing extra and is the normal path. When the table was not returned - the
+   * EDIT/VIEW row and cell limits abort the request before the data arrives -
+   * that list is empty, which is what makes "please filter and try again"
+   * unactionable: the picker has nothing to offer. Fall back to
+   * public/getcols, which reads the column metadata without the rows.
+   */
+  public async setQueryVariablesFromTable(libds: string, cols: any[]) {
+    if (cols && cols.length > 0) {
+      this.setQueryVariables(libds, cols)
+      return
+    }
+
+    this.setQueryVariables(libds, await this.getCols(libds))
+  }
+
   public async getChangeInfo(tableId: any) {
     let obj = { TABLE: tableId }
     let table = { SASControlTable: [obj] }

client/src/app/shared/viewboxes/viewboxes.component.ts

@@ -1404,10 +1404,17 @@ export class ViewboxesComponent implements OnInit, AfterViewInit, OnDestroy {
     this.filter = true
     this.cdf.detectChanges()
 
-    this.sasStoreService.setQueryVariables(
-      this.filterLibds,
-      viewboxTable.hotTable.cols
-    )
+    // Table columns when we have them (the normal, cheapest source); when the
+    // view was aborted by the row limit there is no loaded table to read them
+    // from, so the store falls back to public/getcols.
+    const cols =
+      viewboxTable && viewboxTable.hotTable ? viewboxTable.hotTable.cols : []
+
+    this.sasStoreService
+      .setQueryVariablesFromTable(this.filterLibds, cols)
+      .catch((err: any) =>
+        this.eventService.catchResponseError('public/getcols', err)
+      )
   }
 
   /**

sas/mocks/sasjs/services/dcMockUtils.js

@@ -148,14 +148,63 @@ function _parseCsvLine(line) {
 // ─── SAS format <-> ISO conversions ──────────────────────────────────────────
 
 /**
- * Derives a DDTYPE (DATE, DATETIME, TIME, N, C) from a SAS column format string.
+ * Column metadata, mirroring %mp_getcols.sas.
+ *
+ * The SAS macro runs proc contents and derives, per column: NAME (upcased),
+ * TYPE (C/N), LENGTH, LABEL (defaulting to the name), VARNUM, FMTNAME, FORMAT
+ * and DDTYPE - CHARACTER, NUMERIC, DATETIME, DATE or TIME.  The date list is
+ * the macro's (YYMMDD, MMDDYY, DDMMYY, MONYY, E8601DA, B8601DA, E8601DT,
+ * NLDATM, NLDATE), so a column types the same way here as it does in the
+ * service that shares this code.
+ *
+ * @param {Array} schemaCols the table's columns as the mock data stores them
+ *   ({name, type, length, format, label}), the equivalent of the proc contents
+ *   output the macro reads
+ * @returns {Array} one entry per column, in the macro's shape and order
+ */
+function mpGetcols(schemaCols) {
+    return (schemaCols || []).map((col, index) => {
+        const name = String(col.name || '').toUpperCase()
+        const isChar = String(col.type || '').toUpperCase() === 'C'
+        const fmtname = String(col.format || '')
+            .replace(/[\d.]+$/, '')
+            .toUpperCase()
+
+        let ddtype
+        if (isChar) ddtype = 'CHARACTER'
+        else if (fmtname.startsWith('DATETIME') || fmtname.startsWith('E8601DT')
+            || fmtname.startsWith('NLDATM')) ddtype = 'DATETIME'
+        else if (fmtname.startsWith('DATE') || fmtname.startsWith('DDMMYY')
+            || fmtname.startsWith('MMDDYY') || fmtname.startsWith('YYMMDD')
+            || fmtname.startsWith('E8601DA') || fmtname.startsWith('B8601DA')
+            || fmtname.startsWith('MONYY') || fmtname.startsWith('NLDATE')) ddtype = 'DATE'
+        else if (fmtname.startsWith('TIME')) ddtype = 'TIME'
+        else ddtype = 'NUMERIC'
+
+        return {
+            NAME: name,
+            TYPE: isChar ? 'C' : 'N',
+            LENGTH: Number(col.length) || 0,
+            LABEL: col.label || col.name,
+            VARNUM: index + 1,
+            FORMAT: String(col.format || ''),
+            FMTNAME: fmtname,
+            DDTYPE: ddtype
+        }
+    })
+}
+
+/**
+ * Derives a DDTYPE (DATE, DATETIME, TIME, N, C) from a SAS column format
+ * string, in the shape the client expects.  Delegates to mpGetcols so the
+ * inference is the macro's, rather than a second format list that can drift
+ * away from the service's.
  */
 function getDdType(col) {
-    const fmt = (col.format || '').toLowerCase()
-    if (fmt.startsWith('datetime') || fmt.startsWith('e8601dt')) return 'DATETIME'
-    if (fmt.startsWith('date') || fmt.startsWith('yymmdd') || fmt.startsWith('ddmmyy') || fmt.startsWith('mmddyy')) return 'DATE'
-    if (fmt.startsWith('time')) return 'TIME'
-    return col.type === 'N' ? 'N' : 'C'
+    const ddtype = mpGetcols([col])[0].DDTYPE
+    if (ddtype === 'CHARACTER') return 'C'
+    if (ddtype === 'NUMERIC') return 'N'
+    return ddtype
 }
 
 /**

sas/mocks/sasjs/services/editors/getdata.js

@@ -300,22 +300,25 @@ if (tableData && tableData.columns && tableData.rows) {
         else if (ddtype === 'TIME') tmVars.push(col.name)
     }
 
-    // Build cols array
-    const cols = visibleColumns.map(col => {
-        const ddtype = getDdType(col)
-        const fmtname = col.format
-            ? col.format.replace(/[\d.]+$/, '').replace('datetime', 'DATETIME').replace('date', 'DATE').replace('time', 'TIME').replace('best', 'BEST').replace('E8601DT', 'DATETIME')
-            : ' '
+    // Build cols array.  Every column of the table, as getdata.sas emits them:
+    // the temporal columns are dropped from the data (the rows below) but not
+    // from the cols payload, so a filter can still be built on them.  mpGetcols
+    // is the JS counterpart of %mp_getcols.sas - the macro getdata.sas itself
+    // calls to type these columns - so the editor sees the same DDTYPE the
+    // service would give it.
+    const cols = mpGetcols(tableData.columns).map((c, i) => {
+        const raw = tableData.columns[i]
+        const ddtype = c.DDTYPE === 'CHARACTER' ? 'C' : c.DDTYPE === 'NUMERIC' ? 'N' : c.DDTYPE
         let coltype
-        if (ddtype === 'DATE') coltype = `{"data":"${col.name}","type":"date"}`
-        else if (ddtype === 'DATETIME') coltype = `{"data":"${col.name}","type":"datetime"}`
-        else if (ddtype === 'TIME') coltype = `{"data":"${col.name}","type":"time"}`
-        else if (ddtype === 'N') coltype = `{"data":"${col.name}","type":"numeric","format":"0"}`
-        else coltype = `{"data":"${col.name}"}`
+        if (ddtype === 'DATE') coltype = `{"data":"${raw.name}","type":"date"}`
+        else if (ddtype === 'DATETIME') coltype = `{"data":"${raw.name}","type":"datetime"}`
+        else if (ddtype === 'TIME') coltype = `{"data":"${raw.name}","type":"time"}`
+        else if (ddtype === 'N') coltype = `{"data":"${raw.name}","type":"numeric","format":"0"}`
+        else coltype = `{"data":"${raw.name}"}`
         return {
-            NAME: col.name,
-            LABEL: col.label || col.name,
-            FMTNAME: fmtname || ' ',
+            NAME: c.NAME,
+            LABEL: c.LABEL,
+            FMTNAME: c.FMTNAME || ' ',
             DDTYPE: ddtype,
             CLS_RULE: 'READ',
             MEMLABEL: ' ',

sas/mocks/sasjs/services/public/getcols.js

@@ -0,0 +1,79 @@
+const nodePath = require('path')
+
+let appLoc = nodePath.join(..._program.split('services')[0].split('/'))
+const sasjsRoot = nodePath.resolve(weboutPath, '..', '..', '..')
+const driveRoot = nodePath.resolve(sasjsRoot, 'drive')
+const dcLibref = 'DC_JSLIB'
+
+// Load shared DC mock utilities
+eval(fs.readFileSync(nodePath.resolve(driveRoot, 'files', appLoc, 'services', 'dcMockUtils.js'), 'utf8'))
+
+// ─── Parse input ──────────────────────────────────────────────────────────────
+// getcols.sas reads %mf_getvalue(work.iwant,libds). The adapter serialises the
+// input table as CSV, which fetchTable parses - so the row is `_iwantRow` (the
+// runtime already owns the name `iwant`).
+
+const _iwantRow = fetchTable('iwant')[0] || {}
+const libds = String(_iwantRow.libds || _iwantRow.LIBDS || '').trim()
+
+let libref = dcLibref
+let table = ''
+if (libds) {
+    const parts = libds.split('.')
+    if (parts.length >= 2) {
+        libref = parts[0].trim()
+        table = parts[1].trim()
+    }
+}
+
+// ─── Load the table schema ────────────────────────────────────────────────────
+// Same resolution as getdata.js: the requested libref first, then any libref
+// (a LIBDS that doesn't match the data folder still resolves).
+
+let tableData = table ? makeTableLoader(libDataDir(libref))(table) : null
+if (!tableData) {
+    // Fall back to searching every libref if the table isn't in the requested
+    // libref (e.g. a LIBDS that doesn't match the actual data folder), and take
+    // the libref that actually held it for the registry lookup below.
+    const found = loadTableAnyLib(table)
+    if (found) {
+        tableData = found.data
+        libref = found.libref
+    }
+}
+
+// ─── Build the column list ────────────────────────────────────────────────────
+// Every column, as getdata.sas emits them: the temporal columns are dropped from
+// the data, not from the cols payload, so a filter can still be built on them -
+// which is exactly what "please filter and try again" wants to do.
+
+let cols = []
+
+if (tableData && tableData.columns) {
+    // mpGetcols, then the mapping getcols.sas applies on top of %mp_getcols:
+    // DDTYPE in the client's vocabulary, and TYPE too - on the normal path
+    // mergeColsRules() sets TYPE from the loaded table's formats, and the picker
+    // keys its operator set and its value quoting on it.  This list exists
+    // because that load did not happen, so it has to carry it.
+    cols = mpGetcols(tableData.columns)
+        .map(c => {
+            const ddtype = c.DDTYPE === 'CHARACTER' ? 'C' : c.DDTYPE === 'NUMERIC' ? 'N' : c.DDTYPE
+            return {
+                NAME: c.NAME,
+                LABEL: c.LABEL,
+                FMTNAME: c.FMTNAME || ' ',
+                DDTYPE: ddtype,
+                TYPE: ['DATE', 'DATETIME', 'TIME', 'N'].includes(ddtype) ? 'num' : 'char',
+                CLS_RULE: 'READ',
+                MEMLABEL: ' ',
+                DESC: ' ',
+                LONGDESC: ' ',
+                COLTYPE: ' '
+            }
+        })
+        .sort((a, b) => a.NAME.localeCompare(b.NAME))
+}
+
+webOutOpen()
+webOutObj(cols, 'cols')
+webOutClose()

sas/mocks/sasjs/services/public/viewdata.js

@@ -101,16 +101,15 @@ let vars = {}
 let nobs = 0
 
 if (tableData && tableData.columns) {
-    cols = tableData.columns.map(col => {
-        const ddtype = getDdType(col)
-        const fmtname = col.format
-            ? col.format.replace(/[\d.]+$/, '').replace('datetime', 'DATETIME').replace('date', 'DATE').replace('time', 'TIME').replace('best', 'BEST').replace('E8601DT', 'DATETIME')
-            : ' '
-        const formatStr = col.type === 'N' ? (col.format || '8.') : ('$' + col.length + '.')
+    // mpGetcols, the JS counterpart of %mp_getcols.sas - the macro viewdata.sas
+    // calls for its own column list.
+    cols = mpGetcols(tableData.columns).map(c => {
+        const ddtype = c.DDTYPE === 'CHARACTER' ? 'C' : c.DDTYPE === 'NUMERIC' ? 'N' : c.DDTYPE
+        const formatStr = c.TYPE === 'N' ? (c.FORMAT || '8.') : ('$' + c.LENGTH + '.')
         return {
-            NAME: col.name, TYPE: col.type === 'N' ? 'N' : 'C', LENGTH: col.length,
-            FORMAT: formatStr, FMTNAME: fmtname || ' ', DDTYPE: ddtype,
-            LABEL: col.label || col.name
+            NAME: c.NAME, TYPE: c.TYPE, LENGTH: c.LENGTH,
+            FORMAT: formatStr, FMTNAME: c.FMTNAME || ' ', DDTYPE: ddtype,
+            LABEL: c.LABEL
         }
     })
 

sas/sasjs/services/public/getcols.sas

@@ -6,6 +6,7 @@
   <h4> SAS Macros </h4>
   @li dc_assignlib.sas
   @li mp_abort.sas
+  @li mp_getcols.sas
   @li mp_validatecol.sas
 
   @version 9.2
@@ -43,32 +44,26 @@ run;
 
 %dc_assignlib(READ,%scan(&ds,1,.))
 
-proc contents noprint data=&ds
-  out=droplist1 (keep=name type length label varnum format:);
-run;
-data cols(keep=name type length varnum format label);
-  set droplist1(rename=(format=format2 type=type2));
-  name=upcase(name);
-  if type2=2 then do;
-    length format $49.;
-    if format2='' then format=cats('$',length,'.');
-    else if formatl=0 then format=cats(format2,'.');
-    else format=cats(format2,formatl,'.');
-    type='C';
-    ddtype='C';
-  end;
-  else do;
-    if format2='' then format=cats(length,'.');
-    else if formatl=0 then format=cats(format2,'.');
-    else if formatd=0 then format=cats(format2,formatl,'.');
-    else format=cats(format2,formatl,'.',formatd);
-    type='N';
-    if format=:'DATETIME' then ddtype='DATETIME';
-    else if format=:'DATE' then ddtype='DATE';
-    else if format=:'TIME' then ddtype='TIME';
-    else ddtype='N';
-  end;
-  if label='' then label=name;
+/* Column metadata, typed the way the editor's own load types it: %mp_getcols
+  infers DATE and DATETIME across the whole format list (YYMMDD, MMDDYY,
+  E8601DA, B8601DA, NLDATE, NLDATM, E8601DT, ...), which a test against the
+  format prefix does not - a column formatted YYMMDD would otherwise reach the
+  picker as numeric.  getdata.sas makes the same call for the same reason. */
+%mp_getcols(&ds, outds=cols1)
+
+data cols(keep=name type length varnum fmtname ddtype);
+  set cols1;
+  /* DDTYPE in the client's vocabulary, as getdata.sas maps it */
+  if ddtype='CHARACTER' then ddtype='C';
+  else if ddtype='NUMERIC' then ddtype='N';
+  /* TYPE in the client's vocabulary too.  On the normal path mergeColsRules()
+    sets it from the loaded table's formats - 'num' for the date and time
+    types, the column's own type otherwise - and the picker keys both its
+    operator set (query.component) and its value quoting (sas-store) on it.
+    The fallback list never reaches that pass, because the load that would
+    have fed it is the one that failed, so the service has to carry it. */
+  if ddtype in ('DATE','DATETIME','TIME') or ddtype='N' then type='num';
+  else type='char';
 run;
 
 %mp_abort(iftrue= (&syscc ne 0)