From 632620544c43a66a6341097bdbdad19fff23397c Mon Sep 17 00:00:00 2001 From: Leslie H <142967379+SleepyLeslie@users.noreply.github.com> Date: Fri, 12 Jul 2024 10:58:49 -0400 Subject: [PATCH] Update dropdown conditions on column rename (#1038) Automatically update dropdown condition formulas on Ref, RefList, Choice and ChoiceList columns when a column referred to has been renamed. Also fixed column references in ACL formulas using the "$" notation not being properly renamed. --- sandbox/grist/acl.py | 109 +++++++----- sandbox/grist/acl_formula.py | 50 ------ sandbox/grist/codebuilder.py | 9 +- sandbox/grist/dropdown_condition.py | 69 +++++++- sandbox/grist/predicate_formula.py | 59 ++++++- sandbox/grist/test_acl_renames.py | 18 +- .../grist/test_dropdown_condition_renames.py | 158 ++++++++++++++++++ sandbox/grist/test_predicate_formula.py | 12 +- sandbox/grist/useractions.py | 21 ++- sandbox/grist/usertypes.py | 7 + 10 files changed, 395 insertions(+), 117 deletions(-) delete mode 100644 sandbox/grist/acl_formula.py create mode 100644 sandbox/grist/test_dropdown_condition_renames.py diff --git a/sandbox/grist/acl.py b/sandbox/grist/acl.py index b3c46136f5..278bb92249 100644 --- a/sandbox/grist/acl.py +++ b/sandbox/grist/acl.py @@ -5,10 +5,9 @@ import json import logging -from acl_formula import parse_acl_grist_entities -from predicate_formula import parse_predicate_formula_json import action_obj -import textbuilder +import predicate_formula +from predicate_formula import NamedEntity, parse_predicate_formula_json, TreeConverter log = logging.getLogger(__name__) @@ -32,6 +31,40 @@ class Permissions(object): ALL_SET = frozenset([ALL]) +def parse_acl_formulas(col_values): + """ + Populates `aclFormulaParsed` by parsing `aclFormula` for all `col_values`. + """ + if 'aclFormula' not in col_values: + return + + col_values['aclFormulaParsed'] = [parse_predicate_formula_json(v) + for v + in col_values['aclFormula']] + + +class _ACLEntityCollector(TreeConverter): + def __init__(self): + self.entities = [] # NamedEntity list + + def visit_Attribute(self, node): + parent = self.visit(node.value) + + # We recognize a couple of specific patterns for entities that may be affected by renames. + if parent == ['Name', 'rec'] or parent == ['Name', 'newRec']: + # rec.COL refers to the column from the table that the rule is on. + self.entities.append(NamedEntity('recCol', node.last_token.startpos, node.attr, None)) + elif parent == ['Name', 'user']: + # user.ATTR is a user attribute. + self.entities.append(NamedEntity('userAttr', node.last_token.startpos, node.attr, None)) + elif parent[0] == 'Attr' and parent[1] == ['Name', 'user']: + # user.ATTR.COL is a column from the lookup table of the UserAttribute ATTR. + self.entities.append( + NamedEntity('userAttrCol', node.last_token.startpos, node.attr, parent[2])) + + return ["Attr", parent, node.attr] + + def acl_read_split(action_group): """ Returns an ActionBundle containing actions from the given action_group, all in one envelope. @@ -48,20 +81,20 @@ def acl_read_split(action_group): return bundle -def prepare_acl_table_renames(docmodel, useractions, table_renames_dict): +def prepare_acl_table_renames(useractions, table_renames_dict): """ Given a dict of table renames of the form {table_id: new_table_id}, returns a callback that will apply updates to the affected ACL rules and resources. """ # If there are ACLResources that refer to the renamed table, prepare updates for those. resource_updates = [] - for resource_rec in docmodel.aclResources.all: + for resource_rec in useractions.get_docmodel().aclResources.all: if resource_rec.tableId in table_renames_dict: resource_updates.append((resource_rec, {'tableId': table_renames_dict[resource_rec.tableId]})) # Collect updates for any ACLRules with UserAttributes that refer to the renamed table. rule_updates = [] - for rule_rec in docmodel.aclRules.all: + for rule_rec in useractions.get_docmodel().aclRules.all: if rule_rec.userAttributes: try: rule_info = json.loads(rule_rec.userAttributes) @@ -77,14 +110,14 @@ def do_renames(): return do_renames -def prepare_acl_col_renames(docmodel, useractions, col_renames_dict): +def perform_acl_rule_renames(useractions, col_renames_dict): """ Given a dict of column renames of the form {(table_id, col_id): new_col_id}, returns a callback that will apply updates to the affected ACL rules and resources. """ # Collect updates for ACLResources that refer to the renamed columns. resource_updates = [] - for resource_rec in docmodel.aclResources.all: + for resource_rec in useractions.get_docmodel().aclResources.all: t = resource_rec.tableId if resource_rec.colIds and resource_rec.colIds != '*': new_col_ids = ','.join((col_renames_dict.get((t, c)) or c) @@ -95,7 +128,7 @@ def prepare_acl_col_renames(docmodel, useractions, col_renames_dict): # Collect updates for any ACLRules with UserAttributes that refer to the renamed column. rule_updates = [] user_attr_tables = {} # Maps name of user attribute to its lookup table - for rule_rec in docmodel.aclRules.all: + for rule_rec in useractions.get_docmodel().aclRules.all: if rule_rec.userAttributes: try: rule_info = json.loads(rule_rec.userAttributes) @@ -107,33 +140,33 @@ def prepare_acl_col_renames(docmodel, useractions, col_renames_dict): except Exception as e: log.warning("Error examining aclRule: %s", e) + acl_resources_table = useractions.get_docmodel().aclResources.table # Go through again checking if anything in ACL formulas is affected by the rename. - for rule_rec in docmodel.aclRules.all: - if rule_rec.aclFormula: - formula = rule_rec.aclFormula - patches = [] - - for entity in parse_acl_grist_entities(rule_rec.aclFormula): - if entity.type == 'recCol': - table_id = docmodel.aclResources.table.get_record(int(rule_rec.resource)).tableId - elif entity.type == 'userAttrCol': - table_id = user_attr_tables.get(entity.extra) - else: - continue - col_id = entity.name - new_col_id = col_renames_dict.get((table_id, col_id)) - if not new_col_id: - continue - patch = textbuilder.make_patch( - formula, entity.start_pos, entity.start_pos + len(entity.name), new_col_id) - patches.append(patch) - - replacer = textbuilder.Replacer(textbuilder.Text(formula), patches) - txt = replacer.get_text() - rule_updates.append((rule_rec, {'aclFormula': txt, - 'aclFormulaParsed': parse_predicate_formula_json(txt)})) - - def do_renames(): - useractions.doBulkUpdateFromPairs('_grist_ACLResources', resource_updates) - useractions.doBulkUpdateFromPairs('_grist_ACLRules', rule_updates) - return do_renames + for rule_rec in useractions.get_docmodel().aclRules.all: + + if not rule_rec.aclFormula: + continue + acl_formula = rule_rec.aclFormula + + def renamer(subject): + if subject.type == 'recCol': + table_id = acl_resources_table.get_record(int(rule_rec.resource)).tableId + elif subject.type == 'userAttrCol': + table_id = user_attr_tables.get(subject.extra) + else: + return None + col_id = subject.name + return col_renames_dict.get((table_id, col_id)) + + new_acl_formula = predicate_formula.process_renames(acl_formula, _ACLEntityCollector(), renamer) + # No need to check for syntax errors, but this "if" statement must be present. + # See perform_dropdown_condition_renames for more info. + if new_acl_formula != acl_formula: + new_rule_record = { + "aclFormula": new_acl_formula, + "aclFormulaParsed": parse_predicate_formula_json(new_acl_formula) + } + rule_updates.append((rule_rec, new_rule_record)) + + useractions.doBulkUpdateFromPairs('_grist_ACLResources', resource_updates) + useractions.doBulkUpdateFromPairs('_grist_ACLRules', rule_updates) diff --git a/sandbox/grist/acl_formula.py b/sandbox/grist/acl_formula.py deleted file mode 100644 index 19a1d63b4a..0000000000 --- a/sandbox/grist/acl_formula.py +++ /dev/null @@ -1,50 +0,0 @@ -import ast - -import asttokens - -from predicate_formula import NamedEntity, parse_predicate_formula_json, TreeConverter - -def parse_acl_formulas(col_values): - """ - Populates `aclFormulaParsed` by parsing `aclFormula` for all `col_values`. - """ - if 'aclFormula' not in col_values: - return - - col_values['aclFormulaParsed'] = [parse_predicate_formula_json(v) - for v - in col_values['aclFormula']] - -def parse_acl_grist_entities(acl_formula): - """ - Parse the ACL formula collecting any entities that may be subject to renaming. Returns a - NamedEntity list. - """ - try: - atok = asttokens.ASTTokens(acl_formula, tree=ast.parse(acl_formula, mode='eval')) - converter = _EntityCollector() - converter.visit(atok.tree) - return converter.entities - except SyntaxError as err: - return [] - -class _EntityCollector(TreeConverter): - def __init__(self): - self.entities = [] # NamedEntity list - - def visit_Attribute(self, node): - parent = self.visit(node.value) - - # We recognize a couple of specific patterns for entities that may be affected by renames. - if parent == ['Name', 'rec'] or parent == ['Name', 'newRec']: - # rec.COL refers to the column from the table that the rule is on. - self.entities.append(NamedEntity('recCol', node.last_token.startpos, node.attr, None)) - if parent == ['Name', 'user']: - # user.ATTR is a user attribute. - self.entities.append(NamedEntity('userAttr', node.last_token.startpos, node.attr, None)) - elif parent[0] == 'Attr' and parent[1] == ['Name', 'user']: - # user.ATTR.COL is a column from the lookup table of the UserAttribute ATTR. - self.entities.append( - NamedEntity('userAttrCol', node.last_token.startpos, node.attr, parent[2])) - - return ["Attr", parent, node.attr] diff --git a/sandbox/grist/codebuilder.py b/sandbox/grist/codebuilder.py index f65022bc72..6f6085017a 100644 --- a/sandbox/grist/codebuilder.py +++ b/sandbox/grist/codebuilder.py @@ -132,10 +132,11 @@ def make_formula_body(formula, default_value, assoc_value=None): return final_formula -def replace_dollar_attrs(formula): +def get_dollar_replacer(formula): """ - Translates formula "$" expression into rec. expression. This is extracted from the - make_formula_body function. + Returns a textbuilder.Replacer that would replace all dollar signs ("$") in the given + formula with "rec.". The Replacer tracks extra info we can later use to restore the + dollar signs back. To get the processed text, call .get_text() on the Replacer. """ formula_builder_text = textbuilder.Text(formula) tmp_patches = textbuilder.make_regexp_patches(formula, DOLLAR_REGEX, 'DOLLAR') @@ -150,7 +151,7 @@ def replace_dollar_attrs(formula): if m: patches.append(textbuilder.make_patch(formula, m.start(0), m.end(0), 'rec.')) final_formula = textbuilder.Replacer(formula_builder_text, patches) - return final_formula.get_text() + return final_formula def _create_syntax_error_code(builder, input_text, err): diff --git a/sandbox/grist/dropdown_condition.py b/sandbox/grist/dropdown_condition.py index b57ef3f054..c590db71d5 100644 --- a/sandbox/grist/dropdown_condition.py +++ b/sandbox/grist/dropdown_condition.py @@ -1,10 +1,77 @@ import json import logging +import usertypes -from predicate_formula import parse_predicate_formula_json +from predicate_formula import NamedEntity, parse_predicate_formula_json, TreeConverter +import predicate_formula log = logging.getLogger(__name__) +class _DCEntityCollector(TreeConverter): + def __init__(self): + self.entities = [] + + def visit_Attribute(self, node): + parent = self.visit(node.value) + + if parent == ["Name", "choice"]: + self.entities.append(NamedEntity("choiceAttr", node.last_token.startpos, node.attr, None)) + elif parent == ["Name", "rec"]: + self.entities.append(NamedEntity("recCol", node.last_token.startpos, node.attr, None)) + + return ["Attr", parent, node.attr] + + +def perform_dropdown_condition_renames(useractions, renames): + """ + Given a dict of column renames of the form {(table_id, col_id): new_col_id}, applies updates + to the affected dropdown condition formulas. + """ + updates = [] + + for col in useractions.get_docmodel().columns.all: + + # Find all columns in the document that have dropdown conditions. + try: + widget_options = json.loads(col.widgetOptions) + dc_formula = widget_options["dropdownCondition"]["text"] + except (json.JSONDecodeError, KeyError): + continue + + # Find out what table this column refers to and belongs to. + ref_table_id = usertypes.get_referenced_table_id(col.type) + self_table_id = col.parentId.tableId + + def renamer(subject): + # subject.type is either choiceAttr or recCol, see _DCEntityCollector. + table_id = ref_table_id if subject.type == "choiceAttr" else self_table_id + # Dropdown conditions stay in widgetOptions, even when the current column type can't make + # use of them. Thus, attributes of "choice" do not make sense for columns other than Ref and + # RefList, but they may exist. + # We set ref_table_id to None in this case, so table_id will be None for stray choiceAttrs, + # therefore the subject will not be renamed. + # Columns of "rec" are still renamed accordingly. + return renames.get((table_id, subject.name)) + + new_dc_formula = predicate_formula.process_renames(dc_formula, _DCEntityCollector(), renamer) + + # The data engine stops processing remaining formulas when it hits an internal exception during + # this renaming procedure. Parsing could potentially raise SyntaxErrors, so we must be careful + # not to parse a possibly syntactically wrong formula, or handle SyntaxErrors explicitly. + # Note that new_dc_formula was obtained from process_renames, where syntactically wrong formulas + # are left untouched. It is anticipated that rename-induced changes will not introduce new + # SyntaxErrors, so if the formula text is updated, the new version must be valid, hence safe + # to parse without error handling. + # This also serves as an optimization to avoid unnecessary parsing operations. + if new_dc_formula != dc_formula: + widget_options["dropdownCondition"]["text"] = new_dc_formula + widget_options["dropdownCondition"]["parsed"] = parse_predicate_formula_json(new_dc_formula) + updates.append((col, {"widgetOptions": json.dumps(widget_options)})) + + # Update the dropdown condition in the database. + useractions.doBulkUpdateFromPairs('_grist_Tables_column', updates) + + def parse_dropdown_conditions(col_values): """ Parses any unparsed dropdown conditions in `col_values`. diff --git a/sandbox/grist/predicate_formula.py b/sandbox/grist/predicate_formula.py index f421383302..b74bdaacac 100644 --- a/sandbox/grist/predicate_formula.py +++ b/sandbox/grist/predicate_formula.py @@ -2,11 +2,12 @@ import io import json import tokenize +import sys from collections import namedtuple - +import asttokens +import textbuilder import six - -from codebuilder import replace_dollar_attrs +from codebuilder import get_dollar_replacer # Entities encountered in predicate formulas, which may get renamed. # type : 'recCol'|'userAttr'|'userAttrCol', @@ -38,7 +39,7 @@ def parse_predicate_formula(formula): if isinstance(formula, six.binary_type): formula = formula.decode('utf8') try: - formula = replace_dollar_attrs(formula) + formula = get_dollar_replacer(formula).get_text() tree = ast.parse(formula, mode='eval') result = TreeConverter().visit(tree) for part in tokenize.generate_tokens(io.StringIO(formula).readline): @@ -46,9 +47,12 @@ def parse_predicate_formula(formula): result = ['Comment', result, part[1][1:].strip()] break return result - except SyntaxError as err: + except SyntaxError as e: # In case of an error, include line and offset. - raise SyntaxError("%s on line %s col %s" % (err.args[0], err.lineno, err.offset)) + _, _, exc_traceback = sys.exc_info() + six.reraise(SyntaxError, + SyntaxError("%s on line %s col %s" % (e.args[0], e.lineno, e.offset)), + exc_traceback) def parse_predicate_formula_json(formula): """ @@ -63,6 +67,45 @@ def parse_predicate_formula_json(formula): 'None': None, } + +def process_renames(formula, collector, renamer): + """ + Given a predicate formula, a collector and a renamer, rename all references in the formula + that the renamer wants to rename. This is used to automatically update references in an ACL + or dropdown condition formula when a column it refers to has been renamed. + + The collector should be a subclass of TreeConverter that collects related NamedEntity's and + stores them in the field "entities". See acl._ACLEntityCollector for an example. + + The renamer should be a function taking a NamedEntity as its only argument. It should return + a new name for this NamedEntity when it wants to rename this entity, or None otherwise. + """ + patches = [] + # "$" can be used to refer to "rec." in Grist formulas, but it is not valid Python. + # We need to replace it with "rec." before parsing the formula, and restore it back after + # the surgery. + # Keep the dollar replacer object, so that later we know how to restore properly. + dollar_replacer = get_dollar_replacer(formula) + formula_nodollar = dollar_replacer.get_text() + try: + atok = asttokens.ASTTokens(formula_nodollar, tree=ast.parse(formula_nodollar, mode='eval')) + collector.visit(atok.tree) + except SyntaxError: + # Don't do anything to a syntactically wrong formula. + return formula + + for subject in collector.entities: + new_name = renamer(subject) + if new_name is not None: + _, _, patch = dollar_replacer.map_back_patch( + textbuilder.make_patch(dollar_replacer.get_text(), subject.start_pos, + subject.start_pos + len(subject.name), new_name) + ) + patches.append(patch) + + return textbuilder.Replacer(textbuilder.Text(formula), patches).get_text() + + class TreeConverter(ast.NodeVisitor): # AST nodes are documented here: https://docs.python.org/2/library/ast.html#abstract-grammar # pylint:disable=no-self-use @@ -86,7 +129,7 @@ def visit_UnaryOp(self, node): def visit_Compare(self, node): # We don't try to support chained comparisons like "1 < 2 < 3" (though it wouldn't be hard). if len(node.ops) != 1 or len(node.comparators) != 1: - raise ValueError("Can't use chained comparisons") + raise SyntaxError("Can't use chained comparisons") return [node.ops[0].__class__.__name__, self.visit(node.left), self.visit(node.comparators[0])] def visit_Name(self, node): @@ -115,4 +158,4 @@ def visit_Tuple(self, node): return self.visit_List(node) # We don't distinguish tuples and lists def generic_visit(self, node): - raise ValueError("Unsupported syntax at %s:%s" % (node.lineno, node.col_offset + 1)) + raise SyntaxError("Unsupported syntax at %s:%s" % (node.lineno, node.col_offset + 1)) diff --git a/sandbox/grist/test_acl_renames.py b/sandbox/grist/test_acl_renames.py index be342e3929..908694effd 100644 --- a/sandbox/grist/test_acl_renames.py +++ b/sandbox/grist/test_acl_renames.py @@ -40,6 +40,12 @@ def setUp(self): 'aclFormula': '( rec.schoolName != # ünîcødé comment\n user.School.name)', 'permissionsText': 'none', }], + ['AddRecord', '_grist_ACLRules', None, { + 'resource': -2, + # Test whether both "$" and "rec." are preserved while renaming. + 'aclFormula': '( $firstName not in rec.schoolName or $schoolName + $lastName == rec.firstName)', + 'permissionsText': 'all', + }], ['AddRecord', '_grist_ACLRules', None, { 'resource': -3, 'permissionsText': 'all' @@ -57,7 +63,8 @@ def setUp(self): ['id', 'resource', 'aclFormula', 'permissionsText', 'userAttributes'], [1, 1, '', '', json.dumps(user_attr1)], [2, 2, '( rec.schoolName != # ünîcødé comment\n user.School.name)', 'none', ''], - [3, 3, '', 'all', ''], + [3, 2, '( $firstName not in rec.schoolName or $schoolName + $lastName == rec.firstName)', 'all', ''], + [4, 3, '', 'all', ''], ]) def test_acl_table_renames(self): @@ -78,7 +85,8 @@ def test_acl_table_renames(self): ['id', 'resource', 'aclFormula', 'permissionsText', 'userAttributes'], [1, 1, '', '', json.dumps(user_attr1_renamed)], [2, 2, '( rec.schoolName != # ünîcødé comment\n user.School.name)', 'none', ''], - [3, 3, '', 'all', ''], + [3, 2, '( $firstName not in rec.schoolName or $schoolName + $lastName == rec.firstName)', 'all', ''], + [4, 3, '', 'all', ''], ]) def test_acl_column_renames(self): @@ -101,7 +109,8 @@ def test_acl_column_renames(self): ['id', 'resource', 'aclFormula', 'permissionsText', 'userAttributes'], [1, 1, '', '', json.dumps(user_attr1_renamed)], [2, 2, '( rec.escuela != # ünîcødé comment\n user.School.schoolName)', 'none', ''], - [3, 3, '', 'all', ''], + [3, 2, '( $firstName not in rec.escuela or $escuela + $Family_Name == rec.firstName)', 'all', ''], + [4, 3, '', 'all', ''], ]) def test_multiple_renames(self): @@ -123,5 +132,6 @@ def test_multiple_renames(self): ['id', 'resource', 'aclFormula', 'permissionsText', 'userAttributes'], [1, 1, '', '', json.dumps(user_attr1)], [2, 2, '( rec.escuela != # ünîcødé comment\n user.School.schoolName)', 'none', ''], - [3, 3, '', 'all', ''], + [3, 2, '( $Given_Name not in rec.escuela or $escuela + $Family_Name == rec.Given_Name)', 'all', ''], + [4, 3, '', 'all', ''], ]) diff --git a/sandbox/grist/test_dropdown_condition_renames.py b/sandbox/grist/test_dropdown_condition_renames.py new file mode 100644 index 0000000000..26ef29b5bf --- /dev/null +++ b/sandbox/grist/test_dropdown_condition_renames.py @@ -0,0 +1,158 @@ +# -*- coding: utf-8 -*- + +import json + +import test_engine +import testsamples +import useractions + +# A sample dropdown condition formula for the column Schools.address and alike, of type Ref/RefList. +def build_dc1_text(school_name, address_city): + return "'New' in choice.{address_city} and ${school_name} == rec.{school_name} + rec.choice.city or choice.rec.city != $name2".format(**locals()) + +# Another sample formula for a new column of type ChoiceList (or actually, anything other than Ref/RefList). +def build_dc2_text(school_name, school_address): + # We currently don't support layered attribute access, e.g. rec.address.city, so this is not tested. + # choice.city really is nonsense, as choice will not be an object. + # Just for testing purposes, to make sure nothing is renamed here. + return "choice + ${school_name} == choice.city or rec.{school_address} > 2".format(**locals()) + +def build_dc1(school_name, address_city): + return json.dumps({ + "dropdownCondition": { + "text": build_dc1_text(school_name, address_city), + # The ModifyColumn user action should trigger an auto parse. + # "parsed" is stored as dumped JSON, so we need to explicitly dump it here as well. + "parsed": json.dumps(["Or", ["And", ["In", ["Const", "New"], ["Attr", ["Name", "choice"], address_city]], ["Eq", ["Attr", ["Name", "rec"], school_name], ["Add", ["Attr", ["Name", "rec"], school_name], ["Attr", ["Attr", ["Name", "rec"], "choice"], "city"]]]], ["NotEq", ["Attr", ["Attr", ["Name", "choice"], "rec"], "city"], ["Attr", ["Name", "rec"], "name2"]]]) + } + }) + +def build_dc2(school_name, school_address): + return json.dumps({ + "dropdownCondition": { + "text": build_dc2_text(school_name, school_address), + "parsed": json.dumps(["Or", ["Eq", ["Add", ["Name", "choice"], ["Attr", ["Name", "rec"], school_name]], ["Attr", ["Name", "choice"], "city"]], ["Gt", ["Attr", ["Name", "rec"], school_address], ["Const", 2]]]) + } + }) + +class TestDCRenames(test_engine.EngineTestCase): + + def setUp(self): + super(TestDCRenames, self).setUp() + + self.load_sample(testsamples.sample_students) + + self.engine.apply_user_actions([useractions.from_repr(ua) for ua in ( + # Add some irrelevant columns to the table Schools. These should never be renamed. + ["AddColumn", "Schools", "name2", { + "type": "Text" + }], + ["AddColumn", "Schools", "choice", { + "type": "Ref:Address" + }], + ["AddColumn", "Address", "rec", { + "type": "Text" + }], + # Add a dropdown condition formula to Schools.address (column #12). + ["ModifyColumn", "Schools", "address", { + "widgetOptions": json.dumps({ + "dropdownCondition": { + "text": build_dc1_text("name", "city"), + } + }), + }], + # Create a similar column with an invalid dropdown condition formula. + # This formula should never be touched. + # This column will have the ID 25. + ["AddColumn", "Schools", "address2", { + "type": "Ref:Address", + "widgetOptions": json.dumps({ + "dropdownCondition": { + "text": "+ 'New' in choice.city and $name == rec.name", + } + }), + }], + # And another similar column, but of type RefList. + # This column will have the ID 26. + ["AddColumn", "Schools", "addresses", { + "type": "RefList:Address", + }], + # AddColumn will not trigger parsing. We emulate a real user's action here by creating it first, + # then editing its widgetOptions. + ["ModifyColumn", "Schools", "addresses", { + "widgetOptions": json.dumps({ + "dropdownCondition": { + "text": build_dc1_text("name", "city"), + } + }), + }], + # And another similar column, but of type ChoiceList. + # widgetOptions stay when the column type changes. We do our best to rename stuff in stray widgetOptions. + # This column will have the ID 27. + ["AddColumn", "Schools", "features", { + "type": "ChoiceList", + }], + ["ModifyColumn", "Schools", "features", { + "widgetOptions": json.dumps({ + "dropdownCondition": { + "text": build_dc2_text("name", "address"), + } + }), + }], + )]) + + # This is what we'll have at the beginning, for later tests to refer to. + # Table Schools is 2. + self.assertTableData("_grist_Tables_column", cols="subset", rows="subset", data=[ + ["id", "parentId", "colId", "widgetOptions"], + [12, 2, "address", build_dc1("name", "city")], + [26, 2, "addresses", build_dc1("name", "city")], + [27, 2, "features", build_dc2("name", "address")], + ]) + self.assert_invalid_formula_untouched() + + def assert_invalid_formula_untouched(self): + self.assertTableData("_grist_Tables_column", cols="subset", rows="subset", data=[ + ["id", "parentId", "colId", "widgetOptions"], + [25, 2, "address2", json.dumps({ + "dropdownCondition": { + "text": "+ 'New' in choice.city and $name == rec.name", + } + })] + ]) + + def test_referred_column_renames(self): + self.apply_user_action(["RenameColumn", "Address", "city", "area"]) + self.assertTableData("_grist_Tables_column", cols="subset", rows="subset", data=[ + ["id", "parentId", "colId", "widgetOptions"], + [12, 2, "address", build_dc1("name", "area")], + [26, 2, "addresses", build_dc1("name", "area")], + # Nothing should be renamed here, as only column renames in the table "Schools" are relevant. + [27, 2, "features", build_dc2("name", "address")], + ]) + self.assert_invalid_formula_untouched() + + def test_record_column_renames(self): + self.apply_user_action(["RenameColumn", "Schools", "name", "identifier"]) + self.apply_user_action(["RenameColumn", "Schools", "address", "location"]) + self.assertTableData("_grist_Tables_column", cols="subset", rows="subset", data=[ + ["id", "parentId", "colId", "widgetOptions"], + # Side effect: "address" becomes "location". + [12, 2, "location", build_dc1("identifier", "city")], + [26, 2, "addresses", build_dc1("identifier", "city")], + # Now "$name" should become "$identifier", just like in Ref/RefList columns. Nothing else should change. + [27, 2, "features", build_dc2("identifier", "location")], + ]) + self.assert_invalid_formula_untouched() + + def test_multiple_renames(self): + # Put all renames together. + self.apply_user_action(["RenameColumn", "Address", "city", "area"]) + self.apply_user_action(["RenameColumn", "Schools", "name", "identifier"]) + self.assertTableData("_grist_Tables_column", cols="subset", rows="subset", data=[ + ["id", "parentId", "colId", "widgetOptions"], + [12, 2, "address", build_dc1("identifier", "area")], + [26, 2, "addresses", build_dc1("identifier", "area")], + [27, 2, "features", build_dc2("identifier", "address")], + ]) + self.assert_invalid_formula_untouched() diff --git a/sandbox/grist/test_predicate_formula.py b/sandbox/grist/test_predicate_formula.py index 34914c2ee0..4feb0ce155 100644 --- a/sandbox/grist/test_predicate_formula.py +++ b/sandbox/grist/test_predicate_formula.py @@ -139,14 +139,14 @@ def test_unsupported(self): self.assertRaises(SyntaxError, parse_predicate_formula, "def foo(): pass") # Unsupported node type - self.assertRaisesRegex(ValueError, r'Unsupported syntax', parse_predicate_formula, "max(rec)") - self.assertRaisesRegex(ValueError, r'Unsupported syntax', parse_predicate_formula, "user.id in {1, 2, 3}") - self.assertRaisesRegex(ValueError, r'Unsupported syntax', parse_predicate_formula, "1 if user.IsAnon else 2") + self.assertRaisesRegex(SyntaxError, r'Unsupported syntax', parse_predicate_formula, "max(rec)") + self.assertRaisesRegex(SyntaxError, r'Unsupported syntax', parse_predicate_formula, "user.id in {1, 2, 3}") + self.assertRaisesRegex(SyntaxError, r'Unsupported syntax', parse_predicate_formula, "1 if user.IsAnon else 2") # Unsupported operation - self.assertRaisesRegex(ValueError, r'Unsupported syntax', parse_predicate_formula, "1 | 2") - self.assertRaisesRegex(ValueError, r'Unsupported syntax', parse_predicate_formula, "1 << 2") - self.assertRaisesRegex(ValueError, r'Unsupported syntax', parse_predicate_formula, "~test") + self.assertRaisesRegex(SyntaxError, r'Unsupported syntax', parse_predicate_formula, "1 | 2") + self.assertRaisesRegex(SyntaxError, r'Unsupported syntax', parse_predicate_formula, "1 << 2") + self.assertRaisesRegex(SyntaxError, r'Unsupported syntax', parse_predicate_formula, "~test") # Syntax error self.assertRaises(SyntaxError, parse_predicate_formula, "[(]") diff --git a/sandbox/grist/useractions.py b/sandbox/grist/useractions.py index 95f5a49c31..3c01b996f4 100644 --- a/sandbox/grist/useractions.py +++ b/sandbox/grist/useractions.py @@ -12,8 +12,9 @@ import acl import depend import gencode -from acl_formula import parse_acl_formulas +from acl import parse_acl_formulas from dropdown_condition import parse_dropdown_conditions +import dropdown_condition import actions import column import sort_specs @@ -228,6 +229,12 @@ def __init__(self, eng): self._overrides = {key: method.__get__(self, UserActions) for key, method in six.iteritems(_action_method_overrides)} + def get_docmodel(self): + """ + Getter for the docmodel. + """ + return self._docmodel + @contextmanager def indirect_actions(self): """ @@ -635,7 +642,7 @@ def _updateTableRecords(self, table_id, row_ids, col_values): if 'type' in values: self.doModifyColumn(col.tableId, col.colId, {'type': 'Int'}) - make_acl_updates = acl.prepare_acl_table_renames(self._docmodel, self, table_renames) + make_acl_updates = acl.prepare_acl_table_renames(self, table_renames) # Collect all the table renames, and do the actual schema actions to apply them. for tbl, values in update_pairs: @@ -691,6 +698,9 @@ def _updateColumnRecords(self, table_id, row_ids, col_values): if has_diff_value(values, 'colId', c.colId)} if renames: + # When a column rename has occurred, we need to update the corresponding references in + # formula, ACL rules and dropdown conditions. + # Build up a dictionary mapping col_ref of each affected formula to the new formula text. formula_updates = self._prepare_formula_renames(renames) @@ -698,6 +708,9 @@ def _updateColumnRecords(self, table_id, row_ids, col_values): for col_rec, new_formula in sorted(six.iteritems(formula_updates)): col_updates.setdefault(col_rec, {}).setdefault('formula', new_formula) + acl.perform_acl_rule_renames(self, renames) + dropdown_condition.perform_dropdown_condition_renames(self, renames) + update_pairs = col_updates.items() # Disallow most changes to summary group-by columns, except to match the underlying column. @@ -721,8 +734,6 @@ def _updateColumnRecords(self, table_id, row_ids, col_values): if not allowed_summary_change(key, value, expected): raise ValueError("Cannot modify summary group-by column '%s'" % col.colId) - make_acl_updates = acl.prepare_acl_col_renames(self._docmodel, self, renames) - rename_summary_tables = set() for c, values in update_pairs: # Trigger ModifyColumn and RenameColumn as necessary @@ -745,8 +756,6 @@ def _updateColumnRecords(self, table_id, row_ids, col_values): table = self._engine.tables[table_id] self._engine._update_table_model(table, table.user_table) - make_acl_updates() - for table in rename_summary_tables: groupby_col_ids = [c.colId for c in table.columns if c.summarySourceCol] new_table_id = summary.encode_summary_table_name(table.summarySourceTable.tableId, diff --git a/sandbox/grist/usertypes.py b/sandbox/grist/usertypes.py index d617b927f7..d599a22273 100644 --- a/sandbox/grist/usertypes.py +++ b/sandbox/grist/usertypes.py @@ -63,6 +63,13 @@ def wrapper(method): return method return wrapper +def get_referenced_table_id(col_type): + if col_type.startswith("Ref:"): + return col_type[4:] + if col_type.startswith("RefList:"): + return col_type[8:] + return None + def ifError(value, value_if_error): """