From 9916a2d919cdbf2523d36bcea3ff2470ba810266 Mon Sep 17 00:00:00 2001 From: Alex Hall Date: Fri, 20 Aug 2021 22:35:41 +0200 Subject: [PATCH] (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 | 17 ++++++++--- app/common/gristTypes.ts | 27 ++++++++++++++++- app/plugin/objtypes.ts | 15 ++++++++++ sandbox/grist/objtypes.py | 15 ++++++++-- sandbox/grist/records.py | 3 ++ sandbox/grist/test_formula_undo.py | 40 ++++++++++++++----------- sandbox/grist/test_renames.py | 6 ++++ 7 files changed, 97 insertions(+), 26 deletions(-) diff --git a/app/client/components/TypeConversion.ts b/app/client/components/TypeConversion.ts index 3a5be82e..d48bee5b 100644 --- a/app/client/components/TypeConversion.ts +++ b/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, , ] (a Reference value returned from a formula), and returns the tableId + * [R|r, , ] (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]; } diff --git a/app/common/gristTypes.ts b/app/common/gristTypes.ts index 76cbd7b0..dab8123a 100644 --- a/app/common/gristTypes.ts +++ b/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. diff --git a/app/plugin/objtypes.ts b/app/plugin/objtypes.ts index 65150f98..611dc2c9 100644 --- a/app/plugin/objtypes.ts +++ b/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(); diff --git a/sandbox/grist/objtypes.py b/sandbox/grist/objtypes.py index 807633a7..729dce1c 100644 --- a/sandbox/grist/objtypes.py +++ b/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 ['L'] + [encode_object(int(item)) for item in value] + return ['r', value._table.table_id, value._row_ids] + 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 diff --git a/sandbox/grist/records.py b/sandbox/grist/records.py index 39da0168..ccbf1c14 100644 --- a/sandbox/grist/records.py +++ b/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), diff --git a/sandbox/grist/test_formula_undo.py b/sandbox/grist/test_formula_undo.py index b9ecb0df..540d08e6 100644 --- a/sandbox/grist/test_formula_undo.py +++ b/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): @@ -26,13 +28,13 @@ return '#%s %s' % (table.my_counter, $schoolName) }]) self.assertTableData("Students", cols="subset", data=[ - ["id", "schoolName", "schoolCities", "counter" ], - [1, "Columbia", [1, 2], "#1 Columbia",], - [2, "Yale", [3, 4], "#2 Yale", ], - [3, "Columbia", [1, 2], "#3 Columbia",], - [4, "Yale", [3, 4], "#4 Yale", ], - [5, "Eureka", [], "#5 Eureka", ], - [6, "Yale", [3, 4], "#6 Yale", ], + ["id", "schoolName", "schoolCities", "counter" ], + [1, "Columbia", RecordSetStub("Schools", [1, 2]), "#1 Columbia",], + [2, "Yale", RecordSetStub("Schools", [3, 4]), "#2 Yale", ], + [3, "Columbia", RecordSetStub("Schools", [1, 2]), "#3 Columbia",], + [4, "Yale", RecordSetStub("Schools", [3, 4]), "#4 Yale", ], + [5, "Eureka", RecordSetStub("Schools", []), "#5 Eureka", ], + [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"}], @@ -79,13 +81,15 @@ return '#%s %s' % (table.my_counter, $schoolName) }) self.assertTableData("Students", cols="subset", data=[ - ["id", "schoolName", "schoolCities", "counter" ], - [1, "Columbia", [1, 2], "#1 Columbia",], - [2, "Yale", [3, 4], "#2 Yale", ], - [3, "Columbia", [1, 2], "#3 Columbia",], - [4, "Yale", [3, 4], "#4 Yale", ], - [5, "Eureka", [], "#5 Eureka", ], - [6, "Yale", [3, 4], "#8 Yale", ], # This counter got updated + ["id", "schoolName", "schoolCities", "counter" ], + [1, "Columbia", RecordSetStub("Schools", [1, 2]), "#1 Columbia"], + [2, "Yale", RecordSetStub("Schools", [3, 4]), "#2 Yale", ], + [3, "Columbia", RecordSetStub("Schools", [1, 2]), "#3 Columbia"], + [4, "Yale", RecordSetStub("Schools", [3, 4]), "#4 Yale", ], + [5, "Eureka", RecordSetStub("Schools", []), "#5 Eureka", ], + + # This counter got updated + [6, "Yale", RecordSetStub("Schools", [3, 4]), "#8 Yale", ], ]) def test_save_to_empty_column(self): diff --git a/sandbox/grist/test_renames.py b/sandbox/grist/test_renames.py index 1e18cfb6..0af60c7a 100644 --- a/sandbox/grist/test_renames.py +++ b/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):