From b82209b458661bb430400f6ec618861e75581eea Mon Sep 17 00:00:00 2001 From: George Gevoian Date: Mon, 15 Jan 2024 14:24:51 -0800 Subject: [PATCH] (core) Fix filtering regression Summary: Fixes a recent regression that would cause a record to be erroneously filtered out whenever it was updated from a linked view. Test Plan: Browser test. Reviewers: jarek Reviewed By: jarek Differential Revision: https://phab.getgrist.com/D4163 --- app/client/components/BaseView.js | 6 +-- app/client/models/SectionFilter.ts | 8 +--- app/client/models/rowset.ts | 2 + test/fixtures/docs/ExemptFromFilterBug.grist | Bin 0 -> 176128 bytes test/nbrowser/FilteringBugs.ts | 42 +++++++++++++++++++ 5 files changed, 47 insertions(+), 11 deletions(-) create mode 100644 test/fixtures/docs/ExemptFromFilterBug.grist create mode 100644 test/nbrowser/FilteringBugs.ts 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 0000000000000000000000000000000000000000..1fb0d80353d49165ec06b7e9caa902bf45e4b657 GIT binary patch literal 176128 zcmeI5Uu+vmp5MDkilk(*TVr{amjBGOY0t@w?On^Z#-s5~ZYYVi1}HpQx6RrRkvzhC|9DmK^NTQq&9Z8~mU_qEH? zm?X>6yP76RQj-3kqyOz!fA)O>xzV%06z^!#dtfA*?1$7emGX`9sG$m z=gZei>x&ibVn@2-cm-`~{dReEVNPp{r`XKQ3?(7Wct{GfXByQTi z$>w-PuDecsF)T%NZLe9c>+bHF({yV*gP59OdJRk8jg>zzJ(H{&D|ee@@*X{x))yD4 z@-D0CR_*<5W{WQ!%I8@_l)tz?J(|f+PRfsq0vC(=TW)|;4f3X-Sg|`aT$7`_W3rt% zZ|kBPIKH@&jy=!nx=mguvXQIr@QbXm8uzeOwka-zNI9-Cr@O{eQy2Hg4rTJiqWtJy zl-s;hTd+5sR*P$eMup^B!a?W^Doxmq8jQKU)(jfhhVI>GLDTJT30`fR5Z$4i#6S$d zH|y-XjxB7Xq)f9;{lXgaebe6RzkG3jZ6xTe9xp_Yc)MP8#cFPu_Pp-vA(-yCLtS`8 zDYUaATI&_pX$9g77qk^iH*L+deYVA1tr1*UGw8~iX>@qgh(6P*292(m^3y z-)N5eY*(1Lox|T|8l~x6;el>7nKrw)JlpA~CMU!4HI3Bd>BBU-=Q%Y~_nD#XnErMk ziI!B{TN%#er>5lnTRl{-n-)88HqNviGZI6zEq^LcZSH1y?p82FyrW$#p6}Ev@W{UY zxg&OVZ00W5p0C?A*5=zD9VRDR>5BX3GnxF28}g&~d#Y3l+UHFgOm5BxzO z%*Iqyko2i_ZkeB3XNer$HEY^NpnIOj*I8Ko^`(XP*30co9oGq!)_A&3CcWM3 zbjH)RC-A>?y+ty==WAg)&9Pe-ZsWR}P3P-3WXZG*_R#y#qM2M(Z~9IUt_}oLof;?} ze)R58CV%ak{Js1jOT?6s{v!0Bg?o47ZQ)p$*UlC?2n)p?tT%OUTVxT!S_-&p)a}*o z(>T(}7|w5K6lmL{%ZikdIvHnE3zP9r{Yl9mfxf zav7uUsa2ZSDQ?3tcEkQ6>KpCPq*KkIKFtow2ru2rTD+Z3=k;sP=pC_sk$I$~GWn}l zFl+rImElbQU53-V(oHb^i>E)&p%VSu>lF}FwDWg$KXA0&5ZoOJsv6ji`7?fU~03}n0ax}3?M zJ16fod)V@k(mQbTAee|D8wj5PtZDFe9L&#JgGk3^gNX|FgLX{uokTkS-npl@?oi=K zZ_guWU3vHzy^Se3zQi2kZ+L+K2!H?xfB*=900@8p2!H?xfB*=*tOQ1Q#jyT=S9j zz45(VR@%!Y62E!u@|EeU)2w#!%Ehbtr7KLoJauK$nBJV)FfMOgy*zWpxWuOQiVos!hK&ef7=DGZ(L1mcB22@==q2bvk?$9)Hwi^IZyl)q}^XE9vk0k7Hdvjy6W*D{U z8FpowUAp>a?b7DPrCM!jYG&rLaq-Q!jJMumZyA@TW{fM-QwE!!x^nr=DPu!7re^dj z(=%@c>hfp9G9zGeP}h50&*mt-tvBc)dTcd#l5_J2MTAfBtMVs0dsQ!d0<_8N{ZW%Y zvEDa*i#1jH6g(~cl12~C^BG+sMB20sC%o1#;tKr1?a?b7qRVoYA*ax$Uq@9gawnG6 z-;?McULXJhAOHd&00JNY0w4eaAOHd&00Pe&f$J%{m-SWeH%XuH{r{x;V@dr~{V((f zFAx9$5C8!X009sH0T2KI5C8!X0D;FG{&MOG@KmY_l00ck)1V8`;KmY_l00ck)1R?~6 zlM}kkW`YVS`qwqKqC15zxugE|L_6<5C8!X009sH0T2KI5C8!X009tq z=?J8xl$6Wy^}ocQ%YYXMfB*=900@8p2!H?xfB*=900@AaZbB7^)L9bE9#aC*tGR3>mr|;*hkj`~$7QD`Np`AJVy>>+P2E~^T>p0G z(~9FAM9+=#Y(2FZ6{viP*QO+_&83ssK(FIby%O2ErDm|PJ$EY;h)-2RwPl$T%gnB_ z&9}o9??;6l)7WA@?Qc^gbD3RZwCSx|=9XQjhuu-U#WwxI$FoP2TWLXI=@Kt2TpEAj z!b+ow=^S>jbVgGDzWRFZhohH9oDnViSJ~gr z{4iY~`g^H@{72H6M@N-RzF3r>-1qr8f@*MEr@ByY(ET8OcmbF5+ zu%Mka4Q*kmQod1M)mBy)ZkJZ?YPZUF&ufLCWV-uO=*Z{G*Gub*740HP9oO8tZdz3Y{j^>cghvQ`xi2%ZugGQm1%+ zb>`Ld2CLC8CW^8L8RLZBeY0WOrf=$&*U;&9l=uR6%`r@SOH}-}?%ia%K}R|Sd5e#K zJ(tN(Ps_hii<&05gXL9gj@7K&EgN*jDMB-KB^hKa!cDi+77VemLMh!IENDyXx67*w za~+yfY-VPLh>~VJcfTS1-G

