(core) Include linking rowIds into remembered cursor position and anchor links.

Summary:
When linking using a Reference List column, there may be multiple source
records that show the same target record. With this change, we remember those
(rather than just pick one that shows the target record).

Test Plan: Added a browser test.

Reviewers: jarek

Reviewed By: jarek

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

app/client/components/BaseView.js

@@ -395,7 +395,8 @@ BaseView.prototype.getAnchorLinkForSection = function(sectionId) {
   const fieldIndex = this.cursor.fieldIndex.peek();
   const field = fieldIndex !== null ? this.viewSection.viewFields().peek()[fieldIndex] : null;
   const colRef = field?.colRef.peek();
-  return {hash: {sectionId, rowId, colRef}};
+  const linkingRowIds = sectionId ? this.gristDoc.getLinkingRowIds(sectionId) : undefined;
+  return {hash: {sectionId, rowId, colRef, linkingRowIds}};
 }
 
 // Copy an anchor link for the current row to the clipboard.

app/client/components/CursorMonitor.ts

@@ -59,7 +59,12 @@ export class CursorMonitor extends Disposable {
       if (!this._restored) { return; }
       // store position only when we have valid rowId
       // for some views (like CustomView) cursor position might not reflect actual row
-      if (pos && pos.rowId !== undefined) { this._storePosition(pos); }
+      if (pos && pos.rowId !== undefined) {
+        if (pos.sectionId) {
+          pos = {...pos, linkingRowIds: doc.getLinkingRowIds(pos.sectionId)};
+        }
+        this._storePosition(pos);
+      }
     }));
   }
 

app/client/components/GristDoc.ts

@@ -72,7 +72,7 @@ import {TableData} from 'app/common/TableData';
 import {getGristConfig} from 'app/common/urlUtils';
 import {DocStateComparison} from 'app/common/UserAPI';
 import {AttachedCustomWidgets, IAttachedCustomWidget, IWidgetType} from 'app/common/widgetTypes';
