Skip to content

Commit f719b3c

Browse files
fix(billing): address 3 CodeRabbit findings on pricing contract + refund service
CR1: pin exact safe defaults in contract test (toEqual instead of typeof) so regressions that populate RATIOS/STRIPE_PRICE_CENTS/STRIPE_PACK_CENTS by default are caught. CR2: guard options param in refundCharge before destructuring — null arg caused raw TypeError; now throws a clear validation error. CR3: wiring test imports development.config.js directly (not config/index.js) so it deterministically targets the new billing.pricing block in the defaults file, not whichever env the full pipeline resolves.
1 parent 5968959 commit f719b3c

3 files changed

Lines changed: 25 additions & 23 deletions

File tree

lib/services/tests/billing-pricing-constants.contract.unit.tests.js

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,19 @@ import {
88
} from '../../../config/defaults/billing.pricing.constants.js';
99

1010
describe('billing.pricing.constants — devkit contract:', () => {
11-
test('PRICING_VERSION is a non-empty string with YYYY.MM shape (or 0.0.0 default)', () => {
12-
expect(typeof PRICING_VERSION).toBe('string');
13-
expect(PRICING_VERSION).toMatch(/^\d+\.\d+(\.\d+)?$/);
11+
test('PRICING_VERSION is the safe default 0.0.0', () => {
12+
expect(PRICING_VERSION).toBe('0.0.0');
1413
});
15-
test('PLAN_QUOTAS is an object with at least `free` key (numeric)', () => {
16-
expect(typeof PLAN_QUOTAS).toBe('object');
17-
expect(typeof PLAN_QUOTAS.free).toBe('number');
14+
test('PLAN_QUOTAS is the safe default { free: 0 }', () => {
15+
expect(PLAN_QUOTAS).toEqual({ free: 0 });
1816
});
19-
test('RATIOS is an object (may be empty by default)', () => {
20-
expect(typeof RATIOS).toBe('object');
17+
test('RATIOS is the safe default empty object', () => {
18+
expect(RATIOS).toEqual({});
2119
});
22-
test('STRIPE_PRICE_CENTS is an object (may be empty by default)', () => {
23-
expect(typeof STRIPE_PRICE_CENTS).toBe('object');
20+
test('STRIPE_PRICE_CENTS is the safe default empty object', () => {
21+
expect(STRIPE_PRICE_CENTS).toEqual({});
2422
});
25-
test('STRIPE_PACK_CENTS is an object (may be empty by default)', () => {
26-
expect(typeof STRIPE_PACK_CENTS).toBe('object');
23+
test('STRIPE_PACK_CENTS is the safe default empty object', () => {
24+
expect(STRIPE_PACK_CENTS).toEqual({});
2725
});
2826
});

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@ import getStripe from '../lib/stripe.js';
1818
* @returns {Promise<Object>} The Stripe refund object.
1919
*/
2020
// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik
21-
const refundCharge = async (stripeChargeId, amountCents, { reason } = {}) => {
21+
const refundCharge = async (stripeChargeId, amountCents, options = {}) => {
22+
if (options === null || typeof options !== 'object' || Array.isArray(options)) {
23+
throw new Error('invalid argument: options must be a plain object');
24+
}
25+
const { reason } = options;
2226
if (typeof stripeChargeId !== 'string' || stripeChargeId.trim() === '') {
2327
throw new Error('invalid argument: stripeChargeId must be a non-empty string');
2428
}
Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
import { describe, test, expect } from '@jest/globals';
2-
import config from '../../../config/index.js';
2+
import defaults from '../../../config/defaults/development.config.js';
33

4-
describe('config.billing.pricing wiring:', () => {
4+
describe('config.billing.pricing wiring (development.config.js):', () => {
55
test('exposes PRICING_VERSION, PLAN_QUOTAS, RATIOS, STRIPE_PRICE_CENTS, STRIPE_PACK_CENTS', () => {
6-
expect(config.billing).toBeDefined();
7-
expect(config.billing.pricing).toBeDefined();
8-
expect(config.billing.pricing.PRICING_VERSION).toBeDefined();
9-
expect(config.billing.pricing.PLAN_QUOTAS).toBeDefined();
10-
expect(config.billing.pricing.RATIOS).toBeDefined();
11-
expect(config.billing.pricing.STRIPE_PRICE_CENTS).toBeDefined();
12-
expect(config.billing.pricing.STRIPE_PACK_CENTS).toBeDefined();
6+
expect(defaults.billing).toBeDefined();
7+
expect(defaults.billing.pricing).toBeDefined();
8+
expect(defaults.billing.pricing.PRICING_VERSION).toBeDefined();
9+
expect(defaults.billing.pricing.PLAN_QUOTAS).toBeDefined();
10+
expect(defaults.billing.pricing.RATIOS).toBeDefined();
11+
expect(defaults.billing.pricing.STRIPE_PRICE_CENTS).toBeDefined();
12+
expect(defaults.billing.pricing.STRIPE_PACK_CENTS).toBeDefined();
1313
});
1414
test('PLAN_QUOTAS has at least `free` numeric entry by default', () => {
15-
expect(typeof config.billing.pricing.PLAN_QUOTAS.free).toBe('number');
15+
expect(typeof defaults.billing.pricing.PLAN_QUOTAS.free).toBe('number');
1616
});
1717
});

0 commit comments

Comments
 (0)