diff --git a/sandbox/grist/summary.py b/sandbox/grist/summary.py index 18c1b604..7650def6 100644 --- a/sandbox/grist/summary.py +++ b/sandbox/grist/summary.py @@ -197,27 +197,26 @@ class SummaryActions(object): def update_summary_section(self, view_section, source_table, source_groupby_columns): source_groupby_colset = set(source_groupby_columns) groupby_colids = {c.colId for c in source_groupby_columns} - prev_fields = list(view_section.fields) - # Go through fields figuring out which ones we'll keep. - prev_group_fields, formula_fields, delete_fields, missing_colinfo = [], [], [], [] - for field in prev_fields: - srcCol = field.colRef.summarySourceCol + # Go through columns figuring out which ones we'll keep. + prev_group_cols, formula_colinfo = [], [] + for col in view_section.tableRef.columns: + srcCol = col.summarySourceCol # Records implement __hash__, so we can look them up in sets. if srcCol in source_groupby_colset: - prev_group_fields.append(field) - elif field.colRef.isFormula and field.colRef.colId not in groupby_colids: - formula_fields.append(field) + prev_group_cols.append(col) + elif col.isFormula and col.colId not in groupby_colids: + formula_colinfo.append(_make_col_info(col)) else: # if user is removing a numeric column from the group by columns we must add it back as a # sum formula column - self._append_sister_column_if_any(missing_colinfo, source_table, srcCol) - delete_fields.append(field) + self._append_sister_column_if_any(formula_colinfo, source_table, srcCol) - # Prepare ColInfo for all columns we want to keep. - formula_colinfo = [_make_col_info(f.colRef) for f in formula_fields] + # All fields with a column that we don't keep, must be deleted + colid_keep_set = set(c.colId for c in prev_group_cols + formula_colinfo) + delete_fields = [f for f in view_section.fields if f.colRef.colId not in colid_keep_set] - have_group_col = any(f.colRef.colId == 'group' for f in formula_fields) + have_group_col = any(ci.colId == 'group' for ci in formula_colinfo) if not have_group_col: formula_colinfo.append(_group_colinfo(source_table)) @@ -242,19 +241,18 @@ class SummaryActions(object): # Delete fields no longer relevant. self.docmodel.remove(delete_fields) - # Add missing sum column - for ci in missing_colinfo: - col = self.useractions.AddColumn(summary_table.tableId, ci.colId, - _get_colinfo_dict(ci, with_id=False)) - # AddColumn user action did not add the fields as the view section was not yet updated with - # new table, hence adds it manually - self.docmodel.add(view_section.fields, colRef=[col['colRef']]) - # Update fields for all formula fields and reused group-by fields to point to new columns. + colid_to_field_map = {field.colRef.colId: field for field in view_section.fields} + prev_group_fields = [ + colid_to_field_map[col.colId] for col in prev_group_cols + if col.colId in colid_to_field_map + ] source_col_map = dict(zip(source_groupby_columns, groupby_columns)) prev_group_columns = [source_col_map[f.colRef.summarySourceCol] for f in prev_group_fields] + visible_formula_columns = [c for c in formula_columns if c.colId in colid_to_field_map] + formula_fields = [colid_to_field_map[c.colId] for c in visible_formula_columns] self.docmodel.update(formula_fields + prev_group_fields, - colRef=[c.id for c in formula_columns + prev_group_columns]) + colRef=[c.id for c in visible_formula_columns + prev_group_columns]) # Finally, we need to create fields for newly-added group-by columns. If there were missing # fields for any group-by columns before, they'll be created now. diff --git a/sandbox/grist/test_column_actions.py b/sandbox/grist/test_column_actions.py index b165af2e..def0d1a9 100644 --- a/sandbox/grist/test_column_actions.py +++ b/sandbox/grist/test_column_actions.py @@ -356,9 +356,9 @@ class TestColumnActions(test_engine.EngineTestCase): ]), # Note that the summary table here switches to a new one, without the deleted group-by. Table(4, "GristSummary_7_Address2", 0, 1, columns=[ - Column(22, "count", "Int", True, summarySourceCol=0, formula="len($group)"), - Column(23, "amount", "Numeric", True, summarySourceCol=0, formula="SUM($group.amount)"), - Column(24, "group", "RefList:Address", True, summarySourceCol=0, + Column(23, "count", "Int", True, summarySourceCol=0, formula="len($group)"), + Column(24, "amount", "Numeric", True, summarySourceCol=0, formula="SUM($group.amount)"), + Column(22, "group", "RefList:Address", True, summarySourceCol=0, formula="table.getSummarySourceGroup(rec)"), ]), ]) @@ -372,8 +372,8 @@ class TestColumnActions(test_engine.EngineTestCase): Field(12, colRef=17), ]), Section(5, parentKey="record", tableRef=4, fields=[ - Field(14, colRef=22), - Field(15, colRef=23), + Field(14, colRef=23), + Field(15, colRef=24), ]), ]), View(2, sections=[ diff --git a/sandbox/grist/test_summary.py b/sandbox/grist/test_summary.py index b98fd6d1..6336965e 100644 --- a/sandbox/grist/test_summary.py +++ b/sandbox/grist/test_summary.py @@ -824,9 +824,9 @@ class Address: ]), Table(3, "GristSummary_6_Table1_2", summarySourceTable=1, primaryViewId=0, columns=[ Column(10, "B", "Numeric", False, "", 3), - Column(11, "count", "Int", True, "len($group)", 0), - Column(12, "C", "Numeric", True, "SUM($group.C)", 0), - Column(13, "group", "RefList:Table1", True, "table.getSummarySourceGroup(rec)", 0), + Column(12, "count", "Int", True, "len($group)", 0), + Column(13, "C", "Numeric", True, "SUM($group.C)", 0), + Column(11, "group", "RefList:Table1", True, "table.getSummarySourceGroup(rec)", 0), ]) ]) self.assertTableData('Table1', data=[ diff --git a/sandbox/grist/test_summary2.py b/sandbox/grist/test_summary2.py index 98b15df1..b68754b0 100644 --- a/sandbox/grist/test_summary2.py +++ b/sandbox/grist/test_summary2.py @@ -456,16 +456,16 @@ class TestSummary2(test_engine.EngineTestCase): # Note that Table #2 is gone at this point, since it's unused. Table(3, "GristSummary_7_Address2", 0, 1, columns=[ Column(19, "state", "Text", False, "", 12), - Column(20, "count", "Int", True, "len($group)", 0), - Column(21, "amount", "Numeric", True, "SUM($group.amount)", 0), - Column(22, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(20, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(21, "count", "Int", True, "len($group)", 0), + Column(22, "amount", "Numeric", True, "SUM($group.amount)", 0), ]), ]) self.assertViews([View(1, sections=[ Section(1, parentKey="record", tableRef=3, fields=[ Field(2, colRef=19), - Field(3, colRef=20), - Field(4, colRef=21), + Field(3, colRef=21), + Field(4, colRef=22), ]) ])]) self.assertTableData('GristSummary_7_Address2', cols="subset", data=[ @@ -481,7 +481,7 @@ class TestSummary2(test_engine.EngineTestCase): self.assertTableData('_grist_Tables_column', rows="subset", cols="subset", data=[ ['id', 'colId', 'type', 'formula', 'widgetOptions', 'label'], [19, 'state', 'Text', '', 'WidgetOptions1', 'State'], - [21, 'amount', 'Numeric', 'SUM($group.amount)', 'WidgetOptions2', 'Amount'], + [22, 'amount', 'Numeric', 'SUM($group.amount)', 'WidgetOptions2', 'Amount'], ]) # Change group-by to a different single column ('city') @@ -491,16 +491,16 @@ class TestSummary2(test_engine.EngineTestCase): # Note that Table #3 is gone at this point, since it's unused. Table(4, "GristSummary_7_Address", 0, 1, columns=[ Column(23, "city", "Text", False, "", 11), - Column(24, "count", "Int", True, "len($group)", 0), - Column(25, "amount", "Numeric", True, "SUM($group.amount)", 0), - Column(26, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(24, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(25, "count", "Int", True, "len($group)", 0), + Column(26, "amount", "Numeric", True, "SUM($group.amount)", 0), ]), ]) self.assertViews([View(1, sections=[ Section(1, parentKey="record", tableRef=4, fields=[ Field(5, colRef=23), - Field(3, colRef=24), - Field(4, colRef=25), + Field(3, colRef=25), + Field(4, colRef=26), ]) ])]) self.assertTableData('GristSummary_7_Address', cols="subset", data=[ @@ -520,7 +520,7 @@ class TestSummary2(test_engine.EngineTestCase): self.assertTableData('_grist_Tables_column', rows="subset", cols="subset", data=[ ['id', 'colId', 'type', 'formula', 'widgetOptions', 'label'], [23, 'city', 'Text', '', '', 'City'], - [25, 'amount', 'Numeric', 'SUM($group.amount)', 'WidgetOptions2', 'Amount'], + [26, 'amount', 'Numeric', 'SUM($group.amount)', 'WidgetOptions2', 'Amount'], ]) # Change group-by to no columns (totals) @@ -529,15 +529,15 @@ class TestSummary2(test_engine.EngineTestCase): self.starting_table, # Note that Table #4 is gone at this point, since it's unused. Table(5, "GristSummary_7_Address2", 0, 1, columns=[ - Column(27, "count", "Int", True, "len($group)", 0), - Column(28, "amount", "Numeric", True, "SUM($group.amount)", 0), - Column(29, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(27, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(28, "count", "Int", True, "len($group)", 0), + Column(29, "amount", "Numeric", True, "SUM($group.amount)", 0), ]), ]) self.assertViews([View(1, sections=[ Section(1, parentKey="record", tableRef=5, fields=[ - Field(3, colRef=27), - Field(4, colRef=28), + Field(3, colRef=28), + Field(4, colRef=29), ]) ])]) self.assertTableData('GristSummary_7_Address2', cols="subset", data=[ @@ -554,17 +554,17 @@ class TestSummary2(test_engine.EngineTestCase): Table(6, "GristSummary_7_Address", 0, 1, columns=[ Column(30, "state", "Text", False, "", 12), Column(31, "city", "Text", False, "", 11), - Column(32, "count", "Int", True, "len($group)", 0), - Column(33, "amount", "Numeric", True, "SUM($group.amount)", 0), - Column(34, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(32, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(33, "count", "Int", True, "len($group)", 0), + Column(34, "amount", "Numeric", True, "SUM($group.amount)", 0), ]), ]) self.assertViews([View(1, sections=[ Section(1, parentKey="record", tableRef=6, fields=[ Field(5, colRef=30), Field(6, colRef=31), - Field(3, colRef=32), - Field(4, colRef=33), + Field(3, colRef=33), + Field(4, colRef=34), ]) ])]) self.assertTableData('GristSummary_7_Address', cols="subset", data=[ @@ -588,23 +588,23 @@ class TestSummary2(test_engine.EngineTestCase): Table(6, "GristSummary_7_Address", 0, 1, columns=[ Column(30, "state", "Text", False, "", 12), Column(31, "city", "Text", False, "", 11), - Column(32, "count", "Int", True, "len($group)", 0), - Column(33, "amount", "Numeric", True, "SUM($group.amount)", 0), - Column(34, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(32, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(33, "count", "Int", True, "len($group)", 0), + Column(34, "amount", "Numeric", True, "SUM($group.amount)", 0), ]), ]) self.assertViews([View(1, sections=[ Section(1, parentKey="record", tableRef=6, fields=[ Field(5, colRef=30), Field(6, colRef=31), - Field(3, colRef=32), - Field(4, colRef=33), + Field(3, colRef=33), + Field(4, colRef=34), ]), Section(2, parentKey="record", tableRef=6, fields=[ Field(7, colRef=31), Field(8, colRef=30), - Field(9, colRef=32), - Field(10, colRef=33), + Field(9, colRef=33), + Field(10, colRef=34), ]) ])]) self.assertEqual(get_helper_cols('Address'), ['#summary#GristSummary_7_Address']) @@ -616,26 +616,26 @@ class TestSummary2(test_engine.EngineTestCase): Table(6, "GristSummary_7_Address", 0, 1, columns=[ Column(30, "state", "Text", False, "", 12), Column(31, "city", "Text", False, "", 11), - Column(32, "count", "Int", True, "len($group)", 0), - Column(33, "amount", "Numeric", True, "SUM($group.amount)", 0), - Column(34, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(32, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(33, "count", "Int", True, "len($group)", 0), + Column(34, "amount", "Numeric", True, "SUM($group.amount)", 0), ]), Table(7, "GristSummary_7_Address2", 0, 1, columns=[ - Column(35, "count", "Int", True, "len($group)", 0), - Column(36, "amount", "Numeric", True, "SUM($group.amount)", 0), - Column(37, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(35, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(36, "count", "Int", True, "len($group)", 0), + Column(37, "amount", "Numeric", True, "SUM($group.amount)", 0), ]), ]) self.assertViews([View(1, sections=[ Section(1, parentKey="record", tableRef=6, fields=[ Field(5, colRef=30), Field(6, colRef=31), - Field(3, colRef=32), - Field(4, colRef=33), + Field(3, colRef=33), + Field(4, colRef=34), ]), Section(2, parentKey="record", tableRef=7, fields=[ - Field(9, colRef=35), - Field(10, colRef=36), + Field(9, colRef=36), + Field(10, colRef=37), ]) ])]) self.assertEqual(get_helper_cols('Address'), ['#summary#GristSummary_7_Address', @@ -649,17 +649,17 @@ class TestSummary2(test_engine.EngineTestCase): Table(6, "GristSummary_7_Address", 0, 1, columns=[ Column(30, "state", "Text", False, "", 12), Column(31, "city", "Text", False, "", 11), - Column(32, "count", "Int", True, "len($group)", 0), - Column(33, "amount", "Numeric", True, "SUM($group.amount)", 0), - Column(34, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(32, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(33, "count", "Int", True, "len($group)", 0), + Column(34, "amount", "Numeric", True, "SUM($group.amount)", 0), ]) ]) self.assertViews([View(1, sections=[ Section(1, parentKey="record", tableRef=6, fields=[ Field(5, colRef=30), Field(6, colRef=31), - Field(3, colRef=32), - Field(4, colRef=33), + Field(3, colRef=33), + Field(4, colRef=34), ]) ])]) self.assertEqual(get_helper_cols('Address'), ['#summary#GristSummary_7_Address']) @@ -673,8 +673,8 @@ class TestSummary2(test_engine.EngineTestCase): Column(35, "city", "Text", False, "", 11), Column(36, "state", "Text", False, "", 12), Column(37, "amount", "Numeric", False, "", 13), - Column(38, "count", "Int", True, "len($group)", 0), - Column(39, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(38, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(39, "count", "Int", True, "len($group)", 0), ]), ]) self.assertViews([View(1, sections=[ @@ -682,7 +682,7 @@ class TestSummary2(test_engine.EngineTestCase): Field(6, colRef=35), Field(5, colRef=36), Field(7, colRef=37), - Field(3, colRef=38), + Field(3, colRef=39), ]) ])]) self.apply_user_action(["UpdateSummaryViewSection", 1, [11,12]]) @@ -691,9 +691,9 @@ class TestSummary2(test_engine.EngineTestCase): Table(8, "GristSummary_7_Address", 0, 1, columns=[ Column(40, "city", "Text", False, "", 11), Column(41, "state", "Text", False, "", 12), - Column(42, "count", "Int", True, "len($group)", 0), + Column(42, "amount", "Numeric", True, "SUM($group.amount)", 0), Column(43, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), - Column(44, "amount", "Numeric", True, "SUM($group.amount)", 0), + Column(44, "count", "Int", True, "len($group)", 0), ]), ]) @@ -701,8 +701,28 @@ class TestSummary2(test_engine.EngineTestCase): Section(1, parentKey="record", tableRef=8, fields=[ Field(6, colRef=40), Field(5, colRef=41), - Field(3, colRef=42), - Field(7, colRef=44), + Field(7, colRef=42), + Field(3, colRef=44), + ]) + ])]) + + # Hide a formula and update group by columns; check that the formula columns had not been + # deleted + self.apply_user_action(['RemoveRecord', '_grist_Views_section_field', 7]) + self.apply_user_action(["UpdateSummaryViewSection", 1, [11]]) + self.assertTables([ + self.starting_table, + Table(9, "GristSummary_7_Address2", 0, 1, columns=[ + Column(45, "city", "Text", False, "", 11), + Column(46, "amount", "Numeric", True, "SUM($group.amount)", 0), + Column(48, "count", "Int", True, "len($group)", 0), + Column(47, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + ]), + ]) + self.assertViews([View(1, sections=[ + Section(1, parentKey="record", tableRef=9, fields=[ + Field(6, colRef=45), + Field(3, colRef=48), ]) ])]) @@ -755,9 +775,9 @@ class TestSummary2(test_engine.EngineTestCase): Table(3, "GristSummary_7_Address2", 0, 1, columns=[ Column(19, "city", "Text", False, "", 11), Column(20, "state", "Text", False, "", 12), - Column(21, "count", "Int", True, "len($group)", 0), - Column(22, "amount", "Numeric", True, "SUM($group.amount)", 0), - Column(23, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(21, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(22, "count", "Int", True, "len($group)", 0), + Column(23, "amount", "Numeric", True, "SUM($group.amount)", 0), ]), ]) self.assertViews([View(1, sections=[ @@ -765,8 +785,8 @@ class TestSummary2(test_engine.EngineTestCase): # We requested 'city' to come before 'state', check that this is the case. Field(4, colRef=19), Field(1, colRef=20), - Field(2, colRef=21), - Field(3, colRef=22), + Field(2, colRef=22), + Field(3, colRef=23), ]) ])]) @@ -890,15 +910,15 @@ class TestSummary2(test_engine.EngineTestCase): # Note that Table #2 is gone at this point, since it's unused. Table(3, "GristSummary_7_Address2", 0, 1, columns=[ Column(19, "state", "Text", False, "", 12), - Column(20, "count", "Int", True, "len($group)", 0), - Column(21, "amount", "Numeric", True, "SUM($group.amount)", 0), - Column(22, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(20, "group", "RefList:Address", True, "table.getSummarySourceGroup(rec)", 0), + Column(21, "count", "Int", True, "len($group)", 0), + Column(22, "amount", "Numeric", True, "SUM($group.amount)", 0), ]), ]) # Verify that sortColRefs refers to new columns. self.assertTableData('_grist_Views_section', cols="subset", data=[ ["id", "tableRef", "sortColRefs"], - [1, 3, "[19,-20]"], + [1, 3, "[19,-21]"], ]) #----------------------------------------------------------------------