(core) Automatically remove leaves from layout specs

Summary:
When fields or sections were being removed from Grist documents, any
layout specs that referred to them weren't being updated to no longer
do so. This mismatch was causing various buggy scenarios to manifest
in cases where the stale ids were being reused. We now automatically
update any affected layout specs whenever fields or sections are
removed.

Test Plan: Python tests.

Reviewers: jarek

Reviewed By: jarek

Differential Revision: https://phab.getgrist.com/D4302

sandbox/grist/test_useractions.py

@@ -536,6 +536,12 @@ class TestUserActions(test_engine.EngineTestCase):
   def test_section_removes(self):
     # Add a couple of tables and views, to trigger creation of some related items.
     self.init_views_sample()
+    self.apply_user_action(['BulkUpdateRecord', '_grist_Views', [2], {
+      'layoutSpec': [
+        "{\"children\":[{\"children\":[{\"leaf\":10},{\"leaf\":4}]}],"
+          + "\"collapsed\":[{\"leaf\":6}]}",
+      ]
+    }])
 
     self.assertViews([
       View(1, sections=[
@@ -596,6 +602,56 @@ class TestUserActions(test_engine.EngineTestCase):
       ])
     ])
 
+    # Check that the view layout spec also got updated.
+    self.assertTableData('_grist_Views', rows='subset', cols='subset', data=[
+      ['id', 'layoutSpec'],
+      [2, "{\"children\": [{\"children\": [{\"leaf\": 4}]}], \"collapsed\": []}"],
+    ])
+
+  #----------------------------------------------------------------------
+
+  def test_field_removes(self):
+    self.init_views_sample()
+    self.apply_user_action(['BulkUpdateRecord', '_grist_Views_section', [1, 4, 6, 7], {
+      'layoutSpec': [
+        "",
+        "{\"children\":[{\"children\":[{\"leaf\":10},{\"leaf\":11}]},{\"leaf\":12}]}",
+        "{\"id\":\"d748210f-91e1-4209-8f56-c27f4f62efde\",\"type\":\"Layout\",\"children\":[{\"id"
+          + "\":\"a939f6b0-c728-4b36-bdde-6bb727ce4027\",\"type\":\"Paragraph\",\"text\":\"## **F"
+          + "orm Title**\",\"alignment\":\"center\"},{\"id\":\"428c5f4f-b1f8-4bb6-89e5-5cdab4fca5"
+          + "d0\",\"type\":\"Paragraph\",\"text\":\"Your form description goes here.\",\"alignmen"
+          + "t\":\"center\"},{\"id\":\"9bfaa427-7776-411e-9724-e351fb4d5bdf\",\"type\":\"Section"
+          + "\",\"children\":[{\"id\":\"8192f1c8-918f-4b4f-b51c-c775588a6087\",\"type\":\"Paragra"
+          + "ph\",\"text\":\"### **Header**\"},{\"id\":\"6005123f-069b-4b31-85d1-d8c677c53f7e\","
+          + "\"type\":\"Paragraph\",\"text\":\"Description\"},{\"id\":\"444ffba4-5a99-4a3f-abb9-9"
+          + "09deea5c059\",\"type\":\"Field\",\"leaf\":16},{\"id\":\"c7deaa62-3c06-45a8-aa79-122d"
+          + "01c77832\",\"type\":\"Field\",\"leaf\":17},{\"id\":\"b7326be0-a73e-4cc1-8f8b-6c5033d"
+          + "227e4\",\"type\":\"Field\",\"leaf\":18}]},{\"id\":\"3a70d09f-2b08-4875-a813-3432d2d0"
+          + "ae3d\",\"type\":\"Submit\"}]}",
+        "invalid",
+      ]
+    }])
+
+    # Remove a couple of fields. Ensure the layout specs of their sections get updated.
+    self.apply_user_action(['BulkRemoveRecord', '_grist_Views_section_field', [1, 10, 12, 16, 19]])
+    self.assertTableData('_grist_Views_section', rows='subset', cols='subset', data=[
+      ['id', 'layoutSpec'],
+      [1, ""],
+      [4, "{\"children\": [{\"children\": [{\"leaf\": 11}]}]}"],
+      [6, "{\"children\": [{\"alignment\": \"center\", \"id\": \"a939f6b0-c728-4b36-bdde-6bb727ce4"
+        + "027\", \"text\": \"## **Form Title**\", \"type\": \"Paragraph\"}, {\"alignment\": \"cen"
+        + "ter\", \"id\": \"428c5f4f-b1f8-4bb6-89e5-5cdab4fca5d0\", \"text\": \"Your form descript"
+        + "ion goes here.\", \"type\": \"Paragraph\"}, {\"children\": [{\"id\": \"8192f1c8-918f-4b"
+        + "4f-b51c-c775588a6087\", \"text\": \"### **Header**\", \"type\": \"Paragraph\"}, {\"id\""
+        + ": \"6005123f-069b-4b31-85d1-d8c677c53f7e\", \"text\": \"Description\", \"type\": \"Para"
+        + "graph\"}, {\"id\": \"c7deaa62-3c06-45a8-aa79-122d01c77832\", \"leaf\": 17, \"type\": \""
+        + "Field\"}, {\"id\": \"b7326be0-a73e-4cc1-8f8b-6c5033d227e4\", \"leaf\": 18, \"type\": \""
+        + "Field\"}], \"id\": \"9bfaa427-7776-411e-9724-e351fb4d5bdf\", \"type\": \"Section\"}, {"
+        + "\"id\": \"3a70d09f-2b08-4875-a813-3432d2d0ae3d\", \"type\": \"Submit\"}], \"id\": \"d74"
+        + "8210f-91e1-4209-8f56-c27f4f62efde\", \"type\": \"Layout\"}"],
+      [7, "invalid"],
+    ])
+
   #----------------------------------------------------------------------
 
   def test_schema_consistency_check(self):

