Merge pull request #224 from debtcollective/od/bug-fixes

Fix authentication when doing XHR and other small fixes

Gemfile

@@ -8,7 +8,7 @@ gem 'rails', '~> 6.0.2.1'
 # Use postgresql as the database for Active Record
 gem 'pg', '>= 0.18', '< 2.0'
 # Use Puma as the app server
-gem 'puma', '~> 4.3.0'
+gem 'puma', '4.3.3'
 # Use SCSS for stylesheets
 gem 'sassc', '~> 1.11', '>= 1.11.4'
 # Transpile app-like JavaScript. Read more: https://github.com/rails/webpacker

Gemfile.lock

@@ -206,7 +206,7 @@ GEM
       byebug (~> 11.0)
       pry (~> 0.13.0)
     public_suffix (3.1.1)
-    puma (4.3.0)
+    puma (4.3.3)
       nio4r (~> 2.0)
     raabro (1.1.6)
     rack (2.2.2)
@@ -388,7 +388,7 @@ DEPENDENCIES
   mini_racer
   pg (>= 0.18, < 2.0)
   pry-byebug (~> 3.9.0)
-  puma (~> 4.3.0)
+  puma (= 4.3.3)
   rails (~> 6.0.2.1)
   react_on_rails (~> 11.3)
   recaptcha (~> 5.2, >= 5.2.1)

app/controllers/application_controller.rb

@@ -14,7 +14,12 @@ def current_user
   end
 
   def logged_in?
-    current_user != nil
+    !!current_user
+  end
+
+  def authenticate_user!
+    # TODO: redirect to login instead
+    head :unauthorized unless logged_in?
   end
 
   private

app/controllers/plan_changes_controller.rb

@@ -2,25 +2,26 @@
 
 class PlanChangesController < ApplicationController
   before_action :set_current_user_plan, only: %i[index create]
-  before_action :not_authorized_user, only: %i[create]
+  before_action :authenticate_user!, only: %i[create]
 
-  # GET /users/:user_id/plan_change
+  # GET /users/:user_id/plan_changes
   def index
     redirect_to root_path unless current_user&.active_subscription
+
     @user = current_user
     @plans = Plan.all
   end
 
-  # GET /users/:user_id/plan_change/new
-  # GET /users/:user_id/plan_change/new.json
+  # 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_change
-  # POST /users/:user_id/plan_change.json
+  # POST /users/:user_id/plan_changes
+  # POST /users/:user_id/plan_changes.json
   def create
-    @plan = UserPlanChange.new(user_id: plan_change_params[:user_id], old_plan_id: plan_change_params[:old_plan_id], new_plan_id: plan_change_params[:new_plan_id], status: 'pending')
+    @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')
 
     respond_to do |format|
       if @plan.save
@@ -33,15 +34,11 @@ def create
 
   private
 
-  def not_authorized_user
-    head :not_authorized unless current_user
-  end
-
   def set_current_user_plan
-    @current_plan = current_user&.active_subscription
+    @current_plan = current_user&.active_subscription&.plan
   end
 
   def plan_change_params
-    params.require(:plan_change).permit(:user_id, :old_plan_id, :new_plan_id)
+    params.require(:plan_change).permit(:old_plan_id, :new_plan_id)
   end
 end

app/controllers/subscriptions_controller.rb

@@ -1,26 +1,17 @@
 # frozen_string_literal: true
 
 class SubscriptionsController < ApplicationController
-  before_action :set_user, only: %i[destroy]
+  before_action :authenticate_user!
 
   # DELETE /user/:user_id/subscription
   # DELETE /user/:user_id/subscription.json
   def destroy
-    head :not_authorized unless current_user == @user
+    subscription = current_user.active_subscription
 
-    subscription = @user.active_subscription
-
-    subscription.active = false
-    if subscription.save
+    if subscription&.cancel!
       head :ok
     else
       head :bad_request
     end
   end
-
-  private
-
-  def set_user
-    @user = User.find(params[:user_id])
-  end
 end

app/javascript/bundles/User/components/CurrentPlan.jsx

