refactor: simplify account management by removing address-related methods

- Removed addAddress, removeAddress, and setPrimaryAddress methods from the AccountProvider interface and StalwartAccountProvider implementation to streamline account management.
- Updated AccountService to handle address management directly, ensuring proper deletion and linking of addresses during account operations.
- Adjusted related unit tests to reflect the changes in address handling and account deletion processes.

Author: jzunigax2

migrations/20260326130000-add-deleted-at-to-mail-provider-accounts.js

@@ -0,0 +1,16 @@
+'use strict';
+
+/** @type {import('sequelize-cli').Migration} */
+module.exports = {
+  async up(queryInterface, Sequelize) {
+    await queryInterface.addColumn('mail_provider_accounts', 'deleted_at', {
+      type: Sequelize.DATE,
+      allowNull: true,
+      defaultValue: null,
+    });
+  },
+
+  async down(queryInterface) {
+    await queryInterface.removeColumn('mail_provider_accounts', 'deleted_at');
+  },
+};

src/modules/account/account-provider.port.ts

@@ -4,10 +4,4 @@ export abstract class AccountProvider {
   abstract createAccount(params: CreateAccountParams): Promise<void>;
   abstract deleteAccount(name: string): Promise<void>;
   abstract getAccount(name: string): Promise<AccountInfo | null>;
-  abstract addAddress(name: string, address: string): Promise<void>;
-  abstract removeAddress(name: string, address: string): Promise<void>;
-  abstract setPrimaryAddress(
-    currentName: string,
-    newPrimaryAddress: string,
-  ): Promise<void>;
 }

src/modules/account/account.service.spec.ts

