Merge pull request #18956 from mozilla/FXA-11772

fix(payments-paypal): Customer cannot purchase another product with their saved PayPal account

Author: xlisachan

libs/payments/paypal/src/lib/paypalBillingAgreement.manager.spec.ts

@@ -143,9 +143,8 @@ describe('PaypalBillingAgreementManager', () => {
         .spyOn(paypalClient, 'baUpdate')
         .mockResolvedValue(NVPBAUpdateTransactionResponseFactory());
 
-      const result = await paypalBillingAgreementManager.cancel(
-        billingAgreementId
-      );
+      const result =
+        await paypalBillingAgreementManager.cancel(billingAgreementId);
 
       expect(result).toBeUndefined();
       expect(paypalClient.baUpdate).toBeCalledWith({
@@ -211,9 +210,8 @@ describe('PaypalBillingAgreementManager', () => {
         .spyOn(paypalClient, 'baUpdate')
         .mockResolvedValue(nvpBillingAgreementMock);
 
-      const result = await paypalBillingAgreementManager.retrieve(
-        billingAgreementId
-      );
+      const result =
+        await paypalBillingAgreementManager.retrieve(billingAgreementId);
       expect(result).toEqual({
         city: nvpBillingAgreementMock.CITY,
         countryCode: nvpBillingAgreementMock.COUNTRYCODE,
@@ -239,9 +237,8 @@ describe('PaypalBillingAgreementManager', () => {
         .spyOn(paypalClient, 'baUpdate')
         .mockResolvedValue(nvpBillingAgreementMock);
 
-      const result = await paypalBillingAgreementManager.retrieve(
-        billingAgreementId
-      );
+      const result =
+        await paypalBillingAgreementManager.retrieve(billingAgreementId);
       expect(result).toEqual({
         city: nvpBillingAgreementMock.CITY,
         countryCode: nvpBillingAgreementMock.COUNTRYCODE,
@@ -271,6 +268,33 @@ describe('PaypalBillingAgreementManager', () => {
       expect(result).toEqual(mockPayPalCustomer.billingAgreementId);
     });
 
+    it("returns the customer's current active PayPal billing agreement ID with multiple records", async () => {
+      const uid = faker.string.uuid();
+      const mockPayPalCustomerRecord1 = ResultPaypalCustomerFactory({
+        uid,
+        status: 'Cancelled',
+      });
+      const mockPayPalCustomerRecord2 = ResultPaypalCustomerFactory({
+        uid,
+        status: 'active',
+      });
+      const mockPayPalCustomerRecord3 = ResultPaypalCustomerFactory({
+        uid,
+        status: 'Cancelled',
+      });
+
+      jest
+        .spyOn(paypalCustomerManager, 'fetchPaypalCustomersByUid')
+        .mockResolvedValue([
+          mockPayPalCustomerRecord1,
+          mockPayPalCustomerRecord2,
+          mockPayPalCustomerRecord3,
+        ]);
+
+      const result = await paypalBillingAgreementManager.retrieveActiveId(uid);
+      expect(result).toEqual(mockPayPalCustomerRecord2.billingAgreementId);
+    });
+
     it('returns undefined if no PayPal customer record', async () => {
       const uid = faker.string.uuid();
 
@@ -284,7 +308,9 @@ describe('PaypalBillingAgreementManager', () => {
 
     it('returns undefined if billing agreement is not active', async () => {
       const uid = faker.string.uuid();
-      const mockPayPalCustomer = ResultPaypalCustomerFactory({ status: 'Cancelled' });
+      const mockPayPalCustomer = ResultPaypalCustomerFactory({
+        status: 'Cancelled',
+      });
 
       jest
         .spyOn(paypalCustomerManager, 'fetchPaypalCustomersByUid')
@@ -294,14 +320,28 @@ describe('PaypalBillingAgreementManager', () => {
       expect(result).toEqual(undefined);
     });
 
-    it('throws PaypalCustomerMultipleRecordsError if more than one PayPal customer found', async () => {
+    it('throws PaypalCustomerMultipleRecordsError if more than one active PayPal customer found', async () => {
       const uid = faker.string.uuid();
-      const mockPayPalCustomer1 = ResultPaypalCustomerFactory();
-      const mockPayPalCustomer2 = ResultPaypalCustomerFactory();
+      const mockPayPalCustomerRecord1 = ResultPaypalCustomerFactory({
+        uid,
+        status: 'active',
+      });
+      const mockPayPalCustomerRecord2 = ResultPaypalCustomerFactory({
+        uid,
+        status: 'active',
+      });
+      const mockPayPalCustomerRecord3 = ResultPaypalCustomerFactory({
+        uid,
+        status: 'Cancelled',
+      });
 
       jest
         .spyOn(paypalCustomerManager, 'fetchPaypalCustomersByUid')
-        .mockResolvedValue([mockPayPalCustomer1, mockPayPalCustomer2]);
+        .mockResolvedValue([
+          mockPayPalCustomerRecord1,
+          mockPayPalCustomerRecord2,
+          mockPayPalCustomerRecord3,
+        ]);
 
       await expect(
         paypalBillingAgreementManager.retrieveActiveId(uid)

libs/payments/paypal/src/lib/paypalBillingAgreement.manager.ts

@@ -15,7 +15,7 @@ export class PaypalBillingAgreementManager {
   constructor(
     private client: PayPalClient,
     private paypalCustomerManager: PaypalCustomerManager
-  ) { }
+  ) {}
 
   public async retrieveOrCreateId(
     uid: string,
@@ -99,11 +99,16 @@ export class PaypalBillingAgreementManager {
   async retrieveActiveId(uid: string): Promise<string | undefined> {
     const paypalCustomer =
       await this.paypalCustomerManager.fetchPaypalCustomersByUid(uid);
-    const firstRecord = paypalCustomer.at(0);
+    const activeRecords = paypalCustomer.filter(
+      (customer) => customer.status === 'active'
+    );
 
-    if (!firstRecord || firstRecord?.status !== 'active') return;
-    if (paypalCustomer.length > 1)
+    if (activeRecords.length > 1) {
       throw new PaypalCustomerMultipleRecordsError(uid);
+    }
+
+    const firstRecord = activeRecords.at(0);
+    if (!firstRecord) return;
 
     return firstRecord.billingAgreementId;
   }