fix: update DB immediately on subscription cancellation

apps/api/src/recurring-donation/recurring-donation.service.spec.ts

@@ -5,8 +5,8 @@ import { RecurringDonationService } from './recurring-donation.service'
 import { ConfigService } from '@nestjs/config'
 import { HttpService } from '@nestjs/axios'
 import { STRIPE_CLIENT_TOKEN } from '@golevelup/nestjs-stripe'
-import { INestApplication } from '@nestjs/common'
-import { RecurringDonationStatus } from '@prisma/client'
+import { BadRequestException, INestApplication } from '@nestjs/common'
+import { Prisma, RecurringDonationStatus } from '@prisma/client'
 import { CreateRecurringDonationDto } from './dto/create-recurring-donation.dto'
 import { RecurringDonation } from '../domain/generated/recurringDonation/entities/recurringDonation.entity'
 import { StripeService } from '../stripe/stripe.service'
@@ -102,13 +102,13 @@ describe('RecurringDonationService', () => {
   })
 
   it('should cancel a subscription in db if admin', async () => {
-    prismaMock.recurringDonation.update.mockResolvedValueOnce(mockRecurring)
-    prismaMock.recurringDonation.findUnique.mockResolvedValueOnce(mockRecurring)
+    prismaMock.recurringDonation.findUnique.mockResolvedValueOnce(mockRecurring) // for findOne
+    prismaMock.recurringDonation.update.mockResolvedValueOnce(mockRecurring) // for updateStatus
     const updateDbSpy = jest.spyOn(service, 'updateStatus')
     const stripeSpy = jest.spyOn(stripeService, 'cancelSubscription')
     const donationBelongsToSpy = jest.spyOn(service, 'donationBelongsTo')
     donationBelongsToSpy.mockResolvedValue(false)
-    stripeSpy.mockResolvedValue({ status: 'active' } as any)
+    stripeSpy.mockResolvedValue({ status: 'canceled' } as any)
     await service.cancel('1', {
       sub: '1',
       realm_access: { roles: [RealmViewSupporters.role] },
@@ -125,12 +125,38 @@ describe('RecurringDonationService', () => {
     const updateDbSpy = jest.spyOn(service, 'updateStatus')
     const stripeSpy = jest.spyOn(stripeService, 'cancelSubscription')
     updateDbSpy.mockResolvedValue(mockRecurring)
-    stripeSpy.mockResolvedValue({ status: 'active' } as any)
+    stripeSpy.mockResolvedValue({ status: 'canceled' } as any)
     await service.cancel(mockRecurring.id, { sub: '1', realm_access: { roles: [] } } as any)
     expect(stripeSpy).toHaveBeenCalledWith(mockRecurring.extSubscriptionId)
     expect(updateDbSpy).toHaveBeenCalledWith(mockRecurring.id, RecurringDonationStatus.canceled)
   })
 
+  it('should return early without calling stripe when subscription is already canceled', async () => {
+    const canceledDonation = { ...mockRecurring, status: RecurringDonationStatus.canceled }
+    const findOneSpy = jest.spyOn(service, 'findOne').mockResolvedValue(canceledDonation)
+    const stripeSpy = jest.spyOn(stripeService, 'cancelSubscription')
+    const updateDbSpy = jest.spyOn(service, 'updateStatus')
+
+    const result = await service.cancel('1', { sub: '1', realm_access: { roles: [] } } as any)
+
+    expect(findOneSpy).toHaveBeenCalledWith('1')
+    expect(stripeSpy).not.toHaveBeenCalled()
+    expect(updateDbSpy).not.toHaveBeenCalled()
+    expect(result).toEqual(canceledDonation)
+  })
+
+  it('should throw BadRequestException when stripe cancellation fails', async () => {
+    jest.spyOn(service, 'findOne').mockResolvedValue(mockRecurring)
+    jest.spyOn(service, 'donationBelongsTo').mockResolvedValue(true)
+    jest.spyOn(stripeService, 'cancelSubscription').mockResolvedValue({ status: 'active' } as any)
+    const updateDbSpy = jest.spyOn(service, 'updateStatus')
+
+    await expect(
+      service.cancel(mockRecurring.id, { sub: '1', realm_access: { roles: [] } } as any),
+    ).rejects.toThrow(BadRequestException)
+    expect(updateDbSpy).not.toHaveBeenCalled()
+  })
+
   it('should not allow to cancel a subscription in db if regular user and not own donation', async () => {
     const findOneSpy = jest.spyOn(service, 'findOne').mockResolvedValue(mockRecurring)
     const updateStatusSpy = jest.spyOn(service, 'updateStatus')
@@ -176,6 +202,37 @@ describe('RecurringDonationService', () => {
     })
   })
 
+  it('should skip db update when status is already canceled', async () => {
+    const canceledDonation = { ...mockRecurring, status: RecurringDonationStatus.canceled }
+    prismaMock.recurringDonation.update.mockRejectedValueOnce(
+      new Prisma.PrismaClientKnownRequestError('Record not found', {
+        code: 'P2025',
+        clientVersion: '5.0.0',
+      }),
+    )
+    prismaMock.recurringDonation.findUnique.mockResolvedValueOnce(canceledDonation)
+    const updateSpy = jest.spyOn(prismaMock.recurringDonation, 'update')
+
+    const result = await service.updateStatus('1', RecurringDonationStatus.canceled)
+
+    expect(result).toEqual(canceledDonation)
+    expect(updateSpy).toHaveBeenCalledTimes(1) // called once but rejected due to NOT filter
+  })
+
+  it('should return null from updateStatus when record does not exist', async () => {
+    prismaMock.recurringDonation.update.mockRejectedValueOnce(
+      new Prisma.PrismaClientKnownRequestError('Record not found', {
+        code: 'P2025',
+        clientVersion: '5.0.0',
+      }),
+    )
+    prismaMock.recurringDonation.findUnique.mockResolvedValueOnce(null)
+
+    const result = await service.updateStatus('nonexistent', RecurringDonationStatus.canceled)
+
+    expect(result).toBeNull()
+  })
+
   it('should update a recurring donation', async () => {
     expect(true).toBeTruthy()
   })

apps/api/src/recurring-donation/recurring-donation.service.ts

@@ -1,4 +1,10 @@
-import { Injectable, NotFoundException, Logger, BadRequestException, ForbiddenException } from '@nestjs/common'
+import {
+  Injectable,
+  NotFoundException,
+  Logger,
+  BadRequestException,
+  ForbiddenException,
+} from '@nestjs/common'
 import { Prisma, RecurringDonation } from '@prisma/client'
 import { PrismaService } from '../prisma/prisma.service'
 import { CreateRecurringDonationDto } from './dto/create-recurring-donation.dto'
@@ -10,7 +16,7 @@ import { StripeService } from '../stripe/stripe.service'
 
 @Injectable()
 export class RecurringDonationService {
-  constructor(private prisma: PrismaService, private readonly stripeService:StripeService) {}
+  constructor(private prisma: PrismaService, private readonly stripeService: StripeService) {}
 
   async create(CreateRecurringDonationDto: CreateRecurringDonationDto): Promise<RecurringDonation> {
     return await this.prisma.recurringDonation.create({
@@ -141,20 +147,33 @@ export class RecurringDonationService {
       throw new NotFoundException(`Recurring donation with id ${id} not found`)
     }
 
-
+    if (recurringDonation.status === RecurringDonationStatus.canceled) {
+      return recurringDonation
+    }
+
     const isAdmin = user.realm_access?.roles.includes(RealmViewSupporters.role)
     const belongsTo = await this.donationBelongsTo(recurringDonation.id, user.sub)
     if (!isAdmin && !belongsTo) {
-      throw new ForbiddenException(`User ${user.sub} is not allowed to cancel recurring donation with id ${recurringDonation.id} of person: ${recurringDonation.personId}`,
+      throw new ForbiddenException(
+        `User ${user.sub} is not allowed to cancel recurring donation with id ${recurringDonation.id} of person: ${recurringDonation.personId}`,
+      )
+    }
+
+    const subscription = await this.stripeService.cancelSubscription(
+      recurringDonation.extSubscriptionId,
+    )
+    if (subscription?.status !== 'canceled') {
+      throw new BadRequestException(
+        `Failed to cancel Stripe subscription ${recurringDonation.extSubscriptionId}: ` +
+          `status is ${subscription?.status}`,
       )
     }
-
-    const subscription = await this.stripeService.cancelSubscription(recurringDonation.extSubscriptionId)
-    if (subscription?.status === 'canceled') {
-      Logger.log(`Subscription cancel attempt failed with status of ${subscription.id}: ${subscription.status}`)
-      return null
+
+    const result = await this.updateStatus(id, RecurringDonationStatus.canceled)
+    if (!result) {
+      throw new BadRequestException(`Unable to update recurring donation ${id}`)
     }
-    return await this.updateStatus(id, RecurringDonationStatus.canceled)
+    return result
   }
 
   async remove(id: string): Promise<RecurringDonation | null> {
@@ -175,15 +194,22 @@ export class RecurringDonationService {
     id: string,
     status: RecurringDonationStatus,
   ): Promise<RecurringDonation | null> {
-    const result = await this.prisma.recurringDonation.update({
-      where: { id: id },
-      data: {
-        status: status,
-      },
-    })
-    if (!result) {
-      throw new BadRequestException(`Unable to find and update status of ${id} to ${status}`)
+    try {
+      return await this.prisma.recurringDonation.update({
+        where: { id, NOT: { status } },
+        data: { status },
+      })
+    } catch (e) {
+      if (e instanceof Prisma.PrismaClientKnownRequestError && e.code === 'P2025') {
+        const existing = await this.prisma.recurringDonation.findUnique({ where: { id } })
+        if (!existing) {
+          Logger.warn(`Recurring donation ${id} not found during status update`)
+          return null
+        }
+        Logger.debug(`Recurring donation ${id} already has status ${status}, skipping update`)
+        return existing
+      }
+      throw e
     }
-    return result
   }
 }

apps/api/src/stripe/events/stripe-payment.service.ts

@@ -300,13 +300,13 @@ export class StripePaymentService {
       subscription.id,
     )
     if (!recurringDonation) {
-      this.handleSubscriptionCreated(event)
+      await this.handleSubscriptionCreated(event)
       return
     }
 
     Logger.debug('Updating recurring donation by id ' + recurringDonation.id)
 
-    this.recurringDonationService.updateStatus(
+    await this.recurringDonationService.updateStatus(
       recurringDonation.id,
       string2RecurringDonationStatus(subscription.status),
     )
@@ -360,7 +360,7 @@ export class StripePaymentService {
 
     Logger.debug('Deleting recurring donation by id ' + recurringDonation.id)
 
-    this.recurringDonationService.updateStatus(
+    await this.recurringDonationService.updateStatus(
       recurringDonation.id,
       string2RecurringDonationStatus(subscription.status),
     )

apps/api/src/stripe/stripe.service.ts

@@ -493,9 +493,20 @@ export class StripeService {
     })
   }
 
-  async cancelSubscription(stripeSubscriptionId: string) {
-    const result = await this.stripeClient.subscriptions.cancel(stripeSubscriptionId)
-    return result
+  async cancelSubscription(stripeSubscriptionId: string): Promise<Stripe.Subscription> {
+    try {
+      return await this.stripeClient.subscriptions.cancel(stripeSubscriptionId)
+    } catch (e) {
+      if (e instanceof Stripe.errors.StripeInvalidRequestError && e.code === 'resource_missing') {
+        // Stripe returns resource_missing when canceling an already-canceled subscription,
+        // but the subscription can still be retrieved
+        const subscription = await this.stripeClient.subscriptions.retrieve(stripeSubscriptionId)
+        if (subscription.status === 'canceled') {
+          return subscription
+        }
+      }
+      throw e
+    }
   }
 
   async findChargeById(chargeId: string): Promise<Stripe.Charge> {