From 4b54c7d99b7fa1459fdc723002a61409637349cb Mon Sep 17 00:00:00 2001
From: Cyprien P <cyprien.pindat@gmail.com>
Date: Thu, 2 Jun 2022 10:19:18 +0200
Subject: [PATCH] (core) Fix column and view fields mismatch in filters

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
---
 app/client/models/entities/ViewSectionRec.ts | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/app/client/models/entities/ViewSectionRec.ts b/app/client/models/entities/ViewSectionRec.ts
index 309a0fad..6f952a52 100644
--- a/app/client/models/entities/ViewSectionRec.ts
+++ b/app/client/models/entities/ViewSectionRec.ts
@@ -221,6 +221,10 @@ export interface CustomViewSectionDef {
 }
 
 // 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 {
   // The field or column associated with this filter info.
   fieldOrColumn: ViewFieldRec|ColumnRec;
@@ -333,7 +337,6 @@ export function createViewSectionRec(this: ViewSectionRec, docModel: DocModel):
    */
   this.filters = this.autoDispose(ko.computed(() => {
     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 => {
       const savedFilter = savedFiltersByColRef.get(column.origColRef());
@@ -348,9 +351,9 @@ export function createViewSectionRec(this: ViewSectionRec, docModel: DocModel):
 
       return {
         filter,
-        fieldOrColumn: viewFieldsByColRef.get(column.origColRef()) ?? column,
+        fieldOrColumn: column,
         isFiltered: ko.pureComputed(() => filter() !== '')
-      };
+      } as FilterInfo;
     });
   }));