mui-x icon indicating copy to clipboard operation
mui-x copied to clipboard

[data grid] Boolean operator `is` does not filter for `false` value correctly

Open cravindra opened this issue 1 year ago • 3 comments

Latest version

  • [X] I have tested the latest version

Steps to reproduce

Link to live example: https://codesandbox.io/s/wizardly-glitter-3234st?file=/src/Demo.tsx

Current behavior

All 4 items are displayed when trying to filter for false values

Expected behavior

Only item 3 and 4 that have isAdmin set to false should be shown

Context

Set up:

With columns and rows:


const rows = [
    {
      id: 1,
      isAdmin: true,
      name: "Item 1"
    },
    {
      id: 2,
      isAdmin: true,
      name: "Item 2"
    },
    {
      id: 3,
      isAdmin: false,
      name: "Item 3"
    },
    {
      id: 4,
      isAdmin: false,
      name: "Item 4"
    }
  ];

const cols = [
    {
      field: "name",
      type: "string"
    },
    {
      field: "isAdmin",
      type: "boolean"
    }
  ];

and when the filterModel is set to:

{
      items: [
        {
          field: "isAdmin",
          operator: "is",
          value: false
        }
      ]
    }

Expected behaviour

I'd expect only items 3 and 4 to be displayed

Actual behaviour

All rows are shown

RCA

This is implementation of the is operator for boolean columns:

import { GridFilterInputBoolean } from '../components/panel/filterPanel/GridFilterInputBoolean';
import { GridFilterItem } from '../models/gridFilterItem';
import { GridFilterOperator } from '../models/gridFilterOperator';

export const getGridBooleanOperators = (): GridFilterOperator<any, boolean | null, any>[] => [
  {
    value: 'is',
    getApplyFilterFn: (filterItem: GridFilterItem) => {
      if (!filterItem.value) {
        return null;
      }

      const valueAsBoolean = String(filterItem.value) === 'true';
      return (value): boolean => {
        return Boolean(value) === valueAsBoolean;
      };
    },
    InputComponent: GridFilterInputBoolean,
  },
];

Defined here: packages/x-data-grid/src/colDef/gridBooleanOperators.ts

When the value is false for a boolean field, the check of if(!filterItem.value){...} fails and we return null that turns off filtering altogether.

Workaround

My guess is that the default filter menu sets the value prop on the filter model as a string:

{
      items: [
        {
          field: "isAdmin",
          operator: "is",
          value: "false"
        }
      ]
    }

and can be used if we can control the filterModel (as can be done in the live working reproduction code)

Your environment

npx @mui/envinfo
 System:
    OS: macOS 14.5
  Binaries:
    Node: 20.10.0 - ~/.nvm/versions/node/v20.10.0/bin/node
    npm: 10.2.3 - ~/.nvm/versions/node/v20.10.0/bin/npm
    pnpm: Not Found
  Browsers:
    Chrome: 127.0.6533.89
    Edge: Not Found
    Safari: 17.5
  npmPackages:
    @emotion/react: ^11.11.1 => 11.11.1 
    @emotion/styled: ^11.11.0 => 11.11.0 
    @mui/base:  5.0.0-beta.18 
    @mui/core-downloads-tracker:  5.16.6 
    @mui/icons-material: ^5.14.12 => 5.14.12 
    @mui/lab: ^5.0.0-alpha.132 => 5.0.0-alpha.147 
    @mui/material: ^5.14.12 => 5.16.6 
    @mui/private-theming:  5.16.6 
    @mui/styled-engine:  5.16.6 
    @mui/system:  5.16.6 
    @mui/types:  7.2.15 
    @mui/utils: ^5.14.12 => 5.16.6 
    @mui/x-data-grid: ^7.12.0 => 7.12.0 
    @mui/x-date-pickers: ^6.16.0 => 6.16.0 
    @mui/x-internals:  7.12.0 
    @mui/x-tree-view:  6.0.0-alpha.1 
    @types/react: ^18.2.25 => 18.2.25 
    react: ^18.2.0 => 18.2.0 
    react-dom: ^18.2.0 => 18.2.0 
    typescript: ^4.9.5 => 4.9.5 

