Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow using the existing numeric table IDs in the API #690

Merged
merged 7 commits into from
Oct 12, 2023
Merged

feat: allow using the existing numeric table IDs in the API #690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2dc07f1
feat add technical id for tableId
CamilleLegeron Oct 9, 2023
60fc380
delete echo
CamilleLegeron Oct 9, 2023
89f4066
add technical tableId compatibility in api calls for post and patch
CamilleLegeron Oct 9, 2023
2946107
remove unappropiate code
CamilleLegeron Oct 9, 2023
647bb57
remove line to reload test in github
CamilleLegeron Oct 10, 2023
64237f9
refactor the call to getRealTableId
CamilleLegeron Oct 12, 2023
b7be757
add test for get call api
CamilleLegeron Oct 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions app/server/lib/ActiveDoc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand All @@ -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<string> {
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']);
}
52 changes: 29 additions & 23 deletions app/server/lib/DocApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<SchemaTypes['_grist_Triggers']> = {};

if (url && !isUrlAllowed(url)) {
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);
})
Expand Down Expand Up @@ -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});
})
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}));
Expand Down Expand Up @@ -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);
})
Expand All @@ -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);
})
Expand All @@ -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);
Expand All @@ -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);
})
Expand All @@ -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);
})
Expand All @@ -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),
Expand All @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<TableOperationsImpl> {
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(
Expand Down
Loading
Loading