DocApi: Implement DELETE on columns (#601) (#640)

* Factorize generateDocAndUrl
* Add describe for regrouping /records

app/plugin/TableOperationsImpl.ts

@@ -200,13 +200,16 @@ export async function handleSandboxErrorOnPlatform<T>(
         platform.throwError('', `Invalid row id ${match[1]}`, 400);
       }
       match = message.match(
-        /\[Sandbox] (?:KeyError u?'(?:Table \w+ has no column )?|ValueError No such table: )(\w+)/
+        // eslint-disable-next-line max-len
+        /\[Sandbox] (?:KeyError u?'(?:Table \w+ has no column )?|ValueError No such table: |ValueError No such column: )([\w.]+)/
       );
       if (match) {
         if (match[1] === tableId) {
           platform.throwError('', `Table not found "${tableId}"`, 404);
         } else if (colNames.includes(match[1])) {
           platform.throwError('', `Invalid column "${match[1]}"`, 400);
+        } else if (colNames.includes(match[1].replace(`${tableId}.`, ''))) {
+          platform.throwError('', `Table or column not found "${match[1]}"`, 404);
         }
       }
       platform.throwError('', `Error manipulating data: ${message}`, 400);

app/server/lib/DocApi.ts

@@ -746,6 +746,17 @@ 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 actions = [ [ 'RemoveColumn', tableId, colId ] ];
+        await handleSandboxError(tableId, [colId],
+          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

@@ -877,15 +877,15 @@ function testDocApi() {
     assert.equal(resp.status, 200);
   });
 
-  describe("PUT /docs/{did}/columns", function () {
+  describe("/docs/{did}/tables/{tid}/columns", function () {
-
+    async function generateDocAndUrl(docName: string = "Dummy") {
-    async function generateDocAndUrl() {
       const wid = (await userApi.getOrgWorkspaces('current')).find((w) => w.name === 'Private')!.id;
-      const docId = await userApi.newDoc({name: 'ColumnsPut'}, wid);
+      const docId = await userApi.newDoc({name: docName}, wid);
       const url = `${serverUrl}/api/docs/${docId}/tables/Table1/columns`;
       return { url, docId };
     }
 
+    describe("PUT /docs/{did}/tables/{tid}/columns", function () {
       async function getColumnFieldsMapById(url: string, params: any) {
         const result = await axios.get(url, {...chimpy, params});
         assert.equal(result.status, 200);
@@ -902,7 +902,7 @@ function testDocApi() {
         expectedFieldsByColId: Record<string, object>,
         opts?: { getParams?: any }
       ) {
-      const {url} = await generateDocAndUrl();
+        const {url} = await generateDocAndUrl('ColumnsPut');
         const body: ColumnsPut = { columns };
         const resp = await axios.put(url, body, {...chimpy, params});
         assert.equal(resp.status, 200);
@@ -973,7 +973,7 @@ function testDocApi() {
 
       it('should forbid update by viewers', async function () {
         // given
-      const { url, docId } = await generateDocAndUrl();
+        const { url, docId } = await generateDocAndUrl('ColumnsPut');
         await userApi.updateDocPermissions(docId, {users: {'kiwi@getgrist.com': 'viewers'}});
 
         // when
@@ -985,7 +985,7 @@ function testDocApi() {
 
       it("should return 404 when table is not found", async function() {
         // given
-      const { url } = await generateDocAndUrl();
+        const { url } = await generateDocAndUrl('ColumnsPut');
         const notFoundUrl = url.replace("Table1", "NonExistingTable");
 
         // when
@@ -997,6 +997,50 @@ function testDocApi() {
       });
     });
 
+    describe("DELETE /docs/{did}/tables/{tid}/columns/{colId}", function () {
+      it('should delete some column', async function() {
+        const {url} = await generateDocAndUrl('ColumnDelete');
+        const deleteUrl = url + '/A';
+        const resp = await axios.delete(deleteUrl, chimpy);
+
+        assert.equal(resp.status, 200, "Should succeed in requesting column deletion");
+
+        const listColResp = await axios.get(url, { ...chimpy, params: { hidden: true } });
+        assert.equal(listColResp.status, 200, "Should succeed in listing columns");
+
+        const columnIds = listColResp.data.columns.map(({id}: {id: string}) => id).sort();
+        assert.deepEqual(columnIds, ["B", "C", "manualSort"]);
+      });
+
+      it('should return 404 if table not found', async function() {
+        const {url} = await generateDocAndUrl('ColumnDelete');
+        const deleteUrl = url.replace("Table1", "NonExistingTable") + '/A';
+        const resp = await axios.delete(deleteUrl, chimpy);
+
+        assert.equal(resp.status, 404);
+        assert.equal(resp.data.error, 'Table or column not found "NonExistingTable.A"');
+      });
+
+      it('should return 404 if column not found', async function() {
+        const {url} = await generateDocAndUrl('ColumnDelete');
+        const deleteUrl = url + '/NonExistingColId';
+        const resp = await axios.delete(deleteUrl, chimpy);
+
+        assert.equal(resp.status, 404);
+        assert.equal(resp.data.error, 'Table or column not found "Table1.NonExistingColId"');
+      });
+
+      it('should forbid column deletion by viewers', async function() {
+        const {url, docId} = await generateDocAndUrl('ColumnDelete');
+        await userApi.updateDocPermissions(docId, {users: {'kiwi@getgrist.com': 'viewers'}});
+        const deleteUrl = url + '/A';
+        const resp = await axios.delete(deleteUrl, kiwi);
+
+        assert.equal(resp.status, 403);
+      });
+    });
+  });
+
   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);