Skip to content

Add support for CheckoutSession#129

Open
shahbaz-mehar wants to merge 9 commits into
mainfrom
support-checkout-session
Open

Add support for CheckoutSession#129
shahbaz-mehar wants to merge 9 commits into
mainfrom
support-checkout-session

Conversation

@shahbaz-mehar
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

@BushraAsif BushraAsif left a comment

Choose a reason for hiding this comment

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

Changes reviewed

Comment thread views/paymentClass.tpl
->setShopOrderId( $order_id )
->setAmount( round( $amount, 2 ) )
->setCurrency( $currency )
->setSessionId( $order->get_order_key() );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In WooCommerce, the order_key acts as a bearer capability token—anyone with this key can view the order receipt and potentially access customer PII (address, email).

Is this a problem?

Maybe we can create a hash derived from the order_key, so the identifier is always the same, but it can't be used to see order information?

Comment on lines -122 to -123
} else {
$renewal_order->payment_complete();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Was this intended?

Comment thread views/paymentClass.tpl
Comment on lines +284 to +287
$sessionId = WC()->session->get( 'altapay_checkout_session_id' );
if ( $sessionId !== $order->get_order_key() ) {
$sessionId = null;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this the right approach?

If for any reason a session is created at at time when woocommerce is upgraded, changing the order_key, or we deploy a change that changes the way you define the sessionId, we will see a lot of errors in the flow.

Once the session is created, and established, why not relying on that session, even if it does not have a predefined format you expected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants