diff --git a/app/client/components/GristDoc.ts b/app/client/components/GristDoc.ts index b170874e..4a2ee69b 100644 --- a/app/client/components/GristDoc.ts +++ b/app/client/components/GristDoc.ts @@ -42,16 +42,16 @@ import {startDocTour} from "app/client/ui/DocTour"; import {showDocSettingsModal} from 'app/client/ui/DocumentSettings'; import {isTourActive} from "app/client/ui/OnBoardingPopups"; import {IPageWidget, toPageWidget} from 'app/client/ui/PageWidgetPicker'; -import {IPageWidgetLink, linkFromId, selectBy} from 'app/client/ui/selectBy'; +import {linkFromId, selectBy} from 'app/client/ui/selectBy'; import {startWelcomeTour} from 'app/client/ui/welcomeTour'; import {isNarrowScreen, mediaSmall, testId} from 'app/client/ui2018/cssVars'; -import {invokePrompt} from 'app/client/ui2018/modals'; import {IconName} from 'app/client/ui2018/IconList'; +import {invokePrompt} from 'app/client/ui2018/modals'; import {FieldEditor} from "app/client/widgets/FieldEditor"; import {MinimalActionGroup} from 'app/common/ActionGroup'; import {ClientQuery} from "app/common/ActiveDocAPI"; -import {delay} from 'app/common/delay'; import {CommDocUsage, CommDocUserAction} from 'app/common/CommTypes'; +import {delay} from 'app/common/delay'; import {DisposableWithEvents} from 'app/common/DisposableWithEvents'; import {isSchemaAction, UserAction} from 'app/common/DocActions'; import {OpenLocalDocResult} from 'app/common/DocListAPI'; @@ -62,8 +62,19 @@ import {LocalPlugin} from "app/common/plugin"; import {StringUnion} from 'app/common/StringUnion'; import {TableData} from 'app/common/TableData'; import {DocStateComparison} from 'app/common/UserAPI'; -import {bundleChanges, Computed, dom, Emitter, Holder, IDisposable, IDomComponent, Observable, - styled, subscribe, toKo} from 'grainjs'; +import { + bundleChanges, + Computed, + dom, + Emitter, + Holder, + IDisposable, + IDomComponent, + Observable, + styled, + subscribe, + toKo +} from 'grainjs'; import * as ko from 'knockout'; import cloneDeepWith = require('lodash/cloneDeepWith'); import isEqual = require('lodash/isEqual'); @@ -549,7 +560,6 @@ export class GristDoc extends DisposableWithEvents { */ public async addWidgetToPageImpl(val: IPageWidget, tableId: string|null = null) { const viewRef = this.activeViewId.get(); - const link = linkFromId(val.link); const tableRef = val.table === 'New Table' ? 0 : val.table; const result = await this.docData.sendAction( ['CreateViewSection', tableRef, viewRef, val.type, val.summarize ? val.columns : null, tableId] @@ -557,13 +567,7 @@ export class GristDoc extends DisposableWithEvents { if (val.type === 'chart') { await this._ensureOneNumericSeries(result.sectionRef); } - await this.docData.sendAction( - ['UpdateRecord', '_grist_Views_section', result.sectionRef, { - linkSrcSectionRef: link.srcSectionRef, - linkSrcColRef: link.srcColRef, - linkTargetColRef: link.targetColRef - }] - ); + await this.saveLink(val.link, result.sectionRef); return result; } @@ -647,7 +651,7 @@ export class GristDoc extends DisposableWithEvents { // update link if (oldVal.link !== newVal.link) { - await this.saveLink(linkFromId(newVal.link)); + await this.saveLink(newVal.link); } return section; }, @@ -698,11 +702,23 @@ export class GristDoc extends DisposableWithEvents { })); } - // Save link for the active section. - public async saveLink(link: IPageWidgetLink) { - const viewModel = this.viewModel; + // Save link for a given section, by default the active section. + public async saveLink(linkId: string, sectionId?: number) { + sectionId = sectionId || this.viewModel.activeSection.peek().getRowId(); + const link = linkFromId(linkId); + if (link.targetColRef) { + const targetTable = this.docModel.viewSections.getRowModel(sectionId).table(); + const targetCol = this.docModel.columns.getRowModel(link.targetColRef); + if (targetTable.id() !== targetCol.table().id()) { + // targetColRef is actually not a column in the target table. + // This should mean that the target table is a summary table (which didn't exist when the + // option was selected) and targetColRef is from the source table. + // Change it to the corresponding summary table column instead. + link.targetColRef = targetTable.columns().all().find(c => c.summarySourceCol() === link.targetColRef)!.id(); + } + } return this.docData.sendAction( - ['UpdateRecord', '_grist_Views_section', viewModel.activeSection.peek().getRowId(), { + ['UpdateRecord', '_grist_Views_section', sectionId, { linkSrcSectionRef: link.srcSectionRef, linkSrcColRef: link.srcColRef, linkTargetColRef: link.targetColRef diff --git a/app/client/ui/RightPanel.ts b/app/client/ui/RightPanel.ts index 5c76a055..3c71eb79 100644 --- a/app/client/ui/RightPanel.ts +++ b/app/client/ui/RightPanel.ts @@ -25,7 +25,7 @@ import {reportError} from 'app/client/models/AppModel'; import {ColumnRec, ViewSectionRec} from 'app/client/models/DocModel'; import {GridOptions} from 'app/client/ui/GridOptions'; import {attachPageWidgetPicker, IPageWidget, toPageWidget} from 'app/client/ui/PageWidgetPicker'; -import {linkFromId, linkId, selectBy} from 'app/client/ui/selectBy'; +import {linkId, selectBy} from 'app/client/ui/selectBy'; import {CustomSectionConfig} from 'app/client/ui/CustomSectionConfig'; import {VisibleFieldsConfig} from 'app/client/ui/VisibleFieldsConfig'; import {IWidgetType, widgetTypes} from 'app/client/ui/widgetTypes'; @@ -409,7 +409,7 @@ export class RightPanel extends Disposable { ) ); - link.onWrite((val) => this._gristDoc.saveLink(linkFromId(val))); + link.onWrite((val) => this._gristDoc.saveLink(val)); return [ this._disableIfReadonly(), cssLabel('DATA TABLE'), diff --git a/app/client/ui/selectBy.ts b/app/client/ui/selectBy.ts index 399a469a..ee2f4811 100644 --- a/app/client/ui/selectBy.ts +++ b/app/client/ui/selectBy.ts @@ -4,6 +4,7 @@ import { getReferencedTableId } from 'app/common/gristTypes'; import { IOptionFull } from 'grainjs'; import assert from 'assert'; import * as gutil from "app/common/gutil"; +import isEqual = require('lodash/isEqual'); // some unicode characters const BLACK_CIRCLE = '\u2022'; @@ -243,24 +244,44 @@ function fromPageWidget(docModel: DocModel, pageWidget: IPageWidget): LinkNode[] if (typeof pageWidget.table !== 'number') { return []; } - const table = docModel.tables.getRowModel(pageWidget.table); + let table = docModel.tables.getRowModel(pageWidget.table); const isSummary = pageWidget.summarize; + const groupbyColumns = isSummary ? new Set(pageWidget.columns) : undefined; + let tableExists = true; + if (isSummary) { + const summaryTable = docModel.tables.rowModels.find( + t => t?.summarySourceTable.peek() && isEqual(t.summarySourceColRefs.peek(), groupbyColumns)); + if (summaryTable) { + // The selected source table and groupby columns correspond to this existing summary table. + table = summaryTable; + } else { + // This summary table doesn't exist yet. `fromColumns` will be using columns from the source table. + // Make sure it only uses columns that are in the selected groupby columns. + // The resulting targetColRef will incorrectly be from the source table, + // but will be corrected in GristDoc.saveLink after the summary table is created. + tableExists = false; + } + } + const mainNode: LinkNode = { tableId: table.primaryTableId.peek(), isSummary, - groupbyColumns: isSummary ? new Set(pageWidget.columns) : undefined, + groupbyColumns, widgetType: pageWidget.type, ancestors: new Set(), section: docModel.viewSections.getRowModel(pageWidget.section), }; - return fromColumns(table, mainNode); + return fromColumns(table, mainNode, tableExists); } -function fromColumns(table: TableRec, mainNode: LinkNode): LinkNode[] { +function fromColumns(table: TableRec, mainNode: LinkNode, tableExists: boolean = true): LinkNode[] { const nodes = [mainNode]; const columns = table.columns.peek().peek(); for (const column of columns) { + if (!tableExists && !mainNode.groupbyColumns!.has(column.getRowId())) { + continue; + } const tableId = getReferencedTableId(column.type.peek()); if (tableId) { nodes.push({...mainNode, tableId, column}); diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index c42b1204..0aa78a3b 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -871,6 +871,7 @@ export interface PageWidgetPickerOptions { tableName?: string; selectBy?: RegExp|string; // Optional pattern of SELECT BY option to pick. summarize?: (RegExp|string)[]; // Optional list of patterns to match Group By columns. + dontAdd?: boolean; // If true, configure the widget selection without actually adding to the page } // Add a new page using the 'Add New' menu and wait for the new page to be shown. @@ -939,8 +940,14 @@ export async function selectWidget( await driver.findContent('.test-wselect-selectby option', options.selectBy).doClick(); } - // let's select right type and save + // select right type await driver.findContent('.test-wselect-type', typeRe).doClick(); + + if (options.dontAdd) { + return; + } + + // add the widget await driver.find('.test-wselect-addBtn').doClick(); // if we selected a new table, there will be a popup for a name