From 467fbda06d02647d3f5eed641273e23e9777e2bf Mon Sep 17 00:00:00 2001 From: Hung Phan Viet Date: Sun, 20 Sep 2026 21:17:40 +0700 Subject: [PATCH] fix(inbox): load all connected pages in the broadcast page selector instead of only 50 --- .../src/features/inboxes/api/private.ts | 19 ++++++- .../provider/__tests__/inbox-store.test.ts | 33 ++++++------ .../features/inboxes/provider/inbox-store.ts | 19 ++++--- .../src/features/inboxes/queries/index.ts | 12 ++++- .../src/inbox/__tests__/service.test.ts | 42 +++++++++++++++ packages/business/src/inbox/schema.ts | 45 ++++++++++++---- packages/business/src/inbox/service.ts | 53 +++++++++++++++---- 7 files changed, 175 insertions(+), 48 deletions(-) diff --git a/apps/builder/src/features/inboxes/api/private.ts b/apps/builder/src/features/inboxes/api/private.ts index c33937c76b..1c4f224b6a 100644 --- a/apps/builder/src/features/inboxes/api/private.ts +++ b/apps/builder/src/features/inboxes/api/private.ts @@ -1,7 +1,12 @@ -import { listInboxesRequest, listInboxesResponse } from "@chatbotx.io/business" +import { + listAllConnectedInboxesRequest, + listAllConnectedInboxesResponse, + listInboxesRequest, + listInboxesResponse, +} from "@chatbotx.io/business" import { workspaceAuthorizedMidddleware } from "@/middlewares/auth" import { authorizedAPI } from "@/orpc" -import { listInboxes } from "../queries" +import { listAllConnectedInboxes, listInboxes } from "../queries" export const inboxesAuthenticatedAPI = { listInboxesAuthenticatedAPI: authorizedAPI @@ -15,4 +20,14 @@ export const inboxesAuthenticatedAPI = { .use(workspaceAuthorizedMidddleware, (input) => input.workspaceId) .output(listInboxesResponse) .handler(async ({ input }) => await listInboxes(input)), + + // RPC-only (no `.route()`): the builder inbox store needs the complete + // connected set, which the paginated `list` caps at 50. Kept off the + // documented OpenAPI surface so "return everything" is not offered as a + // public endpoint — the public token API stays paginated. + listAllInboxesAuthenticatedAPI: authorizedAPI + .input(listAllConnectedInboxesRequest) + .use(workspaceAuthorizedMidddleware, (input) => input.workspaceId) + .output(listAllConnectedInboxesResponse) + .handler(async ({ input }) => await listAllConnectedInboxes(input)), } diff --git a/apps/builder/src/features/inboxes/provider/__tests__/inbox-store.test.ts b/apps/builder/src/features/inboxes/provider/__tests__/inbox-store.test.ts index 32924654b0..9a85d2f4fc 100644 --- a/apps/builder/src/features/inboxes/provider/__tests__/inbox-store.test.ts +++ b/apps/builder/src/features/inboxes/provider/__tests__/inbox-store.test.ts @@ -2,13 +2,13 @@ import { ORPCError } from "@orpc/client" import { beforeEach, describe, expect, test, vi } from "vitest" const mocks = vi.hoisted(() => ({ - listInboxesAuthenticatedAPI: vi.fn(), + listAllInboxesAuthenticatedAPI: vi.fn(), })) vi.mock("@/lib/orpc/orpc", () => ({ client: { inboxesAPI: { - listInboxesAuthenticatedAPI: mocks.listInboxesAuthenticatedAPI, + listAllInboxesAuthenticatedAPI: mocks.listAllInboxesAuthenticatedAPI, }, }, })) @@ -16,12 +16,12 @@ vi.mock("@/lib/orpc/orpc", () => ({ const { createInboxStore } = await import("../inbox-store") beforeEach(() => { - mocks.listInboxesAuthenticatedAPI.mockReset() + mocks.listAllInboxesAuthenticatedAPI.mockReset() }) describe("getAllInboxes", () => { - test("fetches inboxes for the workspace with includes and maxPerPage", async () => { - mocks.listInboxesAuthenticatedAPI.mockResolvedValueOnce({ + test("fetches every inbox for the workspace with integrations, unpaginated", async () => { + mocks.listAllInboxesAuthenticatedAPI.mockResolvedValueOnce({ data: [{ id: "inbox-1", name: "Support" }], }) @@ -29,10 +29,9 @@ describe("getAllInboxes", () => { await store.getState().getAllInboxes() - expect(mocks.listInboxesAuthenticatedAPI).toHaveBeenCalledWith({ + expect(mocks.listAllInboxesAuthenticatedAPI).toHaveBeenCalledWith({ workspaceId: "workspace-1", includes: ["integration"], - perPage: 999_999_999, }) expect(store.getState().inboxes).toEqual([ { id: "inbox-1", name: "Support" }, @@ -44,7 +43,7 @@ describe("getAllInboxes", () => { await store.getState().getAllInboxes() - expect(mocks.listInboxesAuthenticatedAPI).not.toHaveBeenCalled() + expect(mocks.listAllInboxesAuthenticatedAPI).not.toHaveBeenCalled() }) test("is a no-op while a fetch is already in flight", async () => { @@ -52,21 +51,21 @@ describe("getAllInboxes", () => { const pending = new Promise<{ data: unknown[] }>((resolve) => { resolveFetch = resolve }) - mocks.listInboxesAuthenticatedAPI.mockReturnValueOnce(pending) + mocks.listAllInboxesAuthenticatedAPI.mockReturnValueOnce(pending) const store = createInboxStore({ workspaceId: "workspace-1" }) const first = store.getState().getAllInboxes() await store.getState().getAllInboxes() - expect(mocks.listInboxesAuthenticatedAPI).toHaveBeenCalledTimes(1) + expect(mocks.listAllInboxesAuthenticatedAPI).toHaveBeenCalledTimes(1) resolveFetch({ data: [] }) await first }) test("sets the ORPCError message on a rejected request", async () => { - mocks.listInboxesAuthenticatedAPI.mockRejectedValueOnce( + mocks.listAllInboxesAuthenticatedAPI.mockRejectedValueOnce( new ORPCError("INTERNAL_SERVER_ERROR", { message: "HTTP 500" }), ) @@ -79,7 +78,7 @@ describe("getAllInboxes", () => { }) test("falls back to a generic message for a non-ORPCError rejection", async () => { - mocks.listInboxesAuthenticatedAPI.mockRejectedValueOnce( + mocks.listAllInboxesAuthenticatedAPI.mockRejectedValueOnce( new Error("network down"), ) @@ -93,7 +92,7 @@ describe("getAllInboxes", () => { describe("initialize", () => { test("calls getAllInboxes once and marks the store initialized", async () => { - mocks.listInboxesAuthenticatedAPI.mockResolvedValueOnce({ + mocks.listAllInboxesAuthenticatedAPI.mockResolvedValueOnce({ data: [{ id: "inbox-1", name: "Support" }], }) @@ -101,7 +100,7 @@ describe("initialize", () => { await store.getState().initialize() - expect(mocks.listInboxesAuthenticatedAPI).toHaveBeenCalledTimes(1) + expect(mocks.listAllInboxesAuthenticatedAPI).toHaveBeenCalledTimes(1) expect(store.getState().inboxes).toEqual([ { id: "inbox-1", name: "Support" }, ]) @@ -109,14 +108,14 @@ describe("initialize", () => { }) test("does not fetch again once already initialized", async () => { - mocks.listInboxesAuthenticatedAPI.mockResolvedValue({ data: [] }) + mocks.listAllInboxesAuthenticatedAPI.mockResolvedValue({ data: [] }) const store = createInboxStore({ workspaceId: "workspace-1" }) await store.getState().initialize() await store.getState().initialize() - expect(mocks.listInboxesAuthenticatedAPI).toHaveBeenCalledTimes(1) + expect(mocks.listAllInboxesAuthenticatedAPI).toHaveBeenCalledTimes(1) }) test("still marks the store initialized when getAllInboxes fails", async () => { @@ -125,7 +124,7 @@ describe("initialize", () => { // failure directly — but its `finally` unconditionally marks // `initialized: true` regardless, and getAllInboxes's error is still // visible on the shared `error` field. - mocks.listInboxesAuthenticatedAPI.mockRejectedValueOnce( + mocks.listAllInboxesAuthenticatedAPI.mockRejectedValueOnce( new ORPCError("INTERNAL_SERVER_ERROR", { message: "HTTP 500" }), ) diff --git a/apps/builder/src/features/inboxes/provider/inbox-store.ts b/apps/builder/src/features/inboxes/provider/inbox-store.ts index 2874eaaccd..3a1480a5e4 100644 --- a/apps/builder/src/features/inboxes/provider/inbox-store.ts +++ b/apps/builder/src/features/inboxes/provider/inbox-store.ts @@ -1,8 +1,7 @@ -import type { ListInboxesResponse } from "@chatbotx.io/business" +import type { ListAllConnectedInboxesResponse } from "@chatbotx.io/business" import { createStore } from "zustand/vanilla" import { getClientErrorMessage } from "@/lib/orpc/client-error" import { client } from "@/lib/orpc/orpc" -import { maxPerPage } from "@/lib/shared-request" export type InboxState = { error: string | null @@ -11,7 +10,7 @@ export type InboxState = { workspaceId: string loadingInboxes: boolean - inboxes: ListInboxesResponse["data"] + inboxes: ListAllConnectedInboxesResponse["data"] } export type InboxActions = { @@ -59,11 +58,15 @@ export const createInboxStore = (props: Partial) => } set({ loadingInboxes: true, error: null }) try { - const { data } = await client.inboxesAPI.listInboxesAuthenticatedAPI({ - workspaceId, - includes: ["integration"], - perPage: maxPerPage, - }) + // The unpaginated endpoint: the paginated list caps at 50 rows, which + // would hide inboxes from every store consumer (broadcast page + // picker, inbox/contacts filters) in a workspace with more than 50. + const { data } = await client.inboxesAPI.listAllInboxesAuthenticatedAPI( + { + workspaceId, + includes: ["integration"], + }, + ) set({ inboxes: data }) } catch (error: unknown) { diff --git a/apps/builder/src/features/inboxes/queries/index.ts b/apps/builder/src/features/inboxes/queries/index.ts index e708a8398f..bd5e225d9e 100644 --- a/apps/builder/src/features/inboxes/queries/index.ts +++ b/apps/builder/src/features/inboxes/queries/index.ts @@ -1,4 +1,8 @@ -import type { ListInboxesResponse } from "@chatbotx.io/business" +import type { + ListAllConnectedInboxesRequest, + ListAllConnectedInboxesResponse, + ListInboxesResponse, +} from "@chatbotx.io/business" import { inboxService, type ListInboxesRequest } from "@chatbotx.io/business" export async function listInboxes( @@ -6,3 +10,9 @@ export async function listInboxes( ): Promise { return await inboxService.list(input) } + +export async function listAllConnectedInboxes( + input: ListAllConnectedInboxesRequest, +): Promise { + return await inboxService.listAllConnectedByWorkspace(input) +} diff --git a/packages/business/src/inbox/__tests__/service.test.ts b/packages/business/src/inbox/__tests__/service.test.ts index 376efdd7dd..df8c2924de 100644 --- a/packages/business/src/inbox/__tests__/service.test.ts +++ b/packages/business/src/inbox/__tests__/service.test.ts @@ -322,3 +322,45 @@ describe("InboxService.list", () => { expect(mocks.count).toHaveBeenCalledTimes(1) }) }) + +describe("InboxService.listAllConnectedByWorkspace", () => { + test("returns every connected inbox with no page limit and no count query", async () => { + mocks.inboxFindMany.mockResolvedValue([ + { id: "inbox-1" }, + { id: "inbox-2" }, + ]) + + const result = await inboxService.listAllConnectedByWorkspace({ + workspaceId: "workspace-1", + }) + + expect(result).toEqual({ data: [{ id: "inbox-1" }, { id: "inbox-2" }] }) + expect(mocks.inboxFindMany).toHaveBeenCalledWith({ + where: { + workspaceId: "workspace-1", + status: "connected", + }, + with: undefined, + }) + // No pagination: the 50-row `maxLimit` that caps `list` must not apply. + const call = mocks.inboxFindMany.mock.calls[0][0] + expect(call).not.toHaveProperty("limit") + expect(call).not.toHaveProperty("offset") + expect(mocks.count).not.toHaveBeenCalled() + }) + + test("eager-loads integrations when includes asks for them", async () => { + mocks.inboxFindMany.mockResolvedValue([]) + + await inboxService.listAllConnectedByWorkspace({ + workspaceId: "workspace-1", + includes: ["integration"], + }) + + expect(mocks.inboxFindMany).toHaveBeenCalledWith( + expect.objectContaining({ + with: expect.objectContaining({ integrationMessenger: true }), + }), + ) + }) +}) diff --git a/packages/business/src/inbox/schema.ts b/packages/business/src/inbox/schema.ts index 9965252d67..746a121bcd 100644 --- a/packages/business/src/inbox/schema.ts +++ b/packages/business/src/inbox/schema.ts @@ -36,18 +36,41 @@ export const inboxResource = createSelectSchema(inboxModel, { id: zodBigintAsString(), workspaceId: zodBigintAsString(), }) +export const inboxWithIntegrationsResource = inboxResource.extend({ + integrationWhatsapp: integrationWhatsappResource.nullish(), + integrationWebchat: integrationWebchatResource.nullish(), + integrationMessenger: integrationMessengerResource.nullish(), + integrationZalo: integrationZaloResource.nullish(), + integrationTelegram: integrationTelegramResource.nullish(), + integrationInstagram: integrationInstagramResource.nullish(), + integrationSmtp: integrationSmtpResource.nullish(), +}) + export const listInboxesResponse = z.object({ - data: z.array( - inboxResource.extend({ - integrationWhatsapp: integrationWhatsappResource.nullish(), - integrationWebchat: integrationWebchatResource.nullish(), - integrationMessenger: integrationMessengerResource.nullish(), - integrationZalo: integrationZaloResource.nullish(), - integrationTelegram: integrationTelegramResource.nullish(), - integrationInstagram: integrationInstagramResource.nullish(), - integrationSmtp: integrationSmtpResource.nullish(), - }), - ), + data: z.array(inboxWithIntegrationsResource), pageCount: z.number(), }) export type ListInboxesResponse = z.infer + +/** + * Unpaginated companion to `listInboxesRequest`. The paginated `list` is + * capped at `maxLimit` (50) rows, which silently truncates a workspace with + * more connected inboxes than that — so any client that needs the complete + * set (e.g. the builder's inbox store, which the broadcast page picker reads + * from) uses this instead. No `page`/`perPage`: the whole connected set is + * always returned. + */ +export const listAllConnectedInboxesRequest = listInboxesRequest.pick({ + workspaceId: true, + includes: true, +}) +export type ListAllConnectedInboxesRequest = z.infer< + typeof listAllConnectedInboxesRequest +> + +export const listAllConnectedInboxesResponse = z.object({ + data: z.array(inboxWithIntegrationsResource), +}) +export type ListAllConnectedInboxesResponse = z.infer< + typeof listAllConnectedInboxesResponse +> diff --git a/packages/business/src/inbox/service.ts b/packages/business/src/inbox/service.ts index 714e57a7cd..0522428852 100644 --- a/packages/business/src/inbox/service.ts +++ b/packages/business/src/inbox/service.ts @@ -31,7 +31,12 @@ import { channelLimitReachedException } from "../errors" import { logger } from "../logger" import { quotaEnforcementService } from "../quota-enforcement/service" import { workspaceUsageService } from "../workspace-usage/service" -import type { ListInboxesRequest, ListInboxesResponse } from "./schema" +import type { + ListAllConnectedInboxesRequest, + ListAllConnectedInboxesResponse, + ListInboxesRequest, + ListInboxesResponse, +} from "./schema" type InboxWhere = Partial<{ id: string; workspaceId: string }> @@ -66,22 +71,33 @@ class InboxService extends BaseService { integrationTiktok: true, } - async list(input: ListInboxesRequest): Promise { - // One `where`, shared by the page query and the count, so the two can - // never drift (they previously repeated the same literal side by side). - const where = { - workspaceId: input.workspaceId, + /** + * Connected inboxes of a workspace. Shared by `list` and + * `listAllConnectedByWorkspace` so the page query, its row count, and the + * unpaginated variant can never filter on different criteria. + */ + private static connectedWhere(workspaceId: string) { + return { + workspaceId, status: inboxStatuses.enum.connected, } + } + + private static integrationsWith(includes: ListInboxesRequest["includes"]) { + return includes?.includes("integration") + ? InboxService.withIntegrations + : undefined + } + + async list(input: ListInboxesRequest): Promise { + const where = InboxService.connectedWhere(input.workspaceId) const pagination = getPaginationWithDefaults(input) const [data, totalRows] = await Promise.all([ db.query.inboxModel.findMany({ ...pagination, where, - with: input.includes?.includes("integration") - ? InboxService.withIntegrations - : undefined, + with: InboxService.integrationsWith(input.includes), }), db.$count(inboxModel, relationsFilterToSQL(inboxModel, where)), ]) @@ -92,6 +108,25 @@ class InboxService extends BaseService { return { data, pageCount } } + /** + * Every connected inbox of a workspace, unpaginated. `list` caps at + * `maxLimit` (50) rows; a workspace with more connected inboxes than that + * would silently lose the rest, which is why any caller that must see the + * complete set (the builder inbox store, and through it the broadcast page + * picker) uses this. Eager-loads integrations only when asked, matching + * `list`'s `includes` contract. + */ + async listAllConnectedByWorkspace( + input: ListAllConnectedInboxesRequest, + ): Promise { + const data = await db.query.inboxModel.findMany({ + where: InboxService.connectedWhere(input.workspaceId), + with: InboxService.integrationsWith(input.includes), + }) + + return { data } + } + async listWithIntegrationsByWorkspace( workspaceId: string, tx: DatabaseClient = db,