From 945af4f140860532f804def23312adc7fe8867a6 Mon Sep 17 00:00:00 2001
From: Faizaan Gagan <faizaan@discourse.org>
Date: Wed, 6 Jul 2022 07:23:27 +0530
Subject: [PATCH] FIX: Allow user to update card details for recurring
 subscriptions (#123)

* add new route for card update

* create backend route

* update label

* basic functionality working

* ran rubocop

* added rspec tests for functionality

* make payment_method param compulsory

* fixed js linting

* improve client side error handling

* improve server side error handling

* improved update card page UI

* improve button UI for user subscriptions page

* give feedback to user about save status

* remove heading from last column

* fix padding on edit/delete buttons for update table

Co-authored-by: Blake Erickson <o.blakeerickson@gmail.com>
---
 .../user/subscriptions_controller.rb          | 25 +++++++++
 .../user-billing-subscriptions-card.js        | 56 +++++++++++++++++++
 .../discourse-subscriptions-user-route-map.js |  4 +-
 .../discourse/routes/user-billing-index.js    |  2 +-
 .../routes/user-billing-subscriptions-card.js |  7 +++
 .../user-billing-subscriptions-index.js       | 42 ++++++++++++++
 .../routes/user-billing-subscriptions.js      | 39 +------------
 .../user/billing/subscriptions/card.hbs       | 17 ++++++
 .../index.hbs}                                |  9 ++-
 assets/stylesheets/common/main.scss           |  8 +--
 config/locales/client.en.yml                  |  3 +
 config/locales/server.en.yml                  |  1 +
 config/routes.rb                              |  2 +-
 plugin.rb                                     |  1 +
 .../user/subscriptions_controller_spec.rb     | 14 +++++
 15 files changed, 183 insertions(+), 47 deletions(-)
 create mode 100644 assets/javascripts/discourse/controllers/user-billing-subscriptions-card.js
 create mode 100644 assets/javascripts/discourse/routes/user-billing-subscriptions-card.js
 create mode 100644 assets/javascripts/discourse/routes/user-billing-subscriptions-index.js
 create mode 100644 assets/javascripts/discourse/templates/user/billing/subscriptions/card.hbs
 rename assets/javascripts/discourse/templates/user/billing/{subscriptions.hbs => subscriptions/index.hbs} (86%)

diff --git a/app/controllers/discourse_subscriptions/user/subscriptions_controller.rb b/app/controllers/discourse_subscriptions/user/subscriptions_controller.rb
index 6eb661f..0e86e6a 100644
--- a/app/controllers/discourse_subscriptions/user/subscriptions_controller.rb
+++ b/app/controllers/discourse_subscriptions/user/subscriptions_controller.rb
@@ -63,6 +63,31 @@ module DiscourseSubscriptions
           render_json_error e.message
         end
       end
+
+      def update
+        params.require(:payment_method)
+
+        subscription = Subscription.where(external_id: params[:id]).first
+        begin
+          attach_method_to_customer(subscription.customer_id, params[:payment_method])
+          subscription = ::Stripe::Subscription.update(params[:id], { default_payment_method: params[:payment_method] })
+          render json: success_json
+        rescue ::Stripe::InvalidRequestError
+          render_json_error I18n.t("discourse_subscriptions.card.invalid")
+        end
+      end
+
+      private
+
+      def attach_method_to_customer(customer_id, method)
+        customer = Customer.find(customer_id)
+        ::Stripe::PaymentMethod.attach(
+          method,
+          {
+            customer: customer.customer_id
+          }
+        )
+      end
     end
   end
 end
