feat(payment): PAYMENTS-11577 submit purchase order number as paymentData to createOrder api for purchase order - #3251
feat(payment): PAYMENTS-11577 submit purchase order number as paymentData to createOrder api for purchase order#3251bc-alfreid wants to merge 3 commits into
Conversation
…purchase order in createOrder endpoint
… for offline strategy
d173764 to
e079558
Compare
4b832c9 to
97083bc
Compare
There was a problem hiding this comment.
Thanks @bc-alfreid. I know it's a draft but let me share my initial thoughts with you. My comments below are based on my idea that HostedInstrument is not the best type that should define the PO data. If you think otherwise, let's discuss first. Also, those suggested changes will touch the core package which is owned by checkout team and must be approved by them. It could be a good idea to get their insights first before addressing my comments to avoid rework in case they don't agree.
| interface PurchaseOrderPaymentData extends HostedInstrument { | ||
| purchaseOrderNumber?: string; | ||
| } |
There was a problem hiding this comment.
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.
| purchaseOrderNumber?: string; | ||
| } | ||
|
|
||
| function isPurchaseOrderPaymentData(data: unknown): data is PurchaseOrderPaymentData { |
There was a problem hiding this comment.
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.
| function isPurchaseOrderPaymentData(data: unknown): data is PurchaseOrderPaymentData { | |
| function isWithPurchaseOrder(data: OrderPaymentRequestBody['paymentData']): data is WithPurchaseOrder { |
| let purchaseOrderPaymentData: PurchaseOrderPaymentData | undefined; | ||
|
|
||
| if ( | ||
| payment?.methodId === 'purchaseorder' && | ||
| isPurchaseOrderPaymentData(payment.paymentData) | ||
| ) { | ||
| purchaseOrderPaymentData = { | ||
| purchaseOrderNumber: payment.paymentData.purchaseOrderNumber, | ||
| }; | ||
| } |
There was a problem hiding this comment.
All we need to do here is to throw an error if the payment method is PO and PO data is not provided.
| 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.
| payment: payment | ||
| ? { | ||
| methodId: payment.methodId, | ||
| ...(purchaseOrderPaymentData && { | ||
| paymentData: purchaseOrderPaymentData, | ||
| }), | ||
| } | ||
| : undefined, |
There was a problem hiding this comment.
| payment: payment | |
| ? { | |
| methodId: payment.methodId, | |
| ...(purchaseOrderPaymentData && { | |
| paymentData: purchaseOrderPaymentData, | |
| }), | |
| } | |
| : undefined, | |
| payment: { | |
| methodId: payment.methodId, | |
| ...(isPurchaseOrderPaymentMethod(payment.methodId) && { | |
| paymentData: payment.paymentData, | |
| }), | |
| }, |
What/Why?
submit purchase order number as paymentData to createOrder api for purchase order
Rollout/Rollback
Revert
Testing
TBD