Skip to content

Commit 036e1bb

Browse files
feat(billing): refund packId + attachUsageHeader + admin endpoints (PR-N7) (#3556)
* feat(billing): refund packId + attachUsageHeader + admin endpoints (PR-N7) P1.4 + P1.5 + P1.6 — round out the billing surface required for safe meter mode. - P1.4: refund correlation now uses packId from charge.metadata when present (no more ambiguous_pack_match silently failing). Legacy fallback to meterUnits heuristic preserved for topups created before the change. createExtrasCheckout already stamped packId in both session.metadata and payment_intent_data.metadata, so no session-side change needed. - P1.5: new billing.attachUsageHeader config flag (default false). When true and meterMode also true, billing routes mount attachUsageContext middleware → emits X-Meter-Remaining header. - P1.6: new admin endpoints, gated by existing CASL platform-admin grant: - POST /api/admin/billing/refund → BillingRefundService.refundCharge - POST /api/admin/billing/plans/bump → BillingPlanService.bumpVersion Zod-validated bodies (invalid → 422 per existing model.isValid). * fix(billing): address review — guard order, reason enum, amountCents optional, bumpVersionWithRetry - billing.routes.js: move attachUsageContext after policy.isAllowed to avoid unnecessary DB reads on unauthorized requests - billing.subscription.schema.js: constrain AdminRefundRequest.reason to Stripe enum ['duplicate','fraudulent','requested_by_customer']; make amountCents optional to support full refunds; require meterQuota in AdminBumpPlanRequest (mirrors Mongoose schema required:true + BillingPlanBump Zod schema) - billing.admin.controller.js: use bumpVersionWithRetry (retry on E11000 race); return 502 Bad Gateway for unexpected errors instead of 422 - billing.extra.service.js: update refundPartial @returns JSDoc to include optional reason field in early-exit paths - tests: mock bumpVersionWithRetry, add tests for reason enum validation, full refund without amountCents, and bump without meterQuota (422) * test(billing): add error-path coverage for admin controller (502 + 422) Cover the upstream error (502 Bad Gateway) and invalid-argument error (422) branches in both adminRefundCharge and adminBumpPlanVersion to restore codecov/project coverage to pre-PR baseline.
1 parent efd7bbc commit 036e1bb

14 files changed

Lines changed: 828 additions & 36 deletions

modules/billing/config/billing.development.config.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ const config = {
4040
* Used by middleware error responses (METER_EXHAUSTED, QUOTA_EXCEEDED).
4141
*/
4242
upgradeUrl: '/billing/plans',
43+
/**
44+
* When true, mounts attachUsageContext on protected /api/billing/* routes.
45+
* Emits X-Meter-Remaining on billing responses. Off by default to avoid
46+
* extra DB reads on routes that do not need the header.
47+
*/
48+
attachUsageHeader: false,
4349
/**
4450
* Plan definitions — DOWNSTREAM-OVERRIDE-REQUIRED for meter mode.
4551
* Used by BillingPlanService.ensureSeeded() at boot to upsert BillingPlan docs.
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/**
2+
* Module dependencies
3+
*/
4+
import responses from '../../../lib/helpers/responses.js';
5+
import BillingRefundService from '../services/billing.refund.service.js';
6+
import BillingPlanService from '../services/billing.plan.service.js';
7+
8+
/**
9+
* @desc Admin endpoint to trigger a Stripe refund for a charge
10+
* @param {Object} req - Express request object
11+
* @param {Object} res - Express response object
12+
* @returns {Promise<void>}
13+
*/
14+
const adminRefundCharge = async (req, res) => {
15+
try {
16+
const { chargeId, amountCents, reason } = req.body;
17+
const refund = await BillingRefundService.refundCharge(chargeId, amountCents, { reason });
18+
responses.success(res, 'billing refund created')(refund);
19+
} catch (err) {
20+
const status = err.message?.startsWith('invalid argument') ? 422 : 502;
21+
const title = status === 422 ? 'Unprocessable Entity' : 'Bad Gateway';
22+
responses.error(res, status, title, 'Failed to refund charge')(err);
23+
}
24+
};
25+
26+
/**
27+
* @desc Admin endpoint to create a new plan version
28+
* @param {Object} req - Express request object
29+
* @param {Object} res - Express response object
30+
* @returns {Promise<void>}
31+
*/
32+
const adminBumpPlanVersion = async (req, res) => {
33+
try {
34+
const { planId, meterQuota, ratios } = req.body;
35+
const nextPlan = await BillingPlanService.bumpVersionWithRetry(planId, { meterQuota, ratios });
36+
responses.success(res, 'billing plan version bumped')(nextPlan);
37+
} catch (err) {
38+
const status = err.message?.startsWith('invalid argument') ? 422 : 502;
39+
const title = status === 422 ? 'Unprocessable Entity' : 'Bad Gateway';
40+
responses.error(res, status, title, 'Failed to bump billing plan version')(err);
41+
}
42+
};
43+
44+
export default {
45+
adminRefundCharge,
46+
adminBumpPlanVersion,
47+
};

modules/billing/models/billing.subscription.schema.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,34 @@ const ExtrasCheckoutRequest = z
7676
})
7777
.strict();
7878

