(core) Show count of formula errors in the column config in the right-side panel.

Summary:
- Cache the count by column, factoring out ColumnCache from
  ColumnACIndexes, which uses a similar pattern.
- Update error counts in response to column selection and to data changes.

Test Plan: Adds a test case for the new message

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D2780
This commit is contained in:
Dmitry S 2021-04-20 17:57:45 -04:00
parent 5479159960
commit 65a722501d
4 changed files with 123 additions and 32 deletions

View File

@ -9,11 +9,10 @@
* It is currently used for auto-complete in the ReferenceEditor widget.
*/
import {ACIndex, ACIndexImpl} from 'app/client/lib/ACIndex';
import {ColumnCache} from 'app/client/models/ColumnCache';
import {UserError} from 'app/client/models/errors';
import {TableData} from 'app/client/models/TableData';
import {DocAction} from 'app/common/DocActions';
import {isBulkUpdateRecord, isUpdateRecord} from 'app/common/DocActions';
import {getSetMapValue, localeCompare, nativeCompare} from 'app/common/gutil';
import {localeCompare, nativeCompare} from 'app/common/gutil';
import {BaseFormatter} from 'app/common/ValueFormatter';
export interface ICellItem {
@ -24,13 +23,9 @@ export interface ICellItem {
export class ColumnACIndexes {
private _cachedColIndexes = new Map<string, ACIndex<ICellItem>>();
private _columnCache = new ColumnCache<ACIndex<ICellItem>>(this._tableData);
constructor(private _tableData: TableData) {
// Whenever a table action is applied, consider invalidating per-column caches.
this._tableData.tableActionEmitter.addListener(this._invalidateCache, this);
this._tableData.dataLoadedEmitter.addListener(this._clearCache, this);
}
constructor(private _tableData: TableData) {}
/**
* Returns the column index for the given column, using a cached one if available.
@ -38,7 +33,7 @@ export class ColumnACIndexes {
* getColACIndex() is called for the same column with the the same formatter.
*/
public getColACIndex(colId: string, formatter: BaseFormatter): ACIndex<ICellItem> {
return getSetMapValue(this._cachedColIndexes, colId, () => this._buildColACIndex(colId, formatter));
return this._columnCache.getValue(colId, () => this._buildColACIndex(colId, formatter));
}
private _buildColACIndex(colId: string, formatter: BaseFormatter): ACIndex<ICellItem> {
@ -56,23 +51,6 @@ export class ColumnACIndexes {
items.sort(itemCompare);
return new ACIndexImpl(items);
}
private _invalidateCache(action: DocAction): void {
if (isUpdateRecord(action) || isBulkUpdateRecord(action)) {
// If the update only affects existing records, only invalidate affected columns.
const colValues = action[3];
for (const colId of Object.keys(colValues)) {
this._cachedColIndexes.delete(colId);
}
} else {
// For add/delete actions and all schema changes, drop the cache entirelly to be on the safe side.
this._clearCache();
}
}
private _clearCache(): void {
this._cachedColIndexes.clear();
}
}
function itemCompare(a: ICellItem, b: ICellItem) {

View File

@ -0,0 +1,42 @@
/**
* Implements a cache of values computed from the data in a Grist column.
*/
import {TableData} from 'app/client/models/TableData';
import {DocAction} from 'app/common/DocActions';
import {isBulkUpdateRecord, isUpdateRecord} from 'app/common/DocActions';
import {getSetMapValue} from 'app/common/gutil';
export class ColumnCache<T> {
private _cachedColIndexes = new Map<string, T>();
constructor(private _tableData: TableData) {
// Whenever a table action is applied, consider invalidating per-column caches.
this._tableData.tableActionEmitter.addListener(this._invalidateCache, this);
this._tableData.dataLoadedEmitter.addListener(this._clearCache, this);
}
/**
* Returns the cached value for the given column, or calculates and caches the value using the
* provided calc() function.
*/
public getValue(colId: string, calc: () => T): T {
return getSetMapValue(this._cachedColIndexes, colId, calc);
}
private _invalidateCache(action: DocAction): void {
if (isUpdateRecord(action) || isBulkUpdateRecord(action)) {
// If the update only affects existing records, only invalidate affected columns.
const colValues = action[3];
for (const colId of Object.keys(colValues)) {
this._cachedColIndexes.delete(colId);
}
} else {
// For add/delete actions and all schema changes, drop the cache entirely to be on the safe side.
this._clearCache();
}
}
private _clearCache(): void {
this._cachedColIndexes.clear();
}
}

View File

@ -2,8 +2,11 @@
* TableData maintains a single table's data.
*/
import {ColumnACIndexes} from 'app/client/models/ColumnACIndexes';
import {ColumnCache} from 'app/client/models/ColumnCache';
import {DocData} from 'app/client/models/DocData';
import {DocAction, ReplaceTableData, TableDataAction, UserAction} from 'app/common/DocActions';
import {isRaisedException} from 'app/common/gristTypes';
import {countIf} from 'app/common/gutil';
import {ColTypeMap, TableData as BaseTableData} from 'app/common/TableData';
import {BaseFormatter} from 'app/common/ValueFormatter';
import {Emitter} from 'grainjs';
@ -19,6 +22,8 @@ export class TableData extends BaseTableData {
public readonly columnACIndexes = new ColumnACIndexes(this);
private _columnErrorCounts = new ColumnCache<number|undefined>(this);
/**
* Constructor for TableData.
* @param {DocData} docData: The root DocData object for this document.
@ -92,6 +97,17 @@ export class TableData extends BaseTableData {
return ret;
}
/**
* Counts and returns the number of error values in the given column. The count is cached to
* keep it faster for large tables, and the cache is cleared as needed on changes to the table.
*/
public countErrors(colId: string): number|undefined {
return this._columnErrorCounts.getValue(colId, () => {
const values = this.getColValues(colId);
return values && countIf(values, isRaisedException);
});
}
/**
* Sends an array of table-specific action to the server to be applied. The tableId should be
* omitted from each `action` parameter and will be inserted automatically.

View File

@ -7,9 +7,10 @@ import {textInput} from 'app/client/ui2018/editableLabel';
import {cssIconButton, icon} from 'app/client/ui2018/icons';
import {menu, menuItem} from 'app/client/ui2018/menus';
import {sanitizeIdent} from 'app/common/gutil';
import {Computed, dom, fromKo, IDisposableOwner, Observable, styled} from 'grainjs';
import {Computed, dom, fromKo, MultiHolder, Observable, styled, subscribe} from 'grainjs';
import debounce = require('lodash/debounce');
export function buildNameConfig(owner: IDisposableOwner, origColumn: ColumnRec) {
export function buildNameConfig(owner: MultiHolder, origColumn: ColumnRec) {
const untieColId = origColumn.untieColIdFromLabel;
const editedLabel = Observable.create(owner, '');
@ -50,10 +51,11 @@ export function buildNameConfig(owner: IDisposableOwner, origColumn: ColumnRec)
type BuildEditor = (cellElem: Element) => void;
export function buildFormulaConfig(
owner: IDisposableOwner, origColumn: ColumnRec, gristDoc: GristDoc, buildEditor: BuildEditor
owner: MultiHolder, origColumn: ColumnRec, gristDoc: GristDoc, buildEditor: BuildEditor
) {
const clearColumn = () => gristDoc.clearColumns([origColumn.id.peek()]);
const convertToData = () => gristDoc.convertFormulasToData([origColumn.id.peek()]);
const errorMessage = createFormulaErrorObs(owner, gristDoc, origColumn);
return dom.maybe(use => {
if (!use(origColumn.id)) { return null; } // Invalid column, show nothing.
@ -73,7 +75,10 @@ export function buildFormulaConfig(
);
}
function buildFormulaRow(placeholder = 'Enter formula') {
return cssRow(dom.create(buildFormula, origColumn, buildEditor, placeholder));
return [
cssRow(dom.create(buildFormula, origColumn, buildEditor, placeholder)),
dom.maybe(errorMessage, errMsg => cssRow(cssError(errMsg), testId('field-error-count'))),
];
}
if (type === "empty") {
return [
@ -104,7 +109,7 @@ export function buildFormulaConfig(
);
}
function buildFormula(owner: IDisposableOwner, column: ColumnRec, buildEditor: BuildEditor, placeholder: string) {
function buildFormula(owner: MultiHolder, column: ColumnRec, buildEditor: BuildEditor, placeholder: string) {
return cssFieldFormula(column.formula, {placeholder, maxLines: 2},
dom.cls('formula_field_sidepane'),
cssFieldFormula.cls('-disabled', column.disableModify),
@ -115,6 +120,52 @@ function buildFormula(owner: IDisposableOwner, column: ColumnRec, buildEditor: B
);
}
/**
* Create and return an observable for the count of errors in a column, which gets updated in
* response to changes in origColumn and in user data.
*/
function createFormulaErrorObs(owner: MultiHolder, gristDoc: GristDoc, origColumn: ColumnRec) {
const errorMessage = Observable.create(owner, '');
// Count errors in origColumn when it's a formula column. Counts get cached by the
// tableData.countErrors() method, and invalidated on relevant data changes.
function countErrors() {
if (owner.isDisposed()) { return; }
const tableData = gristDoc.docData.getTable(origColumn.table.peek().tableId.peek());
if (tableData && origColumn.isRealFormula.peek()) {
const colId = origColumn.colId.peek();
const numCells = tableData.getColValues(colId)?.length || 0;
const numErrors = tableData.countErrors(colId) || 0;
errorMessage.set(
(numErrors === 0) ? '' :
(numCells === 1) ? `Error in the cell` :
(numErrors === numCells) ? `Errors in all ${numErrors} cells` :
`Errors in ${numErrors} of ${numCells} cells`
);
} else {
errorMessage.set('');
}
}
// Debounce the count calculation to defer it to the end of a bundle of actions.
const debouncedCountErrors = debounce(countErrors, 0);
// If there is an update to the data in the table, count errors again. Since the same UI is
// reused when different page widgets are selected, we need to re-create this subscription
// whenever the selected table changes. We use a Computed to both react to changes and dispose
// the previous subscription when it changes.
Computed.create(owner, (use) => {
const tableData = gristDoc.docData.getTable(use(use(origColumn.table).tableId));
return tableData ? use.owner.autoDispose(tableData.tableActionEmitter.addListener(debouncedCountErrors)) : null;
});
// The counts depend on the origColumn and its isRealFormula status, but with the debounced
// callback and subscription to data, subscribe to relevant changes manually (rather than using
// a Computed).
owner.autoDispose(subscribe(use => { use(origColumn.id); use(origColumn.isRealFormula); debouncedCountErrors(); }));
return errorMessage;
}
const cssFieldFormula = styled(buildHighlightedCode, `
flex: auto;
cursor: pointer;
@ -195,3 +246,7 @@ const cssColTieConnectors = styled('div', `
border-left: none;
z-index: -1;
`);
const cssError = styled('div', `
color: ${colors.error};
`);