diff --git a/assets/javascripts/discourse/controllers/user-billing-subscriptions-card.js b/assets/javascripts/discourse/controllers/user-billing-subscriptions-card.js
new file mode 100644
index 0000000..3f0b3eb
--- /dev/null
+++ b/assets/javascripts/discourse/controllers/user-billing-subscriptions-card.js
@@ -0,0 +1,56 @@
+import Controller from "@ember/controller";
+import { action } from "@ember/object";
+import { ajax } from "discourse/lib/ajax";
+import { popupAjaxError } from "discourse/lib/ajax-error";
+import I18n from "I18n";
+import bootbox from "bootbox";
+
+export default Controller.extend({
+  loading: false,
+  saved: false,
+  init() {
+    this._super(...arguments);
+    this.set(
+      "stripe",
+      Stripe(this.siteSettings.discourse_subscriptions_public_key)
+    );
+    const elements = this.get("stripe").elements();
+    this.set("cardElement", elements.create("card", { hidePostalCode: true }));
+  },
+
+  @action
+  async updatePaymentMethod() {
+    this.set("loading", true);
+    this.set("saved", false);
+
+    const paymentMethodObject = await this.stripe.createPaymentMethod({
+      type: "card",
+      card: this.cardElement,
+    });
+
+    if (paymentMethodObject.error) {
+      bootbox.alert(
+        paymentMethodObject.error?.message || I18n.t("generic_error")
+      );
+      this.set("loading", false);
+      return;
+    }
+
+    const paymentMethod = paymentMethodObject.paymentMethod.id;
+
+    try {
+      await ajax(`/s/user/subscriptions/${this.model}`, {
+        method: "PUT",
+        data: {
+          payment_method: paymentMethod,
+        },
+      });
+      this.set("saved", true);
+    } catch (err) {
+      popupAjaxError(err);
+    } finally {
+      this.set("loading", false);
+      this.cardElement?.clear();
+    }
+  },
+});
diff --git a/assets/javascripts/discourse/discourse-subscriptions-user-route-map.js b/assets/javascripts/discourse/discourse-subscriptions-user-route-map.js
index d80c8ce..5f382db 100644
--- a/assets/javascripts/discourse/discourse-subscriptions-user-route-map.js
+++ b/assets/javascripts/discourse/discourse-subscriptions-user-route-map.js
@@ -4,7 +4,9 @@ export default {
   map() {
     this.route("billing", function () {
       this.route("payments");
-      this.route("subscriptions");
+      this.route("subscriptions", function () {
+        this.route("card", { path: "/card/:stripe-subscription-id" });
+      });
     });
   },
 };
diff --git a/assets/javascripts/discourse/routes/user-billing-index.js b/assets/javascripts/discourse/routes/user-billing-index.js
index c728e7e..66bca47 100644
--- a/assets/javascripts/discourse/routes/user-billing-index.js
+++ b/assets/javascripts/discourse/routes/user-billing-index.js
@@ -4,6 +4,6 @@ export default Route.extend({
   templateName: "user/billing/index",
 
   redirect() {
-    this.transitionTo("user.billing.subscriptions");
+    this.transitionTo("user.billing.subscriptions.index");
   },
 });
diff --git a/assets/javascripts/discourse/routes/user-billing-subscriptions-card.js b/assets/javascripts/discourse/routes/user-billing-subscriptions-card.js
new file mode 100644
index 0000000..ea5cc73
--- /dev/null
+++ b/assets/javascripts/discourse/routes/user-billing-subscriptions-card.js
@@ -0,0 +1,7 @@
+import Route from "@ember/routing/route";
+
+export default Route.extend({
+  model(params) {
+    return params["stripe-subscription-id"];
+  },
+});
diff --git a/assets/javascripts/discourse/routes/user-billing-subscriptions-index.js b/assets/javascripts/discourse/routes/user-billing-subscriptions-index.js
new file mode 100644
index 0000000..84a2899
--- /dev/null
+++ b/assets/javascripts/discourse/routes/user-billing-subscriptions-index.js
@@ -0,0 +1,42 @@
+import Route from "@ember/routing/route";
+import UserSubscription from "discourse/plugins/discourse-subscriptions/discourse/models/user-subscription";
+import I18n from "I18n";
+import { action } from "@ember/object";
+import bootbox from "bootbox";
+
+export default Route.extend({
+  model() {
+    return UserSubscription.findAll();
+  },
+
+  @action
+  updateCard(subscriptionId) {
+    this.transitionTo("user.billing.subscriptions.card", subscriptionId);
+  },
+  @action
+  cancelSubscription(subscription) {
+    bootbox.confirm(
+      I18n.t(
+        "discourse_subscriptions.user.subscriptions.operations.destroy.confirm"
+      ),
+      I18n.t("no_value"),
+      I18n.t("yes_value"),
+      (confirmed) => {
+        if (confirmed) {
+          subscription.set("loading", true);
+
+          subscription
+            .destroy()
+            .then((result) => subscription.set("status", result.status))
+            .catch((data) =>
+              bootbox.alert(data.jqXHR.responseJSON.errors.join("\n"))
+            )
+            .finally(() => {
+              subscription.set("loading", false);
+              this.refresh();
+            });
+        }
+      }
+    );
+  },
+});
diff --git a/assets/javascripts/discourse/routes/user-billing-subscriptions.js b/assets/javascripts/discourse/routes/user-billing-subscriptions.js
index 61e970d..0051f5c 100644
--- a/assets/javascripts/discourse/routes/user-billing-subscriptions.js
+++ b/assets/javascripts/discourse/routes/user-billing-subscriptions.js
@@ -1,40 +1,3 @@
 import Route from "@ember/routing/route";
