(core) Support 'new' row in anchor links.

Summary:
- Anchor links with row of 'new' could be created but weren't parsed or used
  correctly. This fixes it.
- Also adds UIRowId type for row IDs which includes the special 'new' row. It's
  already been used in places as `number|'new'`, this diff gives it a name usable in app/common
  (it doesn't touch another name, RowId, that's been available in app/client).

Test Plan: Added a test assert for anchor links to new row

Reviewers: alexmojaki

Reviewed By: alexmojaki

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

app/client/components/Cursor.ts

@@ -8,17 +8,18 @@ import * as BaseView from 'app/client/components/BaseView';
 import * as commands from 'app/client/components/commands';
 import * as BaseRowModel from 'app/client/models/BaseRowModel';
 import {LazyArrayModel} from 'app/client/models/DataTableModel';
+import type {RowId} from 'app/client/models/rowset';
 import {Disposable} from 'grainjs';
 import * as ko from 'knockout';
 
 export interface CursorPos {
-  rowId?: number;
+  rowId?: RowId;
   rowIndex?: number;
   fieldIndex?: number;
   sectionId?: number;
 }
 
-function nullAsUndefined(value: number|null|undefined): number|undefined {
+function nullAsUndefined<T>(value: T|null|undefined): T|undefined {
   return value == null ? undefined : value;
 }
 
@@ -66,7 +67,7 @@ export class Cursor extends Disposable {
   public rowIndex: ko.Computed<number|null>;     // May be null when there are no rows.
   public fieldIndex: ko.Observable<number>;
 
-  private _rowId: ko.Observable<number|null>;    // May be null when there are no rows.
+  private _rowId: ko.Observable<RowId|null>;     // May be null when there are no rows.
 
   // The cursor's _rowId property is always fixed across data changes. When isLive is true,
   // the rowIndex of the cursor is recalculated to match _rowId. When false, they will

app/client/declarations.d.ts

@@ -291,13 +291,14 @@ declare module "app/client/models/DataTableModel" {
   import {SortedRowSet} from "app/client/models/rowset";
   import {TableData} from "app/client/models/TableData";
   import * as TableModel from "app/client/models/TableModel";
+  import {UIRowId} from "app/common/UIRowId";
 
   namespace DataTableModel {
     interface LazyArrayModel<T> extends KoArray<T | null> {
-      getRowId(index: number): number;
-      getRowIndex(index: number): number;
-      getRowIndexWithSub(rowId: number): number;
-      getRowModel(rowId: number): T|undefined;
+      getRowId(index: number): UIRowId;
+      getRowIndex(rowId: UIRowId): number;
+      getRowIndexWithSub(rowId: UIRowId): number;
+      getRowModel(rowId: UIRowId): T|undefined;
     }
   }
 

app/common/TableData.ts

@@ -1,16 +1,20 @@
 /**
  * TableData maintains a single table's data.
  */
+import {ActionDispatcher} from 'app/common/ActionDispatcher';
+import {BulkColValues, CellValue, ColInfo, ColInfoWithId, ColValues, DocAction,
+        isSchemaAction, ReplaceTableData, RowRecord, TableDataAction} from 'app/common/DocActions';
 import {getDefaultForType} from 'app/common/gristTypes';
+import {arrayRemove, arraySplice} from 'app/common/gutil';
+import {SchemaTypes} from "app/common/schema";
+import {UIRowId} from 'app/common/UIRowId';
 import fromPairs = require('lodash/fromPairs');
-import {ActionDispatcher} from './ActionDispatcher';
-import {BulkColValues, CellValue, ColInfo, ColInfoWithId, ColValues, DocAction,
-        isSchemaAction, ReplaceTableData, RowRecord, TableDataAction} from './DocActions';
-import {arrayRemove, arraySplice} from './gutil';
-import {SchemaTypes} from "./schema";
 
 export interface ColTypeMap { [colId: string]: string; }
 
+type RowFunc<T> = (rowId: number) => T;
+type UIRowFunc<T> = (rowId: UIRowId) => T;
+
 interface ColData {
   colId: string;
   type: string;
@@ -23,7 +27,7 @@ interface ColData {
  */
 export interface SkippableRows {
   // If there may be skippable rows, return a function to test rowIds for keeping.
-  getKeepFunc(): undefined | ((rowId: number|"new") => boolean);
+  getKeepFunc(): undefined | UIRowFunc<boolean>;
   // Get a special row id which represents a skipped sequence of rows.
   getSkipRowId(): number;
 }
@@ -149,9 +153,9 @@ export class TableData extends ActionDispatcher implements SkippableRows {
   /**
    * Returns the specified value from this table.
    */
-  public getValue(rowId: number, colId: string): CellValue|undefined {
+  public getValue(rowId: UIRowId, colId: string): CellValue|undefined {
     const colData = this._columns.get(colId);
-    const index = this._rowMap.get(rowId);
+    const index = this._rowMap.get(rowId as number);    // rowId of 'new' will not be found.
     return colData && index !== undefined ? colData.values[index] : undefined;
   }
 
@@ -159,16 +163,16 @@ export class TableData extends ActionDispatcher implements SkippableRows {
    * Given a column name, returns a function that takes a rowId and returns the value for that
    * column of that row. The returned function is faster than getValue() calls.
    */
-  public getRowPropFunc(colId: string): undefined | ((rowId: number|"new") => CellValue|undefined) {
+  public getRowPropFunc(colId: string): undefined | UIRowFunc<CellValue|undefined> {
     const colData = this._columns.get(colId);
     if (!colData) { return undefined; }
     const values = colData.values;
     const rowMap = this._rowMap;
-    return function(rowId: number|"new") { return rowId === "new" ? "new" : values[rowMap.get(rowId)!]; };
+    return function(rowId: UIRowId) { return values[rowMap.get(rowId as number)!]; };
   }
 
   // By default, no rows are skippable, all are kept.
-  public getKeepFunc(): undefined | ((rowId: number|"new") => boolean) {
+  public getKeepFunc(): undefined | UIRowFunc<boolean> {
     return undefined;
   }
 
@@ -494,7 +498,7 @@ export class MetaTableData<TableId extends keyof SchemaTypes> extends TableData
    */
   public getMetaRowPropFunc<ColId extends keyof SchemaTypes[TableId]>(
     colId: ColId
-  ): ((rowId: number | "new") => SchemaTypes[TableId][ColId]) {
+  ): RowFunc<SchemaTypes[TableId][ColId]|undefined> {
     return super.getRowPropFunc(colId as any) as any;
   }
 }

app/common/UIRowId.ts

@@ -0,0 +1,4 @@
+// This is the row ID used in the client, but it's helpful to have available in some common code
+// as well, which is why it's declared in app/common. Note that for data actions and stored data,
+// 'new' is not used.
+export type UIRowId = number | 'new';

app/common/gristUrls.ts

@@ -5,6 +5,7 @@ import {encodeQueryParams, isAffirmative} from 'app/common/gutil';
 import {localhostRegex} from 'app/common/LoginState';
 import {LocalPlugin} from 'app/common/plugin';
 import {StringUnion} from 'app/common/StringUnion';
+import {UIRowId} from 'app/common/UIRowId';
 import {Document} from 'app/common/UserAPI';
 import clone = require('lodash/clone');
 import pickBy = require('lodash/pickBy');
@@ -300,9 +301,15 @@ export function decodeUrl(gristConfig: Partial<GristLoadConfig>, location: Locat
     }
     if (hashMap.has('#') && hashMap.get('#') === 'a1') {
       const link: HashLink = {};
-      for (const key of ['sectionId', 'rowId', 'colRef'] as Array<keyof Omit<HashLink, 'welcomeTour' | 'docTour'>>) {
+      for (const key of ['sectionId', 'rowId', 'colRef'] as Array<keyof HashLink>) {
         const ch = key.substr(0, 1);
-        if (hashMap.has(ch)) { link[key] = parseInt(hashMap.get(ch)!, 10); }
+        if (!hashMap.has(ch)) { continue; }
+        const value = hashMap.get(ch);
+        if (key === 'rowId' && value === 'new') {
+          link[key] = 'new';
+        } else {
+          link[key] = parseInt(value!, 10);
+        }
       }
       state.hash = link;
     }
@@ -620,7 +627,7 @@ export function buildUrlId(parts: UrlIdParts): string {
  */
 export interface HashLink {
   sectionId?: number;
-  rowId?: number;
+  rowId?: UIRowId;
   colRef?: number;
 }
 

app/server/lib/Triggers.ts

@@ -65,7 +65,7 @@ export interface WebHookSecret {
 // An instance of this class should have .handle() called on it exactly once.
 export class TriggersHandler {
   // Converts a column ref to colId by looking it up in _grist_Tables_column
-  private _getColId: (rowId: (number | "new")) => string;
+  private _getColId: (rowId: number) => string|undefined;
 
   constructor(private _activeDoc: ActiveDoc) {
   }
@@ -82,7 +82,7 @@ export class TriggersHandler {
 
     const triggersByTableRef = _.groupBy(triggersTable.getRecords(), "tableRef");
     for (const [tableRef, triggers] of _.toPairs(triggersByTableRef)) {
-      const tableId = getTableId(Number(tableRef));  // groupBy makes tableRef a string
+      const tableId = getTableId(Number(tableRef))!;  // groupBy makes tableRef a string
       const tableDelta = summary.tableDeltas[tableId];
       if (!tableDelta) {
         continue;  // this table was not modified by these actions