(core) Suggest correct table when converting to RefList

Summary: RecordSets now have new encoding and rendering analogous to Records: `['r', 'Table', [1, 2, 3]]` and `Table[[1, 2, 3]]`.

Test Plan: Added to nbrowser/TypeChange.ts.

Reviewers: dsagal

Reviewed By: dsagal

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

app/client/components/TypeConversion.ts

@@ -43,7 +43,7 @@ export function addColTypeSuffix(type: string, column: ColumnRec, docModel: DocM
 
 /**
  * Looks through the data of the given column to find the first value of the form
- * [R, <tableId>, <rowId>] (a Reference value returned from a formula), and returns the tableId
+ * [R|r, <tableId>, <rowId>] (a Reference(List) value returned from a formula), and returns the tableId
  * from that.
  */
 function getRefTableIdFromData(docModel: DocModel, column: ColumnRec): string|null {
@@ -51,12 +51,21 @@ function getRefTableIdFromData(docModel: DocModel, column: ColumnRec): string|nu
   const columnData = tableData && tableData.getColValues(column.colId());
   if (columnData) {
     for (const value of columnData) {
-      if (gristTypes.isObject(value) && value[0] === 'R') {
+      if (gristTypes.isReferencing(value)) {
         return value[1];
+      } else if (gristTypes.isList(value)) {
+        const item = value[1];
+        if (gristTypes.isReference(item)) {
+          return item[1];
+        }
       } else if (typeof value === 'string') {
-        // If it looks like a formatted Ref value (e.g. "Table1[123]"), and the tableId is valid,
+        // If it looks like a formatted Ref(List) value, e.g:
+        //   - Table1[123]
+        //   - [Table1[1], Table1[2], Table1[3]]
+        //   - Table1[[1, 2, 3]]
+        // and the tableId is valid,
         // use it. (This helps if a Ref-returning formula column got converted to Text first.)
-        const match = value.match(/^(\w+)\[\d+\]/);
+        const match = value.match(/^\[?(\w+)\[/);
         if (match && docModel.docData.getTable(match[1])) {
           return match[1];
         }

app/common/gristTypes.ts

@@ -24,6 +24,7 @@ export const enum GristObjCode {
   Skip            = 'S',
   Censored        = 'C',
   Reference       = 'R',
+  ReferenceList   = 'r',
   Exception       = 'E',
   Pending         = 'P',
   Unmarshallable  = 'U',
@@ -142,10 +143,34 @@ export function isCensored(value: CellValue): value is [GristObjCode.Censored] {
 /**
  * Returns whether a value (as received in a DocAction) represents a list.
  */
- export function isList(value: CellValue): value is [GristObjCode.List, ...unknown[]] {
+export function isList(value: CellValue): value is [GristObjCode.List, ...CellValue[]] {
   return Array.isArray(value) && value[0] === GristObjCode.List;
 }
 
+/**
+ * Returns whether a value (as received in a DocAction) represents a reference to a record.
+ */
+export function isReference(value: CellValue): value is [GristObjCode.Reference, string, number] {
+  return Array.isArray(value) && value[0] === GristObjCode.Reference;
+}
+
+/**
+ * Returns whether a value (as received in a DocAction) represents a reference list (RecordSet).
+ */
+export function isReferenceList(value: CellValue): value is [GristObjCode.ReferenceList, string, number[]] {
+  return Array.isArray(value) && value[0] === GristObjCode.ReferenceList;
+}
+
+/**
+ * Returns whether a value (as received in a DocAction) represents a reference or reference list.
+ */
+export function isReferencing(value: CellValue):
+  value is [GristObjCode.ReferenceList|GristObjCode.Reference, string, number[]|number]
+{
+  return Array.isArray(value) &&
+    (value[0] === GristObjCode.ReferenceList || value[0] === GristObjCode.Reference);
+}
+
 /**
  * Returns whether a value (as received in a DocAction) represents a list or is null,
  * which is a valid value for list types in grist.

app/plugin/objtypes.ts

@@ -50,6 +50,18 @@ export class Reference {
   }
 }
 
+/**
+ * A ReferenceList represents a reference to a number of rows in a table. It is simply a pair of a string tableId
+ * and a numeric array rowIds.
+ */
+export class ReferenceList {
+  constructor(public tableId: string, public rowIds: number[]) {}
+
+  public toString(): string {
+    return `${this.tableId}[[${this.rowIds}]]`;
+  }
+}
+
 /**
  * A RaisedException represents a formula error. It includes the exception name, message, and
  * optional details.
@@ -140,6 +152,8 @@ export function encodeObject(value: unknown): CellValue {
       return null;
     } else if (value instanceof Reference) {
       return ['R', value.tableId, value.rowId];
+    } else if (value instanceof ReferenceList) {
+      return ['r', value.tableId, value.rowIds];
     } else if (value instanceof Date) {
       const timestamp = value.valueOf() / 1000;
       if ('timezone' in value) {
@@ -185,6 +199,7 @@ export function decodeObject(value: CellValue): unknown {
       case 'L': return (args as CellValue[]).map(decodeObject);
       case 'O': return mapValues(args[0] as {[key: string]: CellValue}, decodeObject, {sort: true});
       case 'P': return new PendingValue();
+      case 'r': return new ReferenceList(String(args[0]), args[1]);
       case 'R': return new Reference(String(args[0]), args[1]);
       case 'S': return new SkipValue();
       case 'C': return new CensoredValue();

sandbox/grist/objtypes.py

@@ -183,11 +183,12 @@ def encode_object(value):
       return ['d', moment.date_to_ts(value)]
     elif isinstance(value, RaisedException):
       return ['E'] + value.encode_args()
-    elif isinstance(value, (list, tuple, RecordList)):
+    elif isinstance(value, (list, tuple)):
       return ['L'] + [encode_object(item) for item in value]
     elif isinstance(value, records.RecordSet):
-      # Represent RecordSet (e.g. result of lookupRecords) in the same way as a RecordList.
+      return ['r', value._table.table_id, value._row_ids]
-      return ['L'] + [encode_object(int(item)) for item in value]
+    elif isinstance(value, RecordSetStub):
+      return ['r', value.table_id, value.row_ids]
     elif isinstance(value, dict):
       if not all(isinstance(key, six.string_types) for key in value):
         raise UnmarshallableError("Dict with non-string keys")
@@ -217,6 +218,8 @@ def decode_object(value):
     args = value[1:]
     if code == 'R':
       return RecordStub(args[0], args[1])
+    elif code == 'r':
+      return RecordSetStub(args[0], args[1])
     elif code == 'D':
       return moment.ts_to_dt(args[0], moment.Zone(args[1]))
     elif code == 'd':
@@ -323,3 +326,9 @@ class RecordStub(object):
   def __init__(self, table_id, row_id):
     self.table_id = table_id
     self.row_id = row_id
+
+
+class RecordSetStub(object):
+  def __init__(self, table_id, row_ids):
+    self.table_id = table_id
+    self.row_ids = row_ids

sandbox/grist/records.py

@@ -185,6 +185,9 @@ class RecordSet(object):
       return self._get_col(name)
     return self._table._attribute_error(name, self._source_relation)
 
+  def __repr__(self):
+    return "%s[%s]" % (self._table.table_id, self._row_ids)
+
   def _clone_with_relation(self, src_relation):
     return self._table.RecordSet(self._row_ids,
                                  relation=src_relation.compose(self._source_relation),

sandbox/grist/test_formula_undo.py

@@ -1,6 +1,8 @@
 # pylint: disable=line-too-long
 import testsamples
 import test_engine
+from objtypes import RecordSetStub
+
 
 class TestFormulaUndo(test_engine.EngineTestCase):
   def setUp(self):
@@ -27,12 +29,12 @@ return '#%s %s' % (table.my_counter, $schoolName)
 
     self.assertTableData("Students", cols="subset", data=[
       ["id", "schoolName", "schoolCities",                   "counter"     ],
-      [1,    "Columbia",   [1, 2],         "#1 Columbia",],
+      [1,    "Columbia",   RecordSetStub("Schools", [1, 2]), "#1 Columbia",],
-      [2,    "Yale",       [3, 4],         "#2 Yale",    ],
+      [2,    "Yale",       RecordSetStub("Schools", [3, 4]), "#2 Yale",    ],
-      [3,    "Columbia",   [1, 2],         "#3 Columbia",],
+      [3,    "Columbia",   RecordSetStub("Schools", [1, 2]), "#3 Columbia",],
-      [4,    "Yale",       [3, 4],         "#4 Yale",    ],
+      [4,    "Yale",       RecordSetStub("Schools", [3, 4]), "#4 Yale",    ],
-      [5,    "Eureka",     [],             "#5 Eureka",  ],
+      [5,    "Eureka",     RecordSetStub("Schools", []),     "#5 Eureka",  ],
-      [6,    "Yale",       [3, 4],         "#6 Yale",    ],
+      [6,    "Yale",       RecordSetStub("Schools", [3, 4]), "#6 Yale",    ],
     ])
 
     # Applying an action produces expected changes to all formula columns, and corresponding undos.
@@ -41,14 +43,14 @@ return '#%s %s' % (table.my_counter, $schoolName)
       "stored": [
         ["UpdateRecord", "Students", 6, {"schoolName": "Columbia"}],
         ["UpdateRecord", "Students", 6, {"counter": "#7 Columbia"}],
-        ["UpdateRecord", "Students", 6, {"schoolCities": ["L", 1, 2]}],
+        ["UpdateRecord", "Students", 6, {"schoolCities": ["r", "Schools", [1, 2]]}],
         ["UpdateRecord", "Students", 6, {"schoolIds": "1:2"}],
       ],
       "direct": [True, False, False, False],
       "undo": [
         ["UpdateRecord", "Students", 6, {"schoolName": "Yale"}],
         ["UpdateRecord", "Students", 6, {"counter": "#6 Yale"}],
-        ["UpdateRecord", "Students", 6, {"schoolCities": ["L", 3, 4]}],
+        ["UpdateRecord", "Students", 6, {"schoolCities": ["r", "Schools", [3, 4]]}],
         ["UpdateRecord", "Students", 6, {"schoolIds": "3:4"}],
       ],
     })
@@ -63,7 +65,7 @@ return '#%s %s' % (table.my_counter, $schoolName)
     self.assertOutActions(out_actions, {
       "stored": [
         ["UpdateRecord", "Students", 6, {"schoolIds": "3:4"}],
-        ["UpdateRecord", "Students", 6, {"schoolCities": ["L", 3, 4]}],
+        ["UpdateRecord", "Students", 6, {"schoolCities": ["r", "Schools", [3, 4]]}],
         ["UpdateRecord", "Students", 6, {"counter": "#6 Yale"}],
         ["UpdateRecord", "Students", 6, {"schoolName": "Yale"}],
         ["UpdateRecord", "Students", 6, {"counter": "#8 Yale"}],
@@ -71,7 +73,7 @@ return '#%s %s' % (table.my_counter, $schoolName)
       "direct": [True, True, True, True, False],  # undos currently fully direct; formula update is indirect.
       "undo": [
         ["UpdateRecord", "Students", 6, {"schoolIds": "1:2"}],
-        ["UpdateRecord", "Students", 6, {"schoolCities": ["L", 1, 2]}],
+        ["UpdateRecord", "Students", 6, {"schoolCities": ["r", "Schools", [1, 2]]}],
         ["UpdateRecord", "Students", 6, {"counter": "#7 Columbia"}],
         ["UpdateRecord", "Students", 6, {"schoolName": "Columbia"}],
         ["UpdateRecord", "Students", 6, {"counter": "#6 Yale"}],
@@ -80,12 +82,14 @@ return '#%s %s' % (table.my_counter, $schoolName)
 
     self.assertTableData("Students", cols="subset", data=[
       ["id", "schoolName", "schoolCities",                    "counter" ],
-      [1,    "Columbia",   [1, 2],         "#1 Columbia",],
+      [1,    "Columbia",   RecordSetStub("Schools", [1, 2]),  "#1 Columbia"],
-      [2,    "Yale",       [3, 4],         "#2 Yale",    ],
+      [2,    "Yale",       RecordSetStub("Schools", [3, 4]),  "#2 Yale",   ],
-      [3,    "Columbia",   [1, 2],         "#3 Columbia",],
+      [3,    "Columbia",   RecordSetStub("Schools", [1, 2]),  "#3 Columbia"],
-      [4,    "Yale",       [3, 4],         "#4 Yale",    ],
+      [4,    "Yale",       RecordSetStub("Schools", [3, 4]),  "#4 Yale",   ],
-      [5,    "Eureka",     [],             "#5 Eureka",  ],
+      [5,    "Eureka",     RecordSetStub("Schools", []),      "#5 Eureka", ],
-      [6,    "Yale",       [3, 4],         "#8 Yale",    ],   # This counter got updated
+
+      # This counter got updated
+      [6,    "Yale",       RecordSetStub("Schools", [3, 4]),  "#8 Yale",   ],
     ])
 
   def test_save_to_empty_column(self):

sandbox/grist/test_renames.py

@@ -299,6 +299,12 @@ class TestRenames(test_engine.EngineTestCase):
         "type": ["Ref:Persons", "Any"],
         "formula": ["Persons.lookupOne(addr=$id)", "Persons.lookupRecords(addr=$id)"]
       }],
+      ["BulkUpdateRecord", "Address", [11, 12, 13, 14], {
+        "people": [["r", "Persons", [4]],
+                   ["r", "Persons", [1, 3]],
+                   ["r", "Persons", [2]],
+                   ["r", "Persons", []]]
+      }],
     ]})
 
   def test_rename_table_autocomplete(self):