Fix payment form resubmission before server response and other payments flow improvements

app/assets/javascripts/campaigns.js.coffee

@@ -11,7 +11,7 @@ Crowdhoster.campaigns =
 
     # Checkout section functions:
     if($('#checkout').length)
-      $('html,body').animate({scrollTop: $('#checkout').offset().top})
+      $('html,body').animate({scrollTop: $('#header')[0].scrollHeight})
 
     $('#quantity').on "change", (e) ->
       quantity = $(this).val()
@@ -59,6 +59,7 @@ Crowdhoster.campaigns =
         this.submit()
 
   submitPaymentForm: (form) ->
+    $('#refresh-msg').show()
     $('#errors').hide()
     $('#errors').html('')
     $('button[type="submit"]').attr('disabled', true).html('Processing, please wait...')
@@ -76,6 +77,7 @@ Crowdhoster.campaigns =
 
     errors = crowdtilt.card.validate(cardData)
     if !$.isEmptyObject(errors)
+      $('#refresh-msg').hide()
       $.each errors, (index, value) ->
         $('#errors').append('<p>' + value + '</p>')
       $('#errors').show()
@@ -96,8 +98,10 @@ Crowdhoster.campaigns =
         input = $('<input name="ct_card_id" value="' + token + '" type="hidden" />');
         form = document.getElementById('payment_form')
         form.appendChild(input[0])
+        $('#client_timestamp').val((new Date()).getTime())
         form.submit()
       else
+        $('#refresh-msg').hide()
         $('#errors').append('<p>An error occurred. Please check your credit card details and try again.</p><br><p>If you continue to experience issues, please <a href="mailto:[email protected]?subject=Support request for a payment issue&body=PLEASE DESCRIBE YOUR PAYMENT ISSUES HERE">click here</a> to contact support.</p>')
         $('#errors').show()
         $('.loader').hide()

app/controllers/admin/campaigns_controller.rb

@@ -282,11 +282,11 @@ def payments
       if payment
         @payments = [payment]
       else
-        @payments = @campaign.payments.order("created_at ASC")
+        @payments = @campaign.payments_completed.order("created_at ASC")
         flash.now[:error] = "Contributor not found for " + params[:payment_id]
       end
     else
-      @payments = @campaign.payments.order("created_at ASC")
+      @payments = @campaign.payments_completed.order("created_at ASC")
     end
 
     respond_to do |format|

app/controllers/campaigns_controller.rb

@@ -61,6 +61,7 @@ def checkout_payment
 
   def checkout_process
 
+    client_timestamp = params.has_key?(:client_timestamp) ? params[:client_timestamp].to_i : nil
     ct_user_id = params[:ct_user_id]
     ct_card_id = params[:ct_card_id]
     fullname = params[:fullname]
@@ -103,17 +104,20 @@ def checkout_process
     # TODO: Check to make sure the amount is valid here
 
     # Create the payment record in our db, if there are errors, redirect the user
-    @payment = @campaign.payments.new fullname: fullname,
-                                      email: email,
-                                      billing_postal_code: billing_postal_code,
-                                      quantity: quantity,
-                                      address_one: address_one,
-                                      address_two: address_two,
-                                      city: city,
-                                      state: state,
-                                      postal_code: postal_code,
-                                      country: country,
-                                      additional_info: additional_info
+     payment_params = {client_timestamp: client_timestamp,
+                       fullname: fullname,
+                       email: email,
+                       billing_postal_code: billing_postal_code,
+                       quantity: quantity,
+                       address_one: address_one,
+                       address_two: address_two,
+                       city: city,
+                       state: state,
+                       postal_code: postal_code,
+                       country: country,
+                       additional_info: additional_info}
+
+     @payment = @campaign.payments.new(payment_params)
 
     if [email protected]?
       message = ''
@@ -123,6 +127,23 @@ def checkout_process
       redirect_to checkout_amount_url(@campaign), flash: { error: message[0...-2] } and return
     end
 
+    # Check if there's an existing payment with the same payment_params and client_timestamp. 
+    # If exists, look at the status to route accordingly. 
+    if !client_timestamp.nil? && existing_payment = @campaign.payments.where(payment_params).first
+      case existing_payment.status
+      when nil
+        flash_msg = { info: "Your payment is still being processed! If you have not received a confirmation email, please try again or contact support by emailing [email protected]" }
+      when 'error'
+        flash_msg = { error: "There was an error processing your payment. Please try again or contact support by emailing [email protected]." }
+      else
+        # A status other than nil or 'error' indicates success! Treat as original payment
+        redirect_to checkout_confirmation_url(@campaign), :status => 303, :flash => { payment_guid: @payment.ct_payment_id } and return
+      end
+      redirect_to checkout_amount_url(@campaign), flash: flash_msg and return
+    end
+
+    @payment.save
+
     # Execute the payment via the Crowdtilt API, if it fails, redirect user
     begin
       payment = {
@@ -150,8 +171,9 @@ def checkout_process
       logger.info "CROWDTILT API RESPONSE:"
       logger.info response
     rescue => exception
+      @payment.update_attribute(:status, 'error')
       logger.info "ERROR WITH POST TO /payments: #{exception.message}"
-      redirect_to checkout_amount_url(@campaign), flash: { error: "There was an error processing your payment, please try again or contact support by emailing [email protected]" } and return
+      redirect_to checkout_amount_url(@campaign), flash: { error: "There was an error processing your payment. Please try again or contact support by emailing [email protected]" } and return
     end
 
     # Sync payment data
@@ -163,22 +185,22 @@ def checkout_process
     @campaign.update_api_data(response['payment']['campaign'])
     @campaign.save
 
-    # Send a confirmation email
-    begin
-      UserMailer.payment_confirmation(@payment, @campaign).deliver
-    rescue => exception
-      logger.info "ERROR WITH EMAIL RECEIPT: #{exception.message}"
-    end
+    # Send confirmation emails
+    UserMailer.payment_confirmation(@payment, @campaign).deliver rescue 
+      logger.info "ERROR WITH EMAIL RECEIPT: #{$!.message}"
+
+    AdminMailer.payment_notification(@payment.id).deliver rescue 
+      logger.info "ERROR WITH ADMIN NOTIFICATION EMAIL: #{$!.message}"
 
     redirect_to checkout_confirmation_url(@campaign), :status => 303, :flash => { payment_guid: @payment.ct_payment_id }
 
   end
 
   def checkout_confirmation
     @payment = Payment.where(:ct_payment_id => flash[:payment_guid]).first
-    flash[:payment_guid] = nil # Unset flash because application renders all flash vars (long-term should be refactored)
+    flash.keep(:payment_guid) # Preserve on refresh of this page only
 
-    if !@payment
+    if flash[:payment_guid].nil? || !@payment
       redirect_to campaign_home_url(@campaign)
     end
   end

app/models/campaign.rb

@@ -68,12 +68,21 @@ def rewards?
     (self.payment_type != 'fixed' && self.rewards.length > 0)
   end
 
+  def payments_completed
+    self.payments.where(:status => %w(authorized charged released rejected refunded offline))
+  end
+
+  def payments_successful
+    # 'rejected' is a post-tilt state, so they are included in successful payments.
+    self.payments.where(:status => %w(authorized charged released rejected offline))
+  end
+
   def raised_amount
-    payments.where("payments.status!='refunded'").sum(:amount)/100.0
+    self.payments_successful.sum(:amount)/100.0
   end
 
   def number_of_contributions
-    payments.where("payments.status!='refunded'").count
+    self.payments_successful.count
   end
 
   def tilt_percent

app/models/payment.rb

@@ -2,16 +2,14 @@ class Payment < ActiveRecord::Base
   attr_accessible :ct_payment_id, :status, :amount, :user_fee_amount, :admin_fee_amount, :fullname, :email,
                   :card_type, :card_last_four, :card_expiration_month, :card_expiration_year, :billing_postal_code,
                   :address_one, :address_two, :city, :state, :postal_code, :country, :quantity,
-                  :additional_info
+                  :additional_info, :client_timestamp
 
   validates :fullname, :quantity, presence: true
   validates :email, presence: true, email: true
 
   belongs_to :campaign
   belongs_to :reward
 
-  after_create :send_admin_notification
-
   def self.to_csv(options={})
     #db_columns = %w{fullname email quantity amount user_fee_amount created_at status ct_payment_id}
     csv_columns = ['Name', 'Email', 'Quantity', 'Amount', 'User Fee', 'Date', 'Reward',
@@ -60,10 +58,6 @@ def update_api_data(payment)
     self.card_expiration_year = payment['card']['expiration_year']
   end
 
-  def send_admin_notification
-    AdminMailer.payment_notification(self.id).deliver
-  end
-
   def refund!
     self.campaign.production_flag ? Crowdtilt.production(Settings.first) : Crowdtilt.sandbox
     Crowdtilt.post("/campaigns/#{self.campaign.ct_campaign_id}/payments/#{self.ct_payment_id}/refund")

app/views/campaigns/checkout_payment.html.erb

@@ -127,6 +127,7 @@
           <input id="amount" type="hidden" name="amount" value="<%= @amount %>">
           <input id="fee" type="hidden" name="fee" value="<%= @fee %>">
           <input id="reward" type="hidden" name="reward" value="<%= @reward ? @reward.id : '0' %>">
+          <input id="client_timestamp" type="hidden" name="client_timestamp" value="">
 
           <% if [email protected]_flag %>
             <div style="color: red; margin-top: 10px;">This campaign is in sandbox mode, your card will not actually be charged.</div>
@@ -135,6 +136,7 @@
           <div class="payment-submit">
             <button class="btn btn-primary show_loader" type="submit" data-total="<%= number_with_precision(@total, precision: 2) %>" data-loader="payment_form">Confirm payment of $<%= number_with_precision(@total, precision: 2) %></button>
             <span class="loader" data-loader="payment_form" style="display:none"></span>
+            <div id="refresh-msg" style="display: none; color: red; margin-top: 10px">Your payment is being processed! Please do not refresh this page.</div>
           </div>
 
         </form>

app/views/layouts/application.html.erb

@@ -20,7 +20,7 @@
     <%= render 'layouts/social_js' %>
     <%= render 'layouts/navbar' %>
     <%= render 'theme/views/header' %>
-    <% flash.each do |key, value| %>
+    <% flash.to_hash().slice(:success, :info, :notice, :error).each do |key, value| %>
       <% if !value.blank? %><div class="alert alert-<%= key %>"><%= value %></div><% end %>
     <% end %>
     <div id="main">

db/migrate/20131206225359_add_client_timestamp_to_payments.rb

@@ -0,0 +1,5 @@
+class AddClientTimestampToPayments < ActiveRecord::Migration
+  def change
+    add_column :payments, :client_timestamp, :integer, :limit => 8
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20131128003747) do
+ActiveRecord::Schema.define(:version => 20131206225359) do
 
   create_table "campaigns", :force => true do |t|
     t.string   "name"
@@ -112,8 +112,8 @@
     t.string   "card_expiration_month"
     t.string   "card_expiration_year"
     t.integer  "campaign_id"
-    t.datetime "created_at",            :null => false
-    t.datetime "updated_at",            :null => false
+    t.datetime "created_at",                         :null => false
+    t.datetime "updated_at",                         :null => false
     t.string   "address_one"
     t.string   "address_two"
     t.string   "city"
@@ -124,6 +124,7 @@
     t.integer  "reward_id"
     t.text     "additional_info"
     t.string   "billing_postal_code"
+    t.integer  "client_timestamp",      :limit => 8
   end
 
   create_table "rewards", :force => true do |t|