gristlabs / grist-core

Grist is the evolution of spreadsheets.
https://www.getgrist.com/
Apache License 2.0
7.02k stars 311 forks source link

Prevent adding invalid references #616

Open fflorent opened 1 year ago

fflorent commented 1 year ago

When using PUT or PATCH to update columns through its API, if we add invalid references, like nonexistent rules, we cannot delete the column anymore.

To reproduce the issue, I created a unit test, see below.

To fix that, as suggested by @alexmojaki in this comment, we may add some python code to validate the metadata before applying the changes.

Unit test:

commit 1d9952a142b58aa50d98bc136e65222d0d676ec9 (HEAD -> refs/heads/issue-601-implement-missing-columns-api-methods)
Author: Florent FAYOLLE <florent.fayolle@beta.gouv.fr>
Date:   Tue Aug 8 11:37:27 2023 +0200

    Test failing with rules updated #601

diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts
index 887d226d..db36007d 100644
--- a/test/server/lib/DocApi.ts
+++ b/test/server/lib/DocApi.ts
@@ -1021,6 +1021,35 @@ function testDocApi() {
       assert.equal(fieldsByColId.size, 2, "Expecting to have removed the other columns");
     });

+    it('when called with inconsistent rule, deleting should be allowed', async function () {
+      // given
+      const { url } = await generateDocAndUrl();
+
+      const inconsistentRule: [GristObjCode, number] = [ GristObjCode.List, 173];
+      const submittedColumns: ColumnsPut = {
+        columns: [{ ...COLUMN_TO_ADD, fields: { ...COLUMN_TO_ADD.fields, rules: inconsistentRule }}]
+      };
+
+      const resp = await axios.put(url, submittedColumns, {...chimpy });
+
+      assert.equal(resp.status, 200);
+
+      const columnsWithoutPreviousOne: ColumnsPut = {
+        columns: [ COLUMN_TO_UPDATE ]
+      };
+      const paramsForDeletingTheRest = { replaceall: "1" };
+
+      // when
+      const respDelete = await axios.put(url, columnsWithoutPreviousOne, {
+        ...chimpy,
+        params: paramsForDeletingTheRest
+      });
+
+      // then
+      console.log("respDelete.data = ", respDelete.data);
+      assert.equal(respDelete.status, 200);
+    });
+
     it('should forbid update by viewers', async function () {
       // given
       const { url, docId } = await generateDocAndUrl();
vviers commented 6 days ago

Related : https://community.getgrist.com/t/self-sanitizing-reference-fields/6403/2