sandbox/grist/useractions.py

@@ -1267,9 +1267,8 @@ class UserActions(object):
 
     # Remove all view fields for all removed columns.
     # Bypass the check for raw data view sections.
-    field_ids = [f.id for c in col_recs for f in c.viewFields]
-
-    self.doBulkRemoveRecord("_grist_Views_section_field", field_ids)
+    fields = [f for c in col_recs for f in c.viewFields]
+    self._doRemoveViewSectionFieldRecords(fields)
 
     # If there is a displayCol, it may get auto-removed, but may first produce calc actions
     # triggered by the removal of this column. To avoid those, remove displayCols immediately.
@@ -1353,12 +1352,20 @@ class UserActions(object):
     Remove view sections, including their fields, without checking for raw view sections.
     """
     self.doBulkRemoveRecord('_grist_Views_section_field', [f.id for vs in recs for f in vs.fields])
-    self.doBulkRemoveRecord('_grist_Views_section', [r.id for r in recs])
+    views = {r.parentId.id: r.parentId for r in recs}.values()
+    rec_ids = [r.id for r in recs]
+    updates = _get_layout_spec_updates(views, set(rec_ids))
+    if updates:
+      self._do_doc_action(actions.BulkUpdateRecord('_grist_Views',
+        [row_id for (row_id, _) in updates],
+        {'layoutSpec': [value for (_, value) in updates]}
+      ))
+    self.doBulkRemoveRecord('_grist_Views_section', rec_ids)
 
   @override_action('BulkRemoveRecord', '_grist_Views_section_field')
   def _removeViewSectionFieldRecords(self, table_id, row_ids):
     """
-    Remove view sections, including their fields.
+    Remove view section fields.
     Raises an error if trying to remove a field of a table's rawViewSectionRef,
     i.e. hiding a column in a raw data widget.
     """
@@ -1366,7 +1373,21 @@ class UserActions(object):
     for rec in recs:
       if rec.parentId.isRaw:
         raise ValueError("Cannot remove raw view section field")
-    self.doBulkRemoveRecord(table_id, row_ids)
+    self._doRemoveViewSectionFieldRecords(recs)
+
+  def _doRemoveViewSectionFieldRecords(self, recs):
+    """
+    Remove view section fields, without checking for raw view section fields.
+    """
+    view_sections = {r.parentId.id: r.parentId for r in recs}.values()
+    rec_ids = [r.id for r in recs]
+    updates = _get_layout_spec_updates(view_sections, rec_ids)
+    if updates:
+      self._do_doc_action(actions.BulkUpdateRecord('_grist_Views_section',
+        [row_id for (row_id, _) in updates],
+        {'layoutSpec': [value for (_, value) in updates]}
+      ))
+    self.doBulkRemoveRecord('_grist_Views_section_field', rec_ids)
 
   #----------------------------------------
   # User actions on columns.
@@ -2361,3 +2382,30 @@ def _is_transform_col(col_id):
     'gristHelper_Transform',
     'gristHelper_Converted',
   ))
+
+def _get_layout_spec_updates(resources, removed_ids):
+  def get_patched_layout_spec(layout_spec):
+    if not isinstance(layout_spec, dict):
+      return layout_spec
+    if 'leaf' in layout_spec and layout_spec['leaf'] in removed_ids_set:
+      return None
+
+    patched_layout_spec = layout_spec.copy()
+    for key in ('children', 'collapsed'):
+      if key not in layout_spec or not isinstance(layout_spec[key], list):
+        continue
+
+      patched_values = [get_patched_layout_spec(v) for v in layout_spec[key]]
+      patched_layout_spec[key] = [v for v in patched_values if v is not None]
+    return patched_layout_spec
+
+  updates = []
+  removed_ids_set = set(removed_ids)
+  for (row_id, layout_spec) in [(r.id, r.layoutSpec) for r in resources if r.layoutSpec != ""]:
+    try:
+      layout_spec_parsed = json.loads(layout_spec)
+    except ValueError:
+      continue
+    new_layout_spec = json.dumps(get_patched_layout_spec(layout_spec_parsed), sort_keys=True)
+    updates.append((row_id, new_layout_spec))
+  return updates