From a123c4d5d5bc1e62c1245bb480bc15a61b574a7c Mon Sep 17 00:00:00 2001 From: Benjamin Piouffle Date: Thu, 29 Feb 2024 12:13:00 +0100 Subject: [PATCH 1/2] feat(Contributions): ability to pause non-transferable --- cron/daily/60-clean-orders.js | 8 ++ ...ncel-subscriptions-for-cancelled-orders.ts | 40 ++++-- server/constants/activities.ts | 2 + server/constants/order-status.ts | 1 + server/graphql/loaders/index.js | 123 +++++++++++------- server/graphql/schemaV1.graphql | 1 + server/graphql/schemaV2.graphql | 33 +++++ server/graphql/v1/CollectiveInterface.js | 5 +- .../v2/mutation/HostApplicationMutations.ts | 34 +++-- server/graphql/v2/object/AccountStats.js | 65 +++++++-- server/graphql/v2/object/AmountStats.js | 3 +- server/lib/emailTemplates.ts | 1 + server/lib/notifications/email.ts | 1 + server/lib/recurring-contributions.js | 11 +- server/lib/timeline.ts | 1 + server/lib/webhooks.js | 2 +- server/models/Collective.ts | 8 +- server/models/Order.ts | 95 +++++++++++--- templates/emails/collective.unhosted.hbs | 5 +- templates/emails/subscription.paused.hbs | 24 ++++ .../mutation/HostApplicationMutations.test.ts | 123 +++++++++++++++++- .../lib/recurring-contributions.test.js | 4 +- test/server/models/Order.test.js | 95 +++++++++++++- 23 files changed, 565 insertions(+), 120 deletions(-) create mode 100644 templates/emails/subscription.paused.hbs diff --git a/cron/daily/60-clean-orders.js b/cron/daily/60-clean-orders.js index ed4f2472d93..f341f94d4c5 100644 --- a/cron/daily/60-clean-orders.js +++ b/cron/daily/60-clean-orders.js @@ -20,6 +20,14 @@ Promise.all([ AND "Orders"."createdAt" < (NOW() - interval '2 month') `, ), + // Mark all paused Orders as EXPIRED after 1 year + sequelize.query( + `UPDATE "Orders" + SET "status" = 'EXPIRED', "updatedAt" = NOW() + WHERE "Orders"."status" = 'PAUSED' + AND "Orders"."updatedAt" < (NOW() - interval '1 year') + `, + ), ]).then(() => { console.log('>>> Clean Orders: done'); process.exit(0); diff --git a/cron/hourly/70-cancel-subscriptions-for-cancelled-orders.ts b/cron/hourly/70-cancel-subscriptions-for-cancelled-orders.ts index c6e6ad4e4d2..3b90e3cc3c9 100644 --- a/cron/hourly/70-cancel-subscriptions-for-cancelled-orders.ts +++ b/cron/hourly/70-cancel-subscriptions-for-cancelled-orders.ts @@ -10,7 +10,7 @@ import OrderStatuses from '../../server/constants/order-status'; import logger from '../../server/lib/logger'; import { reportErrorToSentry } from '../../server/lib/sentry'; import { sleep } from '../../server/lib/utils'; -import models, { Op } from '../../server/models'; +import models, { Collective, Op } from '../../server/models'; import { OrderModelInterface } from '../../server/models/Order'; /** @@ -32,17 +32,29 @@ const getHostFromOrder = async order => { return models.Collective.findByPk(hostIds[0]); }; -const getOrderCancelationReason = (collective, order, orderHost) => { - if (order.TierId && !order.Tier) { - return ['DELETED_TIER', `Order tier deleted`]; +const getOrderCancelationReason = ( + collective: Collective, + order: OrderModelInterface, + orderHost: Collective, +): { + code: 'TRANSFER' | 'DELETED_TIER' | 'ARCHIVED_ACCOUNT' | 'UNHOSTED_COLLECTIVE' | 'CHANGED_HOST' | 'CANCELLED_ORDER'; + message: string; +} => { + if (order.status === 'PAUSED' && order.data?.pausedForTransfer) { + return { + code: 'TRANSFER', + message: `Your contribution to the Collective was paused. We'll inform you when it will be ready for re-activation.`, + }; + } else if (order.TierId && !order.Tier) { + return { code: 'DELETED_TIER', message: `Order tier deleted` }; } else if (collective.deactivatedAt) { - return ['ARCHIVED_ACCOUNT', `@${collective.slug} archived their account`]; + return { code: 'ARCHIVED_ACCOUNT', message: `@${collective.slug} archived their account` }; } else if (!collective.HostCollectiveId) { - return ['UNHOSTED_COLLECTIVE', `@${collective.slug} was un-hosted`]; + return { code: 'UNHOSTED_COLLECTIVE', message: `@${collective.slug} was un-hosted` }; } else if (collective.HostCollectiveId !== orderHost.id) { - return ['CHANGED_HOST', `@${collective.slug} changed host`]; + return { code: 'CHANGED_HOST', message: `@${collective.slug} changed host` }; } else { - return ['CANCELLED_ORDER', `Order cancelled`]; + return { code: 'CANCELLED_ORDER', message: `Order cancelled` }; } }; @@ -53,7 +65,7 @@ const getOrderCancelationReason = (collective, order, orderHost) => { */ export async function run() { const orphanOrders = await models.Order.findAll({ - where: { status: OrderStatuses.CANCELLED }, + where: { status: [OrderStatuses.CANCELLED, OrderStatuses.PAUSED] }, include: [ { model: models.Tier, @@ -85,14 +97,14 @@ export async function run() { for (const order of accountOrders) { try { const host = await getHostFromOrder(order); - const [reasonCode, reason] = getOrderCancelationReason(collective, order, host); + const reason = getOrderCancelationReason(collective, order, host); logger.debug( `Cancelling subscription ${order.Subscription.id} from order ${order.id} of @${collectiveHandle} (host: ${host.slug})`, ); if (!process.env.DRY) { - await order.Subscription.deactivate(reason, host); + await order.Subscription.deactivate(reason.message, host); await models.Activity.create({ - type: activities.SUBSCRIPTION_CANCELED, + type: reason.code === 'TRANSFER' ? activities.SUBSCRIPTION_PAUSED : activities.SUBSCRIPTION_CANCELED, CollectiveId: order.CollectiveId, FromCollectiveId: order.FromCollectiveId, HostCollectiveId: order.collective.HostCollectiveId, @@ -102,8 +114,8 @@ export async function run() { subscription: order.Subscription, collective: order.collective.minimal, fromCollective: order.fromCollective.minimal, - reasonCode: reasonCode, - reason: reason, + reasonCode: reason.code, + reason: reason.message, order: order.info, tier: order.Tier?.info, }, diff --git a/server/constants/activities.ts b/server/constants/activities.ts index e4a25ff314f..50efddbc51f 100644 --- a/server/constants/activities.ts +++ b/server/constants/activities.ts @@ -83,6 +83,7 @@ enum ActivityTypes { CONTRIBUTION_REJECTED = 'contribution.rejected', SUBSCRIPTION_ACTIVATED = 'subscription.activated', SUBSCRIPTION_CANCELED = 'subscription.canceled', + SUBSCRIPTION_PAUSED = 'subscription.paused', TICKET_CONFIRMED = 'ticket.confirmed', ORDER_CANCELED_ARCHIVED_COLLECTIVE = 'order.canceled.archived.collective', ORDER_PENDING = 'order.pending', @@ -239,6 +240,7 @@ export const ActivitiesPerClass: Record = { ActivityTypes.PAYMENT_FAILED, ActivityTypes.SUBSCRIPTION_ACTIVATED, ActivityTypes.SUBSCRIPTION_CANCELED, + ActivityTypes.SUBSCRIPTION_PAUSED, ActivityTypes.SUBSCRIPTION_CONFIRMED, ], [ActivityClasses.ACTIVITIES_UPDATES]: [ diff --git a/server/constants/order-status.ts b/server/constants/order-status.ts index 59b81e074f9..79b458dd4f7 100644 --- a/server/constants/order-status.ts +++ b/server/constants/order-status.ts @@ -24,6 +24,7 @@ enum OrderStatuses { // Disputed charges from Stripe DISPUTED = 'DISPUTED', REFUNDED = 'REFUNDED', + PAUSED = 'PAUSED', // In review charges from Stripe, IN_REVIEW = 'IN_REVIEW', } diff --git a/server/graphql/loaders/index.js b/server/graphql/loaders/index.js index 6fda0be9c9d..18d70e2e53c 100644 --- a/server/graphql/loaders/index.js +++ b/server/graphql/loaders/index.js @@ -1,6 +1,6 @@ import DataLoader from 'dataloader'; import { createContext } from 'dataloader-sequelize'; -import { get, groupBy } from 'lodash'; +import { get, groupBy, isNil } from 'lodash'; import moment from 'moment'; import { CollectiveType } from '../../constants/collectives'; @@ -372,54 +372,79 @@ export const loaders = req => { }) .then(results => sortResults(ids, results, 'CollectiveId')), ), - activeRecurringContributions: new DataLoader(ids => - models.Order.findAll({ - attributes: [ - 'Order.CollectiveId', - 'Order.currency', - 'Subscription.interval', - [ - sequelize.fn( - 'SUM', - sequelize.literal(`COALESCE("Order"."totalAmount", 0) - COALESCE("Order"."platformTipAmount", 0)`), - ), - 'total', - ], - ], - where: { - CollectiveId: { [Op.in]: ids }, - status: 'ACTIVE', - }, - group: ['Subscription.interval', 'CollectiveId', 'Order.currency'], - include: [ - { - model: models.Subscription, - attributes: [], - where: { isActive: true }, - }, - ], - raw: true, - }).then(rows => { - const results = groupBy(rows, 'CollectiveId'); - return Promise.all( - ids.map(async collectiveId => { - const stats = { CollectiveId: Number(collectiveId), monthly: 0, yearly: 0, currency: null }; - if (results[collectiveId]) { - for (const result of results[collectiveId]) { - const interval = result.interval === 'month' ? 'monthly' : 'yearly'; - // If it's the first total collected, set the currency - if (!stats.currency) { - stats.currency = result.currency; - } - const fxRate = await getFxRate(result.currency, stats.currency); - stats[interval] += result.total * fxRate; - } - } - return stats; - }), - ); - }), - ), + activeRecurringContributions: { + buildLoader({ currency, hasPortability = undefined, includeChildren = undefined } = {}) { + const key = `${currency}-${hasPortability}-${includeChildren}`; + if (!context.loaders.Collective.stats.activeRecurringContributions[key]) { + const collectiveIdCol = !includeChildren + ? 'CollectiveId' + : sequelize.fn('COALESCE', sequelize.col('collective.ParentCollectiveId'), sequelize.col('collective.id')); + + context.loaders.Collective.stats.activeRecurringContributions[key] = new DataLoader(ids => + models.Order.findAll({ + attributes: [ + [collectiveIdCol, 'CollectiveId'], + 'Order.currency', + 'Subscription.interval', + [sequelize.fn('COUNT', sequelize.literal(`DISTINCT "Order"."id"`)), 'count'], + [ + sequelize.fn( + 'SUM', + sequelize.literal(`COALESCE("Order"."totalAmount", 0) - COALESCE("Order"."platformTipAmount", 0)`), + ), + 'total', + ], + ], + where: { + status: 'ACTIVE', + }, + group: ['Subscription.interval', collectiveIdCol, 'Order.currency'], + include: [ + { + association: 'collective', + attributes: [], + required: true, + where: !includeChildren ? { id: ids } : { [Op.or]: [{ id: ids }, { ParentCollectiveId: ids }] }, + }, + { + model: models.Subscription, + attributes: [], + required: true, + where: { + isActive: true, + ...(!isNil(hasPortability) && { isManagedExternally: !hasPortability }), + }, + }, + ], + raw: true, + }).then(rows => { + const results = groupBy(rows, 'CollectiveId'); + return Promise.all( + ids.map(async collectiveId => { + const stats = { + CollectiveId: Number(collectiveId), + monthly: 0, + monthlyCount: 0, + yearly: 0, + yearlyCount: 0, + }; + if (results[collectiveId]) { + for (const result of results[collectiveId]) { + const interval = result.interval === 'month' ? 'monthly' : 'yearly'; + const fxRate = await getFxRate(result.currency, currency); + stats[interval] += result.total * fxRate; + stats[`${interval}Count`] += result.count; + } + } + return stats; + }), + ); + }), + ); + } + return context.loaders.Collective.stats.activeRecurringContributions[key]; + }, + }, orders: new DataLoader(async collectiveIds => { const stats = await sequelize.query( `SELECT * FROM "CollectiveOrderStats" WHERE "CollectiveId" IN (:collectiveIds)`, diff --git a/server/graphql/schemaV1.graphql b/server/graphql/schemaV1.graphql index 20c79dd0ca7..5fc427107fb 100644 --- a/server/graphql/schemaV1.graphql +++ b/server/graphql/schemaV1.graphql @@ -733,6 +733,7 @@ enum OrderStatus { EXPIRED DISPUTED REFUNDED + PAUSED IN_REVIEW } diff --git a/server/graphql/schemaV2.graphql b/server/graphql/schemaV2.graphql index 77a15a5f481..bbc4f7b110d 100644 --- a/server/graphql/schemaV2.graphql +++ b/server/graphql/schemaV2.graphql @@ -1194,6 +1194,7 @@ enum OrderStatus { EXPIRED DISPUTED REFUNDED + PAUSED IN_REVIEW } @@ -4617,6 +4618,27 @@ type AccountStats { """ frequency: ContributionFrequency! = MONTHLY ): Amount + @deprecated(reason: "2024-03-04: Use activeRecurringContributionsBreakdown while we migrate to better semantics.") + + """ + Returns some statistics about active recurring contributions, broken down by frequency + """ + activeRecurringContributionsBreakdown( + """ + Return only the stats for this frequency + """ + frequency: ContributionFrequency + + """ + Filter contributions on whether they can be ported to another fiscal host directly + """ + hasPortability: Boolean + + """ + Include contributions to children accounts (Projects and Events) + """ + includeChildren: Boolean = false + ): [AmountStats!]! """ Returns expense tags for collective sorted by popularity @@ -5311,6 +5333,7 @@ enum ActivityType { CONTRIBUTION_REJECTED SUBSCRIPTION_ACTIVATED SUBSCRIPTION_CANCELED + SUBSCRIPTION_PAUSED TICKET_CONFIRMED ORDER_CANCELED_ARCHIVED_COLLECTIVE ORDER_PENDING @@ -13864,6 +13887,7 @@ enum ActivityAndClassesType { CONTRIBUTION_REJECTED SUBSCRIPTION_ACTIVATED SUBSCRIPTION_CANCELED + SUBSCRIPTION_PAUSED TICKET_CONFIRMED ORDER_CANCELED_ARCHIVED_COLLECTIVE ORDER_PENDING @@ -16229,7 +16253,16 @@ type Mutation { The account to unhost """ account: AccountReferenceInput! + + """ + An optional message to explain the reason for unhosting + """ message: String + + """ + If true, contributions will be paused rather than canceled + """ + pauseContributions: Boolean! = true ): Account! """ diff --git a/server/graphql/v1/CollectiveInterface.js b/server/graphql/v1/CollectiveInterface.js index e0206e161d7..f93498e3c48 100644 --- a/server/graphql/v1/CollectiveInterface.js +++ b/server/graphql/v1/CollectiveInterface.js @@ -522,7 +522,10 @@ export const CollectiveStatsType = new GraphQLObjectType({ activeRecurringContributions: { type: GraphQLJSON, resolve(collective, args, req) { - return req.loaders.Collective.stats.activeRecurringContributions.load(collective.id); + const loader = req.loaders.Collective.stats.activeRecurringContributions.buildLoader({ + currency: collective.currency, + }); + return loader.load(collective.id); }, }, }; diff --git a/server/graphql/v2/mutation/HostApplicationMutations.ts b/server/graphql/v2/mutation/HostApplicationMutations.ts index 38dabee2baa..10b605933ac 100644 --- a/server/graphql/v2/mutation/HostApplicationMutations.ts +++ b/server/graphql/v2/mutation/HostApplicationMutations.ts @@ -1,6 +1,6 @@ import config from 'config'; import express from 'express'; -import { GraphQLList, GraphQLNonNull, GraphQLObjectType, GraphQLString } from 'graphql'; +import { GraphQLBoolean, GraphQLList, GraphQLNonNull, GraphQLObjectType, GraphQLString } from 'graphql'; import { GraphQLJSON } from 'graphql-scalars'; import { activities } from '../../../constants'; @@ -12,7 +12,7 @@ import { purgeAllCachesForAccount, purgeCacheForCollective } from '../../../lib/ import emailLib from '../../../lib/email'; import * as github from '../../../lib/github'; import { OSCValidator, ValidatedRepositoryInfo } from '../../../lib/osc-validator'; -import { getPolicy, hasPolicy } from '../../../lib/policies'; +import { getPolicy } from '../../../lib/policies'; import { stripHTML } from '../../../lib/sanitize-html'; import twoFactorAuthLib from '../../../lib/two-factor-authentication'; import models, { Collective, Op, sequelize } from '../../../models'; @@ -20,7 +20,7 @@ import ConversationModel from '../../../models/Conversation'; import { HostApplicationStatus } from '../../../models/HostApplication'; import { processInviteMembersInput } from '../../common/members'; import { checkRemoteUserCanUseAccount, checkRemoteUserCanUseHost, checkScope } from '../../common/scope-check'; -import { Forbidden, NotFound, Unauthorized, ValidationFailed } from '../../errors'; +import { Forbidden, NotFound, ValidationFailed } from '../../errors'; import { GraphQLProcessHostApplicationAction } from '../enum/ProcessHostApplicationAction'; import { fetchAccountWithReference, GraphQLAccountReferenceInput } from '../input/AccountReferenceInput'; import { GraphQLInviteMemberInput } from '../input/InviteMemberInput'; @@ -224,6 +224,12 @@ const HostApplicationMutations = { }, message: { type: GraphQLString, + description: 'An optional message to explain the reason for unhosting', + }, + pauseContributions: { + type: new GraphQLNonNull(GraphQLBoolean), + description: 'If true, contributions will be paused rather than canceled', + defaultValue: true, }, }, resolve: async (_, args, req: express.Request): Promise => { @@ -238,19 +244,19 @@ const HostApplicationMutations = { if (!host) { return account; } - if (!req.remoteUser.isAdminOfCollective(host) && !(req.remoteUser.isRoot() && checkScope(req, 'root'))) { - throw new Unauthorized(); - } - if (await hasPolicy(host, POLICIES.REQUIRE_2FA_FOR_ADMINS)) { - await twoFactorAuthLib.validateRequest(req, { - alwaysAskForToken: true, - requireTwoFactorAuthEnabled: true, - FromCollectiveId: host.id, - }); + const isHostAdmin = req.remoteUser.isAdminOfCollective(host); + const isAccountAdmin = req.remoteUser.isAdminOfCollective(account); + if (!isHostAdmin && !isAccountAdmin && !(req.remoteUser.isRoot() && checkScope(req, 'root'))) { + throw new Forbidden('Only the host admin or the account admin can trigger this action'); } - await account.changeHost(null); + await twoFactorAuthLib.enforceForAccountsUserIsAdminOf(req, [account, host], { alwaysAskForToken: true }); + + await account.changeHost(null, req.remoteUser, { + pauseContributions: args.pauseContributions, + messageForContributors: args.message, + }); await models.Activity.create({ type: activities.COLLECTIVE_UNHOSTED, @@ -262,6 +268,8 @@ const HostApplicationMutations = { collective: account.info, host: host.info, message: args.message, + isHostAdmin, + isAccountAdmin, }, }); diff --git a/server/graphql/v2/object/AccountStats.js b/server/graphql/v2/object/AccountStats.js index 541854e01b1..9021b1112a5 100644 --- a/server/graphql/v2/object/AccountStats.js +++ b/server/graphql/v2/object/AccountStats.js @@ -4,7 +4,6 @@ import { get, has, pick } from 'lodash'; import moment from 'moment'; import { getCollectiveIds } from '../../../lib/budget'; -import { getFxRate } from '../../../lib/currency'; import queries from '../../../lib/queries'; import sequelize, { QueryTypes } from '../../../lib/sequelize'; import { computeDatesAsISOStrings } from '../../../lib/utils'; @@ -359,11 +358,16 @@ export const GraphQLAccountStats = new GraphQLObjectType({ type: GraphQLJSON, deprecationReason: '2022-10-21: Use activeRecurringContributionsV2 while we migrate to better semantics.', resolve(collective, args, req) { - return req.loaders.Collective.stats.activeRecurringContributions.load(collective.id); + const loader = req.loaders.Collective.stats.activeRecurringContributions.buildLoader({ + currency: collective.currency, + }); + return loader.load(collective.id); }, }, activeRecurringContributionsV2: { type: GraphQLAmount, + deprecationReason: + '2024-03-04: Use activeRecurringContributionsBreakdown while we migrate to better semantics.', args: { frequency: { type: new GraphQLNonNull(GraphQLContributionFrequency), @@ -376,17 +380,60 @@ export const GraphQLAccountStats = new GraphQLObjectType({ if (!['monthly', 'yearly'].includes(key)) { throw new Error('Unsupported frequency.'); } - const stats = await req.loaders.Collective.stats.activeRecurringContributions.load(collective.id); - const currency = collective.currency; - // There is no guarantee that stats are returned in collective.currency, we convert to be sure - const fxRate = await getFxRate(stats.currency, currency); - const value = Math.round(stats[key] * fxRate); + const loader = req.loaders.Collective.stats.activeRecurringContributions.buildLoader({ + currency: collective.currency, + }); + const stats = await loader.load(collective.id); return { - value: value, - currency: currency, + value: stats[key], + currency: collective.currency, }; }, }, + activeRecurringContributionsBreakdown: { + description: 'Returns some statistics about active recurring contributions, broken down by frequency', + type: new GraphQLNonNull(new GraphQLList(new GraphQLNonNull(GraphQLAmountStats))), + args: { + frequency: { + type: GraphQLContributionFrequency, + description: 'Return only the stats for this frequency', + }, + hasPortability: { + type: GraphQLBoolean, + description: 'Filter contributions on whether they can be ported to another fiscal host directly', + }, + includeChildren: { + type: GraphQLBoolean, + description: 'Include contributions to children accounts (Projects and Events)', + defaultValue: false, + }, + }, + async resolve(collective, args, req) { + const interval = args.frequency?.toLowerCase(); + if (interval && !['monthly', 'yearly'].includes(interval)) { + throw new Error('Unsupported frequency.'); + } + const currency = collective.currency; + const loader = req.loaders.Collective.stats.activeRecurringContributions.buildLoader({ + hasPortability: args.hasPortability, + includeChildren: args.includeChildren, + currency, + }); + const stats = await loader.load(collective.id); + const getStatsForInterval = interval => ({ + label: interval, + count: stats[`${interval}Count`], + amount: stats[interval], + currency, + }); + + if (interval) { + return [getStatsForInterval(interval)]; + } else { + return ['monthly', 'yearly'].map(getStatsForInterval); + } + }, + }, expensesTags: { type: new GraphQLList(GraphQLAmountStats), description: 'Returns expense tags for collective sorted by popularity', diff --git a/server/graphql/v2/object/AmountStats.js b/server/graphql/v2/object/AmountStats.js index 1cd75ed8234..163a1860dc3 100644 --- a/server/graphql/v2/object/AmountStats.js +++ b/server/graphql/v2/object/AmountStats.js @@ -1,4 +1,5 @@ import { GraphQLInt, GraphQLNonNull, GraphQLObjectType, GraphQLString } from 'graphql'; +import { isNil } from 'lodash'; import { GraphQLAmount } from '../object/Amount'; @@ -14,7 +15,7 @@ export const GraphQLAmountStats = new GraphQLObjectType({ type: new GraphQLNonNull(GraphQLAmount), description: 'Total amount for this label', resolve(entry) { - if (entry.amount) { + if (!isNil(entry.amount)) { return { value: entry.amount, currency: entry.currency }; } }, diff --git a/server/lib/emailTemplates.ts b/server/lib/emailTemplates.ts index 7f43d2fdd00..527438a163e 100644 --- a/server/lib/emailTemplates.ts +++ b/server/lib/emailTemplates.ts @@ -92,6 +92,7 @@ export const templateNames = [ 'report.platform', 'report.platform.weekly', 'subscription.canceled', + 'subscription.paused', 'taxform.request', 'ticket.confirmed', 'ticket.confirmed.fearlesscitiesbrussels', diff --git a/server/lib/notifications/email.ts b/server/lib/notifications/email.ts index 3ddb7b13bbe..f9fb436585b 100644 --- a/server/lib/notifications/email.ts +++ b/server/lib/notifications/email.ts @@ -239,6 +239,7 @@ export const notifyByEmail = async (activity: Activity) => { break; case ActivityTypes.SUBSCRIPTION_CANCELED: + case ActivityTypes.SUBSCRIPTION_PAUSED: await notify.user(activity); break; diff --git a/server/lib/recurring-contributions.js b/server/lib/recurring-contributions.js index a22673f0aaf..b93fc809151 100644 --- a/server/lib/recurring-contributions.js +++ b/server/lib/recurring-contributions.js @@ -31,13 +31,22 @@ export const MAX_RETRIES = 6; export async function ordersWithPendingCharges({ limit, startDate } = {}) { return models.Order.findAndCountAll({ where: { + status: { [Op.not]: status.PAUSED }, SubscriptionId: { [Op.ne]: null }, deletedAt: null, }, limit: limit, include: [ { model: models.User, as: 'createdByUser' }, - { model: models.Collective, as: 'collective', required: true }, + { + model: models.Collective, + as: 'collective', + required: true, + where: { + HostCollectiveId: { [Op.not]: null }, + isActive: true, + }, + }, { model: models.Collective, as: 'fromCollective', required: true }, { model: models.PaymentMethod, as: 'paymentMethod' }, { model: models.Tier, as: 'Tier' }, diff --git a/server/lib/timeline.ts b/server/lib/timeline.ts index 63a70f77287..79af3ad2a34 100644 --- a/server/lib/timeline.ts +++ b/server/lib/timeline.ts @@ -154,6 +154,7 @@ const makeTimelineQuery = async ( ActivityTypes.PAYMENT_CREDITCARD_EXPIRING, ActivityTypes.PAYMENT_FAILED, ActivityTypes.SUBSCRIPTION_CANCELED, + ActivityTypes.SUBSCRIPTION_PAUSED, ], ); } diff --git a/server/lib/webhooks.js b/server/lib/webhooks.js index 884b9b23a3b..e5d3f145b36 100644 --- a/server/lib/webhooks.js +++ b/server/lib/webhooks.js @@ -142,7 +142,7 @@ export const sanitizeActivity = activity => { cleanActivity.data = pick(activity.data, ['recipient.name']); cleanActivity.data.tier = getTierInfo(activity.data.tier); cleanActivity.data.order = getOrderInfo(activity.data.order); - } else if (type === activities.SUBSCRIPTION_CANCELED) { + } else if (type === activities.SUBSCRIPTION_CANCELED || type === activities.SUBSCRIPTION_PAUSED) { cleanActivity.data = pick(activity.data, ['subscription.id']); cleanActivity.data.order = getOrderInfo(activity.data.order); cleanActivity.data.tier = getTierInfo(activity.data.tier); diff --git a/server/models/Collective.ts b/server/models/Collective.ts index d2829b8180c..ee37323de99 100644 --- a/server/models/Collective.ts +++ b/server/models/Collective.ts @@ -52,6 +52,7 @@ import { SupportedCurrency } from '../constants/currencies'; import expenseStatus from '../constants/expense-status'; import expenseTypes from '../constants/expense-type'; import FEATURE from '../constants/feature'; +import OrderStatuses from '../constants/order-status'; import { PAYMENT_METHOD_SERVICE, PAYMENT_METHOD_TYPE } from '../constants/paymentMethods'; import plans from '../constants/plans'; import POLICIES, { Policies } from '../constants/policies'; @@ -2412,7 +2413,12 @@ class Collective extends Model< }); } - await Order.cancelNonTransferableActiveOrdersByCollectiveId(this.id); + // Pause or cancel all orders that cannot be transferred + await Order.updateNonTransferableActiveOrdersStatusesByCollectiveId( + this.id, + options?.pauseContributions ? OrderStatuses.PAUSED : OrderStatuses.CANCELLED, + 'Changing host', + ); const virtualCards = await VirtualCard.findAll({ where: { CollectiveId: this.id } }); await Promise.all(virtualCards.map(virtualCard => virtualCard.delete())); diff --git a/server/models/Order.ts b/server/models/Order.ts index e9a04ce6470..ee20e089975 100644 --- a/server/models/Order.ts +++ b/server/models/Order.ts @@ -1,4 +1,5 @@ import { TaxType } from '@opencollective/taxes'; +import config from 'config'; import debugLib from 'debug'; import { get } from 'lodash'; import { @@ -41,7 +42,11 @@ interface OrderModelStaticInterface { generateDescription(collective, amount, interval, tier): string; cancelActiveOrdersByCollective(collectiveId: number): Promise<[affectedCount: number]>; cancelActiveOrdersByTierId(tierId: number): Promise<[affectedCount: number]>; - cancelNonTransferableActiveOrdersByCollectiveId(collectiveId: number): Promise<[affectedCount: number]>; + updateNonTransferableActiveOrdersStatusesByCollectiveId( + collectiveId: number, + newStatus: OrderStatus, + context: string, + ): Promise; clearExpiredLocks(): Promise<[null, number]>; } @@ -667,30 +672,82 @@ Order.cancelActiveOrdersByTierId = function (tierId: number) { }; /** - * Cancels all orders with subscriptions that cannot be transferred when changing hosts (i.e. PayPal) + * Update the status of all non-transferable active orders for the given collective. + * + * The only type of contributions that can be transferred are non-Stripe Connect credit card subscriptions. + * In the future, we could add support for recurring contributions between children and parent collectives. + * + * Note: for externally managed subscriptions (PayPal), marking the orders as CANCELLED will trigger the + * cancellation of the associated subscriptions on the payment provider. + * See `cron/hourly/70-cancel-subscriptions-for-cancelled-orders.ts`. */ -Order.cancelNonTransferableActiveOrdersByCollectiveId = function (collectiveId: number) { - return sequelize.query( +Order.updateNonTransferableActiveOrdersStatusesByCollectiveId = async function ( + collectiveId: number, + newStatus: OrderStatus, + updateContext = 'Bulk update for non-transferable active orders', +): Promise { + const [orders] = await sequelize.query( ` - UPDATE public."Orders" - SET - status = 'CANCELLED', - "updatedAt" = NOW() - WHERE id IN ( - SELECT "Orders".id FROM public."Orders" - INNER JOIN public."Subscriptions" ON "Subscriptions".id = "Orders"."SubscriptionId" - WHERE - "Orders".status NOT IN ('PAID', 'CANCELLED', 'REJECTED', 'EXPIRED') AND - "Subscriptions"."isManagedExternally" AND - "Subscriptions"."isActive" AND - "Orders"."CollectiveId" = ? - ) - `, + UPDATE "Orders" + SET + status = :newStatus, + "updatedAt" = NOW(), + "data" = COALESCE("data", '{}'::JSONB) || JSONB_BUILD_OBJECT('updateContext', :updateContext) + WHERE id IN ( + SELECT "Orders".id FROM "Orders" + INNER JOIN "Subscriptions" ON "Subscriptions".id = "Orders"."SubscriptionId" + LEFT JOIN "PaymentMethods" pm ON "Orders"."PaymentMethodId" = pm.id + LEFT JOIN "PaymentMethods" spm ON pm."SourcePaymentMethodId" = spm.id + WHERE "Orders"."CollectiveId" = :collectiveId + AND "Subscriptions"."isActive" IS TRUE + AND CASE + WHEN spm.id IS NOT NULL THEN ( + -- For gift cards, we need to check the source payment method + spm.service != 'stripe' + OR spm.type != 'creditcard' + OR COALESCE(spm.data#>>'{stripeAccount}', :platformStripeAccountId) != :platformStripeAccountId + ) WHEN pm.id IS NOT NULL THEN ( + -- Only legacy stripe credit card contributions are transferable + pm.service != 'stripe' + OR pm.type != 'creditcard' + OR COALESCE(pm.data#>>'{stripeAccount}', :platformStripeAccountId) != :platformStripeAccountId + ) + ELSE TRUE + END + ) + RETURNING id, "SubscriptionId" + `, { type: QueryTypes.UPDATE, - replacements: [collectiveId], + raw: true, + replacements: { + collectiveId, + newStatus, + updateContext, + platformStripeAccountId: config.stripe.accountId, + }, }, ); + + // Update related subscriptions that are managed internally. For the ones managed externally, we need to get + // the payment provider's confirmation before marking them as cancelled (see `cron/hourly/70-cancel-subscriptions-for-cancelled-orders.ts`). + if ([OrderStatus.CANCELLED, OrderStatus.PAUSED].includes(newStatus) && orders.length) { + await sequelize.query( + ` + UPDATE "Subscriptions" + SET "isActive" = FALSE, "updatedAt" = NOW() + WHERE "id" IN (:subscriptionIds) + AND "isManagedExternally" = FALSE + `, + { + replacements: { + subscriptionIds: orders.map(order => order.SubscriptionId), + }, + }, + ); + } + + return orders.length; }; Temporal(Order, sequelize); diff --git a/templates/emails/collective.unhosted.hbs b/templates/emails/collective.unhosted.hbs index d9430955a57..b4299e52ab2 100644 --- a/templates/emails/collective.unhosted.hbs +++ b/templates/emails/collective.unhosted.hbs @@ -6,11 +6,14 @@ Subject: Important: {{{collective.name}}} has been un-hosted by {{{host.name}}} Hi {{collective.name}},

You have been un-hosted by {{> linkCollective collective=host}}. This Fiscal Host will no longer manage money on behalf of your Collective. Any active recurring PayPal contributions or Virtual Cards will be cancelled. + {{#if pauseContributions}} + Once you're ready to start receiving contributions again (either with a new Fiscal Host or as an Independent Collective), we will help you reach out to your contributors to let them know how to resume their contributions. + {{/if}}

{{#if message}}

- Message from {{host.name}}: + Message from {{#if isHostAdmin}}{{host.name}}{{else}}{{collective.name}}{{/if}}:

{{message}}
{{/if}} diff --git a/templates/emails/subscription.paused.hbs b/templates/emails/subscription.paused.hbs new file mode 100644 index 00000000000..d67b22eb99f --- /dev/null +++ b/templates/emails/subscription.paused.hbs @@ -0,0 +1,24 @@ +Subject: Your contribution to {{{collective.name}}} has been paused + +{{> header}} + +

{{> greeting}}

+ +

Your recurring contribution to {{collective.name}} for {{currency subscription.amount currency=subscription.currency}}/{{subscription.interval}} has been paused.

+ +{{#if reason}} +

The reason given by the collective is:

+
{{reason}}
+{{/if}} + +

+ We'll inform you when your contribution is ready for reactivation. Without any further action from you, your contribution will remained paused. +

+ +

Warmly,

+ +

+ – {{collective.name}} +

+ +{{> footer}} diff --git a/test/server/graphql/v2/mutation/HostApplicationMutations.test.ts b/test/server/graphql/v2/mutation/HostApplicationMutations.test.ts index d1a13ee7a20..486d3225303 100644 --- a/test/server/graphql/v2/mutation/HostApplicationMutations.test.ts +++ b/test/server/graphql/v2/mutation/HostApplicationMutations.test.ts @@ -5,6 +5,7 @@ import { createSandbox } from 'sinon'; import { activities, roles } from '../../../../../server/constants'; import OrderStatuses from '../../../../../server/constants/order-status'; +import { PAYMENT_METHOD_SERVICE, PAYMENT_METHOD_TYPE } from '../../../../../server/constants/paymentMethods'; import { TransactionKind } from '../../../../../server/constants/transaction-kind'; import VirtualCardProviders from '../../../../../server/constants/virtual-card-providers'; import { GraphQLProcessHostApplicationAction } from '../../../../../server/graphql/v2/enum'; @@ -14,12 +15,14 @@ import { VirtualCardStatus } from '../../../../../server/models/VirtualCard'; import * as stripeVirtualCardService from '../../../../../server/paymentProviders/stripe/virtual-cards'; import { randEmail } from '../../../../stores'; import { + fakeActiveHost, fakeCollective, fakeEvent, fakeHost, fakeHostApplication, fakeMember, fakeOrder, + fakePaymentMethod, fakeProject, fakeTier, fakeTransaction, @@ -92,8 +95,8 @@ const PROCESS_HOST_APPLICATION_MUTATION = gql` `; const REMOVE_HOST_MUTATION = gql` - mutation UnhostAccount($account: AccountReferenceInput!, $message: String) { - removeHost(account: $account, message: $message) { + mutation UnhostAccount($account: AccountReferenceInput!, $message: String, $pauseContributions: Boolean) { + removeHost(account: $account, message: $message, pauseContributions: $pauseContributions) { id slug name @@ -410,7 +413,7 @@ describe('server/graphql/v2/mutation/HostApplicationMutations', () => { ); }); - it('requires token with host scope', async () => { + it("can't remove if unauthenticated", async () => { const result = await graphqlQueryV2(REMOVE_HOST_MUTATION, { account: { id: 'some id', @@ -420,6 +423,14 @@ describe('server/graphql/v2/mutation/HostApplicationMutations', () => { expect(result.errors[0].message).to.equal('You need to be logged in to manage hosted accounts.'); }); + it("can't remove if not an admin", async () => { + const randomUser = await fakeUser(); + const collective = await fakeCollective(); + const result = await graphqlQueryV2(REMOVE_HOST_MUTATION, { account: { legacyId: collective.id } }, randomUser); + expect(result.errors).to.exist; + expect(result.errors[0].message).to.equal('Only the host admin or the account admin can trigger this action'); + }); + it('results in error if account does not exist', async () => { const result = await graphqlQueryV2( REMOVE_HOST_MUTATION, @@ -475,6 +486,67 @@ describe('server/graphql/v2/mutation/HostApplicationMutations', () => { expect(unhostingActivity.data.host.id).to.eq(host.id); }); + it('pauses the contributions if requested', async () => { + const host = await fakeActiveHost(); + const collective = await fakeCollective({ HostCollectiveId: host.id }); + const stripePm = await fakePaymentMethod({ + service: PAYMENT_METHOD_SERVICE.STRIPE, + type: PAYMENT_METHOD_TYPE.CREDITCARD, + }); + const transferableOrder = await fakeOrder( + { + status: OrderStatuses.ACTIVE, + CollectiveId: collective.id, + interval: 'month', + subscription: { isManagedExternally: false, isActive: true }, + PaymentMethodId: stripePm.id, + }, + { + withSubscription: true, + }, + ); + + const nonTransferableOrder = await fakeOrder( + { + status: OrderStatuses.ACTIVE, + CollectiveId: collective.id, + interval: 'month', + subscription: { isManagedExternally: true, isActive: true }, + }, + { + withSubscription: true, + }, + ); + + const result = await graphqlQueryV2( + REMOVE_HOST_MUTATION, + { + account: { legacyId: collective.id }, + message: 'We are transitioning to a new host', + pauseContributions: true, + }, + rootUser, + ); + + expect(result.errors).to.not.exist; + expect(result.data.removeHost.host).to.be.null; + + const unhostingActivity = await models.Activity.findOne({ + where: { type: activities.COLLECTIVE_UNHOSTED, CollectiveId: collective.id }, + }); + expect(unhostingActivity).to.exist; + expect(unhostingActivity.data.message).to.eq('We are transitioning to a new host'); + expect(unhostingActivity.data.collective.id).to.eq(collective.id); + expect(unhostingActivity.data.host.id).to.eq(host.id); + + await nonTransferableOrder.reload(); + expect(nonTransferableOrder.status).to.eq(OrderStatuses.PAUSED); + expect(nonTransferableOrder.data.updateContext).to.eq('Changing host'); + + await transferableOrder.reload(); + expect(transferableOrder.status).to.eq(OrderStatuses.ACTIVE); + }); + it('validates if collective does not have a parent account', async () => { const host = await fakeHost(); const collective = await fakeCollective({ HostCollectiveId: host.id }); @@ -529,6 +601,30 @@ describe('server/graphql/v2/mutation/HostApplicationMutations', () => { expect(result.errors[0].message).to.equal('Unable to change host: you still have a balance of $24.00'); }); + it('removes the host as a collective admin', async () => { + const host = await fakeActiveHost(); + const collective = await fakeCollective({ HostCollectiveId: host.id }); + const result = await graphqlQueryV2( + REMOVE_HOST_MUTATION, + { + account: { legacyId: collective.id }, + message: 'removing as collective admin', + }, + rootUser, + ); + + expect(result.errors).to.not.exist; + expect(result.data.removeHost.host).to.be.null; + + const unhostingActivity = await models.Activity.findOne({ + where: { type: activities.COLLECTIVE_UNHOSTED, CollectiveId: collective.id }, + }); + expect(unhostingActivity).to.exist; + expect(unhostingActivity.data.message).to.eq('removing as collective admin'); + expect(unhostingActivity.data.collective.id).to.eq(collective.id); + expect(unhostingActivity.data.host.id).to.eq(host.id); + }); + it('validates if user is admin of target host collective', async () => { const user = await fakeUser(); await fakeCollective({ admin: user }); @@ -545,7 +641,7 @@ describe('server/graphql/v2/mutation/HostApplicationMutations', () => { user, ); expect(result.errors).to.exist; - expect(result.errors[0].message).to.equal('You need to be authenticated to perform this action'); + expect(result.errors[0].message).to.equal('Only the host admin or the account admin can trigger this action'); }); it('removes the host from a hosted collective using host admin', async () => { @@ -555,12 +651,25 @@ describe('server/graphql/v2/mutation/HostApplicationMutations', () => { const user = await fakeUser(); const host = await fakeHost({ admin: user }); const collective = await fakeCollective({ HostCollectiveId: host.id }); + const stripeCreditCardPm = await fakePaymentMethod({ + service: PAYMENT_METHOD_SERVICE.STRIPE, + type: PAYMENT_METHOD_TYPE.CREDITCARD, + }); + const paypalPm = await fakePaymentMethod({ + service: PAYMENT_METHOD_SERVICE.PAYPAL, + type: PAYMENT_METHOD_TYPE.SUBSCRIPTION, + }); const orderWithExternalSubscription = await fakeOrder( - { CollectiveId: collective.id, status: OrderStatuses.ACTIVE, subscription: { isManagedExternally: true } }, + { + CollectiveId: collective.id, + status: OrderStatuses.ACTIVE, + PaymentMethodId: paypalPm.id, + subscription: { isManagedExternally: true }, + }, { withSubscription: true }, ); const orderWithInternalSubscription = await fakeOrder( - { CollectiveId: collective.id, status: OrderStatuses.ACTIVE }, + { CollectiveId: collective.id, status: OrderStatuses.ACTIVE, PaymentMethodId: stripeCreditCardPm.id }, { withSubscription: true }, ); @@ -592,7 +701,7 @@ describe('server/graphql/v2/mutation/HostApplicationMutations', () => { expect(unhostingActivity.data.host.id).to.eq(host.id); await orderWithExternalSubscription.reload(); - expect(orderWithExternalSubscription.status).to.eq(OrderStatuses.CANCELLED); + expect(orderWithExternalSubscription.status).to.eq(OrderStatuses.PAUSED); await orderWithInternalSubscription.reload(); expect(orderWithInternalSubscription.status).to.eq(OrderStatuses.ACTIVE); diff --git a/test/server/lib/recurring-contributions.test.js b/test/server/lib/recurring-contributions.test.js index faa21ed1459..41777fe4396 100644 --- a/test/server/lib/recurring-contributions.test.js +++ b/test/server/lib/recurring-contributions.test.js @@ -18,7 +18,7 @@ import { } from '../../../server/lib/recurring-contributions'; import models from '../../../server/models'; import { randEmail } from '../../stores'; -import { fakeOrder } from '../../test-helpers/fake-data'; +import { fakeCollective, fakeOrder } from '../../test-helpers/fake-data'; import * as utils from '../../utils'; async function createOrderWithSubscription(interval, date, quantity = 1) { @@ -472,7 +472,7 @@ describe('server/lib/recurring-contributions', () => { beforeEach(async () => { await utils.resetTestDB(); user = await models.User.createUserWithCollective({ email: randEmail(), name: 'Test McTesterson' }); - collective = await models.Collective.create({ name: 'Parcel' }); + collective = await fakeCollective({ name: 'Parcel' }); tier = await models.Tier.create({ name: 'backer', amount: 0, CollectiveId: collective.id }); }); diff --git a/test/server/models/Order.test.js b/test/server/models/Order.test.js index e3a920a1d07..7a26ee3c2a1 100644 --- a/test/server/models/Order.test.js +++ b/test/server/models/Order.test.js @@ -4,7 +4,7 @@ import sinon from 'sinon'; import models from '../../../server/models'; import { randEmail } from '../../stores'; -import { fakeOrder } from '../../test-helpers/fake-data'; +import { fakeCollective, fakeOrder, fakePaymentMethod, randStr } from '../../test-helpers/fake-data'; import * as utils from '../../utils'; describe('server/models/Order', () => { @@ -122,4 +122,97 @@ describe('server/models/Order', () => { ]); }); }); + + describe('updateNonTransferableActiveOrdersStatusesByCollectiveId', () => { + it('cancels non transferable active orders', async () => { + const collective = await fakeCollective(); + + // Payment methods + const paypalPm = await fakePaymentMethod({ + service: 'paypal', + type: 'subscription', + }); + const stripePm = await fakePaymentMethod({ + service: 'stripe', + type: 'creditcard', + }); + const stripeGiftCardPm = await fakePaymentMethod({ + service: 'opencollective', + type: 'giftcard', + SourcePaymentMethodId: stripePm.id, + }); + const stripeConnectPm = await fakePaymentMethod({ + service: 'stripe', + type: 'creditcard', + data: { stripeAccount: randStr() }, + }); + const stripeConnectGiftCardPm = await fakePaymentMethod({ + service: 'opencollective', + type: 'giftcard', + SourcePaymentMethodId: stripeConnectPm.id, + }); + + // Orders + const fakeActiveOrder = async PaymentMethodId => { + return fakeOrder( + { + CollectiveId: collective.id, + status: 'ACTIVE', + PaymentMethodId, + }, + { + withSubscription: true, + }, + ); + }; + + const activePaypalOrder = await fakeActiveOrder(paypalPm.id); + const activeStripeOrder = await fakeActiveOrder(stripePm.id); + const activeStripeGiftCardOrder = await fakeActiveOrder(stripeGiftCardPm.id); + const activeStripeConnectOrder = await fakeActiveOrder(stripeConnectPm.id); + const activeStripeConnectGiftCardOrder = await fakeActiveOrder(stripeConnectGiftCardPm.id); + const inactiveOrder = await fakeOrder({ + CollectiveId: collective.id, + status: 'CANCELLED', + PaymentMethodId: paypalPm.id, + }); + + // Trigger the update + const nbUpdated = await models.Order.updateNonTransferableActiveOrdersStatusesByCollectiveId( + collective.id, + 'PAUSED', + 'Testing', + ); + + // Check the results + expect(nbUpdated).to.equal(3); + await activePaypalOrder.reload({ include: [models.Subscription] }); + await activeStripeOrder.reload({ include: [models.Subscription] }); + await activeStripeGiftCardOrder.reload({ include: [models.Subscription] }); + await activeStripeConnectOrder.reload({ include: [models.Subscription] }); + await activeStripeConnectGiftCardOrder.reload({ include: [models.Subscription] }); + await inactiveOrder.reload({ include: [models.Subscription] }); + + expect(activePaypalOrder.status).to.equal('PAUSED'); + expect(activeStripeOrder.status).to.equal('ACTIVE'); // Do not touch stripe orders since they can be transfered + expect(activeStripeGiftCardOrder.status).to.equal('ACTIVE'); + expect(activeStripeConnectOrder.status).to.equal('PAUSED'); + expect(activeStripeConnectGiftCardOrder.status).to.equal('PAUSED'); + expect(inactiveOrder.status).to.equal('CANCELLED'); + + expect(activePaypalOrder.data.updateContext).to.equal('Testing'); + expect(activeStripeOrder.data?.updateContext).to.not.exist; + expect(activeStripeGiftCardOrder.data?.updateContext).to.not.exist; + expect(activeStripeConnectOrder.data.updateContext).to.equal('Testing'); + expect(activeStripeConnectGiftCardOrder.data.updateContext).to.equal('Testing'); + expect(inactiveOrder.data?.updateContext).to.not.exist; + + // Check the subscriptions + expect(activePaypalOrder.Subscription.isActive).to.be.false; + expect(activeStripeOrder.Subscription.isActive).to.be.true; + expect(activeStripeGiftCardOrder.Subscription.isActive).to.be.true; + expect(activeStripeConnectOrder.Subscription.isActive).to.be.false; + expect(activeStripeConnectGiftCardOrder.Subscription.isActive).to.be.false; + }); + }); }); From 0d2b0b90dc577a6458aa8941bd33229c9535b8e5 Mon Sep 17 00:00:00 2001 From: Benjamin Piouffle Date: Thu, 7 Mar 2024 11:52:52 +0100 Subject: [PATCH 2/2] enhancement: Iterate on pausing recurring contributions --- ...> 70-handle-batch-subscriptions-update.ts} | 36 +++++++--- server/graphql/v2/mutation/OrderMutations.js | 2 + server/lib/notifications/index.ts | 6 +- server/models/Activity.ts | 8 ++- server/models/Collective.ts | 8 +-- server/models/Order.ts | 67 ++++--------------- templates/emails/subscription.paused.hbs | 8 +-- ...handle-batch-subscriptions-update.test.ts} | 14 ++-- .../mutation/HostApplicationMutations.test.ts | 6 +- test/server/models/Order.test.js | 47 ++++++------- 10 files changed, 90 insertions(+), 112 deletions(-) rename cron/hourly/{70-cancel-subscriptions-for-cancelled-orders.ts => 70-handle-batch-subscriptions-update.ts} (76%) rename test/cron/hourly/{70-cancel-subscriptions-for-cancelled-orders.test.ts => 70-handle-batch-subscriptions-update.test.ts} (93%) diff --git a/cron/hourly/70-cancel-subscriptions-for-cancelled-orders.ts b/cron/hourly/70-handle-batch-subscriptions-update.ts similarity index 76% rename from cron/hourly/70-cancel-subscriptions-for-cancelled-orders.ts rename to cron/hourly/70-handle-batch-subscriptions-update.ts index 3b90e3cc3c9..0dc72105d65 100644 --- a/cron/hourly/70-cancel-subscriptions-for-cancelled-orders.ts +++ b/cron/hourly/70-handle-batch-subscriptions-update.ts @@ -2,7 +2,8 @@ import '../../server/env'; -import { groupBy, size, uniq } from 'lodash'; +import { groupBy, size, sortBy, uniq } from 'lodash'; +import moment from 'moment'; import { activities } from '../../server/constants'; import FEATURE from '../../server/constants/feature'; @@ -14,7 +15,7 @@ import models, { Collective, Op } from '../../server/models'; import { OrderModelInterface } from '../../server/models/Order'; /** - * Since the collective has been archived, its HostCollectiveId has been set to null. + * If the collective has been archived, its HostCollectiveId has been set to null. * We need some more logic to make sure we're loading the right host. */ const getHostFromOrder = async order => { @@ -37,12 +38,12 @@ const getOrderCancelationReason = ( order: OrderModelInterface, orderHost: Collective, ): { - code: 'TRANSFER' | 'DELETED_TIER' | 'ARCHIVED_ACCOUNT' | 'UNHOSTED_COLLECTIVE' | 'CHANGED_HOST' | 'CANCELLED_ORDER'; + code: 'PAUSED' | 'DELETED_TIER' | 'ARCHIVED_ACCOUNT' | 'UNHOSTED_COLLECTIVE' | 'CHANGED_HOST' | 'CANCELLED_ORDER'; message: string; } => { - if (order.status === 'PAUSED' && order.data?.pausedForTransfer) { + if (order.status === 'PAUSED') { return { - code: 'TRANSFER', + code: 'PAUSED', message: `Your contribution to the Collective was paused. We'll inform you when it will be ready for re-activation.`, }; } else if (order.TierId && !order.Tier) { @@ -65,7 +66,13 @@ const getOrderCancelationReason = ( */ export async function run() { const orphanOrders = await models.Order.findAll({ - where: { status: [OrderStatuses.CANCELLED, OrderStatuses.PAUSED] }, + where: { + status: [OrderStatuses.CANCELLED, OrderStatuses.PAUSED], + data: { needsAsyncDeactivation: true }, + updatedAt: { + [Op.gt]: moment().subtract(1, 'month').toDate(), // For performance, only look at orders updated recently + }, + }, include: [ { model: models.Tier, @@ -91,10 +98,11 @@ export async function run() { logger.info(`Found ${orphanOrders.length} recurring contributions to cancel across ${size(groupedOrders)} accounts`); for (const accountOrders of Object.values(groupedOrders)) { - const collective = accountOrders[0].collective; + const sortedAccountOrders = sortBy(accountOrders, ['Subscription.isManagedExternally']); + const collective = sortedAccountOrders[0].collective; const collectiveHandle = collective.slug; - logger.info(`Cancelling ${accountOrders.length} subscriptions for @${collectiveHandle}`); - for (const order of accountOrders) { + logger.info(`Cancelling ${sortedAccountOrders.length} subscriptions for @${collectiveHandle}`); + for (const order of sortedAccountOrders) { try { const host = await getHostFromOrder(order); const reason = getOrderCancelationReason(collective, order, host); @@ -104,7 +112,7 @@ export async function run() { if (!process.env.DRY) { await order.Subscription.deactivate(reason.message, host); await models.Activity.create({ - type: reason.code === 'TRANSFER' ? activities.SUBSCRIPTION_PAUSED : activities.SUBSCRIPTION_CANCELED, + type: reason.code === 'PAUSED' ? activities.SUBSCRIPTION_PAUSED : activities.SUBSCRIPTION_CANCELED, CollectiveId: order.CollectiveId, FromCollectiveId: order.FromCollectiveId, HostCollectiveId: order.collective.HostCollectiveId, @@ -116,11 +124,17 @@ export async function run() { fromCollective: order.fromCollective.minimal, reasonCode: reason.code, reason: reason.message, + messageForContributors: order.data?.messageForContributors, order: order.info, tier: order.Tier?.info, + awaitForDispatch: true, // To make sure we won't kill the process while emails are still being sent }, }); - await sleep(500); // To prevent rate-limiting issues when calling 3rd party payment processor APIs + + await order.update({ data: { ...order.data, needsAsyncDeactivation: false } }); + if (order.Subscription.isManagedExternally) { + await sleep(500); // To prevent rate-limiting issues when calling 3rd party payment processor APIs + } } } catch (e) { logger.error(`Error while cancelling subscriptions for @${collectiveHandle}: ${e.message}`); diff --git a/server/graphql/v2/mutation/OrderMutations.js b/server/graphql/v2/mutation/OrderMutations.js index 5e4c7dafdc0..5b7f1c3504f 100644 --- a/server/graphql/v2/mutation/OrderMutations.js +++ b/server/graphql/v2/mutation/OrderMutations.js @@ -357,6 +357,8 @@ const orderMutations = { throw new Unauthorized("You don't have permission to update this order"); } else if (!order.Subscription.isActive) { throw new Error('Order must be active to be updated'); + } else if (order.status === OrderStatuses.PAUSED) { + throw new Error('Paused orders cannot be updated'); } else if (args.paypalSubscriptionId && args.paymentMethod) { throw new Error('paypalSubscriptionId and paymentMethod are mutually exclusive'); } else if (haveDetailsChanged && hasPaymentMethodChanged) { diff --git a/server/lib/notifications/index.ts b/server/lib/notifications/index.ts index 2f98829effb..191f6da09cc 100644 --- a/server/lib/notifications/index.ts +++ b/server/lib/notifications/index.ts @@ -43,11 +43,13 @@ const dispatch = async (activity: Activity, { onlyChannels = null, force = false const shouldNotifyChannel = channel => !onlyChannels || onlyChannels.includes(channel); if (shouldNotifyChannel(channels.EMAIL)) { - notifyByEmail(activity).catch(e => { + try { + await notifyByEmail(activity); + } catch (e) { if (!['ci', 'test', 'e2e'].includes(config.env)) { console.error(e); } - }); + } } // process notification entries for slack, twitter, etc... diff --git a/server/models/Activity.ts b/server/models/Activity.ts index cc8c6019e32..96be80cf6d4 100644 --- a/server/models/Activity.ts +++ b/server/models/Activity.ts @@ -105,11 +105,13 @@ Activity.init( sequelize, updatedAt: false, hooks: { - afterCreate(activity) { + async afterCreate(activity) { if (activity.data?.notify !== false) { - dispatch(activity); // intentionally no return statement, needs to be async + const dispatchPromise = dispatch(activity); // intentionally no return statement, needs to be async by default + if (activity.data?.awaitForDispatch) { + await dispatchPromise; + } } - return Promise.resolve(); }, }, }, diff --git a/server/models/Collective.ts b/server/models/Collective.ts index ee37323de99..276b82319c7 100644 --- a/server/models/Collective.ts +++ b/server/models/Collective.ts @@ -2414,12 +2414,10 @@ class Collective extends Model< } // Pause or cancel all orders that cannot be transferred - await Order.updateNonTransferableActiveOrdersStatusesByCollectiveId( - this.id, - options?.pauseContributions ? OrderStatuses.PAUSED : OrderStatuses.CANCELLED, - 'Changing host', - ); + const newOrderStatus = options?.pauseContributions ? OrderStatuses.PAUSED : OrderStatuses.CANCELLED; + await Order.stopActiveSubscriptions(this.id, newOrderStatus, options?.messageForContributors); + // Delete all virtual cards const virtualCards = await VirtualCard.findAll({ where: { CollectiveId: this.id } }); await Promise.all(virtualCards.map(virtualCard => virtualCard.delete())); diff --git a/server/models/Order.ts b/server/models/Order.ts index ee20e089975..bb5431c48dc 100644 --- a/server/models/Order.ts +++ b/server/models/Order.ts @@ -1,5 +1,4 @@ import { TaxType } from '@opencollective/taxes'; -import config from 'config'; import debugLib from 'debug'; import { get } from 'lodash'; import { @@ -42,11 +41,7 @@ interface OrderModelStaticInterface { generateDescription(collective, amount, interval, tier): string; cancelActiveOrdersByCollective(collectiveId: number): Promise<[affectedCount: number]>; cancelActiveOrdersByTierId(tierId: number): Promise<[affectedCount: number]>; - updateNonTransferableActiveOrdersStatusesByCollectiveId( - collectiveId: number, - newStatus: OrderStatus, - context: string, - ): Promise; + stopActiveSubscriptions(collectiveId: number, newStatus: OrderStatus, context: string): Promise; clearExpiredLocks(): Promise<[null, number]>; } @@ -677,45 +672,30 @@ Order.cancelActiveOrdersByTierId = function (tierId: number) { * The only type of contributions that can be transferred are non-Stripe Connect credit card subscriptions. * In the future, we could add support for recurring contributions between children and parent collectives. * - * Note: for externally managed subscriptions (PayPal), marking the orders as CANCELLED will trigger the - * cancellation of the associated subscriptions on the payment provider. - * See `cron/hourly/70-cancel-subscriptions-for-cancelled-orders.ts`. + * Note: the linked subscriptions will be cancelled in `cron/hourly/70-handle-batch-subscriptions-update.ts`. */ -Order.updateNonTransferableActiveOrdersStatusesByCollectiveId = async function ( +Order.stopActiveSubscriptions = async function ( collectiveId: number, - newStatus: OrderStatus, - updateContext = 'Bulk update for non-transferable active orders', -): Promise { - const [orders] = await sequelize.query( + newStatus: OrderStatus.CANCELLED | OrderStatus.PAUSED, + messageForContributors: string, +): Promise { + // Update all orders + await sequelize.query( ` UPDATE "Orders" SET status = :newStatus, "updatedAt" = NOW(), - "data" = COALESCE("data", '{}'::JSONB) || JSONB_BUILD_OBJECT('updateContext', :updateContext) + "data" = COALESCE("data", '{}'::JSONB) || JSONB_BUILD_OBJECT( + 'messageForContributors', :messageForContributors, + 'needsAsyncDeactivation', TRUE + ) WHERE id IN ( SELECT "Orders".id FROM "Orders" INNER JOIN "Subscriptions" ON "Subscriptions".id = "Orders"."SubscriptionId" - LEFT JOIN "PaymentMethods" pm ON "Orders"."PaymentMethodId" = pm.id - LEFT JOIN "PaymentMethods" spm ON pm."SourcePaymentMethodId" = spm.id WHERE "Orders"."CollectiveId" = :collectiveId AND "Subscriptions"."isActive" IS TRUE - AND CASE - WHEN spm.id IS NOT NULL THEN ( - -- For gift cards, we need to check the source payment method - spm.service != 'stripe' - OR spm.type != 'creditcard' - OR COALESCE(spm.data#>>'{stripeAccount}', :platformStripeAccountId) != :platformStripeAccountId - ) WHEN pm.id IS NOT NULL THEN ( - -- Only legacy stripe credit card contributions are transferable - pm.service != 'stripe' - OR pm.type != 'creditcard' - OR COALESCE(pm.data#>>'{stripeAccount}', :platformStripeAccountId) != :platformStripeAccountId - ) - ELSE TRUE - END ) - RETURNING id, "SubscriptionId" `, { type: QueryTypes.UPDATE, @@ -723,31 +703,10 @@ Order.updateNonTransferableActiveOrdersStatusesByCollectiveId = async function ( replacements: { collectiveId, newStatus, - updateContext, - platformStripeAccountId: config.stripe.accountId, + messageForContributors: messageForContributors || '', }, }, ); - - // Update related subscriptions that are managed internally. For the ones managed externally, we need to get - // the payment provider's confirmation before marking them as cancelled (see `cron/hourly/70-cancel-subscriptions-for-cancelled-orders.ts`). - if ([OrderStatus.CANCELLED, OrderStatus.PAUSED].includes(newStatus) && orders.length) { - await sequelize.query( - ` - UPDATE "Subscriptions" - SET "isActive" = FALSE, "updatedAt" = NOW() - WHERE "id" IN (:subscriptionIds) - AND "isManagedExternally" = FALSE - `, - { - replacements: { - subscriptionIds: orders.map(order => order.SubscriptionId), - }, - }, - ); - } - - return orders.length; }; Temporal(Order, sequelize); diff --git a/templates/emails/subscription.paused.hbs b/templates/emails/subscription.paused.hbs index d67b22eb99f..ce1dcd1f608 100644 --- a/templates/emails/subscription.paused.hbs +++ b/templates/emails/subscription.paused.hbs @@ -6,13 +6,13 @@ Subject: Your contribution to {{{collective.name}}} has been paused

Your recurring contribution to {{collective.name}} for {{currency subscription.amount currency=subscription.currency}}/{{subscription.interval}} has been paused.

-{{#if reason}} -

The reason given by the collective is:

-
{{reason}}
+{{#if messageForContributors}} +

From {{collective.name}}:

+
{{messageForContributors}}
{{/if}}

- We'll inform you when your contribution is ready for reactivation. Without any further action from you, your contribution will remained paused. + We'll let you know when your contribution is ready for reactivation. Without any further action from you, your contribution will remain paused.

Warmly,

diff --git a/test/cron/hourly/70-cancel-subscriptions-for-cancelled-orders.test.ts b/test/cron/hourly/70-handle-batch-subscriptions-update.test.ts similarity index 93% rename from test/cron/hourly/70-cancel-subscriptions-for-cancelled-orders.test.ts rename to test/cron/hourly/70-handle-batch-subscriptions-update.test.ts index 8975dd10e0c..666330233dc 100644 --- a/test/cron/hourly/70-cancel-subscriptions-for-cancelled-orders.test.ts +++ b/test/cron/hourly/70-handle-batch-subscriptions-update.test.ts @@ -1,7 +1,7 @@ import { expect } from 'chai'; import { assert, createSandbox } from 'sinon'; -import { run as runCronJob } from '../../../cron/hourly/70-cancel-subscriptions-for-cancelled-orders'; +import { run as runCronJob } from '../../../cron/hourly/70-handle-batch-subscriptions-update'; import { activities } from '../../../server/constants'; import OrderStatuses from '../../../server/constants/order-status'; import { PAYMENT_METHOD_SERVICE, PAYMENT_METHOD_TYPE } from '../../../server/constants/paymentMethods'; @@ -35,6 +35,7 @@ const fakePayPalSubscriptionOrder = async collective => { totalAmount: 1000, subscription: { paypalSubscriptionId, isActive: true, isManagedExternally: true }, PaymentMethodId: paymentMethod.id, + data: { needsAsyncDeactivation: true }, }, { withTier: true, @@ -44,7 +45,7 @@ const fakePayPalSubscriptionOrder = async collective => { ); }; -describe('cron/hourly/70-cancel-subscriptions-for-cancelled-orders', () => { +describe('cron/hourly/70-handle-batch-subscriptions-update', () => { let sandbox, host; beforeEach(async () => { @@ -93,7 +94,7 @@ describe('cron/hourly/70-cancel-subscriptions-for-cancelled-orders', () => { it('cancels subscriptions from canceled orders', async () => { const collective = await fakeCollective({ HostCollectiveId: host.id, slug: 'test-collective' }); const order = await fakeOrder( - { CollectiveId: collective.id }, + { CollectiveId: collective.id, data: { needsAsyncDeactivation: true } }, { withSubscription: true, withTransactions: true, withTier: true }, ); @@ -116,7 +117,12 @@ describe('cron/hourly/70-cancel-subscriptions-for-cancelled-orders', () => { const collective = await fakeCollective({ HostCollectiveId: host.id, slug: 'test-collective' }); const tier = await fakeTier({ CollectiveId: collective.id }); const order = await fakeOrder( - { CollectiveId: collective.id, TierId: tier.id, status: OrderStatuses.ACTIVE }, + { + CollectiveId: collective.id, + TierId: tier.id, + status: OrderStatuses.ACTIVE, + data: { needsAsyncDeactivation: true }, + }, { withSubscription: true, withTransactions: true, withTier: true }, ); diff --git a/test/server/graphql/v2/mutation/HostApplicationMutations.test.ts b/test/server/graphql/v2/mutation/HostApplicationMutations.test.ts index 486d3225303..6e22d53206e 100644 --- a/test/server/graphql/v2/mutation/HostApplicationMutations.test.ts +++ b/test/server/graphql/v2/mutation/HostApplicationMutations.test.ts @@ -541,10 +541,10 @@ describe('server/graphql/v2/mutation/HostApplicationMutations', () => { await nonTransferableOrder.reload(); expect(nonTransferableOrder.status).to.eq(OrderStatuses.PAUSED); - expect(nonTransferableOrder.data.updateContext).to.eq('Changing host'); + expect(nonTransferableOrder.data.messageForContributors).to.eq('We are transitioning to a new host'); await transferableOrder.reload(); - expect(transferableOrder.status).to.eq(OrderStatuses.ACTIVE); + expect(transferableOrder.status).to.eq(OrderStatuses.PAUSED); }); it('validates if collective does not have a parent account', async () => { @@ -704,7 +704,7 @@ describe('server/graphql/v2/mutation/HostApplicationMutations', () => { expect(orderWithExternalSubscription.status).to.eq(OrderStatuses.PAUSED); await orderWithInternalSubscription.reload(); - expect(orderWithInternalSubscription.status).to.eq(OrderStatuses.ACTIVE); + expect(orderWithInternalSubscription.status).to.eq(OrderStatuses.PAUSED); await virtualCard.reload(); expect(virtualCard.data.status).to.eq(VirtualCardStatus.CANCELED); diff --git a/test/server/models/Order.test.js b/test/server/models/Order.test.js index 7a26ee3c2a1..129dbf2da5a 100644 --- a/test/server/models/Order.test.js +++ b/test/server/models/Order.test.js @@ -123,8 +123,8 @@ describe('server/models/Order', () => { }); }); - describe('updateNonTransferableActiveOrdersStatusesByCollectiveId', () => { - it('cancels non transferable active orders', async () => { + describe('stopActiveSubscriptions', () => { + it('pauses all active orders', async () => { const collective = await fakeCollective(); // Payment methods @@ -173,19 +173,14 @@ describe('server/models/Order', () => { const activeStripeConnectGiftCardOrder = await fakeActiveOrder(stripeConnectGiftCardPm.id); const inactiveOrder = await fakeOrder({ CollectiveId: collective.id, - status: 'CANCELLED', + status: 'PAID', PaymentMethodId: paypalPm.id, }); // Trigger the update - const nbUpdated = await models.Order.updateNonTransferableActiveOrdersStatusesByCollectiveId( - collective.id, - 'PAUSED', - 'Testing', - ); + await models.Order.stopActiveSubscriptions(collective.id, 'PAUSED', 'Testing'); // Check the results - expect(nbUpdated).to.equal(3); await activePaypalOrder.reload({ include: [models.Subscription] }); await activeStripeOrder.reload({ include: [models.Subscription] }); await activeStripeGiftCardOrder.reload({ include: [models.Subscription] }); @@ -194,25 +189,25 @@ describe('server/models/Order', () => { await inactiveOrder.reload({ include: [models.Subscription] }); expect(activePaypalOrder.status).to.equal('PAUSED'); - expect(activeStripeOrder.status).to.equal('ACTIVE'); // Do not touch stripe orders since they can be transfered - expect(activeStripeGiftCardOrder.status).to.equal('ACTIVE'); + expect(activeStripeOrder.status).to.equal('PAUSED'); + expect(activeStripeGiftCardOrder.status).to.equal('PAUSED'); expect(activeStripeConnectOrder.status).to.equal('PAUSED'); expect(activeStripeConnectGiftCardOrder.status).to.equal('PAUSED'); - expect(inactiveOrder.status).to.equal('CANCELLED'); - - expect(activePaypalOrder.data.updateContext).to.equal('Testing'); - expect(activeStripeOrder.data?.updateContext).to.not.exist; - expect(activeStripeGiftCardOrder.data?.updateContext).to.not.exist; - expect(activeStripeConnectOrder.data.updateContext).to.equal('Testing'); - expect(activeStripeConnectGiftCardOrder.data.updateContext).to.equal('Testing'); - expect(inactiveOrder.data?.updateContext).to.not.exist; - - // Check the subscriptions - expect(activePaypalOrder.Subscription.isActive).to.be.false; - expect(activeStripeOrder.Subscription.isActive).to.be.true; - expect(activeStripeGiftCardOrder.Subscription.isActive).to.be.true; - expect(activeStripeConnectOrder.Subscription.isActive).to.be.false; - expect(activeStripeConnectGiftCardOrder.Subscription.isActive).to.be.false; + expect(inactiveOrder.status).to.equal('PAID'); + + expect(activePaypalOrder.data.messageForContributors).to.equal('Testing'); + expect(activeStripeOrder.data?.messageForContributors).to.equal('Testing'); + expect(activeStripeGiftCardOrder.data?.messageForContributors).to.equal('Testing'); + expect(activeStripeConnectOrder.data.messageForContributors).to.equal('Testing'); + expect(activeStripeConnectGiftCardOrder.data.messageForContributors).to.equal('Testing'); + expect(inactiveOrder.data?.messageForContributors).to.not.exist; + + expect(activePaypalOrder.data.needsAsyncDeactivation).to.be.true; + expect(activeStripeOrder.data.needsAsyncDeactivation).to.be.true; + expect(activeStripeGiftCardOrder.data.needsAsyncDeactivation).to.be.true; + expect(activeStripeConnectOrder.data.needsAsyncDeactivation).to.be.true; + expect(activeStripeConnectGiftCardOrder.data.needsAsyncDeactivation).to.be.true; + expect(inactiveOrder.data?.needsAsyncDeactivation).to.not.exist; }); }); });