diff --git a/app/client/components/BaseView.js b/app/client/components/BaseView.js index b91bb1af..5539a765 100644 --- a/app/client/components/BaseView.js +++ b/app/client/components/BaseView.js @@ -79,13 +79,11 @@ function BaseView(gristDoc, viewSectionModel, options) { // Create a section filter and a filtered row source that subscribes to its changes. // `sectionFilter` also provides `setFilterOverride()` to allow controlling a filter from a column menu. - // Whenever changes are made to filters, exempt rows are reset. - this._sectionFilter = SectionFilter.create( - this, this.viewSection, this.tableModel.tableData, () => this._exemptFromFilterRows.reset() - ); + this._sectionFilter = SectionFilter.create(this, this.viewSection, this.tableModel.tableData); this._filteredRowSource = rowset.FilteredRowSource.create(this, this._sectionFilter.sectionFilterFunc.get()); this._filteredRowSource.subscribeTo(this._mainRowSource); this.autoDispose(this._sectionFilter.sectionFilterFunc.addListener(filterFunc => { + this._exemptFromFilterRows.reset(); this._filteredRowSource.updateFilter(filterFunc); })); diff --git a/app/client/models/SectionFilter.ts b/app/client/models/SectionFilter.ts index fa38194d..4f34cc4a 100644 --- a/app/client/models/SectionFilter.ts +++ b/app/client/models/SectionFilter.ts @@ -28,11 +28,7 @@ export class SectionFilter extends Disposable { private _openFilterOverride: Observable = Observable.create(this, null); - constructor( - public viewSection: ViewSectionRec, - private _tableData: TableData, - private _resetExemptRows: () => void, - ) { + constructor(public viewSection: ViewSectionRec, private _tableData: TableData) { super(); this.sectionFilterFunc = Computed.create(this, this._openFilterOverride, (use, openFilter) => { @@ -65,10 +61,8 @@ export class SectionFilter extends Disposable { * Builds a filter function that combines the filter function of all the columns. You can use * `getFilterFunc(column, colFilter)` to customize the filter func for each column. It calls * `getFilterFunc` right away. - * This also immediately resets rows that were temporarily exempted from filtering. */ public buildFilterFunc(getFilterFunc: ColFilterCB, use: UseCB) { - this._resetExemptRows(); const filters = use(this.viewSection.filters); const funcs: Array | null> = filters.map(({filter, fieldOrColumn}) => { const colFilter = buildColFilter(use(filter), use(use(fieldOrColumn.origCol).type)); diff --git a/app/client/models/rowset.ts b/app/client/models/rowset.ts index c2d5960a..44b3ac56 100644 --- a/app/client/models/rowset.ts +++ b/app/client/models/rowset.ts @@ -802,4 +802,6 @@ export class ExemptFromFilterRowSource extends BaseFilteredRowSource { public reset() { this.onRemoveRows(this.getAllRows()); } + + public onUpdateRows() { /* no-op */ } } diff --git a/test/fixtures/docs/ExemptFromFilterBug.grist b/test/fixtures/docs/ExemptFromFilterBug.grist new file mode 100644 index 00000000..1fb0d803 Binary files /dev/null and b/test/fixtures/docs/ExemptFromFilterBug.grist differ diff --git a/test/nbrowser/FilteringBugs.ts b/test/nbrowser/FilteringBugs.ts new file mode 100644 index 00000000..5742f380 --- /dev/null +++ b/test/nbrowser/FilteringBugs.ts @@ -0,0 +1,42 @@ +import {assert, Key} from 'mocha-webdriver'; +import * as gu from 'test/nbrowser/gristUtils'; +import {setupTestSuite} from 'test/nbrowser/testUtils'; + +describe('FilteringBugs', function() { + this.timeout(20000); + const cleanup = setupTestSuite(); + let session: gu.Session; + + afterEach(() => gu.checkForErrors()); + + before(async function() { + session = await gu.session().login(); + }); + + it('should not filter records edited from another view', async function() { + await session.tempDoc(cleanup, 'ExemptFromFilterBug.grist'); + + // Change column A of the first record from foo to bar; bar should still be + // visible, even though it's excluded by a filter. + await gu.getCell({section: 'TABLE2 Filtered', rowNum: 1, col: 'A'}).click(); + await gu.sendKeys(Key.ENTER, Key.ARROW_DOWN, Key.ENTER); + await gu.waitForServer(); + assert.deepEqual( + await gu.getVisibleGridCells({section: 'TABLE2 Filtered', col: 'A', rowNums: [1, 2, 3]}), + ['bar', 'foo', ''] + ); + + // With the same record selected, change column C in the linked card section. + await gu.getCell({section: 'TABLE2 Filtered', rowNum: 1, col: 'A'}).click(); + await gu.getCardCell('C', 'TABLE2 Card').click(); + await gu.sendKeys('2', Key.ENTER); + await gu.waitForServer(); + + // Check that the record is still visible in the first section. Previously, a + // regression caused it to be filtered out. + assert.deepEqual( + await gu.getVisibleGridCells({section: 'TABLE2 Filtered', col: 'A', rowNums: [1, 2, 3]}), + ['bar', 'foo', ''] + ); + }); +});