BP- 2132 BP-2813 clean refactor push & redirect #770
Conversation
… create the structure of push processors.
…/buckaroo-it/Magento2 into BP-2132-refactor-push # Conflicts: # Model/Push.php
…/buckaroo-it/Magento2 into BP-2132-refactor-push
…/buckaroo-it/Magento2 into BP-2132-refactor-push
…/buckaroo-it/Magento2 into BP-2132-clean-refactor-push-redirect
Model/Push/DefaultProcessor.php
Outdated
| } elseif (in_array($statusKey, $this->buckarooStatusCode->getFailedStatuses())) { | ||
| return $this->processFailedPush($newStatus, $statusMessage); | ||
| } elseif (in_array($statusKey, $this->buckarooStatusCode->getPendingStatuses())) { | ||
| return $this->processPendingPaymentPush(); | ||
| } else { |
There was a problem hiding this comment.
i don't think we need the else if here since we do a return in each branch, it will be nicer to read
| if ($this->receivePushCheckDuplicates()) { | ||
| throw new BuckarooException(__('Skipped handling this push, duplicate')); | ||
| } | ||
| // TODO: Implement processPush() method. |
There was a problem hiding this comment.
I deleted the entire class because was from an old version of push refactor.
Model/Push/PushProcessorsFactory.php
Outdated
| $pushType = $pushTransactionType->getPushType(); | ||
| if ($pushType == PushTransactionType::BUCK_PUSH_TYPE_INVOICE) { | ||
| $pushProcessorClass = $this->pushProcessors['credit_managment']; | ||
| } elseif ($pushType == PushTransactionType::BUCK_PUSH_TYPE_INVOICE_INCOMPLETE) { |
There was a problem hiding this comment.
in my opinion we should do simple ifs and return, will look cleaner
There was a problem hiding this comment.
I proceed like this because a pushProcessorClass can change the value from $paymentMethod to group_transaction or refund. Also I proceed like this to avoid the next error: "This method has 5 returns, which is more than the 3 allowed."
Model/Push/PushTransactionType.php
Outdated
| if ($this->pushRequest->getStatusCode() !== null) { | ||
| $statusCode = $this->pushRequest->getStatusCode(); | ||
| } | ||
| break; |
There was a problem hiding this comment.
will make it cleaner if we move the ifs into different function
There was a problem hiding this comment.
I change it with a shorter version $statusCode = $this->pushRequest->getStatusCode() ?: $statusCode;
Model/Push/RefundProcessor.php
Outdated
| */ | ||
| private function skipPendingRefundPush(PushRequestInterface $pushRequest): bool | ||
| { | ||
| if ($pushRequest->hasAdditionalInformation('initiated_by_magento', 1) |
There was a problem hiding this comment.
will be nice to reduce inline ifs as much as possible
| { | ||
| if (!empty($this->pushRequest->getServiceKlarnakpReservationnumber())) { | ||
| $this->order->setBuckarooReservationNumber($this->pushRequest->getServiceKlarnakpReservationnumber()); | ||
| $this->order->save(); |
There was a problem hiding this comment.
i think we could do better when committing data to the database, like to do this after all the processing was done, was thinking of a class for each entity that will handle the db committing OrderHandler, OrderHistoryHandler, PaymentHandler , set a flag hasChanged and commit the changes if it changed
…/buckaroo-it/Magento2 into BP-2132-clean-refactor-push-redirect # Conflicts: # Controller/Redirect/Process.php
No description provided.