;f=lJhHpAHmD``u^g8AjnQ$}SH!OXFS)w8hvb$>r zmlTPcwr{dIUXkmrQ(p{A5nbDB*6X^vyXG|A8qXl6W`uWpW91J_&m=dEmAg%g^^#dy zUtA>GE~8%rt-Zg^Z1JT-`8;cg@)!4~M>F}!N%?V6;9_A{5IRn7fKv_frl44{J2YIA zqq}3Woj7mnq8lh8uB2nnGrAi}UMI4VtMBlOtg#yRu-JZxNI9-Cr@O{eQy2Hg4h6mD zqkB(E!o1Wk~Xp*!`qzs7u@Imiu# zxIl@8+pX~}0w{!U0OrtcND?HGxCevmYmuEYjOM2H z9n;?qB+-(Jdn?14{M3}Zf2)ToA2AP{jWcb>jKmOa%b&_qo4Z+_yA_P*?`Ri`=R5TZ zJhIO@I$~$XX6}OR`8w^*wD}$k<*jtZyV-QUenXZ_+h7m94=ozHt9sLSf^hY~14==2 zy-9m!j=MYSQlZ<`se$6*{qvbj{>BaY(fd8LL}dlt8wV#4W@8u%l0Fq@%d@!QTf1G_ zMo=4VVZOu8rpZr8c718#z4dZCQ^zYq02)u%$)vYCw9dS$?PUBfU2l;*%k#A`o#xoB z3%4zCJDtw!*PhW5u{Ov&dUq(3zjjUjUVae$7z?Jq2=QlO-Q5?o5G>4VXA2!{g<=oN zo4U6xvI3zj1*|pdMQitISn6a9Cqy);v~AF3Im$?Vmb0ma$@nLK+w@u%=@b%bMBWis z2u&b@R;6vybVPF<#}A5f8Kd5=Rhri+Zo@Hl!|o^Q5ADySQ_Z0+(hkZ9FWt&oe59l@ z`KwpuC)b1~ta?6c^cpaT92#i8Aj#8>*Qo%X^S36UUA^eVgTAN`&L+FWv|KnSg0~c! z4|I3J5qVHSKETE&sqqOQ4R!G;T!-c^15*V3{J<37q4V5**JvAL|86pqzi>f*%)|!i z>BLe4&G0N1=(uE$YF#Fv3Bv$!)8lKhczApcK1lA+IO+CTD5`*E+V=-07|8aWL^}W8 zxuDQi?KYU+wFZig#|QHq zPP-jb#Qy(_a;FA*f&d7B00@8p2!H?xfB*=900@Ae8sREM-b*d+1Mw&ZfSf z{I|(rX<4yy|7!HpL*E&>mi?XVKm8)*XL+J0SHBVL@ciRfA~!s$e6!4}dg10;cyF*f zZldefNBh`7SGGahV8OLwH|-8?6t<5Zv^O01=6)!>#dbwoTE6L9;=QiHd&36X)SH(7 zzG?W|;*GQo+DByWfCO}7OMKSBiQfe0rx5%~%oaL*mB?xOYjlu7RMSn;9Qjbo0 z(9Sax2t(t{_ zZwRXv%D!~n=$i4G7xzvc4)5gl>bBssdYe0G5CmV1(=U(?(vXy}m>3VWvv}JS_fEf( z$$#rx-FKR+q>#nJZZ!0agNP$>I<_)=JzB1e1$#&PzZO)qWz_OR>UwP5s>Vl-3$^Xe z_~kX`x5epTL_GLFu_e*rv*P|qHIskm9r@91q4w>fd#l=xe`RpR+Q|mhZQJpDQJ2n- z=ZRY>-*t)WkqkPjMgz5|zyoFaJ2JilPi?<1TTzmd!_RbhFnux|OgA3Ab|PqTPiR|{ zytoChG`G0QPdfDkUl(F*Cmux4WnOSFOH7Kshg&y2ejbJfKH>DiHSL<&pyPdgRi_54 z*R0O5w0>qJrBRNK=!uIy$h^XjH^!Xv>FCG5zAMs!kBIRd8!5@Xq2&9nxzVJf0HPx5 ztnT#17x&K}4-E8JjT)#zM>XkWP1`{2;A=tywWWg$6cirYC9(%JAn_?rui~wG0O zrzdv@%_hQa%-;OLxzgCPfkGn0lG)r>MTg9akA{u~$8?@dM{yB&y|&GdO|^+dVonO7 zMG_3+Ml(7(VM7J?S&sDNLScW_lEqCTdA>FCyVTA_je$$4PZnK}&hc%ro6OwWCb1}Q zkYLb!m^PB?+^(vO~V^aKd!wp*L7T>u()qFSt|J z%=@U8@{yS1LiHmF2326dsoA*UV?b*yK6)je$(KsKq{}bw`lKMHl;ROO900JNY0w4ea zAOHd&00JNY0wC~`5J)Fdk}M|@DLEw#4~OgjpG)fhQGfoDR1nnw0T2KI5C8!X009sH z0T2KI5CDO{^#n4BAxX~C@_$Gkl7_Rb^?yxLn`-TEJsbc40T2KI5C8!X009sH0T2KI z5CDNMPhgCe+!OiS3jHXGW3zh0-`&f~X=&o*UbDh~Eo92ocdGoibv`Q8woS`$nO%7M zUQfVc`exznsgKWpR0u!L9DJOb`uGDL>dGCadNY#hBBlD^e1U$vz|tH1_hjC__rb?} z|9@2ZN0Rzq)gP(y zK;Y!)5ju@Nk;}=$x!{$}W^+<9;j$Y2Lb`F{5Q&F-#ck=ykshhcGbcRL-l8AjOrIF; z5r<{Bi_VfX+go(9mH9+QC~f6EpMI6Ab*@|MTuZM?X?C%ENxL5W8Xu!d42jBWt9d#_ zWo0KOV#?xo2re1NlSgQ`zE|0nuE!H1VN2DMG6^FDUkM;E&kA4%$`>VH#z zLiYfEp#HAGG38h^ zd`wj)qTvZeIT{IPkB%w%XgIGb--w34p(w9L!kJgclq1pb5mgzFhQ}4lPLQAVQSk+d=#2`7hBN;VqK zCY4MyoJlC@NH~#BDMQilP*O=n!>L3rc}N;=`$IC7OGJW+1hD>p4vYX5KmY_l00ck)1V8`;KmY_l00cnb zYbJp8|JMu{#e)C{fB*=900@8p2!H?xfB*=9z;i$V>;LD#2v7k8KmY_l00ck)1V8`; zKmY_l00h2f0(|{1tN&f1e|Uia2!H?xfB*=900@8p2!H?xfB*=*&;*Xkqx3|;@mw~S z%j9yuc5mu~fB5sTLgD)Vb4mUDg{~l!2LTWO0T2KI5C8!X009sH0T2KI5D*eb$tj6` zaq-Z1q?}ZfMy_RlC-YhQv-I^LJ2jnrocNFOk7Y@!sSkyjpCxkd*so;rx2EO2l4%?4 zVRg$jJ>T2)sw-A=%e3cpU#~W8^TQ^q-enUoWjMR=PIFQ)e>yi3#~pEvQQ+`02>-{cN}PVdNmS>$=_4tu@E>3)<59 z?eglvTu1vUHZwCr6(|)tb#7PeB1O-Nqv!ey;;92~xXG*rbF1^F*Rb^6O9gFqae20L zy`}ZVMRCf@f|SL*Z=Mb?>|YHqtT5Me?C^cE8$%fR3&7BCT!J8~13~DECWQ-8g#+VE zh3~)8t#BCmOILXR%~P5D8*j*uzZDQ}wS_rmS?x9xe14-_`Dn61RS3#lWt(C*sySAy z%yo8nQ)@TGs8hO*L!Lalyu4U0EpDD?UA635-rN!00lVXsaf(r?B zq56|AE*SlzVDZswCo=hS=j10;k%N($tJJi*G~~h8W3}VbL7LGxAJkdwLo_PYP0tI* zX;Ht@=(vo?M0(A-?zr`)rN>C;9IIKk#n(Q_yrR1vZ+QJa5Yex~9tt}nmk6vl#2{`M z=RQ{Qeb?M*`kYn&RqCwn^u-tV&mRvAM86dsF;K-dx3;Lsw|e__@HL@<+R{M=3JQ~vNiL&F+(Dkmc?FZ z*7YrRlbKuF|Q2bmwU`P+7QL?hNEDYx6|w4a?Du*i^;6 zwpHGbSls*7 zqZ#f(-KyWwEz{70wM0w#NX&7e`jG^KDzF*fciAz6(ty@leDq2_lP{IzCl{jR@6de3 zt9s##rMhV{%V<}lUvf-bo&JP_sx^4V7fJ{An1iNIJMk_|qAlhx2dlTep~+0XJSFd) ziY$t&k)}-~TJ5(*BO*K3q7YpOi}PyBORZIKRDQNoc9HM@r__HfssB^`PwEfp1708i z0w4eaAOHd&00JNY0w4eaAOHftWCG)I1xln zgegl#oi+px%h9;y9h)-a`G3D;lj8Cq00JNY0w4eaAOHd&00JNY0wD1JfdF6sC)6L) z^Z)1#FAx9$5C8!X009sH0T2KI5C8!X0DC8t3`XZy%S_I#y@(KkIB9 z;;Vm>lq|Ex`1*fDy&`6ZHq`KUaTK-BFo(ha&L;0T2KI5C8!X009sH0T2KI z5C8!Xh!V(+Wa+E-+3a|3gm19thTFkm9vsV3a9GZck7pCK=T2+z!x^4nI6I!t@C3n9 zd^{b*$>aHSBGryOJjAmOQ{)iO8YDcNYG+P$G9OOzbaaV)GD+!zMgLf$6*rz}U3N^) zO2hOT&$r9M_y412#Agry0T2KI5C8!X009sH0T2KI5O_HWjPPoWy_|JJtv~<-KmY_l w00ck)1V8`;KmY_l00cq;SpUNlKmY_l00ck)1V8`;KmY_l00cnb 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', ''] + ); + }); +});