@@ -25,17 +25,18 @@ function CurrentPlanView ({ user, activePlan, plans }) {
   const changePlan = async selectedPlanId => {
     try {
       await fetch(CHANGE_PLAN_ENDPOINT(user.id), {
-        method: 'post',
-        headers: {
-          'Content-Type': 'application/json'
-        },
         body: JSON.stringify({
           plan_change: {
-            user_id: user.id,
             old_plan_id: activePlan.id,
             new_plan_id: selectedPlanId
           }
-        })
+        }),
+        credentials: 'include',
+        headers: {
+          Accept: 'application/json',
+          'Content-Type': 'application/json'
+        },
+        method: 'post'
       })
       setPlanChanged(true)
     } catch (error) {

app/javascript/bundles/User/components/SubscriptionCancel.jsx

@@ -29,8 +29,9 @@ function SubscriptionCancelView ({
   isSubscriptionChanging
 }) {
   const classes = useStyles()
+  const isSubscriptionActive = subscription && subscription.active
   const [open, setOpen] = useState(false)
-  const [active, toggleActive] = useState(subscription.active)
+  const [active, toggleActive] = useState(isSubscriptionActive)
 
   const handleClickOpen = () => {
     setOpen(true)
@@ -44,17 +45,22 @@ function SubscriptionCancelView ({
     const isSubscriptionCancelled = await fetch(
       SUBSCRIPTION_CANCEL_URL(user.id),
       {
-        method: 'delete'
+        method: 'delete',
+        credentials: 'include',
+        headers: {
+          Accept: 'application/json',
+          'Content-Type': 'application/json'
+        }
       }
     )
 
-    toggleActive(!subscription.active)
+    toggleActive(!isSubscriptionActive)
 
     handleClose()
   }
 
-  if (!subscription.active) {
-    return null
+  if (!isSubscriptionActive) {
+    return "You don't have an active membership"
   }
 
   return (

app/models/subscription.rb

@@ -12,12 +12,16 @@ class Subscription < ApplicationRecord
   validates :plan_id, presence: true
   validates :user_id, uniqueness: { scope: %i[plan_id active] }, if: :user?
 
-  scope :active, -> { where(active: true).first }
-
   def user?
     !user_id.blank?
   end
 
+  def cancel!
+    self.active = false
+
+    save
+  end
+
   private
 
   def store_start_date

app/models/user.rb

@@ -42,6 +42,6 @@ def current_streak
   end
 
   def active_subscription
-    subscriptions.active unless subscriptions.active.blank?
+    subscriptions.where(active: true).first
   end
 end

app/views/static_pages/home.html.erb

@@ -114,34 +114,34 @@
   <nav>
     <ul>
       <li class="title"><h5>Organize</h5></li>
-      <li><a href="#">Collectives</a></li>
-      <li><a href="#">Campaigns</a></li>
-      <li><a href="#">Debt Map</a></li>
+      <li><a href="https://community.debtcollective.org/">Community</a></li>
     </ul>
     <ul>
       <li class="title"><h5>Take Action</h5></li>
-      <li><a href="#">Dispute Your Debt</a></li>
-      <li><a href="#">Campaigns</a></li>
+      <li><a href="https://community.debtcollective.org/calendar">Events</a></li>
+      <li><a href="https://tools.debtcollective.org/">Dispute Your Debt</a></li>
     </ul>
     <ul>
       <li class="title"><h5>Learn More</h5></li>
-      <li><a href="#">The Power Report</a></li>
-      <li><a href="#">About Us</a></li>
-      <li><a href="#">Community Wiki</a></li>
+      <li><a href="https://powerreport.debtcollective.org/">The Power Report</a></li>
+      <li><a href="https://debtcollective.org/#about">About Us</a></li>
+      <li><a href="https://wiki.debtcollective.org/">Community Wiki</a></li>
     </ul>
     <ul>
       <li class="title"><h5>More</h5></li>
-      <li><a href="#">Contact</a></li>
-      <li><a href="#">Donate</a></li>
-      <li><a href="#">Open Source</a></li>
+      <li><a href="https://tools.debtcollective.org/contact">Contact</a></li>
+      <li><a href="https://membership.debtcollective.org">Donate</a></li>
+      <li><a href="https://github.com/debtcollective/">Open Source</a></li>
     </ul>
   </nav>
   <div class="notice">
-    <%= image_tag('logo-dark.svg', alt: 'Debt Collective')%>
+    <a href="https://debtcollective.org">
+      <%= image_tag('logo-dark.svg', alt: 'Debt Collective')%>
+    </a>
     <p>
       <a id="to-top-link" href="#">Back to top</a>
       <span>Copyright <%= Time.new.year %></span>
-      <a href="#">Terms and Conditions</a>
+      <a href="https://community.debtcollective.org/tos">Terms and Conditions</a>
     </p>
   </div>
 </footer>

app/views/subscription_charges/new.html.erb

@@ -5,10 +5,12 @@
 <% end %>
 <div id="checkout">
   <div class='content'>
-    <section id="checkout-plan">
-      <h1><%= @current_plan.name %></h1>
-      <p><%= @current_plan.description %></p>
-    </section>
+    <% if @current_plan %>
+      <section id="checkout-plan">
+        <h1><%= @current_plan.name %></h1>
+        <p><%= @current_plan.description %></p>
+      </section>
+    <% end %>
 
     <section id='checkout-payment'>
       <%= form_with(model: @subscription, url: {controller: "subscription_charges", action: :create }, local: true, id: 'payment-form') do |form| %>
@@ -23,7 +25,7 @@
             </ul>
           </div>
         <% end %>
-        
+
         <!-- Used to display form errors from stripe elements. -->
         <div id="card-errors" role="alert"></div>
 

app/views/users/subscription.html.erb

@@ -1,5 +1,10 @@
 <section class="section-content">
   <p id="notice"><%= notice %></p>
 
-  <%= react_component("SubscriptionCancel", props: {user: @user, subscription: @user.active_subscription, activePlan: @user.active_subscription&.plan, isSubscriptionChanging: @is_subscription_changing}) %>
+  <%= react_component("SubscriptionCancel", props: {
+    user: @user,
+    subscription: @user.active_subscription,
+    activePlan: @user.active_subscription&.plan,
+    isSubscriptionChanging: @is_subscription_changing
+  }) %>
 </section>

config/webpacker.yml

@@ -54,9 +54,6 @@ development:
   <<: *default
   compile: true
 
-  # Verifies that correct packages and versions are installed by inspecting package.json, yarn.lock, and node_modules
-  check_yarn_integrity: true
-
   # Reference: https://webpack.js.org/configuration/dev-server/
   dev_server:
     https: false