From f194d6861b72137ed6f8128cbe2ce530a9f7d058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Wed, 4 May 2022 11:54:30 +0200 Subject: [PATCH] (core) Updating RawData views Summary: - Better focus on the widget title - Adding columns only to the current view section - New popup with options when user wants to delete a page - New dialog to enter table name - New table as a widget doesn't create a separate page - Removing a table doesn't remove the primary view Test Plan: Updated and new tests Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D3410 --- app/client/components/GridView.js | 21 +- app/client/components/GristDoc.ts | 32 ++- app/client/components/duplicatePage.ts | 2 +- app/client/ui/PageWidgetPicker.ts | 15 +- app/client/ui/Pages.ts | 173 ++++++++++++++-- app/client/ui/WidgetTitle.ts | 48 ++--- app/client/ui2018/checkbox.ts | 2 + app/client/ui2018/links.ts | 1 - app/client/ui2018/modals.ts | 90 ++++++++- app/server/lib/ActiveDoc.ts | 2 +- sandbox/grist/test_column_actions.py | 7 +- sandbox/grist/test_derived.py | 8 +- sandbox/grist/test_display_cols.py | 2 +- sandbox/grist/test_rules.py | 4 +- sandbox/grist/test_summary.py | 48 ++--- sandbox/grist/test_summary2.py | 51 ++--- sandbox/grist/test_summary_choicelist.py | 12 +- sandbox/grist/test_summary_undo.py | 2 +- sandbox/grist/test_table_actions.py | 14 +- sandbox/grist/test_undo.py | 4 +- sandbox/grist/test_useractions.py | 242 +++++++++++------------ sandbox/grist/testscript.json | 18 +- sandbox/grist/useractions.py | 156 ++++++++++----- test/nbrowser/gristUtils.ts | 52 ++++- 24 files changed, 676 insertions(+), 330 deletions(-) diff --git a/app/client/components/GridView.js b/app/client/components/GridView.js index 3164e2cf..01966810 100644 --- a/app/client/components/GridView.js +++ b/app/client/components/GridView.js @@ -653,15 +653,22 @@ GridView.prototype.addNewColumn = function() { .then(() => this.scrollPaneRight()); }; -GridView.prototype.insertColumn = function(index) { - var pos = tableUtil.fieldInsertPositions(this.viewSection.viewFields(), index)[0]; +GridView.prototype.insertColumn = async function(index) { + const pos = tableUtil.fieldInsertPositions(this.viewSection.viewFields(), index)[0]; var action = ['AddColumn', null, {"_position": pos}]; - return this.tableModel.sendTableAction(action) - .bind(this).then(function() { - this.selectColumn(index); - this.currentEditingColumnIndex(index); - // this.columnConfigTab.show(); + await this.gristDoc.docData.bundleActions('Insert column', async () => { + const colInfo = await this.tableModel.sendTableAction(action); + if (!this.viewSection.isRaw.peek()){ + const fieldInfo = { + colRef: colInfo.colRef, + parentPos: pos, + parentId: this.viewSection.id.peek() + }; + await this.gristDoc.docModel.viewFields.sendTableAction(['AddRecord', null, fieldInfo]); + } }); + this.selectColumn(index); + this.currentEditingColumnIndex(index); }; GridView.prototype.scrollPaneRight = function() { diff --git a/app/client/components/GristDoc.ts b/app/client/components/GristDoc.ts index 2be9b1a5..17cabe63 100644 --- a/app/client/components/GristDoc.ts +++ b/app/client/components/GristDoc.ts @@ -45,6 +45,7 @@ import {IPageWidget, toPageWidget} from 'app/client/ui/PageWidgetPicker'; import {IPageWidgetLink, 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 {FieldEditor} from "app/client/widgets/FieldEditor"; import {MinimalActionGroup} from 'app/common/ActionGroup'; @@ -500,7 +501,9 @@ export class GristDoc extends DisposableWithEvents { * Sends an action to create a new empty table and switches to that table's primary view. */ public async addEmptyTable(): Promise { - const tableInfo = await this.docData.sendAction(['AddEmptyTable']); + const name = await this._promptForName(); + if (name === undefined) { return; } + const tableInfo = await this.docData.sendAction(['AddEmptyTable', name || null]); await this.openDocPage(this.docModel.tables.getRowModel(tableInfo.id).primaryViewId()); } @@ -510,10 +513,16 @@ export class GristDoc extends DisposableWithEvents { public async addWidgetToPage(val: IPageWidget) { const docData = this.docModel.docData; const viewName = this.viewModel.name.peek(); - + let tableId: string|null|undefined; + if (val.table === 'New Table') { + tableId = await this._promptForName(); + if (tableId === undefined) { + return; + } + } const res = await docData.bundleActions( `Added new linked section to view ${viewName}`, - () => this.addWidgetToPageImpl(val) + () => this.addWidgetToPageImpl(val, tableId ?? null) ); // The newly-added section should be given focus. @@ -523,13 +532,12 @@ export class GristDoc extends DisposableWithEvents { /** * The actual implementation of addWidgetToPage */ - public async addWidgetToPageImpl(val: IPageWidget) { + public async addWidgetToPageImpl(val: IPageWidget, tableId: string|null = null) { const viewRef = this.activeViewId.get(); - const tableRef = val.table === 'New Table' ? 0 : val.table; 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] + ['CreateViewSection', tableRef, viewRef, val.type, val.summarize ? val.columns : null, tableId] ); if (val.type === 'chart') { await this._ensureOneNumericSeries(result.sectionRef); @@ -549,13 +557,15 @@ export class GristDoc extends DisposableWithEvents { */ public async addNewPage(val: IPageWidget) { if (val.table === 'New Table') { - const result = await this.docData.sendAction(['AddEmptyTable']); + const name = await this._promptForName(); + if (name === undefined) { return; } + const result = await this.docData.sendAction(['AddEmptyTable', name]); await this.openDocPage(result.views[0].id); } else { let result: any; await this.docData.bundleActions(`Add new page`, async () => { result = await this.docData.sendAction( - ['CreateViewSection', val.table, 0, val.type, val.summarize ? val.columns : null] + ['CreateViewSection', val.table, 0, val.type, val.summarize ? val.columns : null, null] ); if (val.type === 'chart') { await this._ensureOneNumericSeries(result.sectionRef); @@ -917,6 +927,10 @@ export class GristDoc extends DisposableWithEvents { } } + private async _promptForName() { + return await invokePrompt("Table name", "Create", '', "Default table name"); + } + private async _replaceViewSection(section: ViewSectionRec, newVal: IPageWidget) { const docModel = this.docModel; diff --git a/app/client/components/duplicatePage.ts b/app/client/components/duplicatePage.ts index 18ece246..7ea11069 100644 --- a/app/client/components/duplicatePage.ts +++ b/app/client/components/duplicatePage.ts @@ -186,7 +186,7 @@ async function createNewViewSections(docData: GristDoc['docData'], viewSections: // Helper to create an action that add widget to the view with viewId. function newViewSectionAction(widget: IPageWidget, viewId: number) { - return ['CreateViewSection', widget.table, viewId, widget.type, widget.summarize ? widget.columns : null]; + return ['CreateViewSection', widget.table, viewId, widget.type, widget.summarize ? widget.columns : null, null]; } /** diff --git a/app/client/ui/PageWidgetPicker.ts b/app/client/ui/PageWidgetPicker.ts index 17cad902..1cf24237 100644 --- a/app/client/ui/PageWidgetPicker.ts +++ b/app/client/ui/PageWidgetPicker.ts @@ -167,11 +167,16 @@ export function buildPageWidgetPicker( link: value.link.get(), section: value.section.get(), }); - // If savePromise throws an error, before or after timeout, we let the error propagate as it - // should be handle by the caller. - if (await isLongerThan(savePromise, DELAY_BEFORE_SPINNER_MS)) { - const label = getWidgetTypes(type).label; - await spinnerModal(`Building ${label} widget`, savePromise); + if (value.table.get() === 'New Table') { + // Adding empty table will show a prompt, so we don't want to wait for it. + await savePromise; + } else { + // If savePromise throws an error, before or after timeout, we let the error propagate as it + // should be handle by the caller. + if (await isLongerThan(savePromise, DELAY_BEFORE_SPINNER_MS)) { + const label = getWidgetTypes(type).label; + await spinnerModal(`Building ${label} widget`, savePromise); + } } } diff --git a/app/client/ui/Pages.ts b/app/client/ui/Pages.ts index 7cae8ee4..09c0a5e1 100644 --- a/app/client/ui/Pages.ts +++ b/app/client/ui/Pages.ts @@ -1,15 +1,19 @@ -import { createGroup } from "app/client/components/commands"; -import { duplicatePage } from "app/client/components/duplicatePage"; -import { GristDoc } from "app/client/components/GristDoc"; -import { PageRec } from "app/client/models/DocModel"; -import { urlState } from "app/client/models/gristUrlState"; -import * as MetaTableModel from "app/client/models/MetaTableModel"; -import { find as findInTree, fromTableData, TreeItemRecord, TreeRecord, - TreeTableData} from "app/client/models/TreeModel"; -import { TreeViewComponent } from "app/client/ui/TreeViewComponent"; -import { buildPageDom, PageActions } from "app/client/ui2018/pages"; -import { mod } from 'app/common/gutil'; -import { Computed, Disposable, dom, fromKo, observable, Observable } from "grainjs"; +import {createGroup} from 'app/client/components/commands'; +import {duplicatePage} from 'app/client/components/duplicatePage'; +import {GristDoc} from 'app/client/components/GristDoc'; +import {PageRec} from 'app/client/models/DocModel'; +import {urlState} from 'app/client/models/gristUrlState'; +import * as MetaTableModel from 'app/client/models/MetaTableModel'; +import {find as findInTree, fromTableData, TreeItemRecord, TreeRecord, + TreeTableData} from 'app/client/models/TreeModel'; +import {TreeViewComponent} from 'app/client/ui/TreeViewComponent'; +import {labeledCircleCheckbox} from 'app/client/ui2018/checkbox'; +import {colors} from 'app/client/ui2018/cssVars'; +import {cssLink} from 'app/client/ui2018/links'; +import {ISaveModalOptions, saveModal} from 'app/client/ui2018/modals'; +import {buildPageDom, PageActions} from 'app/client/ui2018/pages'; +import {mod} from 'app/common/gutil'; +import {Computed, Disposable, dom, DomContents, fromKo, makeTestId, observable, Observable, styled} from 'grainjs'; // build dom for the tree view of pages export function buildPagesDom(owner: Disposable, activeDoc: GristDoc, isOpen: Observable) { @@ -49,16 +53,18 @@ export function buildPagesDom(owner: Disposable, activeDoc: GristDoc, isOpen: Ob return dom('div', dom.create(TreeViewComponent, model, {isOpen, selected, isReadonly: activeDoc.isReadonly})); } -function buildDomFromTable(pagesTable: MetaTableModel, activeDoc: GristDoc, id: number) { +const testId = makeTestId('test-removepage-'); + +function buildDomFromTable(pagesTable: MetaTableModel, activeDoc: GristDoc, pageId: number) { const {isReadonly} = activeDoc; - const pageName = pagesTable.rowModels[id].view.peek().name; - const viewId = pagesTable.rowModels[id].view.peek().id.peek(); - const docData = pagesTable.tableData.docData; + const pageName = pagesTable.rowModels[pageId].view.peek().name; + const viewId = pagesTable.rowModels[pageId].view.peek().id.peek(); + const actions: PageActions = { onRename: (newName: string) => newName.length && pageName.saveOnly(newName), - onRemove: () => docData.sendAction(['RemoveRecord', '_grist_Views', viewId]), + onRemove: () => removeView(activeDoc, viewId, pageName.peek()), // TODO: duplicate should prompt user for confirmation - onDuplicate: () => duplicatePage(activeDoc, id), + onDuplicate: () => duplicatePage(activeDoc, pageId), // Can't remove last visible page isRemoveDisabled: () => activeDoc.docModel.visibleDocPages.peek().length <= 1, isReadonly @@ -67,6 +73,46 @@ function buildDomFromTable(pagesTable: MetaTableModel, activeDoc: Grist return buildPageDom(fromKo(pageName), actions, urlState().setLinkUrl({docPage: viewId})); } +function removeView(activeDoc: GristDoc, viewId: number, pageName: string) { + const docData = activeDoc.docData; + // Create a set with tables on other pages (but not on this one). + const tablesOnOtherViews = new Set(activeDoc.docModel.viewSections.rowModels + .filter(vs => !vs.isRaw.peek() && vs.parentId.peek() !== viewId) + .map(vs => vs.tableRef.peek())); + + // Check if this page is a last page for some tables. + const notVisibleTables = [...new Set(activeDoc.docModel.viewSections.rowModels + .filter(vs => vs.parentId.peek() === viewId) // Get all sections on this view + .filter(vs => !vs.table.peek().summarySourceTable.peek()) // Sections that have normal tables + .filter(vs => !tablesOnOtherViews.has(vs.tableRef.peek())) // That aren't on other views + .filter(vs => vs.table.peek().tableId.peek()) // Which we can access (has tableId) + .map(vs => vs.table.peek()))]; // Return tableRec object, and remove duplicates. + + const removePage = () => [['RemoveRecord', '_grist_Views', viewId]]; + const removeAll = () => [ + ...removePage(), + ...notVisibleTables.map(t => ['RemoveTable', t.tableId.peek()]) + ]; + + if (notVisibleTables.length) { + const tableNames = notVisibleTables.map(t => t.tableNameDef.peek()); + buildPrompt(tableNames, async (option) => { + // Errors are handled in the dialog. + if (option === 'data') { + await docData.sendActions(removeAll(), `Remove page ${pageName} with tables ${tableNames}`); + } else if (option === 'page') { + await docData.sendActions(removePage(), `Remove only page ${pageName}`); + } else { + // This should not happen, as save should be disabled when no option is selected. + } + }); + } else { + return docData.sendActions(removePage(), `Remove only page ${pageName}`); + } +} + +type RemoveOption = '' | 'data' | 'page'; + // Select another page in cyclic ordering of pages. Order is downard if given a positive `delta`, // upward otherwise. function otherPage(currentPage: TreeItemRecord, delta: number) { @@ -75,3 +121,94 @@ function otherPage(currentPage: TreeItemRecord, delta: number) { const docPage = records[index].viewRef; return urlState().pushUrl({docPage}); } + +function buildPrompt(tableNames: string[], onSave: (option: RemoveOption) => Promise) { + saveModal((ctl, owner): ISaveModalOptions => { + const selected = Observable.create(owner, ''); + const saveDisabled = Computed.create(owner, use => use(selected) === ''); + const saveFunc = () => onSave(selected.get()); + return { + title: `The following table${tableNames.length > 1 ? 's' : ''} will no longer be visible`, + body: dom('div', + testId('popup'), + buildWarning(tableNames), + cssOptions( + buildOption(selected, 'data', `Delete data and this page.`), + buildOption(selected, 'page', + [ + `Delete this page, but do not delete data. `, + `Table will remain available in `, + cssLink(urlState().setHref({docPage: 'data'}), 'raw data page', { target: '_blank'}), + `.` + ]), + ) + ), + saveDisabled, + saveLabel: 'Delete', + saveFunc, + width: 'fixed-wide', + extraButtons: [], + }; + }); +} + +function buildOption(value: Observable, id: RemoveOption, content: DomContents) { + const selected = Computed.create(null, use => use(value) === id) + .onWrite(val => val ? value.set(id) : void 0); + return dom.update( + labeledCircleCheckbox(selected, content, dom.autoDispose(selected)), + testId(`option-${id}`), + cssBlockCheckbox.cls(''), + cssBlockCheckbox.cls('-block', selected), + ); +} + +function buildWarning(tables: string[]) { + return cssWarning( + dom.forEach(tables, (t) => cssTableName(t, testId('table'))) + ); +} + +const cssOptions = styled('div', ` + display: flex; + flex-direction: column; + gap: 10px; +`); + +// We need to reset top and left of ::before element, as it is wrongly set +// on the inline checkbox. +// To simulate radio button behavior, we will block user input after option is selected, because +// checkbox doesn't support two-way binding. +const cssBlockCheckbox = styled('div', ` + display: flex; + padding: 10px 8px; + border: 1px solid ${colors.mediumGrey}; + border-radius: 3px; + cursor: pointer; + & input::before, & input::after { + top: unset; + left: unset; + } + &:hover { + border-color: ${colors.lightGreen}; + } + &-block { + pointer-events: none; + } + &-block a { + pointer-events: all; + } +`); + +const cssWarning = styled('div', ` + display: flex; + flex-wrap: wrap; + gap: 8px; + margin-bottom: 16px; +`); + +const cssTableName = styled('div', ` + background: #eee; + padding: 3px 6px; + border-radius: 4px; +`); diff --git a/app/client/ui/WidgetTitle.ts b/app/client/ui/WidgetTitle.ts index 271de42e..fe42869c 100644 --- a/app/client/ui/WidgetTitle.ts +++ b/app/client/ui/WidgetTitle.ts @@ -60,16 +60,18 @@ function buildWidgetRenamePopup(ctrl: IOpenController, vs: ViewSectionRec, optio // User input for table name. const inputTableName = Observable.create(ctrl, tableName); // User input for widget title. - const inputWidgetTitle = Observable.create(ctrl, vs.title.peek()); + const inputWidgetTitle = Observable.create(ctrl, vs.title.peek() ?? ''); // Placeholder for widget title: // - when widget title is empty shows a default widget title (what would be shown when title is empty) // - when widget title is set, shows just a text to override it. const inputWidgetPlaceholder = !vs.title.peek() ? 'Override widget title' : vs.defaultWidgetTitle.peek(); - const disableSave = Computed.create(ctrl, (use) => - (use(inputTableName) === tableName || use(inputTableName).trim() === '') && - use(inputWidgetTitle) === vs.title.peek() - ); + const disableSave = Computed.create(ctrl, (use) => { + const newTableName = use(inputTableName)?.trim() ?? ''; + const newWidgetTitle = use(inputWidgetTitle)?.trim() ?? ''; + // Can't save when table name is empty or there wasn't any change. + return !newTableName || (newTableName === tableName && newWidgetTitle === use(vs.title)); + }); const modalCtl = ModalControl.create(ctrl, () => ctrl.close()); @@ -88,9 +90,10 @@ function buildWidgetRenamePopup(ctrl: IOpenController, vs: ViewSectionRec, optio }; const saveWidgetTitle = async () => { + const newTitle = inputWidgetTitle.get()?.trim() ?? ''; // If value was changed. - if (inputWidgetTitle.get() !== vs.title.peek()) { - await vs.title.saveOnly(inputWidgetTitle.get()); + if (newTitle !== vs.title.peek()) { + await vs.title.saveOnly(newTitle); } }; const doSave = modalCtl.doWork(() => Promise.all([ @@ -99,23 +102,20 @@ function buildWidgetRenamePopup(ctrl: IOpenController, vs: ViewSectionRec, optio ]), {close: true}); function initialFocus() { - // Set focus on a thing user is likely to change. - // Initial focus is set on tableName unless: - // - if this is a summary table - as it is not editable, - // - if widgetTitle is not empty - so user wants to change it further, - // - if widgetTitle is empty but the default widget name will have type suffix (like Table1 (Card)), so it won't - // be a table name - so user doesn't want the default value. - if ( - !widgetInput || - isSummary || - vs.title.peek() || - ( - !vs.title.peek() && - vs.defaultWidgetTitle.peek().toUpperCase() !== tableRec.tableName.peek().toUpperCase() - )) { - widgetInput?.focus(); - } else if (!isSummary) { - tableInput?.focus(); + const isRawView = !widgetInput; + const isWidgetTitleEmpty = !vs.title.peek(); + function focus(inputEl?: HTMLInputElement) { + inputEl?.focus(); + inputEl?.select(); + } + if (isSummary) { + focus(widgetInput); + } else if (isRawView) { + focus(tableInput); + } else if (isWidgetTitleEmpty) { + focus(tableInput); + } else { + focus(widgetInput); } } diff --git a/app/client/ui2018/checkbox.ts b/app/client/ui2018/checkbox.ts index acdbec58..3a60dfb5 100644 --- a/app/client/ui2018/checkbox.ts +++ b/app/client/ui2018/checkbox.ts @@ -34,6 +34,8 @@ export const cssLabel = styled('label', ` } `); + + // TODO: the !important markings are to trump bootstrap, and should be removed when it's gone. export const cssCheckboxSquare = styled('input', ` -webkit-appearance: none; diff --git a/app/client/ui2018/links.ts b/app/client/ui2018/links.ts index 0045db8f..1fbb844d 100644 --- a/app/client/ui2018/links.ts +++ b/app/client/ui2018/links.ts @@ -7,7 +7,6 @@ import { dom, IDomArgs, Observable, styled } from 'grainjs'; * Styling for a simple green link. */ -// Match the font-weight of buttons. export const cssLink = styled('a', ` color: ${colors.lightGreen}; --icon-color: ${colors.lightGreen}; diff --git a/app/client/ui2018/modals.ts b/app/client/ui2018/modals.ts index c95ae897..42f20f5f 100644 --- a/app/client/ui2018/modals.ts +++ b/app/client/ui2018/modals.ts @@ -1,10 +1,11 @@ import {FocusLayer} from 'app/client/lib/FocusLayer'; import {reportError} from 'app/client/models/errors'; +import {cssInput} from 'app/client/ui/MakeCopyMenu'; import {bigBasicButton, bigPrimaryButton, cssButton} from 'app/client/ui2018/buttons'; import {colors, mediaSmall, testId, vars} from 'app/client/ui2018/cssVars'; import {loadingSpinner} from 'app/client/ui2018/loaders'; import {waitGrainObs} from 'app/common/gutil'; -import {Computed, Disposable, dom, DomContents, DomElementArg, MultiHolder, Observable, styled} from 'grainjs'; +import {Computed, Disposable, dom, DomContents, DomElementArg, input, MultiHolder, Observable, styled} from 'grainjs'; // IModalControl is passed into the function creating the body of the modal. export interface IModalControl { @@ -287,6 +288,93 @@ export function confirmModal( })); } + +/** + * Creates a simple prompt modal (replacement for the native one). + * Closed via clicking anywhere outside the modal or Cancel button. + * + * Example usage: + * promptModal( + * "Enter your name", + * (name: string) => alert(`Hello ${name}`), + * "Ok" // Confirm button name, + * "John doe", // Initial text (can be empty or undefined) + * "Enter your name", // input placeholder + * () => console.log('User cancelled') // Called when user cancels, or clicks outside. + * ) + * + * @param title: Prompt text. + * @param onConfirm: Handler for Confirm button. + * @param btnText: Text of the confirm button. + * @param initial: Initial value in the input element. + * @param placeholder: Placeholder for the input element. + * @param onCancel: Optional cancel handler. + */ +export function promptModal( + title: string, + onConfirm: (text: string) => Promise, + btnText: string, + initial?: string, + placeholder?: string, + onCancel?: () => void +): void { + saveModal((ctl, owner): ISaveModalOptions => { + let confirmed = false; + const text = Observable.create(owner, initial ?? ''); + const txtInput = input(text, { onInput : true }, { placeholder }, cssInput.cls(''), testId('modal-prompt')); + const options: ISaveModalOptions = { + title, + body: txtInput, + saveLabel: btnText, + saveFunc: () => { + // Mark that confirm was invoked. + confirmed = true; + return onConfirm(text.get() || ''); + }, + width: 'normal' + }; + owner.onDispose(() => { + if (confirmed) { return; } + onCancel?.(); + }); + setTimeout(() => txtInput.focus(), 10); + return options; + }); +} + +/** + * Wraps prompt modal in a promise that is resolved either when user confirms or cancels. + * When user cancels the returned value is always undefined. + * + * Example usage: + * async handler() { + * const name = await invokePrompt("Please enter your name"); + * if (name !== undefined) alert(`Hello ${name}`); + * } + * + * @param title: Prompt text. + * @param btnText: Text of the confirm button, default is "Ok". + * @param initial: Initial value in the input element. + * @param placeholder: Placeholder for the input element. + */ +export function invokePrompt( + title: string, + btnText?: string, + initial?: string, + placeholder?: string +): Promise { + let onResolve: (text: string|undefined) => any; + const prom = new Promise((resolve) => { + onResolve = resolve; + }); + promptModal(title, onResolve!, btnText ?? 'Ok', initial, placeholder, () => { + if (onResolve) { + onResolve(undefined); + } + }); + return prom; +} + /** * Builds a simple spinner modal. The modal gets removed when `promise` resolves. */ diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 19e64df2..84cff6a1 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -611,7 +611,7 @@ export class ActiveDoc extends EventEmitter { public addInitialTable(docSession: OptDocSession) { // Use a non-client-specific session, so that this action is not part of anyone's undo history. const newDocSession = makeExceptionalDocSession('nascent'); - return this.applyUserActions(newDocSession, [["AddEmptyTable"]]); + return this.applyUserActions(newDocSession, [["AddEmptyTable", null]]); } /** diff --git a/sandbox/grist/test_column_actions.py b/sandbox/grist/test_column_actions.py index def0d1a9..5e72f8bb 100644 --- a/sandbox/grist/test_column_actions.py +++ b/sandbox/grist/test_column_actions.py @@ -185,9 +185,10 @@ class TestColumnActions(test_engine.EngineTestCase): def init_sample_data(self): # Add a new view with a section, and a new table to that view, and a summary table. self.load_sample(self.sample2) - self.apply_user_action(["CreateViewSection", 1, 0, "record", None]) - self.apply_user_action(["CreateViewSection", 0, 1, "record", None]) - self.apply_user_action(["CreateViewSection", 1, 1, "record", [12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", None, None]) + self.apply_user_action(["AddEmptyTable", None]) + self.apply_user_action(["CreateViewSection", 2, 1, "record", None, None]) + self.apply_user_action(["CreateViewSection", 1, 1, "record", [12], None]) self.apply_user_action(["BulkAddRecord", "Table1", [None]*3, { "A": ["a", "b", "c"], "B": ["d", "e", "f"], diff --git a/sandbox/grist/test_derived.py b/sandbox/grist/test_derived.py index 028bc01e..ca6a2fae 100644 --- a/sandbox/grist/test_derived.py +++ b/sandbox/grist/test_derived.py @@ -57,7 +57,7 @@ class TestDerived(test_engine.EngineTestCase): self.load_sample(self.sample) # Create a derived table summarizing count and total of orders by year. - self.apply_user_action(["CreateViewSection", 2, 0, 'record', [10]]) + self.apply_user_action(["CreateViewSection", 2, 0, 'record', [10], None]) # Check the results. self.assertPartialData("GristSummary_6_Orders", ["id", "year", "count", "amount", "group" ], [ @@ -134,7 +134,7 @@ class TestDerived(test_engine.EngineTestCase): """ self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 2, 0, 'record', [10, 12]]) + self.apply_user_action(["CreateViewSection", 2, 0, 'record', [10, 12], None]) self.assertPartialData("GristSummary_6_Orders", [ "id", "year", "product", "count", "amount", "group" ], [ @@ -193,7 +193,7 @@ class TestDerived(test_engine.EngineTestCase): self.load_sample(self.sample) # Create a summary on the Customers table. Adding orders involves a lookup for each customer. - self.apply_user_action(["CreateViewSection", 1, 0, 'record', [3]]) + self.apply_user_action(["CreateViewSection", 1, 0, 'record', [3], None]) self.add_column("GristSummary_9_Customers", "totalAmount", formula="sum(sum(Orders.lookupRecords(customer=c).amount) for c in $group)") @@ -263,7 +263,7 @@ class TestDerived(test_engine.EngineTestCase): self.load_sample(self.sample) # Create a summary table summarizing count and total of orders by year. - self.apply_user_action(["CreateViewSection", 2, 0, 'record', [10]]) + self.apply_user_action(["CreateViewSection", 2, 0, 'record', [10], None]) self.assertPartialData("GristSummary_6_Orders", ["id", "year", "count", "amount", "group" ], [ [1, 2012, 1, 15.0, [1]], [2, 2013, 2, 30.0, [2,3]], diff --git a/sandbox/grist/test_display_cols.py b/sandbox/grist/test_display_cols.py index fe29433f..8c571271 100644 --- a/sandbox/grist/test_display_cols.py +++ b/sandbox/grist/test_display_cols.py @@ -135,7 +135,7 @@ class TestUserActions(test_engine.EngineTestCase): ]) # Add a new column with a formula. - self.apply_user_action(['AddColumn', 'Favorites', 'fav_viewers', { + self.apply_user_action(['AddVisibleColumn', 'Favorites', 'fav_viewers', { 'formula': '$favorite.viewers' }]) # Add a field back for the favorites table and set its display formula to the diff --git a/sandbox/grist/test_rules.py b/sandbox/grist/test_rules.py index b045904d..1efaa549 100644 --- a/sandbox/grist/test_rules.py +++ b/sandbox/grist/test_rules.py @@ -201,7 +201,7 @@ class TestRules(test_engine.EngineTestCase): # Test that rules are removed with a column when attached to a field. self.load_sample(self.sample) - self.apply_user_action(['CreateViewSection', 1, 0, 'record', None]) + self.apply_user_action(['CreateViewSection', 1, 0, 'record', None, None]) self.field_add_empty(2) self.field_set_rule(2, 0, "$Stock == 0") before = self.engine.docmodel.columns.lookupOne(colId='gristHelper_ConditionalRule') @@ -218,7 +218,7 @@ class TestRules(test_engine.EngineTestCase): # Test that rules are removed with a field. self.load_sample(self.sample) - self.apply_user_action(['CreateViewSection', 1, 0, 'record', None]) + self.apply_user_action(['CreateViewSection', 1, 0, 'record', None, None]) self.field_add_empty(2) self.field_set_rule(2, 0, "$Stock == 0") rule_id = self.engine.docmodel.columns.lookupOne(colId='gristHelper_ConditionalRule').id diff --git a/sandbox/grist/test_summary.py b/sandbox/grist/test_summary.py index 6336965e..97466299 100644 --- a/sandbox/grist/test_summary.py +++ b/sandbox/grist/test_summary.py @@ -96,7 +96,7 @@ class TestSummary(test_engine.EngineTestCase): self.assertViews([]) # Create a view + section for the initial table. - self.apply_user_action(["CreateViewSection", 1, 0, "record", None]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", None, None]) # Verify that we got a new view, with one section, and three fields. self.assertTables([self.starting_table]) @@ -112,7 +112,7 @@ class TestSummary(test_engine.EngineTestCase): self.assertTableData("Address", self.starting_table_data) # Create a "Totals" section, i.e. a summary with no group-by columns. - self.apply_user_action(["CreateViewSection", 1, 0, "record", []]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [], None]) # Verify that a new table gets created, and a new view, with a section for that table, # and some auto-generated summary fields. @@ -141,7 +141,7 @@ class TestSummary(test_engine.EngineTestCase): ]) # Create a summary section, grouped by the "State" column. - self.apply_user_action(["CreateViewSection", 1, 0, "record", [12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [12], None]) # Verify that a new table gets created again, a new view, and a section for that table. # Note that we also check that summarySourceTable and summarySourceCol fields are correct. @@ -182,7 +182,7 @@ class TestSummary(test_engine.EngineTestCase): ]) # Create a summary section grouped by two columns ("city" and "state"). - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) # Verify the new table and views. summary_table3 = Table(4, "GristSummary_7_Address3", primaryViewId=0, summarySourceTable=1, @@ -229,8 +229,8 @@ class TestSummary(test_engine.EngineTestCase): def test_summary_gencode(self): self.maxDiff = 1000 # If there is a discrepancy, allow the bigger diff. self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", []]) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [], None]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) self.assertMultiLineEqual(self.engine.fetch_table_schema(), """import grist from functions import * # global uppercase functions @@ -266,7 +266,7 @@ class Address: self.load_sample(self.sample) # Create a summary section grouped by two columns ("city" and "state"). - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) # Verify the new table and views. summary_table = Table(2, "GristSummary_7_Address", primaryViewId=0, summarySourceTable=1, @@ -293,8 +293,8 @@ class Address: # Create twoo other views + view sections with the same breakdown (in different order # of group-by fields, which should still reuse the same table). - self.apply_user_action(["CreateViewSection", 1, 0, "record", [12,11]]) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [12,11], None]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) summary_view2 = View(2, sections=[ Section(2, parentKey="record", tableRef=2, fields=[ Field(5, colRef=15), @@ -337,8 +337,8 @@ class Address: # Load table and create a couple summary sections, for totals, and grouped by "state". self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", []]) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [], None]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [12], None]) self.assertTables([ self.starting_table, @@ -376,8 +376,8 @@ class Address: ]) # Now create similar summary sections for the new table. - self.apply_user_action(["CreateViewSection", 4, 0, "record", []]) - self.apply_user_action(["CreateViewSection", 4, 0, "record", [23]]) + self.apply_user_action(["CreateViewSection", 4, 0, "record", [], None]) + self.apply_user_action(["CreateViewSection", 4, 0, "record", [23], None]) # Make sure this creates new section rather than reuses similar ones for the wrong table. self.assertTables([ @@ -421,7 +421,7 @@ class Address: # Load sample and create a summary section grouped by two columns ("city" and "state"). self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) # Verify that the summary table respects all updates to the source table. self._do_test_updates("Address", "GristSummary_7_Address") @@ -536,7 +536,7 @@ class Address: # Load sample and create a couple of summary sections. self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) # Check what tables we have now. self.assertPartialData("_grist_Tables", ["id", "tableId", "summarySourceTable"], [ @@ -561,8 +561,8 @@ class Address: # Similar to the above, verify renames, but now with two summary tables. self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) - self.apply_user_action(["CreateViewSection", 1, 0, "record", []]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [], None]) self.assertPartialData("_grist_Tables", ["id", "tableId", "summarySourceTable"], [ [1, "Address", 0], [2, "GristSummary_7_Address", 1], @@ -604,8 +604,8 @@ class Address: # table, sharing all formulas and differing only in the group-by columns.) self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) - self.apply_user_action(["CreateViewSection", 1, 0, "record", []]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [], None]) # These are the tables and columns we automatically get. self.assertTables([ @@ -667,7 +667,7 @@ class Address: ]) # Add a new summary table, and check that it gets the new formula. - self.apply_user_action(["CreateViewSection", 1, 0, "record", [12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [12], None]) self.assertTables([ self.starting_table, Table(2, "GristSummary_7_Address", 0, 1, columns=[ @@ -710,9 +710,9 @@ class Address: # Verify that we can convert the type of a column when there is a summary table using that # column to group by. Since converting generates extra summary records, this may cause bugs. - self.apply_user_action(["AddEmptyTable"]) + self.apply_user_action(["AddEmptyTable", None]) self.apply_user_action(["BulkAddRecord", "Table1", [None]*3, {"A": [10,20,10], "B": [1,2,3]}]) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [2]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [2], None]) # Verify metadata and actual data initially. self.assertTables([ @@ -778,10 +778,10 @@ class Address: # Verify that we can remove a column when there is a summary table using that column to group # by. (Bug T188.) - self.apply_user_action(["AddEmptyTable"]) + self.apply_user_action(["AddEmptyTable", None]) self.apply_user_action(["BulkAddRecord", "Table1", [None]*3, {"A": ['a','b','c'], "B": [1,1,2], "C": [4,5,6]}]) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [2,3]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [2,3], None]) # Verify metadata and actual data initially. self.assertTables([ diff --git a/sandbox/grist/test_summary2.py b/sandbox/grist/test_summary2.py index b68754b0..1fe88663 100644 --- a/sandbox/grist/test_summary2.py +++ b/sandbox/grist/test_summary2.py @@ -27,8 +27,8 @@ class TestSummary2(test_engine.EngineTestCase): # Start as in test_change_summary_formula() test case; see there for what tables and columns # we expect to have at this point. self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) - self.apply_user_action(["CreateViewSection", 1, 0, "record", []]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [], None]) # Check that we cannot add a non-formula column. with self.assertRaisesRegex(ValueError, r'non-formula column'): @@ -37,16 +37,17 @@ class TestSummary2(test_engine.EngineTestCase): # Add two formula columns: one for 'state' (an existing column name, and a group-by column in # some tables), and one for 'average' (a new column name). - self.apply_user_action(["AddColumn", "GristSummary_7_Address2", "state", + self.apply_user_action(["AddVisibleColumn", "GristSummary_7_Address2", "state", {"formula": "':'.join(sorted(set($group.state)))"}]) - self.apply_user_action(["AddColumn", "GristSummary_7_Address", "average", + + self.apply_user_action(["AddVisibleColumn", "GristSummary_7_Address", "average", {"formula": "$amount / $count"}]) # Add two more summary tables: by 'city', and by 'state', and see what columns they get. - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11]]) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11], None]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [12], None]) # And also a summary table for an existing breakdown. - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) # Check the table and columns for all the summary tables. self.assertTables([ @@ -166,8 +167,8 @@ class TestSummary2(test_engine.EngineTestCase): # Start as in test_change_summary_formula() test case; see there for what tables and columns # we expect to have at this point. self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) - self.apply_user_action(["CreateViewSection", 1, 0, "record", []]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [], None]) # Check that we cannot rename a summary group-by column. (Perhaps it's better to raise an # exception, but currently we translate the invalid request to a no-op.) @@ -336,8 +337,8 @@ class TestSummary2(test_engine.EngineTestCase): # (5) no renaming summary tables. self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) - self.apply_user_action(["CreateViewSection", 1, 0, "record", []]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [], None]) self.assertTableData('GristSummary_7_Address', cols="all", data=[ [ "id", "city", "state", "group", "count", "amount" ], @@ -418,7 +419,7 @@ class TestSummary2(test_engine.EngineTestCase): return [c for c in self.engine.tables[table_id].all_columns if c.startswith('#summary#')] self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) # We should have a single summary table, and a single section referring to it. self.assertTables([ @@ -582,7 +583,7 @@ class TestSummary2(test_engine.EngineTestCase): self.assertEqual(get_helper_cols('Address'), ['#summary#GristSummary_7_Address']) # Now add a different view section with the same group-by columns. - self.apply_user_action(["CreateViewSection", 1, 1, "record", [11,12]]) + self.apply_user_action(["CreateViewSection", 1, 1, "record", [11,12], None]) self.assertTables([ self.starting_table, Table(6, "GristSummary_7_Address", 0, 1, columns=[ @@ -737,8 +738,8 @@ class TestSummary2(test_engine.EngineTestCase): # Verify that if we add a group-by column that conflicts with a formula, group-by column wins. self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [12]]) - self.apply_user_action(["AddColumn", "GristSummary_7_Address", "city", + self.apply_user_action(["CreateViewSection", 1, 0, "record", [12], None]) + self.apply_user_action(["AddVisibleColumn", "GristSummary_7_Address", "city", {"formula": "$state.lower()"}]) # We should have a single summary table, and a single section referring to it. @@ -801,10 +802,10 @@ class TestSummary2(test_engine.EngineTestCase): # Create one view with one summary section, and another view with three sections. self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) # Creates View #1 - self.apply_user_action(["CreateViewSection", 1, 0, "record", []]) # Creates View #2 - self.apply_user_action(["CreateViewSection", 1, 2, "record", [11,12]]) # Refers to View #2 - self.apply_user_action(["CreateViewSection", 1, 2, "record", [12]]) # Refers to View #2 + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) # Creates View #1 + self.apply_user_action(["CreateViewSection", 1, 0, "record", [], None]) # Creates View #2 + self.apply_user_action(["CreateViewSection", 1, 2, "record", [11,12], None]) # Refers to View #2 + self.apply_user_action(["CreateViewSection", 1, 2, "record", [12], None]) # Refers to View #2 # We should have a single summary table, and a single section referring to it. self.assertTables([ @@ -883,7 +884,7 @@ class TestSummary2(test_engine.EngineTestCase): # Verify that we correctly update sort spec when we update a summary view section. self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) self.apply_user_action(["UpdateRecord", "_grist_Views_section", 1, {"sortColRefs": "[15,14,-17]"}]) @@ -927,10 +928,10 @@ class TestSummary2(test_engine.EngineTestCase): self.load_sample(self.sample) # Add a couple of summary tables. - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) - self.apply_user_action(["CreateViewSection", 1, 0, "record", []]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [], None]) # Add a formula column - self.apply_user_action(["AddColumn", "GristSummary_7_Address", "average", + self.apply_user_action(["AddVisibleColumn", "GristSummary_7_Address", "average", {"formula": "$amount / $count"}]) # Check the table and columns for all the summary tables. @@ -1037,11 +1038,11 @@ class TestSummary2(test_engine.EngineTestCase): # Add a summary table and detach it. Then add a summary table of that table. self.load_sample(self.sample) - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11,12], None]) self.apply_user_action(["DetachSummaryViewSection", 1]) # Create a summary of the detached table (tableRef 3) by state (colRef 21). - self.apply_user_action(["CreateViewSection", 3, 0, "record", [21]]) + self.apply_user_action(["CreateViewSection", 3, 0, "record", [21], None]) # Verify the resulting metadata. self.assertTables([ diff --git a/sandbox/grist/test_summary_choicelist.py b/sandbox/grist/test_summary_choicelist.py index 202a8d3b..bfb0bed2 100644 --- a/sandbox/grist/test_summary_choicelist.py +++ b/sandbox/grist/test_summary_choicelist.py @@ -43,7 +43,7 @@ class TestSummaryChoiceList(EngineTestCase): self.assertViews([]) # Create a summary section, grouped by the "choices1" column. - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11], None]) summary_table1 = Table( 2, "GristSummary_6_Source", primaryViewId=0, summarySourceTable=1, @@ -57,7 +57,7 @@ class TestSummaryChoiceList(EngineTestCase): ) # Create another summary section, grouped by both choicelist columns. - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11, 12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11, 12], None]) summary_table2 = Table( 3, "GristSummary_6_Source2", primaryViewId=0, summarySourceTable=1, @@ -72,7 +72,7 @@ class TestSummaryChoiceList(EngineTestCase): ) # Create another summary section, grouped by the non-choicelist column - self.apply_user_action(["CreateViewSection", 1, 0, "record", [10]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [10], None]) summary_table3 = Table( 4, "GristSummary_6_Source3", primaryViewId=0, summarySourceTable=1, @@ -86,7 +86,7 @@ class TestSummaryChoiceList(EngineTestCase): ) # Create another summary section, grouped by the non-choicelist column and choices1 - self.apply_user_action(["CreateViewSection", 1, 0, "record", [10, 11]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [10, 11], None]) summary_table4 = Table( 5, "GristSummary_6_Source4", primaryViewId=0, summarySourceTable=1, @@ -321,7 +321,7 @@ class TestSummaryChoiceList(EngineTestCase): self.assertViews([]) # Create a summary section, grouped by the "choices1" column. - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11], None]) summary_table = Table( 2, "GristSummary_6_Source", primaryViewId=0, summarySourceTable=1, @@ -359,7 +359,7 @@ class TestSummaryChoiceList(EngineTestCase): self.load_sample(self.sample) # Create a summary section, grouped by both choicelist columns. - self.apply_user_action(["CreateViewSection", 1, 0, "record", [11, 12]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [11, 12], None]) summary_table = Table( 2, "GristSummary_6_Source", primaryViewId=0, summarySourceTable=1, diff --git a/sandbox/grist/test_summary_undo.py b/sandbox/grist/test_summary_undo.py index 9f3247aa..ab8ef863 100644 --- a/sandbox/grist/test_summary_undo.py +++ b/sandbox/grist/test_summary_undo.py @@ -30,7 +30,7 @@ class TestSummaryUndo(test_engine.EngineTestCase): # This tests a particular case of a bug when a summary table wasn't fully updated after UNDO. self.load_sample(self.sample) # Create a summary section, grouped by the "State" column. - self.apply_user_action(["CreateViewSection", 1, 0, "record", [1]]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", [1], None]) self.assertTableData('GristSummary_6_Person', cols="subset", data=[ [ "id", "state", "count"], [ 1, "NY", 2], diff --git a/sandbox/grist/test_table_actions.py b/sandbox/grist/test_table_actions.py index d37ad96b..31a93f35 100644 --- a/sandbox/grist/test_table_actions.py +++ b/sandbox/grist/test_table_actions.py @@ -53,9 +53,9 @@ class TestTableActions(test_engine.EngineTestCase): self.apply_user_action(["BulkAddRecord", "People", d.row_ids, d.columns]) # Add a view with several sections, including a summary table. - self.apply_user_action(["CreateViewSection", 1, 0, 'record', None]) - self.apply_user_action(["CreateViewSection", 1, 3, 'record', [3]]) - self.apply_user_action(["CreateViewSection", 2, 3, 'record', None]) + self.apply_user_action(["CreateViewSection", 1, 0, 'record', None, None]) + self.apply_user_action(["CreateViewSection", 1, 3, 'record', [3], None]) + self.apply_user_action(["CreateViewSection", 2, 3, 'record', None, None]) # Verify the new structure of tables and views. self.assertTables([ @@ -211,7 +211,7 @@ class TestTableActions(test_engine.EngineTestCase): self.init_sample_data() # Add a table grouped by a reference column (the 'Ref:Address' column named 'address'). - self.apply_user_action(["CreateViewSection", 2, 0, 'record', [7]]) + self.apply_user_action(["CreateViewSection", 2, 0, 'record', [7], None]) self.assertTableData('_grist_Tables_column', cols="subset", data=[ ["id", "colId", "type", "isFormula", "formula" ], [ 13, "address", "Ref:Address", False, "" ], @@ -258,10 +258,10 @@ class TestTableActions(test_engine.EngineTestCase): self.init_sample_data() # Add one more table, and one more view for tables #1 and #4 (those we are about to delete). - self.apply_user_action(["AddEmptyTable"]) - out_actions = self.apply_user_action(["CreateViewSection", 1, 0, 'detail', None]) + self.apply_user_action(["AddEmptyTable", None]) + out_actions = self.apply_user_action(["CreateViewSection", 1, 0, 'detail', None, None]) self.assertEqual(out_actions.retValues[0]["viewRef"], 5) - self.apply_user_action(["CreateViewSection", 4, 5, 'detail', None]) + self.apply_user_action(["CreateViewSection", 4, 5, 'detail', None, None]) # See what's in TabBar table, to verify after we remove a table. self.assertTableData('_grist_TabBar', cols="subset", data=[ diff --git a/sandbox/grist/test_undo.py b/sandbox/grist/test_undo.py index 76fa1681..439cb7fa 100644 --- a/sandbox/grist/test_undo.py +++ b/sandbox/grist/test_undo.py @@ -6,7 +6,7 @@ class TestUndo(test_engine.EngineTestCase): def test_bad_undo(self): # Sometimes undo can make metadata inconsistent with schema. Check that we disallow it. self.load_sample(testsamples.sample_students) - out_actions1 = self.apply_user_action(['AddEmptyTable']) + out_actions1 = self.apply_user_action(['AddEmptyTable', None]) self.assertPartialData("_grist_Tables", ["id", "tableId", "columns"], [ [1, "Students", [1,2,4,5,6]], [2, "Schools", [10,12]], @@ -60,7 +60,7 @@ class TestUndo(test_engine.EngineTestCase): # during undo of imports when the undo could omit part of the action bundle. self.load_sample(testsamples.sample_students) - out_actions1 = self.apply_user_action(['AddEmptyTable']) + out_actions1 = self.apply_user_action(['AddEmptyTable', None]) out_actions2 = self.add_column('Table1', 'D', type='Text') out_actions3 = self.remove_column('Table1', 'D') out_actions4 = self.apply_user_action(['RemoveTable', 'Table1']) diff --git a/sandbox/grist/test_useractions.py b/sandbox/grist/test_useractions.py index 1b17bddb..9eadf1f3 100644 --- a/sandbox/grist/test_useractions.py +++ b/sandbox/grist/test_useractions.py @@ -77,10 +77,7 @@ class TestUserActions(test_engine.EngineTestCase): 'type': 'Text' }], ["AddRecord", "_grist_Views_section_field", 3, { - "colRef": 24, "parentId": 1, "parentPos": 3.0 - }], - ["AddRecord", "_grist_Views_section_field", 4, { - "colRef": 24, "parentId": 2, "parentPos": 4.0 + "colRef": 24, "parentId": 2, "parentPos": 3.0 }], ["BulkUpdateRecord", "Schools", [1, 2, 3], {"grist_Transform": ["New York", "Colombia", "New York"]}], @@ -124,7 +121,7 @@ class TestUserActions(test_engine.EngineTestCase): out_actions = self.remove_column('Schools', 'grist_Transform') self.assertPartialOutActions(out_actions, { "stored": [ - ["BulkRemoveRecord", "_grist_Views_section_field", [3, 4]], + ["RemoveRecord", "_grist_Views_section_field", 3], ['RemoveRecord', '_grist_Tables_column', 24], ['RemoveColumn', 'Schools', 'grist_Transform'], ]}) @@ -137,7 +134,7 @@ class TestUserActions(test_engine.EngineTestCase): self.assertTables([self.starting_table]) # Create a view + section for the initial table. - self.apply_user_action(["CreateViewSection", 1, 0, "record", None]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", None, None]) # Verify that we got a new view, with one section, and three fields. self.assertViews([View(1, sections=[ @@ -147,7 +144,7 @@ class TestUserActions(test_engine.EngineTestCase): ]) ]) # Create a new section for the same view, check that only a section is added. - self.apply_user_action(["CreateViewSection", 1, 1, "record", None]) + self.apply_user_action(["CreateViewSection", 1, 1, "record", None, None]) self.assertTables([self.starting_table]) self.assertViews([View(1, sections=[ Section(1, parentKey="record", tableRef=1, fields=[ @@ -159,7 +156,7 @@ class TestUserActions(test_engine.EngineTestCase): ]) ]) # Create another section for the same view, this time summarized. - self.apply_user_action(["CreateViewSection", 1, 1, "record", [21]]) + self.apply_user_action(["CreateViewSection", 1, 1, "record", [21], None]) summary_table = Table(2, "GristSummary_7_Address", 0, summarySourceTable=1, columns=[ Column(22, "city", "Text", isFormula=False, formula="", summarySourceCol=21), Column(23, "group", "RefList:Address", isFormula=True, @@ -185,7 +182,7 @@ class TestUserActions(test_engine.EngineTestCase): # Try to create a summary table for an invalid column, and check that it fails. with self.assertRaises(ValueError): - self.apply_user_action(["CreateViewSection", 1, 1, "record", [23]]) + self.apply_user_action(["CreateViewSection", 1, 1, "record", [23], None]) self.assertTables([self.starting_table, summary_table]) self.assertViews([view]) @@ -197,69 +194,55 @@ class TestUserActions(test_engine.EngineTestCase): self.assertTables([self.starting_table]) self.assertViews([]) - # When we create a section/view for new table, we get both a primary view, and the new view we - # are creating. - self.apply_user_action(["CreateViewSection", 0, 0, "record", None]) - new_table = Table(2, "Table1", primaryViewId=1, summarySourceTable=0, columns=[ + # When we create a section/view for new table, we got the new view we are creating, + # without primary view. + self.apply_user_action(["CreateViewSection", 0, 0, "record", None, None]) + new_table = Table(2, "Table1", primaryViewId=0, summarySourceTable=0, columns=[ Column(22, "manualSort", "ManualSortPos", isFormula=False, formula="", summarySourceCol=0), Column(23, "A", "Any", isFormula=True, formula="", summarySourceCol=0), Column(24, "B", "Any", isFormula=True, formula="", summarySourceCol=0), Column(25, "C", "Any", isFormula=True, formula="", summarySourceCol=0), ]) - primary_view = View(1, sections=[ - Section(1, parentKey="record", tableRef=2, fields=[ - Field(1, colRef=23), - Field(2, colRef=24), - Field(3, colRef=25), - ]) - ]) - new_view = View(2, sections=[ - Section(3, parentKey="record", tableRef=2, fields=[ - Field(7, colRef=23), - Field(8, colRef=24), - Field(9, colRef=25), + new_view = View(1, sections=[ + Section(2, parentKey="record", tableRef=2, fields=[ + Field(4, colRef=23), + Field(5, colRef=24), + Field(6, colRef=25), ]) ]) self.assertTables([self.starting_table, new_table]) - self.assertViews([primary_view, new_view]) + self.assertViews([new_view]) # Create another section in an existing view for a new table. - self.apply_user_action(["CreateViewSection", 0, 2, "record", None]) - new_table2 = Table(3, "Table2", primaryViewId=3, summarySourceTable=0, columns=[ + self.apply_user_action(["CreateViewSection", 0, 1, "record", None, None]) + new_table2 = Table(3, "Table2", primaryViewId=0, summarySourceTable=0, columns=[ Column(26, "manualSort", "ManualSortPos", isFormula=False, formula="", summarySourceCol=0), Column(27, "A", "Any", isFormula=True, formula="", summarySourceCol=0), Column(28, "B", "Any", isFormula=True, formula="", summarySourceCol=0), Column(29, "C", "Any", isFormula=True, formula="", summarySourceCol=0), ]) - primary_view2 = View(3, sections=[ + new_view.sections.append( Section(4, parentKey="record", tableRef=3, fields=[ Field(10, colRef=27), Field(11, colRef=28), Field(12, colRef=29), ]) - ]) - new_view.sections.append( - Section(6, parentKey="record", tableRef=3, fields=[ - Field(16, colRef=27), - Field(17, colRef=28), - Field(18, colRef=29), - ]) ) - # Check that we have a new table, only the primary view as new view; and a new section. + # Check that we have a new table, only the new view; and a new section. self.assertTables([self.starting_table, new_table, new_table2]) - self.assertViews([primary_view, new_view, primary_view2]) + self.assertViews([new_view]) # Check that we can't create a summary of a table grouped by a column that doesn't exist yet. with self.assertRaises(ValueError): - self.apply_user_action(["CreateViewSection", 0, 2, "record", [31]]) + self.apply_user_action(["CreateViewSection", 0, 1, "record", [31], None]) self.assertTables([self.starting_table, new_table, new_table2]) - self.assertViews([primary_view, new_view, primary_view2]) + self.assertViews([new_view]) # But creating a new table and showing totals for it is possible though dumb. - self.apply_user_action(["CreateViewSection", 0, 2, "record", []]) + self.apply_user_action(["CreateViewSection", 0, 1, "record", [], None]) # We expect a new table. - new_table3 = Table(4, "Table3", primaryViewId=4, summarySourceTable=0, columns=[ + new_table3 = Table(4, "Table3", primaryViewId=0, summarySourceTable=0, columns=[ Column(30, "manualSort", "ManualSortPos", isFormula=False, formula="", summarySourceCol=0), Column(31, "A", "Any", isFormula=True, formula="", summarySourceCol=0), Column(32, "B", "Any", isFormula=True, formula="", summarySourceCol=0), @@ -271,20 +254,11 @@ class TestUserActions(test_engine.EngineTestCase): formula="table.getSummarySourceGroup(rec)", summarySourceCol=0), Column(35, "count", "Int", isFormula=True, formula="len($group)", summarySourceCol=0), ]) - # The primary view of the new table. - primary_view3 = View(4, sections=[ - Section(7, parentKey="record", tableRef=4, fields=[ - Field(19, colRef=31), - Field(20, colRef=32), - Field(21, colRef=33), - ]) - ]) - # And a new view section for the summary. - new_view.sections.append(Section(9, parentKey="record", tableRef=5, fields=[ - Field(25, colRef=35) - ])) self.assertTables([self.starting_table, new_table, new_table2, new_table3, summary_table]) - self.assertViews([primary_view, new_view, primary_view2, primary_view3]) + new_view.sections.append(Section(6, parentKey="record", tableRef=5, fields=[ + Field(16, colRef=35) + ])) + self.assertViews([new_view]) #---------------------------------------------------------------------- @@ -301,10 +275,10 @@ class TestUserActions(test_engine.EngineTestCase): 'size': [1000, 2000, 3000, 4000], }]) # Add a new view; a second section (summary) to it; and a third view. - self.apply_user_action(['CreateViewSection', 1, 0, 'detail', None]) - self.apply_user_action(['CreateViewSection', 1, 2, 'record', [3]]) - self.apply_user_action(['CreateViewSection', 1, 0, 'chart', None]) - self.apply_user_action(['CreateViewSection', 0, 2, 'record', None]) + self.apply_user_action(['CreateViewSection', 1, 0, 'detail', None, None]) + self.apply_user_action(['CreateViewSection', 1, 2, 'record', [3], None]) + self.apply_user_action(['CreateViewSection', 1, 0, 'chart', None, None]) + self.apply_user_action(['CreateViewSection', 0, 2, 'record', None, None]) # Verify the new structure of tables and views. self.assertTables([ @@ -320,7 +294,7 @@ class TestUserActions(test_engine.EngineTestCase): Column(7, "count", "Int", True, "len($group)", 0), Column(8, "size", "Numeric", True, "SUM($group.size)", 0), ]), - Table(3, 'Table1', 4, 0, columns=[ + Table(3, 'Table1', 0, 0, columns=[ Column(9, "manualSort", "ManualSortPos", False, "", 0), Column(10, "A", "Any", True, "", 0), Column(11, "B", "Any", True, "", 0), @@ -346,10 +320,10 @@ class TestUserActions(test_engine.EngineTestCase): Field(11, colRef=7), Field(12, colRef=8), ]), - Section(8, parentKey='record', tableRef=3, fields=[ - Field(21, colRef=10), - Field(22, colRef=11), - Field(23, colRef=12), + Section(7, parentKey='record', tableRef=3, fields=[ + Field(18, colRef=10), + Field(19, colRef=11), + Field(20, colRef=12), ]), ]), View(3, sections=[ @@ -357,28 +331,19 @@ class TestUserActions(test_engine.EngineTestCase): Field(13, colRef=2), Field(14, colRef=3), ]), - ]), - View(4, sections=[ - Section(6, parentKey='record', tableRef=3, fields=[ - Field(15, colRef=10), - Field(16, colRef=11), - Field(17, colRef=12), - ]), - ]), + ]) ]) self.assertTableData('_grist_TabBar', cols="subset", data=[ ["id", "viewRef"], [1, 1], [2, 2], [3, 3], - [4, 4], ]) self.assertTableData('_grist_Pages', cols="subset", data=[ ["id", "viewRef"], [1, 1], [2, 2], - [3, 3], - [4, 4] + [3, 3] ]) #---------------------------------------------------------------------- @@ -388,7 +353,7 @@ class TestUserActions(test_engine.EngineTestCase): self.init_views_sample() # Remove a view. Ensure related items, sections, fields get removed. - self.apply_user_action(["BulkRemoveRecord", "_grist_Views", [2,3]]) + self.apply_user_action(["BulkRemoveRecord", "_grist_Views", [2, 3]]) # Verify the new structure of tables and views. self.assertTables([ @@ -399,7 +364,7 @@ class TestUserActions(test_engine.EngineTestCase): Column(4, "size", "Numeric", False, "", 0), ]), # Note that the summary table is gone. - Table(3, 'Table1', 4, 0, columns=[ + Table(3, 'Table1', 0, 0, columns=[ Column(9, "manualSort", "ManualSortPos", False, "", 0), Column(10, "A", "Any", True, "", 0), Column(11, "B", "Any", True, "", 0), @@ -413,24 +378,15 @@ class TestUserActions(test_engine.EngineTestCase): Field(2, colRef=3), Field(3, colRef=4), ]), - ]), - View(4, sections=[ - Section(6, parentKey='record', tableRef=3, fields=[ - Field(15, colRef=10), - Field(16, colRef=11), - Field(17, colRef=12), - ]), - ]), + ]) ]) self.assertTableData('_grist_TabBar', cols="subset", data=[ ["id", "viewRef"], [1, 1], - [4, 4], ]) self.assertTableData('_grist_Pages', cols="subset", data=[ ["id", "viewRef"], [1, 1], - [4, 4], ]) #---------------------------------------------------------------------- @@ -444,33 +400,31 @@ class TestUserActions(test_engine.EngineTestCase): [ 'id', 'tableId', 'primaryViewId' ], [ 1, 'Schools', 1], [ 2, 'GristSummary_7_Schools', 0], - [ 3, 'Table1', 4], + [ 3, 'Table1', 0], ]) self.assertTableData('_grist_Views', cols="subset", data=[ [ 'id', 'name', 'primaryViewTable' ], [ 1, 'Schools', 1], [ 2, 'New page', 0], [ 3, 'New page', 0], - [ 4, 'Table1', 3], ]) # Update the names in a few views, and ensure that primary ones won't cause tables to # get renamed. - self.apply_user_action(['BulkUpdateRecord', '_grist_Views', [2,3,4], - {'name': ['A', 'B', 'C']}]) + self.apply_user_action(['BulkUpdateRecord', '_grist_Views', [2, 3], + {'name': ['A', 'B']}]) self.assertTableData('_grist_Tables', cols="subset", data=[ [ 'id', 'tableId', 'primaryViewId' ], [ 1, 'Schools', 1], [ 2, 'GristSummary_7_Schools', 0], - [ 3, 'Table1', 4], + [ 3, 'Table1', 0], ]) self.assertTableData('_grist_Views', cols="subset", data=[ [ 'id', 'name', 'primaryViewTable' ], [ 1, 'Schools', 1], [ 2, 'A', 0], - [ 3, 'B', 0], - [ 4, 'C', 3] + [ 3, 'B', 0] ]) # Now rename a table (by raw view section) and make sure that a view with the same name @@ -482,14 +436,13 @@ class TestUserActions(test_engine.EngineTestCase): ['id', 'tableId'], [1, 'Bars', 1], [2, 'GristSummary_4_Bars', 0], - [3, 'Table1', 4], + [3, 'Table1', 0], ]) self.assertTableData('_grist_Views', cols="subset", data=[ ['id', 'name'], [1, 'Bars'], [2, 'A'], - [3, 'B'], - [4, 'C'] + [3, 'B'] ]) # Now rename tables so that two tables will have same names, to test if only the view @@ -501,47 +454,57 @@ class TestUserActions(test_engine.EngineTestCase): ['id', 'tableId'], [1, 'A', 1], [2, 'GristSummary_1_A', 0], - [3, 'Table1', 4], + [3, 'Table1', 0], ]) self.assertTableData('_grist_Views', cols="subset", data=[ ['id', 'name'], [1, 'A'], [2, 'A'], [3, 'B'], - [4, 'C'] ]) self.apply_user_action(['UpdateRecord', '_grist_Views_section', 2, {'title': 'Z'}]) self.assertTableData('_grist_Tables', cols="subset", data=[ - ['id', 'tableId'], - [1, 'Z', 1], - [2, 'GristSummary_1_Z', 0], - [3, 'Table1', 4], + ['id', 'tableId', 'primaryViewId', 'rawViewSectionRef'], + [1, 'Z', 1, 2], + [2, 'GristSummary_1_Z', 0, 0], + [3, 'Table1', 0, 6], ]) self.assertTableData('_grist_Views', cols="subset", data=[ ['id', 'name'], [1, 'Z'], [2, 'Z'], [3, 'B'], - [4, 'C'] ]) # Add new table, with a view with the same name (Z) and make sure it won't be renamed self.apply_user_action(['AddTable', 'Stations', [ {'id': 'city', 'type': 'Text'}, ]]) - - # Replacing only a page name (though primary) - self.apply_user_action(['UpdateRecord', '_grist_Views', 5, {'name': 'Z'}]) + self.assertTableData('_grist_Tables', cols="subset", data=[ + ['id', 'tableId', 'primaryViewId', 'rawViewSectionRef'], + [1, 'Z', 1, 2], + [2, 'GristSummary_1_Z', 0, 0], + [3, 'Table1', 0, 6], + [4, 'Stations', 4, 9], + ]) self.assertTableData('_grist_Views', cols="subset", data=[ ['id', 'name'], [1, 'Z'], [2, 'Z'], [3, 'B'], - [4, 'C'], - [5, 'Z'] + [4, 'Stations'], + ]) + # Replacing only a page name (though primary) + self.apply_user_action(['UpdateRecord', '_grist_Views', 4, {'name': 'Z'}]) + self.assertTableData('_grist_Views', cols="subset", data=[ + ['id', 'name'], + [1, 'Z'], + [2, 'Z'], + [3, 'B'], + [4, 'Z'] ]) # Rename table Z to Schools. Primary view for Stations (Z) should not be renamed. @@ -560,8 +523,7 @@ class TestUserActions(test_engine.EngineTestCase): [1, 'Schools'], [2, 'Schools'], [3, 'B'], - [4, 'C'], - [5, 'Z'] + [4, 'Z'] ]) #---------------------------------------------------------------------- @@ -570,9 +532,6 @@ class TestUserActions(test_engine.EngineTestCase): # Add a couple of tables and views, to trigger creation of some related items. self.init_views_sample() - # Remove a couple of sections. Ensure their fields get removed. - self.apply_user_action(['BulkRemoveRecord', '_grist_Views_section', [4, 8]]) - self.assertViews([ View(1, sections=[ Section(1, parentKey="record", tableRef=1, fields=[ @@ -587,20 +546,49 @@ class TestUserActions(test_engine.EngineTestCase): Field(8, colRef=3), Field(9, colRef=4), ]), + Section(4, parentKey="record", tableRef=2, fields=[ + Field(10, colRef=5), + Field(11, colRef=7), + Field(12, colRef=8), + ]), + Section(7, parentKey='record', tableRef=3, fields=[ + Field(18, colRef=10), + Field(19, colRef=11), + Field(20, colRef=12), + ]), ]), View(3, sections=[ Section(5, parentKey="chart", tableRef=1, fields=[ Field(13, colRef=2), Field(14, colRef=3), ]), - ]), - View(4, sections=[ - Section(6, parentKey='record', tableRef=3, fields=[ - Field(15, colRef=10), - Field(16, colRef=11), - Field(17, colRef=12), + ]) + ]) + + # Remove a couple of sections. Ensure their fields get removed. + self.apply_user_action(['BulkRemoveRecord', '_grist_Views_section', [4, 7]]) + + self.assertViews([ + View(1, sections=[ + Section(1, parentKey="record", tableRef=1, fields=[ + Field(1, colRef=2), + Field(2, colRef=3), + Field(3, colRef=4), ]), ]), + View(2, sections=[ + Section(3, parentKey="detail", tableRef=1, fields=[ + Field(7, colRef=2), + Field(8, colRef=3), + Field(9, colRef=4), + ]) + ]), + View(3, sections=[ + Section(5, parentKey="chart", tableRef=1, fields=[ + Field(13, colRef=2), + Field(14, colRef=3), + ]), + ]) ]) #---------------------------------------------------------------------- @@ -625,14 +613,14 @@ class TestUserActions(test_engine.EngineTestCase): # Do a schema action to ensure it gets called: this causes a table rename. # 7 is id of raw view section for the Tabl1 table - self.apply_user_action(['UpdateRecord', '_grist_Views_section', 7, {'title': 'C'}]) + self.apply_user_action(['UpdateRecord', '_grist_Views_section', 6, {'title': 'C'}]) self.assertEqual(count_calls[0], 1) self.assertTableData('_grist_Tables', cols="subset", data=[ [ 'id', 'tableId', 'primaryViewId' ], [ 1, 'Schools', 1], [ 2, 'GristSummary_7_Schools', 0], - [ 3, 'C', 4], + [ 3, 'C', 0], ]) # Do another schema and non-schema action. @@ -834,7 +822,6 @@ class TestUserActions(test_engine.EngineTestCase): [ 1, 0], [ 2, 1], [ 3, 0], - [ 4, 0], ]) # Verify that removing page 1 fixes page 2 indentation. @@ -843,7 +830,6 @@ class TestUserActions(test_engine.EngineTestCase): ['id', 'indentation'], [ 2, 0], [ 3, 0], - [ 4, 0], ]) # Removing last page should not fail @@ -962,7 +948,7 @@ class TestUserActions(test_engine.EngineTestCase): # Test filters rename # Create new view section - self.apply_user_action(["CreateViewSection", 1, 0, "record", None]) + self.apply_user_action(["CreateViewSection", 1, 0, "record", None, None]) # Filter it by first column self.apply_user_action(['BulkAddRecord', '_grist_Filters', [None], { @@ -1287,7 +1273,7 @@ class TestUserActions(test_engine.EngineTestCase): # load_sample handles loading basic metadata, but doesn't create any view sections self.load_sample(self.sample) # Create a new table which automatically gets a raw view section - self.apply_user_action(["AddEmptyTable"]) + self.apply_user_action(["AddEmptyTable", None]) # Note the row IDs of the raw view section (2) and fields (4, 5, 6) self.assertTableData('_grist_Views_section', cols="subset", data=[ @@ -1339,7 +1325,7 @@ class TestUserActions(test_engine.EngineTestCase): def test_update_current_time(self): self.load_sample(self.sample) - self.apply_user_action(["AddEmptyTable"]) + self.apply_user_action(["AddEmptyTable", None]) self.add_column('Table1', 'now', isFormula=True, formula='NOW()', type='Any') # No records with NOW() in a formula yet, so this action should have no effect at all. @@ -1374,7 +1360,7 @@ class TestUserActions(test_engine.EngineTestCase): check(1) # Testing that unrelated actions don't change the time - self.apply_user_action(["AddEmptyTable"]) + self.apply_user_action(["AddEmptyTable", None]) self.add_record("Table2") self.apply_user_action(["Calculate"]) # only recalculates for fresh docs check(1) diff --git a/sandbox/grist/testscript.json b/sandbox/grist/testscript.json index 10d0133a..0bfef040 100644 --- a/sandbox/grist/testscript.json +++ b/sandbox/grist/testscript.json @@ -916,7 +916,7 @@ {"parentId": 1, "colRef": 34, "parentPos": 1.0}], // Raw data widget - ["AddRecord", "_grist_Views_section", 2, {"borderWidth": 1, "defaultWidth": 100, "parentKey": "record", "tableRef": 4}], + ["AddRecord", "_grist_Views_section", 2, {"borderWidth": 1, "defaultWidth": 100, "parentKey": "record", "tableRef": 4, "title": ""}], ["AddRecord", "_grist_Views_section_field", 2, {"colRef": 34, "parentId": 2, "parentPos": 2.0}], ["UpdateRecord", "_grist_Tables", 4, {"primaryViewId": 1, "rawViewSectionRef": 2}], @@ -927,10 +927,9 @@ {"colId": "world", "parentPos": 13.0, "formula": "rec.hello.upper()", "parentId": 4, "type": "Text", "isFormula": true, "label": "world", "widgetOptions": ""}], - ["AddRecord", "_grist_Views_section_field", 3, {"colRef": 35, "parentId": 1, "parentPos": 3.0}], - ["AddRecord", "_grist_Views_section_field", 4, {"colRef": 35, "parentId": 2, "parentPos": 4.0}] + ["AddRecord", "_grist_Views_section_field", 3, {"colRef": 35, "parentId": 2, "parentPos": 3.0}] ], - "direct": [true, true, true, true, true, + "direct": [true, true, true, true, true, true, true, true, true, true, true, true, true, true], "undo": [ @@ -947,8 +946,7 @@ ["UpdateRecord", "_grist_Tables", 4, {"primaryViewId": 0, "rawViewSectionRef": 0}], ["RemoveColumn", "Bar", "world"], ["RemoveRecord", "_grist_Tables_column", 35], - ["RemoveRecord", "_grist_Views_section_field", 3], - ["RemoveRecord", "_grist_Views_section_field", 4] + ["RemoveRecord", "_grist_Views_section_field", 3] ], "retValue": [ { @@ -1257,7 +1255,7 @@ "parentId": 1, "parentKey": "record", "sortColRefs": "[]", "title": ""}], ["BulkAddRecord", "_grist_Views_section_field", [1,2], {"parentId": [1,1], "colRef": [31,32], "parentPos": [1.0,2.0]}], - ["AddRecord", "_grist_Views_section", 2, {"borderWidth": 1, "defaultWidth": 100, "parentKey": "record", "tableRef": 4}], + ["AddRecord", "_grist_Views_section", 2, {"borderWidth": 1, "defaultWidth": 100, "parentKey": "record", "tableRef": 4, "title": ""}], ["BulkAddRecord", "_grist_Views_section_field", [3, 4], {"colRef": [31, 32], "parentId": [2, 2], "parentPos": [3.0, 4.0]}], ["UpdateRecord", "_grist_Tables", 4, {"primaryViewId": 1, "rawViewSectionRef": 2}], ["BulkRemoveRecord", "_grist_Views_section_field", [1, 3]], @@ -2200,7 +2198,7 @@ ["AddRecord", "_grist_Views_section", 1, {"tableRef": 4, "defaultWidth": 100, "borderWidth": 1, "parentId": 1, "parentKey": "record", "sortColRefs": "[]", "title": ""}], - ["AddRecord", "_grist_Views_section", 2, {"borderWidth": 1, "defaultWidth": 100, "parentKey": "record", "tableRef": 4}], + ["AddRecord", "_grist_Views_section", 2, {"borderWidth": 1, "defaultWidth": 100, "parentKey": "record", "tableRef": 4, "title": ""}], ["UpdateRecord", "_grist_Tables", 4, {"primaryViewId": 1, "rawViewSectionRef": 2}], ["AddTable", "Bar", [ {"id": "manualSort", "formula": "", "isFormula": false, "type": "ManualSortPos"}, @@ -2230,7 +2228,7 @@ "parentId": 2, "parentKey": "record", "sortColRefs": "[]", "title": ""}], ["BulkAddRecord", "_grist_Views_section_field", [1,2,3], {"parentId": [3,3,3], "colRef": [32,33,34], "parentPos": [1.0,2.0,3.0]}], - ["AddRecord", "_grist_Views_section", 4, {"borderWidth": 1, "defaultWidth": 100, "parentKey": "record", "tableRef": 5}], + ["AddRecord", "_grist_Views_section", 4, {"borderWidth": 1, "defaultWidth": 100, "parentKey": "record", "tableRef": 5, "title": ""}], ["BulkAddRecord", "_grist_Views_section_field", [4, 5, 6], {"colRef": [32, 33, 34], "parentId": [4, 4, 4], "parentPos": [4.0, 5.0, 6.0]}], ["UpdateRecord", "_grist_Tables", 5, {"primaryViewId": 2, "rawViewSectionRef": 4}], ["AddRecord", "Bar", 1, {"foo": 0, "hello": "a", "manualSort": 1.0}], @@ -2334,7 +2332,7 @@ {"tableRef": 4, "defaultWidth": 100, "borderWidth": 1, "parentId": 1, "parentKey": "record", "sortColRefs": "[]", "title": ""}], // Raw data widget - ["AddRecord", "_grist_Views_section", 2, {"borderWidth": 1, "defaultWidth": 100, "parentKey": "record", "tableRef": 4}], + ["AddRecord", "_grist_Views_section", 2, {"borderWidth": 1, "defaultWidth": 100, "parentKey": "record", "tableRef": 4, "title": ""}], // As part of adding a table, we also set the primaryViewId. ["UpdateRecord", "_grist_Tables", 4, {"primaryViewId": 1, "rawViewSectionRef": 2}] ], diff --git a/sandbox/grist/useractions.py b/sandbox/grist/useractions.py index 061d17e0..e05cb2bc 100644 --- a/sandbox/grist/useractions.py +++ b/sandbox/grist/useractions.py @@ -1004,10 +1004,6 @@ class UserActions(object): # Remove any views that no longer have view sections. views_to_remove = [view for view in self._docmodel.views.all if not view.viewSections] - # Also delete the primary views for tables being deleted, even if it has remaining sections. - for t in remove_table_recs: - if t.primaryViewId and t.primaryViewId not in views_to_remove: - views_to_remove.append(t.primaryViewId) self._docmodel.remove(views_to_remove) # Save table IDs, which will be inaccessible once we remove the metadata records. @@ -1211,14 +1207,16 @@ class UserActions(object): ret = self.doAddColumn(table_id, col_id, col_info) - if not transform: - # Add a field for this column to the "raw_data" view(s) for this table. - for section in table_rec.viewSections: - if section.parentKey == 'record': - # TODO: the position of the inserted field or of the inserted column will often be - # bogus, since fields and columns are not the same. This requires better coordination - # with the client-side. - self._docmodel.insert(section.fields, col_info.get('_position'), colRef=ret['colRef']) + if not transform and table_rec.rawViewSectionRef: + # Add a field for this column to the "raw_data" section for this table. + # TODO: the position of the inserted field or of the inserted column will often be + # bogus, since fields and columns are not the same. This requires better coordination + # with the client-side. + self._docmodel.insert( + table_rec.rawViewSectionRef.fields, + col_info.get('_position'), + colRef=ret['colRef'] + ) return ret @@ -1226,6 +1224,33 @@ class UserActions(object): def AddHiddenColumn(self, table_id, col_id, col_info): return self.doAddColumn(table_id, col_id, col_info) + + @useraction + def AddVisibleColumn(self, table_id, col_id, col_info): + '''Inserts column and adds it as a field to all 'record' views''' + + ret = self.AddColumn(table_id, col_id, col_info) + table_rec = self._docmodel.get_table_rec(table_id) + + transform = ( + col_id is not None and + col_id.startswith(( + 'gristHelper_Transform', + 'gristHelper_Converted', + )) + ) + + # Add a field for this column to the view(s) for this table. + if not transform: + for section in table_rec.viewSections: + if section.parentKey == 'record' and section != table_rec.rawViewSectionRef: + # TODO: the position of the inserted field or of the inserted column will often be + # bogus, since fields and columns are not the same. This requires better coordination + # with the client-side. + self._docmodel.insert(section.fields, col_info.get('_position'), colRef=ret['colRef']) + return ret + + @classmethod def _pick_col_name(cls, table_rec, col_id, old_col_id=None, avoid_extra=None): avoid_set = set(c.colId for c in table_rec.columns) @@ -1594,15 +1619,45 @@ class UserActions(object): #---------------------------------------- @useraction - def AddEmptyTable(self): + def AddEmptyTable(self, table_id): """ - Adds an empty table. Currently it makes up the next available table name, and adds three - default columns, also picking default names for them (presumably, A, B, and C). + Adds an empty table. Currently it makes up the next available table name (if not provided), + and adds three default columns, also picking default names for them (presumably, A, B, and C). """ - return self.AddTable(None, [{'id': None, 'isFormula': True} for x in xrange(3)]) + columns = [{'id': None, 'isFormula': True} for x in xrange(3)] + return self.AddTable(table_id, columns) + @useraction def AddTable(self, table_id, columns): + return self.doAddTable( + table_id, + columns, + manual_sort=True, + primary_view=True, + raw_section=True) + + + @useraction + def AddRawTable(self, table_id): + """ + Same as AddEmptyTable but does not create a primary view (and page). + """ + columns = [{'id': None, 'isFormula': True} for x in xrange(3)] + return self.doAddTable( + table_id, + columns, + manual_sort=True, + primary_view=False, + raw_section=True + ) + + + def doAddTable(self, table_id, columns, manual_sort=False, primary_view=False, + raw_section=False, summarySourceTableRef=0): + """ + Add the given table with columns with or without additional views. + """ # For any columns missing 'isFormula' field, default to False when formula is empty. We will # normally default new columns to "empty" (isFormula=True), and AddEmptyTable creates empty # columns, but an AddTable action created e.g. by an import will default to data columns. @@ -1610,35 +1665,14 @@ class UserActions(object): c.setdefault("isFormula", bool(c.get('formula'))) # Add a manualSort column. - columns.insert(0, column.MANUAL_SORT_COL_INFO.copy()) + if manual_sort: + columns.insert(0, column.MANUAL_SORT_COL_INFO.copy()) - # First the tables is created without a primary view assigned as no view for it exists. - result = self.doAddTable(table_id, columns) - # Then its Primary View is created. - primary_view = self.doAddView(result["table_id"], 'raw_data', result["table_id"]) - result["views"] = [primary_view] - - raw_view_section = self._create_plain_view_section( - result["id"], - result["table_id"], - self._docmodel.view_sections, - "record", - ) - - self.UpdateRecord('_grist_Tables', result["id"], { - 'primaryViewId': primary_view["id"], - 'rawViewSectionRef': raw_view_section.id, - }) - - return result - - def doAddTable(self, table_id, columns, summarySourceTableRef=0): - """ - Add the given table with columns without creating views. - """ # If needed, transform table_id into a valid identifier, and add a suffix to make it unique. + table_title = table_id table_id = identifiers.pick_table_ident(table_id, avoid=six.viewkeys(self._engine.tables)) - + if not table_title: + table_title = table_id # Sanitize and de-duplicate column identifiers. col_ids = [c['id'] for c in columns] col_ids = identifiers.pick_col_ident_list(col_ids, avoid={'id'}) @@ -1660,12 +1694,36 @@ class UserActions(object): label = [c.get('label', col_id) for (c, col_id) in zip(columns, col_ids)], widgetOptions = [c.get('widgetOptions', '') for c in columns]) - return { + result = { "id": table_rec.id, "table_id": table_id, "columns": col_ids[1:], # All the column ids, except the auto-added manualSort. } + if primary_view: + # Create a primary view + primary_view = self.doAddView(result["table_id"], 'raw_data', table_title) + result["views"] = [primary_view] + + if raw_section: + # Create raw view section + raw_section = self._create_plain_view_section( + result["id"], + table_id, + self._docmodel.view_sections, + "record", + table_title + ) + + if primary_view or raw_section: + self.UpdateRecord('_grist_Tables', result["id"], { + 'primaryViewId': primary_view["id"] if primary_view else 0, + 'rawViewSectionRef': raw_section.id if raw_section else 0, + }) + + return result + + @useraction def RemoveTable(self, table_id): # We can remove a table via either a "RemoveTable" useraction or by removing a table @@ -1694,7 +1752,7 @@ class UserActions(object): return cols @useraction - def CreateViewSection(self, table_ref, view_ref, section_type, groupby_colrefs): + def CreateViewSection(self, table_ref, view_ref, section_type, groupby_colrefs, table_id): """ Create a new view section. If table_ref is 0, also creates a new empty table. If view_ref is 0, also creates a new view that will contain the new section. If groupby_colrefs is None, @@ -1705,7 +1763,7 @@ class UserActions(object): groupby_cols = self._fetch_table_col_recs(table_ref, groupby_colrefs) if not table_ref: - table_ref = self.AddEmptyTable()['id'] + table_ref = self.AddRawTable(table_id)['id'] table = self._docmodel.tables.table.get_record(table_ref) if not view_ref: @@ -1720,6 +1778,7 @@ class UserActions(object): table.tableId, view.viewSections, section_type, + '' ) return { 'tableRef': table_ref, @@ -1727,9 +1786,12 @@ class UserActions(object): 'sectionRef': section.id } - def _create_plain_view_section(self, tableRef, tableId, view_sections, section_type): + def _create_plain_view_section(self, tableRef, tableId, view_sections, section_type, title): + # If title is the same as tableId leave it empty + if title == tableId: + title = '' section = self._docmodel.add(view_sections, tableRef=tableRef, parentKey=section_type, - borderWidth=1, defaultWidth=100)[0] + title=title, borderWidth=1, defaultWidth=100)[0] # TODO: We should address the automatic selection of fields for charts in a better way. self._RebuildViewFields(tableId, section.id, limit=(2 if section_type == 'chart' else None)) diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index a673c15d..5d8ffe14 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -838,13 +838,21 @@ export function getPageNames(): Promise { /** * Adds a new empty table using the 'Add New' menu. */ -export async function addNewTable() { +export async function addNewTable(name?: string) { await driver.findWait('.test-dp-add-new', 2000).click(); await driver.find('.test-dp-empty-table').click(); + if (name) { + const prompt = await driver.find(".test-modal-prompt"); + await prompt.doClear(); + await prompt.click(); + await driver.sendKeys(name); + } + await driver.find(".test-modal-confirm").click(); await waitForServer(); } 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. } @@ -868,7 +876,7 @@ export async function addNewPage( } // Add a new widget to the current page using the 'Add New' menu. -export async function addNewSection(typeRe: RegExp, tableRe: RegExp, options?: PageWidgetPickerOptions) { +export async function addNewSection(typeRe: RegExp|string, tableRe: RegExp|string, 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(); @@ -918,6 +926,19 @@ export async function selectWidget( // let's select right type and save await driver.findContent('.test-wselect-type', typeRe).doClick(); await driver.find('.test-wselect-addBtn').doClick(); + + // if we selected a new table, there will be a popup for a name + const prompts = await driver.findAll(".test-modal-prompt"); + const prompt = prompts[0]; + if (prompt) { + if (options.tableName) { + await prompt.doClear(); + await prompt.click(); + await driver.sendKeys(options.tableName); + } + await driver.find(".test-modal-confirm").click(); + } + await waitForServer(); } @@ -944,11 +965,36 @@ export async function renamePage(oldName: string|RegExp, newName: string) { /** * Removes a page from the page menu, checks if the page is actually removable. + * By default it will remove only page (handling prompt if necessary). */ -export async function removePage(name: string|RegExp) { +export async function removePage(name: string|RegExp, options: { + expectPrompt?: boolean, // default undefined + withData?: boolean // default only page, + tables?: string[], + cancel?: boolean, +} = { }) { await openPageMenu(name); assert.equal(await driver.find('.test-docpage-remove').matches('.disabled'), false); await driver.find('.test-docpage-remove').click(); + const popups = await driver.findAll(".test-removepage-popup"); + if (options.expectPrompt === true) { + assert.lengthOf(popups, 1); + } else if (options.expectPrompt === false) { + assert.lengthOf(popups, 0); + } + if (popups.length) { + const popup = popups.shift()!; + if (options.tables) { + const popupTables = await driver.findAll(".test-removepage-table", e => e.getText()); + assert.deepEqual(popupTables.sort(), options.tables.sort()); + } + await popup.find(`.test-removepage-option-${options.withData ? 'data': 'page'}`).click(); + if (options.cancel) { + await driver.find(".test-modal-cancel").click(); + } else { + await driver.find(".test-modal-confirm").click(); + } + } await waitForServer(); }