From 0b62981c898114573faf79917ac94161d2b17331 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 7 Feb 2018 07:13:57 -0800
Subject: [PATCH] (stable) Fix a Phortune billing issue where subscription
 autopay could charge disabled cards

Summary:
See support email. There's nothing tricky here, we were just missing a check. The different parts of this got built at different times so I think this was simply overlooked.

Also add a redundant check just to future-proof this and be on the safe side.

Test Plan: Used `bin/phortune invoice` to charge a pact subscription. After deleting the card, the charge failed with an appropriate error.

Reviewers: amckinley

Differential Revision: https://secure.phabricator.com/D19020
---
 .../query/PhortunePaymentMethodQuery.php      | 25 ++++++-------------
 .../phortune/storage/PhortuneCart.php         |  7 ++++++
 .../storage/PhortunePaymentMethod.php         |  4 +++
 .../worker/PhortuneSubscriptionWorker.php     |  4 +++
 4 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/applications/phortune/query/PhortunePaymentMethodQuery.php b/src/applications/phortune/query/PhortunePaymentMethodQuery.php
index 07d49a5909..42d54805e6 100644
--- a/src/applications/phortune/query/PhortunePaymentMethodQuery.php
+++ b/src/applications/phortune/query/PhortunePaymentMethodQuery.php
@@ -34,19 +34,12 @@ final class PhortunePaymentMethodQuery
     return $this;
   }
 
+  public function newResultObject() {
+    return new PhortunePaymentMethod();
+  }
+
   protected function loadPage() {
-    $table = new PhortunePaymentMethod();
-    $conn = $table->establishConnection('r');
-
-    $rows = queryfx_all(
-      $conn,
-      'SELECT * FROM %T %Q %Q %Q',
-      $table->getTableName(),
-      $this->buildWhereClause($conn),
-      $this->buildOrderClause($conn),
-      $this->buildLimitClause($conn));
-
-    return $table->loadAllFromArray($rows);
+    return $this->loadStandardPage($this->newResultObject());
   }
 
   protected function willFilterPage(array $methods) {
@@ -106,8 +99,8 @@ final class PhortunePaymentMethodQuery
     return $methods;
   }
 
-  protected function buildWhereClause(AphrontDatabaseConnection $conn) {
-    $where = array();
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
+    $where = parent::buildWhereClauseParts($conn);
 
     if ($this->ids !== null) {
       $where[] = qsprintf(
@@ -144,9 +137,7 @@ final class PhortunePaymentMethodQuery
         $this->statuses);
     }
 
-    $where[] = $this->buildPagingClause($conn);
-
-    return $this->formatWhereClause($where);
+    return $where;
   }
 
   public function getQueryApplicationClass() {
diff --git a/src/applications/phortune/storage/PhortuneCart.php b/src/applications/phortune/storage/PhortuneCart.php
index af2b386dfe..07554ccb87 100644
--- a/src/applications/phortune/storage/PhortuneCart.php
+++ b/src/applications/phortune/storage/PhortuneCart.php
@@ -118,6 +118,13 @@ final class PhortuneCart extends PhortuneDAO
       ->setAmountAsCurrency($this->getTotalPriceAsCurrency());
 
     if ($method) {
+      if (!$method->isActive()) {
+        throw new Exception(
+          pht(
+            'Attempting to apply a charge using an inactive '.
+            'payment method ("%s")!',
+            $method->getPHID()));
+      }
       $charge->setPaymentMethodPHID($method->getPHID());
     }
 
diff --git a/src/applications/phortune/storage/PhortunePaymentMethod.php b/src/applications/phortune/storage/PhortunePaymentMethod.php
index 8044168ba4..1712d3f973 100644
--- a/src/applications/phortune/storage/PhortunePaymentMethod.php
+++ b/src/applications/phortune/storage/PhortunePaymentMethod.php
@@ -128,6 +128,10 @@ final class PhortunePaymentMethod extends PhortuneDAO
     return $month.'/'.$year;
   }
 
+  public function isActive() {
+    return ($this->getStatus() === self::STATUS_ACTIVE);
+  }
+
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
diff --git a/src/applications/phortune/worker/PhortuneSubscriptionWorker.php b/src/applications/phortune/worker/PhortuneSubscriptionWorker.php
index 097fb54dd9..d05aacbb7c 100644
--- a/src/applications/phortune/worker/PhortuneSubscriptionWorker.php
+++ b/src/applications/phortune/worker/PhortuneSubscriptionWorker.php
@@ -141,6 +141,10 @@ final class PhortuneSubscriptionWorker extends PhabricatorWorker {
     $method = id(new PhortunePaymentMethodQuery())
       ->setViewer($viewer)
       ->withPHIDs(array($subscription->getDefaultPaymentMethodPHID()))
+      ->withStatuses(
+        array(
+          PhortunePaymentMethod::STATUS_ACTIVE,
+        ))
       ->executeOne();
     if (!$method) {
       $issues[] = pht(