Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,70 @@ describe('OfflinePaymentStrategy', () => {
);
});

it('includes only purchaseOrderNumber in paymentData when methodId is purchaseorder', async () => {
const payload = {
...getOrderRequestBody(),
payment: {
methodId: 'purchaseorder',
paymentData: {
purchaseOrderNumber: '1111111',
shouldCreateAccount: true,
shouldSaveInstrument: false,
terms: false,
},
},
};

await strategy.execute(payload, undefined);

expect(paymentIntegrationService.submitOrder).toHaveBeenCalledWith(
{
...payload,
payment: {
methodId: 'purchaseorder',
paymentData: {
purchaseOrderNumber: '1111111',
},
},
},
undefined,
);
});

it('does not include paymentData when methodId is purchaseorder but paymentData is absent', async () => {
const payload = {
...getOrderRequestBody(),
payment: {
methodId: 'purchaseorder',
},
};

await strategy.execute(payload, undefined);

expect(paymentIntegrationService.submitOrder).toHaveBeenCalledWith(
{
...payload,
payment: {
methodId: 'purchaseorder',
},
},
undefined,
);
});

it('does not include paymentData for non-purchaseorder offline methods', async () => {
await strategy.execute(getOrderRequestBody(), undefined);

expect(paymentIntegrationService.submitOrder).toHaveBeenCalledWith(
expect.objectContaining({
payment: {
methodId: 'authorizenet',
},
}),
undefined,
);
});

it('passes the options to submitOrder', async () => {
const options = { myOptions: 'option1', methodId: 'testgateway' };

Expand Down
30 changes: 29 additions & 1 deletion packages/offline-integration/src/offline-payment-strategy.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,47 @@
import {
HostedInstrument,
OrderFinalizationNotRequiredError,
OrderRequestBody,
PaymentIntegrationService,
PaymentRequestOptions,
PaymentStrategy,
} from '@bigcommerce/checkout-sdk/payment-integration-api';

interface PurchaseOrderPaymentData extends HostedInstrument {
purchaseOrderNumber?: string;
}
Comment on lines +10 to +12

@Tharaae Tharaae May 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think purchase order should not extend HostedInstrument as it does not have any of its characteristics. For example, its two fields shouldSaveInstrument and shouldSetAsDefaultInstrument are not relevant to purchase order. AFAIK, PO is a generated number that could be is different for each order and there will be no need to save it as an instrument.

I think with should introduce a new WithPurchaseOrder type to OrderPaymentRequestBody.paymentData and to PaymentInstrument type as well (like WithBankAccountInstrument) where its declaration is simply:

interface WithPurchaseOrder {
  purchaseOrderNumber: string;
}

This will simplify the execute() method as in the following comments.


function isPurchaseOrderPaymentData(data: unknown): data is PurchaseOrderPaymentData {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data is not quite unknown here. We know that it's payment data. We should also check on the same new type we will add to OrderPaymentRequestBody as per the previous comment.

Suggested change
function isPurchaseOrderPaymentData(data: unknown): data is PurchaseOrderPaymentData {
function isWithPurchaseOrder(data: OrderPaymentRequestBody['paymentData']): data is WithPurchaseOrder {

return typeof data === 'object' && data !== null && 'purchaseOrderNumber' in data;
}

export default class OfflinePaymentStrategy implements PaymentStrategy {
constructor(private _paymentIntegrationService: PaymentIntegrationService) {}

async execute(payload: OrderRequestBody, options?: PaymentRequestOptions): Promise<void> {
const { payment } = payload;
let purchaseOrderPaymentData: PurchaseOrderPaymentData | undefined;

if (
payment?.methodId === 'purchaseorder' &&
isPurchaseOrderPaymentData(payment.paymentData)
) {
purchaseOrderPaymentData = {
purchaseOrderNumber: payment.paymentData.purchaseOrderNumber,
};
}
Comment on lines +23 to +32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All we need to do here is to throw an error if the payment method is PO and PO data is not provided.

Suggested change
let purchaseOrderPaymentData: PurchaseOrderPaymentData | undefined;
if (
payment?.methodId === 'purchaseorder' &&
isPurchaseOrderPaymentData(payment.paymentData)
) {
purchaseOrderPaymentData = {
purchaseOrderNumber: payment.paymentData.purchaseOrderNumber,
};
}
if (
!payment ||
(isPurchaseOrderPaymentMethod(payment) && !isWithPurchaseOrder(payment.paymentData))
) {
throw new MissingDataError(MissingDataErrorType.MissingPaymentData);
}

where isPurchaseOrderPaymentMethod is a utility function that returns true if payment?.methodId === 'purchaseorder'. We should create this function because it will be used again.


await this._paymentIntegrationService.submitOrder(
{
...payload,
payment: payload.payment ? { methodId: payload.payment.methodId } : undefined,
payment: payment
? {
methodId: payment.methodId,
...(purchaseOrderPaymentData && {
paymentData: purchaseOrderPaymentData,
}),
}
: undefined,
Comment on lines +37 to +44

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
payment: payment
? {
methodId: payment.methodId,
...(purchaseOrderPaymentData && {
paymentData: purchaseOrderPaymentData,
}),
}
: undefined,
payment: {
methodId: payment.methodId,
...(isPurchaseOrderPaymentMethod(payment.methodId) && {
paymentData: payment.paymentData,
}),
},

},
options,
);
Expand Down