From 2a2fb62229dc619af9fb3283cfe6eae8fed54c7f Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 9 Oct 2014 04:30:47 -0700 Subject: [PATCH] Add a HOLD state to Phortune and handle unusual states better Summary: Ref T2787. When Paypal comes back to us with funds on hold, dead-end the transaction but handle it properly. Generally, smooth out the user interaction on weird states. Implement refudnds/cancels for Paypal. Test Plan: {F215230} Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T2787 Differential Revision: https://secure.phabricator.com/D10667 --- src/__phutil_library_map__.php | 2 + .../PhabricatorPhortuneApplication.php | 1 + .../PhortuneCartCancelController.php | 11 +- .../PhortuneCartCheckoutController.php | 33 +---- .../PhortuneCartUpdateController.php | 31 +++++ .../controller/PhortuneCartViewController.php | 78 ++++++++++-- .../PhortuneProviderActionController.php | 3 +- .../PhortunePayPalPaymentProvider.php | 116 ++++++++++++++---- .../phortune/storage/PhortuneCart.php | 36 +++++- .../phortune/storage/PhortuneCharge.php | 2 + 10 files changed, 246 insertions(+), 67 deletions(-) create mode 100644 src/applications/phortune/controller/PhortuneCartUpdateController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 7fbb210ed3..a3c91f99fa 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2563,6 +2563,7 @@ phutil_register_library_map(array( 'PhortuneCartPHIDType' => 'applications/phortune/phid/PhortuneCartPHIDType.php', 'PhortuneCartQuery' => 'applications/phortune/query/PhortuneCartQuery.php', 'PhortuneCartSearchEngine' => 'applications/phortune/query/PhortuneCartSearchEngine.php', + 'PhortuneCartUpdateController' => 'applications/phortune/controller/PhortuneCartUpdateController.php', 'PhortuneCartViewController' => 'applications/phortune/controller/PhortuneCartViewController.php', 'PhortuneCharge' => 'applications/phortune/storage/PhortuneCharge.php', 'PhortuneChargePHIDType' => 'applications/phortune/phid/PhortuneChargePHIDType.php', @@ -5618,6 +5619,7 @@ phutil_register_library_map(array( 'PhortuneCartPHIDType' => 'PhabricatorPHIDType', 'PhortuneCartQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhortuneCartSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'PhortuneCartUpdateController' => 'PhortuneCartController', 'PhortuneCartViewController' => 'PhortuneCartController', 'PhortuneCharge' => array( 'PhortuneDAO', diff --git a/src/applications/phortune/application/PhabricatorPhortuneApplication.php b/src/applications/phortune/application/PhabricatorPhortuneApplication.php index 5e83945445..4d1ee4cceb 100644 --- a/src/applications/phortune/application/PhabricatorPhortuneApplication.php +++ b/src/applications/phortune/application/PhabricatorPhortuneApplication.php @@ -49,6 +49,7 @@ final class PhabricatorPhortuneApplication extends PhabricatorApplication { '' => 'PhortuneCartViewController', 'checkout/' => 'PhortuneCartCheckoutController', '(?Pcancel|refund)/' => 'PhortuneCartCancelController', + 'update/' => 'PhortuneCartUpdateController', ), 'account/' => array( '' => 'PhortuneAccountListController', diff --git a/src/applications/phortune/controller/PhortuneCartCancelController.php b/src/applications/phortune/controller/PhortuneCartCancelController.php index df7b1678e2..f244712e14 100644 --- a/src/applications/phortune/controller/PhortuneCartCancelController.php +++ b/src/applications/phortune/controller/PhortuneCartCancelController.php @@ -68,6 +68,7 @@ final class PhortuneCartCancelController ->withCartPHIDs(array($cart->getPHID())) ->withStatuses( array( + PhortuneCharge::STATUS_HOLD, PhortuneCharge::STATUS_CHARGED, )) ->execute(); @@ -156,6 +157,10 @@ final class PhortuneCartCancelController throw new Exception(pht('Unable to refund some charges!')); } + // TODO: If every HOLD and CHARGING transaction has been fully refunded + // and we're in a HOLD, PURCHASING or CHARGED cart state we probably + // need to kick the cart back to READY here? + return id(new AphrontRedirectResponse())->setURI($cancel_uri); } } @@ -182,6 +187,10 @@ final class PhortuneCartCancelController 'Really cancel this order? Any payment will be refunded.'); $button = pht('Cancel Order'); + // Don't give the user a "Cancel" button in response to a "Cancel?" + // prompt, as it's confusing. + $cancel_text = pht('Do Not Cancel Order'); + $form = null; } @@ -190,6 +199,6 @@ final class PhortuneCartCancelController ->appendChild($body) ->appendChild($form) ->addSubmitButton($button) - ->addCancelButton($cancel_uri); + ->addCancelButton($cancel_uri, $cancel_text); } } diff --git a/src/applications/phortune/controller/PhortuneCartCheckoutController.php b/src/applications/phortune/controller/PhortuneCartCheckoutController.php index 4ec47de752..d5cede309b 100644 --- a/src/applications/phortune/controller/PhortuneCartCheckoutController.php +++ b/src/applications/phortune/controller/PhortuneCartCheckoutController.php @@ -39,37 +39,12 @@ final class PhortuneCartCheckoutController // This is the expected, normal state for a cart that's ready for // checkout. break; - case PhortuneCart::STATUS_PURCHASING: - // We've started the purchase workflow for this cart, but were not able - // to complete it. If the workflow is on an external site, this could - // happen because the user abandoned the workflow. Just return them to - // the right place so they can resume where they left off. - $uri = $cart->getMetadataValue('provider.checkoutURI'); - if ($uri !== null) { - return id(new AphrontRedirectResponse()) - ->setIsExternal(true) - ->setURI($uri); - } - - return $this->newDialog() - ->setTitle(pht('Charge Failed')) - ->appendParagraph( - pht( - 'Failed to charge this cart.')) - ->addCancelButton($cancel_uri); - break; case PhortuneCart::STATUS_CHARGED: - // TODO: This is really bad (we took your money and at least partially - // failed to fulfill your order) and should have better steps forward. - - return $this->newDialog() - ->setTitle(pht('Purchase Failed')) - ->appendParagraph( - pht( - 'This cart was charged but the purchase could not be '. - 'completed.')) - ->addCancelButton($cancel_uri); + case PhortuneCart::STATUS_PURCHASING: + case PhortuneCart::STATUS_HOLD: case PhortuneCart::STATUS_PURCHASED: + // For these states, kick the user to the order page to give them + // information and options. return id(new AphrontRedirectResponse())->setURI($cart->getDetailURI()); default: throw new Exception( diff --git a/src/applications/phortune/controller/PhortuneCartUpdateController.php b/src/applications/phortune/controller/PhortuneCartUpdateController.php new file mode 100644 index 0000000000..8bbd6eaa95 --- /dev/null +++ b/src/applications/phortune/controller/PhortuneCartUpdateController.php @@ -0,0 +1,31 @@ +id = $data['id']; + } + + public function processRequest() { + $request = $this->getRequest(); + $viewer = $request->getUser(); + + $cart = id(new PhortuneCartQuery()) + ->setViewer($viewer) + ->withIDs(array($this->id)) + ->needPurchases(true) + ->executeOne(); + if (!$cart) { + return new Aphront404Response(); + } + + // TODO: This obviously doesn't do anything for now. + + return id(new AphrontRedirectResponse()) + ->setURI($cart->getDetailURI()); + } + +} diff --git a/src/applications/phortune/controller/PhortuneCartViewController.php b/src/applications/phortune/controller/PhortuneCartViewController.php index 7e5ad6c979..8f2b54c248 100644 --- a/src/applications/phortune/controller/PhortuneCartViewController.php +++ b/src/applications/phortune/controller/PhortuneCartViewController.php @@ -29,8 +29,56 @@ final class PhortuneCartViewController $cart_table = $this->buildCartContentTable($cart); + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $cart, + PhabricatorPolicyCapability::CAN_EDIT); + + $errors = array(); + $resume_uri = null; + switch ($cart->getStatus()) { + case PhortuneCart::STATUS_PURCHASING: + if ($can_edit) { + $resume_uri = $cart->getMetadataValue('provider.checkoutURI'); + if ($resume_uri) { + $errors[] = pht( + 'The checkout process has been started, but not yet completed. '. + 'You can continue checking out by clicking %s, or cancel the '. + 'order, or contact the merchant for assistance.', + phutil_tag('strong', array(), pht('Continue Checkout'))); + } else { + $errors[] = pht( + 'The checkout process has been started, but an error occurred. '. + 'You can cancel the order or contact the merchant for '. + 'assistance.'); + } + } + break; + case PhortuneCart::STATUS_CHARGED: + if ($can_edit) { + $errors[] = pht( + 'You have been charged, but processing could not be completed. '. + 'You can cancel your order, or contact the merchant for '. + 'assistance.'); + } + break; + case PhortuneCart::STATUS_HOLD: + if ($can_edit) { + $errors[] = pht( + 'Payment for this order is on hold. You can click %s to check '. + 'for updates, cancel the order, or contact the merchant for '. + 'assistance.', + phutil_tag('strong', array(), pht('Update Status'))); + } + break; + } + $properties = $this->buildPropertyListView($cart); - $actions = $this->buildActionListView($cart, $can_admin); + $actions = $this->buildActionListView( + $cart, + $can_edit, + $can_admin, + $resume_uri); $properties->setActionList($actions); $header = id(new PHUIHeaderView()) @@ -40,6 +88,7 @@ final class PhortuneCartViewController $cart_box = id(new PHUIObjectBoxView()) ->setHeader($header) + ->setFormErrors($errors) ->appendChild($properties) ->appendChild($cart_table); @@ -106,15 +155,15 @@ final class PhortuneCartViewController return $view; } - private function buildActionListView(PhortuneCart $cart, $can_admin) { + private function buildActionListView( + PhortuneCart $cart, + $can_edit, + $can_admin, + $resume_uri) { + $viewer = $this->getRequest()->getUser(); $id = $cart->getID(); - $can_edit = PhabricatorPolicyFilter::hasCapability( - $viewer, - $cart, - PhabricatorPolicyCapability::CAN_EDIT); - $view = id(new PhabricatorActionListView()) ->setUser($viewer) ->setObject($cart); @@ -123,6 +172,7 @@ final class PhortuneCartViewController $cancel_uri = $this->getApplicationURI("cart/{$id}/cancel/"); $refund_uri = $this->getApplicationURI("cart/{$id}/refund/"); + $update_uri = $this->getApplicationURI("cart/{$id}/update/"); $view->addAction( id(new PhabricatorActionView()) @@ -141,6 +191,20 @@ final class PhortuneCartViewController ->setHref($refund_uri)); } + $view->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Update Status')) + ->setIcon('fa-refresh') + ->setHref($update_uri)); + + if ($can_edit && $resume_uri) { + $view->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Continue Checkout')) + ->setIcon('fa-shopping-cart') + ->setHref($resume_uri)); + } + return $view; } diff --git a/src/applications/phortune/controller/PhortuneProviderActionController.php b/src/applications/phortune/controller/PhortuneProviderActionController.php index 21f28709de..dcb171911f 100644 --- a/src/applications/phortune/controller/PhortuneProviderActionController.php +++ b/src/applications/phortune/controller/PhortuneProviderActionController.php @@ -1,6 +1,7 @@ getProviderConfig()->getMetadataValue(self::PAYPAL_MODE); return ($mode === 'live'); @@ -170,8 +165,31 @@ final class PhortunePayPalPaymentProvider extends PhortunePaymentProvider { protected function executeRefund( PhortuneCharge $charge, PhortuneCharge $refund) { - // TODO: Implement. - throw new PhortuneNotImplementedException($this); + + $transaction_id = $charge->getMetadataValue('paypal.transactionID'); + if (!$transaction_id) { + throw new Exception(pht('Charge has no transaction ID!')); + } + + $refund_amount = $refund->getAmountAsCurrency()->negate(); + $refund_currency = $refund_amount->getCurrency(); + $refund_value = $refund_amount->formatBareValue(); + + $params = array( + 'TRANSACTIONID' => $transaction_id, + 'REFUNDTYPE' => 'Partial', + 'AMT' => $refund_value, + 'CURRENCYCODE' => $refund_currency, + ); + + $result = $this + ->newPaypalAPICall() + ->setRawPayPalQuery('RefundTransaction', $params) + ->resolve(); + + $charge->setMetadataValue( + 'paypal.refundID', + $result['REFUNDTRANSACTIONID']); } private function getPaypalAPIUsername() { @@ -281,7 +299,7 @@ final class PhortunePayPalPaymentProvider extends PhortunePaymentProvider { 'token' => $result['TOKEN'], )); - $cart->setMetadataValue('provider.checkoutURI', $uri); + $cart->setMetadataValue('provider.checkoutURI', (string)$uri); $cart->save(); $charge->setMetadataValue('paypal.token', $result['TOKEN']); @@ -291,6 +309,11 @@ final class PhortunePayPalPaymentProvider extends PhortunePaymentProvider { ->setIsExternal(true) ->setURI($uri); case 'charge': + if ($cart->getStatus() !== PhortuneCart::STATUS_PURCHASING) { + return id(new AphrontRedirectResponse()) + ->setURI($cart->getCheckoutURI()); + } + $token = $request->getStr('token'); $params = array( @@ -302,19 +325,19 @@ final class PhortunePayPalPaymentProvider extends PhortunePaymentProvider { ->setRawPayPalQuery('GetExpressCheckoutDetails', $params) ->resolve(); - var_dump($result); - if ($result['CUSTOM'] !== $charge->getPHID()) { throw new Exception( pht('Paypal checkout does not match Phortune charge!')); } if ($result['CHECKOUTSTATUS'] !== 'PaymentActionNotInitiated') { - throw new Exception( - pht( - 'Expected status "%s", got "%s".', - 'PaymentActionNotInitiated', - $result['CHECKOUTSTATUS'])); + return $controller->newDialog() + ->setTitle(pht('Payment Already Processed')) + ->appendParagraph( + pht( + 'The payment response for this charge attempt has already '. + 'been processed.')) + ->addCancelButton($cart->getCheckoutURI(), pht('Continue')); } $price = $cart->getTotalPriceAsCurrency(); @@ -333,22 +356,53 @@ final class PhortunePayPalPaymentProvider extends PhortunePaymentProvider { ->setRawPayPalQuery('DoExpressCheckoutPayment', $params) ->resolve(); - // TODO: Paypal can send requests back in "PaymentReview" status, - // and does this for test transactions. We're supposed to hold - // the transaction and poll the API every 6 hours. This is unreasonably - // difficult for now and we can't reasonably just fail these charges. + $transaction_id = $result['PAYMENTINFO_0_TRANSACTIONID']; - var_dump($result); - die(); + $success = false; + $hold = false; + switch ($result['PAYMENTINFO_0_PAYMENTSTATUS']) { + case 'Processed': + case 'Completed': + case 'Completed-Funds-Held': + $success = true; + break; + case 'In-Progress': + case 'Pending': + // TODO: We can capture more information about this stuff. + $hold = true; + break; + case 'Denied': + case 'Expired': + case 'Failed': + case 'Partially-Refunded': + case 'Canceled-Reversal': + case 'None': + case 'Refunded': + case 'Reversed': + case 'Voided': + default: + // These are all failure states. + break; + } - $success = false; // TODO: <---- - - // TODO: Clean this up once that mess up there ^^^^^ gets cleaned up. $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + + $charge->setMetadataValue('paypal.transactionID', $transaction_id); + $charge->save(); + if ($success) { $cart->didApplyCharge($charge); $response = id(new AphrontRedirectResponse())->setURI( - $cart->getDoneURI()); + $cart->getDoneURI()); + } else if ($hold) { + $cart->didHoldCharge($charge); + + $response = $controller + ->newDialog() + ->setTitle(pht('Charge On Hold')) + ->appendParagraph( + pht('Your charge is on hold, for reasons?')) + ->addCancelButton($cart->getCheckoutURI(), pht('Continue')); } else { $cart->didFailCharge($charge); @@ -361,8 +415,16 @@ final class PhortunePayPalPaymentProvider extends PhortunePaymentProvider { return $response; case 'cancel': - var_dump($_REQUEST); - break; + if ($cart->getStatus() !== PhortuneCart::STATUS_PURCHASING) { + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + // TODO: Since the user cancelled this, we could conceivably just + // throw it away or make it more clear that it's a user cancel. + $cart->didFailCharge($charge); + unset($unguarded); + } + + return id(new AphrontRedirectResponse()) + ->setURI($cart->getCheckoutURI()); } throw new Exception( diff --git a/src/applications/phortune/storage/PhortuneCart.php b/src/applications/phortune/storage/PhortuneCart.php index 23e8807c92..0f5159648d 100644 --- a/src/applications/phortune/storage/PhortuneCart.php +++ b/src/applications/phortune/storage/PhortuneCart.php @@ -7,6 +7,7 @@ final class PhortuneCart extends PhortuneDAO const STATUS_READY = 'cart:ready'; const STATUS_PURCHASING = 'cart:purchasing'; const STATUS_CHARGED = 'cart:charged'; + const STATUS_HOLD = 'cart:hold'; const STATUS_PURCHASED = 'cart:purchased'; protected $accountPHID; @@ -57,6 +58,7 @@ final class PhortuneCart extends PhortuneDAO self::STATUS_READY => pht('Ready'), self::STATUS_PURCHASING => pht('Purchasing'), self::STATUS_CHARGED => pht('Charged'), + self::STATUS_HOLD => pht('Hold'), self::STATUS_PURCHASED => pht('Purchased'), ); } @@ -113,6 +115,31 @@ final class PhortuneCart extends PhortuneDAO return $charge; } + public function didHoldCharge(PhortuneCharge $charge) { + $charge->setStatus(PhortuneCharge::STATUS_HOLD); + + $this->openTransaction(); + $this->beginReadLocking(); + + $copy = clone $this; + $copy->reload(); + + if ($copy->getStatus() !== self::STATUS_PURCHASING) { + throw new Exception( + pht( + 'Cart has wrong status ("%s") to call didHoldCharge(), '. + 'expected "%s".', + $copy->getStatus(), + self::STATUS_PURCHASING)); + } + + $charge->save(); + $this->setStatus(self::STATUS_HOLD)->save(); + + $this->endReadLocking(); + $this->saveTransaction(); + } + public function didApplyCharge(PhortuneCharge $charge) { $charge->setStatus(PhortuneCharge::STATUS_CHARGED); @@ -198,7 +225,8 @@ final class PhortuneCart extends PhortuneDAO pht('Trying to refund a refund!')); } - if ($charge->getStatus() !== PhortuneCharge::STATUS_CHARGED) { + if (($charge->getStatus() !== PhortuneCharge::STATUS_CHARGED) && + ($charge->getStatus() !== PhortuneCharge::STATUS_HOLD)) { throw new Exception( pht('Trying to refund an uncharged charge!')); } @@ -462,7 +490,11 @@ final class PhortuneCart extends PhortuneDAO } public function getPolicy($capability) { - return $this->getAccount()->getPolicy($capability); + // NOTE: Both view and edit use the account's edit policy. We punch a hole + // through this for merchants, below. + return $this + ->getAccount() + ->getPolicy(PhabricatorPolicyCapability::CAN_EDIT); } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { diff --git a/src/applications/phortune/storage/PhortuneCharge.php b/src/applications/phortune/storage/PhortuneCharge.php index 7254e2f222..1b3390fef8 100644 --- a/src/applications/phortune/storage/PhortuneCharge.php +++ b/src/applications/phortune/storage/PhortuneCharge.php @@ -11,6 +11,7 @@ final class PhortuneCharge extends PhortuneDAO const STATUS_CHARGING = 'charge:charging'; const STATUS_CHARGED = 'charge:charged'; + const STATUS_HOLD = 'charge:hold'; const STATUS_FAILED = 'charge:failed'; protected $accountPHID; @@ -74,6 +75,7 @@ final class PhortuneCharge extends PhortuneDAO return array( self::STATUS_CHARGING => pht('Charging'), self::STATUS_CHARGED => pht('Charged'), + self::STATUS_HOLD => pht('Hold'), self::STATUS_FAILED => pht('Failed'), ); }