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..e560c07 100644 --- a/src/modules/email/email.controller.spec.ts +++ b/src/modules/email/email.controller.spec.ts @@ -38,7 +38,7 @@ describe('EmailController', () => { }); describe('list', () => { - it('When list is called with no query params, then it uses defaults', async () => { + it('when called with no query params, then it lists all emails', async () => { const response = { emails: [newEmailSummary()], total: 1, @@ -46,11 +46,11 @@ describe('EmailController', () => { }; emailService.listEmails.mockResolvedValue(response); - const result = await controller.list(userEmail, 'inbox'); + const result = await controller.list(userEmail); expect(emailService.listEmails).toHaveBeenCalledWith( userEmail, - 'inbox', + undefined, 20, 0, undefined, @@ -58,25 +58,43 @@ describe('EmailController', () => { expect(result).toBe(response); }); - it('When list is called with limit and position, then it parses them', async () => { + it('when called with a mailbox filter, then it filters by mailbox', async () => { emailService.listEmails.mockResolvedValue({ emails: [], total: 0, hasMoreMails: false, }); - await controller.list(userEmail, 'sent', '10', '5'); + await controller.list(userEmail, 'inbox'); + + expect(emailService.listEmails).toHaveBeenCalledWith( + userEmail, + 'inbox', + 20, + 0, + undefined, + ); + }); + + 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', '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 635e713..aa3b42b 100644 --- a/src/modules/email/email.controller.ts +++ b/src/modules/email/email.controller.ts @@ -58,13 +58,14 @@ export class EmailController { @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', @@ -90,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.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.spec.ts b/src/modules/email/email.service.spec.ts index ac8f531..075a70b 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', () => { @@ -51,10 +41,12 @@ describe('EmailService', () => { }); describe('listEmails', () => { - it('when called, then delegates with all parameters', async () => { + it('when called with a mailbox, then delegates with mailbox', async () => { const response = { emails: [newEmailSummary()], total: 1, + hasMoreMails: false, + nextAnchor: undefined, }; provider.listEmails.mockResolvedValue(response); @@ -69,6 +61,27 @@ describe('EmailService', () => { ); expect(result).toBe(response); }); + + it('when called without a mailbox, then delegates with undefined', async () => { + const response = { + emails: [newEmailSummary()], + total: 1, + hasMoreMails: false, + nextAnchor: undefined, + }; + provider.listEmails.mockResolvedValue(response); + + const result = await service.listEmails(userEmail, undefined, 20, 0); + + expect(provider.listEmails).toHaveBeenCalledWith( + userEmail, + undefined, + 20, + 0, + undefined, + ); + expect(result).toBe(response); + }); }); describe('getEmail', () => { diff --git a/src/modules/email/email.service.ts b/src/modules/email/email.service.ts index 2f716c7..0ecffa0 100644 --- a/src/modules/email/email.service.ts +++ b/src/modules/email/email.service.ts @@ -23,7 +23,7 @@ export class EmailService { listEmails( userEmail: string, - mailbox: MailboxType, + mailbox: MailboxType | undefined, limit: number, position: number, anchorId?: string, 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..fa0387a 100644 --- a/src/modules/email/mail-provider.port.ts +++ b/src/modules/email/mail-provider.port.ts @@ -11,7 +11,7 @@ export abstract class MailProvider { abstract getMailboxes(userEmail: string): Promise; abstract listEmails( userEmail: string, - mailbox: MailboxType, + mailbox: MailboxType | undefined, limit: number, position: number, anchorId?: string, 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.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.spec.ts b/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts index 8671815..d6f6186 100644 --- a/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts +++ b/src/modules/infrastructure/jmap/jmap-mail.provider.spec.ts @@ -69,7 +69,31 @@ describe('JmapMailProvider', () => { }); describe('listEmails', () => { - it('When called, then it returns email summaries and total count', async () => { + it('when called without mailbox, then returns all email summaries', async () => { + const jmapEmails = [newJmapEmail(), newJmapEmail()]; + + jmapService.request.mockResolvedValueOnce( + jmapMultiResponse( + { ids: jmapEmails.map((e) => e.id), total: 42 }, + { list: jmapEmails }, + ), + ); + + const result = await provider.listEmails( + 'user@test.com', + undefined, + 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 a mailbox, then filters by that mailbox', async () => { const inboxMailbox = newJmapMailbox({ role: 'inbox' }); const jmapEmails = [newJmapEmail(), newJmapEmail()]; @@ -88,6 +112,48 @@ describe('JmapMailProvider', () => { expect(result.emails).toHaveLength(2); expect(result.total).toBe(42); }); + + 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.listEmails( + 'user@test.com', + undefined, + 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.listEmails( + 'user@test.com', + undefined, + 20, + 0, + ); + + expect(result.hasMoreMails).toBe(false); + expect(result.nextAnchor).toBeUndefined(); + }); }); describe('getEmail', () => { diff --git a/src/modules/infrastructure/jmap/jmap-mail.provider.ts b/src/modules/infrastructure/jmap/jmap-mail.provider.ts index 3943a9c..3a5baa1 100644 --- a/src/modules/infrastructure/jmap/jmap-mail.provider.ts +++ b/src/modules/infrastructure/jmap/jmap-mail.provider.ts @@ -87,24 +87,42 @@ export class JmapMailProvider extends MailProvider { async listEmails( userEmail: string, - mailbox: MailboxType, + mailbox: MailboxType | undefined, limit: number, position: number, anchorId?: string, ): Promise { - const [accountId, mailboxId] = await Promise.all([ - this.jmap.getPrimaryAccountId(userEmail), - this.resolveMailboxId(userEmail, mailbox), - ]); + const accountId = await this.jmap.getPrimaryAccountId(userEmail); + + 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, + }); + } + 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; @@ -130,15 +148,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 }),