Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/web/app/(app)/automation/BulkRunRules.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function BulkRunRules() {
const [startDate, setStartDate] = useState<Date | undefined>();
const [endDate, setEndDate] = useState<Date | undefined>();

const abortRef = useRef<() => void>();
const abortRef = useRef<() => void>(undefined);

return (
<div>
Expand Down
24 changes: 11 additions & 13 deletions apps/web/app/(app)/bulk-unsubscribe/ArchiveProgress.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,23 @@
import { useEffect } from "react";
import { memo, useEffect } from "react";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Issues found with import paths.

The import path for resetTotalThreads is invalid; store/archive-queue.ts does not exist or does not export resetTotalThreads.

🔗 Analysis chain

LGTM! Verify import paths.

The changes to the import statements look good. The addition of memo for component optimization and the reorganization of imports improve code structure and potential performance.

Let's verify the new import paths:

Also applies to: 5-5

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence and exports of the imported modules.

# Test 1: Check if queueAtom is exported from @/store/archive-queue
rg -q 'export.*queueAtom' 'store/archive-queue.ts' && echo "queueAtom export found" || echo "queueAtom export not found"

# Test 2: Check if resetTotalThreads is exported from @/store/archive-queue
rg -q 'export.*resetTotalThreads' 'store/archive-queue.ts' && echo "resetTotalThreads export found" || echo "resetTotalThreads export not found"

Length of output: 388

import { AnimatePresence, motion } from "framer-motion";
import { useAtomValue } from "jotai";
import { ProgressBar } from "@tremor/react";
import { queueAtoms, resetTotalThreads } from "@/store/archive-queue";
import { queueAtom, resetTotalThreads } from "@/store/archive-queue";
import { cn } from "@/utils";

export const ArchiveProgress = () => {
const { totalThreads, activeThreadIds } = useAtomValue(queueAtoms.archive);
export const ArchiveProgress = memo(() => {
const { totalThreads, activeThreads } = useAtomValue(queueAtom);

// Make sure activeThreadIds is an object as this was causing an error.
const threadsRemaining = Object.values(activeThreadIds || {}).filter(
Boolean,
).length;
const totalArchived = totalThreads - threadsRemaining;
const progress = (totalArchived / totalThreads) * 100;
// Make sure activeThreads is an object as this was causing an error.
const threadsRemaining = Object.values(activeThreads || {}).length;
const totalProcessed = totalThreads - threadsRemaining;
const progress = (totalProcessed / totalThreads) * 100;
const isCompleted = progress === 100;

useEffect(() => {
if (isCompleted) {
setTimeout(() => {
resetTotalThreads("archive");
resetTotalThreads();
}, 5_000);
}
}, [isCompleted]);
Expand Down Expand Up @@ -50,11 +48,11 @@ export const ArchiveProgress = () => {
{isCompleted ? "Archiving complete!" : "Archiving emails..."}
</span>
<span>
{totalArchived} of {totalThreads} emails archived
{totalProcessed} of {totalThreads} emails archived
</span>
</p>
</motion.div>
</AnimatePresence>
</div>
);
};
});
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { SearchBar } from "@/app/(app)/bulk-unsubscribe/SearchBar";
import { useToggleSelect } from "@/hooks/useToggleSelect";
import { BulkActions } from "@/app/(app)/bulk-unsubscribe/BulkActions";
import { ArchiveProgress } from "@/app/(app)/bulk-unsubscribe/ArchiveProgress";
import { ClientOnly } from "@/components/ClientOnly";

type Newsletter = NewsletterStatsResponse["newsletters"][number];

Expand Down Expand Up @@ -227,7 +228,9 @@ export function BulkUnsubscribeSection({
</div>
</div>

<ArchiveProgress />
<ClientOnly>
<ArchiveProgress />
</ClientOnly>

{isStatsLoading && !isLoading && !data?.newsletters.length ? (
<div className="p-4">
Expand Down
4 changes: 3 additions & 1 deletion apps/web/app/(app)/bulk-unsubscribe/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ async function autoArchive(
await mutate();
await decrementUnsubscribeCreditAction();
await refetchPremium();
await archiveAllSenderEmails(name, () => {});
await archiveAllSenderEmails(name, () => {}, labelId);
}

export function useAutoArchive<T extends Row>({
Expand Down Expand Up @@ -182,6 +182,8 @@ export function useAutoArchive<T extends Row>({
status: null,
});
await mutate();

setAutoArchiveLoading(false);
}, [item.name, mutate, posthog, refetchPremium]);

const onAutoArchiveAndLabel = useCallback(
Expand Down
1 change: 1 addition & 0 deletions apps/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
"novel": "0.3.1",
"openai": "^4.67.3",
"p-queue": "^8.0.1",
"p-retry": "^6.2.0",
"posthog-js": "^1.167.0",
"posthog-node": "^4.2.0",
"prettier": "^3.3.3",
Expand Down
30 changes: 12 additions & 18 deletions apps/web/store/QueueInitializer.tsx
Original file line number Diff line number Diff line change
@@ -1,28 +1,22 @@
"use client";

import { useAtom } from "jotai";
import { useAtomValue } from "jotai";
import { useEffect } from "react";
import { processQueue, queueAtoms } from "@/store/archive-queue";
import { processQueue, queueAtom } from "@/store/archive-queue";

function useInitializeQueues() {
const [archiveQueue] = useAtom(queueAtoms.archive);
const [deleteQueue] = useAtom(queueAtoms.delete);
const [markReadQueue] = useAtom(queueAtoms.markRead);

useEffect(() => {
const threadIds = Object.keys(archiveQueue.activeThreadIds || {});
if (threadIds.length) processQueue("archive", threadIds);
}, [archiveQueue]);
let isInitialized = false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Avoid using module-scoped variables for component state

Defining isInitialized at the module scope can lead to unexpected behavior, especially if multiple instances of useInitializeQueues are used. All instances will share the same isInitialized variable, which may cause the initialization logic to be skipped unintentionally.

Consider using the useRef hook to track initialization per component instance.

Apply this diff to refactor:

- let isInitialized = false;

  function useInitializeQueues() {
+   const isInitialized = useRef(false);
    const queueState = useAtomValue(queueAtom);

    useEffect(() => {
-     if (!isInitialized) {
-       isInitialized = true;
+     if (!isInitialized.current) {
+       isInitialized.current = true;
        if (queueState.activeThreads) {
          processQueue({ threads: queueState.activeThreads });
        }
      }
    }, [queueState.activeThreads]);
  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let isInitialized = false;
import { useRef } from 'react';
function useInitializeQueues() {
const isInitialized = useRef(false);
const queueState = useAtomValue(queueAtom);
useEffect(() => {
if (!isInitialized.current) {
isInitialized.current = true;
if (queueState.activeThreads) {
processQueue({ threads: queueState.activeThreads });
}
}
}, [queueState.activeThreads]);
}


useEffect(() => {
const threadIds = Object.keys(deleteQueue.activeThreadIds || {});
if (threadIds.length) processQueue("delete", threadIds);
}, [deleteQueue]);
function useInitializeQueues() {
const queueState = useAtomValue(queueAtom);

useEffect(() => {
const threadIds = Object.keys(markReadQueue.activeThreadIds || {});
if (threadIds.length) processQueue("markRead", threadIds);
}, [markReadQueue]);
if (!isInitialized) {
isInitialized = true;
if (queueState.activeThreads) {
processQueue({ threads: queueState.activeThreads });
}
}
}, [queueState.activeThreads]);
Comment on lines +13 to +19
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Ensure the effect responds to changes in queueState.activeThreads

The current useEffect depends on [queueState.activeThreads], but due to the isInitialized check, the effect runs only once. If queueState.activeThreads changes after the initial render, the effect will not run again, potentially missing updates.

If the intention is to process the queue whenever queueState.activeThreads changes, remove the isInitialized check.

Apply this diff to adjust the logic:

  useEffect(() => {
-   if (!isInitialized.current) {
-     isInitialized.current = true;
      if (queueState.activeThreads) {
        processQueue({ threads: queueState.activeThreads });
      }
-   }
  }, [queueState.activeThreads]);

Alternatively, if the effect should run only once, remove queueState.activeThreads from the dependency array:

  useEffect(() => {
    if (!isInitialized.current) {
      isInitialized.current = true;
      if (queueState.activeThreads) {
        processQueue({ threads: queueState.activeThreads });
      }
    }
- }, [queueState.activeThreads]);
+ }, []);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!isInitialized) {
isInitialized = true;
if (queueState.activeThreads) {
processQueue({ threads: queueState.activeThreads });
}
}
}, [queueState.activeThreads]);
useEffect(() => {
if (queueState.activeThreads) {
processQueue({ threads: queueState.activeThreads });
}
}, [queueState.activeThreads]);

}

export function QueueInitializer() {
Expand Down
155 changes: 99 additions & 56 deletions apps/web/store/archive-queue.ts
Original file line number Diff line number Diff line change
@@ -1,103 +1,146 @@
import { atomWithStorage, createJSONStorage } from "jotai/utils";
import pRetry from "p-retry";
import { jotaiStore } from "@/store";
import { emailActionQueue } from "@/utils/queue/email-action-queue";
import {
archiveThreadAction,
trashThreadAction,
markReadThreadAction,
} from "@/utils/actions/mail";
import { isActionError, ServerActionResponse } from "@/utils/error";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use 'import type' for type-only imports to optimize build output

The ServerActionResponse type imported from @/utils/error is only used as a type. Importing it with import type ensures that it is removed during transpilation, reducing bundle size and improving performance.

Apply this diff to adjust the import:

-import { isActionError, ServerActionResponse } from "@/utils/error";
+import { isActionError } from "@/utils/error";
+import type { ServerActionResponse } from "@/utils/error";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { isActionError, ServerActionResponse } from "@/utils/error";
import { isActionError } from "@/utils/error";
import type { ServerActionResponse } from "@/utils/error";
🧰 Tools
🪛 Biome

[error] 9-10: Some named imports are only used as types.

This import is only used as a type.

Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.

(lint/style/useImportType)

import { exponentialBackoff, sleep } from "@/utils/sleep";

type QueueType = "archive" | "delete" | "markRead";
type ActionType = "archive" | "delete" | "markRead";

type QueueItem = {
threadId: string;
actionType: ActionType;
labelId?: string;
};

type QueueState = {
activeThreadIds: Record<string, boolean>;
activeThreads: Record<`${ActionType}-${string}`, QueueItem>;
totalThreads: number;
};

function getInitialState(): QueueState {
return { activeThreadIds: {}, totalThreads: 0 };
}

// some users were somehow getting null for activeThreadIds, this should fix it
// some users were somehow getting null for activeThreads, this should fix it
const createStorage = () => {
if (typeof window === "undefined") return;
const storage = createJSONStorage<QueueState>(() => localStorage);
return {
...storage,
getItem: (key: string, initialValue: QueueState) => {
const storedValue = storage.getItem(key, initialValue);
return {
activeThreadIds: storedValue.activeThreadIds || {},
activeThreads: storedValue.activeThreads || {},
totalThreads: storedValue.totalThreads || 0,
};
},
};
};

// Create atoms with localStorage persistence for each queue type
export const queueAtoms = {
archive: atomWithStorage("archiveQueue", getInitialState(), createStorage()),
delete: atomWithStorage("deleteQueue", getInitialState(), createStorage()),
markRead: atomWithStorage(
"markReadQueue",
getInitialState(),
createStorage(),
),
};
// Create atoms with localStorage persistence
export const queueAtom = atomWithStorage(
"gmailActionQueue",
{ activeThreads: {}, totalThreads: 0 },
createStorage(),
{ getOnInit: true },
);

type ActionFunction = (threadId: string, ...args: any[]) => Promise<any>;
type ActionFunction = (
threadId: string,
labelId?: string,
) => Promise<ServerActionResponse<{}>>;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid using '{}' as a type; specify the correct type or use 'void'

Using {} as a type is discouraged because it represents "any non-nullable value," which can be misleading. If the function does not return any meaningful data, consider using void or defining the actual return type explicitly to ensure type safety.

Apply this diff to fix the type:

-) => Promise<ServerActionResponse<{}>>;
+) => Promise<ServerActionResponse<void>>;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
) => Promise<ServerActionResponse<{}>>;
) => Promise<ServerActionResponse<void>>;
🧰 Tools
🪛 Biome

