From 5912277981fc27cd96cb9239565a1148d7e6fc8f Mon Sep 17 00:00:00 2001 From: Mohamed Bassem Date: Mon, 28 Apr 2025 07:29:23 +0000 Subject: Revert "fix: Fix smart lists not working in list search qualifiers". #1321 This reverts commit 6178736d64180f9bc8954099c90d54aa2f9f35f5. --- packages/trpc/lib/__tests__/search.test.ts | 69 ------------------ packages/trpc/lib/search.ts | 110 +++++++++++++++++------------ packages/trpc/models/lists.ts | 23 ++---- 3 files changed, 69 insertions(+), 133 deletions(-) (limited to 'packages/trpc') diff --git a/packages/trpc/lib/__tests__/search.test.ts b/packages/trpc/lib/__tests__/search.test.ts index 72b53368..9d9b39d7 100644 --- a/packages/trpc/lib/__tests__/search.test.ts +++ b/packages/trpc/lib/__tests__/search.test.ts @@ -34,12 +34,6 @@ beforeEach(async () => { email: "test@example.com", role: "user", }, - { - id: "another-user", - name: "Another User", - email: "another@example.com", - role: "user", - }, ]); // Setup test data @@ -92,14 +86,6 @@ beforeEach(async () => { favourited: false, createdAt: new Date("2024-01-06"), }, - { - id: "b7", - type: BookmarkTypes.ASSET, - userId: "another-user", - archived: true, - favourited: false, - createdAt: new Date("2024-01-06"), - }, ]); await db.insert(bookmarkLinks).values([ @@ -157,21 +143,6 @@ beforeEach(async () => { type: "manual", }, { id: "l4", userId: testUserId, name: "work", icon: "💼", type: "manual" }, - { - id: "l5", - userId: testUserId, - name: "smartlist", - icon: "🧠", - type: "smart", - query: "#tag1 or #tag2", - }, - { - id: "l6", - userId: testUserId, - name: "emptylist", - icon: "∅", - type: "manual", - }, ]); await db.insert(bookmarksInLists).values([ @@ -253,26 +224,6 @@ describe("getBookmarkIdsFromMatcher", () => { expect(result).toEqual(["b1", "b6"]); }); - it("should handle listName matcher with smartList", async () => { - const matcher: Matcher = { - type: "listName", - listName: "smartlist", - inverse: false, - }; - const result = await getBookmarkIdsFromMatcher(mockCtx, matcher); - expect(result).toEqual(["b1", "b2"]); - }); - - it("should handle listName matcher with empty list", async () => { - const matcher: Matcher = { - type: "listName", - listName: "emptylist", - inverse: false, - }; - const result = await getBookmarkIdsFromMatcher(mockCtx, matcher); - expect(result).toEqual([]); - }); - it("should handle listName matcher with inverse=true", async () => { const matcher: Matcher = { type: "listName", @@ -283,26 +234,6 @@ describe("getBookmarkIdsFromMatcher", () => { expect(result.sort()).toEqual(["b2", "b3", "b4", "b5"]); }); - it("should handle listName matcher with smartList with inverse=true", async () => { - const matcher: Matcher = { - type: "listName", - listName: "smartlist", - inverse: true, - }; - const result = await getBookmarkIdsFromMatcher(mockCtx, matcher); - expect(result).toEqual(["b3", "b4", "b5", "b6"]); - }); - - it("should handle listName matcher with empty list with inverse=true", async () => { - const matcher: Matcher = { - type: "listName", - listName: "emptylist", - inverse: true, - }; - const result = await getBookmarkIdsFromMatcher(mockCtx, matcher); - expect(result).toEqual(["b1", "b2", "b3", "b4", "b5", "b6"]); - }); - it("should handle archived matcher", async () => { const matcher: Matcher = { type: "archived", archived: true }; const result = await getBookmarkIdsFromMatcher(mockCtx, matcher); diff --git a/packages/trpc/lib/search.ts b/packages/trpc/lib/search.ts index 7bb78a01..d4130798 100644 --- a/packages/trpc/lib/search.ts +++ b/packages/trpc/lib/search.ts @@ -4,21 +4,21 @@ import { exists, gt, gte, - inArray, isNotNull, like, lt, lte, ne, notExists, - notInArray, notLike, } from "drizzle-orm"; import { bookmarkAssets, bookmarkLinks, + bookmarkLists, bookmarks, + bookmarksInLists, bookmarkTags, rssFeedImportsTable, rssFeedsTable, @@ -28,7 +28,6 @@ import { Matcher } from "@karakeep/shared/types/search"; import { toAbsoluteDate } from "@karakeep/shared/utils/relativeDateUtils"; import { AuthedContext } from ".."; -import { List } from "../models/lists"; interface BookmarkQueryReturnType { id: string; @@ -88,20 +87,21 @@ function union(vals: BookmarkQueryReturnType[][]): BookmarkQueryReturnType[] { } async function getIds( - ctx: AuthedContext, + db: AuthedContext["db"], + userId: string, matcher: Matcher, ): Promise { switch (matcher.type) { case "tagName": { const comp = matcher.inverse ? notExists : exists; - return ctx.db + return db .selectDistinct({ id: bookmarks.id }) .from(bookmarks) .where( and( - eq(bookmarks.userId, ctx.user.id), + eq(bookmarks.userId, userId), comp( - ctx.db + db .select() .from(tagsOnBookmarks) .innerJoin( @@ -111,7 +111,7 @@ async function getIds( .where( and( eq(tagsOnBookmarks.bookmarkId, bookmarks.id), - eq(bookmarkTags.userId, ctx.user.id), + eq(bookmarkTags.userId, userId), eq(bookmarkTags.name, matcher.tagName), ), ), @@ -121,14 +121,14 @@ async function getIds( } case "tagged": { const comp = matcher.tagged ? exists : notExists; - return ctx.db + return db .select({ id: bookmarks.id }) .from(bookmarks) .where( and( - eq(bookmarks.userId, ctx.user.id), + eq(bookmarks.userId, userId), comp( - ctx.db + db .select() .from(tagsOnBookmarks) .where(and(eq(tagsOnBookmarks.bookmarkId, bookmarks.id))), @@ -137,43 +137,59 @@ async function getIds( ); } case "listName": { - const lists = await List.fromName(ctx, matcher.listName); - const ids = await Promise.all(lists.map((l) => l.getBookmarkIds())); - const comp = matcher.inverse ? notInArray : inArray; - return ctx.db - .select({ id: bookmarks.id }) + const comp = matcher.inverse ? notExists : exists; + return db + .selectDistinct({ id: bookmarks.id }) .from(bookmarks) .where( and( - eq(bookmarks.userId, ctx.user.id), - comp(bookmarks.id, ids.flat()), + eq(bookmarks.userId, userId), + comp( + db + .select() + .from(bookmarksInLists) + .innerJoin( + bookmarkLists, + eq(bookmarksInLists.listId, bookmarkLists.id), + ) + .where( + and( + eq(bookmarksInLists.bookmarkId, bookmarks.id), + eq(bookmarkLists.userId, userId), + eq(bookmarkLists.name, matcher.listName), + ), + ), + ), ), ); } case "inlist": { - const lists = await List.getAll(ctx); - const ids = await Promise.all(lists.map((l) => l.getBookmarkIds())); - const comp = matcher.inList ? inArray : notInArray; - return ctx.db + const comp = matcher.inList ? exists : notExists; + return db .select({ id: bookmarks.id }) .from(bookmarks) .where( and( - eq(bookmarks.userId, ctx.user.id), - comp(bookmarks.id, ids.flat()), + eq(bookmarks.userId, userId), + comp( + db + .select() + .from(bookmarksInLists) + .where(and(eq(bookmarksInLists.bookmarkId, bookmarks.id))), + ), ), ); } case "rssFeedName": { const comp = matcher.inverse ? notExists : exists; - return ctx.db + return db .selectDistinct({ id: bookmarks.id }) .from(bookmarks) .where( and( - eq(bookmarks.userId, ctx.user.id), + eq(bookmarks.userId, userId), comp( - ctx.db + db .select() .from(rssFeedImportsTable) .innerJoin( @@ -183,7 +199,7 @@ async function getIds( .where( and( eq(rssFeedImportsTable.bookmarkId, bookmarks.id), - eq(rssFeedsTable.userId, ctx.user.id), + eq(rssFeedsTable.userId, userId), eq(rssFeedsTable.name, matcher.feedName), ), ), @@ -192,36 +208,36 @@ async function getIds( ); } case "archived": { - return ctx.db + return db .select({ id: bookmarks.id }) .from(bookmarks) .where( and( - eq(bookmarks.userId, ctx.user.id), + eq(bookmarks.userId, userId), eq(bookmarks.archived, matcher.archived), ), ); } case "url": { const comp = matcher.inverse ? notLike : like; - return ctx.db + return db .select({ id: bookmarkLinks.id }) .from(bookmarkLinks) .leftJoin(bookmarks, eq(bookmarks.id, bookmarkLinks.id)) .where( and( - eq(bookmarks.userId, ctx.user.id), + eq(bookmarks.userId, userId), comp(bookmarkLinks.url, `%${matcher.url}%`), ), ) .union( - ctx.db + db .select({ id: bookmarkAssets.id }) .from(bookmarkAssets) .leftJoin(bookmarks, eq(bookmarks.id, bookmarkAssets.id)) .where( and( - eq(bookmarks.userId, ctx.user.id), + eq(bookmarks.userId, userId), // When a user is asking for a link, the inverse matcher should match only assets with URLs. isNotNull(bookmarkAssets.sourceUrl), comp(bookmarkAssets.sourceUrl, `%${matcher.url}%`), @@ -230,73 +246,73 @@ async function getIds( ); } case "favourited": { - return ctx.db + return db .select({ id: bookmarks.id }) .from(bookmarks) .where( and( - eq(bookmarks.userId, ctx.user.id), + eq(bookmarks.userId, userId), eq(bookmarks.favourited, matcher.favourited), ), ); } case "dateAfter": { const comp = matcher.inverse ? lt : gte; - return ctx.db + return db .select({ id: bookmarks.id }) .from(bookmarks) .where( and( - eq(bookmarks.userId, ctx.user.id), + eq(bookmarks.userId, userId), comp(bookmarks.createdAt, matcher.dateAfter), ), ); } case "dateBefore": { const comp = matcher.inverse ? gt : lte; - return ctx.db + return db .select({ id: bookmarks.id }) .from(bookmarks) .where( and( - eq(bookmarks.userId, ctx.user.id), + eq(bookmarks.userId, userId), comp(bookmarks.createdAt, matcher.dateBefore), ), ); } case "age": { const comp = matcher.relativeDate.direction === "newer" ? gte : lt; - return ctx.db + return db .select({ id: bookmarks.id }) .from(bookmarks) .where( and( - eq(bookmarks.userId, ctx.user.id), + eq(bookmarks.userId, userId), comp(bookmarks.createdAt, toAbsoluteDate(matcher.relativeDate)), ), ); } case "type": { const comp = matcher.inverse ? ne : eq; - return ctx.db + return db .select({ id: bookmarks.id }) .from(bookmarks) .where( and( - eq(bookmarks.userId, ctx.user.id), + eq(bookmarks.userId, userId), comp(bookmarks.type, matcher.typeName), ), ); } case "and": { const vals = await Promise.all( - matcher.matchers.map((m) => getIds(ctx, m)), + matcher.matchers.map((m) => getIds(db, userId, m)), ); return intersect(vals); } case "or": { const vals = await Promise.all( - matcher.matchers.map((m) => getIds(ctx, m)), + matcher.matchers.map((m) => getIds(db, userId, m)), ); return union(vals); } @@ -311,6 +327,6 @@ export async function getBookmarkIdsFromMatcher( ctx: AuthedContext, matcher: Matcher, ): Promise { - const results = await getIds(ctx, matcher); + const results = await getIds(ctx.db, ctx.user.id, matcher); return results.map((r) => r.id); } diff --git a/packages/trpc/models/lists.ts b/packages/trpc/models/lists.ts index d278f8d9..4da127d2 100644 --- a/packages/trpc/models/lists.ts +++ b/packages/trpc/models/lists.ts @@ -26,7 +26,7 @@ export abstract class List implements PrivacyAware { private static fromData( ctx: AuthedContext, data: ZBookmarkList & { userId: string }, - ): ManualList | SmartList { + ) { if (data.type === "smart") { return new SmartList(ctx, data); } else { @@ -34,21 +34,6 @@ export abstract class List implements PrivacyAware { } } - static async fromName( - ctx: AuthedContext, - name: string, - ): Promise<(ManualList | SmartList)[]> { - // Names are not unique, so we need to find all lists with the same name - const lists = await ctx.db.query.bookmarkLists.findMany({ - where: and( - eq(bookmarkLists.name, name), - eq(bookmarkLists.userId, ctx.user.id), - ), - }); - - return lists.map((l) => this.fromData(ctx, l)); - } - static async fromId( ctx: AuthedContext, id: string, @@ -66,7 +51,11 @@ export abstract class List implements PrivacyAware { message: "List not found", }); } - return this.fromData(ctx, list); + if (list.type === "smart") { + return new SmartList(ctx, list); + } else { + return new ManualList(ctx, list); + } } static async create( -- cgit v1.2.3-70-g09d2