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) Fix column and view fields mismatch in filters

Browse Source 
Summary:
The problem manifested while filtering by a column, closing the filter
would change the set of rows visible on the table. This would happen
only for rare table.

What caused that problem was that the filter being edited was wrongly
applyed also to another column, so depending on the content of the
column, it would entail unexpected behaviour.

The cause of that wrong association, was a mistakingly comparing the
id of two different type of thing: column and view field. The problem
would manifest if in the same section there were a view fields with
same row id as the column being filtered.

What made that confusion possible is the `.fieldOrColumn:
ViewFieldRec|ColumnRec` property of the FileInfo object, which could
hold either a view fields or a column record and was initialized with
view fields or columns if view fields was not found (ie: hidden
column).

Solution was to make sure FieldInfo is initialized with ColumnRec
alwasy (even for hidden column).

I'm not sure what is the reason why FilterInfo needed to support both
column record and view field record in the past, but it looks like
this is not needed anymore.

As a followup commit I think it would be worth the effort to refactor
FileInfo to accept only ColumnRec.

Test Plan: Includes new regression test

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3463
This commit is contained in:
Cyprien P 2022-06-02 10:19:18 +02:00
parent 05d1cdf140
commit 4b54c7d99b
1 changed files with 6 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
app/client/models/entities/ViewSectionRec.ts
View File

@ -221,6 +221,10 @@ export interface CustomViewSectionDef {
} }
// Information about filters for a field or hidden column. // Information about filters for a field or hidden column.
// TODO: It looks like that it is not needed for FilterInfo to support ViewFieldRec anymore (db
// _grist_Filters explicitely maintain a reference to _grist_Tables_column, not
// _grist_Views_section_field). And it has caused a bug (due to mismatching a viewField id against a
// column id).
export interface FilterInfo { export interface FilterInfo {
// The field or column associated with this filter info. // The field or column associated with this filter info.
fieldOrColumn: ViewFieldRec|ColumnRec; fieldOrColumn: ViewFieldRec|ColumnRec;
@ -333,7 +337,6 @@ export function createViewSectionRec(this: ViewSectionRec, docModel: DocModel):
*/ */
this.filters = this.autoDispose(ko.computed(() => { this.filters = this.autoDispose(ko.computed(() => {
const savedFiltersByColRef = new Map(this._savedFilters().all().map(f => [f.colRef(), f])); const savedFiltersByColRef = new Map(this._savedFilters().all().map(f => [f.colRef(), f]));
const viewFieldsByColRef = new Map(this.viewFields().all().map(f => [f.colRef(), f]));
return this.columns().map(column => { return this.columns().map(column => {
const savedFilter = savedFiltersByColRef.get(column.origColRef()); const savedFilter = savedFiltersByColRef.get(column.origColRef());
@ -348,9 +351,9 @@ export function createViewSectionRec(this: ViewSectionRec, docModel: DocModel):
return { return {
filter, filter,
fieldOrColumn: viewFieldsByColRef.get(column.origColRef()) ?? column, fieldOrColumn: column,
isFiltered: ko.pureComputed(() => filter() !== '') isFiltered: ko.pureComputed(() => filter() !== '')
}; } as FilterInfo;
}); });
})); }));