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
16 changes: 8 additions & 8 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
"dependencies": {
"@internxt/css-config": "^1.1.0",
"@internxt/lib": "^1.4.1",
"@internxt/sdk": "^1.15.7",
"@internxt/ui": "^0.1.11",
"@internxt/sdk": "^1.15.8",
"@internxt/ui": "^0.1.12",
"@phosphor-icons/react": "^2.1.10",
"@reduxjs/toolkit": "^2.11.2",
"@tailwindcss/vite": "^4.2.1",
Expand Down
6 changes: 3 additions & 3 deletions src/features/mail/MailView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
const { translate } = useTranslationContext();
const [activeMailId, setActiveMailId] = useState<string | undefined>(undefined);

const { isLoadingListFolder, listFolder, hasMore: hasMoreItems, onLoadMore } = useListFolderPaginated(folder);
const { isLoadingListFolder, listFolderEmails, hasMoreEmails, onLoadMore } = useListFolderPaginated(folder);
const { data: activeMail } = useGetMailMessageQuery({ emailId: activeMailId! }, { skip: !activeMailId });
const [markAsRead] = useMarkAsReadMutation();

Expand Down Expand Up @@ -49,12 +49,12 @@
{/* Tray */}
<TrayList
folderName={folderName}
listFolder={listFolder?.emails}
listFolder={listFolderEmails}
isLoadingListFolder={isLoadingListFolder}
activeMailId={activeMailId}
onMailSelected={onSelectEmail}
loadMore={onLoadMore}
hasMoreItems={hasMoreItems}
hasMoreItems={hasMoreEmails}
/>
{/* Mail Preview */}
<div className="flex flex-col w-full">
Expand All @@ -70,7 +70,7 @@
mail={{
subject: activeMail.subject,
receivedAt: DateService.formatWithTime(activeMail.receivedAt),
htmlBody: (activeMail.htmlBody as string | null) ?? '',

Check warning on line 73 in src/features/mail/MailView.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=internxt_mail-web&issues=AZ1oJwr1RzR8NIC6Hpm-&open=AZ1oJwr1RzR8NIC6Hpm-&pullRequest=35
}}
/>
) : null}
Expand Down
18 changes: 9 additions & 9 deletions src/hooks/mail/useListFolderPaginated.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ describe('List Folder Paginated - custom hook', () => {
waitFor(() => {
expect(result.current.isLoadingListFolder).toBe(true);

expect(result.current.listFolder?.emails).toHaveLength(DEFAULT_FOLDER_LIMIT);
expect(result.current.hasMore).toBeTruthy();
expect(result.current.listFolderEmails).toHaveLength(DEFAULT_FOLDER_LIMIT);
expect(result.current.hasMoreEmails).toBeTruthy();
});
Comment thread
xabg2 marked this conversation as resolved.
});

Expand All @@ -46,7 +46,7 @@ describe('List Folder Paginated - custom hook', () => {
wrapper: createWrapper(store),
});

expect(result.current.hasMore).toBeFalsy();
expect(result.current.hasMoreEmails).toBeFalsy();
});

test('When the user scrolls to the end of the list, then the next batch of emails is loaded and appended', async () => {
Expand All @@ -67,9 +67,9 @@ describe('List Folder Paginated - custom hook', () => {
});

waitFor(() => {
expect(result.current.listFolder?.emails).toHaveLength(DEFAULT_FOLDER_LIMIT * 2);
expect(result.current.listFolder?.emails).toStrictEqual([...page1.emails, ...page2.emails]);
expect(result.current.hasMore).toBeFalsy();
expect(result.current.listFolderEmails).toHaveLength(DEFAULT_FOLDER_LIMIT * 2);
expect(result.current.listFolderEmails).toStrictEqual([...page1.emails, ...page2.emails]);
expect(result.current.hasMoreEmails).toBeFalsy();
});
});

Expand Down Expand Up @@ -105,14 +105,14 @@ describe('List Folder Paginated - custom hook', () => {
});

waitFor(() => {
expect(result.current.listFolder?.emails).toStrictEqual(inboxEmails.emails);
expect(result.current.listFolderEmails).toStrictEqual(inboxEmails.emails);
});

