(core) Backend restrictions for raw data widgets

Summary:
Prevent most updates or removals of raw view sections and their fields in `useractions.py`. Only a fiew columns are allowed to be updated.

Removed the unused method `_UpdateViews` while I was at it.

Test Plan:
Added a Python test.

Tested manually that I can still make all expected changes, i.e. those allowed by the UI, e.g. reordering columns.

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3263
This commit is contained in:
Alex Hall 2022-02-14 14:25:52 +02:00
parent 2f6eafff35
commit c0ca936c1e
5 changed files with 111 additions and 23 deletions

View File

@ -105,6 +105,9 @@ class MetaTableExtras(object):
class _grist_Views_section(object): class _grist_Views_section(object):
fields = _record_set('_grist_Views_section_field', 'parentId', sort_by='parentPos') fields = _record_set('_grist_Views_section_field', 'parentId', sort_by='parentPos')
def isRaw(rec, table):
return rec.tableRef.rawViewSectionRef == rec
class _grist_Filters(object): class _grist_Filters(object):
def setAutoRemove(rec, table): def setAutoRemove(rec, table):
"""Marks the filter for removal if its column no longer exists.""" """Marks the filter for removal if its column no longer exists."""

View File

@ -142,7 +142,7 @@ class TestUserActions(test_engine.EngineTestCase):
# same formula that the new column has. Make sure that the new column is NOT used as # same formula that the new column has. Make sure that the new column is NOT used as
# the display column. # the display column.
self.apply_user_action(['AddRecord', '_grist_Views_section_field', None, { self.apply_user_action(['AddRecord', '_grist_Views_section_field', None, {
'parentId': 2, 'parentId': 3,
'colRef': 25 'colRef': 25
}]) }])
self.apply_user_action(['SetDisplayFormula', 'Favorites', 6, None, '$favorite.viewers']) self.apply_user_action(['SetDisplayFormula', 'Favorites', 6, None, '$favorite.viewers'])

View File

@ -168,8 +168,7 @@ class TestDocModel(test_engine.EngineTestCase):
view = self.engine.docmodel.views.lookupOne(id=1) view = self.engine.docmodel.views.lookupOne(id=1)
self.assertRecordSet(view.viewSections, [1, 5, 6]) self.assertRecordSet(view.viewSections, [1, 5, 6])
self.engine.docmodel.remove(f for vs in table.viewSections for f in vs.fields) self.engine.docmodel.remove(set(table.viewSections) - {table.rawViewSectionRef})
self.engine.docmodel.remove(table.viewSections)
self.assertRecordSet(view.viewSections, [6]) self.assertRecordSet(view.viewSections, [6])

View File

@ -1171,3 +1171,57 @@ class TestUserActions(test_engine.EngineTestCase):
["L", 2], ["L", 2],
"999", "999",
]}]]}) ]}]]})
def test_raw_view_section_restrictions(self):
# load_sample handles loading basic metadata, but doesn't create any view sections
self.load_sample(self.sample)
# Create a new table which automatically gets a raw view section
self.apply_user_action(["AddEmptyTable"])
# Note the row IDs of the raw view section (2) and fields (4, 5, 6)
self.assertTableData('_grist_Views_section', cols="subset", data=[
["id", "parentId", "tableRef"],
[1, 1, 2],
[2, 0, 2], # the raw view section
])
self.assertTableData('_grist_Views_section_field', cols="subset", data=[
["id", "parentId"],
[1, 1],
[2, 1],
[3, 1],
# the raw view section
[4, 2],
[5, 2],
[6, 2],
])
# Test that the records cannot be removed by normal user actions
with self.assertRaisesRegex(ValueError, "Cannot remove raw view section$"):
self.apply_user_action(["RemoveRecord", '_grist_Views_section', 2])
with self.assertRaisesRegex(ValueError, "Cannot remove raw view section field$"):
self.apply_user_action(["RemoveRecord", '_grist_Views_section_field', 4])
# and most of their column values can't be changed
with self.assertRaisesRegex(ValueError, "Cannot modify raw view section$"):
self.apply_user_action(["UpdateRecord", '_grist_Views_section', 2, {"parentId": 1}])
with self.assertRaisesRegex(ValueError, "Cannot modify raw view section fields$"):
self.apply_user_action(["UpdateRecord", '_grist_Views_section_field', 5, {"parentId": 1}])
# Confirm that the records are unchanged
self.assertTableData('_grist_Views_section', cols="subset", data=[
["id", "parentId", "tableRef"],
[1, 1, 2],
[2, 0, 2], # the raw view section
])
self.assertTableData('_grist_Views_section_field', cols="subset", data=[
["id", "parentId"],
[1, 1],
[2, 1],
[3, 1],
# the raw view section
[4, 2],
[5, 2],
[6, 2],
])

View File

