Merged PR 325: テストを最新化(アカウント登録)

## 概要
[Task2397: テストを最新化(アカウント登録)](https://paruru.nds-tyo.co.jp:8443/tfs/ReciproCollection/fa4924a4-d079-4fab-9fb5-a9a11eb205f0/_workitems/edit/2397)

- アカウント登録のテストを最新化(元が無かったので実質新規追加)
- テスト時、外部サービスへの依存をMockに差し替える際のモデルケースを追加
  - `Object.defineProperty` を使って強引にメンバ変数をMock用オブジェクトへ上書きする方式
- migrationとEntityの定義内容が食い違っていた部分をmigration側に合わせるよう修正
  - 実質的にEntityの属性指定は通常実行時には使われないと思われたため、部分的にはテスト用に修正(※意図はコメント済)

## レビューポイント
- テスト時、AdB2cServiceやSendGridService等のDB以外のサービスをMockに差し替えて動かしているが、この方式に問題や懸念点はなさそうか
  - 例えば、`/app/dictation_server/src/features/users/test/utility.ts` に定義されている `makeTestingModuleWithAdb2c` のように構築時にoverrideする形式が既にあるが、そちらの方式でやるべきでは? 等
  - 上記方式ではなく `Object.defineProperty` で良いのでは?という提案を行っているのは、以下の理由
     - モジュール構築時に指定する方法だと依存してるサービス数によっては構築時の指定パラメータが膨大になってしまう懸念がある(3~4個のサービスと連携している場合、`makeTestingModuleWithXXX(...)` の引数指定がえらいことになりそう)
     - `Object.defineProperty` 形式だとテストケース内で必要なサービスを1個1個上書きしていく事が可能なので、汎用関数として切り出して各Serviceで利用できる
- 現状のテストケースとして妥当か
  - 例: SendGridの送信に失敗した場合は最終的にはリカバリを行う実装が入るが、現段階ではリカバリ処理がないためテスト記述なし
- Entityの属性を修正したが問題なさそうか、特にテスト用に指定した箇所があるが問題ないか

## 動作確認状況
- npm run testで成功
This commit is contained in:
湯本 開 2023-08-14 06:52:13 +00:00
parent 356f5fe346
commit 415a76b6bf
6 changed files with 301 additions and 24 deletions

View File

@ -0,0 +1,63 @@
import { ConflictError } from '../../gateways/adb2c/adb2c.service';
/**
* adB2cServiceのモックを作成してTServiceが依存するサービス(adB2cService)
* serviceに指定するオブジェクトは`adB2cService: AdB2cService`
* @param service TService
* @param overrides adB2cServiceの各種メソッドのモックが返す値
*/
export const overrideAdB2cService = <TService>(
service: TService,
overrides: {
createUser?: (
email: string,
password: string,
username: string,
) => Promise<{ sub: string } | ConflictError>;
},
): void => {
const { createUser } = overrides;
Object.defineProperty(service, 'adB2cService', {
value: {
createUser: createUser ?? jest.fn().mockResolvedValue({ sub: 'dummy' }),
},
});
};
/**
* sendgridServiceのモックを作成してTServiceが依存するサービス(sendgridService)
* serviceに指定するオブジェクトは`sendgridService: SendgridService`
* @param service TService
* @param overrides sendgridServiceの各種メソッドのモックが返す値
*/
export const overrideSendgridService = <TService>(
service: TService,
overrides: {
createMailContentFromEmailConfirm?: (
accountId: number,
userId: number,
email: string,
) => Promise<{ subject: string; text: string; html: string }>;
sendMail?: (
to: string,
from: string,
subject: string,
text: string,
html: string,
) => Promise<void>;
},
): void => {
const { createMailContentFromEmailConfirm, sendMail } = overrides;
Object.defineProperty(service, 'sendgridService', {
value: {
createMailContentFromEmailConfirm:
createMailContentFromEmailConfirm ??
jest.fn().mockResolvedValue({
subject: 'dummySubject',
text: 'dummyText',
html: 'dummyHtml',
}),
sendMail: sendMail ?? jest.fn().mockResolvedValue(undefined),
},
});
};

View File

@ -16,6 +16,10 @@ import {
createLicenseOrder,
createUser,
createLicenseSetExpiryDateAndStatus,
getAccount,
getUser,
getAccounts,
getUsers,
} from './test/utility';
import { DataSource } from 'typeorm';
import { makeTestingModule } from '../../common/test/modules';
@ -23,6 +27,176 @@ import { AccountsService } from './accounts.service';
import { makeContext } from '../../common/log';
import { TIERS } from '../../constants';
import { License } from '../../repositories/licenses/entity/license.entity';
import {
overrideAdB2cService,
overrideSendgridService,
} from '../../common/test/overrides';
describe('createAccount', () => {
let source: DataSource = null;
beforeEach(async () => {
source = new DataSource({
type: 'sqlite',
database: ':memory:',
logging: false,
entities: [__dirname + '/../../**/*.entity{.ts,.js}'],
synchronize: true, // trueにすると自動的にmigrationが行われるため注意
});
return source.initialize();
});
afterEach(async () => {
await source.destroy();
source = null;
});
it('アカウントを作成できる', async () => {
const module = await makeTestingModule(source);
const service = module.get<AccountsService>(AccountsService);
const externalId = 'test_external_id';
const companyName = 'test_company_name';
const country = 'US';
const dealerAccountId = 1;
const email = 'dummy@dummy.dummy';
const password = 'dummy_password';
const username = 'dummy_username';
const role = 'admin none';
const acceptedTermsVersion = '1.0.0';
overrideAdB2cService(service, {
createUser: async () => {
return { sub: externalId };
},
});
overrideSendgridService(service, {});
const { accountId, externalUserId, userId } = await service.createAccount(
companyName,
country,
dealerAccountId,
email,
password,
username,
role,
acceptedTermsVersion,
);
// 作成したアカウントのIDが返ってくるか確認
expect(accountId).toBe(1);
expect(externalUserId).toBe(externalId);
expect(userId).toBe(1);
// DB内が想定通りになっているか確認
const account = await getAccount(source, accountId);
const user = await getUser(source, externalUserId);
expect(account.company_name).toBe(companyName);
expect(account.country).toBe(country);
expect(account.parent_account_id).toBe(dealerAccountId);
expect(account.tier).toBe(TIERS.TIER5);
expect(account.primary_admin_user_id).toBe(user.id);
expect(account.secondary_admin_user_id).toBe(null);
expect(user.accepted_terms_version).toBe(acceptedTermsVersion);
expect(user.account_id).toBe(accountId);
expect(user.role).toBe(role);
});
it('アカウントを作成がAzure AD B2Cへの通信失敗によって失敗すると500エラーが発生する', async () => {
const module = await makeTestingModule(source);
const service = module.get<AccountsService>(AccountsService);
overrideAdB2cService(service, {
createUser: async () => {
throw new Error();
},
});
overrideSendgridService(service, {});
const companyName = 'test_company_name';
const country = 'US';
const dealerAccountId = 1;
const email = 'dummy@dummy.dummy';
const password = 'dummy_password';
const username = 'dummy_username';
const role = 'admin none';
const acceptedTermsVersion = '1.0.0';
try {
await service.createAccount(
companyName,
country,
dealerAccountId,
email,
password,
username,
role,
acceptedTermsVersion,
);
} catch (e) {
if (e instanceof HttpException) {
expect(e.getStatus()).toBe(HttpStatus.INTERNAL_SERVER_ERROR);
expect(e.getResponse()).toEqual(makeErrorResponse('E009999'));
} else {
expect(true).toBe(false); // ここには来てはいけない
}
}
// DB内が想定通りになっているか確認
const accounts = await getAccounts(source);
expect(accounts.length).toBe(0);
const users = await getUsers(source);
expect(users.length).toBe(0);
});
it('アカウントを作成がメールアドレス重複によって失敗すると400エラーが発生する', async () => {
const module = await makeTestingModule(source);
const service = module.get<AccountsService>(AccountsService);
overrideAdB2cService(service, {
createUser: async () => {
// EmailのConflictエラーを返す
return { reason: 'email', message: 'dummy' };
},
});
overrideSendgridService(service, {});
const companyName = 'test_company_name';
const country = 'US';
const dealerAccountId = 1;
const email = 'dummy@dummy.dummy';
const password = 'dummy_password';
const username = 'dummy_username';
const role = 'admin none';
const acceptedTermsVersion = '1.0.0';
try {
await service.createAccount(
companyName,
country,
dealerAccountId,
email,
password,
username,
role,
acceptedTermsVersion,
);
} catch (e) {
if (e instanceof HttpException) {
expect(e.getStatus()).toBe(HttpStatus.BAD_REQUEST);
expect(e.getResponse()).toEqual(makeErrorResponse('E010301'));
} else {
expect(true).toBe(false); // ここには来てはいけない
}
}
// DB内が想定通りになっているか確認
const accounts = await getAccounts(source);
expect(accounts.length).toBe(0);
const users = await getUsers(source);
expect(users.length).toBe(0);
});
});
describe('AccountsService', () => {
it('アカウントに紐づくライセンス情報を取得する', async () => {
@ -505,7 +679,7 @@ describe('getPartnerAccount', () => {
);
// 有効期限が14日後のライセンスを追加5ライセンス
let expiryDate = new Date();
const expiryDate = new Date();
expiryDate.setDate(expiryDate.getDate() + 14);
expiryDate.setHours(23, 59, 59, 999);
for (let i = 0; i < 5; i++) {

View File

@ -10,11 +10,7 @@ import {
} from '../../gateways/adb2c/adb2c.service';
import { Account } from '../../repositories/accounts/entity/account.entity';
import { User } from '../../repositories/users/entity/user.entity';
import {
LICENSE_EXPIRATION_THRESHOLD_DAYS,
TIERS,
USER_ROLES,
} from '../../constants';
import { TIERS, USER_ROLES } from '../../constants';
import { makeErrorResponse } from '../../common/error/makeErrorResponse';
import {
TypistGroup,

View File

@ -30,6 +30,50 @@ export const createAccount = async (
return { accountId: account.id };
};
/**
* ユーティリティ: 指定IDのアカウントを取得する
* @param dataSource
* @param id ID
* @returns
*/
export const getAccount = async (dataSource: DataSource, id: number) => {
return await dataSource.getRepository(Account).findOne({
where: { id: id },
});
};
/**
* ユーティリティ: すべてのアカウントを取得する
* @param dataSource
* @returns 
*/
export const getAccounts = async (
dataSource: DataSource,
): Promise<Account[]> => {
return await dataSource.getRepository(Account).find();
};
/**
* ユーティリティ: 指定ExternalIdのアカウントを取得する
* @param dataSource
* @param externalId ID
* @returns
*/
export const getUser = async (dataSource: DataSource, externalId: string) => {
return await dataSource.getRepository(User).findOne({
where: { external_id: externalId },
});
};
/**
* ユーティリティ: すべてのユーザーを取得する
* @param dataSource
* @returns
*/
export const getUsers = async (dataSource: DataSource): Promise<User[]> => {
return await dataSource.getRepository(User).find();
};
export const createLicense = async (
datasource: DataSource,
accountId: number,

View File

@ -22,16 +22,16 @@ export class Account {
@Column()
country: string;
@Column()
@Column({ default: false })
delegation_permission: boolean;
@Column()
@Column({ default: false })
locked: boolean;
@Column()
company_name: string;
@Column()
@Column({ default: false })
verified: boolean;
@Column({ nullable: true })
@ -43,16 +43,16 @@ export class Account {
@Column({ nullable: true })
deleted_at?: Date;
@Column()
created_by: string;
@Column({ nullable: true })
created_by?: string;
@CreateDateColumn()
@CreateDateColumn({ default: () => "datetime('now', 'localtime')" }) // defaultはSQLite用設定値.本番用は別途migrationで設定
created_at: Date;
@Column()
updated_by: string;
@Column({ nullable: true })
updated_by?: string;
@UpdateDateColumn()
@UpdateDateColumn({ default: () => "datetime('now', 'localtime')" }) // defaultはSQLite用設定値.本番用は別途migrationで設定
updated_at: Date;
@OneToMany(() => User, (user) => user.id)

View File

@ -33,16 +33,16 @@ export class User {
@Column({ nullable: true })
accepted_terms_version?: string;
@Column()
@Column({ default: false })
email_verified: boolean;
@Column()
@Column({ default: true })
auto_renew: boolean;
@Column()
@Column({ default: true })
license_alert: boolean;
@Column()
@Column({ default: true })
notification: boolean;
@Column({ nullable: true })
@ -57,16 +57,16 @@ export class User {
@Column({ nullable: true })
deleted_at?: Date;
@Column()
@Column({ nullable: true })
created_by: string;
@CreateDateColumn()
@CreateDateColumn({ default: () => "datetime('now', 'localtime')" }) // defaultはSQLite用設定値.本番用は別途migrationで設定
created_at: Date;
@Column()
updated_by: string;
@Column({ nullable: true })
updated_by?: string;
@UpdateDateColumn()
@UpdateDateColumn({ default: () => "datetime('now', 'localtime')" }) // defaultはSQLite用設定値.本番用は別途migrationで設定
updated_at: Date;
@ManyToOne(() => Account, (account) => account.user)