(core) Mark column changing actions as indirect when adding data to empty column

Summary:
Fixing https://gristlabs.getgrist.com/doc/check-ins/p/12#a1.s19.r1045.c19 :

> Problem: user creates fresh new empty column. Users with access to write to that column, but not modify schema, will not in fact be able to write into it (since on first data entry column type needs to change). Experience is confusing.

Refactored `enter_indirection` and `leave_indirection` to a single context manager method for use with `with` instead of `try/finally`.

Used the new method in `_ensure_column_accepts_data` around column changing actions converting empty column to data column.

Test Plan:
Updated a Python test, reflecting that the correct actions are now marked as direct=False.

Tested manually that I can now add data to a blank column without schema access, while I wasn't able to before, and I still can't make other schema changes.

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D3446
This commit is contained in:
Alex Hall 2022-05-23 19:36:21 +02:00
parent 3d3a5e334a
commit 8ee23f5344
3 changed files with 35 additions and 33 deletions

View File

@ -324,12 +324,10 @@ class Table(object):
if summary_table._summary_simple:
@usertypes.formulaType(usertypes.Reference(summary_table.table_id))
def _updateSummary(rec, table): # pylint: disable=unused-argument
try:
# summary table output should be treated as we treat formula columns, for acl purposes
self._engine.user_actions.enter_indirection()
# summary table output should be treated as we treat formula columns, for acl purposes
with self._engine.user_actions.indirect_actions():
return summary_table.lookupOrAddDerived(**{c: getattr(rec, c) for c in groupby_cols})
finally:
self._engine.user_actions.leave_indirection()
else:
@usertypes.formulaType(usertypes.ReferenceList(summary_table.table_id))
def _updateSummary(rec, table): # pylint: disable=unused-argument
@ -368,14 +366,11 @@ class Table(object):
new_row_ids.append(None)
if new_row_ids and not self._engine.is_triggered_by_table_action(summary_table.table_id):
try:
# summary table output should be treated as we treat formula columns, for acl purposes
self._engine.user_actions.enter_indirection()
# summary table output should be treated as we treat formula columns, for acl purposes
with self._engine.user_actions.indirect_actions():
result += self._engine.user_actions.BulkAddRecord(
summary_table.table_id, new_row_ids, values_to_add
)
finally:
self._engine.user_actions.leave_indirection()
return result

View File

@ -119,7 +119,7 @@ return '#%s %s' % (table.my_counter, $schoolName)
["UpdateRecord", "_grist_Tables_column", 22, {"isFormula": False}],
["UpdateRecord", "Students", 6, {"newCol": "Boo!"}],
],
"direct": [True, True, True, False, True, True],
"direct": [False, False, False, False, False, True],
"undo": [
["ModifyColumn", "Students", "newCol", {"type": "Any"}],
["UpdateRecord", "_grist_Tables_column", 22, {"type": "Any"}],

View File

@ -3,6 +3,7 @@ from collections import namedtuple, Counter, OrderedDict
import re
import json
import sys
from contextlib import contextmanager
import six
from six.moves import xrange
@ -221,19 +222,22 @@ class UserActions(object):
self._overrides = {key: method.__get__(self, UserActions)
for key, method in six.iteritems(_action_method_overrides)}
def enter_indirection(self):
@contextmanager
def indirect_actions(self):
"""
Mark any actions following this call as being indirect, until leave_indirection is
called. Nesting is supported (but not used).
"""
self._indirection_level += 1
Usage:
def leave_indirection(self):
with self.indirect_actions():
# apply actions here
This marks those actions as being indirect, for ACL purposes.
"""
Undo an enter_indirection.
"""
self._indirection_level -= 1
assert self._indirection_level >= 0
try:
self._indirection_level += 1
yield
finally:
self._indirection_level -= 1
assert self._indirection_level >= 0
def _do_doc_action(self, action):
if hasattr(action, 'simplify'):
@ -862,18 +866,21 @@ class UserActions(object):
raise ValueError("Can't save value to formula column %s" % col_id)
# An empty column (isFormula=True, formula=""), now is the time to convert it to data.
if schema_col.type == 'Any':
# Guess the type when it starts out as Any. We unfortunately need to update the column
# separately for type conversion, to recompute type-specific defaults
# before they are used in formula->data conversion.
col_info, values = guess_col_info(values)
# If the values are all blank (None or empty string) leave the column empty
if not col_info:
return values
col_rec = self._docmodel.get_column_rec(table_id, col_id)
self._docmodel.update([col_rec], **col_info)
self.ModifyColumn(table_id, col_id, {'isFormula': False})
return values
# Since the user is merely adding/updating plain records, they shouldn't be blocked by
# ACL rules preventing schema changes, i.e. these column changing actions are not direct.
with self.indirect_actions():
if schema_col.type == 'Any':
# Guess the type when it starts out as Any. We unfortunately need to update the column
# separately for type conversion, to recompute type-specific defaults
# before they are used in formula->data conversion.
col_info, values = guess_col_info(values)
# If the values are all blank (None or empty string) leave the column empty
if not col_info:
return values
col_rec = self._docmodel.get_column_rec(table_id, col_id)
self._docmodel.update([col_rec], **col_info)
self.ModifyColumn(table_id, col_id, {'isFormula': False})
return values
@useraction
def AddOrUpdateRecord(self, table_id, require, col_values, options):