-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Follow up reminders #1247
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Follow up reminders #1247
Changes from all commits
58a762e
7a27102
446f9f1
bf7b06f
0f51985
00f4306
4194b84
5283726
cd92f88
a48e8f4
c2ba759
aa9af0c
a3518a8
1907cee
e810a7c
6264a3c
a574d86
f629e0d
dbb3b04
b1228a4
3821212
6e394f8
ae7a278
ec885c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -31,7 +31,6 @@ import { | |||||
| } from "./helpers/polling"; | ||||||
| import { logStep, clearLogs, setTestStartTime } from "./helpers/logging"; | ||||||
| import type { TestAccount } from "./helpers/accounts"; | ||||||
| import prisma from "@/utils/prisma"; | ||||||
|
|
||||||
| describe.skipIf(!shouldRunFlowTests())("Full Reply Cycle", () => { | ||||||
| let gmail: TestAccount; | ||||||
|
|
@@ -157,26 +156,12 @@ describe.skipIf(!shouldRunFlowTests())("Full Reply Cycle", () => { | |||||
| ); | ||||||
|
|
||||||
| if (labelAction?.labelId) { | ||||||
| // Look up the label name from the database | ||||||
| const label = await prisma.label.findUnique({ | ||||||
| where: { id: labelAction.labelId }, | ||||||
| select: { name: true }, | ||||||
| }); | ||||||
|
|
||||||
| const message = await outlook.emailProvider.getMessage( | ||||||
| outlookMessage.messageId, | ||||||
| ); | ||||||
| expect(message.labelIds).toBeDefined(); | ||||||
|
|
||||||
| // Check if the label name is in the message's labels | ||||||
| // Outlook returns label names (categories), not IDs | ||||||
| if (label?.name) { | ||||||
| expect(message.labelIds).toContain(label.name); | ||||||
| logStep("Label verified on message", { | ||||||
| labelName: label.name, | ||||||
| messageLabels: message.labelIds, | ||||||
| }); | ||||||
| } | ||||||
| expect(message.labelIds).toContain(labelAction.labelId); | ||||||
| logStep("Labels on message", { labels: message.labelIds }); | ||||||
| } | ||||||
|
|
||||||
| // ======================================== | ||||||
|
|
@@ -302,10 +287,10 @@ describe.skipIf(!shouldRunFlowTests())("Full Reply Cycle", () => { | |||||
| body: "This is the reply from Outlook.", | ||||||
| }); | ||||||
|
|
||||||
| // Wait for Gmail to receive - use fullSubject for unique match | ||||||
| // Wait for Gmail to receive | ||||||
| const gmailReply = await waitForMessageInInbox({ | ||||||
| provider: gmail.emailProvider, | ||||||
| subjectContains: initialEmail.fullSubject, | ||||||
| subjectContains: "Thread continuity test", | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Test may match emails from previous runs instead of current test. Changed from Prompt for AI agents
Suggested change
|
||||||
| timeout: TIMEOUTS.EMAIL_DELIVERY, | ||||||
| }); | ||||||
|
Comment on lines
+290
to
295
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent subject matching could cause flaky tests. The test uses 🔧 Suggested fix for consistency- // Wait for Gmail to receive
const gmailReply = await waitForMessageInInbox({
provider: gmail.emailProvider,
- subjectContains: "Thread continuity test",
+ subjectContains: initialEmail.fullSubject,
timeout: TIMEOUTS.EMAIL_DELIVERY,
});🤖 Prompt for AI Agents |
||||||
|
|
||||||
|
|
@@ -325,10 +310,10 @@ describe.skipIf(!shouldRunFlowTests())("Full Reply Cycle", () => { | |||||
| body: "This is the second reply from Gmail.", | ||||||
| }); | ||||||
|
|
||||||
| // Wait for Outlook to receive - use fullSubject for unique match | ||||||
| // Wait for Outlook to receive | ||||||
| const outlookMsg2 = await waitForMessageInInbox({ | ||||||
| provider: outlook.emailProvider, | ||||||
| subjectContains: initialEmail.fullSubject, | ||||||
| subjectContains: "Thread continuity test", | ||||||
| timeout: TIMEOUTS.EMAIL_DELIVERY, | ||||||
| }); | ||||||
|
Comment on lines
+313
to
318
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same inconsistency - use fullSubject for reliable matching. Similar to the issue at lines 290-295, this should use 🔧 Suggested fix for consistency- // Wait for Outlook to receive
const outlookMsg2 = await waitForMessageInInbox({
provider: outlook.emailProvider,
- subjectContains: "Thread continuity test",
+ subjectContains: initialEmail.fullSubject,
timeout: TIMEOUTS.EMAIL_DELIVERY,
});🤖 Prompt for AI Agents |
||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,264 @@ | ||
| "use client"; | ||
|
|
||
| import { useState, useCallback } from "react"; | ||
| import { useForm } from "react-hook-form"; | ||
| import { Button } from "@/components/ui/button"; | ||
| import { SettingCard } from "@/components/SettingCard"; | ||
| import { | ||
| Dialog, | ||
| DialogContent, | ||
| DialogDescription, | ||
| DialogHeader, | ||
| DialogTitle, | ||
| DialogTrigger, | ||
| } from "@/components/ui/dialog"; | ||
| import { Input } from "@/components/Input"; | ||
| import { Toggle } from "@/components/Toggle"; | ||
| import { Badge } from "@/components/Badge"; | ||
| import { useEmailAccountFull } from "@/hooks/useEmailAccountFull"; | ||
| import { useFollowUpRemindersEnabled } from "@/hooks/useFeatureFlags"; | ||
| import { useAccount } from "@/providers/EmailAccountProvider"; | ||
| import { useAction } from "next-safe-action/hooks"; | ||
| import { | ||
| toggleFollowUpRemindersAction, | ||
| updateFollowUpSettingsAction, | ||
| scanFollowUpRemindersAction, | ||
| } from "@/utils/actions/follow-up-reminders"; | ||
| import { | ||
| type SaveFollowUpSettingsFormInput, | ||
| DEFAULT_FOLLOW_UP_DAYS, | ||
| } from "@/utils/actions/follow-up-reminders.validation"; | ||
| import { toastError, toastSuccess } from "@/components/Toast"; | ||
| import { getEmailTerminology } from "@/utils/terminology"; | ||
|
|
||
| export function FollowUpRemindersSetting() { | ||
| const isFeatureEnabled = useFollowUpRemindersEnabled(); | ||
|
|
||
| if (!isFeatureEnabled) return null; | ||
|
|
||
| return <FollowUpRemindersSettingContent />; | ||
| } | ||
|
|
||
| function FollowUpRemindersSettingContent() { | ||
| const [open, setOpen] = useState(false); | ||
| const { data, mutate } = useEmailAccountFull(); | ||
|
|
||
| const enabled = | ||
| data?.followUpAwaitingReplyDays !== null || | ||
| data?.followUpNeedsReplyDays !== null; | ||
|
|
||
| const { execute: executeToggle } = useAction( | ||
| toggleFollowUpRemindersAction.bind(null, data?.id ?? ""), | ||
| { | ||
| onError: (error) => { | ||
| mutate(); | ||
| toastError({ | ||
| description: error.error?.serverError ?? "Failed to update settings", | ||
| }); | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| const handleToggle = useCallback( | ||
| (enable: boolean) => { | ||
| if (!data) return; | ||
|
|
||
| const optimisticData = { | ||
| ...data, | ||
| followUpAwaitingReplyDays: enable ? DEFAULT_FOLLOW_UP_DAYS : null, | ||
| followUpNeedsReplyDays: enable ? DEFAULT_FOLLOW_UP_DAYS : null, | ||
| }; | ||
| mutate(optimisticData, false); | ||
| executeToggle({ enabled: enable }); | ||
| }, | ||
| [data, mutate, executeToggle], | ||
| ); | ||
|
|
||
| return ( | ||
| <SettingCard | ||
| title="Follow-up Reminders" | ||
| description="Get reminded when you haven't heard back or haven't replied." | ||
| right={ | ||
| <div className="flex items-center gap-2"> | ||
| {enabled && ( | ||
| <Dialog open={open} onOpenChange={setOpen}> | ||
| <DialogTrigger asChild> | ||
| <Button variant="outline" size="sm"> | ||
| Configure | ||
| </Button> | ||
| </DialogTrigger> | ||
| <FollowUpSettingsDialog | ||
| emailAccountId={data?.id ?? ""} | ||
| followUpAwaitingReplyDays={data?.followUpAwaitingReplyDays} | ||
| followUpNeedsReplyDays={data?.followUpNeedsReplyDays} | ||
| followUpAutoDraftEnabled={ | ||
| data?.followUpAutoDraftEnabled ?? true | ||
| } | ||
| onSuccess={() => { | ||
| mutate(); | ||
| setOpen(false); | ||
| }} | ||
| /> | ||
| </Dialog> | ||
| )} | ||
| <Toggle | ||
| name="follow-up-enabled" | ||
| enabled={enabled} | ||
| onChange={handleToggle} | ||
| disabled={!data} | ||
| /> | ||
| </div> | ||
| } | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| function FollowUpSettingsDialog({ | ||
| emailAccountId, | ||
| followUpAwaitingReplyDays, | ||
| followUpNeedsReplyDays, | ||
| followUpAutoDraftEnabled, | ||
| onSuccess, | ||
| }: { | ||
| emailAccountId: string; | ||
| followUpAwaitingReplyDays: number | null | undefined; | ||
| followUpNeedsReplyDays: number | null | undefined; | ||
| followUpAutoDraftEnabled: boolean; | ||
| onSuccess: () => void; | ||
| }) { | ||
| const { provider } = useAccount(); | ||
| const terminology = getEmailTerminology(provider); | ||
|
|
||
| const { | ||
| register, | ||
| handleSubmit, | ||
| watch, | ||
| setValue, | ||
| formState: { errors }, | ||
| } = useForm<SaveFollowUpSettingsFormInput>({ | ||
| defaultValues: { | ||
| followUpAwaitingReplyDays: followUpAwaitingReplyDays?.toString() ?? "", | ||
| followUpNeedsReplyDays: followUpNeedsReplyDays?.toString() ?? "", | ||
| followUpAutoDraftEnabled, | ||
| }, | ||
| }); | ||
|
|
||
| const autoDraftValue = watch("followUpAutoDraftEnabled"); | ||
|
|
||
| const { execute, isExecuting } = useAction( | ||
| updateFollowUpSettingsAction.bind(null, emailAccountId), | ||
| { | ||
| onSuccess: () => { | ||
| toastSuccess({ description: "Settings saved!" }); | ||
| onSuccess(); | ||
| }, | ||
| onError: (error) => { | ||
| toastError({ | ||
| description: error.error?.serverError ?? "Failed to save settings", | ||
| }); | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| const { execute: executeScan, isExecuting: isScanning } = useAction( | ||
| scanFollowUpRemindersAction.bind(null, emailAccountId), | ||
| { | ||
| onSuccess: () => { | ||
| toastSuccess({ description: "Scan complete!" }); | ||
| }, | ||
| onError: (error) => { | ||
| toastError({ | ||
| description: error.error?.serverError ?? "Failed to scan", | ||
| }); | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| const onSubmit = (formData: SaveFollowUpSettingsFormInput) => { | ||
| execute({ | ||
| followUpAwaitingReplyDays: formData.followUpAwaitingReplyDays | ||
| ? Number(formData.followUpAwaitingReplyDays) | ||
| : null, | ||
| followUpNeedsReplyDays: formData.followUpNeedsReplyDays | ||
| ? Number(formData.followUpNeedsReplyDays) | ||
| : null, | ||
| followUpAutoDraftEnabled: formData.followUpAutoDraftEnabled, | ||
| }); | ||
| }; | ||
|
|
||
| return ( | ||
| <DialogContent> | ||
| <DialogHeader> | ||
| <DialogTitle>Follow-up Reminders</DialogTitle> | ||
| <DialogDescription> | ||
| Get reminded about conversations that need attention. | ||
| <br /> | ||
| We'll add a <Badge color="blue">Follow-up</Badge>{" "} | ||
| {terminology.label.singular} so you can easily find them. | ||
| </DialogDescription> | ||
| </DialogHeader> | ||
|
|
||
| <form onSubmit={handleSubmit(onSubmit)} className="space-y-4"> | ||
| <Input | ||
| type="number" | ||
| name="followUpAwaitingReplyDays" | ||
| label="Remind me when they haven't replied after" | ||
| explainText="Leave blank to disable" | ||
| registerProps={register("followUpAwaitingReplyDays")} | ||
| error={errors.followUpAwaitingReplyDays} | ||
| min={0.001} | ||
| max={90} | ||
| step={0.001} | ||
| rightText="days" | ||
| /> | ||
|
|
||
| <Input | ||
| type="number" | ||
| name="followUpNeedsReplyDays" | ||
| label="Remind me when I haven't replied after" | ||
| explainText="Leave blank to disable" | ||
| registerProps={register("followUpNeedsReplyDays")} | ||
| error={errors.followUpNeedsReplyDays} | ||
| min={0.001} | ||
| max={90} | ||
| step={0.001} | ||
| rightText="days" | ||
| /> | ||
|
elie222 marked this conversation as resolved.
|
||
|
|
||
| <div className="flex items-center justify-between"> | ||
| <div> | ||
| <label | ||
| htmlFor="followUpAutoDraftEnabled" | ||
| className="block text-sm font-medium text-foreground" | ||
| > | ||
| Auto-generate drafts | ||
| </label> | ||
| <p className="text-muted-foreground text-sm"> | ||
| Draft a nudge when you haven't heard back. | ||
| </p> | ||
| </div> | ||
| <Toggle | ||
| name="followUpAutoDraftEnabled" | ||
| enabled={autoDraftValue} | ||
| onChange={(value) => setValue("followUpAutoDraftEnabled", value)} | ||
| /> | ||
| </div> | ||
|
Comment on lines
+228
to
+245
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix label → control association for “Auto-generate drafts”.
Options:
(Based on coding guidelines: “Make sure label elements have text content and are associated with an input”.) 🤖 Prompt for AI Agents |
||
|
|
||
| <div className="flex items-center gap-2"> | ||
| <Button type="submit" size="sm" loading={isExecuting}> | ||
| Save | ||
| </Button> | ||
| <Button | ||
| type="button" | ||
| variant="outline" | ||
| size="sm" | ||
| loading={isScanning} | ||
| onClick={() => executeScan({})} | ||
| > | ||
| Scan now | ||
| </Button> | ||
| </div> | ||
| </form> | ||
| </DialogContent> | ||
| ); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Label verification is comparing database ID to label names array. For Outlook,
message.labelIdscontains category names, not database IDs. The assertionexpect(message.labelIds).toContain(labelAction.labelId)will fail becauselabelAction.labelIdis a database ID whilemessage.labelIdscontains label names. Need to fetch the label name from the database (as the removed code did) or use a helper that maps IDs to names.Prompt for AI agents