-import UserSubscription from "discourse/plugins/discourse-subscriptions/discourse/models/user-subscription";
-import I18n from "I18n";
-import { action } from "@ember/object";
-import bootbox from "bootbox";
 
-export default Route.extend({
-  templateName: "user/billing/subscriptions",
-
-  model() {
-    return UserSubscription.findAll();
-  },
-
-  @action
-  cancelSubscription(subscription) {
-    bootbox.confirm(
-      I18n.t(
-        "discourse_subscriptions.user.subscriptions.operations.destroy.confirm"
-      ),
-      I18n.t("no_value"),
-      I18n.t("yes_value"),
-      (confirmed) => {
-        if (confirmed) {
-          subscription.set("loading", true);
-
-          subscription
-            .destroy()
-            .then((result) => subscription.set("status", result.status))
-            .catch((data) =>
-              bootbox.alert(data.jqXHR.responseJSON.errors.join("\n"))
-            )
-            .finally(() => {
-              subscription.set("loading", false);
-              this.refresh();
-            });
-        }
-      }
-    );
-  },
-});
+export default Route.extend();
diff --git a/assets/javascripts/discourse/templates/user/billing/subscriptions/card.hbs b/assets/javascripts/discourse/templates/user/billing/subscriptions/card.hbs
new file mode 100644
index 0000000..8b351b2
--- /dev/null
+++ b/assets/javascripts/discourse/templates/user/billing/subscriptions/card.hbs
@@ -0,0 +1,17 @@
+<h3>{{i18n "discourse_subscriptions.user.subscriptions.update_card.heading" sub_id=model}}</h3>
+
+<div class="form-vertical">
+  <div class="control-group">
+    {{subscribe-card
+    cardElement=cardElement
+    class="input-xxlarge"
+    }}
+  </div>
+
+  {{save-controls
+    action=(action "updatePaymentMethod")
+    saved=saved
+    saveDisabled=loading
+    savingText="discourse_subscriptions.user.subscriptions.update_card.single"
+  }}
+</div>
diff --git a/assets/javascripts/discourse/templates/user/billing/subscriptions.hbs b/assets/javascripts/discourse/templates/user/billing/subscriptions/index.hbs
similarity index 86%
rename from assets/javascripts/discourse/templates/user/billing/subscriptions.hbs
rename to assets/javascripts/discourse/templates/user/billing/subscriptions/index.hbs
index 9134ecb..461db5d 100644
--- a/assets/javascripts/discourse/templates/user/billing/subscriptions.hbs
+++ b/assets/javascripts/discourse/templates/user/billing/subscriptions/index.hbs
@@ -31,10 +31,15 @@
                 }}
               {{else}}
                 {{d-button
+                  action= (route-action "updateCard" subscription.id)
+                  icon="far-edit"
+                  class="btn no-text btn-icon"
+                }}
+                {{d-button
+                  class="btn-danger btn no-text btn-icon"
+                  icon="trash-alt"
                   disabled=subscription.canceled_at
-                  label="discourse_subscriptions.user.subscriptions.cancel"
                   action=(route-action "cancelSubscription" subscription)
-                  icon="times"
                 }}
               {{/if}}
             {{/if}}
