Merge pull request #319 from debtcollective/od/subscription-fixes

Fix subscription plan change
debtcollective · Sep 15, 2020 · de59b7c · de59b7c
2 parents 7f341f9 + 370c476
commit de59b7c
Show file tree

Hide file tree

Showing 29 changed files with 230 additions and 124 deletions.
diff --git a/app/assets/stylesheets/_main-nav.scss b/app/assets/stylesheets/_main-nav.scss
@@ -5,7 +5,7 @@
   align-items: center;
   padding: 1rem 5rem;
 
-  @media(min-width: 80rem) {
+  @media (min-width: 80rem) {
     justify-content: space-between;
   }
 
@@ -17,7 +17,7 @@
       margin-right: auto;
     }
 
-    @media(min-width: 80rem) {
+    @media (min-width: 80rem) {
       display: flex;
     }
   }
@@ -27,10 +27,9 @@
 
     li {
       margin: 0 2rem;
-      font-weight: bold;
     }
 
-    @media(min-width: 80rem) {
+    @media (min-width: 80rem) {
       display: flex;
     }
   }
@@ -49,7 +48,7 @@
     display: none;
     padding: 1rem 3rem;
 
-    @media(min-width: 80rem) {
+    @media (min-width: 80rem) {
       display: flex;
     }
   }

diff --git a/app/controllers/plan_changes_controller.rb b/app/controllers/plan_changes_controller.rb
@@ -9,25 +9,30 @@ def index
     redirect_to root_path unless current_user&.active_subscription
 
     @user = current_user
+    @pending_plan_change = @user.pending_plan_change
     @plans = Plan.all
   end
 
-  # GET /users/:user_id/plan_changes/new
-  # GET /users/:user_id/plan_changes/new.json
-  def new
-    @plan_change = UserPlanChange.new
-  end
-
   # POST /users/:user_id/plan_changes
   # POST /users/:user_id/plan_changes.json
   def create
-    @plan = UserPlanChange.new(user_id: current_user.id, old_plan_id: plan_change_params[:old_plan_id], new_plan_id: plan_change_params[:new_plan_id], status: 'pending')
+    @plan_change =
+      current_user.user_plan_changes.new(
+        old_plan_id: plan_change_params[:old_plan_id],
+        new_plan_id: plan_change_params[:new_plan_id],
+        status: 'pending'
+      )
 
     respond_to do |format|
-      if @plan.save
-        format.json { render json: @plan, status: :ok }
+      if @plan_change.save
+        format.html { render :index, notice: 'Plan changed successfully' }
+        format.json { render json: @plan_change, status: :ok }
       else
-        format.html { render :index }
+        format.html { render :index, notice: 'Error while changing plans' }
+        format.json do
+          render json: { errors: @plan_change.errors.full_messages },
+                 status: :ok
+        end
       end
     end
   end

