(core) New type conversion in the backend

Summary: This is https://phab.getgrist.com/D3205 plus some changes (https://github.com/dsagal/grist/compare/type-convert...type-convert-server?expand=1) that move the conversion process to the backend. A new user action ConvertFromColumn uses `call_external` so that the data engine can delegate back to ActiveDoc. Code for creating formatters and parsers is significantly refactored so that most of the logic is in `common` and can be used in different ways.

Test Plan: The original diff adds plenty of tests.

Reviewers: georgegevoian

Reviewed By: georgegevoian

Subscribers: dsagal

Differential Revision: https://phab.getgrist.com/D3240

app/client/components/ColumnTransform.ts

@@ -142,6 +142,10 @@ export class ColumnTransform extends Disposable {
     return actions.every(action => (
       // ['AddColumn', USER_TABLE, 'gristHelper_Transform', colInfo]
       (action[2] === 'gristHelper_Transform') ||
+      // ['AddColumn', USER_TABLE, 'gristHelper_Converted', colInfo]
+      (action[2] === 'gristHelper_Converted') ||
+      // ['ConvertFromColumn', USER_TABLE, SOURCE_COLUMN, 'gristHelper_Converted']
+      (action[3] === 'gristHelper_Converted') ||
       // ["SetDisplayFormula", USER_TABLE, ...]
       (action[0] === 'SetDisplayFormula') ||
       // ['UpdateRecord', '_grist_Table_column', transformColId, ...]
@@ -192,7 +196,6 @@ export class ColumnTransform extends Disposable {
     // Define variables used after await, since this will be disposed by then.
     const transformColId = this.transformColumn.colId();
     const field = this.field;
-    const fieldBuilder = this._fieldBuilder;
     const origRef = this.origColumn.getRowId();
     const tableData = this._tableData;
     this.isCallPending(true);
@@ -201,17 +204,36 @@ export class ColumnTransform extends Disposable {
         // TODO: Values flicker during executing since transform column remains a formula as values are copied
         // back to the original column. The CopyFromColumn useraction really ought to be "CopyAndRemove" since
         // that seems the best way to avoid calculating the formula on wrong values.
-        return await tableData.sendTableAction(['CopyFromColumn', transformColId, this.origColumn.colId(),
-          JSON.stringify(fieldBuilder.options())]);
+        await this.gristDoc.docData.sendActions(this.executeActions());
       }
     } finally {
       // Wait until the change completed to set column back, to avoid value flickering.
       field.colRef(origRef);
       void tableData.sendTableAction(['RemoveColumn', transformColId]);
+      this.cleanup();
       this.dispose();
     }
   }
 
+  /**
+   * The user actions to send when actually executing the transform.
+   */
+  protected executeActions(): UserAction[] {
+    return [
+      [
+        'CopyFromColumn',
+        this._tableData.tableId,
+        this.transformColumn.colId(),
+        this.origColumn.colId(),
+        JSON.stringify(this._fieldBuilder.options()),
+      ],
+    ];
+  }
+
+  protected cleanup() {
+    // For overriding
+  }
+
   protected getIdentityFormula() {
     return 'return $' + this.origColumn.colId();
   }

app/client/components/TypeConversion.ts

@@ -6,7 +6,6 @@
 
 import {DocModel} from 'app/client/models/DocModel';
 import {ColumnRec} from 'app/client/models/entities/ColumnRec';
-import * as UserType from 'app/client/widgets/UserType';
 import * as gristTypes from 'app/common/gristTypes';
 import {isFullReferencingType} from 'app/common/gristTypes';
 import * as gutil from 'app/common/gutil';
@@ -90,7 +89,7 @@ export async function prepTransformColInfo(docModel: DocModel, origCol: ColumnRe
     type: addColTypeSuffix(toTypeMaybeFull, origCol, docModel),
     isFormula: true,
     visibleCol: 0,
-    formula: "",          // Will be filled in at the end.
+    formula: "CURRENT_CONVERSION(rec)",
   };
 
   const prevOptions = origCol.widgetOptionsJson.peek() || {};
@@ -139,23 +138,35 @@ export async function prepTransformColInfo(docModel: DocModel, origCol: ColumnRe
     case 'RefList':
     {
       // Set suggested destination table and visible column.
-      // Null if toTypeMaybeFull is a pure type (e.g. converting to Ref before a table is chosen).
-      const optTableId = gutil.removePrefix(toTypeMaybeFull, `${toType}:`)!;
+      // Undefined if toTypeMaybeFull is a pure type (e.g. converting to Ref before a table is chosen).
+      const optTableId = gutil.removePrefix(toTypeMaybeFull, `${toType}:`) || undefined;
 
-      // Finds a reference suggestion column and sets it as the current reference value.
-      const columnData = tableData.getDistinctValues(origDisplayCol.colId(), 100);
-      if (!columnData) { break; }
-      columnData.delete(gristTypes.getDefaultForType(origCol.type()));
+      let suggestedColRef: number;
+      let suggestedTableId: string;
+      const origColTypeInfo = gristTypes.extractInfoFromColType(origCol.type.peek());
+      if (!optTableId && origColTypeInfo.type === "Ref" || origColTypeInfo.type === "RefList") {
+        // When converting between Ref and Reflist, initially suggest the same table and visible column.
+        // When converting, if the table is the same, it's a special case.
+        // The visible column will not affect conversion.
+        // It will simply wrap the reference (row ID) in a list or extract the one element of a reference list.
+        suggestedColRef = origCol.visibleCol.peek();
+        suggestedTableId = origColTypeInfo.tableId;
+      } else {
+        // Finds a reference suggestion column and sets it as the current reference value.
+        const columnData = tableData.getDistinctValues(origDisplayCol.colId(), 100);
+        if (!columnData) { break; }
+        columnData.delete(gristTypes.getDefaultForType(origCol.type()));
 
-      // 'findColFromValues' function requires an array since it sends the values to the sandbox.
-      const matches: number[] = await docModel.docData.findColFromValues(Array.from(columnData), 2, optTableId);
-      const suggestedColRef = matches.find(match => match !== origCol.getRowId());
-      if (!suggestedColRef) { break; }
-      const suggestedCol = docModel.columns.getRowModel(suggestedColRef);
-      const suggestedTableId = suggestedCol.table().tableId();
-      if (optTableId && suggestedTableId !== optTableId) {
-        console.warn("Inappropriate column received from findColFromValues");
-        break;
+        // 'findColFromValues' function requires an array since it sends the values to the sandbox.
+        const matches: number[] = await docModel.docData.findColFromValues(Array.from(columnData), 2, optTableId);
+        suggestedColRef = matches.find(match => match !== origCol.getRowId())!;
+        if (!suggestedColRef) { break; }
+        const suggestedCol = docModel.columns.getRowModel(suggestedColRef);
+        suggestedTableId = suggestedCol.table().tableId();
+        if (optTableId && suggestedTableId !== optTableId) {
+          console.warn("Inappropriate column received from findColFromValues");
+          break;
+        }
       }
       colInfo.type = `${toType}:${suggestedTableId}`;
       colInfo.visibleCol = suggestedColRef;
@@ -163,11 +174,9 @@ export async function prepTransformColInfo(docModel: DocModel, origCol: ColumnRe
     }
   }
 
-  const newOptions = UserType.mergeOptions(widgetOptions || {}, colInfo.type);
   if (widgetOptions) {
     colInfo.widgetOptions = JSON.stringify(widgetOptions);
   }
-  colInfo.formula = getDefaultFormula(docModel, origCol, colInfo.type, colInfo.visibleCol, newOptions);
   return colInfo;
 }
 
@@ -184,62 +193,6 @@ export async function setDisplayFormula(
   }
 }
 
-// Given the original column and info about the new column, returns the formula to use for the
-// transform column to do the transformation.
-export function getDefaultFormula(
-  docModel: DocModel, origCol: ColumnRec, newType: string,
-  newVisibleCol: number, newWidgetOptions: any): string {
-
-  const colId = origCol.colId();
-  const oldVisibleColName = isReferenceCol(origCol) ?
-    getVisibleColName(docModel, origCol.visibleCol()) : undefined;
-
-  let origValFormula = oldVisibleColName ?
-    // The `str()` below converts AltText to plain text.
-    `($${colId}.${oldVisibleColName}
-    if ISREF($${colId}) or ISREFLIST($${colId})
-    else str($${colId}))`
-    : `$${colId}`;
-
-  if (origCol.type.peek() === 'ChoiceList') {
-    origValFormula = `grist.ChoiceList.toString($${colId})`;
-  }
-
-  const toTypePure: string = gristTypes.extractTypeFromColType(newType);
-
-  // The args are used to construct the call to grist.TYPE.typeConvert(value, [params]).
-  // Optional parameters depend on the type; see sandbox/grist/usertypes.py
-  const args: string[] = [origValFormula];
-  switch (toTypePure) {
-    case 'Ref':
-    case 'RefList':
-    {
-      const table = gutil.removePrefix(newType, toTypePure + ":");
-      args.push(table || 'None');
-      const visibleColName = getVisibleColName(docModel, newVisibleCol);
-      if (visibleColName) {
-        args.push(q(visibleColName));
-      }
-      break;
-    }
-    case 'Date': {
-      args.push(q(newWidgetOptions.dateFormat));
-      break;
-    }
-    case 'DateTime': {
-      const timezone = gutil.removePrefix(newType, "DateTime:") || '';
-      const format = newWidgetOptions.dateFormat + ' ' + newWidgetOptions.timeFormat;
-      args.push(q(format), q(timezone));
-      break;
-    }
-  }
-  return `grist.${gristTypes.getGristType(toTypePure)}.typeConvert(${args.join(', ')})`;
-}
-
-function q(value: string): string {
-  return "'" + value.replace(/'/g, "\\'") + "'";
-}
-
 // Returns the name of the visibleCol given its rowId.
 function getVisibleColName(docModel: DocModel, visibleColRef: number): string|undefined {
   return visibleColRef ? docModel.columns.getRowModel(visibleColRef).colId() : undefined;

app/client/components/TypeTransform.ts

@@ -10,13 +10,12 @@ import {ColumnTransform} from 'app/client/components/ColumnTransform';
 import {GristDoc} from 'app/client/components/GristDoc';
 import * as TypeConversion from 'app/client/components/TypeConversion';
 import {reportError} from 'app/client/models/errors';
-import * as modelUtil from 'app/client/models/modelUtil';
 import {cssButtonRow} from 'app/client/ui/RightPanel';
 import {basicButton, primaryButton} from 'app/client/ui2018/buttons';
 import {testId} from 'app/client/ui2018/cssVars';
 import {FieldBuilder} from 'app/client/widgets/FieldBuilder';
 import {NewAbstractWidget} from 'app/client/widgets/NewAbstractWidget';
-import {ColValues} from 'app/common/DocActions';
+import {ColValues, UserAction} from 'app/common/DocActions';
 import {Computed, dom, fromKo, Observable} from 'grainjs';
 import isEmpty = require('lodash/isEmpty');
 import pickBy = require('lodash/pickBy');
@@ -86,15 +85,6 @@ export class TypeTransform extends ColumnTransform {
     );
   }
 
-  protected async resetToDefaultFormula() {
-    if (!this.isFinalizing()) {
-      const toType = this.transformColumn.type.peek();
-      const formula = TypeConversion.getDefaultFormula(this.gristDoc.docModel, this.origColumn,
-        toType, this.field.visibleColRef(), this.field.widgetOptionsJson());
-      await modelUtil.setSaveValue(this.transformColumn.formula, formula);
-    }
-  }
-
   /**
    * Overrides parent method to initialize the transform column with guesses as to the particular
    * type and column options.
@@ -103,20 +93,55 @@ export class TypeTransform extends ColumnTransform {
   protected async addTransformColumn(toType: string) {
     const docModel = this.gristDoc.docModel;
     const colInfo = await TypeConversion.prepTransformColInfo(docModel, this.origColumn, this.origDisplayCol, toType);
-    const newColInfo = await this._tableData.sendTableAction(['AddColumn', 'gristHelper_Transform', colInfo]);
-    const tcol = docModel.columns.getRowModel(newColInfo.colRef);
-    await TypeConversion.setDisplayFormula(docModel, tcol);
-    return newColInfo.colRef;
+    const newColInfos = await this._tableData.sendTableActions([
+      ['AddColumn', 'gristHelper_Converted', {...colInfo, isFormula: false, formula: ''}],
+      ['AddColumn', 'gristHelper_Transform', colInfo],
+    ]);
+    const transformColRef = newColInfos[1].colRef;
+    this.transformColumn = docModel.columns.getRowModel(transformColRef);
+    await this.convertValues();
+    return transformColRef;
+  }
+
+  protected convertValuesActions(): UserAction[] {
+    const tableId = this._tableData.tableId;
+    const srcColId = this.origColumn.colId.peek();
+    const dstColId = "gristHelper_Converted";
+    const type = this.transformColumn.type.peek();
+    const widgetOptions = this.transformColumn.widgetOptions.peek();
+    const visibleColRef = this.transformColumn.visibleCol.peek();
+    return [['ConvertFromColumn', tableId, srcColId, dstColId, type, widgetOptions, visibleColRef]];
+  }
+
+  protected async convertValues() {
+    await Promise.all([
+      this.gristDoc.docData.sendActions(this.convertValuesActions()),
+      TypeConversion.setDisplayFormula(this.gristDoc.docModel, this.transformColumn),
+    ]);
+  }
+
+  protected executeActions(): UserAction[] {
+    return [...this.convertValuesActions(), ...super.executeActions()];
   }
 
   /**
    * Overrides parent method to subscribe to changes to the transform column.
    */
   protected postAddTransformColumn() {
-    // When a user-initiated change is saved to type or widgetOptions, update the formula.
-    this.autoDispose(this.transformColumn.type.subscribe(this.resetToDefaultFormula, this, "save"));
-    this.autoDispose(this.transformColumn.visibleCol.subscribe(this.resetToDefaultFormula, this, "save"));
-    this.autoDispose(this.field.widgetOptionsJson.subscribe(this.resetToDefaultFormula, this, "save"));
+    // When a user-initiated change is saved to type or widgetOptions, reconvert the values
+    // Need to subscribe to both 'change' and 'save' for type which can come from setting the type itself
+    // or e.g. a change to DateTime timezone.
+    this.autoDispose(this.transformColumn.type.subscribe(this.convertValues, this, "change"));
+    this.autoDispose(this.transformColumn.type.subscribe(this.convertValues, this, "save"));
+    this.autoDispose(this.transformColumn.visibleCol.subscribe(this.convertValues, this, "save"));
+    this.autoDispose(this.field.widgetOptionsJson.subscribe(this.convertValues, this, "save"));
+  }
+
+  /**
+   * Overrides parent method to delete extra column
+   */
+  protected cleanup() {
+    void this._tableData.sendTableAction(['RemoveColumn', 'gristHelper_Converted']);
   }
 
   /**
@@ -129,9 +154,10 @@ export class TypeTransform extends ColumnTransform {
     const tcol = this.transformColumn;
     const changedInfo = pickBy(colInfo, (val, key) =>
       (val !== tcol[key as keyof TypeConversion.ColInfo].peek()));
-    return Promise.all([
-      isEmpty(changedInfo) ? undefined : tcol.updateColValues(changedInfo as ColValues),
-      TypeConversion.setDisplayFormula(docModel, tcol, changedInfo.visibleCol)
-    ]);
+    if (!isEmpty(changedInfo)) {
+      // Update the transform column, particularly the type.
+      // This will trigger the subscription in postAddTransformColumn and lead to calling convertValues.
+      await tcol.updateColValues(changedInfo as ColValues);
+    }
   }
 }

app/client/models/DocModel.ts

@@ -22,7 +22,7 @@ import {urlState} from 'app/client/models/gristUrlState';
 import * as MetaRowModel from 'app/client/models/MetaRowModel';
 import * as MetaTableModel from 'app/client/models/MetaTableModel';
 import * as rowset from 'app/client/models/rowset';
-import {isHiddenTable} from 'app/client/models/isHiddenTable';
+import {isHiddenTable} from 'app/common/isHiddenTable';
 import {schema, SchemaTypes} from 'app/common/schema';
 
 import {ACLRuleRec, createACLRuleRec} from 'app/client/models/entities/ACLRuleRec';

app/client/models/entities/ColumnRec.ts

@@ -2,8 +2,13 @@ import {KoArray} from 'app/client/lib/koArray';
 import {DocModel, IRowModel, recordSet, refRecord, TableRec, ViewFieldRec} from 'app/client/models/DocModel';
 import {jsonObservable, ObjObservable} from 'app/client/models/modelUtil';
 import * as gristTypes from 'app/common/gristTypes';
-import {getReferencedTableId, isFullReferencingType} from 'app/common/gristTypes';
-import {BaseFormatter, createFormatter} from 'app/common/ValueFormatter';
+import {getReferencedTableId} from 'app/common/gristTypes';
+import {
+  BaseFormatter,
+  createFullFormatterRaw,
+  createVisibleColFormatterRaw,
+  FullFormatterArgs
+} from 'app/common/ValueFormatter';
 import * as ko from 'knockout';
 
 // Represents a column in a user-defined table.
@@ -124,38 +129,23 @@ export function createColumnRec(this: ColumnRec, docModel: DocModel): void {
 
   // Helper for Reference/ReferenceList columns, which returns a formatter according to the visibleCol
   // associated with this column. If no visible column available, return formatting for the column itself.
-  this.visibleColFormatter = ko.pureComputed(() => visibleColFormatterForRec(this, this, docModel));
+  this.visibleColFormatter = ko.pureComputed(() => formatterForRec(this, this, docModel, 'vcol'));
 
-  this.formatter = ko.pureComputed(() => formatterForRec(this, this, docModel, this.visibleColFormatter()));
-}
-
-export function visibleColFormatterForRec(
-  rec: ColumnRec | ViewFieldRec, colRec: ColumnRec, docModel: DocModel
-): BaseFormatter {
-  const vcol = rec.visibleColModel();
-  const documentSettings = docModel.docInfoRow.documentSettingsJson();
-  const type = colRec.type();
-  if (isFullReferencingType(type)) {
-    if (vcol.getRowId() === 0) {
-      // This column displays the Row ID, e.g. Table1[2]
-      // referencedTableId may actually be empty if the table is hidden
-      const referencedTableId: string = colRec.refTable()?.tableId() || "";
-      return createFormatter('Id', {tableId: referencedTableId}, documentSettings);
-    } else {
-      return createFormatter(vcol.type(), vcol.widgetOptionsJson(), documentSettings);
-    }
-  } else {
-    // For non-reference columns, there's no 'visible column' and we just return a regular formatter
-    return createFormatter(type, rec.widgetOptionsJson(), documentSettings);
-  }
+  this.formatter = ko.pureComputed(() => formatterForRec(this, this, docModel, 'full'));
 }
 
 export function formatterForRec(
-  rec: ColumnRec | ViewFieldRec, colRec: ColumnRec, docModel: DocModel, visibleColFormatter: BaseFormatter
+  rec: ColumnRec | ViewFieldRec, colRec: ColumnRec, docModel: DocModel, kind: 'full' | 'vcol'
 ): BaseFormatter {
-  const type = colRec.type();
-  // Ref/RefList columns delegate most formatting to the visibleColFormatter
-  const widgetOpts = {...rec.widgetOptionsJson(), visibleColFormatter};
-  const documentSettings = docModel.docInfoRow.documentSettingsJson();
-  return createFormatter(type, widgetOpts, documentSettings);
+  const vcol = rec.visibleColModel();
+  const func = kind === 'full' ? createFullFormatterRaw : createVisibleColFormatterRaw;
+  const args: FullFormatterArgs = {
+    docData: docModel.docData,
+    type: colRec.type(),
+    widgetOpts: rec.widgetOptionsJson(),
+    visibleColType: vcol?.type(),
+    visibleColWidgetOpts: vcol?.widgetOptionsJson(),
+    docSettings: docModel.docInfoRow.documentSettingsJson(),
+  };
+  return func(args);
 }

app/client/models/entities/ViewFieldRec.ts

@@ -1,5 +1,5 @@
 import {ColumnRec, DocModel, IRowModel, refRecord, ViewSectionRec} from 'app/client/models/DocModel';
-import {formatterForRec, visibleColFormatterForRec} from 'app/client/models/entities/ColumnRec';
+import {formatterForRec} from 'app/client/models/entities/ColumnRec';
 import * as modelUtil from 'app/client/models/modelUtil';
 import * as UserType from 'app/client/widgets/UserType';
 import {DocumentSettings} from 'app/common/DocumentSettings';
@@ -172,13 +172,14 @@ export function createViewFieldRec(this: ViewFieldRec, docModel: DocModel): void
 
   // Helper for Reference/ReferenceList columns, which returns a formatter according to the visibleCol
   // associated with this field. If no visible column available, return formatting for the field itself.
-  this.visibleColFormatter = ko.pureComputed(() => visibleColFormatterForRec(this, this.column(), docModel));
+  this.visibleColFormatter = ko.pureComputed(() => formatterForRec(this, this.column(), docModel, 'vcol'));
 
-  this.formatter = ko.pureComputed(() => formatterForRec(this, this.column(), docModel, this.visibleColFormatter()));
+  this.formatter = ko.pureComputed(() => formatterForRec(this, this.column(), docModel, 'full'));
 
   this.createValueParser = function() {
     const fieldRef = this.useColOptions.peek() ? undefined : this.id.peek();
-    return createParser(docModel.docData, this.colRef.peek(), fieldRef);
+    const parser = createParser(docModel.docData, this.colRef.peek(), fieldRef);
+    return parser.cleanParse.bind(parser);
   };
 
   // The widgetOptions to read and write: either the column's or the field's own.

app/client/models/isHiddenTable.ts

@@ -1,12 +0,0 @@
-import {RowId} from 'app/client/models/rowset';
-import {TableData} from 'app/client/models/TableData';
-
-/**
- * Return whether a table identified by the rowId of its metadata record, should normally be
- * hidden from the user (e.g. as an option in the page-widget picker).
- */
-export function isHiddenTable(tablesData: TableData, tableRef: RowId): boolean {
-  const tableId = tablesData.getValue(tableRef, 'tableId') as string|undefined;
-  return tablesData.getValue(tableRef, 'summarySourceTable') !== 0 ||
-    Boolean(tableId?.startsWith('GristHidden'));
-}

app/client/ui/Pages.ts

@@ -3,7 +3,7 @@ import { duplicatePage } from "app/client/components/duplicatePage";
 import { GristDoc } from "app/client/components/GristDoc";
 import { PageRec } from "app/client/models/DocModel";
 import { urlState } from "app/client/models/gristUrlState";
-import { isHiddenTable } from 'app/client/models/isHiddenTable';
+import { isHiddenTable } from 'app/common/isHiddenTable';
 import * as MetaTableModel from "app/client/models/MetaTableModel";
 import { find as findInTree, fromTableData, TreeItemRecord, TreeRecord,
          TreeTableData} from "app/client/models/TreeModel";