(core) Fix imports into reference columns, and support two ways to import Numeric as a reference.

Summary:
- When importing into a Ref column, use lookupOne() formula for correct previews.
- When selecting columns to import into a Ref column, now a Numeric column like
  'Order' will produce two options: "Order" and "Order (as row ID)".
- Fixes exports to correct the formatting of visible columns. This addresses multiple bugs:
  1. Formatting wasn't used, e.g. a Ref showing a custom-formatted date was still presented as YYYY-MM-DD in CSVs.
  2. Ref showing a Numeric column was formatted as if a row ID (e.g. `Table1[1.5]`), which is very wrong.
- If importing into a table that doesn't have a primary view, don't switch page after import.

Refactorings:
- Generalize GenImporterView to be usable in more cases; removed near-duplicated logic from node side
- Some other refactoring in importing code.
- Fix field/column option selection in ValueParser
- Add NUM() helper to turn integer-valued floats into ints, useful for "as row ID" lookups.

Test Plan: Added test cases for imports into reference columns, updated Exports test fixtures.

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3875
This commit is contained in:
Dmitry S
2023-04-25 17:11:25 -04:00
parent 7a12a8ef28
commit 65013331a3
17 changed files with 290 additions and 339 deletions

View File

@@ -5,11 +5,11 @@ import * as _ from 'underscore';
import {ColumnDelta, createEmptyActionSummary} from 'app/common/ActionSummary';
import {ApplyUAResult, DataSourceTransformed, ImportOptions, ImportResult, ImportTableResult,
MergeOptions, MergeOptionsMap, MergeStrategy, SKIP_TABLE, TransformColumn,
MergeOptions, MergeOptionsMap, MergeStrategy, SKIP_TABLE,
TransformRule,
TransformRuleMap} from 'app/common/ActiveDocAPI';
import {ApiError} from 'app/common/ApiError';
import {BulkColValues, CellValue, fromTableDataAction, TableRecordValue, UserAction} from 'app/common/DocActions';
import {BulkColValues, CellValue, fromTableDataAction, UserAction} from 'app/common/DocActions';
import * as gutil from 'app/common/gutil';
import {DocStateComparison} from 'app/common/UserAPI';
import {guessColInfoForImports} from 'app/common/ValueGuesser';
@@ -339,9 +339,9 @@ export class ActiveDocImport {
if (isHidden) {
// Generate formula columns, view sections, etc
const results: ApplyUAResult = await this._activeDoc.applyUserActions(docSession,
[['GenImporterView', hiddenTableId, destTableId, ruleCanBeApplied ? transformRule : null]]);
[['GenImporterView', hiddenTableId, destTableId, ruleCanBeApplied ? transformRule : null, null]]);
transformSectionRef = results.retValues[0];
transformSectionRef = results.retValues[0].viewSectionRef;
createdTableId = hiddenTableId;
} else {
@@ -391,36 +391,21 @@ export class ActiveDocImport {
* the source and destination table.
* @returns {string} The table id of the new or updated destination table.
*/
private async _transformAndFinishImport(docSession: OptDocSession,
hiddenTableId: string, destTableId: string,
intoNewTable: boolean, transformRule: TransformRule|null,
mergeOptions: MergeOptions|null): Promise<string> {
private async _transformAndFinishImport(
docSession: OptDocSession,
hiddenTableId: string, destTableId: string,
intoNewTable: boolean, transformRule: TransformRule|null,
mergeOptions: MergeOptions|null
): Promise<string> {
log.info("ActiveDocImport._transformAndFinishImport(%s, %s, %s, %s, %s)",
hiddenTableId, destTableId, intoNewTable, transformRule, mergeOptions);
const srcCols = await this._activeDoc.getTableCols(docSession, hiddenTableId);
// Use a default transform rule if one was not provided by the client.
if (!transformRule) {
const transformDest = intoNewTable ? null : destTableId;
transformRule = await this._makeDefaultTransformRule(docSession, srcCols, transformDest);
}
// Transform rules from client may have prefixed column ids, so we need to strip them.
stripRulePrefixes(transformRule);
if (intoNewTable) {
// Transform rules for new tables don't have filled in destination column ids.
const result = await this._activeDoc.applyUserActions(docSession, [['FillTransformRuleColIds', transformRule]]);
transformRule = result.retValues[0] as TransformRule;
// Encode Refs as Ints, to avoid table dependency issues. We'll convert back to Ref at the end.
encodeRuleReferences(transformRule);
} else if (transformRule.destCols.some(c => c.colId === null)) {
throw new Error('Column ids in transform rule must be filled when importing into an existing table');
}
await this._activeDoc.applyUserActions(docSession,
[['MakeImportTransformColumns', hiddenTableId, transformRule, false]]);
const transformDestTableId = intoNewTable ? null : destTableId;
const result = await this._activeDoc.applyUserActions(docSession, [[
'GenImporterView', hiddenTableId, transformDestTableId, transformRule,
{createViewSection: false, genAll: false, refsAsInts: true},
]]);
transformRule = result.retValues[0].transformRule as TransformRule;
if (!intoNewTable && mergeOptions && mergeOptions.mergeCols.length > 0) {
await this._mergeAndFinishImport(docSession, hiddenTableId, destTableId, transformRule, mergeOptions);
@@ -430,6 +415,7 @@ export class ActiveDocImport {
const hiddenTableData = fromTableDataAction(await this._activeDoc.fetchTable(docSession, hiddenTableId, true));
const columnData: BulkColValues = {};
const srcCols = await this._activeDoc.getTableCols(docSession, hiddenTableId);
const srcColIds = srcCols.map(c => c.id as string);
// Only include destination columns that weren't skipped.
@@ -591,38 +577,6 @@ export class ActiveDocImport {
return this._activeDoc.docStorage.decodeMarshalledDataFromTables(result);
}
/**
* Returns a default TransformRule using column definitions from `destTableId`. If `destTableId`
* is null (in the case when the import destination is a new table), the `srcCols` are used instead.
*
* @param {TableRecordValue[]} srcCols Source column definitions.
* @param {string|null} destTableId The destination table id. If null, the destination is assumed
* to be a new table, and `srcCols` are used to build the transform rule.
* @returns {Promise<TransformRule>} The constructed transform rule.
*/
private async _makeDefaultTransformRule(docSession: OptDocSession, srcCols: TableRecordValue[],
destTableId: string|null): Promise<TransformRule> {
const targetCols = destTableId ? await this._activeDoc.getTableCols(docSession, destTableId) : srcCols;
const destCols: TransformColumn[] = [];
const srcColIds = srcCols.map(c => c.id as string);
for (const {id, fields} of targetCols) {
destCols.push({
colId: destTableId ? id as string : null,
label: fields.label as string,
type: fields.type as string,
widgetOptions: fields.widgetOptions as string,
formula: srcColIds.includes(id as string) ? `$${id}` : ''
});
}
return {
destTableId,
destCols,
sourceCols: srcColIds
};
}
/**
* This function removes temporary hidden tables which were created during the import process
*
@@ -694,32 +648,6 @@ function isBlank(value: CellValue): boolean {
return value === null || (typeof value === 'string' && value.trim().length === 0);
}
/**
* Changes every Ref column to an Int column in `destCols`.
*
* Encoding references as ints can be useful when finishing imports to avoid
* issues such as importing linked tables in the wrong order. When encoding references,
* ActiveDocImport._fixReferences should be called at the end of importing to
* decode Ints back to Refs.
*/
function encodeRuleReferences({destCols}: TransformRule): void {
for (const col of destCols) {
const refTableId = gutil.removePrefix(col.type, "Ref:");
if (refTableId) {
col.type = 'Int';
}
}
}
// Helper function that strips import prefixes from columns in transform rules (if ids are present).
function stripRulePrefixes({destCols}: TransformRule): void {
for (const col of destCols) {
const colId = col.colId;
if (colId && colId.startsWith(IMPORT_TRANSFORM_COLUMN_PREFIX)) {
col.colId = colId.slice(IMPORT_TRANSFORM_COLUMN_PREFIX.length);
}
}
}
// Helper function that returns new `colIds` with import prefixes stripped.
function stripPrefixes(colIds: string[]): string[] {

View File

@@ -1,6 +1,7 @@
import {ApiError} from 'app/common/ApiError';
import {buildColFilter} from 'app/common/ColumnFilterFunc';
import {TableDataAction} from 'app/common/DocActions';
import {DocData} from 'app/common/DocData';
import {DocumentSettings} from 'app/common/DocumentSettings';
import * as gristTypes from 'app/common/gristTypes';
import * as gutil from 'app/common/gutil';
@@ -11,6 +12,7 @@ import {schema, SchemaTypes} from 'app/common/schema';
import {SortFunc} from 'app/common/SortFunc';
import {Sort} from 'app/common/SortSpec';
import {MetaRowRecord, MetaTableData} from 'app/common/TableData';
import {BaseFormatter, createFullFormatterFromDocData} from 'app/common/ValueFormatter';
import {ActiveDoc} from 'app/server/lib/ActiveDoc';
import {RequestWithLogin} from 'app/server/lib/Authorizer';
import {docSessionFromRequest} from 'app/server/lib/DocSession';
@@ -23,23 +25,16 @@ import * as _ from 'underscore';
type Access = (row: number) => any;
// Helper interface with information about the column
interface ExportColumn {
export interface ExportColumn {
id: number;
colId: string;
label: string;
type: string;
widgetOptions: any;
formatter: BaseFormatter;
parentPos: number;
description: string;
}
// helper for empty column
const emptyCol: ExportColumn = {
id: 0,
colId: '',
label: '',
type: '',
widgetOptions: null,
parentPos: 0
};
/**
* Bare data that is exported - used to convert to various formats.
*/
@@ -173,41 +168,33 @@ export async function exportTable(
{metaTables}: {metaTables?: FilteredMetaTables} = {},
): Promise<ExportData> {
metaTables = metaTables || await getMetaTables(activeDoc, req);
const docData = new DocData((tableId) => { throw new Error("Unexpected DocData fetch"); }, metaTables);
const tables = safeTable(metaTables, '_grist_Tables');
const metaColumns = safeTable(metaTables, '_grist_Tables_column');
checkTableAccess(tables, tableRef);
const table = safeRecord(tables, tableRef);
const tableColumns = safeTable(metaTables, '_grist_Tables_column')
.getRecords()
// Select only columns that belong to this table.
const tableColumns = metaColumns.filterRecords({parentId: tableRef})
// sort by parentPos and id, which should be the same order as in raw data
.sort((c1, c2) => nativeCompare(c1.parentPos, c2.parentPos) || nativeCompare(c1.id, c2.id))
// remove manual sort column
.filter(col => col.colId !== gristTypes.MANUALSORT);
.sort((c1, c2) => nativeCompare(c1.parentPos, c2.parentPos) || nativeCompare(c1.id, c2.id));
// Produce a column description matching what user will see / expect to export
const tableColsById = _.indexBy(tableColumns, 'id');
const columns = tableColumns.map(tc => {
// remove all columns that don't belong to this table
if (tc.parentId !== tableRef) {
return emptyCol;
}
// remove all helpers
if (gristTypes.isHiddenCol(tc.colId)) {
return emptyCol;
}
const columns: ExportColumn[] = tableColumns
.filter(tc => !gristTypes.isHiddenCol(tc.colId)) // Exclude helpers
.map<ExportColumn>(tc => {
// for reference columns, return display column, and copy settings from visible column
const displayCol = tableColsById[tc.displayCol || tc.id];
const colOptions = gutil.safeJsonParse(tc.widgetOptions, {});
const displayOptions = gutil.safeJsonParse(displayCol.widgetOptions, {});
const widgetOptions = Object.assign(displayOptions, colOptions);
const displayCol = metaColumns.getRecord(tc.displayCol) || tc;
return {
id: displayCol.id,
colId: displayCol.colId,
label: tc.label,
type: displayCol.type,
widgetOptions,
type: tc.type,
formatter: createFullFormatterFromDocData(docData, tc.id),
parentPos: tc.parentPos,
description: displayCol.description,
description: tc.description,
};
}).filter(tc => tc !== emptyCol);
});
// fetch actual data
const {tableData} = await activeDoc.fetchTable(docSessionFromRequest(req as RequestWithLogin), table.tableId, true);
@@ -251,37 +238,35 @@ export async function exportSection(
{metaTables}: {metaTables?: FilteredMetaTables} = {},
): Promise<ExportData> {
metaTables = metaTables || await getMetaTables(activeDoc, req);
const docData = new DocData((tableId) => { throw new Error("Unexpected DocData fetch"); }, metaTables);
const viewSections = safeTable(metaTables, '_grist_Views_section');
const viewSection = safeRecord(viewSections, viewSectionId);
safe(viewSection.tableRef, `Cannot find or access table`);
const tables = safeTable(metaTables, '_grist_Tables');
checkTableAccess(tables, viewSection.tableRef);
const table = safeRecord(tables, viewSection.tableRef);
const columns = safeTable(metaTables, '_grist_Tables_column')
.filterRecords({parentId: table.id});
const metaColumns = safeTable(metaTables, '_grist_Tables_column');
const columns = metaColumns.filterRecords({parentId: table.id});
const viewSectionFields = safeTable(metaTables, '_grist_Views_section_field');
const fields = viewSectionFields.filterRecords({parentId: viewSection.id});
const savedFilters = safeTable(metaTables, '_grist_Filters')
.filterRecords({viewSectionRef: viewSection.id});
const tableColsById = _.indexBy(columns, 'id');
const fieldsByColRef = _.indexBy(fields, 'colRef');
const savedFiltersByColRef = _.indexBy(savedFilters, 'colRef');
const unsavedFiltersByColRef = _.indexBy(filters ?? [], 'colRef');
// Produce a column description matching what user will see / expect to export
const viewify = (col: GristTablesColumn, field?: GristViewsSectionField) => {
const displayCol = tableColsById[field?.displayCol || col.displayCol || col.id];
const colWidgetOptions = gutil.safeJsonParse(col.widgetOptions, {});
const fieldWidgetOptions = field ? gutil.safeJsonParse(field.widgetOptions, {}) : {};
const viewify = (col: GristTablesColumn, field?: GristViewsSectionField): ExportColumn => {
const displayCol = metaColumns.getRecord(field?.displayCol || col.displayCol) || col;
return {
id: displayCol.id,
colId: displayCol.colId,
label: col.label,
type: col.type,
formatter: createFullFormatterFromDocData(docData, col.id, field?.id),
parentPos: col.parentPos,
description: col.description,
widgetOptions: Object.assign(colWidgetOptions, fieldWidgetOptions),
};
};
const buildFilters = (col: GristTablesColumn, field?: GristViewsSectionField) => {
@@ -297,14 +282,14 @@ export async function exportSection(
const columnsForFilters = columns
.filter(column => !gristTypes.isHiddenCol(column.colId))
.map(column => buildFilters(column, fieldsByColRef[column.id]));
const viewColumns = _.sortBy(fields, 'parentPos')
.map((field) => viewify(tableColsById[field.colRef], field));
const viewColumns: ExportColumn[] = _.sortBy(fields, 'parentPos')
.map((field) => viewify(metaColumns.getRecord(field.colRef)!, field));
// The columns named in sort order need to now become display columns
sortSpec = sortSpec || gutil.safeJsonParse(viewSection.sortColRefs, []);
sortSpec = sortSpec!.map((colSpec) => {
const colRef = Sort.getColRef(colSpec);
const col = tableColsById[colRef];
const col = metaColumns.getRecord(colRef);
if (!col) {
return 0;
}

View File

@@ -1,5 +1,4 @@
import {ApiError} from 'app/common/ApiError';
import {createFormatter} from 'app/common/ValueFormatter';
import {ActiveDoc} from 'app/server/lib/ActiveDoc';
import {DownloadOptions, ExportData, exportSection, exportTable, Filter} from 'app/server/lib/Export';
import log from 'app/server/lib/log';
@@ -86,7 +85,7 @@ function convertToCsv({
}: ExportData) {
// create formatters for columns
const formatters = viewColumns.map(col => createFormatter(col.type, col.widgetOptions, docSettings));
const formatters = viewColumns.map(col => col.formatter);
// Arrange the data into a row-indexed matrix, starting with column headers.
const csvMatrix = [viewColumns.map(col => col.label)];
// populate all the rows with values as strings

View File

@@ -1,18 +1,7 @@
import * as express from 'express';
import {ApiError} from 'app/common/ApiError';
import {WidgetOptions} from 'app/common/WidgetOptions';
import {ActiveDoc} from 'app/server/lib/ActiveDoc';
import {DownloadOptions, exportTable} from 'app/server/lib/Export';
interface ExportColumn {
id: number;
colId: string;
label: string;
type: string;
widgetOptions: WidgetOptions;
description?: string;
parentPos: number;
}
import {DownloadOptions, ExportColumn, exportTable} from 'app/server/lib/Export';
interface FrictionlessFormat {
name: string;
@@ -86,35 +75,36 @@ function columnsToTableSchema(
function buildTypeField(col: ExportColumn, locale: string) {
const type = col.type.split(':', 1)[0];
const widgetOptions = col.formatter.widgetOpts;
switch (type) {
case 'Text':
return {
type: 'string',
format: col.widgetOptions.widget === 'HyperLink' ? 'uri' : 'default',
format: widgetOptions.widget === 'HyperLink' ? 'uri' : 'default',
};
case 'Numeric':
return {
type: 'number',
bareNumber: col.widgetOptions?.numMode === 'decimal',
bareNumber: widgetOptions?.numMode === 'decimal',
...getNumberSeparators(locale),
};
case 'Integer':
return {
type: 'integer',
bareNumber: col.widgetOptions?.numMode === 'decimal',
bareNumber: widgetOptions?.numMode === 'decimal',
groupChar: getNumberSeparators(locale).groupChar,
};
case 'Date':
return {
type: 'date',
format: 'any',
gristFormat: col.widgetOptions?.dateFormat || 'YYYY-MM-DD',
gristFormat: widgetOptions?.dateFormat || 'YYYY-MM-DD',
};
case 'DateTime':
return {
type: 'datetime',
format: 'any',
gristFormat: `${col.widgetOptions?.dateFormat} ${col.widgetOptions?.timeFormat}`,
gristFormat: `${widgetOptions?.dateFormat} ${widgetOptions?.timeFormat}`,
};
case 'Bool':
return {
@@ -125,12 +115,12 @@ function buildTypeField(col: ExportColumn, locale: string) {
case 'Choice':
return {
type: 'string',
constraints: {enum: col.widgetOptions?.choices},
constraints: {enum: widgetOptions?.choices},
};
case 'ChoiceList':
return {
type: 'array',
constraints: {enum: col.widgetOptions?.choices},
constraints: {enum: widgetOptions?.choices},
};
case 'Reference':
return {type: 'string'};
@@ -148,4 +138,4 @@ function getNumberSeparators(locale: string) {
groupChar: parts.find(obj => obj.type === 'group')?.value,
decimalChar: parts.find(obj => obj.type === 'decimal')?.value,
};
}
}

View File

@@ -130,7 +130,7 @@ async function convertToExcel(tables: ExportData[], testDates: boolean) {
const { columns, rowIds, access, tableName } = table;
const ws = wb.addWorksheet(sanitizeWorksheetName(tableName));
// Build excel formatters.
const formatters = columns.map(col => createExcelFormatter(col.type, col.widgetOptions));
const formatters = columns.map(col => createExcelFormatter(col.formatter.type, col.formatter.widgetOpts));
// Generate headers for all columns with correct styles for whole column.
// Actual header style for a first row will be overwritten later.
ws.columns = columns.map((col, c) => ({ header: col.label, style: formatters[c].style() }));