[error] 53-53: Don't use '{}' as a type.

Prefer explicitly define the object shape. '{}' means "any non-nullable value".

(lint/complexity/noBannedTypes)


const actionMap: Record<QueueType, ActionFunction> = {
archive: archiveThreadAction,
const actionMap: Record<ActionType, ActionFunction> = {
archive: (threadId: string, labelId?: string) =>
archiveThreadAction(threadId, labelId),
delete: trashThreadAction,
markRead: (threadId: string) => markReadThreadAction(threadId, true),
};

export const addThreadsToQueue = (
queueType: QueueType,
threadIds: string[],
refetch?: () => void,
) => {
const queueAtom = queueAtoms[queueType];
export const addThreadsToQueue = ({
actionType,
threadIds,
labelId,
refetch,
}: {
actionType: ActionType;
threadIds: string[];
labelId?: string;
refetch?: () => void;
}) => {
const threads = Object.fromEntries(
threadIds.map((threadId) => [
`${actionType}-${threadId}`,
{ threadId, actionType, labelId },
]),
);

jotaiStore.set(queueAtom, (prev) => ({
activeThreadIds: {
...prev.activeThreadIds,
...Object.fromEntries(threadIds.map((id) => [id, true])),
activeThreads: {
...prev.activeThreads,
...threads,
},
totalThreads: prev.totalThreads + threadIds.length,
totalThreads: prev.totalThreads + Object.keys(threads).length,
}));

processQueue(queueType, threadIds, refetch);
processQueue({ threads, refetch });
};

export function processQueue(
queueType: QueueType,
threadIds: string[],
refetch?: () => void,
) {
const queueAtom = queueAtoms[queueType];
const action = actionMap[queueType];

export function processQueue({
threads,
refetch,
}: {
threads: Record<string, QueueItem>;
refetch?: () => void;
}) {
emailActionQueue.addAll(
threadIds.map((threadId) => async () => {
await action(threadId);

// remove completed thread from activeThreadIds
jotaiStore.set(queueAtom, (prev) => {
const { [threadId]: _, ...remainingThreads } = prev.activeThreadIds;
return {
...prev,
activeThreadIds: remainingThreads,
};
});

refetch?.();
}),
Object.entries(threads).map(
([_key, { threadId, actionType, labelId }]) =>
async () => {
await pRetry(
async (attemptCount) => {
console.log(
`Queue: ${actionType}. Processing ${threadId}` +
(attemptCount > 1 ? ` (attempt ${attemptCount})` : ""),
Comment on lines +105 to +106
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use a single template literal instead of string concatenation

Combining the template literals into a single expression enhances readability and avoids unnecessary string concatenation.

Apply this diff to fix the issue:

-`Queue: ${actionType}. Processing ${threadId}` +
-  (attemptCount > 1 ? ` (attempt ${attemptCount})` : ""),
+`Queue: ${actionType}. Processing ${threadId}${attemptCount > 1 ? ` (attempt ${attemptCount})` : ""}`,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
`Queue: ${actionType}. Processing ${threadId}` +
(attemptCount > 1 ? ` (attempt ${attemptCount})` : ""),
`Queue: ${actionType}. Processing ${threadId}${attemptCount > 1 ? ` (attempt ${attemptCount})` : ""}`,
🧰 Tools
🪛 Biome

[error] 105-106: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

);

const result = await actionMap[actionType](threadId, labelId);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure correct function signatures for action functions

The labelId parameter is being passed to all action functions in actionMap, but trashThreadAction and markReadThreadAction may not accept a labelId parameter. This could lead to unexpected behavior or runtime errors. Please ensure that labelId is only passed to functions that accept it.

Apply this diff to conditionally pass labelId only when needed:

-const result = await actionMap[actionType](threadId, labelId);
+const actionFn = actionMap[actionType];
+const result =
+  actionType === "archive"
+    ? await actionFn(threadId, labelId)
+    : await actionFn(threadId);

Alternatively, adjust the actionMap to ensure all functions have consistent signatures, or refactor the code to handle this appropriately.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const result = await actionMap[actionType](threadId, labelId);
const actionFn = actionMap[actionType];
const result =
actionType === "archive"
? await actionFn(threadId, labelId)
: await actionFn(threadId);


// when Gmail API returns a rate limit error, throw an error so it can be retried
if (isActionError(result)) {
await sleep(exponentialBackoff(attemptCount, 1_000));
throw new Error(result.error);
}
refetch?.();
},
{ retries: 3 },
);

// remove completed thread from activeThreads
jotaiStore.set(queueAtom, (prev) => {
const remainingThreads = Object.fromEntries(
Object.entries(prev.activeThreads).filter(
([_key, value]) =>
!(
value.threadId === threadId &&
value.actionType === actionType
),
),
);

return {
...prev,
activeThreads: remainingThreads,
};
});
Comment on lines +122 to +137
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Optimize removal of completed threads from 'activeThreads' using the key directly

Currently, removing completed threads involves filtering over all entries, which can be inefficient. Since keys are constructed as ${actionType}-${threadId}, you can directly remove the specific entry using object destructuring, improving performance and code clarity.

Apply this diff to optimize the removal:

-jotaiStore.set(queueAtom, (prev) => {
-  const remainingThreads = Object.fromEntries(
-    Object.entries(prev.activeThreads).filter(
-      ([_key, value]) =>
-        !(
-          value.threadId === threadId &&
-          value.actionType === actionType
-        ),
-    ),
-  );
-  return {
-    ...prev,
-    activeThreads: remainingThreads,
-  };
-});
+jotaiStore.set(queueAtom, (prev) => {
+  const { [`${actionType}-${threadId}`]: _, ...remainingThreads } = prev.activeThreads;
+  return {
+    ...prev,
+    activeThreads: remainingThreads,
+  };
+});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
jotaiStore.set(queueAtom, (prev) => {
const remainingThreads = Object.fromEntries(
Object.entries(prev.activeThreads).filter(
([_key, value]) =>
!(
value.threadId === threadId &&
value.actionType === actionType
),
),
);
return {
...prev,
activeThreads: remainingThreads,
};
});
jotaiStore.set(queueAtom, (prev) => {
const { [`${actionType}-${threadId}`]: _, ...remainingThreads } = prev.activeThreads;
return {
...prev,
activeThreads: remainingThreads,
};
});

},
),
);
}

export const resetTotalThreads = (queueType: QueueType) => {
const queueAtom = queueAtoms[queueType];
export const resetTotalThreads = () => {
jotaiStore.set(queueAtom, (prev) => ({
...prev,
totalThreads: 0,
Expand Down
3 changes: 2 additions & 1 deletion apps/web/utils/actions/mail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const isStatusOk = (status: number) => status >= 200 && status < 300;

export const archiveThreadAction = withActionInstrumentation(
"archiveThread",
async (threadId: string) => {
async (threadId: string, labelId?: string) => {
const { gmail, user, error } = await getSessionAndGmailClient();
if (error) return { error };
if (!gmail) return { error: "Could not load Gmail" };
Expand All @@ -35,6 +35,7 @@ export const archiveThreadAction = withActionInstrumentation(
threadId,
ownerEmail: user.email,
actionSource: "user",
labelId,
});

if (!isStatusOk(res.status)) return { error: "Failed to archive thread" };
Expand Down
12 changes: 9 additions & 3 deletions apps/web/utils/gmail/label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,25 @@ export async function labelThread(options: {
});
}

export async function archiveThread(options: {
export async function archiveThread({
gmail,
threadId,
ownerEmail,
actionSource,
labelId,
}: {
gmail: gmail_v1.Gmail;
threadId: string;
ownerEmail: string;
actionSource: TinybirdEmailAction["actionSource"];
labelId?: string;
}) {
const { gmail, threadId, ownerEmail, actionSource } = options;

const archivePromise = gmail.users.threads.modify({
userId: "me",
id: threadId,
requestBody: {
removeLabelIds: [INBOX_LABEL_ID],
...(labelId ? { addLabelIds: [labelId] } : {}),
},
});

Expand Down
2 changes: 1 addition & 1 deletion apps/web/utils/queue/email-action-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
import PQueue from "p-queue";

// Avoid overwhelming Gmail API
export const emailActionQueue = new PQueue({ concurrency: 3 });
export const emailActionQueue = new PQueue({ concurrency: 1 });
Loading