Skip to content

Commit 5b7f77b

Browse files
NicolappsConvex, Inc.
authored andcommitted
system-udfs: make search filter clauses an array (#40632)
I realized when working on the UI that it’s more convenient for SearchIndexFilters.clauses to be an array rather than an object. In particular, it ensures that the order is stable, and allows users to define multiple filters for the same field (but enable only one of them). There are no worries about backwards compatibility since the feature isn’t live yet. GitOrigin-RevId: 2b7f029cded4cc89d83c40a8499300e8243f9eb1
1 parent f0ca1c0 commit 5b7f77b

File tree

3 files changed

+114
-74
lines changed

3 files changed

+114
-74
lines changed

npm-packages/system-udfs/convex/_system/frontend/lib/filters.test.ts

Lines changed: 81 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -548,10 +548,10 @@ describe("filters", () => {
548548
it("should apply search index filters correctly", () => {
549549
const mockBuilder = new MockSearchBuilder();
550550
const searchString = "test search";
551-
const searchIndexFilters = {
552-
userId: { enabled: true, value: "user123" },
553-
organizationId: { enabled: true, value: 42 },
554-
};
551+
const searchIndexFilters = [
552+
{ field: "userId", enabled: true, value: "user123" },
553+
{ field: "organizationId", enabled: true, value: 42 },
554+
];
555555
const selectedIndex: SearchIndex = {
556556
indexDescriptor: "testSearchIndex",
557557
searchField: "description",
@@ -572,38 +572,13 @@ describe("filters", () => {
572572
]);
573573
});
574574

575-
it("should handle empty filters object", () => {
576-
const mockBuilder = new MockSearchBuilder();
577-
const searchString = "test search";
578-
const searchIndexFilters = {};
579-
const selectedIndex: SearchIndex = {
580-
indexDescriptor: "testSearchIndex",
581-
searchField: "description",
582-
filterFields: ["userId", "organizationId"],
583-
};
584-
585-
applySearchIndexFilters(
586-
mockBuilder as any,
587-
searchString,
588-
searchIndexFilters,
589-
selectedIndex,
590-
);
591-
592-
expect(mockBuilder.operations).toEqual([
593-
{ op: "search", field: "description", value: "test search" },
594-
]);
595-
});
596-
597575
it("should only apply filters for fields that exist in the filter", () => {
598576
const mockBuilder = new MockSearchBuilder();
599577
const searchString = "test search";
600-
const searchIndexFilters = {
601-
userId: {
602-
enabled: true,
603-
value: "user123",
604-
},
578+
const searchIndexFilters = [
579+
{ field: "userId", enabled: true, value: "user123" },
605580
// organizationId is missing
606-
};
581+
];
607582
const selectedIndex: SearchIndex = {
608583
indexDescriptor: "testSearchIndex",
609584
searchField: "description",
@@ -626,16 +601,18 @@ describe("filters", () => {
626601
it("should only apply filters for enabled fields", () => {
627602
const mockBuilder = new MockSearchBuilder();
628603
const searchString = "test search";
629-
const searchIndexFilters = {
630-
userId: {
604+
const searchIndexFilters = [
605+
{
606+
field: "userId",
631607
enabled: true,
632608
value: "user123",
633609
},
634-
organizationId: {
610+
{
611+
field: "organizationId",
635612
enabled: false,
636613
value: "org123",
637614
},
638-
};
615+
];
639616
const selectedIndex: SearchIndex = {
640617
indexDescriptor: "testSearchIndex",
641618
searchField: "description",
@@ -658,7 +635,6 @@ describe("filters", () => {
658635
it("should handle search index with no filter fields", () => {
659636
const mockBuilder = new MockSearchBuilder();
660637
const searchString = "test search";
661-
const searchIndexFilters = {};
662638
const selectedIndex: SearchIndex = {
663639
indexDescriptor: "testSearchIndex",
664640
searchField: "description",
@@ -668,7 +644,7 @@ describe("filters", () => {
668644
applySearchIndexFilters(
669645
mockBuilder as any,
670646
searchString,
671-
searchIndexFilters,
647+
[],
672648
selectedIndex,
673649
);
674650

@@ -805,10 +781,10 @@ describe("filters", () => {
805781

806782
const result = validateSearchIndexFilter(
807783
indexName,
808-
{
809-
userId: { enabled: true, value: BigInt(123) },
810-
organization: { enabled: true, value: "exampleOrg" },
811-
},
784+
[
785+
{ field: "userId", enabled: true, value: BigInt(123) },
786+
{ field: "organization", enabled: true, value: "exampleOrg" },
787+
],
812788
selectedIndex,
813789
"asc",
814790
);
@@ -821,7 +797,7 @@ describe("filters", () => {
821797

822798
const result = validateSearchIndexFilter(
823799
indexName,
824-
{},
800+
[],
825801
selectedIndex,
826802
"asc",
827803
);
@@ -841,15 +817,15 @@ describe("filters", () => {
841817

842818
const result = validateSearchIndexFilter(
843819
indexName,
844-
{
845-
userId: { enabled: true, value: BigInt(123) },
846-
organization: { enabled: true, value: "exampleOrg" },
847-
},
820+
[
821+
{ field: "userId", enabled: true, value: BigInt(123) },
822+
{ field: "organization", enabled: true, value: "exampleOrg" },
823+
],
848824
selectedIndex,
849825
"asc",
850826
);
851827
expect(result).toEqual({
852-
filter: -1,
828+
filter: 1,
853829
error:
854830
"Invalid index filter selection: found a filter for field `organization` which is not part of the filter fields of the search index `testIndex`.",
855831
});
@@ -865,17 +841,17 @@ describe("filters", () => {
865841

866842
const result = validateSearchIndexFilter(
867843
indexName,
868-
{
869-
userId: { enabled: true, value: BigInt(123) },
870-
organization: { enabled: false, value: "exampleOrg" },
871-
},
844+
[
845+
{ field: "userId", enabled: true, value: BigInt(123) },
846+
{ field: "organization", enabled: false, value: "exampleOrg" },
847+
],
872848
selectedIndex,
873849
"asc",
874850
);
875851
expect(result).toEqual(undefined);
876852
});
877853

878-
it("should return error when a search index filter references the search field", () => {
854+
it("should return an error when a search index filter references the search field", () => {
879855
const indexName = "testIndex";
880856
const selectedIndex: SearchIndex = {
881857
indexDescriptor: "testSearchIndex",
@@ -885,19 +861,66 @@ describe("filters", () => {
885861

886862
const result = validateSearchIndexFilter(
887863
indexName,
888-
{
889-
description: { enabled: true, value: "test" },
890-
},
864+
[
865+
{
866+
field: "description",
867+
enabled: true,
868+
value: "test",
869+
},
870+
],
891871
selectedIndex,
892872
"asc",
893873
);
894874
expect(result).toEqual({
895-
filter: -1,
875+
filter: 0,
896876
error:
897877
"Invalid index filter selection: found a filter for field `description` which is not part of the filter fields of the search index `testIndex`.",
898878
});
899879
});
900880

881+
it("should return error when there are two indexes for the same field", () => {
882+
const indexName = "testIndex";
883+
const selectedIndex: SearchIndex = {
884+
indexDescriptor: "testSearchIndex",
885+
searchField: "description",
886+
filterFields: ["userId"],
887+
};
888+
889+
const result = validateSearchIndexFilter(
890+
indexName,
891+
[
892+
{ field: "userId", enabled: true, value: BigInt(123) },
893+
{ field: "userId", enabled: true, value: BigInt(123) },
894+
],
895+
selectedIndex,
896+
"asc",
897+
);
898+
expect(result).toEqual({
899+
filter: 0,
900+
error: "Invalid filter: there are multiple filters for field `userId`.",
901+
});
902+
});
903+
904+
it("should not return an error when there are two indexes for the same field but only one is enabled", () => {
905+
const indexName = "testIndex";
906+
const selectedIndex: SearchIndex = {
907+
indexDescriptor: "testSearchIndex",
908+
searchField: "description",
909+
filterFields: ["userId", "organizationId"],
910+
};
911+
912+
const result = validateSearchIndexFilter(
913+
indexName,
914+
[
915+
{ field: "userId", enabled: true, value: BigInt(123) },
916+
{ field: "userId", enabled: false, value: BigInt(456) },
917+
],
918+
selectedIndex,
919+
"asc",
920+
);
921+
expect(result).toEqual(undefined);
922+
});
923+
901924
it("should return error when querying a search index in descending order", () => {
902925
const indexName = "testIndex";
903926
const selectedIndex: SearchIndex = {
@@ -908,7 +931,7 @@ describe("filters", () => {
908931

909932
const result = validateSearchIndexFilter(
910933
indexName,
911-
{},
934+
[],
912935
selectedIndex,
913936
"desc",
914937
);

npm-packages/system-udfs/convex/_system/frontend/lib/filters.ts

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,12 @@ type DatabaseIndexFilter = {
4242
type SearchIndexFilter = {
4343
name: string;
4444
search: string;
45-
filters: Record<
46-
string,
47-
{
48-
enabled: boolean;
49-
value: Value;
50-
}
51-
>;
45+
/** The clauses on the filter fields of the search index */
46+
clauses: {
47+
field: string;
48+
enabled: boolean;
49+
value: Value;
50+
}[];
5251
};
5352

5453
export type FilterCommon = {
@@ -377,13 +376,13 @@ export function applySearchIndexFilters<
377376
>(
378377
q: SearchFilterBuilder<Document, SearchIndexConfig>,
379378
search: string,
380-
filters: Record<string, { enabled: boolean; value: Value }>,
379+
filters: Array<{ field: string; enabled: boolean; value: Value }>,
381380
selectedIndex: SearchIndex,
382381
): SearchFilterFinalizer<Document, SearchIndexConfig> {
383382
let builder = q.search(selectedIndex.searchField, search);
384383

385384
// Apply filters
386-
for (const [field, { enabled, value }] of Object.entries(filters)) {
385+
for (const { field, enabled, value } of filters) {
387386
if (enabled) {
388387
builder = builder.eq(field, value as any);
389388
}
@@ -467,7 +466,7 @@ export function validateIndexFilter(
467466
*/
468467
export function validateSearchIndexFilter(
469468
indexName: string,
470-
filters: Record<string, { enabled: boolean; value: Value }>,
469+
filters: Array<{ field: string; enabled: boolean; value: Value }>,
471470
selectedIndex: Index | SearchIndex | undefined,
472471
order: "asc" | "desc",
473472
) {
@@ -493,11 +492,29 @@ export function validateSearchIndexFilter(
493492
};
494493
}
495494

496-
for (const [key, value] of Object.entries(filters)) {
497-
if (value.enabled && !selectedIndex.filterFields.includes(key)) {
495+
for (const [filterIndex, filter] of filters.entries()) {
496+
if (!filter.enabled) continue;
497+
498+
// Field not a filter field?
499+
if (!selectedIndex.filterFields.includes(filter.field)) {
498500
return {
499-
filter: -1,
500-
error: `Invalid index filter selection: found a filter for field \`${key}\` which is not part of the filter fields of the search index \`${indexName}\`.`,
501+
filter: filterIndex,
502+
error: `Invalid index filter selection: found a filter for field \`${filter.field}\` which is not part of the filter fields of the search index \`${indexName}\`.`,
503+
};
504+
}
505+
506+
// Duplicated filter for field?
507+
if (
508+
filters.some(
509+
(otherFilter, otherFilterIndex) =>
510+
filterIndex !== otherFilterIndex &&
511+
otherFilter.enabled &&
512+
filter.field === otherFilter.field,
513+
)
514+
) {
515+
return {
516+
filter: filterIndex,
517+
error: `Invalid filter: there are multiple filters for field \`${filter.field}\`.`,
501518
};
502519
}
503520
}

npm-packages/system-udfs/convex/_system/frontend/paginatedTableDocuments.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ export default queryGeneric({
111111
const validationError = isSearchIndex
112112
? validateSearchIndexFilter(
113113
indexFilter.name,
114-
indexFilter.filters,
114+
indexFilter.clauses,
115115
selectedIndex,
116116
order,
117117
)
@@ -134,7 +134,7 @@ export default queryGeneric({
134134
applySearchIndexFilters(
135135
q,
136136
indexFilter.search,
137-
indexFilter.filters,
137+
indexFilter.clauses,
138138
selectedIndex as SearchIndex,
139139
),
140140
)

0 commit comments

Comments
 (0)