[WC-3417] Fix string empty/not-empty filters

packages/pluggableWidgets/datagrid-web/CHANGELOG.md

@@ -12,6 +12,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ### Fixed
 
+- We fixed an issue where "is empty" and "is not empty" string filters were not working correctly in some cases.
+
 - We fixed an issue where custom content columns ignored the export type setting, causing numbers and dates to always export as text in Excel.
 
 - We fixed an issue where exported date values included a hidden time component even when the format specified date-only parts.

packages/pluggableWidgets/datagrid-web/e2e/filtering/DataGridFilteringEmptyString.spec.js

@@ -0,0 +1,34 @@
+import { expect, test } from "@mendix/run-e2e/fixtures";
+import { waitForMendixApp } from "@mendix/run-e2e/mendix-helpers";
+
+test.describe("datagrid-web filtering empty strings", () => {
+    test.beforeEach(async ({ page }) => {
+        await page.goto("/p/filtering-empty-string");
+        await waitForMendixApp(page);
+    });
+
+    test("filter rows by Empty and Not empty", async ({ page }) => {
+        const column = n => page.locator(`[role="gridcell"]:nth-child(${n})`);
+        const filter = n => page.locator(`[role="columnheader"]:nth-child(${n})`).locator(".filter-container");
+        const filterSelectorButton = n => filter(n).getByRole("combobox");
+        const filterSelectorOption = (n, name) =>
+            filter(n).getByRole("listbox").getByRole("option", { name, exact: true });
+
+        // all 3 records are shown
+        await expect(column(1)).toHaveText(["User 1 (with value)", 'User 3 ("")', "User 3 (empty)"]);
+
+        // select Empty option
+        await filterSelectorButton(2).click({ delay: 20 });
+        await filterSelectorOption(2, "Empty").click({ delay: 20 });
+
+        // both, `empty` and `""` records are visible. Record with text is filtered out.
+        await expect(column(1)).toHaveText(['User 3 ("")', "User 3 (empty)"]);
+
+        // select "Not empty" option
+        await filterSelectorButton(2).click({ delay: 20 });
+        await filterSelectorOption(2, "Not empty").click({ delay: 20 });
+
+        // Record with text is visible, `empty` and `""` records are filtered out.
+        await expect(column(1)).toHaveText(["User 1 (with value)"]);
+    });
+});

packages/shared/filter-commons/src/__tests__/condition-utils.spec.ts

@@ -1,8 +1,18 @@
 jest.mock("mendix/filters/builders");
 
 import { AndCondition, FilterCondition } from "mendix/filters";
-import { equals, literal } from "mendix/filters/builders";
-import { reduceArray, reduceMap, restoreArray, restoreMap, tag } from "../condition-utils";
+import { and, attribute, equals, literal, notEqual, or } from "mendix/filters/builders";
+import {
+    isEmptyExp,
+    isEmptyStringExp,
+    isNotEmptyExp,
+    isNotEmptyStringExp,
+    reduceArray,
+    reduceMap,
+    restoreArray,
+    restoreMap,
+    tag
+} from "../condition-utils";
 
 describe("condition-utils", () => {
     describe("reduceMap", () => {
@@ -412,4 +422,158 @@ describe("condition-utils", () => {
             expect(restored).toEqual(originalInput);
         });
     });
+
+    describe("isEmptyStringExp", () => {
+        it("identifies correct empty expression", () => {
+            const emptyStringExpression = or(
+                equals(attribute("testAttrId" as any), literal(undefined)),
+                equals(attribute("testAttrId" as any), literal(""))
+            );
+            expect(isEmptyStringExp(emptyStringExpression)).toBe(true);
+        });
+
+        it("returns false for expressions with different attribute ids", () => {
+            const mismatchedAttributes = or(
+                equals(attribute("testAttrId1" as any), literal(undefined)),
+                equals(attribute("testAttrId2" as any), literal(""))
+            );
+            expect(isEmptyStringExp(mismatchedAttributes)).toBe(false);
+        });
+
+        it("returns false for expressions with incorrect arguments order", () => {
+            const mismatchedOrder = or(
+                equals(attribute("testAttrId" as any), literal("")),
+                equals(attribute("testAttrId" as any), literal(undefined))
+            );
+            expect(isEmptyStringExp(mismatchedOrder)).toBe(false);
+        });
+
+        it("returns false for expressions with incorrect number of arguments", () => {
+            const mismatchedNumberOffArgs = or(
+                equals(attribute("testAttrId" as any), literal("")),
+                equals(attribute("testAttrId" as any), literal(undefined)),
+                equals(attribute("testAttrId" as any), literal(undefined))
+            );
+            expect(isEmptyStringExp(mismatchedNumberOffArgs)).toBe(false);
+        });
+
+        it("returns false for and-based expression (isNotEmptyStringExp shape)", () => {
+            const notEmptyStringExpression = and(
+                notEqual(attribute("testAttrId" as any), literal(undefined)),
+                notEqual(attribute("testAttrId" as any), literal(""))
+            );
+            expect(isEmptyStringExp(notEmptyStringExpression)).toBe(false);
+        });
+    });
+
+    describe("isNotEmptyStringExp", () => {
+        it("identifies correct not-empty expression", () => {
+            const notEmptyStringExpression = and(
+                notEqual(attribute("testAttrId" as any), literal(undefined)),
+                notEqual(attribute("testAttrId" as any), literal(""))
+            );
+            expect(isNotEmptyStringExp(notEmptyStringExpression)).toBe(true);
+        });
+
+        it("returns false for expressions with different attribute ids", () => {
+            const mismatchedAttributes = and(
+                notEqual(attribute("testAttrId1" as any), literal(undefined)),
+                notEqual(attribute("testAttrId2" as any), literal(""))
+            );
+            expect(isNotEmptyStringExp(mismatchedAttributes)).toBe(false);
+        });
+
+        it("returns false for expressions with incorrect arguments order", () => {
+            const mismatchedOrder = and(
+                notEqual(attribute("testAttrId" as any), literal("")),
+                notEqual(attribute("testAttrId" as any), literal(undefined))
+            );
+            expect(isNotEmptyStringExp(mismatchedOrder)).toBe(false);
+        });
+
+        it("returns false for expressions with incorrect number of arguments", () => {
+            const mismatchedNumberOfArgs = and(
+                notEqual(attribute("testAttrId" as any), literal(undefined)),
+                notEqual(attribute("testAttrId" as any), literal("")),
+                notEqual(attribute("testAttrId" as any), literal(undefined))
+            );
+            expect(isNotEmptyStringExp(mismatchedNumberOfArgs)).toBe(false);
+        });
+
+        it("returns false for or-based expression (isEmptyStringExp shape)", () => {
+            const emptyStringExpression = or(
+                equals(attribute("testAttrId" as any), literal(undefined)),
+                equals(attribute("testAttrId" as any), literal(""))
+            );
+            expect(isNotEmptyStringExp(emptyStringExpression)).toBe(false);
+        });
+    });
+
+    describe("isEmptyExp", () => {
+        it("returns true for equals expression with undefined literal", () => {
+            const emptyExp = equals(attribute("testAttrId" as any), literal(undefined));
+            expect(isEmptyExp(emptyExp)).toBe(true);
+        });
+
+        it("returns false for equals with non-undefined literal", () => {
+            const nonEmpty = equals(attribute("testAttrId" as any), literal("value"));
+            expect(isEmptyExp(nonEmpty)).toBe(false);
+        });
+
+        it("returns false for notEqual with undefined literal", () => {
+            const notEmptyExp = notEqual(attribute("testAttrId" as any), literal(undefined));
+            expect(isEmptyExp(notEmptyExp)).toBe(false);
+        });
+
+        it("returns false for non-binary expression", () => {
+            const orExp = or(
+                equals(attribute("testAttrId" as any), literal(undefined)),
+                equals(attribute("testAttrId" as any), literal(""))
+            );
+            expect(isEmptyExp(orExp)).toBe(false);
+        });
+
+        it("returns false for isEmptyStringExp expression", () => {
+            const emptyStringExpression = or(
+                equals(attribute("testAttrId" as any), literal(undefined)),
+                equals(attribute("testAttrId" as any), literal(""))
+            );
+            expect(isEmptyStringExp(emptyStringExpression)).toBe(true);
+            expect(isEmptyExp(emptyStringExpression)).toBe(false);
+        });
+    });
+
+    describe("isNotEmptyExp", () => {
+        it("returns true for notEqual expression with undefined literal", () => {
+            const notEmptyExp = notEqual(attribute("testAttrId" as any), literal(undefined));
+            expect(isNotEmptyExp(notEmptyExp)).toBe(true);
+        });
+
+        it("returns false for notEqual with non-undefined literal", () => {
+            const withValue = notEqual(attribute("testAttrId" as any), literal("value"));
+            expect(isNotEmptyExp(withValue)).toBe(false);
+        });
+
+        it("returns false for equals with undefined literal", () => {
+            const emptyExp = equals(attribute("testAttrId" as any), literal(undefined));
+            expect(isNotEmptyExp(emptyExp)).toBe(false);
+        });
+
+        it("returns false for non-binary expression", () => {
+            const andExp = and(
+                notEqual(attribute("testAttrId" as any), literal(undefined)),
+                notEqual(attribute("testAttrId" as any), literal(""))
+            );
+            expect(isNotEmptyExp(andExp)).toBe(false);
+        });
+
+        it("returns false for isNotEmptyStringExp expression", () => {
+            const notEmptyStringExpression = and(
+                notEqual(attribute("testAttrId" as any), literal(undefined)),
+                notEqual(attribute("testAttrId" as any), literal(""))
+            );
+            expect(isNotEmptyStringExp(notEmptyStringExpression)).toBe(true);
+            expect(isNotEmptyExp(notEmptyStringExpression)).toBe(false);
+        });
+    });
 });

packages/shared/filter-commons/src/condition-utils.ts

@@ -4,7 +4,8 @@ import {
     EqualsCondition,
     FilterCondition,
     LiteralExpression,
-    OrCondition
+    OrCondition,
+    ValueExpression
 } from "mendix/filters";
 import { and, literal, notEqual } from "mendix/filters/builders";
 
@@ -26,12 +27,62 @@ export function isOr(exp: FilterCondition): exp is OrCondition {
     return exp.type === "function" && exp.name === "or";
 }
 
+function isSameAttr(a: ValueExpression, b: ValueExpression): boolean {
+    return a.type === "attribute" && b.type === "attribute" && a.attributeId === b.attributeId;
+}
+
+function isUndefinedLiteral(a: ValueExpression): boolean {
+    return a.type === "literal" && a.valueType === "undefined";
+}
+
+function isEmptyStringLiteral(a: ValueExpression): boolean {
+    return a.type === "literal" && a.valueType === "string" && a.value === "";
+}
+
+export function isEmptyStringExp(exp: FilterCondition): boolean {
+    if (!isOr(exp)) {
+        return false;
+    }
+
+    if (exp.args.length !== 2) {
+        return false;
+    }
+
+    const [left, right] = exp.args;
+
+    if (!isBinary(left) || left.name !== "=" || !isBinary(right) || right.name !== "=") {
+        return false;
+    }
+
+    // identical attributes compared to undefined and empty string
+    return isSameAttr(left.arg1, right.arg1) && isUndefinedLiteral(left.arg2) && isEmptyStringLiteral(right.arg2);
+}
+
+export function isNotEmptyStringExp(exp: FilterCondition): boolean {
+    if (!isAnd(exp)) {
+        return false;
+    }
+
+    if (exp.args.length !== 2) {
+        return false;
+    }
+
+    const [left, right] = exp.args;
+
+    if (!isBinary(left) || left.name !== "!=" || !isBinary(right) || right.name !== "!=") {
+        return false;
+    }
+
+    // identical attributes compared to undefined and empty string
+    return isSameAttr(left.arg1, right.arg1) && isUndefinedLiteral(left.arg2) && isEmptyStringLiteral(right.arg2);
+}
+
 export function isEmptyExp(exp: FilterCondition): boolean {
-    return isBinary(exp) && exp.arg2.type === "literal" && exp.name === "=" && exp.arg2.valueType === "undefined";
+    return isBinary(exp) && exp.name === "=" && isUndefinedLiteral(exp.arg2);
 }
 
 export function isNotEmptyExp(exp: FilterCondition): boolean {
-    return isBinary(exp) && exp.arg2.type === "literal" && exp.name === "!=" && exp.arg2.valueType === "undefined";
+    return isBinary(exp) && exp.name === "!=" && isUndefinedLiteral(exp.arg2);
 }
 
 interface TagName {
@@ -178,7 +229,15 @@ export function inputStateFromCond<Fn, V>(
     cond: FilterCondition,
     fn: (func: FilterFunction | "between" | "empty" | "notEmpty") => Fn,
     val: (exp: LiteralExpression) => V
-): null | [Fn, V] | [Fn, V, V] {
+): null | [Fn] | [Fn, V] | [Fn, V, V] {
+    if (isEmptyStringExp(cond)) {
+        return [fn("empty")];
+    }
+
+    if (isNotEmptyStringExp(cond)) {
+        return [fn("notEmpty")];
+    }
+
     // Or - condition build for multiple attrs, get state from the first one.
     if (isOr(cond)) {
         return inputStateFromCond(cond.args[0], fn, val);

packages/shared/widget-plugin-filtering/src/stores/input/BaseInputFilterStore.ts

@@ -5,6 +5,7 @@ import {
     and,
     attribute,
     contains,
+    empty,
     endsWith,
     equals,
     greaterThan,
@@ -140,6 +141,25 @@ function getFilterCondition<T extends string | Big | Date>(
         );
     }
 
+    if (operation === "empty") {
+        if (listAttribute.type === "String") {
+            return or(equals(attribute(listAttribute.id), empty()), equals(attribute(listAttribute.id), literal("")));
+        } else {
+            return equals(attribute(listAttribute.id), empty());
+        }
+    }
+
+    if (operation === "notEmpty") {
+        if (listAttribute.type === "String") {
+            return and(
+                notEqual(attribute(listAttribute.id), empty()),
+                notEqual(attribute(listAttribute.id), literal(""))
+            );
+        } else {
+            return notEqual(attribute(listAttribute.id), empty());
+        }
+    }
+
     const filters = {
         contains,
         startsWith,
@@ -149,13 +169,8 @@ function getFilterCondition<T extends string | Big | Date>(
         equal: equals,
         notEqual,
         smaller: lessThan,
-        smallerEqual: lessThanOrEqual,
-        empty: equals,
-        notEmpty: notEqual
+        smallerEqual: lessThanOrEqual
     };
 
-    return filters[operation](
-        attribute(listAttribute.id),
-        literal(operation === "empty" || operation === "notEmpty" ? undefined : value)
-    );
+    return filters[operation](attribute(listAttribute.id), literal(value));
 }