From f66ecbd6df910b77925f8f780fb64a67661afce0 Mon Sep 17 00:00:00 2001 From: CamilleLegeron Date: Thu, 12 Oct 2023 19:32:22 +0200 Subject: [PATCH] feat: allow using the existing numeric table IDs in the API (#690) --- app/server/lib/ActiveDoc.ts | 29 ++- app/server/lib/DocApi.ts | 52 +++--- test/server/lib/DocApi.ts | 359 +++++++++++++++++++----------------- 3 files changed, 248 insertions(+), 192 deletions(-) diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 0413f2ea..96bc80ea 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -85,7 +85,7 @@ import {AccessTokenOptions, AccessTokenResult, GristDocAPI, UIRowId} from 'app/p import {compileAclFormula} from 'app/server/lib/ACLFormula'; import {AssistanceSchemaPromptV1Context} from 'app/server/lib/Assistance'; import {AssistanceContext} from 'app/common/AssistancePrompts'; -import {Authorizer} from 'app/server/lib/Authorizer'; +import {Authorizer, RequestWithLogin} from 'app/server/lib/Authorizer'; import {checksumFile} from 'app/server/lib/checksumFile'; import {Client} from 'app/server/lib/Client'; import {DEFAULT_CACHE_TTL, DocManager} from 'app/server/lib/DocManager'; @@ -141,6 +141,7 @@ import remove = require('lodash/remove'); import sum = require('lodash/sum'); import without = require('lodash/without'); import zipObject = require('lodash/zipObject'); +import { getMetaTables } from './DocApi'; bluebird.promisifyAll(tmp); @@ -2791,7 +2792,6 @@ export function tableIdToRef(metaTables: { [p: string]: TableDataAction }, table // Helper that converts a Grist column colId to a ref given the corresponding table. export function colIdToRef(metaTables: {[p: string]: TableDataAction}, tableId: string, colId: string) { - const tableRef = tableIdToRef(metaTables, tableId); const [, , colRefs, columnData] = metaTables._grist_Tables_column; @@ -2804,6 +2804,31 @@ export function colIdToRef(metaTables: {[p: string]: TableDataAction}, tableId: return colRefs[colRowIndex]; } +// Helper that check if tableRef is used instead of tableId and return real tableId +// If metaTables is not define, activeDoc and req allow it to be created +interface MetaTables { + metaTables: { [p: string]: TableDataAction } +} +interface ActiveDocAndReq { + activeDoc: ActiveDoc, req: RequestWithLogin +} +export async function getRealTableId( + tableId: string, + options: MetaTables | ActiveDocAndReq + ): Promise { + if (parseInt(tableId)) { + const metaTables = "metaTables" in options + ? options.metaTables + : await getMetaTables(options.activeDoc, options.req); + const [, , tableRefs, tableData] = metaTables._grist_Tables; + if (tableRefs.indexOf(parseInt(tableId)) >= 0) { + const tableRowIndex = tableRefs.indexOf(parseInt(tableId)); + return tableData.tableId[tableRowIndex]!.toString(); + } + } + return tableId; +} + export function sanitizeApplyUAOptions(options?: ApplyUAOptions): ApplyUAOptions { return pick(options||{}, ['desc', 'otherId', 'linkId', 'parseStrings']); } diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index bcf93c7a..095cd307 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -32,7 +32,7 @@ import { TableOperationsImpl, TableOperationsPlatform } from 'app/plugin/TableOperationsImpl'; -import {ActiveDoc, colIdToRef as colIdToReference, tableIdToRef} from "app/server/lib/ActiveDoc"; +import {ActiveDoc, colIdToRef as colIdToReference, getRealTableId, tableIdToRef} from "app/server/lib/ActiveDoc"; import {appSettings} from "app/server/lib/AppSettings"; import {sendForCompletion} from 'app/server/lib/Assistance'; import { @@ -201,7 +201,7 @@ export class DocWorkerApi { if (!Object.keys(filters).every(col => Array.isArray(filters[col]))) { throw new ApiError("Invalid query: filter values must be arrays", 400); } - const tableId = optTableId || req.params.tableId; + const tableId = await getRealTableId(optTableId || req.params.tableId, {activeDoc, req}); const session = docSessionFromRequest(req); const {tableData} = await handleSandboxError(tableId, [], activeDoc.fetchQuery( session, {tableId, filters}, !immediate)); @@ -262,11 +262,6 @@ export class DocWorkerApi { }) ); - async function getMetaTables(activeDoc: ActiveDoc, req: RequestWithLogin) { - return await handleSandboxError("", [], - activeDoc.fetchMetaTables(docSessionFromRequest(req))); - } - const registerWebhook = async (activeDoc: ActiveDoc, req: RequestWithLogin, webhook: WebhookFields) => { const {fields, url} = await getWebhookSettings(activeDoc, req, null, webhook); if (!fields.eventTypes?.length) { @@ -337,7 +332,8 @@ export class DocWorkerApi { const trigger = webhookId ? activeDoc.triggers.getWebhookTriggerRecord(webhookId) : undefined; let currentTableId = trigger ? tablesTable.getValue(trigger.tableRef, 'tableId')! : undefined; const {url, eventTypes, isReadyColumn, name} = webhook; - const tableId = req.params.tableId || webhook.tableId; + const tableId = await getRealTableId(req.params.tableId || webhook.tableId, {metaTables}); + const fields: Partial = {}; if (url && !isUrlAllowed(url)) { @@ -387,7 +383,7 @@ export class DocWorkerApi { // Get the columns of the specified table in recordish format this._app.get('/api/docs/:docId/tables/:tableId/columns', canView, withDoc(async (activeDoc, req, res) => { - const tableId = req.params.tableId; + const tableId = await getRealTableId(req.params.tableId, {activeDoc, req}); const includeHidden = isAffirmative(req.query.hidden); const columns = await handleSandboxError('', [], activeDoc.getTableCols(docSessionFromRequest(req), tableId, includeHidden)); @@ -498,7 +494,7 @@ export class DocWorkerApi { withDoc(async (activeDoc, req, res) => { const colValues = req.body as BulkColValues; const count = colValues[Object.keys(colValues)[0]].length; - const op = getTableOperations(req, activeDoc); + const op = await getTableOperations(req, activeDoc); const ids = await op.addRecords(count, colValues); res.json(ids); }) @@ -527,7 +523,7 @@ export class DocWorkerApi { } } validateCore(RecordsPost, req, body); - const ops = getTableOperations(req, activeDoc); + const ops = await getTableOperations(req, activeDoc); const records = await ops.create(body.records); res.json({records}); }) @@ -558,7 +554,7 @@ export class DocWorkerApi { this._app.post('/api/docs/:docId/tables/:tableId/columns', canEdit, validate(ColumnsPost), withDoc(async (activeDoc, req, res) => { const body = req.body as Types.ColumnsPost; - const {tableId} = req.params; + const tableId = await getRealTableId(req.params.tableId, {activeDoc, req}); const actions = body.columns.map(({fields, id: colId}) => // AddVisibleColumn adds the column to all widgets of the table. // This isn't necessarily what the user wants, but it seems like a good default. @@ -590,7 +586,7 @@ export class DocWorkerApi { this._app.post('/api/docs/:docId/tables/:tableId/data/delete', canEdit, withDoc(async (activeDoc, req, res) => { const rowIds = req.body; - const op = getTableOperations(req, activeDoc); + const op = await getTableOperations(req, activeDoc); await op.destroy(rowIds); res.json(null); })); @@ -659,7 +655,7 @@ export class DocWorkerApi { const rowIds = columnValues.id; // sandbox expects no id column delete columnValues.id; - const ops = getTableOperations(req, activeDoc); + const ops = await getTableOperations(req, activeDoc); await ops.updateRecords(columnValues, rowIds); res.json(null); }) @@ -669,7 +665,7 @@ export class DocWorkerApi { this._app.patch('/api/docs/:docId/tables/:tableId/records', canEdit, validate(RecordsPatch), withDoc(async (activeDoc, req, res) => { const body = req.body as Types.RecordsPatch; - const ops = getTableOperations(req, activeDoc); + const ops = await getTableOperations(req, activeDoc); await ops.update(body.records); res.json(null); }) @@ -680,7 +676,7 @@ export class DocWorkerApi { withDoc(async (activeDoc, req, res) => { const tablesTable = activeDoc.docData!.getMetaTable("_grist_Tables"); const columnsTable = activeDoc.docData!.getMetaTable("_grist_Tables_column"); - const {tableId} = req.params; + const tableId = await getRealTableId(req.params.tableId, {activeDoc, req}); const tableRef = tablesTable.findMatchingRowId({tableId}); if (!tableRef) { throw new ApiError(`Table not found "${tableId}"`, 404); @@ -693,7 +689,7 @@ export class DocWorkerApi { } return {...col, id}; }); - const ops = getTableOperations(req, activeDoc, "_grist_Tables_column"); + const ops = await getTableOperations(req, activeDoc, "_grist_Tables_column"); await ops.update(columns); res.json(null); }) @@ -711,7 +707,7 @@ export class DocWorkerApi { } return {...table, id}; }); - const ops = getTableOperations(req, activeDoc, "_grist_Tables"); + const ops = await getTableOperations(req, activeDoc, "_grist_Tables"); await ops.update(tables); res.json(null); }) @@ -720,7 +716,7 @@ export class DocWorkerApi { // Add or update records given in records format this._app.put('/api/docs/:docId/tables/:tableId/records', canEdit, validate(RecordsPut), withDoc(async (activeDoc, req, res) => { - const ops = getTableOperations(req, activeDoc); + const ops = await getTableOperations(req, activeDoc); const body = req.body as Types.RecordsPut; const options = { add: !isAffirmative(req.query.noadd), @@ -740,7 +736,7 @@ export class DocWorkerApi { withDoc(async (activeDoc, req, res) => { const tablesTable = activeDoc.docData!.getMetaTable("_grist_Tables"); const columnsTable = activeDoc.docData!.getMetaTable("_grist_Tables_column"); - const {tableId} = req.params; + const tableId = await getRealTableId(req.params.tableId, {activeDoc, req}); const tableRef = tablesTable.findMatchingRowId({tableId}); if (!tableRef) { throw new ApiError(`Table not found "${tableId}"`, 404); @@ -785,7 +781,8 @@ export class DocWorkerApi { this._app.delete('/api/docs/:docId/tables/:tableId/columns/:colId', canEdit, withDoc(async (activeDoc, req, res) => { - const {tableId, colId} = req.params; + const {colId} = req.params; + const tableId = await getRealTableId(req.params.tableId, {activeDoc, req}); const actions = [ [ 'RemoveColumn', tableId, colId ] ]; await handleSandboxError(tableId, [colId], activeDoc.applyUserActions(docSessionFromRequest(req), actions) @@ -1941,12 +1938,21 @@ function getErrorPlatform(tableId: string): TableOperationsPlatform { }; } -function getTableOperations(req: RequestWithLogin, activeDoc: ActiveDoc, tableId?: string): TableOperationsImpl { +export async function getMetaTables(activeDoc: ActiveDoc, req: RequestWithLogin) { + return await handleSandboxError("", [], + activeDoc.fetchMetaTables(docSessionFromRequest(req))); +} + +async function getTableOperations( + req: RequestWithLogin, + activeDoc: ActiveDoc, + tableId?: string): Promise { const options: OpOptions = { parseStrings: !isAffirmative(req.query.noparse) }; + const realTableId = await getRealTableId(tableId ?? req.params.tableId, {activeDoc, req}); const platform: TableOperationsPlatform = { - ...getErrorPlatform(tableId ?? req.params.tableId), + ...getErrorPlatform(realTableId), applyUserActions(actions, opts) { if (!activeDoc) { throw new Error('no document'); } return activeDoc.applyUserActions( diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index 8c7750d2..dabb2fe0 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -547,9 +547,7 @@ function testDocApi() { } it("GET /docs/{did}/tables/{tid}/data retrieves data in column format", async function () { - const resp = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/Table1/data`, chimpy); - assert.equal(resp.status, 200); - assert.deepEqual(resp.data, { + const data = { id: [1, 2, 3, 4], A: ['hello', '', '', ''], B: ['', 'world', '', ''], @@ -557,68 +555,73 @@ function testDocApi() { D: [null, null, null, null], E: ['HELLO', '', '', ''], manualSort: [1, 2, 3, 4] - }); + }; + const respWithTableId = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/Table1/data`, chimpy); + assert.equal(respWithTableId.status, 200); + assert.deepEqual(respWithTableId.data, data); + const respWithTableRef = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/1/data`, chimpy); + assert.equal(respWithTableRef.status, 200); + assert.deepEqual(respWithTableRef.data, data); }); it("GET /docs/{did}/tables/{tid}/records retrieves data in records format", async function () { - const resp = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/Table1/records`, chimpy); - assert.equal(resp.status, 200); - assert.deepEqual(resp.data, - { - records: - [ - { - id: 1, - fields: { - A: 'hello', - B: '', - C: '', - D: null, - E: 'HELLO', - }, + const data = { + records: + [ + { + id: 1, + fields: { + A: 'hello', + B: '', + C: '', + D: null, + E: 'HELLO', }, - { - id: 2, - fields: { - A: '', - B: 'world', - C: '', - D: null, - E: '', - }, + }, + { + id: 2, + fields: { + A: '', + B: 'world', + C: '', + D: null, + E: '', }, - { - id: 3, - fields: { - A: '', - B: '', - C: '', - D: null, - E: '', - }, + }, + { + id: 3, + fields: { + A: '', + B: '', + C: '', + D: null, + E: '', }, - { - id: 4, - fields: { - A: '', - B: '', - C: '', - D: null, - E: '', - }, + }, + { + id: 4, + fields: { + A: '', + B: '', + C: '', + D: null, + E: '', }, - ] - }); + }, + ] + }; + const respWithTableId = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/Table1/records`, chimpy); + assert.equal(respWithTableId.status, 200); + assert.deepEqual(respWithTableId.data, data); + const respWithTableRef = await axios.get( + `${serverUrl}/api/docs/${docIds.Timesheets}/tables/1/records`, chimpy); + assert.equal(respWithTableRef.status, 200); + assert.deepEqual(respWithTableRef.data, data); }); it('GET /docs/{did}/tables/{tid}/records honors the "hidden" param', async function () { const params = { hidden: true }; - const resp = await axios.get( - `${serverUrl}/api/docs/${docIds.Timesheets}/tables/Table1/records`, - {...chimpy, params } - ); - assert.equal(resp.status, 200); - assert.deepEqual(resp.data.records[0], { + const data = { id: 1, fields: { manualSort: 1, @@ -628,7 +631,19 @@ function testDocApi() { D: null, E: 'HELLO', }, - }); + }; + const respWithTableId = await axios.get( + `${serverUrl}/api/docs/${docIds.Timesheets}/tables/Table1/records`, + {...chimpy, params } + ); + assert.equal(respWithTableId.status, 200); + assert.deepEqual(respWithTableId.data.records[0], data); + const respWithTableRef = await axios.get( + `${serverUrl}/api/docs/${docIds.Timesheets}/tables/1/records`, + {...chimpy, params } + ); + assert.equal(respWithTableRef.status, 200); + assert.deepEqual(respWithTableRef.data.records[0], data); }); it("GET /docs/{did}/tables/{tid}/records handles errors and hidden columns", async function () { @@ -683,119 +698,121 @@ function testDocApi() { }); it("GET /docs/{did}/tables/{tid}/columns retrieves columns", async function () { - const resp = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/Table1/columns`, chimpy); - assert.equal(resp.status, 200); - assert.deepEqual(resp.data, - { - columns: [ - { - id: 'A', - fields: { - colRef: 2, - parentId: 1, - parentPos: 1, - type: 'Text', - widgetOptions: '', - isFormula: false, - formula: '', - label: 'A', - description: '', - untieColIdFromLabel: false, - summarySourceCol: 0, - displayCol: 0, - visibleCol: 0, - rules: null, - recalcWhen: 0, - recalcDeps: null - } - }, - { - id: 'B', - fields: { - colRef: 3, - parentId: 1, - parentPos: 2, - type: 'Text', - widgetOptions: '', - isFormula: false, - formula: '', - label: 'B', - description: '', - untieColIdFromLabel: false, - summarySourceCol: 0, - displayCol: 0, - visibleCol: 0, - rules: null, - recalcWhen: 0, - recalcDeps: null - } - }, - { - id: 'C', - fields: { - colRef: 4, - parentId: 1, - parentPos: 3, - type: 'Text', - widgetOptions: '', - isFormula: false, - formula: '', - label: 'C', - description: '', - untieColIdFromLabel: false, - summarySourceCol: 0, - displayCol: 0, - visibleCol: 0, - rules: null, - recalcWhen: 0, - recalcDeps: null - } - }, - { - id: 'D', - fields: { - colRef: 5, - parentId: 1, - parentPos: 3, - type: 'Any', - widgetOptions: '', - isFormula: true, - formula: '', - label: 'D', - description: '', - untieColIdFromLabel: false, - summarySourceCol: 0, - displayCol: 0, - visibleCol: 0, - rules: null, - recalcWhen: 0, - recalcDeps: null - } - }, - { - id: 'E', - fields: { - colRef: 6, - parentId: 1, - parentPos: 4, - type: 'Any', - widgetOptions: '', - isFormula: true, - formula: '$A.upper()', - label: 'E', - description: '', - untieColIdFromLabel: false, - summarySourceCol: 0, - displayCol: 0, - visibleCol: 0, - rules: null, - recalcWhen: 0, - recalcDeps: null - } + const data = { + columns: [ + { + id: 'A', + fields: { + colRef: 2, + parentId: 1, + parentPos: 1, + type: 'Text', + widgetOptions: '', + isFormula: false, + formula: '', + label: 'A', + description: '', + untieColIdFromLabel: false, + summarySourceCol: 0, + displayCol: 0, + visibleCol: 0, + rules: null, + recalcWhen: 0, + recalcDeps: null } - ] - } - ); + }, + { + id: 'B', + fields: { + colRef: 3, + parentId: 1, + parentPos: 2, + type: 'Text', + widgetOptions: '', + isFormula: false, + formula: '', + label: 'B', + description: '', + untieColIdFromLabel: false, + summarySourceCol: 0, + displayCol: 0, + visibleCol: 0, + rules: null, + recalcWhen: 0, + recalcDeps: null + } + }, + { + id: 'C', + fields: { + colRef: 4, + parentId: 1, + parentPos: 3, + type: 'Text', + widgetOptions: '', + isFormula: false, + formula: '', + label: 'C', + description: '', + untieColIdFromLabel: false, + summarySourceCol: 0, + displayCol: 0, + visibleCol: 0, + rules: null, + recalcWhen: 0, + recalcDeps: null + } + }, + { + id: 'D', + fields: { + colRef: 5, + parentId: 1, + parentPos: 3, + type: 'Any', + widgetOptions: '', + isFormula: true, + formula: '', + label: 'D', + description: '', + untieColIdFromLabel: false, + summarySourceCol: 0, + displayCol: 0, + visibleCol: 0, + rules: null, + recalcWhen: 0, + recalcDeps: null + } + }, + { + id: 'E', + fields: { + colRef: 6, + parentId: 1, + parentPos: 4, + type: 'Any', + widgetOptions: '', + isFormula: true, + formula: '$A.upper()', + label: 'E', + description: '', + untieColIdFromLabel: false, + summarySourceCol: 0, + displayCol: 0, + visibleCol: 0, + rules: null, + recalcWhen: 0, + recalcDeps: null + } + } + ] + }; + const respWithTableId = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/Table1/columns`, chimpy); + assert.equal(respWithTableId.status, 200); + assert.deepEqual(respWithTableId.data, data); + const respWithTableRef = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/1/columns`, chimpy); + assert.equal(respWithTableRef.status, 200); + assert.deepEqual(respWithTableRef.data, data); }); it('GET /docs/{did}/tables/{tid}/columns retrieves hidden columns when "hidden" is set', async function () { @@ -869,6 +886,13 @@ function testDocApi() { ] }); + // POST /columns: Create new columns using tableRef in URL + resp = await axios.post(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/5/columns`, { + columns: [{id: "NewCol6", fields: {}}], + }, chimpy); + assert.equal(resp.status, 200); + assert.deepEqual(resp.data, {columns: [{id: "NewCol6"}]}); + // POST /columns to invalid table ID resp = await axios.post(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/NoSuchTable/columns`, {columns: [{}]}, chimpy); @@ -1032,6 +1056,7 @@ function testDocApi() { {colId: "NewCol4", label: 'NewCol4'}, {colId: "NewCol4_2", label: 'NewCol4_2'}, // NewCol5 is hidden by ACL + {colId: "NewCol6", label: 'NewCol6'}, ]); resp = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/NewTable2_2/columns`, chimpy);