From 2fbd3f17060cdb32e6dd400fd4ea9d9c96af9d47 Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Wed, 9 Sep 2020 21:48:08 -0400 Subject: [PATCH] (core) Fix lookups in default formulas Test Plan: TODO Reviewers: paulfitz Reviewed By: paulfitz Differential Revision: https://phab.getgrist.com/D2603 --- sandbox/grist/docmodel.py | 3 +- sandbox/grist/engine.py | 2 +- sandbox/grist/functions/date.py | 7 +- sandbox/grist/test_default_formulas.py | 129 +++++++++++++++++++++++++ 4 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 sandbox/grist/test_default_formulas.py diff --git a/sandbox/grist/docmodel.py b/sandbox/grist/docmodel.py index 639c801e..4ddc6900 100644 --- a/sandbox/grist/docmodel.py +++ b/sandbox/grist/docmodel.py @@ -181,8 +181,7 @@ class DocModel(object): def __init__(self, engine): self._engine = engine global global_docmodel # pylint: disable=global-statement - if not global_docmodel: - global_docmodel = self + global_docmodel = self # Set of records scheduled for automatic removal. self._auto_remove_set = set() diff --git a/sandbox/grist/engine.py b/sandbox/grist/engine.py index 9065e152..9204298d 100644 --- a/sandbox/grist/engine.py +++ b/sandbox/grist/engine.py @@ -467,7 +467,7 @@ class Engine(object): """ Returns the compute frame currently being computed, or None if there isn't one. """ - return self._compute_stack[-1] if self._compute_stack else None + return self._compute_stack[-1] if self._compute_stack and self._compute_stack[-1].node else None def _use_node(self, node, relation, row_ids=[]): # This is used whenever a formula accesses any part of any record. It's hot code, and diff --git a/sandbox/grist/functions/date.py b/sandbox/grist/functions/date.py index 7262c09c..bcdd73fc 100644 --- a/sandbox/grist/functions/date.py +++ b/sandbox/grist/functions/date.py @@ -22,9 +22,10 @@ def _make_datetime(value): raise ValueError('Invalid date %r' % (value,)) def _get_global_tz(): - if docmodel.global_docmodel: - return docmodel.global_docmodel.doc_info.lookupOne(id=1).tzinfo - return moment.TZ_UTC + # If doc_info record is missing (e.g. in tests), default to UTC. We should not return None, + # since that would produce naive datetime objects, which is not what we want. + dm = docmodel.global_docmodel + return (dm.doc_info.lookupOne(id=1).tzinfo if dm else None) or moment.TZ_UTC def _get_tzinfo(zonelabel): """ diff --git a/sandbox/grist/test_default_formulas.py b/sandbox/grist/test_default_formulas.py new file mode 100644 index 00000000..497a2340 --- /dev/null +++ b/sandbox/grist/test_default_formulas.py @@ -0,0 +1,129 @@ +import time +import logger +import testutil +import test_engine + +log = logger.Logger(__name__, logger.INFO) + +class TestDefaultFormulas(test_engine.EngineTestCase): + sample = testutil.parse_test_sample({ + "SCHEMA": [ + [1, "Customers", [ + [1, "Name", "Text", False, "", "", ""], + [2, "Region", "Ref:Regions", False, "", "", ""], + [3, "RegName", "Text", True, "$Region.Region", "", ""], + [4, "SalesRep", "Text", False, "$Region.Rep", "", ""], + [5, "CID", "Int", False, "$id + 1000", "", ""], + ]], + [2, "Regions", [ + [11, "Region", "Text", False, "", "", ""], + [12, "Rep", "Text", False, "", "", ""] + ]], + ], + "DATA": { + "Customers": [ + ["id","Name", "Region", "SalesRep", "CID"], + [1, "Dolphin", 2, "Neptune", 0 ], + ], + "Regions": [ + ["id", "Region", "Rep"], + [1, "Pacific", "Watatsumi"], + [2, "Atlantic", "Poseidon"], + [3, "Indian", "Neptune"], + [4, "Arctic", "Poseidon"], + ], + } + }) + + def test_default_formula_plain(self): + self.load_sample(self.sample) + + # The defaults don't affect data that's loaded + self.assertTableData("Customers", data=[ + ["id","Name", "Region", "RegName", "SalesRep", "CID" ], + [1, "Dolphin", 2, "Atlantic", "Neptune", 0], + ]) + + # Defaults affect new records + self.add_record("Customers", Name="Shark", Region=2) + self.add_record("Customers", Name="Squid", Region=1) + self.assertTableData("Customers", data=[ + ["id","Name", "Region", "RegName", "SalesRep", "CID"], + [1, "Dolphin", 2, "Atlantic", "Neptune", 0], + [2, "Shark", 2, "Atlantic", "Poseidon", 1002], # New record + [3, "Squid", 1, "Pacific", "Watatsumi",1003], # New record + ]) + + # Changed defaults don't affect previously-added records + self.modify_column('Customers', 'CID', formula='$id + 2000') + self.add_record("Customers", Name="Hammerhead", Region=3) + self.assertTableData("Customers", data=[ + ["id","Name", "Region", "RegName", "SalesRep", "CID"], + [1, "Dolphin", 2, "Atlantic", "Neptune", 0], + [2, "Shark", 2, "Atlantic", "Poseidon", 1002], + [3, "Squid", 1, "Pacific", "Watatsumi",1003], + [4, "Hammerhead", 3, "Indian", "Neptune", 2004], # New record + ]) + + # Defaults don't affect changes to existing records + self.update_record("Customers", 2, Region=3) + self.assertTableData("Customers", data=[ + ["id","Name", "Region", "RegName", "SalesRep", "CID"], + [1, "Dolphin", 2, "Atlantic", "Neptune", 0], + [2, "Shark", 3, "Indian", "Poseidon", 1002], # Region changed + [3, "Squid", 1, "Pacific", "Watatsumi",1003], + [4, "Hammerhead", 3, "Indian", "Neptune", 2004], + ]) + + + def test_default_formula_with_lookups(self): + self.load_sample(self.sample) + self.modify_column('Customers', 'RegName', isFormula=False, formula="") + self.modify_column('Customers', 'Region', isFormula=False, + formula="Regions.lookupOne(Region=$RegName)") + self.assertTableData("Customers", data=[ + ["id","Name", "Region", "RegName", "SalesRep", "CID" ], + [1, "Dolphin", 2, "Atlantic", "Neptune", 0], + ]) + + # Lookup-based defaults work. + self.add_record("Customers", Name="Shark", RegName="Atlantic") + self.add_record("Customers", Name="Squid", RegName="Pacific") + self.assertTableData("Customers", data=[ + ["id","Name", "Region", "RegName", "SalesRep", "CID"], + [1, "Dolphin", 2, "Atlantic", "Neptune", 0], + [2, "Shark", 2, "Atlantic", "Poseidon", 1002], # New record + [3, "Squid", 1, "Pacific", "Watatsumi",1003], # New record + ]) + + def test_time_defaults(self): + self.load_sample(self.sample) + self.add_column('Customers', 'AddTime', + type="DateTime:America/Los_Angeles", isFormula=False, formula="NOW()") + self.add_column('Customers', 'AddDate', + type="Date", isFormula=False, formula="TODAY()") + + self.assertTableData("Customers", data=[ + ["id","Name", "Region", "RegName", "SalesRep", "CID", "AddTime", "AddDate" ], + [1, "Dolphin", 2, "Atlantic", "Neptune", 0, None, None ], + ]) + self.add_record("Customers", Name="Shark", Region=2) + self.add_record("Customers", Name="Squid", Region=1) + + now = time.time() + midnight = now - (now % (24*60*60)) + + # Check columns except AddTime, which we check separately below. + self.assertTableData("Customers", cols="subset", data=[ + ["id","Name", "Region", "RegName", "SalesRep", "CID", "AddDate"], + [1, "Dolphin", 2, "Atlantic", "Neptune", 0, None], + [2, "Shark", 2, "Atlantic", "Poseidon", 1002, midnight], # New record + [3, "Squid", 1, "Pacific", "Watatsumi",1003, midnight], # New record + ]) + + # AddTime column is hard to be precise about, check it separately. Note that the timestamp + # does not depend on timezone, and should not change based on the timezone in the column type. + observed_data = self.engine.fetch_table('Customers') + self.assertEqual(observed_data.columns['AddTime'][0], None) + self.assertLessEqual(abs(observed_data.columns['AddTime'][1] - now), 2) + self.assertLessEqual(abs(observed_data.columns['AddTime'][2] - now), 2)