Skip to content
Closed
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
78 changes: 78 additions & 0 deletions apps/mail/components/mail/empty-folder-button.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { useState, useCallback } from 'react';
import { toast } from 'sonner';
import { Button } from '@/components/ui/button';
import { Trash } from '@/components/icons/icons';
import { trpcClient } from '@/providers/query-provider';
import { useOptimisticActions } from '@/hooks/use-optimistic-actions';

interface Props {
folder: string;
}

// A small utility component to permanently delete all emails in the currently opened folder (spam / bin)
export default function EmptyFolderButton({ folder }: Props) {
const { optimisticDeleteThreads, optimisticMoveThreadsTo } = useOptimisticActions();
const [isLoading, setIsLoading] = useState(false);

const handleEmptyFolder = useCallback(async () => {
if (isLoading) return;

const confirmText =
folder === 'spam'
? 'This will delete all emails in the Spam folder and move them to Bin. Continue?'
: 'This will permanently delete all emails in the Bin folder. Continue?';

if (!window.confirm(confirmText)) return;

setIsLoading(true);
Comment on lines +17 to +27
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation and rate limiting.

The function lacks folder validation and protection against rapid successive clicks.

Add proper validation and debouncing:

  const handleEmptyFolder = useCallback(async () => {
-   if (isLoading) return;
+   if (isLoading || !['spam', 'bin'].includes(folder)) return;

    const confirmText =
      folder === 'spam'
        ? 'This will delete all emails in the Spam folder and move them to Bin. Continue?'
        : 'This will permanently delete all emails in the Bin folder. Continue?';
📝 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 handleEmptyFolder = useCallback(async () => {
if (isLoading) return;
const confirmText =
folder === 'spam'
? 'This will delete all emails in the Spam folder and move them to Bin. Continue?'
: 'This will permanently delete all emails in the Bin folder. Continue?';
if (!window.confirm(confirmText)) return;
setIsLoading(true);
const handleEmptyFolder = useCallback(async () => {
if (isLoading || !['spam', 'bin'].includes(folder)) return;
const confirmText =
folder === 'spam'
? 'This will delete all emails in the Spam folder and move them to Bin. Continue?'
: 'This will permanently delete all emails in the Bin folder. Continue?';
if (!window.confirm(confirmText)) return;
setIsLoading(true);
🤖 Prompt for AI Agents
In apps/mail/components/mail/empty-folder-button.tsx around lines 17 to 27, the
handleEmptyFolder function lacks validation for the folder value and does not
prevent rapid successive clicks. Add validation to ensure the folder variable
only accepts expected values like 'spam' or 'bin'. Implement debouncing or a
rate-limiting mechanism to ignore or delay repeated clicks while the function is
already processing, preventing multiple simultaneous executions.

try {
const ids: string[] = [];
let cursor = '';
const MAX_PER_PAGE = 100;
while (true) {
// Fetch mails in pages of 100 until there is no nextPageToken
const page = await trpcClient.mail.listThreads.query({
folder,
q: '',
max: MAX_PER_PAGE,
cursor,
} as any);
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove unsafe type assertion.

The as any cast bypasses TypeScript's type safety and could lead to runtime errors if the API contract changes.

Define a proper type for the query parameters:

+interface ListThreadsParams {
+  folder: string;
+  q: string;
+  max: number;
+  cursor: string;
+}

-        const page = await trpcClient.mail.listThreads.query({
-          folder,
-          q: '',
-          max: MAX_PER_PAGE,
-          cursor,
-        } as any);
+        const page = await trpcClient.mail.listThreads.query({
+          folder,
+          q: '',
+          max: MAX_PER_PAGE,
+          cursor,
+        } satisfies ListThreadsParams);
📝 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
} as any);
// Define a proper parameter type
interface ListThreadsParams {
folder: string;
q: string;
max: number;
cursor: string;
}
// …inside your async function…
- const page = await trpcClient.mail.listThreads.query({
- folder,
- q: '',
- max: MAX_PER_PAGE,
- cursor,
- } as any);
+ const page = await trpcClient.mail.listThreads.query({
+ folder,
+ q: '',
+ max: MAX_PER_PAGE,
+ cursor,
+ } satisfies ListThreadsParams);
🤖 Prompt for AI Agents
In apps/mail/components/mail/empty-folder-button.tsx at line 39, remove the
unsafe 'as any' type assertion used for query parameters. Instead, define and
use a proper TypeScript type or interface that accurately represents the
expected shape of the query parameters to ensure type safety and prevent
potential runtime errors.

if (page?.threads?.length) {
ids.push(...page.threads.map((t: { id: string }) => t.id));
}
if (!page?.nextPageToken) break;
cursor = page.nextPageToken;
}
Comment on lines +32 to +45
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add safety mechanism for pagination loop.

The while loop could theoretically run indefinitely if the backend returns inconsistent pagination data, leading to an infinite loop.

Add a safety counter to prevent infinite loops:

      const ids: string[] = [];
      let cursor = '';
      const MAX_PER_PAGE = 100;
+     const MAX_PAGES = 1000; // Safety limit
+     let pageCount = 0;
      while (true) {
+       if (++pageCount > MAX_PAGES) {
+         console.warn('Maximum page limit reached, stopping pagination');
+         break;
+       }
        // Fetch mails in pages of 100 until there is no nextPageToken
📝 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
while (true) {
// Fetch mails in pages of 100 until there is no nextPageToken
const page = await trpcClient.mail.listThreads.query({
folder,
q: '',
max: MAX_PER_PAGE,
cursor,
} as any);
if (page?.threads?.length) {
ids.push(...page.threads.map((t: { id: string }) => t.id));
}
if (!page?.nextPageToken) break;
cursor = page.nextPageToken;
}
const ids: string[] = [];
let cursor = '';
const MAX_PER_PAGE = 100;
+ const MAX_PAGES = 1000; // Safety limit
+ let pageCount = 0;
while (true) {
+ if (++pageCount > MAX_PAGES) {
+ console.warn('Maximum page limit reached, stopping pagination');
+ break;
+ }
// Fetch mails in pages of 100 until there is no nextPageToken
const page = await trpcClient.mail.listThreads.query({
folder,
q: '',
max: MAX_PER_PAGE,
cursor,
} as any);
if (page?.threads?.length) {
ids.push(...page.threads.map((t: { id: string }) => t.id));
}
if (!page?.nextPageToken) break;
cursor = page.nextPageToken;
}
🤖 Prompt for AI Agents
In apps/mail/components/mail/empty-folder-button.tsx around lines 32 to 45, the
while loop fetching paginated mail threads lacks a safety mechanism and could
run indefinitely if pagination data is inconsistent. Add a counter variable
initialized before the loop and increment it each iteration; break the loop if
the counter exceeds a reasonable maximum (e.g., 100) to prevent infinite
looping.


if (!ids.length) {
toast.success('Folder already empty');
return;
}

if (folder === 'spam') {
// Move all spam to bin
optimisticMoveThreadsTo(ids, folder, 'bin');
} else {
// Permanently delete everything from bin
optimisticDeleteThreads(ids, folder);
}
} catch (error: any) {
console.error('Failed to empty folder', error);
toast.error(error?.message ?? 'Failed to empty folder');
} finally {
setIsLoading(false);
}
}, [folder, isLoading, optimisticDeleteThreads, optimisticMoveThreadsTo]);

return (
<Button
size="sm"
variant="destructive"
disabled={isLoading}
onClick={handleEmptyFolder}
className="px-4 flex items-center justify-center"
>
{folder === 'spam' ? 'Empty Spam' : 'Empty Bin'}
</Button>
);
}
21 changes: 21 additions & 0 deletions apps/mail/components/mail/mail-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import { useQueryState } from 'nuqs';
import { Categories } from './mail';
import { useAtom } from 'jotai';
import { useThreadNotes } from '@/hooks/use-notes';
import { Checkbox } from '@/components/ui/checkbox';

const Thread = memo(
function Thread({
Expand Down Expand Up @@ -420,6 +421,26 @@ const Thread = memo(
</div>

<div className="relative flex w-full items-center justify-between gap-4 px-4">
{/* Selection checkbox */}
<Checkbox
className="mr-2 h-4 w-4"
checked={isMailBulkSelected}
onClick={(e) => {
e.stopPropagation();
}}
onCheckedChange={() => {
if (!idToUse) return;
setMail((prev) => {
const already = prev.bulkSelected.includes(idToUse);
return {
...prev,
bulkSelected: already
? prev.bulkSelected.filter((id) => id !== idToUse)
: [...prev.bulkSelected, idToUse],
};
});
}}
/>
<div>
<Avatar
className={cn(
Expand Down
11 changes: 11 additions & 0 deletions apps/mail/components/mail/mail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import { useQueryState } from 'nuqs';
import { useAtom } from 'jotai';
import { toast } from 'sonner';
import SelectAllCheckbox from './select-all-checkbox';
import EmptyFolderButton from './empty-folder-button';

interface ITag {
id: string;
Expand Down Expand Up @@ -519,6 +520,16 @@ export function MailLayout() {
) : null}
</div>
<AutoLabelingSettings />
{/* Bulk action buttons when emails are selected */}
{mail.bulkSelected.length > 0 && (
<div className="dark:bg-iconDark/20 relative ml-2 rounded-full" aria-hidden>
<BulkSelectActions />
</div>
)}
{/* Empty folder actions for Spam & Bin */}
{['spam', 'bin'].includes(folder) && (
<EmptyFolderButton folder={folder} />
)}
<div className="dark:bg-iconDark/20 relative ml-2 h-3 w-0.5 rounded-full bg-[#E7E7E7]" />{' '}
<Button
onClick={() => {
Expand Down
2 changes: 1 addition & 1 deletion docker-compose.db.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,4 @@ services:

volumes:
valkey-data:
postgres-data:
postgres-data:
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add missing newline at end of file.

The YAMLlint tool correctly identified that the file should end with a newline character for POSIX compliance and better Git handling.

Apply this diff to fix the formatting issue:

-  postgres-data:
+  postgres-data:

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 39-39: no new line character at the end of file

(new-line-at-end-of-file)

🤖 Prompt for AI Agents
In docker-compose.db.yaml at line 39, the file is missing a newline character at
the end. Add a single newline character after the last line to ensure POSIX
compliance and proper handling by Git and YAMLlint.