Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 36 additions & 5 deletions apps/backend/src/donations/donations.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { DonationsController } from './donations.controller';
import { DonationsService } from './donations.service';
import { DonationsRepository } from './donations.repository';
import { CreateDonationDto } from './dtos/create-donation-dto';
import { CallHandler, ExecutionContext } from '@nestjs/common';
import { CurrentUserInterceptor } from '../interceptors/current-user.interceptor';
import {
DonationType,
RecurringInterval,
Expand Down Expand Up @@ -37,6 +39,7 @@ describe('DonationsController', () => {
findPublic: jest.fn(),
getTotalDonations: jest.fn(),
exportToCsv: jest.fn(),
getLapsedDonors: jest.fn(),
};

const mockRepository = {
Expand Down Expand Up @@ -74,11 +77,17 @@ describe('DonationsController', () => {
useValue: mockUsersService,
},
],
}).compile();

controller = module.get<DonationsController>(DonationsController);
service = module.get<DonationsService>(DonationsService);
repository = module.get<DonationsRepository>(DonationsRepository);
})
.overrideInterceptor(CurrentUserInterceptor)
.useValue({
intercept: (_context: ExecutionContext, next: CallHandler) =>
next.handle(),
})
.compile();

controller = module.get(DonationsController);
service = module.get(DonationsService);
repository = module.get(DonationsRepository);
});

afterEach(() => {
Expand Down Expand Up @@ -169,6 +178,28 @@ describe('DonationsController', () => {
});
});

describe('getLapsedDonors', () => {
it('should call service.getLapsedDonors with provided numMonths', async () => {
mockService.getLapsedDonors.mockResolvedValue({
emails: ['a@example.com'],
});

const result = await controller.getLapsedDonors(9);

expect(service.getLapsedDonors).toHaveBeenCalledWith(9);
expect(result).toEqual({ emails: ['a@example.com'] });
});

it('should default numMonths to 6 when not provided', async () => {
mockService.getLapsedDonors.mockResolvedValue({ emails: [] });

const result = await controller.getLapsedDonors(undefined);

expect(service.getLapsedDonors).toHaveBeenCalledWith(6);
expect(result).toEqual({ emails: [] });
});
});

