refactor: Cancelling subscription (#690)

* fix: Cancelling recurring payment

* refactor: Subscription cancelling

* chore: Remove redundant logs

* chore: Remove redundant stripeService

* chore: Remove comment

* chore: Fix merge conflict

* chore: Remove merge conlifct artifaxt

* chore: Remove redundant endpoint

Author: sashko9807

apps/api/src/donations/donations.module.ts

@@ -1,10 +1,6 @@
-import { StripeModule } from '@golevelup/nestjs-stripe'
 import { Module } from '@nestjs/common'
 import { ConfigModule, ConfigService } from '@nestjs/config'
-import { StripeConfigFactory } from './helpers/stripe-config-factory'
 import { CampaignModule } from '../campaign/campaign.module'
-import { CampaignService } from '../campaign/campaign.service'
-import { RecurringDonationService } from '../recurring-donation/recurring-donation.service'
 import { ExportService } from '../export/export.service'
 import { PersonModule } from '../person/person.module'
 import { PersonService } from '../person/person.service'
@@ -13,7 +9,6 @@ import { VaultModule } from '../vault/vault.module'
 import { VaultService } from '../vault/vault.service'
 import { DonationsController } from './donations.controller'
 import { DonationsService } from './donations.service'
-import { StripePaymentService } from '../stripe/events/stripe-payment.service'
 import { HttpModule } from '@nestjs/axios'
 import { ExportModule } from './../export/export.module'
 import { NotificationModule } from '../sockets/notifications/notification.module'
@@ -36,7 +31,6 @@ import { PrismaModule } from '../prisma/prisma.module'
   controllers: [DonationsController],
   providers: [
     DonationsService,
-    RecurringDonationService,
     VaultService,
     PersonService,
     ExportService,

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

@@ -7,12 +7,11 @@ import { UpdateRecurringDonationDto } from './dto/update-recurring-donation.dto'
 import { RealmViewSupporters, ViewSupporters } from '@podkrepi-bg/podkrepi-types'
 import { ApiTags } from '@nestjs/swagger'
 import { KeycloakTokenParsed } from '../auth/keycloak'
-import { StripeService } from '../stripe/stripe.service'
 
 @ApiTags('recurring-donation')
 @Controller('recurring-donation')
 export class RecurringDonationController {
-  constructor(private readonly recurringDonationService: RecurringDonationService, private readonly stripeService:StripeService) {}
+  constructor(private readonly recurringDonationService: RecurringDonationService) {}
 
   @Get('list')
   @Roles({
@@ -46,13 +45,12 @@ export class RecurringDonationController {
     return this.recurringDonationService.create(createRecurringDonationDto)
   }
 
-  //TODO: Deprecate this endpont after FE is configured to call stripe cancel webhook
-  @Get('cancel/:id')
+  @Patch(':id/cancel')
   async cancelSubscription(
     @Param('id') id: string,
     @AuthenticatedUser() user: KeycloakTokenParsed,
   ) {
-    return await this.stripeService.cancelSubscription(id, user)
+    return await this.recurringDonationService.cancel(id, user)
   }
 
   @Patch(':id')

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

@@ -10,6 +10,11 @@ import { INestApplication } from '@nestjs/common'
 import { 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'
+import { CampaignService } from '../campaign/campaign.service'
+import { DonationsService } from '../donations/donations.service'
+import { RealmViewSupporters } from '@podkrepi-bg/podkrepi-types'
+
 
 const mockCreateRecurring = new CreateRecurringDonationDto()
 mockCreateRecurring.amount = 1
@@ -38,6 +43,7 @@ const mockRecurring = {
 describe('RecurringDonationService', () => {
   let service: RecurringDonationService
   let app: INestApplication
+  let stripeService: StripeService
 
   const stripeMock = {
     checkout: { sessions: { create: jest.fn() } },
@@ -50,10 +56,22 @@ describe('RecurringDonationService', () => {
         RecurringDonationService,
         MockPrismaService,
         ConfigService,
+        {
+          provide: StripeService,
+          useValue: mockDeep<StripeService>()
+        },
         {
           provide: HttpService,
           useValue: mockDeep<HttpService>(),
         },
+        {
+          provide: CampaignService,
+          useValue: mockDeep<CampaignService>(),
+        },
+        {
+          provide: DonationsService,
+          useValue: mockDeep<DonationsService>(),
+        },
         Stripe,
         {
           provide: STRIPE_CLIENT_TOKEN,
@@ -63,7 +81,7 @@ describe('RecurringDonationService', () => {
     }).compile()
 
     service = module.get<RecurringDonationService>(RecurringDonationService)
-
+    stripeService = module.get<StripeService>(StripeService)
     app = module.createNestApplication()
     await app.init()
   })
@@ -86,18 +104,51 @@ describe('RecurringDonationService', () => {
     expect(result).toStrictEqual(mockRecurring)
   })
 
-  it('should cancel a subscription in db', async () => {
-    prismaMock.recurringDonation.update.mockResolvedValueOnce(mockRecurring)
-    await service.cancel('1')
+  it('should cancel a subscription in db if admin', async () => {
+        prismaMock.recurringDonation.update.mockResolvedValueOnce(mockRecurring)
+    prismaMock.recurringDonation.findUnique.mockResolvedValueOnce(mockRecurring)
+    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)
+    await service.cancel('1', { sub: '1', realm_access: {roles: [RealmViewSupporters.role]} } as any)
+    expect(stripeSpy).toHaveBeenCalledWith(mockRecurring.extSubscriptionId)
+    expect(updateDbSpy).toHaveBeenCalledWith(mockRecurring.id, RecurringDonationStatus.canceled)
+  })
+
+  it('should cancel a subscription in db if regular user and own donation', async () => {
+        // prismaMock.recurringDonation.update.mockResolvedValueOnce(mockRecurring)
+    const findOneSpy = jest.spyOn(service, 'findOne').mockResolvedValue(mockRecurring)
+    const donationBelongsToSpy = jest.spyOn(service, 'donationBelongsTo')
+    donationBelongsToSpy.mockResolvedValue(true)
+    const updateDbSpy = jest.spyOn(service, 'updateStatus')
+    const stripeSpy = jest.spyOn(stripeService, 'cancelSubscription')
+    updateDbSpy.mockResolvedValue(mockRecurring)
+    stripeSpy.mockResolvedValue({status: 'active'} 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 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')
+    const donationBelongsToSpy = jest.spyOn(service, 'donationBelongsTo')
+    donationBelongsToSpy.mockResolvedValue(false)
     const updateDbSpy = jest.spyOn(prismaMock.recurringDonation, 'update')
-    expect(updateDbSpy).toHaveBeenCalledWith({
+    const stripeSpy = jest.spyOn(stripeService, 'cancelSubscription')
+    expect(service.cancel(mockRecurring.id, { sub: '1', realm_access: {roles: []} } as any)).rejects.toThrow()
+    expect(stripeSpy).not.toHaveBeenCalledWith(mockRecurring.extSubscriptionId)
+    expect(updateStatusSpy).not.toHaveBeenCalledWith({
       where: { id: '1' },
       data: {
         status: RecurringDonationStatus.canceled,
       },
     })
   })
-
+  
   it('should create  a subscription in db', async () => {
     prismaMock.recurringDonation.create.mockResolvedValueOnce(mockRecurring)
 
@@ -123,4 +174,8 @@ describe('RecurringDonationService', () => {
       },
     })
   })
+
+ it('should update a recurring donation', async () => {
+  expect(true).toBeTruthy()
+  })
 })

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

@@ -1,13 +1,16 @@
-import { Injectable, NotFoundException, Logger, BadRequestException } 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'
 import { UpdateRecurringDonationDto } from './dto/update-recurring-donation.dto'
 import { RecurringDonationStatus } from '@prisma/client'
+import { KeycloakTokenParsed } from '../auth/keycloak'
+import { RealmViewSupporters } from '@podkrepi-bg/podkrepi-types'
+import { StripeService } from '../stripe/stripe.service'
 
 @Injectable()
 export class RecurringDonationService {
-  constructor(private prisma: PrismaService) {}
+  constructor(private prisma: PrismaService, private readonly stripeService:StripeService) {}
 
   async create(CreateRecurringDonationDto: CreateRecurringDonationDto): Promise<RecurringDonation> {
     return await this.prisma.recurringDonation.create({
@@ -132,7 +135,25 @@ export class RecurringDonationService {
     return result
   }
 
-  async cancel(id: string): Promise<RecurringDonation | null> {
+  async cancel(id: string, user: KeycloakTokenParsed): Promise<RecurringDonation | null> {
+    const recurringDonation = await this.findOne(id)
+    if (!recurringDonation) {
+      throw new NotFoundException(`Recurring donation with id ${id} not found`)
+    }
+
+    
+    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}`,
+      )
+    }
+    
+    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
+    }
     return await this.updateStatus(id, RecurringDonationStatus.canceled)
   }
 

apps/api/src/stripe/stripe.controller.ts

@@ -144,10 +144,6 @@ export class StripeController {
     return this.stripeService.listPrices('one_time')
   }
 
-  @Post(':id/cancel-subscription')
-  cancelSubscription(@Param('id') subscriptionId: string, @AuthenticatedUser() user: KeycloakTokenParsed) {
-    return this.stripeService.cancelSubscription(subscriptionId, user)
-  }
 
   @Get('prices/recurring')
   @Public()

apps/api/src/stripe/stripe.module.ts

@@ -23,7 +23,7 @@ import { TemplateService } from '../email/template.service'
     CampaignModule,
     PersonModule,
     DonationsModule,
-    forwardRef(()=>RecurringDonationModule),
+    forwardRef(() => RecurringDonationModule),
   ],
   providers: [StripeService, StripePaymentService, EmailService, TemplateService],
   controllers: [StripeController],

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

@@ -20,11 +20,9 @@ import { RealmViewSupporters } from '@podkrepi-bg/podkrepi-types'
 export class StripeService {
   constructor(
     @InjectStripeClient() private stripeClient: Stripe,
-    private personService: PersonService,
     private campaignService: CampaignService,
     private donationService: DonationsService,
     private configService: ConfigService,
-    private reacurringDonationService: RecurringDonationService,
   ) {}
 
   /**
@@ -421,36 +419,9 @@ export class StripeService {
     })
   }
 
-  async cancelSubscription(subscriptionId: string, user?: KeycloakTokenParsed) {
-
-    const rd = await this.reacurringDonationService.findOne(subscriptionId)
-    if (!rd) {
-      throw new Error(`Recurring donation with id ${subscriptionId} not found`)
-    }
-
-    if (user) {
-    
-      const isAdmin = user.realm_access?.roles.includes(RealmViewSupporters.role)
-  
-      if (!isAdmin && !this.reacurringDonationService.donationBelongsTo(subscriptionId, user.sub)) {
-        throw new Error(`User ${user.sub} is not allowed to cancel recurring donation with id ${subscriptionId} of person: ${rd.personId}`,
-        )
-      }
-    }
-
-    Logger.log(`Canceling subscription with api request to cancel: ${subscriptionId}`)
-    const result = await this.stripeClient.subscriptions.cancel(rd.extSubscriptionId)
-    if (result.status !== 'canceled') {
-      Logger.log(`Subscription cancel attempt failed with status of ${result.id}: ${result.status}`)
-      return
-    }
-    
-
-    // the webhook will handle this as well.
-    // but we cancel it here, in case the webhook is slow.
-    if (rd) {
-      return this.reacurringDonationService.cancel(rd.id)
-    }
+  async cancelSubscription(stripeSubscriptionId: string) {
+    const result = await this.stripeClient.subscriptions.cancel(stripeSubscriptionId)
+    return result
   }
 
   async findChargeById(chargeId: string): Promise<Stripe.Charge> {