From 8958d68ec648c50c5d96a143952bc0a710cc5d40 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 23 Nov 2016 13:31:17 -0800
Subject: [PATCH] When a Phortune subscription has a removed payment method, be
 more explicit about it

Summary:
Currently, when a payment method is invalid we still render the full name and let you save the form without making changes. This can be confusing.

Instead:

  - Render "<Deleted Payment Method>", literally.
  - Render an error immediately.
  - Prevent the form from being saved without changing the method.

Test Plan: {F1955487}

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16935
---
 .../PhortuneSubscriptionEditController.php    | 24 ++++++++++++-------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/applications/phortune/controller/PhortuneSubscriptionEditController.php b/src/applications/phortune/controller/PhortuneSubscriptionEditController.php
index 8ba0592be4..7af8aa3456 100644
--- a/src/applications/phortune/controller/PhortuneSubscriptionEditController.php
+++ b/src/applications/phortune/controller/PhortuneSubscriptionEditController.php
@@ -50,6 +50,11 @@ final class PhortuneSubscriptionEditController extends PhortuneController {
 
     $current_phid = $subscription->getDefaultPaymentMethodPHID();
 
+    $e_method = null;
+    if ($current_phid && empty($valid_methods[$current_phid])) {
+      $e_method = pht('Needs Update');
+    }
+
     $errors = array();
     if ($request->isFormPost()) {
 
@@ -57,12 +62,14 @@ final class PhortuneSubscriptionEditController extends PhortuneController {
       if (!$default_method_phid) {
         $default_method_phid = null;
         $e_method = null;
-      } else if ($default_method_phid == $current_phid) {
-        // If you have an invalid setting already, it's OK to retain it.
-        $e_method = null;
-      } else {
-        if (empty($valid_methods[$default_method_phid])) {
-          $e_method = pht('Invalid');
+      } else if (empty($valid_methods[$default_method_phid])) {
+        $e_method = pht('Invalid');
+        if ($default_method_phid == $current_phid) {
+          $errors[] = pht(
+            'This subscription is configured to autopay with a payment method '.
+            'that has been deleted. Choose a valid payment method or disable '.
+            'autopay.');
+        } else {
           $errors[] = pht('You must select a valid default payment method.');
         }
       }
@@ -86,11 +93,9 @@ final class PhortuneSubscriptionEditController extends PhortuneController {
 
     // Don't require the user to make a valid selection if the current method
     // has become invalid.
-    // TODO: This should probably have a note about why this is bogus.
     if ($current_phid && empty($valid_methods[$current_phid])) {
-      $handles = $this->loadViewerHandles(array($current_phid));
       $current_options = array(
-        $current_phid => $handles[$current_phid]->getName(),
+        $current_phid => pht('<Deleted Payment Method>'),
       );
     } else {
       $current_options = array();
@@ -129,6 +134,7 @@ final class PhortuneSubscriptionEditController extends PhortuneController {
           ->setName('defaultPaymentMethodPHID')
           ->setLabel(pht('Autopay With'))
           ->setValue($current_phid)
+          ->setError($e_method)
           ->setOptions($options))
       ->appendChild(
         id(new AphrontFormMarkupControl())