Archives/gristlabs_grist-core
1
0
Fork 0
mirror of https://github.com/gristlabs/grist-core.git synced 2024-10-27 20:44:07 +00:00
Code Issues Projects Releases Wiki Activity

(core) Disambiguate label for link between summary table and source table with self reference

Browse Source 
Summary:
Following up on a small bug introduced in https://phab.getgrist.com/D3464. When a table has a column referencing the same table, then there can be two 'select by' options with the same label which is just the name of a summary table on the same page. The first option is simply filtering based on the summary table. The second option is linking the ref column in the source against the group column in the summary, but the name of the group column is hidden which leads to the ambiguity.

The solution in this diff is to always show the target node (source table) column name if the source node (summary table) column was the hidden group column. This also changes the label in the case where the reference to the source table isn't in the source table - see the updated test. This isn't strictly necessary in this case so I'm not 100% about the desired behaviour, but I don't think it hurts.

Test Plan: Tested disambiguation manually. Updated existing test.

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D3472
This commit is contained in:
Alex Hall 2022-06-07 13:42:04 +02:00
parent 4b54c7d99b
commit 519f1be93a
1 changed files with 11 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
app/client/ui/selectBy.ts
View File

@ -93,8 +93,8 @@ function isValidLink(source: LinkNode, target: LinkNode) {
// Instead the ref column must link against the group column of the summary table, which is allowed above.
// The 'group' column name will be hidden from the options so it feels like linking using summaryness.
if (
(source.groupbyColumns && !source.column && target.column) ||
(target.groupbyColumns && !target.column && source.column)
(source.isSummary && !source.column && target.column) ||
(target.isSummary && !target.column && source.column)
) {
return false;
}
@ -105,9 +105,9 @@ function isValidLink(source: LinkNode, target: LinkNode) {
if (
!source.column &&
!target.column &&
target.groupbyColumns && !(
source.groupbyColumns &&
gutil.isSubset(source.groupbyColumns, target.groupbyColumns)
target.isSummary && !(
source.isSummary &&
gutil.isSubset(source.groupbyColumns!, target.groupbyColumns!)
)
) {
return false;
@ -175,8 +175,12 @@ export function selectBy(docModel: DocModel, sources: ViewSectionRec[],
label += ` ${BLACK_CIRCLE} ${srcNode.column.label.peek()}`;
}
// add the target column name (except for 'group') only if target has multiple valid nodes
if (hasMany && tgtNode.column && !isSummaryGroup(tgtNode)) {
// add the target column name (except for 'group') when clarification is needed, i.e. if either:
// - target has multiple valid nodes, or
// - source col is 'group' and is thus hidden.
// Need at least one column name to distinguish from simply selecting by summary table.
// This is relevant when a table has a column referencing itself.
if (tgtNode.column && !isSummaryGroup(tgtNode) && (hasMany || isSummaryGroup(srcNode))) {
label += ` ${RIGHT_ARROW} ${tgtNode.column.label.peek()}`;
}