Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ECP-9431] Handling Refund Delay and Order Cancellation Issues when using Giftcard + redirected Payment method #2771

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open

[ECP-9431] Handling Refund Delay and Order Cancellation Issues when using Giftcard + redirected Payment method #2771

wants to merge 22 commits into from

Conversation

khushboo-singhvi
Copy link
Contributor

Description
When a customer places an order using a gift card in combination with any redirected payment method, an order is created which redeems the gift card balance, even if the full payment is not completed. As a result, the customer is unable to use the gift card again, despite the full order is not successfully placed.
This PR handles this situation wherein we cancel the already authorised partial payments(Giftcard Payments) if the payment with the redirect payment method fails.

Tested scenarios
Klarna, 3DS, Ideal Successful and failing situations tested.

Fixes

@khushboo-singhvi khushboo-singhvi requested a review from a team as a code owner October 15, 2024 14:39
@khushboo-singhvi khushboo-singhvi changed the title Handling Refund Delay and Order Cancellation Issues when using Giftcard + redirected Payment method [ECP-9431] Handling Refund Delay and Order Cancellation Issues when using Giftcard + redirected Payment method Oct 16, 2024
candemiralp
candemiralp reviewed Oct 23, 2024
View reviewed changes
Helper/PaymentResponseHandler.php Outdated
Comment on lines 293 to 347
$getGiftcardDetails = $this->hasActiveGiftCardPayments(
$paymentsDetailsResponse['merchantReference']
);
if (!empty($getGiftcardDetails)) {
//Cancel the Authorised Payments
$storeId = $order->getStoreId();
$client = $this->dataHelper->initializeAdyenClient($storeId);
$service = $this->dataHelper->initializeOrdersApi($client);
foreach ($getGiftcardDetails as $giftcardData) {
try {
// Decode JSON response and validate it
$response = json_decode($giftcardData['response'], true);
if (json_last_error() !== JSON_ERROR_NONE || !isset($response['order'])) {
throw new InvalidArgumentException('Invalid giftcard response data');
}

// Extract order data and PSPRef
$orderData = $response['order']['orderData'] ?? null;
$pspReference = $response['order']['pspReference'] ?? null;

if (!$orderData || !$pspReference) {
throw new InvalidArgumentException('Missing orderData or pspReference in the response');
}

// Prepare cancel request
$merchantAccount = $this->configHelper->getAdyenAbstractConfigData("merchant_account", $storeId);
$cancelRequest = [
'order' => [
'pspReference' => $pspReference,
'orderData' => $orderData,
],
'merchantAccount' => $merchantAccount,
];

// Call the cancel service
$cancelResponse = $service->cancelOrder(new CancelOrderRequest($cancelRequest));
$response = $cancelResponse->toArray();

if (is_null($response['resultCode'])) {
// In case the result is unknown we log the request and don't update the history
$this->adyenLogger->error(
"Unexpected result query parameter for cancel order request. Response: " . json_encode($response)
);

return false;
}
} catch (\Exception $e) {
// Log the error with relevant information for debugging
$this->adyenLogger->error('Error canceling partial payments', [
'exception' => $e->getMessage(),
'giftcardData' => $giftcardData,
]);
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

What would you think about moving this logic to a separate private method? Currently, it decreases the readability and increases the overall complexity of the switch block.

candemiralp
candemiralp reviewed Oct 23, 2024
View reviewed changes
Helper/PaymentResponseHandler.php Outdated
"Unexpected result query parameter for cancel order request. Response: " . json_encode($response)
);

return false;
Copy link
Member

@candemiralp candemiralp Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Returning false here will terminate the execution of the rest of the method where we cancel the Magento order. In addition, checking the validity of a single field of an API response is not scalable.

Alternatively, you can catch the exception thrown from the PHP API Library and log the error message only. For responseCode 200, there is only one available value in the API Explorer which is resultCode: Received. So, if there is no exception thrown from the library, we can assume everything is good.

Edit: There is already a catch block wrapped around. A separate catch block for the API call might not be required.

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Oct 23, 2024

Please retry analysis of this Pull-Request directly on SonarCloud

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Oct 23, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
88.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

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

@candemiralp candemiralp Awaiting requested review from candemiralp

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@khushboo-singhvi @candemiralp