docs: consolidate audit and analysis reports into docs/audits/
Move 36 root-level audit/analysis documents and 7 web app audit documents into docs/audits/ directory to declutter the project root. Remove stale EXPLORATION_SUMMARY.txt. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Ho Ngoc Hai
517
docs/audits/DETAILED_HANDLER_COMPARISON.md
Normal file
517
docs/audits/DETAILED_HANDLER_COMPARISON.md
Normal file
@@ -0,0 +1,517 @@
|
||||
# Detailed Handler Comparison & Code Patterns
|
||||
|
||||
## File Structure Comparison
|
||||
|
||||
### Tested Handler Pattern: approve-listing
|
||||
```
|
||||
approve-listing/
|
||||
├── approve-listing.command.ts (simple class)
|
||||
├── approve-listing.handler.ts (the handler to test)
|
||||
└── (no query.ts - it's a Command, not a Query)
|
||||
|
||||
Test file:
|
||||
└── approve-listing.handler.spec.ts
|
||||
```
|
||||
|
||||
### Untested Handler: reject-listing
|
||||
```
|
||||
reject-listing/
|
||||
├── reject-listing.command.ts (simple class)
|
||||
├── reject-listing.handler.ts (NEEDS TEST)
|
||||
└── (no query.ts - it's a Command, not a Query)
|
||||
|
||||
Test file:
|
||||
└── ❌ MISSING: reject-listing.handler.spec.ts
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Side-by-Side Handler Comparison
|
||||
|
||||
### APPROVE Listing Handler:
|
||||
```typescript
|
||||
@CommandHandler(ApproveListingCommand)
|
||||
export class ApproveListingHandler implements ICommandHandler<ApproveListingCommand> {
|
||||
constructor(
|
||||
@Inject(LISTING_REPOSITORY) private readonly listingRepo: IListingRepository,
|
||||
private readonly eventBus: EventBus,
|
||||
) {}
|
||||
|
||||
async execute(command: ApproveListingCommand): Promise<ApproveLis tingResult> {
|
||||
// 1. Find listing
|
||||
const listing = await this.listingRepo.findById(command.listingId);
|
||||
if (!listing) {
|
||||
throw new NotFoundException('Listing không tồn tại');
|
||||
}
|
||||
|
||||
// 2. Check status
|
||||
if (listing.status !== 'PENDING_REVIEW') {
|
||||
throw new ValidationException(
|
||||
`Listing đang ở trạng thái ${listing.status}, chỉ có thể phê duyệt listing đang chờ duyệt`,
|
||||
{ currentStatus: listing.status },
|
||||
);
|
||||
}
|
||||
|
||||
// 3. Apply domain logic
|
||||
listing.approve(command.notes);
|
||||
|
||||
// 4. Persist
|
||||
await this.listingRepo.update(listing);
|
||||
|
||||
// 5. Publish event
|
||||
this.eventBus.publish(
|
||||
new ListingApprovedEvent(listing.id, command.adminId, command.notes),
|
||||
);
|
||||
|
||||
// 6. Return result
|
||||
return {
|
||||
listingId: listing.id,
|
||||
status: 'ACTIVE',
|
||||
message: 'Listing đã được phê duyệt',
|
||||
};
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### REJECT Listing Handler (virtually identical pattern):
|
||||
```typescript
|
||||
@CommandHandler(RejectListingCommand)
|
||||
export class RejectListingHandler implements ICommandHandler<RejectListingCommand> {
|
||||
constructor(
|
||||
@Inject(LISTING_REPOSITORY) private readonly listingRepo: IListingRepository,
|
||||
private readonly eventBus: EventBus,
|
||||
) {}
|
||||
|
||||
async execute(command: RejectListingCommand): Promise<RejectListingResult> {
|
||||
// 1. Find listing
|
||||
const listing = await this.listingRepo.findById(command.listingId);
|
||||
if (!listing) {
|
||||
throw new NotFoundException('Listing không tồn tại');
|
||||
}
|
||||
|
||||
// 2. Check status (same as approve!)
|
||||
if (listing.status !== 'PENDING_REVIEW') {
|
||||
throw new ValidationException(
|
||||
`Listing đang ở trạng thái ${listing.status}, chỉ có thể từ chối listing đang chờ duyệt`,
|
||||
{ currentStatus: listing.status },
|
||||
);
|
||||
}
|
||||
|
||||
// 3. Apply domain logic (different method: reject instead of approve)
|
||||
listing.reject(command.reason);
|
||||
|
||||
// 4. Persist
|
||||
await this.listingRepo.update(listing);
|
||||
|
||||
// 5. Publish event (different event type)
|
||||
this.eventBus.publish(
|
||||
new ListingRejectedEvent(listing.id, command.adminId, command.reason),
|
||||
);
|
||||
|
||||
// 6. Return result (different status)
|
||||
return {
|
||||
listingId: listing.id,
|
||||
status: 'REJECTED',
|
||||
message: 'Listing đã bị từ chối',
|
||||
};
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Differences:
|
||||
| Aspect | Approve | Reject |
|
||||
|--------|---------|--------|
|
||||
| Domain Method | `listing.approve()` | `listing.reject()` |
|
||||
| Event | `ListingApprovedEvent` | `ListingRejectedEvent` |
|
||||
| Result Status | `'ACTIVE'` | `'REJECTED'` |
|
||||
| Result Message | `'Listing đã được phê duyệt'` | `'Listing đã bị từ chối'` |
|
||||
|
||||
---
|
||||
|
||||
## Test Code Walkthrough
|
||||
|
||||
### ApproveListingHandler Test:
|
||||
|
||||
```typescript
|
||||
describe('ApproveListingHandler', () => {
|
||||
let handler: ApproveListingHandler;
|
||||
let mockListingRepo: { [K in keyof IListingRepository]: ReturnType<typeof vi.fn> };
|
||||
let mockEventBus: { publish: ReturnType<typeof vi.fn> };
|
||||
|
||||
// SETUP: Create fresh mocks for each test
|
||||
beforeEach(() => {
|
||||
mockListingRepo = {
|
||||
findById: vi.fn(),
|
||||
findByIdWithProperty: vi.fn(),
|
||||
save: vi.fn(),
|
||||
update: vi.fn(),
|
||||
search: vi.fn(),
|
||||
findByStatus: vi.fn(),
|
||||
findBySellerId: vi.fn(),
|
||||
};
|
||||
|
||||
mockEventBus = { publish: vi.fn() };
|
||||
|
||||
// Instantiate handler with mocks
|
||||
handler = new ApproveListingHandler(
|
||||
mockListingRepo as any,
|
||||
mockEventBus as any,
|
||||
);
|
||||
});
|
||||
|
||||
// TEST 1: Happy Path - Successfully approve
|
||||
it('approves a pending listing successfully', async () => {
|
||||
// Arrange: Create a listing entity in PENDING_REVIEW state
|
||||
const listing = createPendingListing();
|
||||
mockListingRepo.findById.mockResolvedValue(listing);
|
||||
mockListingRepo.update.mockResolvedValue(undefined);
|
||||
|
||||
// Act: Execute the command
|
||||
const command = new ApproveListingCommand('listing-1', 'admin-1', 'Looks good');
|
||||
const result = await handler.execute(command);
|
||||
|
||||
// Assert: Verify result
|
||||
expect(result.status).toBe('ACTIVE');
|
||||
expect(result.listingId).toBe('listing-1');
|
||||
|
||||
// Assert: Verify side effects
|
||||
expect(mockListingRepo.update).toHaveBeenCalledTimes(1);
|
||||
expect(mockEventBus.publish).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// TEST 2: Error - Listing not found
|
||||
it('throws NotFoundException when listing does not exist', async () => {
|
||||
// Arrange: Mock returns null (not found)
|
||||
mockListingRepo.findById.mockResolvedValue(null);
|
||||
|
||||
// Act & Assert: Expect exception
|
||||
const command = new ApproveListingCommand('nonexistent', 'admin-1');
|
||||
await expect(handler.execute(command)).rejects.toThrow('Listing không tồn tại');
|
||||
});
|
||||
|
||||
// TEST 3: Error - Wrong status
|
||||
it('throws ValidationException when listing is not pending review', async () => {
|
||||
// Arrange: Create listing NOT in PENDING_REVIEW status
|
||||
const price = Price.create(500_000_000n).unwrap();
|
||||
const listing = ListingEntity.createNew(
|
||||
'listing-1', 'prop-1', 'seller-1', 'SALE', price, 80,
|
||||
);
|
||||
listing.clearDomainEvents();
|
||||
mockListingRepo.findById.mockResolvedValue(listing);
|
||||
|
||||
// Act & Assert: Expect exception
|
||||
const command = new ApproveListingCommand('listing-1', 'admin-1');
|
||||
await expect(handler.execute(command)).rejects.toThrow(/trạng thái/);
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
### How to adapt for RejectListingHandler:
|
||||
|
||||
1. **Import changes:**
|
||||
```typescript
|
||||
import { RejectListingCommand } from '../commands/reject-listing/reject-listing.command';
|
||||
import { RejectListingHandler } from '../commands/reject-listing/reject-listing.handler';
|
||||
// Keep everything else the same
|
||||
```
|
||||
|
||||
2. **Test 1 (Happy path) changes:**
|
||||
```typescript
|
||||
it('rejects a pending listing successfully', async () => {
|
||||
const listing = createPendingListing();
|
||||
mockListingRepo.findById.mockResolvedValue(listing);
|
||||
mockListingRepo.update.mockResolvedValue(undefined);
|
||||
|
||||
const command = new RejectListingCommand('listing-1', 'admin-1', 'Too many issues');
|
||||
const result = await handler.execute(command);
|
||||
|
||||
expect(result.status).toBe('REJECTED'); // Changed from 'ACTIVE'
|
||||
expect(result.message).toContain('từ chối'); // Changed assertion
|
||||
expect(mockListingRepo.update).toHaveBeenCalledTimes(1);
|
||||
expect(mockEventBus.publish).toHaveBeenCalled();
|
||||
});
|
||||
```
|
||||
|
||||
3. **Tests 2 & 3 remain almost identical** (only import names change)
|
||||
|
||||
---
|
||||
|
||||
## Query Handler Comparison
|
||||
|
||||
### Tested Query Handler: get-dashboard-stats
|
||||
```typescript
|
||||
@QueryHandler(GetDashboardStatsQuery)
|
||||
export class GetDashboardStatsHandler implements IQueryHandler<GetDashboardStatsQuery> {
|
||||
constructor(
|
||||
@Inject(ADMIN_QUERY_REPOSITORY) private readonly adminQueryRepo: IAdminQueryRepository,
|
||||
) {}
|
||||
|
||||
async execute(query: GetDashboardStatsQuery): Promise<DashboardStats> {
|
||||
return this.adminQueryRepo.getDashboardStats();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Untested Query Handler: get-revenue-stats
|
||||
```typescript
|
||||
@QueryHandler(GetRevenueStatsQuery)
|
||||
export class GetRevenueStatsHandler implements IQueryHandler<GetRevenueStatsQuery> {
|
||||
constructor(
|
||||
@Inject(ADMIN_QUERY_REPOSITORY) private readonly adminQueryRepo: IAdminQueryRepository,
|
||||
) {}
|
||||
|
||||
async execute(query: GetRevenueStatsQuery): Promise<RevenueStatsItem[]> {
|
||||
// KEY DIFFERENCE: Passes query params to repo method
|
||||
return this.adminQueryRepo.getRevenueStats(query.startDate, query.endDate, query.groupBy);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Query Handler Test Pattern:
|
||||
```typescript
|
||||
describe('GetRevenueStatsHandler', () => {
|
||||
let handler: GetRevenueStatsHandler;
|
||||
let mockAdminQueryRepo: { [K in keyof IAdminQueryRepository]: ReturnType<typeof vi.fn> };
|
||||
|
||||
beforeEach(() => {
|
||||
mockAdminQueryRepo = {
|
||||
getModerationQueue: vi.fn(),
|
||||
getDashboardStats: vi.fn(),
|
||||
getRevenueStats: vi.fn(), // This one will be tested
|
||||
getUsers: vi.fn(),
|
||||
};
|
||||
|
||||
handler = new GetRevenueStatsHandler(mockAdminQueryRepo as any);
|
||||
});
|
||||
|
||||
it('returns revenue stats for date range', async () => {
|
||||
// Arrange: Mock data
|
||||
const mockStats: RevenueStatsItem[] = [
|
||||
{
|
||||
period: '2024-04',
|
||||
totalRevenue: 50000000n,
|
||||
subscriptionRevenue: 30000000n,
|
||||
listingFeeRevenue: 15000000n,
|
||||
featuredListingRevenue: 5000000n,
|
||||
transactionCount: 125,
|
||||
},
|
||||
];
|
||||
mockAdminQueryRepo.getRevenueStats.mockResolvedValue(mockStats);
|
||||
|
||||
// Act
|
||||
const startDate = new Date('2024-04-01');
|
||||
const endDate = new Date('2024-04-30');
|
||||
const query = new GetRevenueStatsQuery(startDate, endDate, 'month');
|
||||
const result = await handler.execute(query);
|
||||
|
||||
// Assert: Verify result
|
||||
expect(result).toEqual(mockStats);
|
||||
|
||||
// Assert: Verify params passed correctly
|
||||
expect(mockAdminQueryRepo.getRevenueStats).toHaveBeenCalledWith(
|
||||
startDate,
|
||||
endDate,
|
||||
'month'
|
||||
);
|
||||
expect(mockAdminQueryRepo.getRevenueStats).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('supports day grouping', async () => {
|
||||
mockAdminQueryRepo.getRevenueStats.mockResolvedValue([]);
|
||||
|
||||
const query = new GetRevenueStatsQuery(
|
||||
new Date('2024-04-01'),
|
||||
new Date('2024-04-30'),
|
||||
'day' // Changed parameter
|
||||
);
|
||||
await handler.execute(query);
|
||||
|
||||
expect(mockAdminQueryRepo.getRevenueStats).toHaveBeenCalledWith(
|
||||
expect.any(Date),
|
||||
expect.any(Date),
|
||||
'day' // Verify 'day' was passed
|
||||
);
|
||||
});
|
||||
|
||||
it('returns empty array when no data', async () => {
|
||||
mockAdminQueryRepo.getRevenueStats.mockResolvedValue([]);
|
||||
|
||||
const query = new GetRevenueStatsQuery(
|
||||
new Date('2024-01-01'),
|
||||
new Date('2024-01-01')
|
||||
);
|
||||
const result = await handler.execute(query);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
expect(result.length).toBe(0);
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Listener Comparison
|
||||
|
||||
### UserBannedListener (Tested):
|
||||
```typescript
|
||||
@Injectable()
|
||||
export class UserBannedListener {
|
||||
constructor(
|
||||
private readonly commandBus: CommandBus,
|
||||
private readonly prisma: PrismaService,
|
||||
private readonly logger: LoggerService,
|
||||
) {}
|
||||
|
||||
@OnEvent('user.banned', { async: true })
|
||||
async handle(event: UserBannedEvent): Promise<void> {
|
||||
this.logger.log(`Handling user.banned for user ${event.aggregateId}`, 'UserBannedListener');
|
||||
|
||||
// Deactivate listings
|
||||
const deactivated = await this.prisma.listing.updateMany({
|
||||
where: {
|
||||
sellerId: event.aggregateId,
|
||||
status: { in: ['ACTIVE', 'PENDING_REVIEW', 'DRAFT'] },
|
||||
},
|
||||
data: { status: 'EXPIRED' },
|
||||
});
|
||||
|
||||
this.logger.log(
|
||||
`Deactivated ${deactivated.count} listings for banned user ${event.aggregateId}`,
|
||||
'UserBannedListener',
|
||||
);
|
||||
|
||||
// Send email notification
|
||||
const user = await this.prisma.user.findUnique({
|
||||
where: { id: event.aggregateId },
|
||||
select: { id: true, email: true },
|
||||
});
|
||||
|
||||
if (user?.email) {
|
||||
await this.commandBus.execute(
|
||||
new SendNotificationCommand(
|
||||
user.id,
|
||||
'EMAIL',
|
||||
'user.banned',
|
||||
{ reason: event.reason },
|
||||
user.email,
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### UserDeactivatedListener (Untested):
|
||||
```typescript
|
||||
@Injectable()
|
||||
export class UserDeactivatedListener {
|
||||
constructor(
|
||||
private readonly prisma: PrismaService,
|
||||
private readonly logger: LoggerService,
|
||||
) {}
|
||||
|
||||
@OnEvent('user.deactivated', { async: true })
|
||||
async handle(event: UserDeactivatedEvent): Promise<void> {
|
||||
this.logger.log(`Handling user.deactivated for user ${event.aggregateId}`, 'UserDeactivatedListener');
|
||||
|
||||
// Similar to UserBannedListener but:
|
||||
// 1. NO CommandBus (simpler)
|
||||
// 2. NO email notification
|
||||
// 3. Different status list: ['ACTIVE', 'PENDING_REVIEW'] (no DRAFT)
|
||||
const deactivated = await this.prisma.listing.updateMany({
|
||||
where: {
|
||||
sellerId: event.aggregateId,
|
||||
status: { in: ['ACTIVE', 'PENDING_REVIEW'] },
|
||||
},
|
||||
data: { status: 'EXPIRED' },
|
||||
});
|
||||
|
||||
this.logger.log(
|
||||
`Expired ${deactivated.count} listings for deactivated user ${event.aggregateId}`,
|
||||
'UserDeactivatedListener',
|
||||
);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Key Differences:
|
||||
| Aspect | UserBanned | UserDeactivated |
|
||||
|--------|-----------|-----------------|
|
||||
| Event name | `'user.banned'` | `'user.deactivated'` |
|
||||
| Has CommandBus | ✅ Yes (for email) | ❌ No |
|
||||
| Status list | `['ACTIVE', 'PENDING_REVIEW', 'DRAFT']` | `['ACTIVE', 'PENDING_REVIEW']` |
|
||||
| Sends notification | ✅ Yes (email) | ❌ No |
|
||||
| Complexity | Higher | Lower |
|
||||
|
||||
### Listener Test Pattern (simplified for UserDeactivated):
|
||||
```typescript
|
||||
describe('UserDeactivatedListener', () => {
|
||||
let listener: UserDeactivatedListener;
|
||||
let mockPrisma: {
|
||||
listing: { updateMany: ReturnType<typeof vi.fn> };
|
||||
};
|
||||
let mockLogger: { log: ReturnType<typeof vi.fn> };
|
||||
|
||||
beforeEach(() => {
|
||||
mockPrisma = {
|
||||
listing: { updateMany: vi.fn().mockResolvedValue({ count: 5 }) },
|
||||
};
|
||||
mockLogger = { log: vi.fn() };
|
||||
|
||||
listener = new UserDeactivatedListener(mockPrisma as any, mockLogger as any);
|
||||
});
|
||||
|
||||
it('expires all active and pending review listings for deactivated user', async () => {
|
||||
await listener.handle({
|
||||
aggregateId: 'user-123',
|
||||
eventName: 'user.deactivated',
|
||||
occurredAt: new Date(),
|
||||
});
|
||||
|
||||
expect(mockPrisma.listing.updateMany).toHaveBeenCalledWith({
|
||||
where: {
|
||||
sellerId: 'user-123',
|
||||
status: { in: ['ACTIVE', 'PENDING_REVIEW'] },
|
||||
},
|
||||
data: { status: 'EXPIRED' },
|
||||
});
|
||||
});
|
||||
|
||||
it('logs handling start and result', async () => {
|
||||
await listener.handle({
|
||||
aggregateId: 'user-123',
|
||||
eventName: 'user.deactivated',
|
||||
occurredAt: new Date(),
|
||||
});
|
||||
|
||||
expect(mockLogger.log).toHaveBeenCalledTimes(2);
|
||||
expect(mockLogger.log).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.stringContaining('user-123'),
|
||||
'UserDeactivatedListener'
|
||||
);
|
||||
expect(mockLogger.log).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.stringContaining('Expired 5 listings'),
|
||||
'UserDeactivatedListener'
|
||||
);
|
||||
});
|
||||
|
||||
it('handles zero listings case', async () => {
|
||||
mockPrisma.listing.updateMany.mockResolvedValue({ count: 0 });
|
||||
|
||||
await listener.handle({
|
||||
aggregateId: 'user-xyz',
|
||||
eventName: 'user.deactivated',
|
||||
occurredAt: new Date(),
|
||||
});
|
||||
|
||||
expect(mockLogger.log).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.stringContaining('Expired 0 listings'),
|
||||
'UserDeactivatedListener'
|
||||
);
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
Reference in New Issue
Block a user