diff --git a/apps/web/modules/response-pipeline/lib/process-response-pipeline-job.test.ts b/apps/web/modules/response-pipeline/lib/process-response-pipeline-job.test.ts index 758ca5333657..246a6a322fa6 100644 --- a/apps/web/modules/response-pipeline/lib/process-response-pipeline-job.test.ts +++ b/apps/web/modules/response-pipeline/lib/process-response-pipeline-job.test.ts @@ -9,6 +9,7 @@ const { mockCreatePinnedDispatcher, mockDispatcherDestroy, mockGetIntegrations, + mockGetFinishedResponseCountBySurveyId, mockGetResponseCountBySurveyId, mockHandleIntegrations, mockLoggerError, @@ -34,6 +35,7 @@ const { mockCreatePinnedDispatcher: vi.fn(() => ({ destroy: dispatcherDestroy })), mockDispatcherDestroy: dispatcherDestroy, mockGetIntegrations: vi.fn(), + mockGetFinishedResponseCountBySurveyId: vi.fn(), mockGetResponseCountBySurveyId: vi.fn(), mockHandleIntegrations: vi.fn(), mockLoggerError: vi.fn(), @@ -105,6 +107,10 @@ vi.mock("@/lib/response/service", () => ({ getResponseCountBySurveyId: mockGetResponseCountBySurveyId, })); +vi.mock("@/modules/survey/lib/response", () => ({ + getFinishedResponseCountBySurveyId: mockGetFinishedResponseCountBySurveyId, +})); + vi.mock("./posthog", () => ({ captureSurveyResponsePostHogEvent: mockCaptureSurveyResponsePostHogEvent, })); @@ -210,7 +216,8 @@ describe("processResponsePipelineJob", () => { mockGetIntegrations.mockResolvedValue([]); mockPrismaWebhookFindMany.mockResolvedValue([]); mockPrismaUserFindMany.mockResolvedValue([]); - mockGetResponseCountBySurveyId.mockResolvedValue(1); + mockGetResponseCountBySurveyId.mockResolvedValue(7); + mockGetFinishedResponseCountBySurveyId.mockResolvedValue(1); mockHandleIntegrations.mockResolvedValue(undefined); mockValidateAndResolveWebhookUrl.mockResolvedValue({ ip: "93.184.216.34", family: 4 }); mockDispatcherDestroy.mockResolvedValue(undefined); @@ -393,7 +400,9 @@ describe("processResponsePipelineJob", () => { "workspace_123", expect.objectContaining({ id: "survey_123" }), baseData.response, - 1 + // The notification email deliberately reports the *total* response count, not the + // completed-only count the response limit uses. + 7 ); expect(mockPrismaSurveyUpdate).toHaveBeenCalledWith({ data: { @@ -416,6 +425,142 @@ describe("processResponsePipelineJob", () => { expect(mockSendTelemetryEvents).not.toHaveBeenCalled(); }); + test("only counts finished responses towards the auto-complete response limit", async () => { + // Total responses (starts) far exceed the limit, but finished responses do not. + mockGetResponseCountBySurveyId.mockResolvedValue(500); + mockGetFinishedResponseCountBySurveyId.mockResolvedValue(3); + mockPrismaSurveyFindUnique.mockResolvedValue({ + ...survey, + autoComplete: 5, + }); + + await expect( + processResponsePipelineJob( + { + ...baseData, + event: "responseFinished", + }, + baseContext + ) + ).resolves.toBeUndefined(); + + // The auto-complete decision must be based on finished responses only. + expect(mockGetFinishedResponseCountBySurveyId).toHaveBeenCalledWith("survey_123"); + // 3 finished responses < limit of 5 → the survey must stay open. + expect(mockPrismaSurveyUpdate).not.toHaveBeenCalled(); + }); + + test("auto-completes the survey once finished responses reach the limit", async () => { + mockGetFinishedResponseCountBySurveyId.mockResolvedValue(5); + mockPrismaSurveyFindUnique.mockResolvedValue({ + ...survey, + autoComplete: 5, + }); + + await expect( + processResponsePipelineJob( + { + ...baseData, + event: "responseFinished", + }, + baseContext + ) + ).resolves.toBeUndefined(); + + expect(mockPrismaSurveyUpdate).toHaveBeenCalledWith({ + data: { + status: "completed", + }, + where: { + id: "survey_123", + }, + }); + }); + + test("does not count finished responses when no response limit is set", async () => { + // The default survey fixture has autoComplete: null. + await expect( + processResponsePipelineJob( + { + ...baseData, + event: "responseFinished", + }, + baseContext + ) + ).resolves.toBeUndefined(); + + expect(mockGetFinishedResponseCountBySurveyId).not.toHaveBeenCalled(); + expect(mockPrismaSurveyUpdate).not.toHaveBeenCalled(); + }); + + test("does not re-close a survey that is already completed", async () => { + mockGetFinishedResponseCountBySurveyId.mockResolvedValue(50); + mockPrismaSurveyFindUnique.mockResolvedValue({ + ...survey, + autoComplete: 5, + status: "completed", + }); + + await expect( + processResponsePipelineJob( + { + ...baseData, + event: "responseFinished", + }, + baseContext + ) + ).resolves.toBeUndefined(); + + expect(mockGetFinishedResponseCountBySurveyId).not.toHaveBeenCalled(); + expect(mockPrismaSurveyUpdate).not.toHaveBeenCalled(); + expect(mockQueueAuditEventWithoutRequest).not.toHaveBeenCalled(); + }); + + test("does not count total responses when nobody subscribes to notifications", async () => { + // No user has response notifications enabled, so nothing consumes the total count. + await expect( + processResponsePipelineJob( + { + ...baseData, + event: "responseFinished", + }, + baseContext + ) + ).resolves.toBeUndefined(); + + expect(mockPrismaUserFindMany).toHaveBeenCalled(); + expect(mockGetResponseCountBySurveyId).not.toHaveBeenCalled(); + expect(mockSendResponseFinishedEmail).not.toHaveBeenCalled(); + }); + + test("skips auto-complete when the finished response count cannot be loaded", async () => { + const countError = new Error("count offline"); + mockPrismaSurveyFindUnique.mockResolvedValue({ + ...survey, + autoComplete: 1, + }); + mockGetFinishedResponseCountBySurveyId.mockRejectedValue(countError); + + await expect( + processResponsePipelineJob( + { + ...baseData, + event: "responseFinished", + }, + baseContext + ) + ).resolves.toBeUndefined(); + + expect(mockLoggerError).toHaveBeenCalledWith( + expect.objectContaining({ + autoCompleteThreshold: 1, + err: countError, + }), + "Response pipeline survey auto-complete skipped because the finished response count could not be loaded" + ); + expect(mockPrismaSurveyUpdate).not.toHaveBeenCalled(); + }); + test("logs responseFinished side-effect failures without failing the job", async () => { mockPrismaUserFindMany.mockResolvedValue([ { @@ -633,6 +778,13 @@ describe("processResponsePipelineJob", () => { url: "https://example.com/webhook", }, ]); + // The total count is only looked up when a notification recipient consumes it. + mockPrismaUserFindMany.mockResolvedValue([ + { + email: "owner@example.com", + locale: "en", + }, + ]); mockGetResponseCountBySurveyId.mockRejectedValue(responseCountError); await expect( @@ -654,6 +806,7 @@ describe("processResponsePipelineJob", () => { }), "Response pipeline response count lookup failed" ); + expect(mockSendResponseFinishedEmail).not.toHaveBeenCalled(); }); test("logs telemetry failures without failing the responseCreated job", async () => { diff --git a/apps/web/modules/response-pipeline/lib/process-response-pipeline-job.ts b/apps/web/modules/response-pipeline/lib/process-response-pipeline-job.ts index 1aaff496a1bc..889f675efc45 100644 --- a/apps/web/modules/response-pipeline/lib/process-response-pipeline-job.ts +++ b/apps/web/modules/response-pipeline/lib/process-response-pipeline-job.ts @@ -20,6 +20,7 @@ import { captureSurveyResponsePostHogEvent } from "@/modules/response-pipeline/l import { resolveStorageUrlsInObject } from "@/modules/storage/utils"; import { sendFollowUpsForResponse } from "@/modules/survey/follow-ups/lib/follow-ups"; import { FollowUpSendError } from "@/modules/survey/follow-ups/types/follow-up"; +import { getFinishedResponseCountBySurveyId } from "@/modules/survey/lib/response"; import { handleIntegrations } from "./handle-integrations"; import { sendTelemetryEvents } from "./telemetry"; @@ -322,47 +323,54 @@ const deliverWebhooks = async ({ ); }; -const loadResponseFinishedContext = async ({ - data, +const loadIntegrationsSafely = async ({ logContext, workspaceId, }: { - data: TResponsePipelineJobData; logContext: ReturnType; workspaceId: string; -}): Promise<{ - integrations: Awaited>; - responseCount: number | null; -}> => { - const [integrationsResult, responseCountResult] = await Promise.allSettled([ - getIntegrations(workspaceId), - getResponseCountBySurveyId(data.surveyId), - ]); - - if (integrationsResult.status === "rejected") { +}): Promise>> => { + try { + return await getIntegrations(workspaceId); + } catch (error) { logger.error( { ...logContext, - err: integrationsResult.reason, + err: error, }, "Response pipeline integration lookup failed" ); + + return []; } +}; - if (responseCountResult.status === "rejected") { +/** + * Response counts are optional side inputs: a failed lookup must never fail the job, so it + * resolves to `null` and the consumer skips its own work instead. + */ +const loadResponseCountSafely = async ({ + count, + failureMessage, + logContext, +}: { + count: () => Promise; + failureMessage: string; + logContext: Record; +}): Promise => { + try { + return await count(); + } catch (error) { logger.error( { ...logContext, - err: responseCountResult.reason, + err: error, }, - "Response pipeline response count lookup failed" + failureMessage ); - } - return { - integrations: integrationsResult.status === "fulfilled" ? integrationsResult.value : [], - responseCount: responseCountResult.status === "fulfilled" ? responseCountResult.value : null, - }; + return null; + } }; const getUsersWithNotifications = async ({ @@ -535,32 +543,37 @@ const sendNotificationEmailsSafely = async ({ ); }; +/** + * The completed-response count that closes this survey, or `null` when the response limit + * cannot apply — no limit configured, or the survey is already closed. + * + * The limit is defined in terms of *completed* responses, so only finished responses count + * towards it. Counting every response would close the survey once the number of starts + * (partial + finished) hit the limit. + */ +const getAutoCompleteThreshold = (survey: TPipelineSurvey): number | null => + survey.autoComplete && survey.status !== "completed" ? survey.autoComplete : null; + const handleSurveyAutoCompleteSafely = async ({ + finishedResponseCount, logContext, organizationId, - responseCount, survey, }: { + finishedResponseCount: number | null; logContext: ReturnType; organizationId: string; - responseCount: number | null; survey: TPipelineSurvey; }): Promise => { - if (responseCount === null) { - if (survey.autoComplete) { - logger.error( - { - ...logContext, - autoCompleteThreshold: survey.autoComplete, - }, - "Response pipeline survey auto-complete skipped because the response count could not be loaded" - ); - } + const autoCompleteThreshold = getAutoCompleteThreshold(survey); + // `finishedResponseCount` is only looked up when a threshold applies, so a `null` here means + // the lookup failed — already logged by loadResponseCountSafely. + if (autoCompleteThreshold === null || finishedResponseCount === null) { return; } - if (!survey.autoComplete || responseCount < survey.autoComplete) { + if (finishedResponseCount < autoCompleteThreshold) { return; } @@ -628,9 +641,8 @@ const runResponseFinishedSideEffects = async ({ survey: TPipelineSurvey; workspaceId: string; }) => { - const [{ integrations, responseCount }, usersWithNotifications] = await Promise.all([ - loadResponseFinishedContext({ - data, + const [integrations, usersWithNotifications] = await Promise.all([ + loadIntegrationsSafely({ logContext, workspaceId, }), @@ -641,6 +653,30 @@ const runResponseFinishedSideEffects = async ({ }), ]); + // Neither count is consumed until the notification/auto-complete steps at the end, so start + // them here to overlap with the integration and follow-up work below. Each is skipped + // entirely when nothing would consume it, keeping this off the hot path for the common case. + const autoCompleteThreshold = getAutoCompleteThreshold(survey); + + const responseCountPromise = + usersWithNotifications.length > 0 + ? loadResponseCountSafely({ + count: () => getResponseCountBySurveyId(data.surveyId), + failureMessage: "Response pipeline response count lookup failed", + logContext, + }) + : Promise.resolve(null); + + const finishedResponseCountPromise = + autoCompleteThreshold !== null + ? loadResponseCountSafely({ + count: () => getFinishedResponseCountBySurveyId(survey.id), + failureMessage: + "Response pipeline survey auto-complete skipped because the finished response count could not be loaded", + logContext: { ...logContext, autoCompleteThreshold }, + }) + : Promise.resolve(null); + if (integrations.length > 0) { try { await handleIntegrations(integrations, data, survey); @@ -676,16 +712,16 @@ const runResponseFinishedSideEffects = async ({ await sendNotificationEmailsSafely({ data, logContext, - responseCount, + responseCount: await responseCountPromise, survey, usersWithNotifications, workspaceId, }); await handleSurveyAutoCompleteSafely({ + finishedResponseCount: await finishedResponseCountPromise, logContext, organizationId, - responseCount, survey, }); }; diff --git a/apps/web/modules/survey/editor/components/response-options-card.tsx b/apps/web/modules/survey/editor/components/response-options-card.tsx index e5b0eb0d7b2a..6a1130ac5316 100644 --- a/apps/web/modules/survey/editor/components/response-options-card.tsx +++ b/apps/web/modules/survey/editor/components/response-options-card.tsx @@ -26,7 +26,7 @@ import { Slider } from "@/modules/ui/components/slider"; interface ResponseOptionsCardProps { localSurvey: TSurvey; setLocalSurvey: (survey: TSurvey | ((prev: TSurvey) => TSurvey)) => void; - responseCount: number; + finishedResponseCount: number; isSpamProtectionAllowed: boolean; surveySchedulingConfig: TSurveySchedulingConfig; locale: TUserLocale; @@ -35,7 +35,7 @@ interface ResponseOptionsCardProps { export const ResponseOptionsCard = ({ localSurvey, setLocalSurvey, - responseCount, + finishedResponseCount, isSpamProtectionAllowed, surveySchedulingConfig, locale, @@ -245,7 +245,7 @@ export const ResponseOptionsCard = ({ const updatedSurvey = { ...localSurvey, autoComplete: null }; setLocalSurvey(updatedSurvey); } else { - const updatedSurvey = { ...localSurvey, autoComplete: Math.max(25, responseCount + 5) }; + const updatedSurvey = { ...localSurvey, autoComplete: Math.max(25, finishedResponseCount + 5) }; setLocalSurvey(updatedSurvey); } }; @@ -266,10 +266,10 @@ export const ResponseOptionsCard = ({ return; } - if (Number.parseInt(e.target.value) <= responseCount) { + if (Number.parseInt(e.target.value) <= finishedResponseCount) { toast.error( t("workspace.surveys.edit.response_limit_needs_to_exceed_number_of_received_responses", { - responseCount, + responseCount: finishedResponseCount, }), { id: "response-limit-error", @@ -423,7 +423,7 @@ export const ResponseOptionsCard = ({ >; workspace: Workspace; responseCount: number; + finishedResponseCount: number; selectedLanguageCode: string; setSelectedLanguageCode: (selectedLanguage: string) => void; isCxMode: boolean; @@ -56,6 +57,7 @@ export const SurveyMenuBar = ({ setInvalidElements, workspace, responseCount, + finishedResponseCount, selectedLanguageCode, isCxMode, locale, @@ -408,7 +410,7 @@ export const SurveyMenuBar = ({ } try { - const isSurveyValidResult = isSurveyValid(localSurvey, selectedLanguageCode, t, responseCount); + const isSurveyValidResult = isSurveyValid(localSurvey, selectedLanguageCode, t, finishedResponseCount); if (!isSurveyValidResult) { setIsSurveySaving(false); return false; @@ -492,7 +494,7 @@ export const SurveyMenuBar = ({ } try { - const isSurveyValidResult = isSurveyValid(localSurvey, selectedLanguageCode, t, responseCount); + const isSurveyValidResult = isSurveyValid(localSurvey, selectedLanguageCode, t, finishedResponseCount); if (!isSurveyValidResult) { isSurveyPublishingRef.current = false; setIsSurveyPublishing(false); @@ -550,7 +552,7 @@ export const SurveyMenuBar = ({ } try { - const isSurveyValidResult = isSurveyValid(localSurvey, selectedLanguageCode, t, responseCount); + const isSurveyValidResult = isSurveyValid(localSurvey, selectedLanguageCode, t, finishedResponseCount); if (!isSurveyValidResult) { isSurveyPublishingRef.current = false; setIsSurveyPublishing(false); diff --git a/apps/web/modules/survey/editor/lib/validation.ts b/apps/web/modules/survey/editor/lib/validation.ts index 5ebefad72e52..2f8e66f466ab 100644 --- a/apps/web/modules/survey/editor/lib/validation.ts +++ b/apps/web/modules/survey/editor/lib/validation.ts @@ -262,7 +262,11 @@ export const isSurveyValid = ( survey: TSurvey, selectedLanguageCode: string, t: TFunction, - responseCount?: number + /** + * Completed responses only — `survey.autoComplete` is a limit on completions, so partial + * starts must not count towards it. + */ + finishedResponseCount?: number ) => { const questionWithEmptyFallback = checkForEmptyFallBackValue(survey, selectedLanguageCode); if (questionWithEmptyFallback) { @@ -284,16 +288,16 @@ export const isSurveyValid = ( } // Response limit validation - if (survey.autoComplete !== null && responseCount !== undefined) { + if (survey.autoComplete !== null && finishedResponseCount !== undefined) { if (survey.autoComplete === 0) { toast.error(t("workspace.surveys.edit.response_limit_can_t_be_set_to_0")); return false; } - if (survey.autoComplete <= responseCount) { + if (survey.autoComplete <= finishedResponseCount) { toast.error( t("workspace.surveys.edit.response_limit_needs_to_exceed_number_of_received_responses", { - responseCount, + responseCount: finishedResponseCount, }), { id: "response-limit-error", diff --git a/apps/web/modules/survey/editor/page.tsx b/apps/web/modules/survey/editor/page.tsx index c02fa708252f..3f782fa6b527 100644 --- a/apps/web/modules/survey/editor/page.tsx +++ b/apps/web/modules/survey/editor/page.tsx @@ -24,7 +24,10 @@ import { getWorkspaceLanguages } from "@/modules/survey/editor/lib/workspace"; import { getSurveyFollowUpsPermission } from "@/modules/survey/follow-ups/lib/utils"; import { getActionClasses } from "@/modules/survey/lib/action-class"; import { getExternalUrlsPermission } from "@/modules/survey/lib/permission"; -import { getResponseCountBySurveyId } from "@/modules/survey/lib/response"; +import { + getFinishedResponseCountBySurveyId, + getResponseCountBySurveyId, +} from "@/modules/survey/lib/response"; import { getOrganizationBilling, getSurvey } from "@/modules/survey/lib/survey"; import { getWorkspaceWithTeamIds } from "@/modules/survey/lib/workspace"; import { SURVEY_SCHEDULING_CONFIG } from "@/modules/survey/scheduling/lib/constants"; @@ -53,15 +56,23 @@ export const SurveyEditorPage = async (props: { const t = await getTranslate(); - const [survey, workspaceWithTeamIds, actionClasses, contactAttributeKeys, responseCount, segments] = - await Promise.all([ - getSurvey(params.surveyId), - getWorkspaceWithTeamIds(params.workspaceId), - getActionClasses(workspace.id), - getContactAttributeKeys(workspace.id), - getResponseCountBySurveyId(params.surveyId), - getSegments(workspace.id), - ]); + const [ + survey, + workspaceWithTeamIds, + actionClasses, + contactAttributeKeys, + responseCount, + finishedResponseCount, + segments, + ] = await Promise.all([ + getSurvey(params.surveyId), + getWorkspaceWithTeamIds(params.workspaceId), + getActionClasses(workspace.id), + getContactAttributeKeys(workspace.id), + getResponseCountBySurveyId(params.surveyId), + getFinishedResponseCountBySurveyId(params.surveyId), + getSegments(workspace.id), + ]); if (!workspaceWithTeamIds) { throw new ResourceNotFoundError(t("common.workspace"), null); @@ -119,6 +130,7 @@ export const SurveyEditorPage = async (props: { actionClasses={actionClasses} contactAttributeKeys={contactAttributeKeys} responseCount={responseCount} + finishedResponseCount={finishedResponseCount} membershipRole={currentUserMembership.role} workspacePermission={workspacePermission} colors={SURVEY_BG_COLORS} diff --git a/apps/web/modules/survey/lib/response.test.ts b/apps/web/modules/survey/lib/response.test.ts index 5f07e8ab7811..1dac2f4b6a97 100644 --- a/apps/web/modules/survey/lib/response.test.ts +++ b/apps/web/modules/survey/lib/response.test.ts @@ -2,7 +2,7 @@ import { afterEach, describe, expect, test, vi } from "vitest"; import { prisma } from "@formbricks/database"; import { Prisma } from "@formbricks/database/prisma"; import { DatabaseError } from "@formbricks/types/errors"; -import { getResponseCountBySurveyId } from "./response"; +import { getFinishedResponseCountBySurveyId, getResponseCountBySurveyId } from "./response"; vi.mock("react", async () => { const actual = await vi.importActual("react"); @@ -62,3 +62,37 @@ describe("getResponseCountBySurveyId", () => { }); }); }); + +describe("getFinishedResponseCountBySurveyId", () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + test("should count only finished responses, excluding partial starts", async () => { + vi.mocked(prisma.response.count).mockResolvedValue(3); + + const result = await getFinishedResponseCountBySurveyId(surveyId); + + expect(result).toBe(3); + expect(prisma.response.count).toHaveBeenCalledWith({ + where: { surveyId, finished: true }, + }); + }); + + test("should throw DatabaseError if PrismaClientKnownRequestError occurs", async () => { + const prismaError = new Prisma.PrismaClientKnownRequestError("Test Prisma Error", { + code: "P2002", + clientVersion: "2.0.0", + }); + vi.mocked(prisma.response.count).mockRejectedValue(prismaError); + + await expect(getFinishedResponseCountBySurveyId(surveyId)).rejects.toThrow(DatabaseError); + }); + + test("should throw generic error if an unknown error occurs", async () => { + const genericError = new Error("Test Generic Error"); + vi.mocked(prisma.response.count).mockRejectedValue(genericError); + + await expect(getFinishedResponseCountBySurveyId(surveyId)).rejects.toThrow(genericError); + }); +}); diff --git a/apps/web/modules/survey/lib/response.ts b/apps/web/modules/survey/lib/response.ts index aecdfb065d80..68787e0a72d8 100644 --- a/apps/web/modules/survey/lib/response.ts +++ b/apps/web/modules/survey/lib/response.ts @@ -3,14 +3,9 @@ import { prisma } from "@formbricks/database"; import { Prisma } from "@formbricks/database/prisma"; import { DatabaseError } from "@formbricks/types/errors"; -export const getResponseCountBySurveyId = reactCache(async (surveyId: string): Promise => { +const countResponses = async (where: Prisma.ResponseWhereInput): Promise => { try { - const responseCount = await prisma.response.count({ - where: { - surveyId, - }, - }); - return responseCount; + return await prisma.response.count({ where }); } catch (error) { if (error instanceof Prisma.PrismaClientKnownRequestError) { throw new DatabaseError(error.message); @@ -18,4 +13,18 @@ export const getResponseCountBySurveyId = reactCache(async (surveyId: string): P throw error; } -}); +}; + +/** Counts every response row for the survey, including partial starts. */ +export const getResponseCountBySurveyId = reactCache( + async (surveyId: string): Promise => countResponses({ surveyId }) +); + +/** + * Counts completed responses only. The response limit ("Close survey on response limit") is + * defined in terms of completed responses, so partial starts must never count towards it — + * anything comparing against `survey.autoComplete` has to use this count. + */ +export const getFinishedResponseCountBySurveyId = reactCache( + async (surveyId: string): Promise => countResponses({ surveyId, finished: true }) +);