gristlabs_grist-core/sandbox/grist/test_derived.py
Alex Hall 56624c4a95 (core) Fix undo error for automatically removed rows, especially in summary tables
Summary:
Fixes a bug noted here: https://grist.slack.com/archives/C069RUP71/p1662564341132349

This bug could happen quite easily as follows:

1. Have a formula in a summary table such as `$group.amount`. Typically there's also a `SUM` but that's not essential.
2. Find a group with nonzero values of `amount`.
3. Delete all rows in that group in the source table. Typically that just means one row in a lonely group.
4. The summary table row is automatically deleted.
5. Try to undo. This raises an error about trying to update a non-existent summary table row.

I tried to account for this undo problem in https://phab.getgrist.com/D3489 by not saving the updated value for `$group` when it was found to be empty. The reason this was insufficient is that `$group.amount` is immediately invalidated anyway when the source row(s) are deleted (I think because that's just how dependency relations involving references work) *and* the calculated value of `$group.amount` changes even if `$group` doesn't. For example, `$group.amount` may have previously been `[100, 200]`. After deleting the rows, `$group.amount` becomes `[0, 0]`. Keeping `$group` unchanged prevents `$group.amount` from just being `[]`, but deleting the source rows means that the amounts become the numeric default `0` which is still a change. This change in value is then noted which leads to saving an undo action to update the summary table record. All this happens in step 3 above, and the summary record is only deleted after that point.

This diff removes that special handling for `group` and instead adds a more general fix to `action_summary.py`. This inserts undo actions for deleted rows at the beginning of the undo list rather than at the end, which was already done for deleted tables and columns.

Test Plan: Python tests

Reviewers: dsagal

Reviewed By: dsagal

Subscribers: dsagal

Differential Revision: https://phab.getgrist.com/D3626
2022-09-09 22:15:45 +02:00

320 lines
13 KiB
Python

