-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
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
[Payment] Refactor order payment state machine #5605
[Payment] Refactor order payment state machine #5605
Conversation
GSadee
commented
Jul 26, 2016
Q | A |
---|---|
Bug fix? | no |
New feature? | no |
BC breaks? | yes |
Related tickets | |
License | MIT |
ca0e0ad
to
612e6d5
Compare
$paymentState = PaymentInterface::STATE_PROCESSING; | ||
} | ||
if (OrderInterface::STATE_CANCELLED === $order->getState()) { | ||
$order->setPaymentState(OrderPaymentStates::STATE_CANCELLED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a transition of state machine. We should never ever set states through setters.
612e6d5
to
2246116
Compare
/** | ||
* @param FactoryInterface $stateMachineFactory | ||
*/ | ||
public function __construct( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be in one line
2246116
to
bdf1b88
Compare
@ui | ||
Scenario: Checking order payment state of a cancelled order | ||
Given I view the summary of the order "#00000022" | ||
And I cancel this order |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the order was canceled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Given the customer canceled this order
step should be renamed to the above. The implementation is fine :)
bdf1b88
to
b94b7cc
Compare
b94b7cc
to
89e7b0e
Compare
|
||
if (OrderPaymentStates::STATE_PAID === $order->getPaymentState()) { | ||
return; | ||
} | ||
|
||
if ($order->hasPayments()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we need this big if? Couldn't we simply add the following code to check the amount?
if (0 === $completedPaymentTotal) {
return;
}
Thanks Grzesiu! Please apply all comments in a separate PR. |