Skip to content
This repository has been archived by the owner on Jan 25, 2024. It is now read-only.

Fix order status handling #75

Merged

Conversation

mgiadach
Copy link
Contributor

The main purpose of this fix is to actually use the "Estado de Nueva Orden" defined by the admin in the module config. Previously, Magento's default status was used (pending) and there was no way to use the value from the config.

Also, I did some refactoring regarding status to make the code more readable and added a double status check before cancelling an order (this prevents a cancellation to happen if multiple requests from Trabsbank are received when the order is already paid).

@gdespirito
Copy link
Contributor

Thanks for your contribution 🎉
We will check that everything works as expected and merge it, so it becomes available on the next release!

Controller/Transaction/CommitWebpayM22.php Outdated Show resolved Hide resolved
@@ -152,7 +151,7 @@ protected function getOrder()
$objectManager = \Magento\Framework\App\ObjectManager::getInstance();

return $objectManager->create('\Magento\Sales\Model\Order')->load($orderId);
} catch (Exception $e) {
} catch (\Exception $e) {

Choose a reason for hiding this comment

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

Aquí claramente se está arreglando un bug de namespace

Copy link
Contributor

Choose a reason for hiding this comment

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

👌

Copy link
Contributor

@gdespirito gdespirito left a comment

Choose a reason for hiding this comment

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

We tested and everything works as expected, but @barbazul discovered a bug in the code. Can you solve this to approve the PR? Thanks again!

@gdespirito gdespirito merged commit a010113 into TransbankDevelopers:master Apr 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants