diff --git a/packages/pluggableWidgets/datagrid-web/CHANGELOG.md b/packages/pluggableWidgets/datagrid-web/CHANGELOG.md index 178fb4a63d..732b2ab281 100644 --- a/packages/pluggableWidgets/datagrid-web/CHANGELOG.md +++ b/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. diff --git a/packages/pluggableWidgets/datagrid-web/e2e/filtering/DataGridFilteringEmptyString.spec.js b/packages/pluggableWidgets/datagrid-web/e2e/filtering/DataGridFilteringEmptyString.spec.js new file mode 100644 index 0000000000..e33a2cdb21 --- /dev/null +++ b/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)"]); + }); +}); diff --git a/packages/shared/filter-commons/src/__tests__/condition-utils.spec.ts b/packages/shared/filter-commons/src/__tests__/condition-utils.spec.ts index 70e06443ca..4edbae008b 100644 --- a/packages/shared/filter-commons/src/__tests__/condition-utils.spec.ts +++ b/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); + }); + }); }); diff --git a/packages/shared/filter-commons/src/condition-utils.ts b/packages/shared/filter-commons/src/condition-utils.ts index 107b10012f..db8362ad96 100644 --- a/packages/shared/filter-commons/src/condition-utils.ts +++ b/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( 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); diff --git a/packages/shared/widget-plugin-filtering/src/stores/input/BaseInputFilterStore.ts b/packages/shared/widget-plugin-filtering/src/stores/input/BaseInputFilterStore.ts index e6fa451a85..2f87f4886c 100644 --- a/packages/shared/widget-plugin-filtering/src/stores/input/BaseInputFilterStore.ts +++ b/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( ); } + 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( 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)); }