diff --git a/src/bounties/bounties.service.spec.ts b/src/bounties/bounties.service.spec.ts index 2450d71..9c1be21 100644 --- a/src/bounties/bounties.service.spec.ts +++ b/src/bounties/bounties.service.spec.ts @@ -228,23 +228,20 @@ describe('BountiesService', () => { }); it('expireOverdue flips overdue bounties to EXPIRED and returns count', async () => { - const overdueBounties = [ - { id: 'b1', status: BountyStatus.OPEN }, - { id: 'b2', status: BountyStatus.FUNDED }, - ]; const mockQueryBuilder = { + update: jest.fn().mockReturnThis(), + set: jest.fn().mockReturnThis(), where: jest.fn().mockReturnThis(), andWhere: jest.fn().mockReturnThis(), - getMany: jest.fn().mockResolvedValue(overdueBounties), + execute: jest.fn().mockResolvedValue({ affected: 2 }), }; bountyRepo.createQueryBuilder.mockReturnValue(mockQueryBuilder); const count = await service.expireOverdue(); expect(count).toBe(2); - expect(bountyRepo.save).toHaveBeenCalledTimes(2); - expect(overdueBounties[0].status).toBe(BountyStatus.EXPIRED); - expect(overdueBounties[1].status).toBe(BountyStatus.EXPIRED); + expect(mockQueryBuilder.update).toHaveBeenCalledWith(Bounty); + expect(mockQueryBuilder.set).toHaveBeenCalledWith({ status: BountyStatus.EXPIRED }); }); it('markPrClosedWithoutMerge transitions IN_REVIEW back to CLAIMED', async () => { diff --git a/src/bounties/bounties.service.ts b/src/bounties/bounties.service.ts index 32b3dba..32b7bf6 100644 --- a/src/bounties/bounties.service.ts +++ b/src/bounties/bounties.service.ts @@ -243,25 +243,23 @@ export class BountiesService { /** Marks bounties whose deadline has passed and that were never merged as expired. */ async expireOverdue(): Promise { - const overdue = await this.bountyRepo - .createQueryBuilder('bounty') - .where('bounty.deadline IS NOT NULL AND bounty.deadline < :now', { + const result = await this.bountyRepo + .createQueryBuilder() + .update(Bounty) + .set({ status: BountyStatus.EXPIRED }) + .where('deadline IS NOT NULL AND deadline < :now', { now: new Date(), }) - .andWhere('bounty.status IN (:...statuses)', { + .andWhere('status IN (:...statuses)', { statuses: [ BountyStatus.OPEN, BountyStatus.FUNDED, BountyStatus.CLAIMED, ], }) - .getMany(); + .execute(); - for (const bounty of overdue) { - bounty.status = BountyStatus.EXPIRED; - await this.bountyRepo.save(bounty); - } - return overdue.length; + return result.affected ?? 0; } async list(options: ListBountiesOptions = {}): Promise { diff --git a/src/common/entities/payment.entity.ts b/src/common/entities/payment.entity.ts index 93024c8..0376f93 100644 --- a/src/common/entities/payment.entity.ts +++ b/src/common/entities/payment.entity.ts @@ -10,6 +10,7 @@ import { } from 'typeorm'; import { Escrow } from './escrow.entity'; import { User } from './user.entity'; +import { Issue } from './issue.entity'; import { AssetType, PaymentStatus } from '../enums'; /** @@ -25,6 +26,7 @@ import { AssetType, PaymentStatus } from '../enums'; */ @Entity('payments') @Index('IDX_payment_escrow', ['escrowId']) +@Index('IDX_escrow_maintenance_issue', ['escrowId', 'maintenanceIssueId'], { unique: true, where: '"maintenanceIssueId" IS NOT NULL' }) export class Payment { @PrimaryGeneratedColumn('uuid') id: string; @@ -51,6 +53,13 @@ export class Payment { @Column({ type: 'varchar', nullable: true }) recipientAddress: string | null; + @ManyToOne(() => Issue, { onDelete: 'SET NULL', nullable: true }) + @JoinColumn() + maintenanceIssue: Issue | null; + + @Column({ type: 'uuid', nullable: true }) + maintenanceIssueId: string | null; + @Column({ type: 'decimal', precision: 20, scale: 7 }) amount: string; diff --git a/src/escrow/escrow.service.ts b/src/escrow/escrow.service.ts index 6d2dd28..941bbfb 100644 --- a/src/escrow/escrow.service.ts +++ b/src/escrow/escrow.service.ts @@ -396,6 +396,7 @@ export class EscrowService { amount: string, recipientAddress: string, recipientId?: string, + maintenanceIssueId?: string, ): Promise { const escrow = await this.getOrThrow(escrowId); this.assertLocked(escrow); @@ -415,6 +416,7 @@ export class EscrowService { escrowId: escrow.id, recipientId: recipientId ?? null, recipientAddress, + maintenanceIssueId: maintenanceIssueId ?? null, amount, asset: escrow.asset, status: PaymentStatus.CONFIRMED, diff --git a/src/maintenance-pool/maintenance-pool.service.spec.ts b/src/maintenance-pool/maintenance-pool.service.spec.ts index 88ff0c4..51f3e30 100644 --- a/src/maintenance-pool/maintenance-pool.service.spec.ts +++ b/src/maintenance-pool/maintenance-pool.service.spec.ts @@ -407,13 +407,13 @@ describe('MaintenancePoolService', () => { expect(escrowService.poolWithdraw).not.toHaveBeenCalled(); }); - it('rejects when the issue has already received a reward from this pool (#273)', async () => { + it('rejects when the same issue receives a reward twice (even for different recipients) (#273, #458)', async () => { poolRepo.findOne.mockResolvedValue({ id: 'pool-1', balance: '100', escrowId: 'escrow-1', }); - // Mock the payment query to return an existing payment + // Mock the payment query to return an existing payment for this issue const mockPaymentQueryBuilder = { innerJoin: jest.fn().mockReturnThis(), where: jest.fn().mockReturnThis(), @@ -423,11 +423,65 @@ describe('MaintenancePoolService', () => { paymentRepo.createQueryBuilder.mockReturnValue(mockPaymentQueryBuilder); await expect( - service.assignReward('pool-1', 'issue-1', '10', 'GRECIPIENT', 'user-1'), + service.assignReward('pool-1', 'issue-1', '10', 'GRECIPIENT_B', 'user-2'), ).rejects.toThrow(ConflictException); + + expect(mockPaymentQueryBuilder.andWhere).toHaveBeenCalledWith( + 'payment.maintenanceIssueId = :issueId', + { issueId: 'issue-1' }, + ); expect(escrowService.poolWithdraw).not.toHaveBeenCalled(); }); + it('allows the same recipient to be rewarded for two different issues', async () => { + poolRepo.findOne.mockResolvedValue({ + id: 'pool-1', + balance: '100', + escrowId: 'escrow-1', + }); + const mockQueryBuilder = { + update: jest.fn().mockReturnThis(), + set: jest.fn().mockReturnThis(), + where: jest.fn().mockReturnThis(), + setParameter: jest.fn().mockReturnThis(), + execute: jest.fn().mockResolvedValue({ affected: 1 }), + }; + poolRepo.createQueryBuilder.mockReturnValue(mockQueryBuilder); + escrowService.poolWithdraw.mockResolvedValue({ id: 'payment-1' }); + + // Return null, meaning no payment for this issue exists yet + const mockPaymentQueryBuilder = { + innerJoin: jest.fn().mockReturnThis(), + where: jest.fn().mockReturnThis(), + andWhere: jest.fn().mockReturnThis(), + getOne: jest.fn().mockResolvedValue(null), + }; + paymentRepo.createQueryBuilder.mockReturnValue(mockPaymentQueryBuilder); + + issueRepo.findOne.mockResolvedValue({ + id: 'issue-2', + isMaintenanceType: true, + repositoryId: 'repository-1', + }); + + const payment = await service.assignReward( + 'pool-1', + 'issue-2', + '10', + 'GRECIPIENT', + 'user-1', + ); + + expect(payment).toEqual({ id: 'payment-1' }); + expect(escrowService.poolWithdraw).toHaveBeenCalledWith( + 'escrow-1', + '10', + 'GRECIPIENT', + 'user-1', + 'issue-2', + ); + }); + // Regression test for #51 (MaintenancePool.balance was a hand-maintained // running total with a lost-update race across concurrent // deposit/assignReward calls): assignReward now decrements via an diff --git a/src/maintenance-pool/maintenance-pool.service.ts b/src/maintenance-pool/maintenance-pool.service.ts index 13415a6..4327b8a 100644 --- a/src/maintenance-pool/maintenance-pool.service.ts +++ b/src/maintenance-pool/maintenance-pool.service.ts @@ -144,19 +144,11 @@ export class MaintenancePoolService { } // Guard against double/triple payout for the same issue (#273). - const existingPayment = await this.paymentRepo.findOne({ - where: { - recipientId: recipientId ?? null, - }, - }); - // Check if there's already a payment for this issue from this pool's escrow. const existingPoolPayment = await this.paymentRepo .createQueryBuilder('payment') .innerJoin('payment.escrow', 'escrow') .where('escrow.maintenancePoolId = :poolId', { poolId: id }) - .andWhere('payment.recipientId = :recipientId', { - recipientId: recipientId ?? null, - }) + .andWhere('payment.maintenanceIssueId = :issueId', { issueId }) .getOne(); if (existingPoolPayment) { throw new ConflictException( @@ -190,6 +182,7 @@ export class MaintenancePoolService { amount, recipientAddress, recipientId, + issueId, ); return payment;