-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Description
Describe the bug
Fixed by #4136
When refunding order items across multiple payments, the refundOrder mutation creates duplicate RefundLine records in the database. This occurs because:
- The Admin UI (both Angular and React dashboard) sends the same
linesarray to each payment's refund call - The backend
refundOrdermutation does not validate whether the line quantities have already been recorded in a previous refund - Each call blindly creates new
RefundLineentities, resulting in inflated refund quantity tracking
This causes RefundLine data to incorrectly report more items refunded than actually exist in the order, breaking any logic that calculates remaining refundable quantities.
To Reproduce
- Create an order with 2 items (e.g., 2x Balloon Chairs at $40 each = $80 total)
- Complete the order with two payments:
- Payment A: $50 (e.g., gift card)
- Payment B: $30 (e.g., another gift card)
- Open the Admin UI and navigate to the order
- Click "Refund & Cancel"
- Select both Balloon Chairs for refund (quantity: 2)
- The refund total is $80, which will be split across both payments ($50 from A, $30 from B)
- Submit the refund
- Query the database for RefundLine records associated with this order
Expected behavior*
The database should contain RefundLine records totaling 2 items (the actual quantity refunded):
Actual behavior
The database contains RefundLine records totaling 4 items (double the actual quantity):
This causes:
getRefundedQuantity()calculations to return incorrect values- Probably business logic errors
Error logs
If applicable, paste any relevant error messages or stack traces:
Minimal reproduction
See the linked PR that adds a test in the e2e suite that reproduces it:
#4136
Additional context
In packages/core/src/service/services/payment.service.ts:373-383:
const refundLines: RefundLine[] = [];
for (const { orderLineId, quantity } of input.lines || []) {
const refundLine = await this.connection.getRepository(ctx, RefundLine).save(
new RefundLine({
refund,
orderLineId,
quantity,
}),
);
refundLines.push(refundLine);
}1. Backend creates RefundLines without validation
In packages/core/src/service/services/payment.service.ts:373-383:
const refundLines: RefundLine[] = [];
for (const { orderLineId, quantity } of input.lines || []) {
const refundLine = await this.connection.getRepository(ctx, RefundLine).save(
new RefundLine({
refund,
orderLineId,
quantity,
}),
);
refundLines.push(refundLine);
}This code creates new RefundLine records for every refundOrder call without checking:
- Whether this orderLineId already has RefundLine records
- Whether the cumulative refunded quantity would exceed
orderPlacedQuantity
2. AlreadyRefundedError exists but is never used
The error type AlreadyRefundedError is defined in the GraphQL schema (packages/core/src/api/schema/admin-api/order.api.graphql:327-331):
"Returned if an attempting to refund an OrderItem which has already been refunded"
type AlreadyRefundedError implements ErrorResult {
errorCode: ErrorCode!
message: String!
refundId: ID!
}And in TypeScript (packages/core/src/common/error/generated-graphql-admin-errors.ts:22-26):
export class AlreadyRefundedError extends ErrorResult {
readonly __typename = 'AlreadyRefundedError';
readonly errorCode = 'ALREADY_REFUNDED_ERROR' as any;
readonly message = 'ALREADY_REFUNDED_ERROR';
readonly refundId: Scalars['ID'];
}However, this error is never thrown anywhere in the service layer. It appears to be dead code or an incomplete feature.
3. Only monetary validation exists
In packages/core/src/service/services/payment.service.ts:302-304:
const refundableAmount = paymentToRefund.amount - this.getPaymentRefundTotal(paymentToRefund);
if (refundableAmount < input.amount) {
return new RefundAmountError({ maximumRefundable: refundableAmount });
}This validates that the monetary amount doesn't exceed what's refundable from the payment. But there is no validation that the line quantities don't exceed what's available to refund.
4. Admin UI sends same lines to all payments
In packages/admin-ui/src/lib/order/src/components/refund-order-dialog/refund-order-dialog.component.ts:184-195:
this.resolveWith({
refunds: this.refundablePayments
.filter(rp => rp.selected && 0 < rp.amountToRefundControl.value)
.map(payment => {
return {
lines: refundLines, // SAME lines for ALL payments
reason: this.reason,
paymentId: payment.id,
amount: payment.amountToRefundControl.value,
shipping: 0,
adjustment: 0,
};
}),
// ...
});