describe('findPublic', () => {
it('should return public donations', async () => {
mockService.findPublic.mockResolvedValue([mockDomainDonation]);
Expand Down
34 changes: 34 additions & 0 deletions apps/backend/src/donations/donations.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,40 @@ export class DonationsController {
return stats;
}

@Get('lapsed')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For lapsed donor we also need auth since this endpoint deals with sensitive donor data. Fix is to add two lines under here:

@UseGuards(AuthGuard('jwt'))
@ApiBearerAuth()

otherwise, someone can call this endpoint and get info without auth.

@ApiOperation({
summary: 'get lapsed donor emails',
description:
'retrieve unique donor emails for donors who have not donated successfully in numMonths months and do not have recurring donations',
})
@ApiQuery({
name: 'numMonths',
required: false,
type: Number,
description: 'number of months since last successful donation',
example: 6,
})
@ApiResponse({
status: 200,
description: 'list of lapsed donor emails',
schema: {
type: 'object',
properties: {
emails: {
type: 'array',
items: { type: 'string' },
example: ['alice@example.com', 'bob@example.com'],
},
},
},
})
async getLapsedDonors(
@Query('numMonths', new ParseIntPipe({ optional: true }))
numMonths?: number,
): Promise<{ emails: string[] }> {
return this.donationsService.getLapsedDonors(numMonths ?? 6);
}

@Get()
@UseGuards(AuthGuard('jwt'))
@ApiBearerAuth()
Expand Down
149 changes: 149 additions & 0 deletions apps/backend/src/donations/donations.repository.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ describe('DonationsRepository', () => {
};

beforeEach(async () => {
const mockSubQueryBuilder = {
select: jest.fn().mockReturnThis(),
from: jest.fn().mockReturnThis(),
where: jest.fn().mockReturnThis(),
andWhere: jest.fn().mockReturnThis(),
getQuery: jest.fn().mockReturnValue('(subquery)'),
} as unknown as jest.Mocked<SelectQueryBuilder<unknown>>;
// Create mock query builder with all necessary methods
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment could be removed

mockQueryBuilder = {
where: jest.fn().mockReturnThis(),
Expand All @@ -43,9 +50,19 @@ describe('DonationsRepository', () => {
limit: jest.fn().mockReturnThis(),
select: jest.fn().mockReturnThis(),
addSelect: jest.fn().mockReturnThis(),

// added for findLapsedDonors
groupBy: jest.fn().mockReturnThis(),
having: jest.fn().mockReturnThis(),
setParameter: jest.fn().mockReturnThis(),
subQuery: jest.fn().mockReturnValue(mockSubQueryBuilder),

getManyAndCount: jest.fn(),
getMany: jest.fn(),
getRawOne: jest.fn(),

// used by findLapsedDonors
getRawMany: jest.fn(),
} as unknown as jest.Mocked<SelectQueryBuilder<Donation>>;

// Create mock TypeORM repository
Expand Down Expand Up @@ -217,6 +234,70 @@ describe('DonationsRepository', () => {
});
});

describe('findLapsedDonors', () => {
it('should filter to SUCCEEDED donations and apply cutoff via HAVING MAX(createdAt)', async () => {
mockQueryBuilder.getRawMany.mockResolvedValue([
{ email: 'alice@example.com' },
]);

const result = await repository.findLapsedDonors(6);

expect(mockTypeOrmRepo.createQueryBuilder).toHaveBeenCalledWith(
'donation',
);

expect(mockQueryBuilder.where).toHaveBeenCalledWith(
'donation.status = :succeededStatus',
{ succeededStatus: DonationStatus.SUCCEEDED },
);

expect(mockQueryBuilder.having).toHaveBeenCalledWith(
'MAX(donation.createdAt) < :cutoff',
expect.objectContaining({ cutoff: expect.any(Date) }),
);

expect(result).toEqual(['alice@example.com']);
});

it('should return unique, normalized emails (trim + lowercase)', async () => {
mockQueryBuilder.getRawMany.mockResolvedValue([
{ email: 'ALICE@EXAMPLE.COM' },
{ email: 'alice@example.com' },
{ email: ' Alice@Example.com ' },
]);

const result = await repository.findLapsedDonors(6);

expect(result).toEqual(['alice@example.com']);
});

it('should exclude donors with recurring donations using NOT EXISTS subquery', async () => {
mockQueryBuilder.getRawMany.mockResolvedValue([
{ email: 'x@example.com' },
]);

await repository.findLapsedDonors(6);

expect(mockQueryBuilder.subQuery).toHaveBeenCalled();
expect(mockQueryBuilder.andWhere).toHaveBeenCalledWith(
expect.stringContaining('NOT EXISTS'),
);
expect(mockQueryBuilder.setParameter).toHaveBeenCalledWith(
'recurringType',
DonationType.RECURRING,
);
});

it('should throw when numMonths is not positive', async () => {
await expect(repository.findLapsedDonors(0)).rejects.toThrow(
'numMonths must be a positive number',
);
await expect(repository.findLapsedDonors(-1)).rejects.toThrow(
'numMonths must be a positive number',
);
});
});

describe('searchByDonorNameOrEmail', () => {
it('should search by donor name or email with default limit', async () => {
const mockResults = [mockDonation];
Expand Down Expand Up @@ -379,6 +460,74 @@ describe('DonationsRepository', () => {
});
});

describe('findLapsedDonors', () => {
it('should filter to SUCCEEDED donations and apply cutoff via HAVING MAX(createdAt)', async () => {
mockQueryBuilder.getRawMany.mockResolvedValue([
{ email: 'alice@example.com' },
]);

const result = await repository.findLapsedDonors(6);

expect(mockTypeOrmRepo.createQueryBuilder).toHaveBeenCalledWith(
'donation',
);

expect(mockQueryBuilder.where).toHaveBeenCalledWith(
'donation.status = :succeededStatus',
{ succeededStatus: DonationStatus.SUCCEEDED },
);

expect(mockQueryBuilder.having).toHaveBeenCalledWith(
'MAX(donation.createdAt) < :cutoff',
expect.objectContaining({
cutoff: expect.any(Date),
}),
);

expect(result).toEqual(['alice@example.com']);
});

it('should return unique, normalized emails (trim + lowercase)', async () => {
mockQueryBuilder.getRawMany.mockResolvedValue([
{ email: 'ALICE@EXAMPLE.COM' },
{ email: 'alice@example.com' },
{ email: ' Alice@Example.com ' },
]);

const result = await repository.findLapsedDonors(6);

expect(result).toEqual(['alice@example.com']);
});

it('should exclude donors with recurring donations using NOT EXISTS subquery', async () => {
mockQueryBuilder.getRawMany.mockResolvedValue([
{ email: 'x@example.com' },
]);

await repository.findLapsedDonors(6);

expect(mockQueryBuilder.subQuery).toHaveBeenCalled();

expect(mockQueryBuilder.andWhere).toHaveBeenCalledWith(
expect.stringContaining('NOT EXISTS'),
);

expect(mockQueryBuilder.setParameter).toHaveBeenCalledWith(
'recurringType',
DonationType.RECURRING,
);
});

it('should throw when numMonths is not positive', async () => {
await expect(repository.findLapsedDonors(0)).rejects.toThrow(
'numMonths must be a positive number',
);
await expect(repository.findLapsedDonors(-2)).rejects.toThrow(
'numMonths must be a positive number',
);
});
});

describe('deleteById', () => {
it('should delete donation by id', async () => {
mockTypeOrmRepo.delete.mockResolvedValue({
Expand Down
41 changes: 41 additions & 0 deletions apps/backend/src/donations/donations.repository.ts
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QueryBuilder is stateful, so when we are using it qb for the main query builder we shouldn't reuse it again for the subQuery builder. This is bc it will mutate the same instance from a dirty state. It works in tests but in a real DB there can be issues. Better to be safe and use a separate this.repository.createQueryBuilder() call for the subquery and another clean one for the main query.

Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,47 @@ export class DonationsRepository {
);
}

/**
* Find unique donor emails that have not made a successful donation in `numMonths` months,
* and do NOT have any recurring donation record.
*
* - Only considers SUCCEEDED donations for "last donated" logic
* - Excludes donors with at least one recurring donation (any status)
*/
async findLapsedDonors(numMonths: number): Promise<string[]> {
if (!Number.isFinite(numMonths) || numMonths <= 0) {
throw new Error('numMonths must be a positive number');
}

const cutoff = new Date();
cutoff.setMonth(cutoff.getMonth() - numMonths);

const qb = this.repository.createQueryBuilder('donation');

const recurringExistsSubquery = qb
.subQuery()
.select('1')
.from(Donation, 'recurringDonation')
.where('LOWER(recurringDonation.email) = LOWER(donation.email)')
.andWhere('recurringDonation.donationType = :recurringType')
.getQuery();

const rows = await qb
.select('LOWER(donation.email)', 'email')
.where('donation.status = :succeededStatus', {
succeededStatus: DonationStatus.SUCCEEDED,
})
.andWhere('donation.email IS NOT NULL')
.andWhere("donation.email <> ''")
.andWhere(`NOT EXISTS ${recurringExistsSubquery}`)
.setParameter('recurringType', DonationType.RECURRING)
.groupBy('LOWER(donation.email)')
.having('MAX(donation.createdAt) < :cutoff', { cutoff })
.getRawMany<{ email: string }>();

return [...new Set(rows.map((r) => r.email.trim().toLowerCase()))];
}

/**
* Delete a donation by ID (admin-only destructive operation)
*/
Expand Down
Loading