Issue #601: implement PUT on columns (#605)

* Also Add includeHidden param for GET on columns

app/plugin/DocApiTypes-ti.ts

@@ -68,6 +68,10 @@ export const ColumnsPatch = t.iface([], {
   "columns": t.tuple("RecordWithStringId", t.rest(t.array("RecordWithStringId"))),
 });
 
+export const ColumnsPut = t.iface([], {
+  "columns": t.tuple("RecordWithStringId", t.rest(t.array("RecordWithStringId"))),
+});
+
 export const TablePost = t.iface(["ColumnsPost"], {
   "id": t.opt("string"),
 });
@@ -93,6 +97,7 @@ const exportedTypeSuite: t.ITypeSuite = {
   MinimalRecord,
   ColumnsPost,
   ColumnsPatch,
+  ColumnsPut,
   TablePost,
   TablesPost,
   TablesPatch,

app/plugin/DocApiTypes.ts

@@ -88,6 +88,10 @@ export interface ColumnsPatch {
   columns: [RecordWithStringId, ...RecordWithStringId[]]; // at least one column is required
 }
 
+export interface ColumnsPut {
+  columns: [RecordWithStringId, ...RecordWithStringId[]]; // at least one column is required
+}
+
 /**
  * Creating tables requires a list of columns.
  * `fields` is not accepted because it's not generally sensible to set the metadata fields on new tables.

app/server/lib/DocApi.ts

@@ -2,7 +2,14 @@ import {concatenateSummaries, summarizeAction} from "app/common/ActionSummarizer
 import {createEmptyActionSummary} from "app/common/ActionSummary";
 import {ApiError, LimitType} from 'app/common/ApiError';
 import {BrowserSettings} from "app/common/BrowserSettings";
-import {BulkColValues, ColValues, fromTableDataAction, TableColValues, TableRecordValue} from 'app/common/DocActions';
+import {
+  BulkColValues,
+  ColValues,
+  fromTableDataAction,
+  TableColValues,
+  TableRecordValue,
+  UserAction
+} from 'app/common/DocActions';
 import {isRaisedException} from "app/common/gristTypes";
 import {buildUrlId, parseUrlId} from "app/common/gristUrls";
 import {isAffirmative} from "app/common/gutil";
@@ -94,7 +101,7 @@ type WithDocHandler = (activeDoc: ActiveDoc, req: RequestWithLogin, resp: Respon
 // Schema validators for api endpoints that creates or updates records.
 const {
   RecordsPatch, RecordsPost, RecordsPut,
-  ColumnsPost, ColumnsPatch,
+  ColumnsPost, ColumnsPatch, ColumnsPut,
   TablesPost, TablesPatch,
 } = t.createCheckers(DocApiTypesTI, GristDataTI);
 
@@ -357,8 +364,9 @@ export class DocWorkerApi {
     this._app.get('/api/docs/:docId/tables/:tableId/columns', canView,
       withDoc(async (activeDoc, req, res) => {
         const tableId = req.params.tableId;
+        const includeHidden = isAffirmative(req.query.includeHidden);
         const columns = await handleSandboxError('', [],
-          activeDoc.getTableCols(docSessionFromRequest(req), tableId));
+          activeDoc.getTableCols(docSessionFromRequest(req), tableId, includeHidden));
         res.json({columns});
       })
     );
@@ -681,6 +689,54 @@ export class DocWorkerApi {
       })
     );
 
+    // Add or update records given in records format
+    this._app.put('/api/docs/:docId/tables/:tableId/columns', canEdit, validate(ColumnsPut),
+      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 tableRef = tablesTable.findMatchingRowId({tableId});
+        if (!tableRef) {
+          throw new ApiError(`Table not found "${tableId}"`, 404);
+        }
+        const body = req.body as Types.ColumnsPut;
+
+        const addActions: UserAction[] = [];
+        const updateActions: UserAction[] = [];
+        const updatedColumnsIds = new Set();
+
+        for (const col of body.columns) {
+          const id = columnsTable.findMatchingRowId({parentId: tableRef, colId: col.id});
+          if (id) {
+            updateActions.push( ['UpdateRecord', '_grist_Tables_column', id, col.fields] );
+            updatedColumnsIds.add( id );
+          } else {
+            addActions.push( ['AddVisibleColumn', tableId, col.id, col.fields] );
+          }
+        }
+
+        const getRemoveAction = async () => {
+          const columns = await handleSandboxError('', [],
+            activeDoc.getTableCols(docSessionFromRequest(req), tableId));
+          const columnsToRemove = columns
+            .map(col => col.fields.colRef as number)
+            .filter(colRef => !updatedColumnsIds.has(colRef));
+
+          return [ 'BulkRemoveRecord', '_grist_Tables_column', columnsToRemove ];
+        };
+
+        const actions = [
+          ...(!isAffirmative(req.query.noupdate) ? updateActions : []),
+          ...(!isAffirmative(req.query.noadd) ? addActions : []),
+          ...(isAffirmative(req.query.replaceall) ? [ await getRemoveAction() ] : [] )
+        ];
+        await handleSandboxError(tableId, [],
+          activeDoc.applyUserActions(docSessionFromRequest(req), actions)
+        );
+        res.json(null);
+      })
+    );
+
     // Add a new webhook and trigger
     this._app.post('/api/docs/:docId/webhooks', isOwner, validate(WebhookSubscribeCollection),
       withDoc(async (activeDoc, req, res) => {

test/server/lib/DocApi.ts

@@ -5,7 +5,7 @@ import {arrayRepeat} from 'app/common/gutil';
 import {WebhookSummary} from 'app/common/Triggers';
 import {DocAPI, DocState, UserAPIImpl} from 'app/common/UserAPI';
 import {testDailyApiLimitFeatures} from 'app/gen-server/entity/Product';
-import {AddOrUpdateRecord, Record as ApiRecord} from 'app/plugin/DocApiTypes';
+import {AddOrUpdateRecord, Record as ApiRecord, ColumnsPut, RecordWithStringId} from 'app/plugin/DocApiTypes';
 import {CellValue, GristObjCode} from 'app/plugin/GristData';
 import {
   applyQueryParameters,
@@ -610,6 +610,21 @@ function testDocApi() {
     );
   });
 
+  it("GET /docs/{did}/tables/{tid}/columns retrieves hidden columns when includeHidden is set", async function () {
+    const params = { includeHidden: true };
+    const resp = await axios.get(
+      `${serverUrl}/api/docs/${docIds.Timesheets}/tables/Table1/columns`,
+      { ...chimpy, params }
+    );
+    assert.equal(resp.status, 200);
+    const columnsMap = new Map(resp.data.columns.map(({id, fields}: {id: string, fields: object}) => [id, fields]));
+    assert.include([...columnsMap.keys()], "manualSort");
+    assert.deepInclude(columnsMap.get("manualSort"), {
+      colRef: 1,
+      type: 'ManualSortPos',
+    });
+  });
+
   it("GET/POST/PATCH /docs/{did}/tables and /columns", async function () {
     // POST /tables: Create new tables
     let resp = await axios.post(`${serverUrl}/api/docs/${docIds.Timesheets}/tables`, {
@@ -842,6 +857,119 @@ function testDocApi() {
     assert.equal(resp.status, 200);
   });
 
+  describe("PUT /docs/{did}/columns", function () {
+
+    async function generateDocAndUrl() {
+      const wid = (await userApi.getOrgWorkspaces('current')).find((w) => w.name === 'Private')!.id;
+      const docId = await userApi.newDoc({name: 'ColumnsPut'}, wid);
+      const url = `${serverUrl}/api/docs/${docId}/tables/Table1/columns`;
+      return { url, docId };
+    }
+
+    async function getColumnFieldsMapById(url: string) {
+      const result = await axios.get(url, chimpy);
+      assert.equal(result.status, 200);
+      return new Map<string, object>(
+          result.data.columns.map(
+            ({id, fields}: {id: string, fields: object}) => [id, fields]
+          )
+      );
+    }
+
+    async function checkPut(
+      columns: [RecordWithStringId, ...RecordWithStringId[]],
+      params: Record<string, any>,
+      expectedFieldsByColId: Record<string, object>,
+    ) {
+      const {url} = await generateDocAndUrl();
+      const body: ColumnsPut = { columns };
+      const resp = await axios.put(url, body, {...chimpy, params});
+      assert.equal(resp.status, 200);
+      const fieldsByColId = await getColumnFieldsMapById(url);
+
+      assert.deepEqual(
+        [...fieldsByColId.keys()],
+        Object.keys(expectedFieldsByColId),
+        "The updated table should have the expected columns"
+      );
+
+      for (const [colId, expectedFields] of Object.entries(expectedFieldsByColId)) {
+        assert.deepInclude(fieldsByColId.get(colId), expectedFields);
+      }
+    }
+
+    const COLUMN_TO_ADD = {
+      id: "Foo",
+      fields: {
+        type: "Text",
+        label: "FooLabel",
+      }
+    };
+
+    const COLUMN_TO_UPDATE = {
+      id: "A",
+      fields: {
+        type: "Numeric",
+        colId: "NewA"
+      }
+    };
+
+    it('should create new columns', async function () {
+      await checkPut([COLUMN_TO_ADD], {}, {
+        A: {}, B: {}, C: {}, Foo: COLUMN_TO_ADD.fields
+      });
+    });
+
+    it('should update existing columns and create new ones', async function () {
+      await checkPut([COLUMN_TO_ADD, COLUMN_TO_UPDATE], {}, {
+        NewA: {type: "Numeric", label: "A"}, B: {}, C: {}, Foo: COLUMN_TO_ADD.fields
+      });
+    });
+
+    it('should only update existing columns when noadd is set', async function () {
+      await checkPut([COLUMN_TO_ADD, COLUMN_TO_UPDATE], {noadd: "1"}, {
+        NewA: {type: "Numeric"}, B: {}, C: {}
+      });
+    });
+
+    it('should only add columns when noupdate is set', async function () {
+      await checkPut([COLUMN_TO_ADD, COLUMN_TO_UPDATE], {noupdate: "1"}, {
+        A: {type: "Any"}, B: {}, C: {}, Foo: COLUMN_TO_ADD.fields
+      });
+    });
+
+    it('should remove existing columns if replaceall is set', async function () {
+      await checkPut([COLUMN_TO_ADD, COLUMN_TO_UPDATE], {replaceall: "1"}, {
+        NewA: {type: "Numeric"}, Foo: COLUMN_TO_ADD.fields
+      });
+    });
+
+    it('should forbid update by viewers', async function () {
+      // given
+      const { url, docId } = await generateDocAndUrl();
+      await userApi.updateDocPermissions(docId, {users: {'kiwi@getgrist.com': 'viewers'}});
+
+      // when
+      const resp = await axios.put(url, { columns: [ COLUMN_TO_ADD ] }, kiwi);
+
+      // then
+      assert.equal(resp.status, 403);
+    });
+
+    it("should return 404 when table is not found", async function() {
+      // given
+      const { url } = await generateDocAndUrl();
+      const notFoundUrl = url.replace("Table1", "NonExistingTable");
+
+      // when
+      const resp = await axios.put(notFoundUrl, { columns: [ COLUMN_TO_ADD ] }, chimpy);
+
+      // then
+      assert.equal(resp.status, 404);
+      assert.equal(resp.data.error, 'Table not found "NonExistingTable"');
+    });
+  });
+
   it("GET /docs/{did}/tables/{tid}/data returns 404 for non-existent doc", async function () {
     const resp = await axios.get(`${serverUrl}/api/docs/typotypotypo/tables/Table1/data`, chimpy);
     assert.equal(resp.status, 404);