79+
const AdminRefundRequest = z
80+
.object({
81+
chargeId: z.string().trim().min(1, 'chargeId is required'),
82+
amountCents: z.number().int().positive().optional(),
83+
// Stripe refund reason: https://stripe.com/docs/api/refunds/create#create_refund-reason
84+
reason: z.enum(['duplicate', 'fraudulent', 'requested_by_customer']).optional(),
85+
})
86+
.strict();
87+
88+
const AdminBumpPlanRequest = z
89+
.object({
90+
planId: z.string().trim().min(1, 'planId is required'),
91+
meterQuota: z.number().int().nonnegative(),
92+
ratios: z.record(z.string(), z.number().nonnegative()).optional(),
93+
})
94+
.strict();
95+
96+
export {
97+
AdminRefundRequest,
98+
AdminBumpPlanRequest,
99+
};
100+
79101
export default {
80102
Subscription,
81103
SubscriptionUpdate,
82104
CheckoutRequest,
83105
PortalRequest,
84106
ExtrasCheckoutRequest,
107+
AdminRefundRequest,
108+
AdminBumpPlanRequest,
85109
};

modules/billing/policies/billing.policy.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ export function billingSubjectRegistration({ registerPathSubject }) {
1919
registerPathSubject('/api/billing/extras/checkout', 'BillingExtrasCheckout');
2020
registerPathSubject('/api/billing/extras/balance', 'BillingExtrasBalance');
2121
registerPathSubject('/api/billing/extras/ledger', 'BillingExtrasLedger');
22+
registerPathSubject('/api/admin/billing/refund', 'BillingAdminRefund');
23+
registerPathSubject('/api/admin/billing/plans/bump', 'BillingAdminPlanBump');
2224
// Webhook is mounted in billing.preroute.js before body parsing and auth middleware,
2325
// so it bypasses policy.isAllowed entirely. Registered here for documentation completeness.
2426
registerPathSubject('/api/billing/webhook', 'BillingWebhook');
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* Module dependencies
3+
*/
4+
import passport from 'passport';
5+
6+
import policy from '../../../lib/middlewares/policy.js';
7+
import model from '../../../lib/middlewares/model.js';
8+
import billingAdmin from '../controllers/billing.admin.controller.js';
9+
import {
10+
AdminRefundRequest,
11+
AdminBumpPlanRequest,
12+
} from '../models/billing.subscription.schema.js';
13+
14+
/**
15+
* Routes
16+
* @param {Object} app - Express application instance
17+
* @returns {void}
18+
*/
19+
export default (app) => {
20+
app
21+
.route('/api/admin/billing/refund')
22+
.all(passport.authenticate('jwt', { session: false }), policy.isAllowed)
23+
.post(model.isValid(AdminRefundRequest), billingAdmin.adminRefundCharge);
24+
25+
app
26+
.route('/api/admin/billing/plans/bump')
27+
.all(passport.authenticate('jwt', { session: false }), policy.isAllowed)
28+
.post(model.isValid(AdminBumpPlanRequest), billingAdmin.adminBumpPlanVersion);
29+
};
Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/**
22
* Module dependencies
33
*/
4+
import config from '../../../config/index.js';
45
import passport from 'passport';
56

67
import model from '../../../lib/middlewares/model.js';
@@ -9,55 +10,65 @@ import organization from '../../organizations/middlewares/organizations.middlewa
910
import billingSchema from '../models/billing.subscription.schema.js';
1011
import billingPlans from '../controllers/billing.plans.controller.js';
1112
import billing from '../controllers/billing.controller.js';
13+
import attachUsageContext from '../middlewares/billing.attachUsageContext.js';
1214

1315
/**
1416
* @desc Register billing routes
1517
* @param {Object} app - Express application instance
1618
* @returns {void}
1719
*/
1820
export default (app) => {
21+
const billingGuards = [
22+
passport.authenticate('jwt', { session: false }),
23+
organization.resolveOrganization,
24+
policy.isAllowed,
25+
...(config.billing?.meterMode === true && config.billing?.attachUsageHeader === true
26+
? [attachUsageContext]
27+
: []),
28+
];
29+
1930
// plans (public)
2031
app.route('/api/billing/plans').get(billingPlans.getPlans);
2132

2233
// checkout
2334
app
2435
.route('/api/billing/checkout')
25-
.all(passport.authenticate('jwt', { session: false }), organization.resolveOrganization, policy.isAllowed)
36+
.all(...billingGuards)
2637
.post(model.isValid(billingSchema.CheckoutRequest), billing.checkout);
2738

2839
// portal
2940
app
3041
.route('/api/billing/portal')
31-
.all(passport.authenticate('jwt', { session: false }), organization.resolveOrganization, policy.isAllowed)
42+
.all(...billingGuards)
3243
.post(model.isValid(billingSchema.PortalRequest), billing.portal);
3344

3445
// subscription
3546
app
3647
.route('/api/billing/subscription')
37-
.all(passport.authenticate('jwt', { session: false }), organization.resolveOrganization, policy.isAllowed)
48+
.all(...billingGuards)
3849
.get(billing.getSubscription);
3950

4051
// usage
4152
app
4253
.route('/api/billing/usage')
43-
.all(passport.authenticate('jwt', { session: false }), organization.resolveOrganization, policy.isAllowed)
54+
.all(...billingGuards)
4455
.get(billing.getUsage);
4556

4657
// extras checkout (one-time pack purchase)
4758
app
4859
.route('/api/billing/extras/checkout')
49-
.all(passport.authenticate('jwt', { session: false }), organization.resolveOrganization, policy.isAllowed)
60+
.all(...billingGuards)
5061
.post(model.isValid(billingSchema.ExtrasCheckoutRequest), billing.extrasCheckout);
5162

5263
// extras balance
5364
app
5465
.route('/api/billing/extras/balance')
55-
.all(passport.authenticate('jwt', { session: false }), organization.resolveOrganization, policy.isAllowed)
66+
.all(...billingGuards)
5667
.get(billing.extrasBalance);
5768

5869
// extras ledger (paginated: ?page=1&limit=20)
5970
app
6071
.route('/api/billing/extras/ledger')
61-
.all(passport.authenticate('jwt', { session: false }), organization.resolveOrganization, policy.isAllowed)
72+
.all(...billingGuards)
6273
.get(billing.extrasLedger);
6374
};

modules/billing/services/billing.extra.service.js

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,17 @@ const expireOldEntries = (orgId) =>
7171
* @param {string} orgId - The organization ObjectId (string).
7272
* @param {string} stripeSessionId - Stripe session ID of the original purchase (to find the pack).
7373
* @param {number} amountRefundedCents - Amount refunded in cents (integer).
74-
* @returns {Promise<{doc: Object, applied: boolean, refundUnits: number}>}
74+
* @param {string|undefined} [packId] - Optional pack identifier from Stripe metadata for exact correlation.
75+
* @returns {Promise<{doc: Object|null, applied: boolean, refundUnits: number, reason?: string}>}
76+
* `reason` is present only when `applied === false`: 'meter_mode_disabled' | 'invalid_org' |
77+
* 'pack_not_found' | 'ambiguous_pack_match'.
7578
*/
7679
// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik
77-
const refundPartial = async (orgId, stripeSessionId, amountRefundedCents) => {
80+
const refundPartial = async (orgId, stripeSessionId, amountRefundedCents, packId) => {
81+
if (!config?.billing?.meterMode) {
82+
return { doc: null, applied: false, reason: 'meter_mode_disabled', refundUnits: 0 };
83+
}
84+
7885
// Find the topup ledger entry for this session to identify the pack
7986
const doc = await BillingExtraBalanceRepository.getOrCreate(orgId);
8087
if (!doc) return { doc: null, applied: false, reason: 'invalid_org', refundUnits: 0 };
@@ -87,23 +94,27 @@ const refundPartial = async (orgId, stripeSessionId, amountRefundedCents) => {
8794
return { doc, applied: false, refundUnits: 0 };
8895
}
8996

90-
// Find the pack config to compute proportion using the topup entry's meterUnits.
91-
// Ambiguity guard: if 0 or >1 packs share the same meterUnits, fall back to
92-
// applied=false rather than using a wrong priceUsd.
93-
// NOTE: packId is not passed to refundPartial — charge.refunded carries it in
94-
// charge.metadata but the webhook call-site only passes (orgId, stripeSessionId, amountCents).
95-
// This heuristic is therefore intentionally retained; the ambiguity guard is the safety net.
9697
const packs = config?.billing?.packs ?? [];
97-
const matchingPacks = packs.filter(
98-
(p) => (p.meterUnits ?? p.computeUnits) === topupEntry.amount,
99-
);
98+
let matchingPack;
99+
100+
if (packId) {
101+
matchingPack = packs.find((p) => p.packId === packId);
102+
if (!matchingPack) {
103+
return { doc, applied: false, reason: 'pack_not_found', refundUnits: 0 };
104+
}
105+
} else {
106+
const matchingPacks = packs.filter(
107+
(p) => (p.meterUnits ?? p.computeUnits) === topupEntry.amount,
108+
);
109+
110+
if (matchingPacks.length !== 1) {
111+
// Ambiguous or unknown pack — cannot compute proportional refund safely.
112+
return { doc, applied: false, reason: 'ambiguous_pack_match', refundUnits: 0 };
113+
}
100114

101-
if (matchingPacks.length !== 1) {
102-
// Ambiguous or unknown pack — cannot compute proportional refund safely.
103-
return { doc, applied: false, reason: 'ambiguous_pack_match', refundUnits: 0 };
115+
matchingPack = matchingPacks[0];
104116
}
105117

106-
const matchingPack = matchingPacks[0];
107118
let refundUnits;
108119
if (matchingPack.priceUsd && matchingPack.priceUsd > 0) {
109120
const packUnits = matchingPack.meterUnits ?? matchingPack.computeUnits;

modules/billing/services/billing.refund.service.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@ import getStripe from '../lib/stripe.js';
1313
*
1414
* @param {string} stripeChargeId - Stripe charge ID (ch_xxx). Must be non-empty.
1515
* @param {number|undefined} [amountCents] - Amount to refund in cents. Omit for full refund. Must be > 0 if provided.
16+
* @param {Object} [options={}] - Optional Stripe refund options.
17+
* @param {string} [options.reason] - Optional Stripe refund reason.
1618
* @returns {Promise<Object>} The Stripe refund object.
1719
*/
1820
// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik
19-
const refundCharge = async (stripeChargeId, amountCents) => {
21+
const refundCharge = async (stripeChargeId, amountCents, { reason } = {}) => {
2022
if (typeof stripeChargeId !== 'string' || stripeChargeId.trim() === '') {
2123
throw new Error('invalid argument: stripeChargeId must be a non-empty string');
2224
}
@@ -33,7 +35,7 @@ const refundCharge = async (stripeChargeId, amountCents) => {
3335

3436
const params = {
3537
charge: stripeChargeId,
36-
reason: 'requested_by_customer',
38+
reason: reason || 'requested_by_customer',
3739
};
3840
if (amountCents !== undefined) {
3941
params.amount = amountCents;

modules/billing/services/billing.webhook.service.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -339,12 +339,12 @@ const handleInvoicePaymentSucceeded = async (invoice) => {
339339

340340
/**
341341
* @description Handle charge.refunded event — debit ledger proportionally.
342-
* Reads charge.metadata.{organizationId, stripeSessionId} which must be propagated
342+
* Reads charge.metadata.{organizationId, stripeSessionId, packId} which must be propagated
343343
* via the upstream session creation pattern:
344344
* stripe.checkout.sessions.create({
345345
* ...,
346-
* metadata: { organizationId, stripeSessionId, ... },
347-
* payment_intent_data: { metadata: { organizationId, stripeSessionId, ... } },
346+
* metadata: { organizationId, stripeSessionId, packId, ... },
347+
* payment_intent_data: { metadata: { organizationId, stripeSessionId, packId, ... } },
348348
* })
349349
* Without payment_intent_data.metadata, charge.metadata will be empty and refunds
350350
* silently skip. Downstream (trawl_node) is responsible for setting these at session creation.
@@ -363,7 +363,7 @@ const handleChargeRefunded = async (charge) => {
363363
// The session ID and organizationId must have been stamped on charge metadata
364364
// via payment_intent_data.metadata at session creation (not automatic — caller must set both
365365
// session.metadata and payment_intent_data.metadata explicitly).
366-
const { organizationId, stripeSessionId } = metadata ?? {};
366+
const { organizationId, stripeSessionId, packId } = metadata ?? {};
367367

368368
if (!organizationId || !mongoose.Types.ObjectId.isValid(organizationId)) return;
369369
if (!stripeSessionId) return;
@@ -374,7 +374,7 @@ const handleChargeRefunded = async (charge) => {
374374
if (!thisRefundAmount || thisRefundAmount <= 0) return;
375375

376376
// Service layer computes proportional refundUnits from config.billing.packs.
377-
await BillingExtraService.refundPartial(organizationId, stripeSessionId, thisRefundAmount);
377+
await BillingExtraService.refundPartial(organizationId, stripeSessionId, thisRefundAmount, packId);
378378
};
379379

380380
export default {

0 commit comments

Comments
 (0)