(core) A quick attempt to fix summarizing by reference columns

Test Plan: TBD

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D2884

app/client/widgets/FieldBuilder.ts

@@ -266,7 +266,11 @@ export class FieldBuilder extends Disposable {
       cssLabel('DATA FROM TABLE'),
       cssRow(
         dom.autoDispose(allTables),
-        select(fromKo(this._refTableId), allTables),
+        select(fromKo(this._refTableId), allTables, {
+          // Disallow changing the destination table when the column should not be modified
+          // (specifically when it's a group-by column of a summary table).
+          disabled: this.origColumn.disableModifyBase,
+        }),
         testId('ref-table-select')
       )
     ];

sandbox/grist/summary.py

@@ -4,6 +4,7 @@ import re
 
 import six
 
+from column import is_visible_column
 import logger
 log = logger.Logger(__name__, logger.INFO)
 
@@ -20,7 +21,8 @@ def _make_col_info(col=None, **values):
 
 def _get_colinfo_dict(col_info, with_id=False):
   """Return a dict suitable to use with AddColumn or AddTable (when with_id=True) actions."""
-  col_values = {k: v for k, v in six.iteritems(col_info._asdict()) if v is not None and k != 'colId'}
+  col_values = {k: v for k, v in six.iteritems(col_info._asdict())
+                     if v is not None and k != 'colId'}
   if with_id:
     col_values['id'] = col_info.colId
   return col_values
@@ -140,7 +142,10 @@ class SummaryActions(object):
     if created:
       # Set the summarySourceCol field for all the group-by columns in the table.
       self.docmodel.update(groupby_columns,
-                           summarySourceCol=[c.id for c in source_groupby_columns])
+                           summarySourceCol=[c.id for c in source_groupby_columns],
+                           visibleCol=[c.visibleCol for c in source_groupby_columns])
+      for col in groupby_columns:
+        self.useractions.maybe_copy_display_formula(col.summarySourceCol, col)
       assert summary_table.summaryKey == key
 
     return (summary_table, groupby_columns, formula_columns)
@@ -242,7 +247,7 @@ class SummaryActions(object):
     # same-named column with the sum of the values in the group.
     groupby_col_ids = {c.colId for c in source_groupby_columns}
     for col in source_table.columns:
-      if col.colId in groupby_col_ids or col.colId == 'group':
+      if col.colId in groupby_col_ids or col.colId == 'group' or not is_visible_column(col.colId):
         continue
       c = self._find_sister_column(source_table, col.colId)
       if c:

sandbox/grist/useractions.py

@@ -567,7 +567,8 @@ class UserActions(object):
       if col.summarySourceCol:
         underlying = col_updates.get(col.summarySourceCol, {})
         if not all(value == getattr(col, key) or has_value(underlying, key, value)
-                   for key, value in six.iteritems(values)):
+                   for key, value in six.iteritems(values)
+                   if key not in ('displayCol', 'visibleCol')):
           raise ValueError("Cannot modify summary group-by column '%s'" % col.colId)
 
     make_acl_updates = acl.prepare_acl_col_renames(self._docmodel, self, renames)
@@ -1216,10 +1217,7 @@ class UserActions(object):
                           displayCol=[dst_col.displayCol if src_col.displayCol else 0])
 
     # Copy over display column as well, if the source column has one.
-    # TODO: Should use the same formula renaming logic that is used when renaming columns.
+    self.maybe_copy_display_formula(src_col, dst_col)
-    if src_col.displayCol:
-      self.SetDisplayFormula(dst_col.parentId.tableId, None, dst_col.id,
-        re.sub((r'\$%s\b' % src_col.colId), '$' + dst_col.colId, src_col.displayCol.formula))
 
     # Get the values from the columns and check which have changed.
     all_row_ids = list(table.row_ids)
@@ -1236,6 +1234,16 @@ class UserActions(object):
     self._do_doc_action(actions.BulkUpdateRecord(table_id, changed_rows,
                                                  {dst_col_id: changed_values}))
 
+
+  def maybe_copy_display_formula(self, src_col, dst_col):
+    """
+    If src_col has a displayCol set, create an equivalent one for dst_col.
+    """
+    # TODO: Should use the same formula renaming logic that is used when renaming columns.
+    if src_col.displayCol:
+      self.SetDisplayFormula(dst_col.parentId.tableId, None, dst_col.id,
+        re.sub((r'\$%s\b' % src_col.colId), '$' + dst_col.colId, src_col.displayCol.formula))
+
   #----------------------------------------
   # User actions on tables.
   #----------------------------------------