diff --git a/app/client/components/BaseView.js b/app/client/components/BaseView.js index 2e888d49..6a328317 100644 --- a/app/client/components/BaseView.js +++ b/app/client/components/BaseView.js @@ -1,5 +1,3 @@ -/* global window */ - var _ = require('underscore'); var ko = require('knockout'); var moment = require('moment-timezone'); @@ -26,6 +24,7 @@ const {copyToClipboard} = require('app/client/lib/copyToClipboard'); const {setTestState} = require('app/client/lib/testState'); const {ExtraRows} = require('app/client/models/DataTableModelWithDiff'); const {createFilterMenu} = require('app/client/ui/ColumnFilterMenu'); +const {LinkConfig} = require('app/client/ui/selectBy'); const {encodeObject} = require("app/plugin/objtypes"); /** @@ -133,10 +132,16 @@ function BaseView(gristDoc, viewSectionModel, options) { let v = this.viewSection; let src = v.linkSrcSection(); const filterByAllShown = v.optionsObj.prop('filterByAllShown'); - return src.getRowId() ? - LinkingState.create.bind(LinkingState, this.gristDoc, - src, v.linkSrcCol().colId(), v, v.linkTargetCol().colId(), filterByAllShown()) : - null; + if (!src.getRowId()) { + return null; + } + try { + const config = new LinkConfig(v); + return LinkingState.create.bind(LinkingState, this.gristDoc, config, filterByAllShown()); + } catch (err) { + console.warn(`Can't create LinkingState: ${err.message}`); + return null; + } })); this._linkingFilter = this.autoDispose(ko.computed(() => { @@ -213,7 +218,7 @@ function BaseView(gristDoc, viewSectionModel, options) { const linkingFilter = this._linkingFilter(); this._queryRowSource.makeQuery(linkingFilter.filters, linkingFilter.operations, (err) => { if (this.isDisposed()) { return; } - if (err) { window.gristNotify(`Query error: ${err.message}`); } + if (err) { reportError(err); } this.onTableLoaded(); }); })); diff --git a/app/client/components/LinkingState.js b/app/client/components/LinkingState.js index ddce580c..01c3bda8 100644 --- a/app/client/components/LinkingState.js +++ b/app/client/components/LinkingState.js @@ -46,7 +46,8 @@ function isSummaryOf(summary, detail) { * value at the cursor. The user can use column filters on srcSection to control what's shown * in the linked tgtSection. */ -function LinkingState(gristDoc, srcSection, srcColId, tgtSection, tgtColId, byAllShown) { +function LinkingState(gristDoc, linkConfig, byAllShown) { + const {srcSection, srcColId, tgtSection, tgtCol, tgtColId} = linkConfig; this._srcSection = srcSection; let srcTableModel = gristDoc.getTableModel(srcSection.table().tableId()); @@ -66,7 +67,6 @@ function LinkingState(gristDoc, srcSection, srcColId, tgtSection, tgtColId, byAl // A computed that evaluates to a filter function to use, or null if not filtering. If // filtering, depends on srcSection.activeRowId(). if (tgtColId) { - const tgtCol = tgtSection.table().columns().all().find(c => c.colId() === tgtColId); const operations = {[tgtColId]: isRefListType(tgtCol.type()) ? 'intersects' : 'in'}; if (byAllShown) { // (This is legacy code that isn't currently reachable) diff --git a/app/client/ui/PageWidgetPicker.ts b/app/client/ui/PageWidgetPicker.ts index decc814c..586f7a7f 100644 --- a/app/client/ui/PageWidgetPicker.ts +++ b/app/client/ui/PageWidgetPicker.ts @@ -371,6 +371,9 @@ export class PageWidgetSelect extends Disposable { } private _selectTable(tid: TableId) { + if (tid !== this._value.table.get()) { + this._value.link.set(NoLink); + } this._value.table.set(tid); this._closeSummarizePanel(); } @@ -386,6 +389,7 @@ export class PageWidgetSelect extends Disposable { if (tid !== this._value.table.get()) { this._value.columns.set([]); this._value.table.set(tid); + this._value.link.set(NoLink); } this._openSummarizePanel(); } diff --git a/app/client/ui/selectBy.ts b/app/client/ui/selectBy.ts index bcbaa082..bef7c9ff 100644 --- a/app/client/ui/selectBy.ts +++ b/app/client/ui/selectBy.ts @@ -2,6 +2,7 @@ import { ColumnRec, DocModel, TableRec, ViewSectionRec } from 'app/client/models import { IPageWidget } from 'app/client/ui/PageWidgetPicker'; import { getReferencedTableId } from 'app/common/gristTypes'; import { IOptionFull } from 'grainjs'; +import * as assert from 'assert'; // some unicode characters const BLACK_CIRCLE = '\u2022'; @@ -213,3 +214,48 @@ export function linkFromId(linkid: string) { const [srcSectionRef, srcColRef, targetColRef] = JSON.parse(linkid); return {srcSectionRef, srcColRef, targetColRef}; } + +export class LinkConfig { + public readonly srcSection: ViewSectionRec; + public readonly tgtSection: ViewSectionRec; + // Note that srcCol and tgtCol may be the empty column records if that column is not used. + public readonly srcCol: ColumnRec; + public readonly tgtCol: ColumnRec; + public readonly srcColId: string|undefined; + public readonly tgtColId: string|undefined; + + // The constructor throws an exception if settings are invalid. When used from inside a knockout + // computed, the constructor subscribes to all parts relevant for linking. + constructor(tgtSection: ViewSectionRec) { + this.tgtCol = tgtSection.linkTargetCol(); + this.srcCol = tgtSection.linkSrcCol(); + this.srcSection = tgtSection.linkSrcSection(); + this.tgtSection = tgtSection; + this.srcColId = this.srcCol.colId(); + this.tgtColId = this.tgtCol.colId(); + this._assertValid(); + } + + // Check if section-linking configuration is valid, and throw exception if not. + private _assertValid(): void { + // Use null for unset cols (rather than an empty ColumnRec) for easier comparisons below. + const srcCol = this.srcCol?.getRowId() ? this.srcCol : null; + const tgtCol = this.tgtCol?.getRowId() ? this.tgtCol : null; + const srcTableId = (srcCol ? getReferencedTableId(srcCol.type.peek()) : + this.srcSection.table.peek().primaryTableId.peek()); + const tgtTableId = (tgtCol ? getReferencedTableId(tgtCol.type.peek()) : + this.tgtSection.table.peek().primaryTableId.peek()); + try { + assert(!tgtCol || tgtCol.parentId.peek() === this.tgtSection.tableRef.peek(), "tgtCol belongs to wrong table"); + assert(!srcCol || srcCol.parentId.peek() === this.srcSection.tableRef.peek(), "srcCol belongs to wrong table"); + assert(this.srcSection.getRowId() !== this.tgtSection.getRowId(), "srcSection links to itself"); + assert(tgtTableId, "tgtCol not a valid reference"); + assert(srcTableId, "srcCol not a valid reference"); + assert(srcTableId === tgtTableId, "mismatched tableIds"); + } catch (e) { + throw new Error(`LinkConfig invalid: ` + + `${this.srcSection.getRowId()}:${this.srcCol?.getRowId()}[${srcTableId}] -> ` + + `${this.tgtSection.getRowId()}:${this.tgtCol?.getRowId()}[${tgtTableId}]: ${e}`); + } + } +} diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index 5f69e612..4b9aa8c4 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -764,8 +764,13 @@ export async function addNewTable() { await waitForServer(); } +export interface PageWidgetPickerOptions { + summarize?: RegExp[]; // Optional list of patterns to match Group By columns. + selectBy?: RegExp; // Optional pattern of SELECT BY option to pick. +} + // Add a new page using the 'Add New' menu and wait for the new page to be shown. -export async function addNewPage(typeRe: RegExp, tableRe: RegExp, summarize?: RegExp[]) { +export async function addNewPage(typeRe: RegExp, tableRe: RegExp, options?: PageWidgetPickerOptions) { const url = await driver.getCurrentUrl(); // Click the 'Page' entry in the 'Add New' menu @@ -773,27 +778,25 @@ export async function addNewPage(typeRe: RegExp, tableRe: RegExp, summarize?: Re await driver.find('.test-dp-add-new-page').doClick(); // add widget - await selectWidget(typeRe, tableRe, summarize); + await selectWidget(typeRe, tableRe, options); // wait new page to be selected await driver.wait(async () => (await driver.getCurrentUrl()) !== url, 2000); } // Add a new widget to the current page using the 'Add New' menu. -export async function addNewSection(typeRe: RegExp, tableRe: RegExp, summarize?: RegExp[]) { - +export async function addNewSection(typeRe: RegExp, tableRe: RegExp, options?: PageWidgetPickerOptions) { // Click the 'Add widget to page' entry in the 'Add New' menu await driver.findWait('.test-dp-add-new', 2000).doClick(); await driver.findWait('.test-dp-add-widget-to-page', 500).doClick(); // add widget - await selectWidget(typeRe, tableRe, summarize); + await selectWidget(typeRe, tableRe, options); } - // Select type and table that matches respectivelly typeRe and tableRe and save. The widget picker // must be already opened when calling this function. -export async function selectWidget(typeRe: RegExp, tableRe: RegExp, summarize?: RegExp[]) { +export async function selectWidget(typeRe: RegExp, tableRe: RegExp, options: PageWidgetPickerOptions = {}) { const tableEl = driver.findContent('.test-wselect-table', tableRe); @@ -806,16 +809,22 @@ export async function selectWidget(typeRe: RegExp, tableRe: RegExp, summarize?: await tableEl.click(); - if (summarize) { + if (options.summarize) { // if summarize is requested, let's select the corresponding pivot icon await tableEl.find('.test-wselect-pivot').click(); // and all the columns - for (const colRef of summarize) { + for (const colRef of options.summarize) { await driver.findContent('.test-wselect-column', colRef).click(); } } + if (options.selectBy) { + // select link + await driver.find('.test-wselect-selectby').doClick(); + await driver.findContent('.test-wselect-selectby option', options.selectBy).doClick(); + } + // let's select right type and save await driver.findContent('.test-wselect-type', typeRe).doClick(); await driver.find('.test-wselect-addBtn').doClick();