Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 task done
cravindra opened this issue Aug 2, 2024 · 7 comments
Closed
1 task done
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Filtering Related to the data grid Filtering feature good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@cravindra
Copy link

cravindra commented Aug 2, 2024

Latest version

  • 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 cravindra added bug 🐛 Something doesn't work status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 2, 2024
@github-actions github-actions bot added the component: data grid This is the name of the generic UI component, not the React module! label Aug 2, 2024
@michelengelen michelengelen changed the title [DataGrid] Boolean operator is does not filter for false value correctly [data grid] Boolean operator is does not filter for false value correctly Aug 2, 2024
@michelengelen michelengelen added feature: Filtering Related to the data grid Filtering feature and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 2, 2024
@michelengelen michelengelen added the good first issue Great for first contributions. Enable to learn the contribution process. label Aug 2, 2024
@github-project-automation github-project-automation bot moved this to 🆕 Needs refinement in MUI X Data Grid Aug 2, 2024
@michelengelen
Copy link
Member

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

if (!filterItem.value) {
return null;
}

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;
       }

@flaviendelangle
Copy link
Member

@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

@timozander
Copy link

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

@cravindra
Copy link
Author

This is fixed in #15252

Copy link

github-actions bot commented Nov 4, 2024

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.

@michelengelen
Copy link
Member

michelengelen commented Nov 5, 2024

@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! 🙇🏼

@cravindra
Copy link
Author

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Filtering Related to the data grid Filtering feature good first issue Great for first contributions. Enable to learn the contribution process.
Projects
Status: 🆕 Needs refinement
Development

Successfully merging a pull request may close this issue.

4 participants