Browser info:

Arc	127.0.6533.73 (Official Build) (arm64) 
Revision	b59f345ebd6c6bd0b5eb2a715334e912b514773d-refs/branch-heads/6533@{#1761}
OS	macOS Version 14.5 (Build 23F79)
JavaScript	V8 12.7.224.16
User agent	Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/127.0.0.0 Safari/537.36
Command Line	******* --flag-switches-begin --flag-switches-end
Executable Path	*****
Profile Path	*****
Linker	lld
Variations Seed Type	Null

Search keywords: is, boolean, filter, data-grid, datagrid

cravindra avatar Aug 02 '24 08:08 cravindra

Hey @cravindra Thanks for raising this. There is a bad if clause in the getApplyFilterFn that causes wrong behavior.

https://github.com/mui/mui-x/blob/7d86cfd390f19bee22531ea25dc444d83c38e6b6/packages/x-data-grid/src/colDef/gridBooleanOperators.ts#L9-L11

Here is a diff with a fix:

diff --git a/packages/x-data-grid/src/colDef/gridBooleanOperators.ts b/packages/x-data-grid/src/colDef/gridBooleanOperators.ts
index 840019455..07f0f6b1f 100644
--- a/packages/x-data-grid/src/colDef/gridBooleanOperators.ts
+++ b/packages/x-data-grid/src/colDef/gridBooleanOperators.ts
@@ -6,7 +6,7 @@ export const getGridBooleanOperators = (): GridFilterOperator<any, boolean | nul
   {
     value: 'is',
     getApplyFilterFn: (filterItem: GridFilterItem) => {
-      if (!filterItem.value) {
+      if (filterItem.value === null || filterItem.value === undefined) {
         return null;
       }

michelengelen avatar Aug 02 '24 13:08 michelengelen

@michelengelen out of curiosity, what is the advantage of your code compared to the simpler:

diff --git a/packages/x-data-grid/src/colDef/gridBooleanOperators.ts b/packages/x-data-grid/src/colDef/gridBooleanOperators.ts
index 840019455..07f0f6b1f 100644
--- a/packages/x-data-grid/src/colDef/gridBooleanOperators.ts
+++ b/packages/x-data-grid/src/colDef/gridBooleanOperators.ts
@@ -6,7 +6,7 @@ export const getGridBooleanOperators = (): GridFilterOperator<any, boolean | nul
   {
     value: 'is',
     getApplyFilterFn: (filterItem: GridFilterItem) => {
-      if (!filterItem.value) {
+      if (filterItem.value == null) {
         return null;
       }

For me this is THE use case where double equality is superior to triple one

flaviendelangle avatar Aug 05 '24 07:08 flaviendelangle

This is a fundamental feature that is just fully broken since V7, it's majorly impacting us and is unfixed since August. Please take some time to look at this.

Order ID: 95800

timozander avatar Oct 31 '24 07:10 timozander

This is fixed in https://github.com/mui/mui-x/pull/15252

cravindra avatar Nov 04 '24 21:11 cravindra

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

[!NOTE] @cravindra How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

github-actions[bot] avatar Nov 04 '24 21:11 github-actions[bot]

@cravindra Sry that we missed your PR with the fix completely. We can certainly do better than this. Next time make sure to tag me on your opened PR and I will handle adding reviewers and see it through! Again: Sry that this happened and I hope you understand that this can, but shouldn't, happen in a big codebase like this! 🙇🏼

michelengelen avatar Nov 05 '24 10:11 michelengelen

Thanks, @michelengelen! I appreciate the acknowledgement - no hard feelings - I can barely keep up on my tiny codebases so I can completely understand this happening on large project like MUI

Glad that this is fixed though which I guess is the important part :)

cravindra avatar Nov 05 '24 12:11 cravindra