rerender({ mailbox: 'sent' });

waitFor(() => {
expect(result.current.listFolder?.emails).toStrictEqual(sentEmails.emails);
expect(result.current.listFolder?.emails).toHaveLength(DEFAULT_FOLDER_LIMIT);
expect(result.current.listFolderEmails).toStrictEqual(sentEmails.emails);
expect(result.current.listFolderEmails).toHaveLength(DEFAULT_FOLDER_LIMIT);
});
});
});
22 changes: 8 additions & 14 deletions src/hooks/mail/useListFolderPaginated.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
/* eslint-disable react-hooks/set-state-in-effect */
import { DEFAULT_FOLDER_LIMIT } from '@/constants';
import { useGetListFolderQuery } from '@/store/api/mail';
import type { FolderType } from '@/types/mail';
import { useEffect, useState } from 'react';
import { useState } from 'react';

const useListFolderPaginated = (mailbox: FolderType) => {
const [position, setPosition] = useState(0);

useEffect(() => {
setPosition(0);
}, [mailbox]);
const [anchorId, setAnchorId] = useState<string | undefined>(undefined);
Comment thread
xabg2 marked this conversation as resolved.

const {
data: listFolder,
Expand All @@ -19,26 +14,25 @@ const useListFolderPaginated = (mailbox: FolderType) => {
{
mailbox,
limit: DEFAULT_FOLDER_LIMIT,
position,
anchorId,
},
{
pollingInterval: 30000,
skip: !mailbox,
},
);

const hasMore = (listFolder?.emails.length ?? 0) < (listFolder?.total ?? 0);

const onLoadMore = () => {
if (isFetching || !hasMore) return;
setPosition((prev) => prev + DEFAULT_FOLDER_LIMIT);
if (isFetching || !listFolder?.hasMoreMails) return;

setAnchorId(listFolder?.nextAnchor);
};
Comment thread
xabg2 marked this conversation as resolved.

return {
listFolder,
listFolderEmails: listFolder?.emails,
isLoadingListFolder,
onLoadMore,
hasMore,
hasMoreEmails: listFolder?.hasMoreMails,
};
};

Expand Down
15 changes: 4 additions & 11 deletions src/services/sdk/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ import type { ApiSecurity, AppDetails } from '@internxt/sdk/dist/shared';
import packageJson from '../../../package.json';
import { ConfigService } from '../config';
import { LocalStorageService } from '../local-storage';
import { AuthService } from './auth';
import { NavigationService } from '../navigation';
import { AppView } from '@/routes/paths';
import { store } from '@/store';
import { logoutThunk } from '@/store/slices/user/thunks';

export type SdkManagerApiSecurity = ApiSecurity & { newToken: string };

Expand Down Expand Up @@ -33,14 +32,8 @@ export class SdkManager {
private getNewTokenApiSecurity(): ApiSecurity {
return {
token: LocalStorageService.instance?.getToken() ?? '',
unauthorizedCallback: () => {
if (LocalStorageService.instance) {
LocalStorageService.instance.clearCredentials();
AuthService.instance.logOut().catch((error) => {
console.error(error);
});
NavigationService.instance.navigate({ id: AppView.Welcome });
}
unauthorizedCallback: async () => {
await store.dispatch(logoutThunk());
},
};
}
Expand Down
16 changes: 11 additions & 5 deletions src/services/sdk/sdk.service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@ import { beforeEach, describe, expect, test, vi, afterEach } from 'vitest';
import { SdkManager } from '.';
import { ConfigService } from '../config';
import { LocalStorageService } from '../local-storage';
import { AuthService } from './auth';
import { NavigationService } from '../navigation';
import { store } from '@/store';

vi.mock('@/store', () => ({
store: {
dispatch: vi.fn(),
},
}));

vi.mock('./auth', () => ({
AuthService: {
Expand Down Expand Up @@ -202,7 +208,7 @@ describe('SDK Manager', () => {
const securityArg = (Drive.Users.client as any).mock.calls[0][2];
securityArg.unauthorizedCallback();

expect(AuthService.instance.logOut).toHaveBeenCalled();
expect(store.dispatch).toHaveBeenCalled();
});
});

Expand Down Expand Up @@ -232,7 +238,7 @@ describe('SDK Manager', () => {
const securityArg = (Drive.Storage.client as any).mock.calls[0][2];
securityArg.unauthorizedCallback();

expect(AuthService.instance.logOut).toHaveBeenCalled();
expect(store.dispatch).toHaveBeenCalled();
});
});

Expand Down Expand Up @@ -262,7 +268,7 @@ describe('SDK Manager', () => {
const securityArg = (Drive.Payments.client as any).mock.calls[0][2];
securityArg.unauthorizedCallback();

expect(AuthService.instance.logOut).toHaveBeenCalled();
expect(store.dispatch).toHaveBeenCalled();
});
});

Expand Down Expand Up @@ -292,7 +298,7 @@ describe('SDK Manager', () => {
const securityArg = (Mail.client as any).mock.calls[0][2];
securityArg.unauthorizedCallback();

expect(AuthService.instance.logOut).toHaveBeenCalled();
expect(store.dispatch).toHaveBeenCalled();
});
});
});
15 changes: 7 additions & 8 deletions src/store/api/mail/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,17 @@ export const mailApi = api.injectEndpoints({
getListFolder: builder.query<EmailListResponse, ListEmailsQuery>({
serializeQueryArgs: ({ queryArgs }) => ({ mailbox: queryArgs?.mailbox }),
merge: (currentCache, newItems, { arg }) => {
const currentPosition = arg?.position ?? 0;

// This prevents the concatenation of the existent cached emails with the new ones (repeated emails)
if (currentPosition === 0) {
currentCache.emails = newItems.emails;
} else {
// No anchorId means first page — replace instead of accumulate
if (arg?.anchorId) {
currentCache.emails.push(...newItems.emails);
} else {
currentCache.emails = newItems.emails;
}
currentCache.total = newItems.total;
currentCache.hasMoreMails = newItems.hasMoreMails;
currentCache.nextAnchor = newItems.nextAnchor;
},
forceRefetch: ({ currentArg, previousArg }) =>
currentArg?.mailbox !== previousArg?.mailbox || currentArg?.position !== previousArg?.position,
currentArg?.mailbox !== previousArg?.mailbox || currentArg?.anchorId !== previousArg?.anchorId,
async queryFn(query) {
try {
const mailList = await MailService.instance.listFolder(query);
Expand Down
6 changes: 3 additions & 3 deletions src/store/api/mail/mail.api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ describe('Mail Query', () => {
const store = createTestStore();
await store.dispatch(mailApi.endpoints.getListFolder.initiate(query as ListEmailsQuery));
await store.dispatch(
mailApi.endpoints.getListFolder.initiate({ ...query, position: DEFAULT_FOLDER_LIMIT } as ListEmailsQuery),
mailApi.endpoints.getListFolder.initiate({ ...query, anchorId: 'anchor-page-2' } as ListEmailsQuery),
);

const state = store.getState() as unknown as RootState;
Expand All @@ -111,7 +111,7 @@ describe('Mail Query', () => {
const cache = mailApi.endpoints.getListFolder.select(query as ListEmailsQuery)(state);

expect(cache.data?.emails).toHaveLength(DEFAULT_FOLDER_LIMIT);
expect(cache.data?.emails).toEqual(reload.emails);
expect(cache.data?.emails).toStrictEqual(reload.emails);
});
});

Expand Down Expand Up @@ -141,7 +141,7 @@ describe('Mail Query', () => {
});

describe('Mark Mail As Read', () => {
const mailboxQuery = { mailbox: 'inbox', limit: DEFAULT_FOLDER_LIMIT, position: 0 } as ListEmailsQuery;
const mailboxQuery = { mailbox: 'inbox', limit: DEFAULT_FOLDER_LIMIT } as ListEmailsQuery;

const setupOptimisticStore = async () => {
const mockedMails = getMockedMails();
Expand Down
4 changes: 4 additions & 0 deletions src/test-utils/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ const createEmailAddress = () => ({

export const getMockedMail = (): EmailResponse => ({
id: faker.string.uuid(),
mailboxIds: [faker.string.alphanumeric(1), faker.string.alphanumeric(1)],
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use realistic mailbox IDs in fixtures.

Line 147 generates 1-char mailbox IDs, but mailbox fixtures use UUIDs. This breaks referential realism and can hide mailbox-linking bugs in tests.

Proposed fix
-  mailboxIds: [faker.string.alphanumeric(1), faker.string.alphanumeric(1)],
+  mailboxIds: [faker.string.uuid(), faker.string.uuid()],
📝 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
mailboxIds: [faker.string.alphanumeric(1), faker.string.alphanumeric(1)],
mailboxIds: [faker.string.uuid(), faker.string.uuid()],
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/test-utils/fixtures.ts` at line 147, The mailboxIds fixture is generating
1-character alphanumeric IDs which don't match the UUIDs used elsewhere and can
mask linkage bugs; update the mailboxIds array in src/test-utils/fixtures.ts
(the mailboxIds fixture) to generate realistic UUIDs (use your project's faker
UUID helper, e.g., faker.string.uuid() or faker.datatype.uuid() depending on
your faker version) for both entries so the fixtures mirror production mailbox
IDs.

threadId: faker.string.uuid(),
from: [createEmailAddress()],
to: [createEmailAddress()],
Expand All @@ -167,6 +168,7 @@ export const getMockedMails = (count = 3): EmailListResponse => ({
const mail = getMockedMail();
return {
id: mail.id,
mailboxIds: mail.mailboxIds,
threadId: mail.threadId,
from: mail.from,
to: mail.to,
Expand All @@ -180,6 +182,8 @@ export const getMockedMails = (count = 3): EmailListResponse => ({
};
}),
total: faker.number.int({ min: count, max: 500 }),
hasMoreMails: faker.datatype.boolean(),
nextAnchor: faker.string.uuid(),
Comment on lines +185 to +186
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Make pagination metadata deterministic and internally consistent.

Lines 185-186 use random values; this can introduce flaky tests. Also, nextAnchor should depend on hasMoreMails (no next page => no anchor).

Proposed refactor
-export const getMockedMails = (count = 3): EmailListResponse => ({
+export const getMockedMails = (
+  count = 3,
+  options: Partial<Pick<EmailListResponse, 'hasMoreMails' | 'nextAnchor'>> = {}
+): EmailListResponse => {
+  const hasMoreMails = options.hasMoreMails ?? false;
+  return {
   emails: Array.from({ length: count }, () => {
     const mail = getMockedMail();
     return {
       id: mail.id,
       mailboxIds: mail.mailboxIds,
       threadId: mail.threadId,
       from: mail.from,
       to: mail.to,
       subject: mail.subject,
       receivedAt: mail.receivedAt,
       preview: mail.preview,
       isRead: mail.isRead,
       isFlagged: mail.isFlagged,
       hasAttachment: mail.hasAttachment,
       size: mail.size,
     };
   }),
   total: faker.number.int({ min: count, max: 500 }),
-  hasMoreMails: faker.datatype.boolean(),
-  nextAnchor: faker.string.uuid(),
-});
+  hasMoreMails,
+  nextAnchor: hasMoreMails ? options.nextAnchor ?? faker.string.uuid() : '',
+  };
+};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/test-utils/fixtures.ts` around lines 185 - 186, The pagination fixture
currently uses random values for hasMoreMails and nextAnchor which causes flaky
tests; change the fixture so hasMoreMails is deterministic (e.g., a fixed
boolean or driven by an optional parameter) and compute nextAnchor from it: if
hasMoreMails is true generate/return a stable UUID, otherwise set nextAnchor to
null/undefined. Update the code that constructs the object with the hasMoreMails
and nextAnchor properties (references: hasMoreMails, nextAnchor) so nextAnchor
always reflects hasMoreMails.

});

export const getMockedMailBoxes = (): MailboxResponse[] => [
Expand Down
1 change: 1 addition & 0 deletions vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export default defineConfig({
alias: {
'@': path.resolve(__dirname, './src'),
},
preserveSymlinks: true,
},
optimizeDeps: {
include: ['@internxt/sdk'],
Expand Down
Loading