import actions
import logger
import testutil
import test_engine
log = logger.Logger(__name__, logger.INFO)
def _bulk_update(table_name, col_names, row_data):
return actions.BulkUpdateRecord(
*testutil.table_data_from_rows(table_name, col_names, row_data))
class TestDerived(test_engine.EngineTestCase):
sample = testutil.parse_test_sample({
"SCHEMA": [
[1, "Customers", [
[1, "firstName", "Text", False, "", "", ""],
[2, "lastName", "Text", False, "", "", ""],
[3, "state", "Text", False, "", "", ""],
]],
[2, "Orders", [
[10, "year", "Int", False, "", "", ""],
[11, "customer", "Ref:Customers", False, "", "", ""],
[12, "product", "Text", False, "", "", ""],
[13, "amount", "Numeric", False, "", "", ""],
]],
],
"DATA": {
"Customers": [
["id", "firstName", "lastName", "state"],
[1, "Lois", "Long", "NY"],
[2, "Felix", "Myers", "NY"],
[3, "Grace", "Hawkins", "CT"],
[4, "Bessie", "Green", "NJ"],
[5, "Jerome", "Daniel", "CT"],
],
"Orders": [
["id", "year", "customer", "product", "amount" ],
[1, 2012, 3, "A", 15 ],
[2, 2013, 2, "A", 15 ],
[3, 2013, 3, "A", 15 ],
[4, 2014, 1, "B", 35 ],
[5, 2014, 5, "B", 35 ],
[6, 2014, 3, "A", 16 ],
[7, 2015, 1, "A", 17 ],
[8, 2015, 2, "B", 36 ],
[9, 2015, 3, "B", 36 ],
[10, 2015, 5, "A", 17 ],
]
}
})
def test_group_by_one(self):
"""
Test basic summary table operation, for a table grouped by one columns.
"""
self.load_sample(self.sample)
# Create a derived table summarizing count and total of orders by year.
self.apply_user_action(["CreateViewSection", 2, 0, 'record', [10], None])
# Check the results.
self.assertPartialData("Orders_summary_year", ["id", "year", "count", "amount", "group" ], [
[1, 2012, 1, 15, [1]],
[2, 2013, 2, 30, [2,3]],
[3, 2014, 3, 86, [4,5,6]],
[4, 2015, 4, 106, [7,8,9,10]],
])
# Updating amounts should cause totals to be updated in the summary.
out_actions = self.update_records("Orders", ["id", "amount"], [
[1, 14],
[2, 14]
])
self.assertPartialOutActions(out_actions, {
"stored": [
actions.BulkUpdateRecord("Orders", [1,2], {'amount': [14, 14]}),
actions.BulkUpdateRecord("Orders_summary_year", [1,2], {'amount': [14, 29]})
],
"calls": {"Orders_summary_year": {"amount": 2}}
})
# Changing a record from one product to another should cause the two affected lines to change.
out_actions = self.update_record("Orders", 10, year=2012)
self.assertPartialOutActions(out_actions, {
"stored": [
actions.UpdateRecord("Orders", 10, {"year": 2012}),
actions.BulkUpdateRecord("Orders_summary_year", [1,4], {"amount": [31.0, 89.0]}),
actions.BulkUpdateRecord("Orders_summary_year", [1,4], {"count": [2,3]}),
actions.BulkUpdateRecord("Orders_summary_year", [1,4], {"group": [[1,10], [7,8,9]]}),
],
"calls": {"Orders_summary_year": {"group": 2, "amount": 2, "count": 2},
"Orders": {"#lookup##summary#Orders_summary_year": 1,
"#summary#Orders_summary_year": 1}}
})
self.assertPartialData("Orders_summary_year", ["id", "year", "count", "amount", "group" ], [
[1, 2012, 2, 31.0, [1,10]],
[2, 2013, 2, 29.0, [2,3]],
[3, 2014, 3, 86.0, [4,5,6]],
[4, 2015, 3, 89.0, [7,8,9]],
])
# Changing a record to a new year that wasn't in the summary should cause an add-record.
out_actions = self.update_record("Orders", 10, year=1999)
self.assertPartialOutActions(out_actions, {
"stored": [
actions.UpdateRecord("Orders", 10, {"year": 1999}),
actions.AddRecord("Orders_summary_year", 5, {'year': 1999}),
actions.BulkUpdateRecord("Orders_summary_year", [1,5], {"amount": [14.0, 17.0]}),
actions.BulkUpdateRecord("Orders_summary_year", [1,5], {"count": [1,1]}),
actions.BulkUpdateRecord("Orders_summary_year", [1,5], {"group": [[1], [10]]}),
],
"calls": {
"Orders_summary_year": {
'#lookup#year': 1, "group": 2, "amount": 2, "count": 2, "#lookup#": 1
},
"Orders": {"#lookup##summary#Orders_summary_year": 1,
"#summary#Orders_summary_year": 1}}
})
self.assertPartialData("Orders_summary_year", ["id", "year", "count", "amount", "group" ], [
[1, 2012, 1, 14.0, [1]],
[2, 2013, 2, 29.0, [2,3]],
[3, 2014, 3, 86.0, [4,5,6]],
[4, 2015, 3, 89.0, [7,8,9]],
[5, 1999, 1, 17.0, [10]],
])
def test_group_by_two(self):
"""
Test a summary table created by grouping on two columns.
"""
self.load_sample(self.sample)
self.apply_user_action(["CreateViewSection", 2, 0, 'record', [10, 12], None])
self.assertPartialData("Orders_summary_product_year", [
"id", "year", "product", "count", "amount", "group"
], [
[1, 2012, "A", 1, 15.0, [1]],
[2, 2013, "A", 2, 30.0, [2,3]],
[3, 2014, "B", 2, 70.0, [4,5]],
[4, 2014, "A", 1, 16.0, [6]],
[5, 2015, "A", 2, 34.0, [7,10]],
[6, 2015, "B", 2, 72.0, [8,9]],
])
# Changing a record from one product to another should cause the two affected lines to change,
# or new lines to be created as needed.
out_actions = self.update_records("Orders", ["id", "product"], [
[2, "B"],
[6, "B"],
[7, "C"],
])
self.assertPartialOutActions(out_actions, {
"stored": [
actions.BulkUpdateRecord("Orders", [2, 6, 7], {"product": ["B", "B", "C"]}),
actions.AddRecord("Orders_summary_product_year", 7, {'year': 2013, 'product': 'B'}),
actions.AddRecord("Orders_summary_product_year", 8, {'year': 2015, 'product': 'C'}),
actions.RemoveRecord("Orders_summary_product_year", 4),
actions.BulkUpdateRecord("Orders_summary_product_year", [2,3,5,7,8], {
"amount": [15.0, 86.0, 17.0, 15.0, 17.0]
}),
actions.BulkUpdateRecord("Orders_summary_product_year", [2,3,5,7,8], {
"count": [1, 3, 1, 1, 1]
}),
actions.BulkUpdateRecord("Orders_summary_product_year", [2,3,5,7,8], {
"group": [[3], [4,5,6], [10], [2], [7]]
}),
],
})
# Verify the results.
self.assertPartialData("Orders_summary_product_year", [
"id", "year", "product", "count", "amount", "group"
], [
[1, 2012, "A", 1, 15.0, [1]],
[2, 2013, "A", 1, 15.0, [3]],
[3, 2014, "B", 3, 86.0, [4,5,6]],
[5, 2015, "A", 1, 17.0, [10]],
[6, 2015, "B", 2, 72.0, [8,9]],
[7, 2013, "B", 1, 15.0, [2]],
[8, 2015, "C", 1, 17.0, [7]],
])
def test_group_with_references(self):
"""
Test summary tables grouped on indirect values. In this example we want for each
customer.state, the number of customers and the total of their orders, which we can do either
as a summary on the Customers table, or a summary on the Orders table.
"""
self.load_sample(self.sample)
# Create a summary on the Customers table. Adding orders involves a lookup for each customer.
self.apply_user_action(["CreateViewSection", 1, 0, 'record', [3], None])
self.add_column("Customers_summary_state", "totalAmount",
formula="sum(sum(Orders.lookupRecords(customer=c).amount) for c in $group)")
self.assertPartialData("Customers_summary_state", ["id", "state", "count", "totalAmount"], [
[1, "NY", 2, 103.0 ],
[2, "CT", 2, 134.0 ],
[3, "NJ", 1, 0.0 ],
])
# # Create the same summary on the Orders table, looking up 'state' via the Customer reference.
# self.apply_user_action(["AddDerivedTableSource", "Summary4", "Orders",
# {"state": "$customer.state"}])
# self.add_column("Summary4", "numCustomers", formula="len(set($source_Orders.customer))")
# self.add_column("Summary4", "totalAmount", formula="sum($source_Orders.amount)")
# self.assertPartialData("Summary4", ["id", "state", "numCustomers", "totalAmount"], [
# [1, "CT", 2, 134.0 ],
# [2, "NY", 2, 103.0 ],
# ])
# In either case, changing an amount (from 36->37 for a CT customer) should update summaries.
out_actions = self.update_record('Orders', 9, amount=37)
self.assertPartialOutActions(out_actions, {
"stored": [
actions.UpdateRecord("Orders", 9, {"amount": 37}),
actions.UpdateRecord("Customers_summary_state", 2, {"totalAmount": 135.0}),
]
})
# In either case, changing a customer's state should trigger recomputation too.
# We are changing a NY customer with $51 in orders to MA.
self.update_record('Customers', 2, state="MA")
self.assertPartialData("Customers_summary_state", ["id", "state", "count", "totalAmount"], [
[1, "NY", 1, 52.0 ],
[2, "CT", 2, 135.0 ],
[3, "NJ", 1, 0.0 ],
[4, "MA", 1, 51.0 ],
])
# self.assertPartialData("Summary4", ["id", "state", "numCustomers", "totalAmount"], [
# [1, "CT", 2, 135.0 ],
# [2, "NY", 1, 52.0 ],
# [3, "MA", 1, 51.0 ],
# ])
# Similarly, changing an Order to refer to a different customer should update both tables.
# Here we are changing a $17 order (#7) for a NY customer (#1) to a NJ customer (#4).
out_actions = self.update_record("Orders", 7, customer=4)
# self.assertPartialOutActions(out_actions, {
# "stored": [actions.UpdateRecord("Orders", 7, {"customer": 4}),
# actions.AddRecord("Summary4", 4, {"state": "NJ"}),
# actions.UpdateRecord("Summary4", 4, {"manualSort": 4.0})]
# })
self.assertPartialData("Customers_summary_state", ["id", "state", "count", "totalAmount"], [
[1, "NY", 1, 35.0 ],
[2, "CT", 2, 135.0 ],
[3, "NJ", 1, 17.0 ],
[4, "MA", 1, 51.0 ],
])
# self.assertPartialData("Summary4", ["id", "state", "numCustomers", "totalAmount"], [
# [1, "CT", 2, 135.0 ],
# [2, "NY", 1, 35.0 ],
# [3, "MA", 1, 51.0 ],
# [4, "NJ", 1, 17.0 ],
# ])
def test_deletions(self):
self.load_sample(self.sample)
# Create a summary table summarizing count and total of orders by year.
self.apply_user_action(["CreateViewSection", 2, 0, 'record', [10], None])
self.assertPartialData("Orders_summary_year", ["id", "year", "count", "amount", "group" ], [
[1, 2012, 1, 15.0, [1]],
[2, 2013, 2, 30.0, [2,3]],
[3, 2014, 3, 86.0, [4,5,6]],
[4, 2015, 4, 106.0, [7,8,9,10]],
])
# Update a record so that a new line appears in the summary table.
out_actions_update = self.update_record("Orders", 1, year=2007)
self.assertPartialData("Orders_summary_year", ["id", "year", "count", "amount", "group" ], [
[2, 2013, 2, 30.0, [2,3]],
[3, 2014, 3, 86.0, [4,5,6]],
[4, 2015, 4, 106.0, [7,8,9,10]],
[5, 2007, 1, 15.0, [1]],
])
self.assertPartialOutActions(out_actions_update, {
'stored': [
['UpdateRecord', 'Orders', 1, {'year': 2007}],
['AddRecord', 'Orders_summary_year', 5, {'year': 2007}],
['RemoveRecord', 'Orders_summary_year', 1],
['UpdateRecord', 'Orders_summary_year', 5, {'amount': 15.0}],
['UpdateRecord', 'Orders_summary_year', 5, {'count': 1}],
['UpdateRecord', 'Orders_summary_year', 5, {'group': ['L', 1]}],
],
'undo': [
['UpdateRecord', 'Orders_summary_year', 1, {'group': ['L', 1]}],
['UpdateRecord', 'Orders_summary_year', 1, {'count': 1}],
['UpdateRecord', 'Orders_summary_year', 1, {'amount': 15.0}],
['UpdateRecord', 'Orders', 1, {'year': 2012}],
['RemoveRecord', 'Orders_summary_year', 5],
['AddRecord', 'Orders_summary_year', 1, {'group': ['L'], 'year': 2012}],
]})
# Undo and ensure that the new line is gone from the summary table.
out_actions_undo = self.apply_undo_actions(out_actions_update.undo)
self.assertPartialData("Orders_summary_year", ["id", "year", "count", "amount", "group" ], [
[1, 2012, 1, 15.0, [1]],
[2, 2013, 2, 30.0, [2,3]],
[3, 2014, 3, 86.0, [4,5,6]],
[4, 2015, 4, 106.0, [7,8,9,10]],
])
self.assertPartialOutActions(out_actions_undo, {
"stored": out_actions_update.undo[::-1],
"calls": {
"Orders_summary_year": {
"#lookup#": 1, "#lookup#year": 1, "group": 1, "amount": 1, "count": 1
},
"Orders": {
"#lookup##summary#Orders_summary_year": 1, "#summary#Orders_summary_year": 1,
},
},
})