From 94e0cbb619f82d2aa14337152960104745bd6c64 Mon Sep 17 00:00:00 2001 From: Xavier Abad <77491413+xabg2@users.noreply.github.com> Date: Mon, 6 Apr 2026 11:34:25 +0200 Subject: [PATCH 1/3] feat: get all mails from all mailboxes EP --- src/modules/email/email.controller.ts | 55 ++++++++++++++++++- src/modules/email/email.dto.ts | 3 + src/modules/email/email.service.ts | 9 +++ src/modules/email/email.types.ts | 1 + src/modules/email/mail-provider.port.ts | 6 ++ .../infrastructure/jmap/jmap-mail.mapper.ts | 1 + .../infrastructure/jmap/jmap-mail.provider.ts | 54 +++++++++++++++++- 7 files changed, 125 insertions(+), 4 deletions(-) diff --git a/src/modules/email/email.controller.ts b/src/modules/email/email.controller.ts index 635e713..7cf48a3 100644 --- a/src/modules/email/email.controller.ts +++ b/src/modules/email/email.controller.ts @@ -50,8 +50,49 @@ export class EmailController { 'Returns every mailbox for the authenticated user, including folder counts.', }) @ApiOkResponse({ type: [MailboxResponseDto] }) - getMailboxes(@User('email') email: string) { - return this.emailService.getMailboxes(email); + getMailboxes() { + return this.emailService.getMailboxes(STUB_USER); + } + + @Get('/all') + @ApiOperation({ + summary: 'List emails', + description: 'Get all the emails no matter the mailbox.', + }) + @ApiQuery({ + name: 'limit', + required: false, + type: Number, + description: 'Maximum number of emails to return. Defaults to `20`.', + example: 20, + }) + @ApiQuery({ + name: 'position', + required: false, + type: Number, + description: 'Zero-based offset for pagination. Defaults to `0`.', + example: 0, + }) + @ApiQuery({ + name: 'anchorId', + required: false, + type: String, + description: 'Anchor ID for pagination.', + example: 'Ma1f09b…', + }) + @ApiOkResponse({ type: EmailListResponseDto }) + getAll( + @User('email') email: string, + @Query('limit') limit?: string, + @Query('position') position?: string, + @Query('anchorId') anchorId?: string, + ) { + return this.emailService.getAllEmails( + email, + limit ? Number(limit) || 20 : 20, + position ? Number(position) || 0 : 0, + anchorId, + ); } @Get() @@ -89,7 +130,6 @@ export class EmailController { }) @ApiOkResponse({ type: EmailListResponseDto }) list( - @User('email') email: string, @Query('mailbox') mailbox: MailboxType = 'inbox', @Query('limit') limit?: string, @Query('position') position?: string, @@ -97,6 +137,7 @@ export class EmailController { ) { return this.emailService.listEmails( email, + STUB_USER, mailbox, limit ? Number(limit) || 20 : 20, position ? Number(position) || 0 : 0, @@ -115,6 +156,7 @@ export class EmailController { @ApiNotFoundResponse({ description: 'Email not found' }) get(@User('email') email: string, @Param('id') id: string) { return this.emailService.getEmail(email, id); + return this.emailService.getEmail(STUB_USER, id); } @Post('send') @@ -131,6 +173,7 @@ export class EmailController { }) send(@User('email') email: string, @Body() dto: SendEmailRequestDto) { return this.emailService.sendEmail(email, dto); + return this.emailService.sendEmail(STUB_USER, dto); } @Post('drafts') @@ -146,6 +189,7 @@ export class EmailController { }) saveDraft(@User('email') email: string, @Body() dto: DraftEmailRequestDto) { return this.emailService.saveDraft(email, dto); + return this.emailService.saveDraft(STUB_USER, dto); } @Patch(':id') @@ -165,15 +209,19 @@ export class EmailController { @Param('id') id: string, @Body() body: UpdateEmailRequestDto, ) { + async update(@Param('id') id: string, @Body() body: UpdateEmailRequestDto) { const ops: Promise[] = []; if (body.mailbox !== undefined) { ops.push(this.emailService.moveEmail(email, id, body.mailbox)); + ops.push(this.emailService.moveEmail(STUB_USER, id, body.mailbox)); } if (body.isRead !== undefined) { ops.push(this.emailService.markAsRead(email, id, body.isRead)); + ops.push(this.emailService.markAsRead(STUB_USER, id, body.isRead)); } if (body.isFlagged !== undefined) { ops.push(this.emailService.markAsFlagged(email, id, body.isFlagged)); + ops.push(this.emailService.markAsFlagged(STUB_USER, id, body.isFlagged)); } await Promise.all(ops); } @@ -189,5 +237,6 @@ export class EmailController { @ApiNotFoundResponse({ description: 'Email not found' }) delete(@User('email') email: string, @Param('id') id: string) { return this.emailService.deleteEmail(email, id); + return this.emailService.deleteEmail(STUB_USER, id); } } diff --git a/src/modules/email/email.dto.ts b/src/modules/email/email.dto.ts index d1180c2..5932208 100644 --- a/src/modules/email/email.dto.ts +++ b/src/modules/email/email.dto.ts @@ -107,6 +107,9 @@ export class EmailSummaryResponseDto { @ApiProperty({ example: 'Ma1f09b…' }) id!: string; + @ApiProperty({ type: [String], example: ['d', 'a'] }) + mailboxIds!: string[]; + @ApiProperty({ example: 'T1a2b3c…' }) threadId!: string; diff --git a/src/modules/email/email.service.ts b/src/modules/email/email.service.ts index 2f716c7..281a77d 100644 --- a/src/modules/email/email.service.ts +++ b/src/modules/email/email.service.ts @@ -21,6 +21,15 @@ export class EmailService { return this.mail.getMailboxes(userEmail); } + getAllEmails( + userEmail: string, + limit: number, + position: number, + anchorId?: string, + ): Promise { + return this.mail.getAllEmails(userEmail, limit, position, anchorId); + } + listEmails( userEmail: string, mailbox: MailboxType, diff --git a/src/modules/email/email.types.ts b/src/modules/email/email.types.ts index 39f17d9..3deeee3 100644 --- a/src/modules/email/email.types.ts +++ b/src/modules/email/email.types.ts @@ -23,6 +23,7 @@ export interface Mailbox { export interface EmailSummary { id: string; threadId: string; + mailboxIds: string[]; from: EmailAddress[]; to: EmailAddress[]; subject: string; diff --git a/src/modules/email/mail-provider.port.ts b/src/modules/email/mail-provider.port.ts index 781bddd..1a70084 100644 --- a/src/modules/email/mail-provider.port.ts +++ b/src/modules/email/mail-provider.port.ts @@ -9,6 +9,12 @@ import type { export abstract class MailProvider { abstract getMailboxes(userEmail: string): Promise; + abstract getAllEmails( + userEmail: string, + limit: number, + position: number, + anchorId?: string, + ): Promise; abstract listEmails( userEmail: string, mailbox: MailboxType, diff --git a/src/modules/infrastructure/jmap/jmap-mail.mapper.ts b/src/modules/infrastructure/jmap/jmap-mail.mapper.ts index 47c6749..345d491 100644 --- a/src/modules/infrastructure/jmap/jmap-mail.mapper.ts +++ b/src/modules/infrastructure/jmap/jmap-mail.mapper.ts @@ -57,6 +57,7 @@ export function mapJmapEmailToSummary(e: JmapEmail): EmailSummary { return { id: e.id, threadId: e.threadId, + mailboxIds: Object.keys(e.mailboxIds), from: e.from ?? [], to: e.to ?? [], subject: e.subject ?? '', diff --git a/src/modules/infrastructure/jmap/jmap-mail.provider.ts b/src/modules/infrastructure/jmap/jmap-mail.provider.ts index 3943a9c..9917c0a 100644 --- a/src/modules/infrastructure/jmap/jmap-mail.provider.ts +++ b/src/modules/infrastructure/jmap/jmap-mail.provider.ts @@ -85,6 +85,58 @@ export class JmapMailProvider extends MailProvider { return jmapMailboxes.map(mapJmapMailbox); } + async getAllEmails( + userEmail: string, + limit: number, + position: number, + anchorId?: string, + ): Promise { + const accountId = await this.jmap.getPrimaryAccountId(userEmail); + + const queryParams: Record = { + accountId, + sort: [{ property: 'receivedAt', isAscending: false }], + limit, + calculateTotal: true, + }; + + if (anchorId) { + queryParams.anchor = anchorId; + queryParams.anchorOffset = 1; + } else { + queryParams.position = position; + } + + const response = await this.jmap.request(userEmail, [ + ['Email/query', queryParams, 'r0'], + [ + 'Email/get', + { + accountId, + '#ids': { resultOf: 'r0', name: 'Email/query', path: '/ids' }, + properties: EMAIL_LIST_PROPERTIES, + }, + 'r1', + ], + ]); + + const queryResult = response.methodResponses[0]![1] as JmapQueryResponse; + const getResult = response + .methodResponses[1]![1] as JmapGetResponse; + + const emails = getResult.list.map(mapJmapEmailToSummary); + + const nextAnchor = emails.length ? emails.at(-1)?.id : undefined; + const hasMoreEmails = emails.length >= limit; + + return { + emails, + total: queryResult.total ?? 0, + hasMoreMails: hasMoreEmails, + nextAnchor: hasMoreEmails ? nextAnchor : undefined, + }; + } + async listEmails( userEmail: string, mailbox: MailboxType, @@ -119,7 +171,7 @@ export class JmapMailProvider extends MailProvider { { accountId, '#ids': { resultOf: 'r0', name: 'Email/query', path: '/ids' }, - properties: EMAIL_LIST_PROPERTIES, + properties: [...EMAIL_LIST_PROPERTIES, 'mailboxIds'], }, 'r1', ], From f41718189a646ea740221044840731b1350819b6 Mon Sep 17 00:00:00 2001 From: jzunigax2 <125698953+jzunigax2@users.noreply.github.com> Date: Mon, 6 Apr 2026 10:40:11 -0600 Subject: [PATCH 2/3] refactor: streamline email and account handling - Removed DomainRepository export from AccountModule. - Updated AccountService to include a new method for listing active domains. - Refactored EmailController to use user email instead of a stubbed user for various email operations. - Enhanced JmapMailProvider to utilize a new queryEmails method for improved email fetching logic. - Added unit tests for new email retrieval functionality and adjusted existing tests for consistency. --- src/modules/account/account.module.ts | 2 +- src/modules/account/account.service.ts | 5 ++ src/modules/email/email.controller.spec.ts | 38 +++++++++ src/modules/email/email.controller.ts | 14 +--- src/modules/email/email.service.spec.ts | 62 ++++++++------ .../gateway/gateway.controller.spec.ts | 7 +- src/modules/gateway/gateway.controller.ts | 8 +- .../jmap/jmap-mail.provider.spec.ts | 82 +++++++++++++++++++ .../infrastructure/jmap/jmap-mail.provider.ts | 67 +++++---------- test/fixtures.ts | 1 + 10 files changed, 191 insertions(+), 95 deletions(-) diff --git a/src/modules/account/account.module.ts b/src/modules/account/account.module.ts index 53b0c1b..ec529b8 100644 --- a/src/modules/account/account.module.ts +++ b/src/modules/account/account.module.ts @@ -28,6 +28,6 @@ import { DomainRepository } from './repositories/domain.repository.js'; DomainRepository, AccountService, ], - exports: [AccountService, DomainRepository], + exports: [AccountService], }) export class AccountModule {} diff --git a/src/modules/account/account.service.ts b/src/modules/account/account.service.ts index 0644fc3..ba400ad 100644 --- a/src/modules/account/account.service.ts +++ b/src/modules/account/account.service.ts @@ -8,6 +8,7 @@ import { } from '@nestjs/common'; import { AccountProvider } from './account-provider.port.js'; import { MailAccount } from './domain/mail-account.domain.js'; +import { MailDomain } from './domain/mail-domain.domain.js'; import { AccountRepository } from './repositories/account.repository.js'; import { AddressRepository } from './repositories/address.repository.js'; import { DomainRepository } from './repositories/domain.repository.js'; @@ -27,6 +28,10 @@ export class AccountService { return this.getAccountOrFail(userId); } + async listActiveDomains(): Promise { + return this.domains.findAllActive(); + } + async findAccount(userId: string): Promise { return this.accounts.findByUserId(userId); } diff --git a/src/modules/email/email.controller.spec.ts b/src/modules/email/email.controller.spec.ts index 1693421..7e8bfd6 100644 --- a/src/modules/email/email.controller.spec.ts +++ b/src/modules/email/email.controller.spec.ts @@ -37,6 +37,44 @@ describe('EmailController', () => { }); }); + describe('getAll', () => { + it('when called with no query params, then it uses defaults', async () => { + const response = { + emails: [newEmailSummary()], + total: 1, + hasMoreMails: false, + }; + emailService.getAllEmails.mockResolvedValue(response); + + const result = await controller.getAll(userEmail); + + expect(emailService.getAllEmails).toHaveBeenCalledWith( + userEmail, + 20, + 0, + undefined, + ); + expect(result).toBe(response); + }); + + it('when called with limit, position and anchorId, then it parses them', async () => { + emailService.getAllEmails.mockResolvedValue({ + emails: [], + total: 0, + hasMoreMails: false, + }); + + await controller.getAll(userEmail, '10', '5', 'Ma1f09b'); + + expect(emailService.getAllEmails).toHaveBeenCalledWith( + userEmail, + 10, + 5, + 'Ma1f09b', + ); + }); + }); + describe('list', () => { it('When list is called with no query params, then it uses defaults', async () => { const response = { diff --git a/src/modules/email/email.controller.ts b/src/modules/email/email.controller.ts index 7cf48a3..1421636 100644 --- a/src/modules/email/email.controller.ts +++ b/src/modules/email/email.controller.ts @@ -50,8 +50,8 @@ export class EmailController { 'Returns every mailbox for the authenticated user, including folder counts.', }) @ApiOkResponse({ type: [MailboxResponseDto] }) - getMailboxes() { - return this.emailService.getMailboxes(STUB_USER); + getMailboxes(@User('email') email: string) { + return this.emailService.getMailboxes(email); } @Get('/all') @@ -130,6 +130,7 @@ export class EmailController { }) @ApiOkResponse({ type: EmailListResponseDto }) list( + @User('email') email: string, @Query('mailbox') mailbox: MailboxType = 'inbox', @Query('limit') limit?: string, @Query('position') position?: string, @@ -137,7 +138,6 @@ export class EmailController { ) { return this.emailService.listEmails( email, - STUB_USER, mailbox, limit ? Number(limit) || 20 : 20, position ? Number(position) || 0 : 0, @@ -156,7 +156,6 @@ export class EmailController { @ApiNotFoundResponse({ description: 'Email not found' }) get(@User('email') email: string, @Param('id') id: string) { return this.emailService.getEmail(email, id); - return this.emailService.getEmail(STUB_USER, id); } @Post('send') @@ -173,7 +172,6 @@ export class EmailController { }) send(@User('email') email: string, @Body() dto: SendEmailRequestDto) { return this.emailService.sendEmail(email, dto); - return this.emailService.sendEmail(STUB_USER, dto); } @Post('drafts') @@ -189,7 +187,6 @@ export class EmailController { }) saveDraft(@User('email') email: string, @Body() dto: DraftEmailRequestDto) { return this.emailService.saveDraft(email, dto); - return this.emailService.saveDraft(STUB_USER, dto); } @Patch(':id') @@ -209,19 +206,15 @@ export class EmailController { @Param('id') id: string, @Body() body: UpdateEmailRequestDto, ) { - async update(@Param('id') id: string, @Body() body: UpdateEmailRequestDto) { const ops: Promise[] = []; if (body.mailbox !== undefined) { ops.push(this.emailService.moveEmail(email, id, body.mailbox)); - ops.push(this.emailService.moveEmail(STUB_USER, id, body.mailbox)); } if (body.isRead !== undefined) { ops.push(this.emailService.markAsRead(email, id, body.isRead)); - ops.push(this.emailService.markAsRead(STUB_USER, id, body.isRead)); } if (body.isFlagged !== undefined) { ops.push(this.emailService.markAsFlagged(email, id, body.isFlagged)); - ops.push(this.emailService.markAsFlagged(STUB_USER, id, body.isFlagged)); } await Promise.all(ops); } @@ -237,6 +230,5 @@ export class EmailController { @ApiNotFoundResponse({ description: 'Email not found' }) delete(@User('email') email: string, @Param('id') id: string) { return this.emailService.deleteEmail(email, id); - return this.emailService.deleteEmail(STUB_USER, id); } } diff --git a/src/modules/email/email.service.spec.ts b/src/modules/email/email.service.spec.ts index ac8f531..4905e8b 100644 --- a/src/modules/email/email.service.spec.ts +++ b/src/modules/email/email.service.spec.ts @@ -1,7 +1,9 @@ -import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach } from 'vitest'; +import { Test } from '@nestjs/testing'; import { NotFoundException, BadRequestException } from '@nestjs/common'; +import { createMock, type DeepMocked } from '@golevelup/ts-vitest'; import { EmailService } from './email.service.js'; -import { type MailProvider } from './mail-provider.port.js'; +import { MailProvider } from './mail-provider.port.js'; import { newMailbox, newEmail, @@ -10,32 +12,20 @@ import { newDraftEmailDto, } from '../../../test/fixtures.js'; -type MockMailProvider = { - [K in keyof MailProvider]: ReturnType; -}; - -function createMockMailProvider(): MockMailProvider { - return { - getMailboxes: vi.fn(), - listEmails: vi.fn(), - getEmail: vi.fn(), - sendEmail: vi.fn(), - saveDraft: vi.fn(), - moveEmail: vi.fn(), - deleteEmail: vi.fn(), - markAsRead: vi.fn(), - markAsFlagged: vi.fn(), - }; -} - describe('EmailService', () => { let service: EmailService; - let provider: MockMailProvider; + let provider: DeepMocked; const userEmail = 'test@example.com'; - beforeEach(() => { - provider = createMockMailProvider(); - service = new EmailService(provider); + beforeEach(async () => { + const module = await Test.createTestingModule({ + providers: [EmailService], + }) + .useMocker(() => createMock()) + .compile(); + + service = module.get(EmailService); + provider = module.get>(MailProvider); }); describe('getMailboxes', () => { @@ -50,11 +40,35 @@ describe('EmailService', () => { }); }); + describe('getAllEmails', () => { + it('when called, then delegates with all parameters', async () => { + const response = { + emails: [newEmailSummary()], + total: 1, + hasMoreMails: false, + nextAnchor: undefined, + }; + provider.getAllEmails.mockResolvedValue(response); + + const result = await service.getAllEmails(userEmail, 20, 0); + + expect(provider.getAllEmails).toHaveBeenCalledWith( + userEmail, + 20, + 0, + undefined, + ); + expect(result).toBe(response); + }); + }); + describe('listEmails', () => { it('when called, then delegates with all parameters', async () => { const response = { emails: [newEmailSummary()], total: 1, + hasMoreMails: false, + nextAnchor: undefined, }; provider.listEmails.mockResolvedValue(response); diff --git a/src/modules/gateway/gateway.controller.spec.ts b/src/modules/gateway/gateway.controller.spec.ts index 6c16943..e40fc5e 100644 --- a/src/modules/gateway/gateway.controller.spec.ts +++ b/src/modules/gateway/gateway.controller.spec.ts @@ -3,7 +3,6 @@ import { Test, type TestingModule } from '@nestjs/testing'; import { createMock, type DeepMocked } from '@golevelup/ts-vitest'; import { GatewayController } from './gateway.controller.js'; import { AccountService } from '../account/account.service.js'; -import { DomainRepository } from '../account/repositories/domain.repository.js'; import { MailAccount } from '../account/domain/mail-account.domain.js'; import { MailDomain } from '../account/domain/mail-domain.domain.js'; import { @@ -16,7 +15,6 @@ import { v4 } from 'uuid'; describe('GatewayController', () => { let controller: GatewayController; let accountService: DeepMocked; - let domains: DeepMocked; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -27,7 +25,6 @@ describe('GatewayController', () => { controller = module.get(GatewayController); accountService = module.get(AccountService); - domains = module.get(DomainRepository); }); describe('provisionAccount', () => { @@ -74,7 +71,7 @@ describe('GatewayController', () => { MailDomain.build(newMailDomainAttributes({ domain: 'internxt.com' })), MailDomain.build(newMailDomainAttributes({ domain: 'internxt.me' })), ]; - domains.findAllActive.mockResolvedValue(domainList); + accountService.listActiveDomains.mockResolvedValue(domainList); const result = await controller.listDomains(); @@ -85,7 +82,7 @@ describe('GatewayController', () => { }); it('when no active domains, then returns empty array', async () => { - domains.findAllActive.mockResolvedValue([]); + accountService.listActiveDomains.mockResolvedValue([]); const result = await controller.listDomains(); diff --git a/src/modules/gateway/gateway.controller.ts b/src/modules/gateway/gateway.controller.ts index a8bf68d..18cab11 100644 --- a/src/modules/gateway/gateway.controller.ts +++ b/src/modules/gateway/gateway.controller.ts @@ -12,7 +12,6 @@ import { import { ApiBearerAuth, ApiOperation, ApiTags } from '@nestjs/swagger'; import { Public } from '../auth/decorators/public.decorator.js'; import { AccountService } from '../account/account.service.js'; -import { DomainRepository } from '../account/repositories/domain.repository.js'; import { GatewayAuthGuard } from './gateway.guard.js'; import { ProvisionAccountRequestDto } from './gateway.dto.js'; @@ -24,10 +23,7 @@ import { ProvisionAccountRequestDto } from './gateway.dto.js'; export class GatewayController { private readonly logger = new Logger(GatewayController.name); - constructor( - private readonly accountService: AccountService, - private readonly domains: DomainRepository, - ) {} + constructor(private readonly accountService: AccountService) {} @Post('accounts') @HttpCode(HttpStatus.CREATED) @@ -58,7 +54,7 @@ export class GatewayController { summary: 'List available mail domains (called by the auth service)', }) async listDomains() { - const activeDomains = await this.domains.findAllActive(); + const activeDomains = await this.accountService.listActiveDomains(); return activeDomains.map((d) => ({ domain: d.domain })); } diff --git a/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts b/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts index 8671815..4c94e6c 100644 --- a/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts +++ b/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts @@ -68,6 +68,88 @@ describe('JmapMailProvider', () => { }); }); + describe('getAllEmails', () => { + it('when called with position, then returns email summaries with mailboxIds', async () => { + const jmapEmails = [newJmapEmail(), newJmapEmail()]; + + jmapService.request.mockResolvedValueOnce( + jmapMultiResponse( + { ids: jmapEmails.map((e) => e.id), total: 42 }, + { list: jmapEmails }, + ), + ); + + const result = await provider.getAllEmails('user@test.com', 20, 0); + + expect(result.emails).toHaveLength(2); + expect(result.total).toBe(42); + expect(result.emails[0]!.mailboxIds).toEqual( + Object.keys(jmapEmails[0]!.mailboxIds), + ); + }); + + it('when called with anchorId, then uses anchor-based pagination', async () => { + const jmapEmails = [newJmapEmail()]; + + jmapService.request.mockResolvedValueOnce( + jmapMultiResponse( + { ids: jmapEmails.map((e) => e.id), total: 10 }, + { list: jmapEmails }, + ), + ); + + await provider.getAllEmails('user@test.com', 20, 0, 'anchor-id'); + + const queryParams = jmapService.request.mock.calls[0]![1][0]![1]; + expect(queryParams['anchor']).toBe('anchor-id'); + expect(queryParams['anchorOffset']).toBe(1); + expect(queryParams['position']).toBeUndefined(); + }); + + it('when result count equals limit, then hasMoreMails is true with nextAnchor', async () => { + const jmapEmails = [newJmapEmail(), newJmapEmail()]; + + jmapService.request.mockResolvedValueOnce( + jmapMultiResponse( + { ids: jmapEmails.map((e) => e.id), total: 10 }, + { list: jmapEmails }, + ), + ); + + const result = await provider.getAllEmails('user@test.com', 2, 0); + + expect(result.hasMoreMails).toBe(true); + expect(result.nextAnchor).toBe(jmapEmails[1]!.id); + }); + + it('when result count is less than limit, then hasMoreMails is false', async () => { + const jmapEmails = [newJmapEmail()]; + + jmapService.request.mockResolvedValueOnce( + jmapMultiResponse( + { ids: jmapEmails.map((e) => e.id), total: 1 }, + { list: jmapEmails }, + ), + ); + + const result = await provider.getAllEmails('user@test.com', 20, 0); + + expect(result.hasMoreMails).toBe(false); + expect(result.nextAnchor).toBeUndefined(); + }); + + it('when no filter is applied, then query has no inMailbox filter', async () => { + jmapService.request.mockResolvedValueOnce( + jmapMultiResponse({ ids: [], total: 0 }, { list: [] }), + ); + + await provider.getAllEmails('user@test.com', 20, 0); + + const queryParams = jmapService.request.mock.calls[0]![1][0]![1]; + expect(queryParams['filter']).toBeUndefined(); + }); + }); + describe('listEmails', () => { it('When called, then it returns email summaries and total count', async () => { const inboxMailbox = newJmapMailbox({ role: 'inbox' }); diff --git a/src/modules/infrastructure/jmap/jmap-mail.provider.ts b/src/modules/infrastructure/jmap/jmap-mail.provider.ts index 9917c0a..b31756b 100644 --- a/src/modules/infrastructure/jmap/jmap-mail.provider.ts +++ b/src/modules/infrastructure/jmap/jmap-mail.provider.ts @@ -92,49 +92,7 @@ export class JmapMailProvider extends MailProvider { anchorId?: string, ): Promise { const accountId = await this.jmap.getPrimaryAccountId(userEmail); - - const queryParams: Record = { - accountId, - sort: [{ property: 'receivedAt', isAscending: false }], - limit, - calculateTotal: true, - }; - - if (anchorId) { - queryParams.anchor = anchorId; - queryParams.anchorOffset = 1; - } else { - queryParams.position = position; - } - - const response = await this.jmap.request(userEmail, [ - ['Email/query', queryParams, 'r0'], - [ - 'Email/get', - { - accountId, - '#ids': { resultOf: 'r0', name: 'Email/query', path: '/ids' }, - properties: EMAIL_LIST_PROPERTIES, - }, - 'r1', - ], - ]); - - const queryResult = response.methodResponses[0]![1] as JmapQueryResponse; - const getResult = response - .methodResponses[1]![1] as JmapGetResponse; - - const emails = getResult.list.map(mapJmapEmailToSummary); - - const nextAnchor = emails.length ? emails.at(-1)?.id : undefined; - const hasMoreEmails = emails.length >= limit; - - return { - emails, - total: queryResult.total ?? 0, - hasMoreMails: hasMoreEmails, - nextAnchor: hasMoreEmails ? nextAnchor : undefined, - }; + return this.queryEmails(userEmail, accountId, limit, position, anchorId); } async listEmails( @@ -148,15 +106,30 @@ export class JmapMailProvider extends MailProvider { this.jmap.getPrimaryAccountId(userEmail), this.resolveMailboxId(userEmail, mailbox), ]); + return this.queryEmails(userEmail, accountId, limit, position, anchorId, { + inMailbox: mailboxId, + }); + } + private async queryEmails( + userEmail: string, + accountId: string, + limit: number, + position: number, + anchorId?: string, + filter?: Record, + ): Promise { const queryParams: Record = { accountId, - filter: { inMailbox: mailboxId }, sort: [{ property: 'receivedAt', isAscending: false }], limit, calculateTotal: true, }; + if (filter) { + queryParams.filter = filter; + } + if (anchorId) { queryParams.anchor = anchorId; queryParams.anchorOffset = 1; @@ -171,7 +144,7 @@ export class JmapMailProvider extends MailProvider { { accountId, '#ids': { resultOf: 'r0', name: 'Email/query', path: '/ids' }, - properties: [...EMAIL_LIST_PROPERTIES, 'mailboxIds'], + properties: EMAIL_LIST_PROPERTIES, }, 'r1', ], @@ -182,15 +155,13 @@ export class JmapMailProvider extends MailProvider { .methodResponses[1]![1] as JmapGetResponse; const emails = getResult.list.map(mapJmapEmailToSummary); - - const nextAnchor = emails.length ? emails.at(-1)?.id : undefined; const hasMoreEmails = emails.length >= limit; return { emails, total: queryResult.total ?? 0, hasMoreMails: hasMoreEmails, - nextAnchor: hasMoreEmails ? nextAnchor : undefined, + nextAnchor: hasMoreEmails ? emails.at(-1)?.id : undefined, }; } diff --git a/test/fixtures.ts b/test/fixtures.ts index 7dbbbd1..89f7cc1 100644 --- a/test/fixtures.ts +++ b/test/fixtures.ts @@ -90,6 +90,7 @@ export function newEmailSummary(attrs?: Partial): EmailSummary { return { id: randomId(), threadId: randomId(), + mailboxIds: [randomId()], from: [newEmailAddress()], to: [newEmailAddress()], subject: random.sentence({ words: 5 }), From 0560a0e96bc4893695b5ef765c819933f38501d0 Mon Sep 17 00:00:00 2001 From: jzunigax2 <125698953+jzunigax2@users.noreply.github.com> Date: Mon, 6 Apr 2026 16:39:57 -0600 Subject: [PATCH 3/3] refactor: rename and streamline email retrieval methods - Renamed `getAllEmails` to `listEmails` in EmailController and EmailService for clarity. - Updated related tests to reflect the new method name and adjusted parameters accordingly. - Enhanced API documentation to clarify the behavior of the `list` method, including mailbox filtering. - Removed deprecated `getAllEmails` method from MailProvider and JmapMailProvider, consolidating email retrieval logic. --- src/modules/email/email.controller.spec.ts | 46 ++++------- src/modules/email/email.controller.ts | 50 ++---------- src/modules/email/email.service.spec.ts | 19 +++-- src/modules/email/email.service.ts | 11 +-- src/modules/email/mail-provider.port.ts | 8 +- .../jmap/jmap-mail.provider.spec.ts | 76 ++++++++----------- .../infrastructure/jmap/jmap-mail.provider.ts | 21 ++--- 7 files changed, 66 insertions(+), 165 deletions(-) diff --git a/src/modules/email/email.controller.spec.ts b/src/modules/email/email.controller.spec.ts index 7e8bfd6..e560c07 100644 --- a/src/modules/email/email.controller.spec.ts +++ b/src/modules/email/email.controller.spec.ts @@ -37,19 +37,20 @@ describe('EmailController', () => { }); }); - describe('getAll', () => { - it('when called with no query params, then it uses defaults', async () => { + describe('list', () => { + it('when called with no query params, then it lists all emails', async () => { const response = { emails: [newEmailSummary()], total: 1, hasMoreMails: false, }; - emailService.getAllEmails.mockResolvedValue(response); + emailService.listEmails.mockResolvedValue(response); - const result = await controller.getAll(userEmail); + const result = await controller.list(userEmail); - expect(emailService.getAllEmails).toHaveBeenCalledWith( + expect(emailService.listEmails).toHaveBeenCalledWith( userEmail, + undefined, 20, 0, undefined, @@ -57,34 +58,14 @@ describe('EmailController', () => { expect(result).toBe(response); }); - it('when called with limit, position and anchorId, then it parses them', async () => { - emailService.getAllEmails.mockResolvedValue({ + it('when called with a mailbox filter, then it filters by mailbox', async () => { + emailService.listEmails.mockResolvedValue({ emails: [], total: 0, hasMoreMails: false, }); - await controller.getAll(userEmail, '10', '5', 'Ma1f09b'); - - expect(emailService.getAllEmails).toHaveBeenCalledWith( - userEmail, - 10, - 5, - 'Ma1f09b', - ); - }); - }); - - describe('list', () => { - it('When list is called with no query params, then it uses defaults', async () => { - const response = { - emails: [newEmailSummary()], - total: 1, - hasMoreMails: false, - }; - emailService.listEmails.mockResolvedValue(response); - - const result = await controller.list(userEmail, 'inbox'); + await controller.list(userEmail, 'inbox'); expect(emailService.listEmails).toHaveBeenCalledWith( userEmail, @@ -93,28 +74,27 @@ describe('EmailController', () => { 0, undefined, ); - expect(result).toBe(response); }); - it('When list is called with limit and position, then it parses them', async () => { + it('when called with limit, position and anchorId, then it parses them', async () => { emailService.listEmails.mockResolvedValue({ emails: [], total: 0, hasMoreMails: false, }); - await controller.list(userEmail, 'sent', '10', '5'); + await controller.list(userEmail, 'sent', '10', '5', 'Ma1f09b'); expect(emailService.listEmails).toHaveBeenCalledWith( userEmail, 'sent', 10, 5, - undefined, + 'Ma1f09b', ); }); - it('When list is called with non-numeric strings, then it falls back to defaults', async () => { + it('when called with non-numeric strings, then it falls back to defaults', async () => { emailService.listEmails.mockResolvedValue({ emails: [], total: 0, diff --git a/src/modules/email/email.controller.ts b/src/modules/email/email.controller.ts index 1421636..aa3b42b 100644 --- a/src/modules/email/email.controller.ts +++ b/src/modules/email/email.controller.ts @@ -54,58 +54,18 @@ export class EmailController { return this.emailService.getMailboxes(email); } - @Get('/all') - @ApiOperation({ - summary: 'List emails', - description: 'Get all the emails no matter the mailbox.', - }) - @ApiQuery({ - name: 'limit', - required: false, - type: Number, - description: 'Maximum number of emails to return. Defaults to `20`.', - example: 20, - }) - @ApiQuery({ - name: 'position', - required: false, - type: Number, - description: 'Zero-based offset for pagination. Defaults to `0`.', - example: 0, - }) - @ApiQuery({ - name: 'anchorId', - required: false, - type: String, - description: 'Anchor ID for pagination.', - example: 'Ma1f09b…', - }) - @ApiOkResponse({ type: EmailListResponseDto }) - getAll( - @User('email') email: string, - @Query('limit') limit?: string, - @Query('position') position?: string, - @Query('anchorId') anchorId?: string, - ) { - return this.emailService.getAllEmails( - email, - limit ? Number(limit) || 20 : 20, - position ? Number(position) || 0 : 0, - anchorId, - ); - } - @Get() @ApiOperation({ summary: 'List emails', description: - 'Paginated list of email summaries for a given mailbox. Defaults to the inbox.', + 'Paginated list of email summaries. Filter by mailbox or omit to list all.', }) @ApiQuery({ name: 'mailbox', required: false, enum: ['inbox', 'drafts', 'sent', 'trash', 'spam', 'archive'], - description: 'Mailbox to list. Defaults to `inbox`.', + description: + 'Mailbox to filter by. Omit to list emails from all mailboxes.', }) @ApiQuery({ name: 'limit', @@ -131,14 +91,14 @@ export class EmailController { @ApiOkResponse({ type: EmailListResponseDto }) list( @User('email') email: string, - @Query('mailbox') mailbox: MailboxType = 'inbox', + @Query('mailbox') mailbox?: MailboxType, @Query('limit') limit?: string, @Query('position') position?: string, @Query('anchorId') anchorId?: string, ) { return this.emailService.listEmails( email, - mailbox, + mailbox ?? undefined, limit ? Number(limit) || 20 : 20, position ? Number(position) || 0 : 0, anchorId, diff --git a/src/modules/email/email.service.spec.ts b/src/modules/email/email.service.spec.ts index 4905e8b..075a70b 100644 --- a/src/modules/email/email.service.spec.ts +++ b/src/modules/email/email.service.spec.ts @@ -40,30 +40,29 @@ describe('EmailService', () => { }); }); - describe('getAllEmails', () => { - it('when called, then delegates with all parameters', async () => { + describe('listEmails', () => { + it('when called with a mailbox, then delegates with mailbox', async () => { const response = { emails: [newEmailSummary()], total: 1, hasMoreMails: false, nextAnchor: undefined, }; - provider.getAllEmails.mockResolvedValue(response); + provider.listEmails.mockResolvedValue(response); - const result = await service.getAllEmails(userEmail, 20, 0); + const result = await service.listEmails(userEmail, 'inbox', 20, 0); - expect(provider.getAllEmails).toHaveBeenCalledWith( + expect(provider.listEmails).toHaveBeenCalledWith( userEmail, + 'inbox', 20, 0, undefined, ); expect(result).toBe(response); }); - }); - describe('listEmails', () => { - it('when called, then delegates with all parameters', async () => { + it('when called without a mailbox, then delegates with undefined', async () => { const response = { emails: [newEmailSummary()], total: 1, @@ -72,11 +71,11 @@ describe('EmailService', () => { }; provider.listEmails.mockResolvedValue(response); - const result = await service.listEmails(userEmail, 'inbox', 20, 0); + const result = await service.listEmails(userEmail, undefined, 20, 0); expect(provider.listEmails).toHaveBeenCalledWith( userEmail, - 'inbox', + undefined, 20, 0, undefined, diff --git a/src/modules/email/email.service.ts b/src/modules/email/email.service.ts index 281a77d..0ecffa0 100644 --- a/src/modules/email/email.service.ts +++ b/src/modules/email/email.service.ts @@ -21,18 +21,9 @@ export class EmailService { return this.mail.getMailboxes(userEmail); } - getAllEmails( - userEmail: string, - limit: number, - position: number, - anchorId?: string, - ): Promise { - return this.mail.getAllEmails(userEmail, limit, position, anchorId); - } - listEmails( userEmail: string, - mailbox: MailboxType, + mailbox: MailboxType | undefined, limit: number, position: number, anchorId?: string, diff --git a/src/modules/email/mail-provider.port.ts b/src/modules/email/mail-provider.port.ts index 1a70084..fa0387a 100644 --- a/src/modules/email/mail-provider.port.ts +++ b/src/modules/email/mail-provider.port.ts @@ -9,15 +9,9 @@ import type { export abstract class MailProvider { abstract getMailboxes(userEmail: string): Promise; - abstract getAllEmails( - userEmail: string, - limit: number, - position: number, - anchorId?: string, - ): Promise; abstract listEmails( userEmail: string, - mailbox: MailboxType, + mailbox: MailboxType | undefined, limit: number, position: number, anchorId?: string, diff --git a/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts b/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts index 4c94e6c..d6f6186 100644 --- a/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts +++ b/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts @@ -68,8 +68,8 @@ describe('JmapMailProvider', () => { }); }); - describe('getAllEmails', () => { - it('when called with position, then returns email summaries with mailboxIds', async () => { + describe('listEmails', () => { + it('when called without mailbox, then returns all email summaries', async () => { const jmapEmails = [newJmapEmail(), newJmapEmail()]; jmapService.request.mockResolvedValueOnce( @@ -79,7 +79,12 @@ describe('JmapMailProvider', () => { ), ); - const result = await provider.getAllEmails('user@test.com', 20, 0); + const result = await provider.listEmails( + 'user@test.com', + undefined, + 20, + 0, + ); expect(result.emails).toHaveLength(2); expect(result.total).toBe(42); @@ -88,22 +93,24 @@ describe('JmapMailProvider', () => { ); }); - it('when called with anchorId, then uses anchor-based pagination', async () => { - const jmapEmails = [newJmapEmail()]; + it('when called with a mailbox, then filters by that mailbox', async () => { + const inboxMailbox = newJmapMailbox({ role: 'inbox' }); + const jmapEmails = [newJmapEmail(), newJmapEmail()]; + jmapService.request.mockResolvedValueOnce( + jmapResponse({ list: [inboxMailbox] }), + ); jmapService.request.mockResolvedValueOnce( jmapMultiResponse( - { ids: jmapEmails.map((e) => e.id), total: 10 }, + { ids: jmapEmails.map((e) => e.id), total: 42 }, { list: jmapEmails }, ), ); - await provider.getAllEmails('user@test.com', 20, 0, 'anchor-id'); + const result = await provider.listEmails('user@test.com', 'inbox', 20, 0); - const queryParams = jmapService.request.mock.calls[0]![1][0]![1]; - expect(queryParams['anchor']).toBe('anchor-id'); - expect(queryParams['anchorOffset']).toBe(1); - expect(queryParams['position']).toBeUndefined(); + expect(result.emails).toHaveLength(2); + expect(result.total).toBe(42); }); it('when result count equals limit, then hasMoreMails is true with nextAnchor', async () => { @@ -116,7 +123,12 @@ describe('JmapMailProvider', () => { ), ); - const result = await provider.getAllEmails('user@test.com', 2, 0); + const result = await provider.listEmails( + 'user@test.com', + undefined, + 2, + 0, + ); expect(result.hasMoreMails).toBe(true); expect(result.nextAnchor).toBe(jmapEmails[1]!.id); @@ -132,44 +144,16 @@ describe('JmapMailProvider', () => { ), ); - const result = await provider.getAllEmails('user@test.com', 20, 0); + const result = await provider.listEmails( + 'user@test.com', + undefined, + 20, + 0, + ); expect(result.hasMoreMails).toBe(false); expect(result.nextAnchor).toBeUndefined(); }); - - it('when no filter is applied, then query has no inMailbox filter', async () => { - jmapService.request.mockResolvedValueOnce( - jmapMultiResponse({ ids: [], total: 0 }, { list: [] }), - ); - - await provider.getAllEmails('user@test.com', 20, 0); - - const queryParams = jmapService.request.mock.calls[0]![1][0]![1]; - expect(queryParams['filter']).toBeUndefined(); - }); - }); - - describe('listEmails', () => { - it('When called, then it returns email summaries and total count', async () => { - const inboxMailbox = newJmapMailbox({ role: 'inbox' }); - const jmapEmails = [newJmapEmail(), newJmapEmail()]; - - jmapService.request.mockResolvedValueOnce( - jmapResponse({ list: [inboxMailbox] }), - ); - jmapService.request.mockResolvedValueOnce( - jmapMultiResponse( - { ids: jmapEmails.map((e) => e.id), total: 42 }, - { list: jmapEmails }, - ), - ); - - const result = await provider.listEmails('user@test.com', 'inbox', 20, 0); - - expect(result.emails).toHaveLength(2); - expect(result.total).toBe(42); - }); }); describe('getEmail', () => { diff --git a/src/modules/infrastructure/jmap/jmap-mail.provider.ts b/src/modules/infrastructure/jmap/jmap-mail.provider.ts index b31756b..3a5baa1 100644 --- a/src/modules/infrastructure/jmap/jmap-mail.provider.ts +++ b/src/modules/infrastructure/jmap/jmap-mail.provider.ts @@ -85,27 +85,20 @@ export class JmapMailProvider extends MailProvider { return jmapMailboxes.map(mapJmapMailbox); } - async getAllEmails( + async listEmails( userEmail: string, + mailbox: MailboxType | undefined, limit: number, position: number, anchorId?: string, ): Promise { const accountId = await this.jmap.getPrimaryAccountId(userEmail); - return this.queryEmails(userEmail, accountId, limit, position, anchorId); - } - async listEmails( - userEmail: string, - mailbox: MailboxType, - limit: number, - position: number, - anchorId?: string, - ): Promise { - const [accountId, mailboxId] = await Promise.all([ - this.jmap.getPrimaryAccountId(userEmail), - this.resolveMailboxId(userEmail, mailbox), - ]); + if (!mailbox) { + return this.queryEmails(userEmail, accountId, limit, position, anchorId); + } + + const mailboxId = await this.resolveMailboxId(userEmail, mailbox); return this.queryEmails(userEmail, accountId, limit, position, anchorId, { inMailbox: mailboxId, });