@@ -63,16 +63,24 @@ describe('AccountService', () => {
   });
 
   describe('deleteAccount', () => {
-    it('when account has a principal name, then deletes from provider and DB', async () => {
-      const attrs = newMailAccountAttributes();
-      const account = MailAccount.build(attrs);
+    it('when account has addresses, then deletes all principals and account', async () => {
+      const addr1 = newMailAddressAttributes({ isDefault: true });
+      const addr2 = newMailAddressAttributes({ isDefault: false });
+      const account = MailAccount.build(
+        newMailAccountAttributes({ addresses: [addr1, addr2] }),
+      );
       accounts.findByUserId.mockResolvedValue(account);
 
-      await service.deleteAccount(attrs.userId);
+      await service.deleteAccount(account.userId);
 
       expect(provider.deleteAccount).toHaveBeenCalledWith(
-        account.providerAccountId,
+        addr1.providerExternalId,
+      );
+      expect(provider.deleteAccount).toHaveBeenCalledWith(
+        addr2.providerExternalId,
       );
+      expect(addresses.deleteProviderLink).toHaveBeenCalledWith(addr1.id);
+      expect(addresses.deleteProviderLink).toHaveBeenCalledWith(addr2.id);
       expect(accounts.delete).toHaveBeenCalledWith(account.id);
     });
 
@@ -86,7 +94,7 @@ describe('AccountService', () => {
   });
 
   describe('addAddress', () => {
-    it('when all conditions met, then creates address and links provider', async () => {
+    it('when all conditions met, then creates principal and links provider', async () => {
       const accountAttrs = newMailAccountAttributes();
       const account = MailAccount.build(accountAttrs);
       const domainAttrs = newMailDomainAttributes();
@@ -103,6 +111,8 @@ describe('AccountService', () => {
         accountAttrs.userId,
         newAddr,
         domainAttrs.domain,
+        'password123',
+        'Display Name',
       );
 
       expect(addresses.create).toHaveBeenCalledWith({
@@ -111,14 +121,16 @@ describe('AccountService', () => {
         domainId: domain.id,
         isDefault: false,
       });
-      expect(provider.addAddress).toHaveBeenCalledWith(
-        account.providerAccountId,
-        newAddr,
-      );
+      expect(provider.createAccount).toHaveBeenCalledWith({
+        accountId: newAddressId,
+        primaryAddress: newAddr,
+        displayName: 'Display Name',
+        password: 'password123',
+      });
       expect(addresses.createProviderLink).toHaveBeenCalledWith({
         mailAddressId: newAddressId,
         provider: 'stalwart',
-        externalId: account.providerAccountId,
+        externalId: newAddr,
       });
     });
 
@@ -130,7 +142,7 @@ describe('AccountService', () => {
       addresses.findByAddress.mockResolvedValue(null);
 
       await expect(
-        service.addAddress('unknown', 'a@b.com', 'b.com'),
+        service.addAddress('unknown', 'a@b.com', 'b.com', 'pass'),
       ).rejects.toThrow(NotFoundException);
     });
 
@@ -141,7 +153,7 @@ describe('AccountService', () => {
       addresses.findByAddress.mockResolvedValue(null);
 
       await expect(
-        service.addAddress(account.userId, 'a@b.com', 'unknown.com'),
+        service.addAddress(account.userId, 'a@b.com', 'unknown.com', 'pass'),
       ).rejects.toThrow(NotFoundException);
     });
 
@@ -155,7 +167,12 @@ describe('AccountService', () => {
       addresses.findByAddress.mockResolvedValue(existing);
 
       await expect(
-        service.addAddress(account.userId, existing.address, domain.domain),
+        service.addAddress(
+          account.userId,
+          existing.address,
+          domain.domain,
+          'pass',
+        ),
       ).rejects.toThrow(ConflictException);
     });
 
@@ -168,10 +185,15 @@ describe('AccountService', () => {
       domains.findByDomain.mockResolvedValue(domain);
       addresses.findByAddress.mockResolvedValue(null);
       addresses.create.mockResolvedValue(newAddressId);
-      provider.addAddress.mockRejectedValue(new Error('provider down'));
+      provider.createAccount.mockRejectedValue(new Error('provider down'));
 
       await expect(
-        service.addAddress(account.userId, 'new@example.com', domain.domain),
+        service.addAddress(
+          account.userId,
+          'new@example.com',
+          domain.domain,
+          'pass',
+        ),
       ).rejects.toThrow('provider down');
 
       expect(addresses.delete).toHaveBeenCalledWith(newAddressId);
@@ -180,7 +202,7 @@ describe('AccountService', () => {
   });
 
   describe('removeAddress', () => {
-    it('when address exists and is not default, then removes it', async () => {
+    it('when address exists and is not default, then deletes principal and address', async () => {
       const nonDefaultAddr = newMailAddressAttributes({ isDefault: false });
       const account = MailAccount.build(
         newMailAccountAttributes({
@@ -194,9 +216,8 @@ describe('AccountService', () => {
 
       await service.removeAddress(account.userId, nonDefaultAddr.address);
 
-      expect(provider.removeAddress).toHaveBeenCalledWith(
-        account.providerAccountId,
-        nonDefaultAddr.address,
+      expect(provider.deleteAccount).toHaveBeenCalledWith(
+        nonDefaultAddr.providerExternalId,
       );
       expect(addresses.deleteProviderLink).toHaveBeenCalledWith(
         nonDefaultAddr.id,

src/modules/account/account.service.ts

@@ -29,9 +29,12 @@ export class AccountService {
   async deleteAccount(userId: string): Promise<void> {
     const account = await this.getAccountOrFail(userId);
 
-    if (account.providerAccountId) {
-      await this.provider.deleteAccount(account.providerAccountId);
-    }
+    await Promise.all(
+      account.addresses.map(async (a) => {
+        await this.provider.deleteAccount(a.providerExternalId);
+        await this.addresses.deleteProviderLink(a.id);
+      }),
+    );
 
     await this.accounts.delete(account.id);
     this.logger.log(`Deleted account for user '${userId}'`);
@@ -41,6 +44,8 @@ export class AccountService {
     userId: string,
     address: string,
     domainName: string,
+    password: string,
+    displayName?: string,
   ): Promise<void> {
     const [account, domain, existing] = await Promise.all([
       this.accounts.findByUserId(userId),
@@ -51,8 +56,6 @@ export class AccountService {
     if (!account) {
       throw new NotFoundException(`No mail account for user '${userId}'`);
     }
-    const providerAccountId = this.requireProviderAccountId(account);
-
     if (!domain) {
       throw new NotFoundException(`Domain '${domainName}' not found`);
     }
@@ -68,7 +71,12 @@ export class AccountService {
     });
 
     try {
-      await this.provider.addAddress(providerAccountId, address);
+      await this.provider.createAccount({
+        accountId: newAddressId,
+        primaryAddress: address,
+        displayName: displayName ?? '',
+        password,
+      });
     } catch (error) {
       await this.addresses.delete(newAddressId);
       throw error;
@@ -77,7 +85,7 @@ export class AccountService {
     await this.addresses.createProviderLink({
       mailAddressId: newAddressId,
       provider: 'stalwart',
-      externalId: providerAccountId,
+      externalId: address,
     });
 
     this.logger.log(`Added address '${address}' to account '${userId}'`);
@@ -99,9 +107,7 @@ export class AccountService {
       );
     }
 
-    const providerAccountId = this.requireProviderAccountId(account);
-
-    await this.provider.removeAddress(providerAccountId, address);
+    await this.provider.deleteAccount(addressRecord.providerExternalId);
     await Promise.all([
       this.addresses.deleteProviderLink(addressRecord.id),
       this.addresses.delete(addressRecord.id),
@@ -138,12 +144,4 @@ export class AccountService {
     }
     return account;
   }
-
-  private requireProviderAccountId(account: MailAccount): string {
-    const id = account.providerAccountId;
-    if (!id) {
-      throw new UnprocessableEntityException('Account has no primary address');
-    }
-    return id;
-  }
 }

src/modules/account/domain/mail-account.domain.ts

@@ -30,8 +30,4 @@ export class MailAccount {
   get defaultAddress(): MailAddress | undefined {
     return this.addresses.find((a) => a.isDefault);
   }
-
-  get providerAccountId(): string | undefined {
-    return this.defaultAddress?.providerExternalId;
-  }
 }

src/modules/account/domain/mail-address.domain.ts

@@ -4,7 +4,7 @@ export interface MailAddressAttributes {
   address: string;
   domainId: string;
   isDefault: boolean;
-  providerExternalId: string | null;
+  providerExternalId: string;
   createdAt: Date;
   updatedAt: Date;
 }
@@ -15,7 +15,7 @@ export class MailAddress {
   readonly address!: string;
   readonly domainId!: string;
   readonly isDefault!: boolean;
-  readonly providerExternalId!: string | null;
+  readonly providerExternalId!: string;
   readonly createdAt!: Date;
   readonly updatedAt!: Date;
 

src/modules/account/domain/mail-domain.domain.ts

@@ -1,15 +1,20 @@
+export enum MailDomainStatus {
+  Active = 'active',
+  Inactive = 'inactive',
+}
+
 export interface MailDomainAttributes {
   id: string;
   domain: string;
-  status: string;
+  status: MailDomainStatus;
   createdAt: Date;
   updatedAt: Date;
 }
 
 export class MailDomain {
   readonly id!: string;
   readonly domain!: string;
-  readonly status!: string;
+  readonly status!: MailDomainStatus;
   readonly createdAt!: Date;
   readonly updatedAt!: Date;
 

src/modules/account/models/mail-provider-account.model.ts

@@ -15,6 +15,7 @@ import { MailAddressModel } from './mail-address.model.js';
 @Table({
   underscored: true,
   timestamps: true,
+  paranoid: true,
   tableName: 'mail_provider_accounts',
   indexes: [{ unique: true, fields: ['provider', 'external_id'] }],
 })
@@ -38,6 +39,9 @@ export class MailProviderAccountModel extends Model {
   @Column(DataType.STRING(255))
   declare externalId: string;
 
+  @Column(DataType.DATE)
+  declare deletedAt: Date | null;
+
   @BelongsTo(() => MailAddressModel)
   declare address: MailAddressModel;
 }

src/modules/account/repositories/address.repository.ts

@@ -11,13 +11,18 @@ import { MailProviderAccountModel } from '../models/mail-provider-account.model.
 export function toAddressAttributes(
   model: MailAddressModel,
 ): MailAddressAttributes {
+  const providerExternalId = model.providerAccount?.externalId;
+  if (!providerExternalId) {
+    throw new Error(`Address '${model.id}' has no provider link`);
+  }
+
   return {
     id: model.id,
     mailAccountId: model.mailAccountId,
     address: model.address,
     domainId: model.domainId,
     isDefault: model.isDefault,
-    providerExternalId: model.providerAccount?.externalId ?? null,
+    providerExternalId,
     createdAt: model.createdAt as Date,
     updatedAt: model.updatedAt as Date,
   };
@@ -67,9 +72,9 @@ export class AddressRepository {
     address: string;
     domainId: string;
     isDefault: boolean;
-  }): Promise<MailAddress> {
+  }): Promise<string> {
     const model = await this.addressModel.create(params);
-    return MailAddress.build(toAddressAttributes(model));
+    return model.id;
   }
 
   async delete(id: string): Promise<void> {
@@ -94,15 +99,4 @@ export class AddressRepository {
   async deleteProviderLink(mailAddressId: string): Promise<void> {
     await this.providerAccountModel.destroy({ where: { mailAddressId } });
   }
-
-  async updateAllProviderExternalIds(
-    mailAccountId: string,
-    newExternalId: string,
-  ): Promise<void> {
-    await this.sequelize.query(
-      `UPDATE mail_provider_accounts SET external_id = :newExternalId
-       WHERE mail_address_id IN (SELECT id FROM mail_addresses WHERE mail_account_id = :mailAccountId)`,
-      { replacements: { newExternalId, mailAccountId } },
-    );
-  }
 }

src/modules/account/repositories/domain.repository.ts

@@ -1,6 +1,6 @@
 import { Injectable } from '@nestjs/common';
 import { InjectModel } from '@nestjs/sequelize';
-import { MailDomain } from '../domain/mail-domain.domain.js';
+import { MailDomain, MailDomainStatus } from '../domain/mail-domain.domain.js';
 import { MailDomainModel } from '../models/mail-domain.model.js';
 
 @Injectable()
@@ -19,7 +19,7 @@ export class DomainRepository {
     return MailDomain.build({
       id: model.id,
       domain: model.domain,
-      status: model.status,
+      status: model.status as MailDomainStatus,
       createdAt: model.createdAt as Date,
       updatedAt: model.updatedAt as Date,
     });