From 5e1fa4c74a8dad577e122804d47016cadf409c9a Mon Sep 17 00:00:00 2001 From: George Gevoian Date: Fri, 23 Aug 2024 10:58:51 -0400 Subject: [PATCH] (core) Revert D4302 Summary: This revision introduced a regression when modifying view layouts, leaving documents with layouts having a broken appearance in the client. Test Plan: Existing tests. Reviewers: dsagal Reviewed By: dsagal Differential Revision: https://phab.getgrist.com/D4330 --- sandbox/grist/test_useractions.py | 56 ----------------------------- sandbox/grist/useractions.py | 60 ++++--------------------------- 2 files changed, 6 insertions(+), 110 deletions(-) diff --git a/sandbox/grist/test_useractions.py b/sandbox/grist/test_useractions.py index c6e43645..2238028a 100644 --- a/sandbox/grist/test_useractions.py +++ b/sandbox/grist/test_useractions.py @@ -536,12 +536,6 @@ 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=[ @@ -602,56 +596,6 @@ 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): diff --git a/sandbox/grist/useractions.py b/sandbox/grist/useractions.py index eb2e4c72..19bfc1c0 100644 --- a/sandbox/grist/useractions.py +++ b/sandbox/grist/useractions.py @@ -1267,8 +1267,9 @@ class UserActions(object): # Remove all view fields for all removed columns. # Bypass the check for raw data view sections. - fields = [f for c in col_recs for f in c.viewFields] - self._doRemoveViewSectionFieldRecords(fields) + field_ids = [f.id for c in col_recs for f in c.viewFields] + + self.doBulkRemoveRecord("_grist_Views_section_field", field_ids) # 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. @@ -1352,20 +1353,12 @@ 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]) - 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) + self.doBulkRemoveRecord('_grist_Views_section', [r.id for r in recs]) @override_action('BulkRemoveRecord', '_grist_Views_section_field') def _removeViewSectionFieldRecords(self, table_id, row_ids): """ - Remove view section fields. + Remove view sections, including their 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. """ @@ -1373,21 +1366,7 @@ class UserActions(object): for rec in recs: if rec.parentId.isRaw: raise ValueError("Cannot remove raw view section field") - 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) + self.doBulkRemoveRecord(table_id, row_ids) #---------------------------------------- # User actions on columns. @@ -2382,30 +2361,3 @@ 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