(core) Add other direction of linking by reflist

Summary:
Allows selecting by a reflist in another table. This generalises cursor-linking with a ref column, but now it's filter linking.

Added another case to LinkingState where the source column is a reflist to the target table, filtering by the id column.

Updated convertQueryFromRefs and related functions to handle this since the id column has no column ref. In this case the string 'id' is used instead of a number.

LinkingState also checks if the source value is a reflist and uses that as the list of filter values instead of a single-element list of the cell value.

Indirect linking also works, where the source and target columns both are both references to the same table. This was the plan for a source reflist and target ref column.
I was surprised to see it also works perfectly when both columns are reflists, and it filters rows where there's an intersection!

Adding rows to the target section using the selected source record for default values is iffy. When filtering by row ID, there's no column for defaults, so the new row disappears.
For a source reflist and target ref, the first value of the reflist is the default, which is okayish. When both are reflists, the full source reflist is the default for the target column.
This seems like a bit much but just using the first value seems a bit arbitrary when there's room for all of them?

While doing all this I noticed an unrelated bug which I fixed as I was refactoring. Previously cursor linking based on a reference column did not update the cursor in the link target
when the value of the selected reference cell changed. Now cursor linking uses a floating row model like most other cases to observe the value correctly.

Test Plan: Extended SelectByRefList test and fixture, added previously failing test to RightPanelSelectBy.

Reviewers: dsagal

Reviewed By: dsagal

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

app/client/components/BaseView.js

