Dont throw away extra paypal months (#12149)

* Issue 10605 - Don't pointlessly format dates

* fix(10605): Be defensive about setting plan termination dates

test/api/unit/libs/payments/stripe/calculateSubscriptionTerminationDate.test.js

@@ -1,7 +1,9 @@
 import moment from 'moment';
-import { calculateSubscriptionTerminationDate } from '../../../../../../website/server/libs/payments/util';
+import calculateSubscriptionTerminationDate from '../../../../../../website/server/libs/payments/calculateSubscriptionTerminationDate';
 import api from '../../../../../../website/server/libs/payments/payments';
 
+const groupPlanId = api.constants.GROUP_PLAN_CUSTOMER_ID;
+
 describe('#calculateSubscriptionTerminationDate', () => {
   let plan;
   let nextBill;
@@ -13,52 +15,67 @@ describe('#calculateSubscriptionTerminationDate', () => {
     };
     nextBill = moment();
   });
+
   it('should extend date to the exact amount of days left before the next bill will occur', () => {
     nextBill = moment()
       .add(5, 'days');
     const expectedTerminationDate = moment()
       .add(5, 'days');
 
-    const terminationDate = calculateSubscriptionTerminationDate(nextBill, plan, api.constants);
+    const terminationDate = calculateSubscriptionTerminationDate(nextBill, plan, groupPlanId);
     expect(expectedTerminationDate.diff(terminationDate, 'days')).to.eql(0);
   });
+
   it('if nextBill is null, add 30 days to termination date', () => {
     nextBill = null;
     const expectedTerminationDate = moment()
       .add(30, 'days');
-    const terminationDate = calculateSubscriptionTerminationDate(nextBill, plan, api.constants);
+    const terminationDate = calculateSubscriptionTerminationDate(nextBill, plan, groupPlanId);
 
     expect(expectedTerminationDate.diff(terminationDate, 'days')).to.eql(0);
   });
+
   it('if nextBill is null and it\'s a group plan, add 2 days instead of 30', () => {
     nextBill = null;
     plan.customerId = api.constants.GROUP_PLAN_CUSTOMER_ID;
     const expectedTerminationDate = moment()
       .add(2, 'days');
 
-    const terminationDate = calculateSubscriptionTerminationDate(nextBill, plan, api.constants);
+    const terminationDate = calculateSubscriptionTerminationDate(nextBill, plan, groupPlanId);
     expect(expectedTerminationDate.diff(terminationDate, 'days')).to.eql(0);
   });
+
   it('should add 30.5 days for each extraMonth', () => {
     plan.extraMonths = 4;
     const expectedTerminationDate = moment()
       .add(30.5 * 4, 'days');
 
-    const terminationDate = calculateSubscriptionTerminationDate(nextBill, plan, api.constants);
+    const terminationDate = calculateSubscriptionTerminationDate(nextBill, plan, groupPlanId);
     expect(expectedTerminationDate.diff(terminationDate, 'days')).to.eql(0);
   });
+
   it('should round up if total days gained by extraMonth is a decimal number', () => {
     plan.extraMonths = 5;
     const expectedTerminationDate = moment()
       .add(Math.ceil(30.5 * 5), 'days');
 
-    const terminationDate = calculateSubscriptionTerminationDate(nextBill, plan, api.constants);
+    const terminationDate = calculateSubscriptionTerminationDate(nextBill, plan, groupPlanId);
     expect(expectedTerminationDate.diff(terminationDate, 'days')).to.eql(0);
   });
+
   it('behaves like extraMonths is 0 if it\'s set to a negative number', () => {
     plan.extraMonths = -5;
     const expectedTerminationDate = moment();
-    const terminationDate = calculateSubscriptionTerminationDate(nextBill, plan, api.constants);
+    const terminationDate = calculateSubscriptionTerminationDate(nextBill, plan, groupPlanId);
     expect(expectedTerminationDate.diff(terminationDate, 'days')).to.eql(0);
   });
+
+  it('returns current terminated date if it exists and is later than newly calculated date', () => {
+    const expectedTerminationDate = moment().add({ months: 5 }).toDate();
+    plan.terminationDate = expectedTerminationDate;
+
+    const terminationDate = calculateSubscriptionTerminationDate(nextBill, plan, groupPlanId);
+
+    expect(terminationDate).to.equal(expectedTerminationDate);
+  });
 });

test/api/v3/integration/groups/POST-groups_groupId_leave.test.js

@@ -1,7 +1,5 @@
 import { v4 as generateUUID } from 'uuid';