diff --git a/assets/stylesheets/common/main.scss b/assets/stylesheets/common/main.scss
index 3be6365..b6f8640 100644
--- a/assets/stylesheets/common/main.scss
+++ b/assets/stylesheets/common/main.scss
@@ -48,12 +48,12 @@ table.discourse-subscriptions-user-table {
   width: 100%;
   th,
   td {
-    padding: 10px;
+    padding-top: 8px;
+    padding-bottom: 8px;
+    padding-left: 8px;
   }
   th:first-child,
-  td:first-child,
-  th:last-child,
-  td:last-child {
+  td:first-child {
     padding-left: 0;
   }
 }
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 30c9840..ad82851 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -91,6 +91,9 @@ en:
           status: Status
           discounted: Discounted
           renews: Renews
+          update_card:
+            heading: "Update Card for subscription: %{sub_id}"
+            single: "Update"
           created_at: Created
           cancel: cancel
           cancelled: cancelled
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index e9f328b..ff7edbf 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -3,3 +3,4 @@ en:
     customer_not_found: Customer not found
     card:
       declined: Card Declined
+      invalid: Card Invalid
diff --git a/config/routes.rb b/config/routes.rb
index 6a09d4a..dcb0674 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -18,7 +18,7 @@ DiscourseSubscriptions::Engine.routes.draw do
 
   namespace :user do
     resources :payments, only: [:index]
-    resources :subscriptions, only: [:index, :destroy]
+    resources :subscriptions, only: [:index, :update, :destroy]
   end
 
   get '/' => 'subscribe#index'
diff --git a/plugin.rb b/plugin.rb
index ad929fc..2647f6e 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -40,6 +40,7 @@ Discourse::Application.routes.append do
   get '/admin/plugins/discourse-subscriptions/coupons' => 'admin/plugins#index', constraints: AdminConstraint.new
   get 'u/:username/billing' => 'users#show', constraints: { username: USERNAME_ROUTE_FORMAT }
   get 'u/:username/billing/:id' => 'users#show', constraints: { username: USERNAME_ROUTE_FORMAT }
+  get 'u/:username/billing/subscriptions/card/:subscription_id' => 'users#show', constraints: { username: USERNAME_ROUTE_FORMAT }
 end
 
 load File.expand_path('lib/discourse_subscriptions/engine.rb', __dir__)
diff --git a/spec/requests/user/subscriptions_controller_spec.rb b/spec/requests/user/subscriptions_controller_spec.rb
index 8105713..9d29668 100644
--- a/spec/requests/user/subscriptions_controller_spec.rb
+++ b/spec/requests/user/subscriptions_controller_spec.rb
@@ -18,6 +18,12 @@ module DiscourseSubscriptions
         ::Stripe::Subscription.expects(:delete).never
         patch "/s/user/subscriptions/sub_12345.json"
       end
+
+      it "doesn't update payment method for subscription" do
+        ::Stripe::Subscription.expects(:update).never
+        ::Stripe::PaymentMethod.expects(:attach).never
+        put "/s/user/subscriptions/sub_12345.json", params: { payment_method: "pm_abc123abc" }
+      end
     end
 
     context "authenticated" do
@@ -82,6 +88,14 @@ module DiscourseSubscriptions
           )
         end
       end
+
+      describe "update" do
+        it "updates the payment method for subscription" do
+          ::Stripe::Subscription.expects(:update).once
+          ::Stripe::PaymentMethod.expects(:attach).once
+          put "/s/user/subscriptions/sub_1234.json", params: { payment_method: "pm_abc123abc" }
+        end
+      end
     end
   end
 end