@ -375,14 +375,33 @@ class UserActions(object):
#---------------------------------------- #----------------------------------------
# UpdateRecords & co. # UpdateRecords & co.
#---------------------------------------- # ----------------------------------------
def doBulkUpdateRecord(self, table_id, row_ids, columns): def doBulkUpdateRecord(self, table_id, row_ids, columns):
# Convert passed-in values to the column's correct types (or alttext, or errors) and trim any # Convert passed-in values to the column's correct types (or alttext, or errors) and trim any
# unchanged values. # unchanged values.
action, extra_actions = self._engine.convert_action_values( action, extra_actions = self._engine.convert_action_values(
actions.BulkUpdateRecord(table_id, row_ids, columns)) actions.BulkUpdateRecord(table_id, row_ids, columns))
action = self._engine.trim_update_action(action) action = [_, row_ids, column_values] = self._engine.trim_update_action(action)
# Prevent modifying raw data widgets and their fields
# This is done here so that the trimmed action can be checked,
# preventing spurious errors when columns are set to default values
if (
table_id == "_grist_Views_section"
and any(rec.isRaw for i, rec in self._bulk_action_iter(table_id, row_ids))
# Only these fields are allowed to be modified
and not set(column_values) <= {"title", "options", "sortColRefs"}
):
raise ValueError("Cannot modify raw view section")
if (
table_id == "_grist_Views_section_field"
and any(rec.parentId.isRaw for i, rec in self._bulk_action_iter(table_id, row_ids))
# Only these fields are allowed to be modified
and not set(column_values) <= {"parentPos", "width"}
):
raise ValueError("Cannot modify raw view section fields")
# If any extra actions were generated (e.g. to adjust positions), apply them. # If any extra actions were generated (e.g. to adjust positions), apply them.
for a in extra_actions: for a in extra_actions:
@ -393,7 +412,6 @@ class UserActions(object):
# Invalidate trigger-formula columns affected by this update. # Invalidate trigger-formula columns affected by this update.
table = self._engine.tables[table_id] table = self._engine.tables[table_id]
column_values = action[2]
if column_values: # Only if this is a non-trivial update. if column_values: # Only if this is a non-trivial update.
for col_id, col_obj in six.iteritems(table.all_columns): for col_id, col_obj in six.iteritems(table.all_columns):
if col_obj.is_formula() or not col_obj.has_formula(): if col_obj.is_formula() or not col_obj.has_formula():
@ -913,7 +931,8 @@ class UserActions(object):
self._docmodel.remove(self._collect_back_references(remove_table_recs)) self._docmodel.remove(self._collect_back_references(remove_table_recs))
# Remove all view sections and fields for all tables being removed. # Remove all view sections and fields for all tables being removed.
self._docmodel.remove(vs for t in remove_table_recs for vs in t.viewSections) # Bypass the check for raw data view sections.
self._doRemoveViewSectionRecords([vs for t in remove_table_recs for vs in t.viewSections])
# TODO: we need sandbox-side tests for this logic and similar logic elsewhere that deals with # TODO: we need sandbox-side tests for this logic and similar logic elsewhere that deals with
# application-level relationships; it is not tested by testscript (nor should be, most likely; # application-level relationships; it is not tested by testscript (nor should be, most likely;
@ -981,7 +1000,9 @@ class UserActions(object):
self._docmodel.update(re_sort_sections, sortColRefs=re_sort_specs) self._docmodel.update(re_sort_sections, sortColRefs=re_sort_specs)
# Remove all view fields for all removed columns. # Remove all view fields for all removed columns.
self._docmodel.remove([f for c in col_recs for f in c.viewFields]) # 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)
# If there is a displayCol, it may get auto-removed, but may first produce calc actions # 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. # triggered by the removal of this column. To avoid those, remove displayCols immediately.
@ -1044,11 +1065,34 @@ class UserActions(object):
def _removeViewSectionRecords(self, table_id, row_ids): def _removeViewSectionRecords(self, table_id, row_ids):
""" """
Remove view sections, including their fields. Remove view sections, including their fields.
Raises an error if trying to remove a table's rawViewSectionRef.
To bypass that check, call _doRemoveViewSectionRecords.
""" """
view_section_recs = [rec for i, rec in self._bulk_action_iter(table_id, row_ids)] recs = [rec for i, rec in self._bulk_action_iter(table_id, row_ids)]
self._docmodel.remove(f for vs in view_section_recs for f in vs.fields) for rec in recs:
self.doBulkRemoveRecord(table_id, row_ids) if rec.isRaw:
raise ValueError("Cannot remove raw view section")
self._doRemoveViewSectionRecords(recs)
def _doRemoveViewSectionRecords(self, recs):
"""
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])
@override_action('BulkRemoveRecord', '_grist_Views_section_field')
def _removeViewSectionFieldRecords(self, table_id, row_ids):
"""
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.
"""
recs = [rec for i, rec in self._bulk_action_iter(table_id, row_ids)]
for rec in recs:
if rec.parentId.isRaw:
raise ValueError("Cannot remove raw view section field")
self.doBulkRemoveRecord(table_id, row_ids)
#---------------------------------------- #----------------------------------------
# User actions on columns. # User actions on columns.
@ -1672,18 +1716,6 @@ class UserActions(object):
# Methods for creating and maintaining default views. This is a work-in-progress. # Methods for creating and maintaining default views. This is a work-in-progress.
#-------------------------------------------------------------------------------- #--------------------------------------------------------------------------------
def _UpdateViews(self, table_id):
"""
Updates records for default Views to include those fields that they should include by default
(such as all fields for the Raw View, and first 4 fields for 'list' section of List View).
"""
table_row_id = self._engine.tables['_grist_Tables'].get(tableId=table_id)
meta_view_sections = self._engine.tables['_grist_Views_section']
for view_section_row_id in meta_view_sections.filter(tableRef=table_row_id):
parent_key = meta_view_sections.all_columns['parentKey'].raw_get(view_section_row_id)
limit = 4 if parent_key == 'list' else None
self._RebuildViewFields(table_id, view_section_row_id, limit=limit)
def _RebuildViewFields(self, table_id, section_row_id, limit=None): def _RebuildViewFields(self, table_id, section_row_id, limit=None):
""" """
Does the actual work of rebuilding ViewFields to correspond to the table's columns. Does the actual work of rebuilding ViewFields to correspond to the table's columns.