-import {CursorPos} from 'app/plugin/GristAPI';
+import {CursorPos, UIRowId} from 'app/plugin/GristAPI';
 import {
   bundleChanges,
   Computed,
@@ -1181,6 +1181,29 @@ export class GristDoc extends DisposableWithEvents {
     return rulesTable.numRecords() > rulesTable.filterRowIds({permissionsText: '', permissions: 63}).length;
   }
 
+  /**
+   * If the given section is the target of linking, collect and return the active rowIDs up the
+   * chain of links, returning the list of rowIds starting with the current section's parent. This
+   * method is intended for when there is ambiguity such as when RefList linking is involved.
+   * In other cases, returns undefined.
+   */
+  public getLinkingRowIds(sectionId: number): UIRowId[]|undefined {
+    const linkingRowIds: UIRowId[] = [];
+    let anyAmbiguity = false;
+    let section = this.docModel.viewSections.getRowModel(sectionId);
+    const seen = new Set<number>();
+    while (section?.id.peek() && !seen.has(section.id.peek())) {
+      seen.add(section.id.peek());
+      const rowId = section.activeRowId.peek() || 'new';
+      if (isRefListType(section.linkTargetCol.peek().type.peek()) || rowId === 'new') {
+        anyAmbiguity = true;
+      }
+      linkingRowIds.push(rowId);
+      section = section.linkSrcSection.peek();
+    }
+    return anyAmbiguity ? linkingRowIds.slice(1) : undefined;
+  }
+
   /**
    * Move to the desired cursor position.  If colRef is supplied, the cursor will be
    * moved to a field with that colRef.  Any linked sections that need their cursors
@@ -1211,6 +1234,8 @@ export class GristDoc extends DisposableWithEvents {
       }
 
       const srcSection = section.linkSrcSection.peek();
+      const linkingRowId = cursorPos.linkingRowIds?.[0];
+      const linkingRowIds = cursorPos.linkingRowIds?.slice(1);
       if (srcSection.id.peek()) {
         // We're in a linked section, so we need to recurse to make sure the row we want
         // will be visible.
@@ -1218,7 +1243,11 @@ export class GristDoc extends DisposableWithEvents {
         let controller: any;
         if (linkTargetCol.colId.peek()) {
           const destTable = await this._getTableData(section);
-          controller = destTable.getValue(cursorPos.rowId, linkTargetCol.colId.peek());
+          if (cursorPos.rowId === 'new') {
+            controller = 'new';
+          } else {
+            controller = destTable.getValue(cursorPos.rowId, linkTargetCol.colId.peek());
+          }
         } else {
           controller = cursorPos.rowId;
         }
@@ -1228,8 +1257,15 @@ export class GristDoc extends DisposableWithEvents {
         if (!colId && !isSrcSummary) {
           // Simple case - source linked by rowId, not a summary.
           if (isList(controller)) {
-            // Should be a reference list. Pick the first reference.
-            controller = controller[1];  // [0] is the L type code, [1] is the first value
+            // Should be a reference list. Use linkingRowId if available and present in the list,
+            if (linkingRowId && controller.indexOf(linkingRowId) > 0) {
+              controller = linkingRowId;
+            } else {
+              // Otherwise, pick the first reference.
+              controller = controller[1];  // [0] is the L type code, [1] is the first value
+            }
+          } else if (controller === 'new' && linkingRowId) {
+            controller = linkingRowId;
           }
           srcRowId = controller;
         } else {
@@ -1253,12 +1289,13 @@ export class GristDoc extends DisposableWithEvents {
           }
           srcRowId = srcTable.getRowIds().find(getFilterFunc(this.docData, query));
         }
-        if (!srcRowId || typeof srcRowId !== 'number') {
+        if (!srcRowId || (typeof srcRowId !== 'number' && srcRowId !== 'new')) {
           throw new Error('cannot trace rowId');
         }
         await this.recursiveMoveToCursorPos({
           rowId: srcRowId,
           sectionId: srcSection.id.peek(),
+          linkingRowIds,
         }, false, silent, visitedSections.concat([section.id.peek()]));
       }
       const view: ViewRec = section.view.peek();
@@ -1694,6 +1731,7 @@ export class GristDoc extends DisposableWithEvents {
       if (fieldIndex >= 0) {
         cursorPos.fieldIndex = fieldIndex;
       }
+      cursorPos.linkingRowIds = hash.linkingRowIds;
     }
     return cursorPos;
   }

app/common/gristUrls.ts

@@ -313,7 +313,11 @@ export function encodeUrl(gristConfig: Partial<GristLoadConfig>,
       hashParts.push('a1');
     }
     for (const key of ['sectionId', 'rowId', 'colRef'] as Array<keyof HashLink>) {
-      const partValue = hash[key];
+      let enhancedRowId: string|undefined;
+      if (key === 'rowId' && hash.linkingRowIds?.length) {
+        enhancedRowId = [hash.rowId, ...hash.linkingRowIds].join("-");
+      }
+      const partValue = enhancedRowId ?? hash[key];
       if (partValue) {
         const partKey = key === 'rowId' && state.hash?.rickRow ? 'rr' : key[0];
         hashParts.push(`${partKey}${partValue}`);
@@ -512,7 +516,7 @@ export function decodeUrl(gristConfig: Partial<GristLoadConfig>, location: Locat
         'sectionId',
         'rowId',
         'colRef',
-      ] as Array<Exclude<keyof HashLink, 'popup' | 'rickRow' | 'recordCard'>>;
+      ] as Array<'sectionId'|'rowId'|'colRef'>;
       for (const key of keys) {
         let ch: string;
         if (key === 'rowId' && hashMap.has('rr')) {
@@ -525,6 +529,10 @@ export function decodeUrl(gristConfig: Partial<GristLoadConfig>, location: Locat
         const value = hashMap.get(ch);
         if (key === 'rowId' && value === 'new') {
           link[key] = 'new';
+        } else if (key === 'rowId' && value && value.includes("-")) {
+          const rowIdParts = value.split("-").map(p => (p === 'new' ? p : parseInt(p, 10)));
+          link[key] = rowIdParts[0];
+          link.linkingRowIds = rowIdParts.slice(1);
         } else {
           link[key] = parseInt(value!, 10);
         }
@@ -1005,6 +1013,7 @@ export interface HashLink {
   popup?: boolean;
   rickRow?: boolean;
   recordCard?: boolean;
+  linkingRowIds?: UIRowId[];
 }
 
 // Check whether a urlId is a prefix of the docId, and adequately long to be

app/plugin/GristAPI-ti.ts

@@ -11,6 +11,7 @@ export const CursorPos = t.iface([], {
   "rowIndex": t.opt("number"),
   "fieldIndex": t.opt("number"),
   "sectionId": t.opt("number"),
+  "linkingRowIds": t.opt(t.array("UIRowId")),
 });
 
 export const ComponentKind = t.union(t.lit("safeBrowser"), t.lit("safePython"), t.lit("unsafeNode"));

app/plugin/GristAPI.ts

@@ -65,6 +65,11 @@ export interface CursorPos {
    * The id of a section that this cursor is in. Ignored when setting a cursor position for a particular view.
    */
   sectionId?: number;
+  /**
+   * When in a linked section, CursorPos may include which rows in the controlling sections are
+   * selected: the rowId in the linking-source section, in _that_ section's linking source, etc.
+   */
+  linkingRowIds?: UIRowId[];
 }
 
 export type ComponentKind = "safeBrowser" | "safePython" | "unsafeNode";

