diff --git a/app/client/components/LinkingState.ts b/app/client/components/LinkingState.ts index 5481ff38..8fb13a09 100644 --- a/app/client/components/LinkingState.ts +++ b/app/client/components/LinkingState.ts @@ -350,7 +350,11 @@ export class LinkingState extends Disposable { * Returns a boolean indicating whether editing should be disabled in the destination section. */ public disableEditing(): boolean { - return Boolean(this.filterState) && this._srcSection.activeRowId() === 'new'; + if (!this.filterState) { + return false; + } + const srcRowId = this._srcSection.activeRowId(); + return srcRowId === 'new' || srcRowId === null; } @@ -438,11 +442,6 @@ export class LinkingState extends Disposable { //Get selector-rowId const srcRowId = this._srcSection.activeRowId(); - if (srcRowId === null) { - console.warn("LinkingState._makeFilterObs activeRowId is null"); - return EmptyFilterState; - } - //Get values from selector row const selectorCellVal = selectorValGetter(srcRowId); const displayCellVal = displayValGetter(srcRowId); @@ -473,30 +472,45 @@ export class LinkingState extends Disposable { } } - //Need to use 'intersects' for ChoiceLists or RefLists + // ==== Determine operation to use for filter ==== + // Common case: use 'in' for single vals, or 'intersects' for ChoiceLists & RefLists let operation = (tgtColId && isListType(tgtCol!.type())) ? 'intersects' : 'in'; - // If selectorVal is a blank-cell value, need to change operation for correct behavior with lists + // # Special case 1: // Blank selector shouldn't mean "show no records", it should mean "show records where tgt column is also blank" - if(srcRowId !== 'new') { //(EXCEPTION: the add-row, which is when we ACTUALLY want to show no records) - - // If tgtCol is a list (RefList or Choicelist) and selectorVal is null/blank, operation must be 'empty' - if (tgtCol?.type() === "ChoiceList" && !isSrcRefList && selectorCellVal === "") { operation = 'empty'; } - else if (isTgtRefList && !isSrcRefList && selectorCellVal === 0) { operation = 'empty'; } - else if (isTgtRefList && isSrcRefList && filterValues.length === 0) { operation = 'empty'; } - // other types can have falsey values when non-blank (e.g. for numbers, 0 is a valid value; blank cell is null) - // However, we don't need to check for those here, since we only care about lists (Reflist or Choicelist) - - // If tgtCol is a single ref, nullness is represented by [0], not by [], so need to create that null explicitly - else if (!isTgtRefList && isSrcRefList && filterValues.length === 0) { - filterValues = [0]; - displayValues = ['']; - } + // This is the default behavior for single-ref -> single-ref links + // However, if tgtCol is a list and the selectorVal is blank/empty, the default behavior ([] intersects tgtlist) + // doesn't work, we need to explicitly specify the operation to be 'empty', to select empty cells + if (tgtCol?.type() === "ChoiceList" && !isSrcRefList && selectorCellVal === "") { operation = 'empty'; } + else if (isTgtRefList && !isSrcRefList && selectorCellVal === 0) { operation = 'empty'; } + else if (isTgtRefList && isSrcRefList && filterValues.length === 0) { operation = 'empty'; } + // Note, we check each case separately since they have different "blank" values" + // Other types can have different falsey values when non-blank (e.g. a Ref=0 is a blank cell, but for numbers, + // 0 would be a valid value, and to check for an empty number-cell you'd check for null) + // However, we don't need to check for those here, since they can't be linked to list types + + // NOTES ON CHOICELISTS: they only show up in a few cases. + // - ChoiceList can only ever appear in links as the tgtcol + // (ChoiceLists can only be linked from summ. tables, and summary flattens lists, so srcCol would be 'Choice') + // - empty Choice is [""]. + + // # Special case 2: + // If tgtCol is a single ref, blankness is represented by [0] + // However if srcCol is a RefList, blankness is represented by [], which won't match the [0]. + // We create the 0 explicitly so the filter will select the blank Refs + else if (!isTgtRefList && isSrcRefList && filterValues.length === 0) { + filterValues = [0]; + displayValues = ['']; + } - // NOTES ON CHOICELISTS: they only show up in a few cases. - // - ChoiceList can only ever appear in links as the tgtcol - // (ChoiceLists can only be linked from summ. tables, and summary flattens lists, so srcCol would be 'Choice') - // - empty choicelist is [""]. + // # Special case 3: + // If the srcSection has no row selected (cursor on the add-row, or no data in srcSection), we should + // show no rows in tgtSection. (we also gray it out and show the "No row selected in $SRCSEC" msg) + // This should line up with when this.disableEditing() returns true + if (srcRowId === 'new' || srcRowId === null) { + operation = 'in'; + filterValues = []; + displayValues = []; } // Run values through formatters (for dates, numerics, Refs with visCol = rowId) diff --git a/test/fixtures/docs/FilterLinkChain.grist b/test/fixtures/docs/FilterLinkChain.grist new file mode 100644 index 00000000..03587d14 Binary files /dev/null and b/test/fixtures/docs/FilterLinkChain.grist differ diff --git a/test/nbrowser/FilterLinkChain.ts b/test/nbrowser/FilterLinkChain.ts new file mode 100644 index 00000000..0e8ba832 --- /dev/null +++ b/test/nbrowser/FilterLinkChain.ts @@ -0,0 +1,130 @@ +import {assert} from 'mocha-webdriver'; +import * as gu from 'test/nbrowser/gristUtils'; +import {setupTestSuite} from 'test/nbrowser/testUtils'; + +describe('FilterLinkChain', function () { + this.timeout(10000); + const cleanup = setupTestSuite(); + + before(async function () { + const session = await gu.session().teamSite.login(); + await session.tempDoc(cleanup, 'FilterLinkChain.grist'); + }); + + it('should work with chains of filter links', async function () { + async function checkCells(sectionName: string, cols: string[], expected: string[]) { + assert.deepEqual( + await gu.getVisibleGridCells({section: sectionName, cols, rowNums: [1, 2]}), + expected + ); + + // Sanity-check the selectors in checkSectionEmpty() below. + const section = gu.getSection(sectionName); + assert.isEmpty(await section.findAll('.disable_viewpane')); + assert.isNotEmpty(await section.findAll('.gridview_row .gridview_data_row_num')); + assert.isNotEmpty(await section.findAll('.gridview_row .record')); + assert.isNotEmpty(await section.findAll('.gridview_row .field')); + } + + async function checkTopCells(expected: string[]) { + await checkCells('TOP', ['Top'], expected); + } + + async function checkMiddleCells(expected: string[]) { + await checkCells('MIDDLE', ['Top', 'Middle'], expected); + } + + async function checkBottomCells(expected: string[]) { + await checkCells('BOTTOM', ['Top', 'Middle', 'Bottom'], expected); + } + + async function checkSectionEmpty(sectionName: string, text: string) { + const section = gu.getSection(sectionName); + assert.equal(await section.find('.disable_viewpane').getText(), text); + assert.isEmpty(await section.findAll('.gridview_row .gridview_data_row_num')); + assert.isEmpty(await section.findAll('.gridview_row .record')); + assert.isEmpty(await section.findAll('.gridview_row .field')); + } + + async function checkBottomEmpty() { + await checkSectionEmpty('BOTTOM', 'No row selected in MIDDLE'); + } + + async function checkMiddleEmpty() { + await checkSectionEmpty('MIDDLE', 'No row selected in TOP'); + } + + // The initially visible data. + // The bottom section is selected by the middle section, + // which is selected by the top section. + await checkTopCells([ + 'A', // selected initially + 'B', + ]); + // Filtered to 'A' + await checkMiddleCells([ + 'A', 'A1', // selected initially + 'A', 'A2', + ]); + // Filtered to 'A1' + await checkBottomCells([ + 'A', 'A1', '1', + 'A', 'A1', '2', + ]); + + // Select 'A2' + await gu.getCell({section: 'MIDDLE', col: 'Middle', rowNum: 2}).click(); + await checkBottomCells([ + 'A', 'A2', '3', + 'A', 'A2', '4', + ]); + + // Select the 'new' row + await gu.getCell({section: 'MIDDLE', col: 'Middle', rowNum: 3}).click(); + await checkSectionEmpty('BOTTOM', 'No row selected in MIDDLE'); + + // Select 'B' + await gu.getCell({section: 'TOP', col: 'Top', rowNum: 2}).click(); + await checkMiddleCells([ + 'B', 'B1', // selected initially + 'B', 'B2', + ]); + // Filtered to 'B1' + await checkBottomCells([ + 'B', 'B1', '5', + 'B', 'B1', '6', + ]); + + // Select 'B2' + await gu.getCell({section: 'MIDDLE', col: 'Middle', rowNum: 2}).click(); + await checkBottomCells([ + 'B', 'B2', '7', + 'B', 'B2', '8', + ]); + + // Select the 'new' row, making the bottom empty + await gu.getCell({section: 'MIDDLE', col: 'Middle', rowNum: 3}).click(); + await checkBottomEmpty(); + + // Select the 'new' in the top section, which makes middle empty, which means bottom stays empty. + await gu.getCell({section: 'TOP', col: 'Top', rowNum: 3}).click(); + await checkMiddleEmpty(); + await checkBottomEmpty(); + + // Double-check: make all sections show some data again, + // and then make both the middle and bottom empty in one click instead of one at a time. + await gu.getCell({section: 'TOP', col: 'Top', rowNum: 2}).click(); + await checkMiddleCells([ + 'B', 'B1', // selected initially + 'B', 'B2', + ]); + await checkBottomCells([ + 'B', 'B1', '5', + 'B', 'B1', '6', + ]); + + await gu.getCell({section: 'TOP', col: 'Top', rowNum: 3}).click(); + await checkMiddleEmpty(); + await checkBottomEmpty(); + }); +});