From f53861aa9da27dbc5b53580398edf44c61d51d4d Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 9 Oct 2014 16:58:26 -0700 Subject: [PATCH] Smooth out Phortune purchase completion flow Summary: Ref T2787. Currently, we dump the user back into the application. Instead, give them a confirmation screen and then let them continue. Also fix a couple of unit tests I adjusted the underlying behavior of somewhat-recently in libphutil. Test Plan: {F215498} Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T2787 Differential Revision: https://secure.phabricator.com/D10672 --- .../fund/phortune/FundBackerCart.php | 4 +++ .../cart/PhortuneCartImplementation.php | 4 +++ .../PhortuneCartCheckoutController.php | 2 +- .../controller/PhortuneCartViewController.php | 27 +++++++++++++++- .../PhortunePayPalPaymentProvider.php | 2 +- .../provider/PhortuneWePayPaymentProvider.php | 2 +- .../phortune/storage/PhortuneCart.php | 4 +++ .../__tests__/PhabricatorUnitsTestCase.php | 32 +++++++++---------- src/view/phui/PHUIObjectBoxView.php | 2 +- 9 files changed, 58 insertions(+), 21 deletions(-) diff --git a/src/applications/fund/phortune/FundBackerCart.php b/src/applications/fund/phortune/FundBackerCart.php index 239b50baa7..3dc25d4bbf 100644 --- a/src/applications/fund/phortune/FundBackerCart.php +++ b/src/applications/fund/phortune/FundBackerCart.php @@ -81,4 +81,8 @@ final class FundBackerCart extends PhortuneCartImplementation { return '/'.$this->getInitiative()->getMonogram(); } + public function getDoneActionName(PhortuneCart $cart) { + return pht('Return to Initiative'); + } + } diff --git a/src/applications/phortune/cart/PhortuneCartImplementation.php b/src/applications/phortune/cart/PhortuneCartImplementation.php index cf2cf28e09..09c0187f49 100644 --- a/src/applications/phortune/cart/PhortuneCartImplementation.php +++ b/src/applications/phortune/cart/PhortuneCartImplementation.php @@ -16,6 +16,10 @@ abstract class PhortuneCartImplementation { abstract public function getCancelURI(PhortuneCart $cart); abstract public function getDoneURI(PhortuneCart $cart); + public function getDoneActionName(PhortuneCart $cart) { + return pht('Return to Application'); + } + public function assertCanCancelOrder(PhortuneCart $cart) { switch ($cart->getStatus()) { case PhortuneCart::STATUS_PURCHASED: diff --git a/src/applications/phortune/controller/PhortuneCartCheckoutController.php b/src/applications/phortune/controller/PhortuneCartCheckoutController.php index d5cede309b..8d02ff3759 100644 --- a/src/applications/phortune/controller/PhortuneCartCheckoutController.php +++ b/src/applications/phortune/controller/PhortuneCartCheckoutController.php @@ -102,7 +102,7 @@ final class PhortuneCartCheckoutController $cart->didApplyCharge($charge); - $done_uri = $cart->getDoneURI(); + $done_uri = $cart->getCheckoutURI(); return id(new AphrontRedirectResponse())->setURI($done_uri); } } diff --git a/src/applications/phortune/controller/PhortuneCartViewController.php b/src/applications/phortune/controller/PhortuneCartViewController.php index b909a996f5..017ec45fa2 100644 --- a/src/applications/phortune/controller/PhortuneCartViewController.php +++ b/src/applications/phortune/controller/PhortuneCartViewController.php @@ -35,6 +35,7 @@ final class PhortuneCartViewController PhabricatorPolicyCapability::CAN_EDIT); $errors = array(); + $error_view = null; $resume_uri = null; switch ($cart->getStatus()) { case PhortuneCart::STATUS_PURCHASING: @@ -71,6 +72,12 @@ final class PhortuneCartViewController phutil_tag('strong', array(), pht('Update Status'))); } break; + case PhortuneCart::STATUS_PURCHASED: + $error_view = id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) + ->appendChild(pht('This purchase has been completed.')); + + break; } $properties = $this->buildPropertyListView($cart); @@ -85,12 +92,30 @@ final class PhortuneCartViewController ->setUser($viewer) ->setHeader(pht('Order Detail')); + if ($cart->getStatus() == PhortuneCart::STATUS_PURCHASED) { + $done_uri = $cart->getDoneURI(); + if ($done_uri) { + $header->addActionLink( + id(new PHUIButtonView()) + ->setTag('a') + ->setHref($done_uri) + ->setIcon(id(new PHUIIconView()) + ->setIconFont('fa-check-square green')) + ->setText($cart->getDoneActionName())); + } + } + $cart_box = id(new PHUIObjectBoxView()) ->setHeader($header) - ->setFormErrors($errors) ->appendChild($properties) ->appendChild($cart_table); + if ($errors) { + $cart_box->setFormErrors($errors); + } else if ($error_view) { + $cart_box->setErrorView($error_view); + } + $charges = id(new PhortuneChargeQuery()) ->setViewer($viewer) ->withCartPHIDs(array($cart->getPHID())) diff --git a/src/applications/phortune/provider/PhortunePayPalPaymentProvider.php b/src/applications/phortune/provider/PhortunePayPalPaymentProvider.php index 258d8875f2..bc19602901 100644 --- a/src/applications/phortune/provider/PhortunePayPalPaymentProvider.php +++ b/src/applications/phortune/provider/PhortunePayPalPaymentProvider.php @@ -450,7 +450,7 @@ final class PhortunePayPalPaymentProvider extends PhortunePaymentProvider { if ($success) { $cart->didApplyCharge($charge); $response = id(new AphrontRedirectResponse())->setURI( - $cart->getDoneURI()); + $cart->getCheckoutURI()); } else if ($hold) { $cart->didHoldCharge($charge); diff --git a/src/applications/phortune/provider/PhortuneWePayPaymentProvider.php b/src/applications/phortune/provider/PhortuneWePayPaymentProvider.php index 0bc54fe193..b520443d8c 100644 --- a/src/applications/phortune/provider/PhortuneWePayPaymentProvider.php +++ b/src/applications/phortune/provider/PhortuneWePayPaymentProvider.php @@ -349,7 +349,7 @@ final class PhortuneWePayPaymentProvider extends PhortunePaymentProvider { $cart->didApplyCharge($charge); $response = id(new AphrontRedirectResponse())->setURI( - $cart->getDoneURI()); + $cart->getCheckoutURI()); break; default: // It's not clear if we can ever get here on the web workflow, diff --git a/src/applications/phortune/storage/PhortuneCart.php b/src/applications/phortune/storage/PhortuneCart.php index 4b0225c68c..76aae18b62 100644 --- a/src/applications/phortune/storage/PhortuneCart.php +++ b/src/applications/phortune/storage/PhortuneCart.php @@ -332,6 +332,10 @@ final class PhortuneCart extends PhortuneDAO return $this->getImplementation()->getDoneURI($this); } + public function getDoneActionName() { + return $this->getImplementation()->getDoneActionName($this); + } + public function getCancelURI() { return $this->getImplementation()->getCancelURI($this); } diff --git a/src/view/__tests__/PhabricatorUnitsTestCase.php b/src/view/__tests__/PhabricatorUnitsTestCase.php index 4671532a3b..bd0f535008 100644 --- a/src/view/__tests__/PhabricatorUnitsTestCase.php +++ b/src/view/__tests__/PhabricatorUnitsTestCase.php @@ -7,13 +7,13 @@ final class PhabricatorUnitsTestCase extends PhabricatorTestCase { public function testByteFormatting() { $tests = array( - 1 => '1 B', - 1000 => '1 KB', - 1000000 => '1 MB', - 10000000 => '10 MB', - 100000000 => '100 MB', - 1000000000 => '1 GB', - 999 => '999 B', + 1 => '1 B', + 1024 => '1 KB', + 1024 * 1024 => '1 MB', + 10 * 1024 * 1024 => '10 MB', + 100 * 1024 * 1024 => '100 MB', + 1024 * 1024 * 1024 => '1 GB', + 999 => '999 B', ); foreach ($tests as $input => $expect) { @@ -27,16 +27,16 @@ final class PhabricatorUnitsTestCase extends PhabricatorTestCase { public function testByteParsing() { $tests = array( '1' => 1, - '1k' => 1000, - '1K' => 1000, - '1kB' => 1000, - '1Kb' => 1000, - '1KB' => 1000, - '1MB' => 1000000, - '1GB' => 1000000000, - '1.5M' => 1500000, + '1k' => 1024, + '1K' => 1024, + '1kB' => 1024, + '1Kb' => 1024, + '1KB' => 1024, + '1MB' => 1024 * 1024, + '1GB' => 1024 * 1024 * 1024, + '1.5M' => (int)(1024 * 1024 * 1.5), '1 000' => 1000, - '1,234.56 KB' => 1234560, + '1,234.56 KB' => (int)(1024 * 1234.56), ); foreach ($tests as $input => $expect) { diff --git a/src/view/phui/PHUIObjectBoxView.php b/src/view/phui/PHUIObjectBoxView.php index fdc7a35869..da7372d203 100644 --- a/src/view/phui/PHUIObjectBoxView.php +++ b/src/view/phui/PHUIObjectBoxView.php @@ -85,7 +85,7 @@ final class PHUIObjectBoxView extends AphrontView { } public function setFormErrors(array $errors, $title = null) { - if (nonempty($errors)) { + if ($errors) { $this->formErrors = id(new AphrontErrorView()) ->setTitle($title) ->setErrors($errors);