test/nbrowser/CursorSaving.ts

@@ -0,0 +1,223 @@
+import {assert, driver} from 'mocha-webdriver';
+import * as gu from 'test/nbrowser/gristUtils';
+import {cleanupExtraWindows, setupTestSuite} from 'test/nbrowser/testUtils';
+
+describe('CursorSaving', function() {
+  this.timeout(20000);
+  cleanupExtraWindows();
+  const cleanup = setupTestSuite();
+  const clipboard = gu.getLockableClipboard();
+  afterEach(() => gu.checkForErrors());
+
+  describe('WithRefLists', function() {
+    before(async function() {
+      const session = await gu.session().login();
+      await session.tempDoc(cleanup, "CursorWithRefLists1.grist");
+    });
+
+    it('should remember positions when record is linked from multiple source records', async function() {
+      // Select Tag 'a' (row 1), and Item 'Apples' (row 1), which has tags 'b' and 'a'.
+      await clickAndCheck({section: 'Tags', rowNum: 1, col: 0}, 'a');
+      await clickAndCheck({section: 'Items', rowNum: 1, col: 0}, 'Apple');
+
+      await gu.reloadDoc();
+      assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 1, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 1, col: 0});
+      assert.equal(await gu.getCardCell('Name', 'Items Card').getText(), 'Apple');
+
+      // Now select a different Tag, but the same Item.
+      await clickAndCheck({section: 'Tags', rowNum: 2, col: 0}, 'b');
+      await clickAndCheck({section: 'Items', rowNum: 1, col: 0}, 'Apple');
+
+      await gu.reloadDoc();
+      assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 2, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 1, col: 0});
+      assert.equal(await gu.getCardCell('Name', 'Items Card').getText(), 'Apple');
+
+      // Try the third section.
+      await clickAndCheck({section: 'Items', rowNum: 3, col: 0}, 'Orange');
+      await clickAndCheckCard({section: 'ITEMS Card', col: 'Name', rowNum: 1}, 'Orange');
+
+      await gu.reloadDoc();
+      assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 2, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 3, col: 0});
+      assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card');
+      assert.equal(await gu.getCardCell('Name', 'ITEMS Card').getText(), 'Orange');
+
+      // Try getting to the same card via different selections.
+      await clickAndCheck({section: 'Tags', rowNum: 1, col: 0}, 'a');
+      await clickAndCheck({section: 'Items', rowNum: 2, col: 0}, 'Orange');
+      await clickAndCheckCard({section: 'ITEMS Card', col: 'Name', rowNum: 1}, 'Orange');
+
+      await gu.reloadDoc();
+      assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 1, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 2, col: 0});
+      assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card');
+      assert.equal(await gu.getCardCell('Name', 'ITEMS Card').getText(), 'Orange');
+    });
+
+    it('should remember positions when "new" row is involved', async function() {
+      // Try a position when when the parent record is on a "new" row.
+      await clickAndCheck({section: 'Tags', rowNum: 2, col: 0}, 'b');
+      await clickAndCheck({section: 'Items', rowNum: 4, col: 0}, '');
+      await clickAndCheckCard({section: 'ITEMS Card', col: 'Tags', rowNum: 1}, '');
+
+      await gu.reloadDoc();
+      assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 2, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 4, col: 0});
+      assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card');
+      assert.equal(await gu.getCardCell('Tags', 'ITEMS Card').getText(), '');
+
+      // Try a position when when the grandparent parent record is on a "new" row.
+      await clickAndCheck({section: 'Tags', rowNum: 4, col: 0}, '');
+      assert.match(await gu.getSection('Items').find('.disable_viewpane').getText(), /No row selected/);
+      await clickAndCheckCard({section: 'ITEMS Card', col: 'Tags', rowNum: 1}, '');
+
+      await gu.reloadDoc();
+      assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 4, col: 0});
+      assert.match(await gu.getSection('Items').find('.disable_viewpane').getText(), /No row selected/);
+      assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card');
+      assert.equal(await gu.getCardCell('Tags', 'ITEMS Card').getText(), '');
+    });
+
+    it('should create anchor links that preserve row positions in linking sources', async function() {
+      await clickAndCheck({section: 'Tags', rowNum: 1, col: 0}, 'a');
+      await clickAndCheck({section: 'Items', rowNum: 1, col: 0}, 'Apple');
+      await gu.openRowMenu(1);
+
+      const anchorLinks: string[] = [];
+      await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); });
+
+      // Now select a different Tag, but the same Item.
+      await clickAndCheck({section: 'Tags', rowNum: 2, col: 0}, 'b');
+      await clickAndCheck({section: 'Items', rowNum: 1, col: 0}, 'Apple');
+      await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); });
+
+      // Try the third section.
+      await clickAndCheck({section: 'Items', rowNum: 3, col: 0}, 'Orange');
+      await clickAndCheckCard({section: 'ITEMS Card', col: 'Name', rowNum: 1}, 'Orange');
+      await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); });
+
+      // A different way to get to the same value in third section.
+      await clickAndCheck({section: 'Tags', rowNum: 1, col: 0}, 'a');
+      await clickAndCheck({section: 'Items', rowNum: 2, col: 0}, 'Orange');
+      await gu.getCardCell('Name', 'ITEMS Card').click();
+      await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); });
+
+      // Now go through the anchor links, and make sure each gets us to the expected point.
+      await driver.get(anchorLinks[0]);
+      assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 1, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 1, col: 0});
+      assert.equal(await gu.getCardCell('Name', 'Items Card').getText(), 'Apple');
+
+      await driver.get(anchorLinks[1]);
+      assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 2, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 1, col: 0});
+      assert.equal(await gu.getCardCell('Name', 'Items Card').getText(), 'Apple');
+
+      await driver.get(anchorLinks[2]);
+      assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 2, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 3, col: 0});
+      assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card');
+      assert.equal(await gu.getCardCell('Name', 'ITEMS Card').getText(), 'Orange');
+
+      await driver.get(anchorLinks[3]);
+      assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 1, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 2, col: 0});
+      assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card');
+      assert.equal(await gu.getCardCell('Name', 'ITEMS Card').getText(), 'Orange');
+    });
+
+    it('should handle anchor links when "new" row is involved', async function() {
+      const anchorLinks: string[] = [];
+
+      // Try a position when when the parent record is on a "new" row.
+      await clickAndCheck({section: 'Tags', rowNum: 2, col: 0}, 'b');
+      await clickAndCheck({section: 'Items', rowNum: 4, col: 0}, '');
+      await clickAndCheckCard({section: 'ITEMS Card', col: 'Tags', rowNum: 1}, '');
+
+      await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); });
+
+      // Try a position when when the grandparent parent record is on a "new" row.
+      await clickAndCheck({section: 'Tags', rowNum: 4, col: 0}, '');
+      assert.match(await gu.getSection('Items').find('.disable_viewpane').getText(), /No row selected/);
+      await clickAndCheckCard({section: 'ITEMS Card', col: 'Tags', rowNum: 1}, '');
+
+      await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); });
+
+      await driver.get(anchorLinks[0]);
+      assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 2, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('Items'), {rowNum: 4, col: 0});
+      assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card');
+      assert.equal(await gu.getCardCell('Tags', 'ITEMS Card').getText(), '');
+
+      await driver.get(anchorLinks[1]);
+      assert.deepEqual(await gu.getCursorPosition('Tags'), {rowNum: 4, col: 0});
+      assert.match(await gu.getSection('Items').find('.disable_viewpane').getText(), /No row selected/);
+      assert.equal(await gu.getActiveSectionTitle(), 'ITEMS Card');
+      assert.equal(await gu.getCardCell('Tags', 'ITEMS Card').getText(), '');
+    });
+  });
+
+  describe('WithRefs', function() {
+    // This is a similar test to the above, but without RefLists. In particular it checks that
+    // when a cursor is in the "new" row, enough is remembered to restore positions.
+
+    before(async function() {
+      const session = await gu.session().login();
+      const doc = await session.tempDoc(cleanup, "World.grist", {load: false});
+      await session.loadDoc(`/doc/${doc.id}/p/5`, {wait: true});
+    });
+
+    it('should remember row positions in linked sections', async function() {
+      // Select a country and a city within it.
+      await clickAndCheck({section: 'Country', rowNum: 2, col: 0}, 'AFG');
+      await clickAndCheck({section: 'City', rowNum: 4, col: 1}, 'Balkh');
+      await gu.reloadDoc();
+      assert.deepEqual(await gu.getCursorPosition('Country'), {rowNum: 2, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('City'), {rowNum: 4, col: 1});
+
+      // Now select a country, and the "new" row in the linked City widget.
+      await clickAndCheck({section: 'Country', rowNum: 3, col: 0}, 'AGO');
+      await clickAndCheck({section: 'City', rowNum: 6, col: 1}, '');
+      await gu.reloadDoc();
+      assert.deepEqual(await gu.getCursorPosition('Country'), {rowNum: 3, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('City'), {rowNum: 6, col: 1});
+    });
+
+    it('should create anchor links that preserve row positions in linked sections', async function() {
+      const anchorLinks: string[] = [];
+
+      // Select a country and a city within it.
+      await clickAndCheck({section: 'Country', rowNum: 2, col: 0}, 'AFG');
+      await clickAndCheck({section: 'City', rowNum: 4, col: 1}, 'Balkh');
+      await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); });
+
+      // Now select a country, and the "new" row in the linked City widget.
+      await clickAndCheck({section: 'Country', rowNum: 3, col: 0}, 'AGO');
+      await clickAndCheck({section: 'City', rowNum: 6, col: 1}, '');
+
+      await clipboard.lockAndPerform(async () => { anchorLinks.push(await gu.getAnchor()); });
+
+      await driver.get(anchorLinks[0]);
+      assert.deepEqual(await gu.getCursorPosition('Country'), {rowNum: 2, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('City'), {rowNum: 4, col: 1});
+
+      await driver.get(anchorLinks[1]);
+      assert.deepEqual(await gu.getCursorPosition('Country'), {rowNum: 3, col: 0});
+      assert.deepEqual(await gu.getCursorPosition('City'), {rowNum: 6, col: 1});
+    });
+  });
+});
+
+async function clickAndCheck(options: gu.ICellSelect, expectedValue: string) {
+  const cell = gu.getCell(options);
+  await cell.click();
+  assert.equal(await cell.getText(), expectedValue);
+}
+
+async function clickAndCheckCard(options: gu.ICellSelect, expectedValue: string) {
+  const cell = gu.getDetailCell(options);
+  await cell.click();
+  assert.equal(await cell.getText(), expectedValue);
+}