From ce0134e7843bb1acba9920a2478b239947c3f6d9 Mon Sep 17 00:00:00 2001 From: Eliezer Steinbock <3090527+elie222@users.noreply.github.com> Date: Thu, 5 Jun 2025 22:02:10 +0300 Subject: [PATCH] Delete old drafts --- .cursor/rules/testing.mdc | 3 +- .../ai/choose-rule/draft-management.test.ts | 227 ++++++++++++++++++ apps/web/utils/ai/actions.ts | 19 +- .../utils/ai/choose-rule/draft-management.ts | 44 +++- version.txt | 2 +- 5 files changed, 281 insertions(+), 14 deletions(-) create mode 100644 apps/web/__tests__/ai/choose-rule/draft-management.test.ts diff --git a/.cursor/rules/testing.mdc b/.cursor/rules/testing.mdc index 6203429365..5938ed34eb 100644 --- a/.cursor/rules/testing.mdc +++ b/.cursor/rules/testing.mdc @@ -41,4 +41,5 @@ describe("example", () => { - Use descriptive test names - Mock external dependencies - Clean up mocks between tests -- Avoid testing implementation details \ No newline at end of file +- Avoid testing implementation details +- Do not mock the Logger \ No newline at end of file diff --git a/apps/web/__tests__/ai/choose-rule/draft-management.test.ts b/apps/web/__tests__/ai/choose-rule/draft-management.test.ts new file mode 100644 index 0000000000..dc54f3d015 --- /dev/null +++ b/apps/web/__tests__/ai/choose-rule/draft-management.test.ts @@ -0,0 +1,227 @@ +import { describe, it, expect, vi, beforeEach, type Mock } from "vitest"; +import { handlePreviousDraftDeletion } from "@/utils/ai/choose-rule/draft-management"; +import prisma from "@/utils/prisma"; +import { getDraft, deleteDraft } from "@/utils/gmail/draft"; +import { ActionType } from "@prisma/client"; +import type { gmail_v1 } from "@googleapis/gmail"; +import { createScopedLogger } from "@/utils/logger"; +import type { ParsedMessage } from "@/utils/types"; + +vi.mock("@/utils/prisma", () => ({ + default: { + executedAction: { + findFirst: vi.fn(), + update: vi.fn(), + }, + }, +})); + +vi.mock("@/utils/gmail/draft", () => ({ + getDraft: vi.fn(), + deleteDraft: vi.fn(), +})); + +describe("handlePreviousDraftDeletion", () => { + const mockGmail = {} as gmail_v1.Gmail; + const logger = createScopedLogger("test"); + const mockExecutedRule = { + id: "rule-123", + threadId: "thread-456", + emailAccountId: "account-789", + }; + + const mockFindFirst = prisma.executedAction.findFirst as Mock; + const mockUpdate = prisma.executedAction.update as Mock; + const mockGetDraft = getDraft as Mock; + const mockDeleteDraft = deleteDraft as Mock; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("should delete unmodified draft and update wasDraftSent", async () => { + const mockPreviousDraft = { + id: "action-111", + draftId: "draft-222", + content: "Hello, this is a test draft", + }; + + const mockCurrentDraft: ParsedMessage = { + id: "msg-123", + threadId: "thread-456", + textPlain: + "Hello, this is a test draft\n\nOn Monday wrote:\n> Previous message", + textHtml: undefined, + snippet: "Hello, this is a test draft", + historyId: "12345", + internalDate: "1234567890", + headers: { + from: "test@example.com", + to: "recipient@example.com", + subject: "Test Subject", + date: "Mon, 1 Jan 2024 12:00:00 +0000", + }, + labelIds: [], + inline: [], + }; + + mockFindFirst.mockResolvedValue(mockPreviousDraft); + mockGetDraft.mockResolvedValue(mockCurrentDraft); + + await handlePreviousDraftDeletion({ + gmail: mockGmail, + executedRule: mockExecutedRule, + logger, + }); + + expect(mockFindFirst).toHaveBeenCalledWith({ + where: { + executedRule: { + threadId: "thread-456", + emailAccountId: "account-789", + }, + type: ActionType.DRAFT_EMAIL, + draftId: { not: null }, + executedRuleId: { not: "rule-123" }, + draftSendLog: null, + }, + orderBy: { + createdAt: "desc", + }, + select: { + id: true, + draftId: true, + content: true, + }, + }); + + expect(mockGetDraft).toHaveBeenCalledWith("draft-222", mockGmail); + expect(mockDeleteDraft).toHaveBeenCalledWith(mockGmail, "draft-222"); + expect(mockUpdate).toHaveBeenCalledWith({ + where: { id: "action-111" }, + data: { wasDraftSent: false }, + }); + }); + + it("should not delete modified draft", async () => { + const mockPreviousDraft = { + id: "action-111", + draftId: "draft-222", + content: "Hello, this is a test draft", + }; + + const mockCurrentDraft: ParsedMessage = { + id: "msg-123", + threadId: "thread-456", + textPlain: + "Hello, this is a MODIFIED draft\n\nOn Monday wrote:\n> Previous message", + textHtml: undefined, + snippet: "Hello, this is a MODIFIED draft", + historyId: "12345", + internalDate: "1234567890", + headers: { + from: "test@example.com", + to: "recipient@example.com", + subject: "Test Subject", + date: "Mon, 1 Jan 2024 12:00:00 +0000", + }, + labelIds: [], + inline: [], + }; + + mockFindFirst.mockResolvedValue(mockPreviousDraft); + mockGetDraft.mockResolvedValue(mockCurrentDraft); + + await handlePreviousDraftDeletion({ + gmail: mockGmail, + executedRule: mockExecutedRule, + logger, + }); + + expect(mockDeleteDraft).not.toHaveBeenCalled(); + expect(mockUpdate).not.toHaveBeenCalled(); + }); + + it("should handle no previous draft found", async () => { + mockFindFirst.mockResolvedValue(null); + + await handlePreviousDraftDeletion({ + gmail: mockGmail, + executedRule: mockExecutedRule, + logger, + }); + + expect(mockGetDraft).not.toHaveBeenCalled(); + expect(mockDeleteDraft).not.toHaveBeenCalled(); + }); + + it("should handle draft not found in Gmail", async () => { + const mockPreviousDraft = { + id: "action-111", + draftId: "draft-222", + content: "Hello, this is a test draft", + }; + + mockFindFirst.mockResolvedValue(mockPreviousDraft); + mockGetDraft.mockResolvedValue(null); + + await handlePreviousDraftDeletion({ + gmail: mockGmail, + executedRule: mockExecutedRule, + logger, + }); + + expect(mockDeleteDraft).not.toHaveBeenCalled(); + }); + + it("should handle errors gracefully", async () => { + const error = new Error("Database error"); + mockFindFirst.mockRejectedValue(error); + + // Should not throw - errors are caught and logged + await expect( + handlePreviousDraftDeletion({ + gmail: mockGmail, + executedRule: mockExecutedRule, + logger, + }), + ).resolves.not.toThrow(); + }); + + it("should handle draft with no textPlain content", async () => { + const mockPreviousDraft = { + id: "action-111", + draftId: "draft-222", + content: "Hello, this is a test draft", + }; + + const mockCurrentDraft = { + id: "msg-123", + threadId: "thread-456", + // No textPlain property + textHtml: "

HTML content

", + snippet: "HTML content", + historyId: "12345", + internalDate: "1234567890", + headers: { + from: "test@example.com", + to: "recipient@example.com", + subject: "Test Subject", + date: "Mon, 1 Jan 2024 12:00:00 +0000", + }, + labelIds: [], + inline: [], + }; + + mockFindFirst.mockResolvedValue(mockPreviousDraft); + mockGetDraft.mockResolvedValue(mockCurrentDraft); + + await handlePreviousDraftDeletion({ + gmail: mockGmail, + executedRule: mockExecutedRule, + logger, + }); + + expect(mockDeleteDraft).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/utils/ai/actions.ts b/apps/web/utils/ai/actions.ts index 721b30c9eb..b54d828c42 100644 --- a/apps/web/utils/ai/actions.ts +++ b/apps/web/utils/ai/actions.ts @@ -19,6 +19,7 @@ import { callWebhook } from "@/utils/webhook"; import type { ActionItem, EmailForAction } from "@/utils/ai/types"; import { coordinateReplyProcess } from "@/utils/reply-tracker/inbound"; import { internalDateToDate } from "@/utils/date"; +import { handlePreviousDraftDeletion } from "@/utils/ai/choose-rule/draft-management"; const logger = createScopedLogger("ai-actions"); @@ -120,8 +121,22 @@ const label: ActionFunction<{ label: string } | any> = async ({ // content: string; // attachments?: Attachment[]; // }, -const draft: ActionFunction = async ({ gmail, email, args }) => { - const result = await draftEmail(gmail, email, args); +const draft: ActionFunction = async ({ + gmail, + email, + args, + executedRule, +}) => { + // Run draft creation and previous draft deletion in parallel + const [result] = await Promise.all([ + draftEmail(gmail, email, args), + handlePreviousDraftDeletion({ + gmail, + executedRule, + logger, + }), + ]); + return { draftId: result.data.message?.id }; }; diff --git a/apps/web/utils/ai/choose-rule/draft-management.ts b/apps/web/utils/ai/choose-rule/draft-management.ts index ae842ffe7e..3019170346 100644 --- a/apps/web/utils/ai/choose-rule/draft-management.ts +++ b/apps/web/utils/ai/choose-rule/draft-management.ts @@ -22,12 +22,13 @@ export async function handlePreviousDraftDeletion({ const previousDraftAction = await prisma.executedAction.findFirst({ where: { executedRule: { - threadId: executedRule.threadId, // Match threadId - emailAccountId: executedRule.emailAccountId, // Match emailAccountId for safety + threadId: executedRule.threadId, + emailAccountId: executedRule.emailAccountId, }, type: ActionType.DRAFT_EMAIL, draftId: { not: null }, // Ensure it has a draftId - executedRuleId: { not: executedRule.id }, // Explicitly exclude actions from the current rule execution + executedRuleId: { not: executedRule.id }, // Explicitly exclude current executedRule from the current rule execution + draftSendLog: null, // Only consider drafts not logged as sent }, orderBy: { createdAt: "desc", // Get the most recent one @@ -52,10 +53,24 @@ export async function handlePreviousDraftDeletion({ if (currentDraftDetails?.textPlain) { // Basic comparison: Compare original content with current plain text - const quoteHeaderRegex = /\n\nOn .* wrote:/; - const currentReplyContent = currentDraftDetails.textPlain - .split(quoteHeaderRegex)[0] - ?.trim(); + // Try multiple quote header patterns + const quoteHeaderPatterns = [ + /\n\nOn .* wrote:/, + /\n\n----+ Original Message ----+/, + /\n\n>+ On .*/, + /\n\nFrom: .*/, + ]; + + let currentReplyContent = currentDraftDetails.textPlain; + for (const pattern of quoteHeaderPatterns) { + const parts = currentReplyContent.split(pattern); + if (parts.length > 1) { + currentReplyContent = parts[0]; + break; + } + } + currentReplyContent = currentReplyContent.trim(); + const originalContent = previousDraftAction.content?.trim(); logger.info("Comparing draft content", { @@ -65,7 +80,17 @@ export async function handlePreviousDraftDeletion({ if (originalContent === currentReplyContent) { logger.info("Draft content matches, deleting draft."); - await deleteDraft(gmail, previousDraftAction.draftId); + + // Delete the draft and mark as not sent + await Promise.all([ + deleteDraft(gmail, previousDraftAction.draftId), + prisma.executedAction.update({ + where: { id: previousDraftAction.id }, + data: { wasDraftSent: false }, + }), + ]); + + logger.info("Deleted draft and updated action status."); } else { logger.info("Draft content modified by user, skipping deletion."); } @@ -99,7 +124,7 @@ export async function updateExecutedActionWithDraftId({ try { await prisma.executedAction.update({ where: { id: actionId }, - data: { draftId: draftId }, + data: { draftId }, }); logger.info("Updated executed action with draft ID", { actionId, draftId }); } catch (error) { @@ -108,6 +133,5 @@ export async function updateExecutedActionWithDraftId({ draftId, error, }); - // Depending on requirements, you might want to re-throw or handle this error differently. } } diff --git a/version.txt b/version.txt index 06f8fe6766..97455a7b94 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -v1.3.20 \ No newline at end of file +v1.3.21 \ No newline at end of file