From 2933571c62e9a8f58a24cdb73e5d7eed768b8812 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 28 Feb 2024 08:34:41 +0000 Subject: [PATCH 1/8] update runLuceneQuery in client to allow for all filter matching --- packages/shared-core/src/filters.ts | 59 +++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 15 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 46d765a7b5..2c4861ed60 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -390,23 +390,52 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { } ) - // Match a document against all criteria const docMatch = (doc: any) => { - return ( - stringMatch(doc) && - fuzzyMatch(doc) && - rangeMatch(doc) && - equalMatch(doc) && - notEqualMatch(doc) && - emptyMatch(doc) && - notEmptyMatch(doc) && - oneOf(doc) && - contains(doc) && - containsAny(doc) && - notContains(doc) - ) - } + // Determine active filters based on query object + const activeFilterKeys = Object.entries(query || {}) + .filter( + ([key, value]) => + !["allOr", "onEmptyFilter"].includes(key) && + Object.keys(value).length > 0 + ) + .map(([key]) => key) + // Apply filters dynamically based on activeFilterKeys + const results = activeFilterKeys.map(filterKey => { + switch (filterKey) { + case "string": + return stringMatch(doc) + case "fuzzy": + return fuzzyMatch(doc) + case "range": + return rangeMatch(doc) + case "equal": + return equalMatch(doc) + case "notEqual": + return notEqualMatch(doc) + case "empty": + return emptyMatch(doc) + case "notEmpty": + return notEmptyMatch(doc) + case "oneOf": + return oneOf(doc) + case "contains": + return contains(doc) + case "containsAny": + return containsAny(doc) + case "notContains": + return notContains(doc) + default: + return true // If the filter type is not recognized, default to true (assuming pass) + } + }) + + if (query!.allOr) { + return results.some(result => result === true) + } else { + return results.every(result => result === true) + } + } // Process all docs return docs.filter(docMatch) } From e3c514e45aee8398c4b65b530c86536f7169dafd Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Mon, 4 Mar 2024 09:48:47 +0000 Subject: [PATCH 2/8] Update test lucene builder and add more tests --- packages/shared-core/src/filters.ts | 57 ++++---- .../shared-core/src/tests/filters.test.ts | 128 +++++++++++++----- 2 files changed, 116 insertions(+), 69 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 2c4861ed60..5f975ff541 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -391,43 +391,32 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { ) const docMatch = (doc: any) => { - // Determine active filters based on query object - const activeFilterKeys = Object.entries(query || {}) + const filterFunctions = { + string: stringMatch, + fuzzy: fuzzyMatch, + range: rangeMatch, + equal: equalMatch, + notEqual: notEqualMatch, + empty: emptyMatch, + notEmpty: notEmptyMatch, + oneOf: oneOf, + contains: contains, + containsAny: containsAny, + notContains: notContains, + } + const activeFilterKeys: (keyof typeof filterFunctions)[] = Object.entries( + query + ) .filter( - ([key, value]) => + ([key, value]: [string, any]) => !["allOr", "onEmptyFilter"].includes(key) && - Object.keys(value).length > 0 + Object.keys(value as Record).length > 0 ) - .map(([key]) => key) + .map(([key]) => key as keyof typeof filterFunctions) - // Apply filters dynamically based on activeFilterKeys - const results = activeFilterKeys.map(filterKey => { - switch (filterKey) { - case "string": - return stringMatch(doc) - case "fuzzy": - return fuzzyMatch(doc) - case "range": - return rangeMatch(doc) - case "equal": - return equalMatch(doc) - case "notEqual": - return notEqualMatch(doc) - case "empty": - return emptyMatch(doc) - case "notEmpty": - return notEmptyMatch(doc) - case "oneOf": - return oneOf(doc) - case "contains": - return contains(doc) - case "containsAny": - return containsAny(doc) - case "notContains": - return notContains(doc) - default: - return true // If the filter type is not recognized, default to true (assuming pass) - } + const results: boolean[] = activeFilterKeys.map(filterKey => { + const filterFunction = filterFunctions[filterKey] + return filterFunction ? filterFunction(doc) : true }) if (query!.allOr) { @@ -436,7 +425,7 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { return results.every(result => result === true) } } - // Process all docs + return docs.filter(docMatch) } diff --git a/packages/shared-core/src/tests/filters.test.ts b/packages/shared-core/src/tests/filters.test.ts index 8586d58777..1e0a68de89 100644 --- a/packages/shared-core/src/tests/filters.test.ts +++ b/packages/shared-core/src/tests/filters.test.ts @@ -47,10 +47,7 @@ describe("runLuceneQuery", () => { }, ] - function buildQuery( - filterKey: string, - value: { [key: string]: any } - ): SearchQuery { + function buildQuery(filters: { [filterKey: string]: any }): SearchQuery { const query: SearchQuery = { string: {}, fuzzy: {}, @@ -63,8 +60,13 @@ describe("runLuceneQuery", () => { notContains: {}, oneOf: {}, containsAny: {}, + allOr: false, } - query[filterKey as SearchQueryOperators] = value + + for (const filterKey in filters) { + query[filterKey as SearchQueryOperators] = filters[filterKey] + } + return query } @@ -73,16 +75,17 @@ describe("runLuceneQuery", () => { }) it("should return matching rows for equal filter", () => { - const query = buildQuery("equal", { - order_status: 4, + const query = buildQuery({ + equal: { order_status: 4 }, }) expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([1, 2]) }) it("should return matching row for notEqual filter", () => { - const query = buildQuery("notEqual", { - order_status: 4, + const query = buildQuery({ + notEqual: { order_status: 4 }, }) + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([3]) }) @@ -90,48 +93,56 @@ describe("runLuceneQuery", () => { expect( runLuceneQuery( docs, - buildQuery("fuzzy", { - description: "sm", + buildQuery({ + fuzzy: { description: "sm" }, }) ).map(row => row.description) ).toEqual(["Small box"]) expect( runLuceneQuery( docs, - buildQuery("string", { - description: "SM", + buildQuery({ + string: { description: "SM" }, }) ).map(row => row.description) ).toEqual(["Small box"]) }) it("should return rows within a range filter", () => { - const query = buildQuery("range", { - customer_id: { - low: 500, - high: 1000, + const query = buildQuery({ + range: { + customer_id: { + low: 500, + high: 1000, + }, }, }) + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([3]) }) it("should return rows with numeric strings within a range filter", () => { - const query = buildQuery("range", { - customer_id: { - low: "500", - high: "1000", + const query = buildQuery({ + range: { + customer_id: { + low: "500", + high: "1000", + }, }, }) expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([3]) }) it("should return rows with ISO date strings within a range filter", () => { - const query = buildQuery("range", { - order_date: { - low: "2016-01-04T00:00:00.000Z", - high: "2016-01-11T00:00:00.000Z", + const query = buildQuery({ + range: { + order_date: { + low: "2016-01-04T00:00:00.000Z", + high: "2016-01-11T00:00:00.000Z", + }, }, }) + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([2]) }) @@ -150,40 +161,87 @@ describe("runLuceneQuery", () => { label: "", }, ] - const query = buildQuery("range", { - order_date: { - low: "2016-01-04T00:00:00.000Z", - high: "2016-01-11T00:00:00.000Z", + + const query = buildQuery({ + range: { + order_date: { + low: "2016-01-04T00:00:00.000Z", + high: "2016-01-11T00:00:00.000Z", + }, }, }) + expect(runLuceneQuery(docs, query)).toEqual(docs) }) it("should return rows with matches on empty filter", () => { - const query = buildQuery("empty", { - label: null, + const query = buildQuery({ + empty: { + label: null, + }, }) + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([1]) }) it("should return rows with matches on notEmpty filter", () => { - const query = buildQuery("notEmpty", { - label: null, + const query = buildQuery({ + notEmpty: { + label: null, + }, }) + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([2, 3]) }) test.each([[523, 259], "523,259"])( "should return rows with matches on numeric oneOf filter", input => { - let query = buildQuery("oneOf", { - customer_id: input, + const query = buildQuery({ + oneOf: { + customer_id: input, + }, }) + expect(runLuceneQuery(docs, query).map(row => row.customer_id)).toEqual([ 259, 523, ]) } ) + + it("should return matching results if allOr is true and only one filter matches", () => { + const query = buildQuery({ + allOr: true, + oneOf: { staff_id: [10] }, + contains: { description: ["box"] }, + }) + + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([ + 1, 2, 3, + ]) + }) + + // what should the name of this test be if it's the same test as above but with different operands + + it("should return matching results if allOr is true and only one filter matches with different operands", () => { + const query = buildQuery({ + allOr: true, + equal: { order_status: 4 }, + oneOf: { label: ["FRAGILE"] }, + }) + + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([1, 2]) + }) + + it("should return nothing if allOr is false and only one filter matches", () => { + const query = buildQuery({ + allOr: false, + oneOf: { staff_id: [10] }, + contains: { description: ["box"] }, + }) + + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([]) + }) }) describe("buildLuceneQuery", () => { From 5679acb86811c290dd84faf1d81b19d615680f6b Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Mon, 4 Mar 2024 09:55:28 +0000 Subject: [PATCH 3/8] fix types --- packages/shared-core/src/filters.ts | 34 ++++++++++++++--------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 5f975ff541..6d81bbdc62 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -391,28 +391,28 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { ) const docMatch = (doc: any) => { - const filterFunctions = { - string: stringMatch, - fuzzy: fuzzyMatch, - range: rangeMatch, - equal: equalMatch, - notEqual: notEqualMatch, - empty: emptyMatch, - notEmpty: notEmptyMatch, - oneOf: oneOf, - contains: contains, - containsAny: containsAny, - notContains: notContains, - } - const activeFilterKeys: (keyof typeof filterFunctions)[] = Object.entries( - query - ) + const filterFunctions: Record boolean> = + { + string: stringMatch, + fuzzy: fuzzyMatch, + range: rangeMatch, + equal: equalMatch, + notEqual: notEqualMatch, + empty: emptyMatch, + notEmpty: notEmptyMatch, + oneOf: oneOf, + contains: contains, + containsAny: containsAny, + notContains: notContains, + } + + const activeFilterKeys: SearchQueryOperators[] = Object.entries(query) .filter( ([key, value]: [string, any]) => !["allOr", "onEmptyFilter"].includes(key) && Object.keys(value as Record).length > 0 ) - .map(([key]) => key as keyof typeof filterFunctions) + .map(([key]) => key as any) const results: boolean[] = activeFilterKeys.map(filterKey => { const filterFunction = filterFunctions[filterKey] From 3d9a7e5ddf5f76236a304e65239c899e8e865cd7 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Mon, 4 Mar 2024 10:07:06 +0000 Subject: [PATCH 4/8] fix type --- packages/shared-core/src/filters.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 6d81bbdc62..0a1673e558 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -406,7 +406,7 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { notContains: notContains, } - const activeFilterKeys: SearchQueryOperators[] = Object.entries(query) + const activeFilterKeys: SearchQueryOperators[] = Object.entries(query || {}) .filter( ([key, value]: [string, any]) => !["allOr", "onEmptyFilter"].includes(key) && From b232371efff95f7925c93960ba92862324cb1a46 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 6 Mar 2024 10:01:42 +0000 Subject: [PATCH 5/8] remove uneeded comment --- packages/shared-core/src/tests/filters.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/shared-core/src/tests/filters.test.ts b/packages/shared-core/src/tests/filters.test.ts index 1e0a68de89..0cf7e0e92a 100644 --- a/packages/shared-core/src/tests/filters.test.ts +++ b/packages/shared-core/src/tests/filters.test.ts @@ -221,8 +221,6 @@ describe("runLuceneQuery", () => { ]) }) - // what should the name of this test be if it's the same test as above but with different operands - it("should return matching results if allOr is true and only one filter matches with different operands", () => { const query = buildQuery({ allOr: true, From eb00ce401f9819406acde58c60018945bc95864e Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 6 Mar 2024 10:10:28 +0000 Subject: [PATCH 6/8] pr comments --- packages/shared-core/src/filters.ts | 7 ++++--- packages/shared-core/src/tests/filters.test.ts | 10 ++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 0a1673e558..84b6076d56 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -12,6 +12,7 @@ import { import dayjs from "dayjs" import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants" import { deepGet } from "./helpers" +import test from "node:test" const HBS_REGEX = /{{([^{].*?)}}/g @@ -359,6 +360,7 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { const oneOf = match( SearchQueryOperators.ONE_OF, (docValue: any, testValue: any) => { + console.log(testValue) if (typeof testValue === "string") { testValue = testValue.split(",") if (typeof docValue === "number") { @@ -410,13 +412,13 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { .filter( ([key, value]: [string, any]) => !["allOr", "onEmptyFilter"].includes(key) && + value && Object.keys(value as Record).length > 0 ) .map(([key]) => key as any) const results: boolean[] = activeFilterKeys.map(filterKey => { - const filterFunction = filterFunctions[filterKey] - return filterFunction ? filterFunction(doc) : true + return filterFunctions[filterKey]?.(doc) ?? false }) if (query!.allOr) { @@ -425,7 +427,6 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { return results.every(result => result === true) } } - return docs.filter(docMatch) } diff --git a/packages/shared-core/src/tests/filters.test.ts b/packages/shared-core/src/tests/filters.test.ts index 0cf7e0e92a..1f8f534f0d 100644 --- a/packages/shared-core/src/tests/filters.test.ts +++ b/packages/shared-core/src/tests/filters.test.ts @@ -240,6 +240,16 @@ describe("runLuceneQuery", () => { expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([]) }) + + it("should handle when a value is null or undefined", () => { + const query = buildQuery({ + allOr: true, + equal: { order_status: null }, + oneOf: { label: ["FRAGILE"] }, + }) + + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([2]) + }) }) describe("buildLuceneQuery", () => { From 1f107041a108aeaf677da20659819bfe2d06ec03 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 6 Mar 2024 11:57:45 +0000 Subject: [PATCH 7/8] use vitest each --- .../shared-core/src/tests/filters.test.ts | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/packages/shared-core/src/tests/filters.test.ts b/packages/shared-core/src/tests/filters.test.ts index 1f8f534f0d..de969562af 100644 --- a/packages/shared-core/src/tests/filters.test.ts +++ b/packages/shared-core/src/tests/filters.test.ts @@ -209,16 +209,19 @@ describe("runLuceneQuery", () => { } ) - it("should return matching results if allOr is true and only one filter matches", () => { + test.each([ + [false, []], + [true, [1, 2, 3]], + ])("should return %s if allOr is %s ", (allOr, expectedResult) => { const query = buildQuery({ - allOr: true, + allOr, oneOf: { staff_id: [10] }, contains: { description: ["box"] }, }) - expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([ - 1, 2, 3, - ]) + expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual( + expectedResult + ) }) it("should return matching results if allOr is true and only one filter matches with different operands", () => { @@ -231,16 +234,6 @@ describe("runLuceneQuery", () => { expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([1, 2]) }) - it("should return nothing if allOr is false and only one filter matches", () => { - const query = buildQuery({ - allOr: false, - oneOf: { staff_id: [10] }, - contains: { description: ["box"] }, - }) - - expect(runLuceneQuery(docs, query).map(row => row.order_id)).toEqual([]) - }) - it("should handle when a value is null or undefined", () => { const query = buildQuery({ allOr: true, From 632b9a26f4313216c28458db46dc9334aea7e909 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 6 Mar 2024 14:42:30 +0000 Subject: [PATCH 8/8] remove log --- packages/shared-core/src/filters.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 84b6076d56..d9fe533c88 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -12,7 +12,6 @@ import { import dayjs from "dayjs" import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants" import { deepGet } from "./helpers" -import test from "node:test" const HBS_REGEX = /{{([^{].*?)}}/g @@ -360,7 +359,6 @@ export const runLuceneQuery = (docs: any[], query?: SearchQuery) => { const oneOf = match( SearchQueryOperators.ONE_OF, (docValue: any, testValue: any) => { - console.log(testValue) if (typeof testValue === "string") { testValue = testValue.split(",") if (typeof docValue === "number") {