@@ -386,7 +386,7 @@ BaseView.prototype._parsePasteForView = function(data, cols) {
 BaseView.prototype._getDefaultColValues = function() {
   const {filters, operations} = this._linkingFilter.peek();
   return _.mapObject(
-      _.pick(filters, v => (v.length > 0)),
+      _.pick(filters, (value, key) => value.length > 0 && key !== "id"),
       (value, key) => operations[key] === "intersects" ? encodeObject(value) : value[0]
   );
 };

app/client/components/LinkingState.ts

@@ -1,10 +1,13 @@
 import {GristDoc} from "app/client/components/GristDoc";
 import {DataRowModel} from "app/client/models/DataRowModel";
+import * as DataTableModel from "app/client/models/DataTableModel";
+import {ColumnRec} from "app/client/models/entities/ColumnRec";
 import {TableRec} from "app/client/models/entities/TableRec";
 import {ViewSectionRec} from "app/client/models/entities/ViewSectionRec";
+import {RowId} from "app/client/models/rowset";
 import {LinkConfig} from "app/client/ui/selectBy";
 import {ClientQuery, QueryOperation} from "app/common/ActiveDocAPI";
-import {isRefListType} from "app/common/gristTypes";
+import {isList, isRefListType} from "app/common/gristTypes";
 import * as gutil from "app/common/gutil";
 import {Disposable} from "grainjs";
 import * as  ko from "knockout";
@@ -55,51 +58,36 @@ type FilterColValues = Pick<ClientQuery, "filters" | "operations">;
  *    in the linked tgtSection.
  */
 export class LinkingState extends Disposable {
-  public readonly cursorPos: ko.Computed<number> | null;
+  // If linking affects target section's cursor, this will be a computed for the cursor rowId.
-  public readonly filterColValues: ko.Computed<FilterColValues> | null;
+  public readonly cursorPos?: ko.Computed<number>;
+
+  // If linking affects filtering, this is a computed for the current filtering state, as a
+  // {[colId]: colValues} mapping, with a dependency on srcSection.activeRowId()
+  public readonly filterColValues?: ko.Computed<FilterColValues>;
+
   private _srcSection: ViewSectionRec;
+  private _srcTableModel: DataTableModel;
+  private _srcCol: ColumnRec;
+  private _srcColId: string | undefined;
 
   constructor(gristDoc: GristDoc, linkConfig: LinkConfig) {
     super();
-    const {srcSection, srcColId, tgtSection, tgtCol, tgtColId} = linkConfig;
+    const {srcSection, srcCol, srcColId, tgtSection, tgtCol, tgtColId} = linkConfig;
     this._srcSection = srcSection;
+    this._srcCol = srcCol;
+    this._srcColId = srcColId;
+    this._srcTableModel = gristDoc.getTableModel(srcSection.table().tableId());
+    const srcTableData = this._srcTableModel.tableData;
 
-    const srcTableModel = gristDoc.getTableModel(srcSection.table().tableId());
-    const srcTableData = srcTableModel.tableData;
-
-    // Function from srcRowId (i.e. srcSection.activeRowId()) to the source value. It is used for
-    // filtering or for cursor positioning, depending on the setting of tgtCol.
-    const srcValueFunc = srcColId ? srcTableData.getRowPropFunc(srcColId)! : _.identity;
-
-    // If linking affects target section's cursor, this will be a computed for the cursor rowId.
-    this.cursorPos = null;
-
-    // If linking affects filtering, this is a computed for the current filtering state, as a
-    // {[colId]: colValues} mapping, with a dependency on srcSection.activeRowId(). Otherwise, null.
-    this.filterColValues = null;
-
-    // A computed that evaluates to a filter function to use, or null if not filtering. If
-    // filtering, depends on srcSection.activeRowId().
     if (tgtColId) {
-      const operations = {[tgtColId]: isRefListType(tgtCol.type()) ? 'intersects' : 'in' as QueryOperation};
+      const operation = isRefListType(tgtCol.type()) ? 'intersects' : 'in';
       if (srcColId) {
-        const srcRowModel = this.autoDispose(srcTableModel.createFloatingRowModel()) as DataRowModel;
+        this.filterColValues = this._srcCellFilter(tgtColId, operation);
-        const srcCell = srcRowModel.cells[srcColId];
-        // If no srcCell, linking is broken; do nothing. This shouldn't happen, but may happen
-        // transiently while the separate linking-related observables get updated.
-        if (srcCell) {
-          this.filterColValues = this.autoDispose(ko.computed(() => {
-            const srcRowId = srcSection.activeRowId();
-            srcRowModel.assign(srcRowId);
-            return {filters: {[tgtColId]: [srcCell()]}, operations} as FilterColValues;
-          }));
-        }
       } else {
-        this.filterColValues = this.autoDispose(ko.computed(() => {
+        this.filterColValues = this._simpleFilter(tgtColId, operation, (rowId => [rowId]));
-          const srcRowId = srcSection.activeRowId();
-          return {filters: {[tgtColId]: [srcRowId]}, operations} as FilterColValues;
-        }));
       }
+    } else if (srcColId && isRefListType(srcCol.type())) {
+      this.filterColValues = this._srcCellFilter('id', 'in');
     } else if (isSummaryOf(srcSection.table(), tgtSection.table())) {
       // We filter summary tables when a summary section is linked to a more detailed one without
       // specifying src or target column. The filtering is on the shared group-by column (i.e. all
@@ -128,11 +116,12 @@ export class LinkingState extends Disposable {
       // TODO: We should move the cursor, but don't currently it for summaries. For that, we need a
       // column or map representing the inverse of summary table's "group" column.
     } else {
-      this.cursorPos = this.autoDispose(ko.computed(() =>
+      const srcValueFunc = srcColId ? this._makeSrcCellGetter() : _.identity;
-        srcValueFunc(
+      if (srcValueFunc) {
-          srcSection.activeRowId() as number
+        this.cursorPos = this.autoDispose(ko.computed(() =>
-        ) as number
+          srcValueFunc(srcSection.activeRowId()) as number
-      ));
+        ));
+      }
     }
   }
 
@@ -142,4 +131,54 @@ export class LinkingState extends Disposable {
   public disableEditing(): boolean {
     return Boolean(this.filterColValues) && this._srcSection.activeRowId() === 'new';
   }
+
+  // Value for this.filterColValues filtering based on a single column
+  private _simpleFilter(
+    colId: string, operation: QueryOperation, valuesFunc: (rowId: RowId|null) => any[]
+  ): ko.Computed<FilterColValues> {
+    return this.autoDispose(ko.computed(() => {
+      const srcRowId = this._srcSection.activeRowId();
+      const values = valuesFunc(srcRowId);
+      return {filters: {[colId]: values}, operations: {[colId]: operation}} as FilterColValues;
+    }));
+  }
+
+  // Value for this.filterColValues based on the value in srcCol at the selected row
+  private _srcCellFilter(colId: string, operation: QueryOperation): ko.Computed<FilterColValues> | undefined {
+    const srcCellGetter = this._makeSrcCellGetter();
+    if (srcCellGetter) {
+      const isSrcRefList = isRefListType(this._srcCol.type());
+      return this._simpleFilter(colId, operation, rowId => {
+        const value = srcCellGetter(rowId);
+        if (isSrcRefList) {
+          if (isList(value)) {
+            return value.slice(1);
+          } else {
+            // The cell value is invalid, so the filter should be empty
+            return [];
+          }
+        } else {
+          return [value];
+        }
+      });
+    }
+  }
+
+  // Returns a function which returns the value of the cell
+  // in srcCol in the selected record of srcSection.
+  // Uses a row model to create a dependency on the cell's value,
+  // so changes to the cell value will notify observers
+  private _makeSrcCellGetter() {
+    const srcRowModel = this.autoDispose(this._srcTableModel.createFloatingRowModel()) as DataRowModel;
+    const srcCellObs = srcRowModel.cells[this._srcColId!];
+    // If no srcCellObs, linking is broken; do nothing. This shouldn't happen, but may happen
+    // transiently while the separate linking-related observables get updated.
+    if (!srcCellObs) {
+      return null;
+    }
+    return (rowId: RowId | null) => {
+      srcRowModel.assign(rowId);
+      return srcCellObs();
+    };
+  }
 }

app/client/models/QuerySet.ts

@@ -59,7 +59,8 @@ export interface QueryRefs {
   filterTuples: Array<FilterTuple>;
 }
 
-type FilterTuple = [number, QueryOperation, any[]];
+type ColRef = number | 'id';
+type FilterTuple = [ColRef, QueryOperation, any[]];
 
 /**
  * QuerySetManager keeps track of all queries for a GristDoc instance. It is also responsible for
@@ -332,7 +333,7 @@ export function getFilterFunc(docData: DocData, query: ClientQuery): RowFilterFu
 function convertQueryToRefs(docModel: DocModel, query: ClientQuery): QueryRefs {
   const tableRec: any = docModel.dataTables[query.tableId].tableMetaRow;
 
-  const colRefsByColId: {[colId: string]: number} = {};
+  const colRefsByColId: {[colId: string]: ColRef} = {id: 'id'};
   for (const col of tableRec.columns.peek().peek()) {
     colRefsByColId[col.colId.peek()] = col.getRowId();
   }
@@ -358,7 +359,7 @@ function convertQueryFromRefs(docModel: DocModel, queryRefs: QueryRefs): ClientQ
   const filters: {[colId: string]: any[]} = {};
   const operations: {[colId: string]: QueryOperation} = {};
   for (const [colRef, operation, values] of queryRefs.filterTuples) {
-    const colId = docModel.columns.getRowModel(colRef).colId.peek();
+    const colId = colRef === 'id' ? 'id' : docModel.columns.getRowModel(colRef).colId.peek();
     filters[colId] = values;
     operations[colId] = operation;
   }
@@ -388,7 +389,7 @@ function decodeQuery(queryKey: string): QueryRefs {
  */
 function makeQueryInvalidComputed(docModel: DocModel, queryRefs: QueryRefs): ko.Computed<boolean> {
   const tableFlag: ko.Observable<boolean> = docModel.tables.getRowModel(queryRefs.tableRef)._isDeleted;
-  const colFlags: Array<ko.Observable<boolean>> = queryRefs.filterTuples.map(
+  const colFlags: Array<ko.Observable<boolean> | null> = queryRefs.filterTuples.map(
-    ([colRef, , ]) => docModel.columns.getRowModel(colRef)._isDeleted);
+    ([colRef, , ]) => colRef === 'id' ? null : docModel.columns.getRowModel(colRef)._isDeleted);
-  return ko.computed(() => Boolean(tableFlag() || colFlags.some((c) => c())));
+  return ko.computed(() => Boolean(tableFlag() || colFlags.some((c) => c?.())));
 }

app/client/ui/selectBy.ts

@@ -210,7 +210,7 @@ export function linkId(link: IPageWidgetLink) {
 }
 
 // Returns link's properties from its identifier.
-export function linkFromId(linkid: string) {
+export function linkFromId(linkid: string): IPageWidgetLink {
   const [srcSectionRef, srcColRef, targetColRef] = JSON.parse(linkid);
   return {srcSectionRef, srcColRef, targetColRef};
 }