From b878395c216e219a8dbacb72efa6ae3edb2638fd Mon Sep 17 00:00:00 2001 From: Alex Hall Date: Thu, 12 May 2022 14:59:17 +0200 Subject: [PATCH] (core) Allow linking summary tables based on ref/reflist columns (except `group`) Summary: Relax the restriction in `selectBy.isValidLink` so that summary tables can be linked by a column like other tables, except the `group` column. See the discussion on https://grist.slack.com/archives/C0234CPPXPA/p1651773623256959 (the replies are on the following message) for more info on this decision. Tweaked `LinkingState.ts` since linking with summary tables can now involve a column. Test Plan: Added a new nbrowser test and fixture checking the options to select by given a summary table with a few ref/reflist columns. Manually tested the behaviour of each option. Reviewers: jarek Reviewed By: jarek Differential Revision: https://phab.getgrist.com/D3416 --- app/client/components/LinkingState.ts | 2 +- app/client/ui/selectBy.ts | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/client/components/LinkingState.ts b/app/client/components/LinkingState.ts index 3f27f4ae..278f5dd1 100644 --- a/app/client/components/LinkingState.ts +++ b/app/client/components/LinkingState.ts @@ -96,7 +96,7 @@ export class LinkingState extends Disposable { } } else if (srcColId && isRefListType(srcCol.type())) { this.filterColValues = this._srcCellFilter('id', 'in'); - } else if (isSummaryOf(srcSection.table(), tgtSection.table())) { + } else if (!srcColId && isSummaryOf(srcSection.table(), tgtSection.table())) { // We filter summary tables when a summary section is linked to a more detailed one without // specifying src or target column. The filtering is on the shared group-by column (i.e. all // those in the srcSection). diff --git a/app/client/ui/selectBy.ts b/app/client/ui/selectBy.ts index 00be302b..c92160ba 100644 --- a/app/client/ui/selectBy.ts +++ b/app/client/ui/selectBy.ts @@ -69,8 +69,13 @@ function isValidLink(source: LinkNode, target: LinkNode) { return false; } - // summary table can only link to and from the main node (node with no column) - if ((source.isSummary || target.isSummary) && (source.column || target.column)) { + // cannot link to the somewhat special 'group' reflist column of summary tables + // because in most cases it's equivalent to the usual summary table linking but potentially slower, + // and in other cases it may cause confusion with overwhelming options. + if ( + (source.isSummary && source.column?.colId.peek() === "group") || + (target.isSummary && target.column?.colId.peek() === "group") + ) { return false; }