-import {
-  each,
-} from 'lodash';
+import each from 'lodash/each';
 import moment from 'moment';
 import {
   generateChallenge,
@@ -13,7 +11,7 @@ import {
 } from '../../../../helpers/api-integration/v3';
 import { model as User } from '../../../../../website/server/models/user';
 import payments from '../../../../../website/server/libs/payments/payments';
-import { calculateSubscriptionTerminationDate } from '../../../../../website/server/libs/payments/util';
+import calculateSubscriptionTerminationDate from '../../../../../website/server/libs/payments/calculateSubscriptionTerminationDate';
 
 describe('POST /groups/:groupId/leave', () => {
   const typesOfGroups = {
@@ -359,6 +357,7 @@ describe('POST /groups/:groupId/leave', () => {
           'purchased.plan.extraMonths': extraMonths,
         });
       });
+
       it('calculates dateTerminated and sets extraMonths to zero after user leaves the group', async () => {
         const userBeforeLeave = await User.findById(member._id).exec();
 
@@ -373,7 +372,7 @@ describe('POST /groups/:groupId/leave', () => {
         const expectedTerminationDate = calculateSubscriptionTerminationDate(null, {
           customerId: payments.constants.GROUP_PLAN_CUSTOMER_ID,
           extraMonths,
-        }, payments.constants);
+        }, payments.constants.GROUP_PLAN_CUSTOMER_ID);
 
         expect(extraMonthsBefore).to.gte(12);
         expect(extraMonthsAfter).to.equal(0);

website/server/libs/payments/calculateSubscriptionTerminationDate.js

@@ -0,0 +1,23 @@
+import moment from 'moment';
+
+const DEFAULT_REMAINING_DAYS = 30;
+const DEFAULT_REMAINING_DAYS_FOR_GROUP_PLAN = 2;
+
+export default function calculateSubscriptionTerminationDate (
+  nextBill, purchasedPlan, groupPlanCustomerId,
+) {
+  const defaultRemainingDays = purchasedPlan.customerId === groupPlanCustomerId
+    ? DEFAULT_REMAINING_DAYS_FOR_GROUP_PLAN : DEFAULT_REMAINING_DAYS;
+
+  const remaining = nextBill
+    ? moment(nextBill).diff(new Date(), 'days', true)
+    : defaultRemainingDays;
+
+  const extraMonths = Math.max(purchasedPlan.extraMonths, 0);
+  const extraDays = Math.ceil(30.5 * extraMonths);
+
+  const calculatedTerminationDate = moment().startOf('day').add({ days: remaining + extraDays });
+
+  return calculatedTerminationDate.isBefore(purchasedPlan.terminationDate)
+    ? purchasedPlan.terminationDate : calculatedTerminationDate.toDate();
+}

website/server/libs/payments/subscriptions.js

@@ -17,7 +17,7 @@ import {
 } from '../errors';
 import shared from '../../../common';
 import { sendNotification as sendPushNotification } from '../pushNotifications'; // eslint-disable-line import/no-cycle
-import { calculateSubscriptionTerminationDate } from './util';
+import calculateSubscriptionTerminationDate from './calculateSubscriptionTerminationDate';
 
 // @TODO: Abstract to shared/constant
 const JOINED_GROUP_PLAN = 'joined group plan';
@@ -314,7 +314,9 @@ async function cancelSubscription (data) {
     sendEmail = false; // because group-member-cancel email has already been sent
   }
 
-  plan.dateTerminated = calculateSubscriptionTerminationDate(data.nextBill, plan, this.constants);
+  plan.dateTerminated = calculateSubscriptionTerminationDate(
+    data.nextBill, plan, this.constants.GROUP_PLAN_CUSTOMER_ID,
+  );
 
   // clear extra time. If they subscribe again, it'll be recalculated from p.dateTerminated
   plan.extraMonths = 0;

website/server/libs/payments/util.js

@@ -1,32 +0,0 @@
-import moment from 'moment';
-
-const DEFAULT_REMAINING_DAYS = 30;
-const DEFAULT_REMAINING_DAYS_FOR_GROUP_PLAN = 2;
-
-/**
- * paymentsApiConstants is provided as parameter because of a dependency cycle
- * with subscriptions api which will occur if api.constants would be used directly
- */
-export function calculateSubscriptionTerminationDate (
-  nextBill, purchasedPlan, paymentsApiConstants,
-) {
-  const defaultRemainingDays = (
-    purchasedPlan.customerId === paymentsApiConstants.GROUP_PLAN_CUSTOMER_ID
-  ) ? DEFAULT_REMAINING_DAYS_FOR_GROUP_PLAN
-    : DEFAULT_REMAINING_DAYS;
-  const now = moment();
-
-  const remaining = nextBill
-    ? moment(nextBill).diff(new Date(), 'days', true)
-    : defaultRemainingDays;
-
-  const extraMonths = Math.max(purchasedPlan.extraMonths, 0);
-  const extraDays = Math.ceil(30.5 * extraMonths);
-  const nowStr = `${now.format('MM')}/${now.format('DD')}/${now.format('YYYY')}`;
-  const nowStrFormat = 'MM/DD/YYYY';
-
-  return moment(nowStr, nowStrFormat)
-    .add({ days: remaining })
-    .add({ days: extraDays })
-    .toDate();
-}