diff --git a/app/javascript/bundles/User/components/CurrentPlan.jsx b/app/javascript/bundles/User/components/CurrentPlan.jsx
@@ -18,16 +18,18 @@ const useStyles = makeStyles(theme => ({
 
 const CHANGE_PLAN_ENDPOINT = id => `/users/${id}/plan_changes`
 
-function CurrentPlanView ({ user, activePlan, plans }) {
+function CurrentPlanView ({ user, currentPlan, pendingPlanChange, plans }) {
   const classes = useStyles()
-  const [changedPlan, setPlanChanged] = useState(false)
+  const [planChange, setPlanChange] = useState(pendingPlanChange)
+  const pendingPlanChangePlan =
+    planChange && plans.find(plan => plan.id == planChange.new_plan_id)
 
   const changePlan = async selectedPlanId => {
     try {
-      await fetch(CHANGE_PLAN_ENDPOINT(user.id), {
+      const response = await fetch(CHANGE_PLAN_ENDPOINT(user.id), {
         body: JSON.stringify({
           plan_change: {
-            old_plan_id: activePlan.id,
+            old_plan_id: currentPlan.id,
             new_plan_id: selectedPlanId
           }
         }),
@@ -38,9 +40,12 @@ function CurrentPlanView ({ user, activePlan, plans }) {
         },
         method: 'post'
       })
-      setPlanChanged(true)
+
+      const json = await response.json()
+      setPlanChange(json)
     } catch (error) {
-      console.error(error) // TODO: Replace with sentry
+      console.error(error)
+      Sentry.captureException(error)
     }
   }
 
@@ -49,16 +54,17 @@ function CurrentPlanView ({ user, activePlan, plans }) {
       <Paper className={classes.root}>
         <h3>Change your susbcription</h3>
         <p>
-          The plan you're using to contribute is{' '}
-          <strong> {activePlan.name}</strong>.
+          Your membership tier is <strong>{currentPlan.name}</strong>.
         </p>
       </Paper>
       <br />
-      <h2>Available plans</h2>
-      <p>You can change your current plan for another one at anytime.</p>
-      {changedPlan && (
+      <h2>Available tiers</h2>
+      <p>You can change your membership tier to another one at anytime.</p>
+      {planChange && (
         <p className='notice--subscription'>
-          We have changed your subscription successfully.
+          Your plan is scheduled to change to{' '}
+          <strong>{pendingPlanChangePlan.name}</strong>. The new amount will be
+          charged on your next billing cycle.
         </p>
       )}
       {plans.map(plan => {
@@ -78,10 +84,10 @@ function CurrentPlanView ({ user, activePlan, plans }) {
             <Button
               variant='contained'
               color='primary'
-              disabled={activePlan.id === plan.id || changedPlan}
+              disabled={currentPlan.id === plan.id || !!planChange}
               onClick={changeHandler}
             >
-              Pick plan
+              Select this tier
             </Button>
           </Paper>
         )

diff --git a/app/javascript/bundles/User/components/DonationsHistory.jsx b/app/javascript/bundles/User/components/DonationsHistory.jsx
@@ -34,7 +34,7 @@ const calcDonationTotal = donations => {
   return donations.reduce((acc, donation) => (acc += donation), 0)
 }
 
-function DonationsView ({ activePlan, donations }) {
+function DonationsView ({ currentPlan, donations }) {
   const classes = useStyles()
   const donatedAmount = calcDonationTotal(
     donations.map(donation => Number(donation.amount))

diff --git a/app/javascript/bundles/User/components/SubscriptionCancel.jsx b/app/javascript/bundles/User/components/SubscriptionCancel.jsx
@@ -48,7 +48,7 @@ const NoSubscriptionView = ({ user }) => {
 function SubscriptionCancelView ({
   user,
   subscription,
-  activePlan,
+  currentPlan,
   isSubscriptionChanging
 }) {
   const classes = useStyles()
@@ -95,8 +95,7 @@ function SubscriptionCancelView ({
       <Paper className={classes.root}>
         <h3>You're subscribed</h3>
         <p>
-          The plan you're using to contribute is{' '}
-          <strong> {activePlan.name}</strong>.
+          Your membership tier is <strong> {currentPlan.name}</strong>.
         </p>
         {!isSubscriptionChanging && (
           <Button
@@ -122,12 +121,6 @@ function SubscriptionCancelView ({
         <DialogTitle id='cancel-subscription-title'>
           Do you want to terminate your current subscription?
         </DialogTitle>
-        <DialogContent>
-          <DialogContentText id='cancel-subscription-description'>
-            Terminating your subscription will stop your current plan and the
-            benefits you receive from it.
-          </DialogContentText>
-        </DialogContent>
         <DialogActions>
           <Button onClick={handleClose} color='primary'>
             Close

diff --git a/app/jobs/find_overdue_subscriptions_job.rb b/app/jobs/find_overdue_subscriptions_job.rb
@@ -4,11 +4,11 @@ class FindOverdueSubscriptionsJob < ApplicationJob
   queue_as :default
 
   def perform
-    subscriptions_to_charge = Subscription.where(active: true).select do |subscription|
-      # create charge if there's no last_charge_at date, meaning it would be the
-      # first time we attempt to charge this customer this subscription.
-      subscription.last_charge_at ? subscription.last_charge_at <= 30.days.ago : true
-    end
+    subscriptions_to_charge =
+      Subscription.where(active: true).where(
+        'last_charge_at IS NULL OR last_charge_at <= ?',
+        30.days.ago
+      )
 
     subscriptions_to_charge.each do |subscription_to_charge|
       SubscriptionPaymentJob.perform_later(subscription_to_charge)

diff --git a/app/jobs/toggle_user_active_subscription_plan_job.rb b/app/jobs/toggle_user_active_subscription_plan_job.rb
@@ -4,24 +4,31 @@ class ToggleUserActiveSubscriptionPlanJob < ApplicationJob
   queue_as :default
 
   def perform(user_plan_change)
-    # find the user
+    # find user
     user = User.find(user_plan_change.user_id)
 
     # disable current user subscription.
     old_subscription = user.active_subscription
+
     ActiveRecord::Base.transaction do
       old_subscription.update(active: false)
 
       # creates a new subscription with the new plan details.
-      new_subscription = Subscription.new(
-        user_id: user_plan_change.user_id,
-        plan_id: user_plan_change.new_plan_id,
-        last_charge_at: old_subscription.last_charge_at,
-        active: true
-      )
+      new_subscription =
+        Subscription.new(
+          user_id: user.id,
+          plan_id: user_plan_change.new_plan_id,
+          last_charge_at: old_subscription.last_charge_at,
+          active: true
+        )
 
       if new_subscription.save
-        UserPlanChange.find(user_plan_change.id).update(status: "succeeded")
+        UserPlanChange.find(user_plan_change.id).update(status: 'succeeded')
+      else
+        Raven.capture_message(
+          "We couldn't process subscription plan change",
+          extra: { user_id: user.id, subscription_id: old_subscription.id }
+        )
       end
     end
   end

diff --git a/app/models/subscription.rb b/app/models/subscription.rb
@@ -15,18 +15,16 @@
 #
 # Indexes
 #
-#  index_subscriptions_on_plan_id                         (plan_id)
-#  index_subscriptions_on_user_id                         (user_id)
-#  index_subscriptions_on_user_id_and_plan_id_and_active  (user_id,plan_id,active) UNIQUE
+#  index_subscriptions_on_plan_id  (plan_id)
+#  index_subscriptions_on_user_id  (user_id)
 #
 class Subscription < ApplicationRecord
   before_create :store_start_date
 
   belongs_to :user, optional: true
   belongs_to :plan
 
-  validates :plan_id, presence: true
-  validates :user_id, uniqueness: {scope: %i[plan_id active]}, if: :user?
+  validate :only_one_active_subscription, on: :create
 
   def user?
     !user_id.blank?
@@ -43,4 +41,10 @@ def cancel!
   def store_start_date
     self.start_date = DateTime.now if start_date.nil?
   end
+
+  def only_one_active_subscription
+    if Subscription.exists?(user_id: user_id, active: true)
+      errors.add(:base, 'already has an active subscription')
+    end
+  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
@@ -32,6 +32,7 @@ class User < ApplicationRecord
 
   has_many :subscriptions
   has_many :donations
+  has_many :user_plan_changes
 
   validates :external_id, presence: true
 
@@ -56,16 +57,26 @@ def admin?
   end
 
   def current_streak
-    return nil unless active_subscription
+    subscription = active_subscription
 
-    start_date ||= active_subscription.start_date
+    return nil unless subscription
 
-    return 1 if (Date.today - start_date.to_date).zero? # first month of subscription
+    start_date = subscription.start_date
+    today = Date.today
 
-    ((Date.today - start_date.to_date).to_f / 365 * 12).round
+    months =
+      (today.year * 12 + today.month) -
+        (start_date.year * 12 + start_date.month)
+    months += 1 if months == 0
+
+    months
+  end
+
+  def pending_plan_change
+    user_plan_changes.where(status: :pending).first
   end
 
   def active_subscription
-    subscriptions.eager_load(:plan).where(active: true).first
+    subscriptions.includes(:plan).where(active: true).last
   end
 end
diff --git a/app/models/user_plan_change.rb b/app/models/user_plan_change.rb
@@ -15,8 +15,16 @@
 class UserPlanChange < ApplicationRecord
   belongs_to :user
 
-  enum status: {succeeded: 0, pending: 1, failed: 2}
+  enum status: { succeeded: 0, pending: 1, failed: 2 }
 
   validates :old_plan_id, :new_plan_id, :user_id, presence: true
-  validates :user_id, uniqueness: true
+  validate :only_one_pending_plan_change, on: :create
+
+  private
+
+  def only_one_pending_plan_change
+    if UserPlanChange.exists?(user_id: user_id, status: 'pending')
+      errors.add(:base, 'already has a pending plan change')
+    end
+  end
 end
diff --git a/app/views/layouts/admin.html.erb b/app/views/layouts/admin.html.erb
@@ -8,7 +8,7 @@
     <%= stylesheet_link_tag 'application', media: 'all', 'data-turbolinks-track': 'reload' %>
     <link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Roboto:300,400,500,700&display=swap" />
     <link rel="stylesheet" href="https://fonts.googleapis.com/icon?family=Material+Icons" />
-    <link href="https://fonts.googleapis.com/css?family=Libre+Franklin&display=swap" rel="stylesheet">
+    <link href="https://fonts.googleapis.com/css2?family=Libre+Franklin:wght@300;400;500;700&display=swap" rel="stylesheet">
 
     <%= render 'layouts/sentry' %>
 

diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
@@ -7,14 +7,8 @@
     <%= csp_meta_tag %>
     <%= content_for?(:head) ? yield(:head) : '' %>
     <%= stylesheet_link_tag 'application', media: 'all', 'data-turbolinks-track': 'reload' %>
-    <link
-      rel="stylesheet"
-      href="https://fonts.googleapis.com/icon?family=Material+Icons"
-    />
-    <link
-      href="https://fonts.googleapis.com/css?family=Libre+Franklin&display=swap"
-      rel="stylesheet"
-    />
+    <link rel="stylesheet" href="https://fonts.googleapis.com/icon?family=Material+Icons" />
+    <link href="https://fonts.googleapis.com/css2?family=Libre+Franklin:wght@300;400;500;700&display=swap" rel="stylesheet" />
 
     <link rel="apple-touch-icon" sizes="152x152" href="/apple-touch-icon.png" />
     <link rel="icon" type="image/png" sizes="32x32" href="/favicon-32x32.png" />

diff --git a/app/views/layouts/backoffice.html.erb b/app/views/layouts/backoffice.html.erb
@@ -9,7 +9,7 @@
 
     <%= stylesheet_link_tag 'application', media: 'all', 'data-turbolinks-track': 'reload' %>
     <link rel="stylesheet" href="https://fonts.googleapis.com/icon?family=Material+Icons" />
-    <link href="https://fonts.googleapis.com/css?family=Libre+Franklin&display=swap" rel="stylesheet">
+    <link href="https://fonts.googleapis.com/css2?family=Libre+Franklin:wght@300;400;500;700&display=swap" rel="stylesheet">
 
     <link rel="apple-touch-icon" sizes="152x152" href="/apple-touch-icon.png">
     <link rel="icon" type="image/png" sizes="32x32" href="/favicon-32x32.png">

diff --git a/app/views/plan_changes/index.html.erb b/app/views/plan_changes/index.html.erb
@@ -1,3 +1,3 @@
 <section class="section-content">
-  <%= react_component("CurrentPlan", props: {user: @user, activePlan: @user.active_subscription&.plan, plans: @plans}) %>
+  <%= react_component("CurrentPlan", props: {user: @user, currentPlan: @user.active_subscription&.plan, pendingPlanChange: @pending_plan_change, plans: @plans}) %>
 </section>
diff --git a/app/views/users/subscription.html.erb b/app/views/users/subscription.html.erb
@@ -4,7 +4,7 @@
   <%= react_component("SubscriptionCancel", props: {
     user: @user,
     subscription: @subscription,
-    activePlan: @subscription&.plan,
+    currentPlan: @subscription&.plan,
     isSubscriptionChanging: @is_subscription_changing
   }) %>
 </section>