Refactor PaymentsController for current_user and enhance validations

app/controllers/payments_controller.rb

@@ -2,6 +2,8 @@
   require 'time'
 
   class PaymentsController < ApplicationController
+    MAX_PAYMENT_AMOUNT = 2000
+
     protect_from_forgery with: :exception
     skip_before_action :verify_authenticity_token, only: [:payment_receipt]
     before_action :verify_payment_callback, only: [:payment_receipt]
@@ -41,7 +43,13 @@ def payment_receipt
     end
 
     def make_payment
-      processed_url = generate_hash(@current_user, params['amount'])
+      amount = validated_payment_amount(params['amount'])
+      if amount.nil?
+        redirect_to all_payments_path, alert: 'Please enter a valid payment amount.'
+        return
+      end
+
+      processed_url = generate_hash(current_user, amount)
       redirect_to processed_url, allow_other_host: true
     end
 
@@ -55,6 +63,7 @@ def payment_show
       @cost_subscription = current_application_settings.subscription_cost.to_f
       @total_cost = cost_lodging + cost_partner + (@has_subscription ?  @cost_subscription : 0)
       @balance_due = @total_cost - @ttl_paid
+      @max_payment_amount = max_payment_amount_for(@balance_due)
     end
 
     def delete_manual_payment
@@ -115,6 +124,36 @@ def generate_hash(current_user, amount=100)
         final_url = connection_hash[url_to_use] + '?' + url_for_payment + 'hash=' + encoded_hash
       end
 
+      def validated_payment_amount(raw_amount)
+        amount = Integer(raw_amount, exception: false)
+        return nil if amount.nil? || amount <= 0
+
+        balance_due = current_balance_due
+        return nil if balance_due <= 0
+
+        max_amount = max_payment_amount_for(balance_due)
+        return nil if amount > max_amount
+
+        amount
+      end
+
+      def max_payment_amount_for(balance_due)
+        [balance_due.floor, MAX_PAYMENT_AMOUNT].min
+      end
+
+      def current_balance_due
+        current_application
+        return 0.0 if @current_application.nil?
+
+        cost_lodging = Lodging.find_by(description: @current_application.lodging_selection)&.cost.to_f
+        cost_partner = @current_application.partner_registration&.cost.to_f
+        has_subscription = @current_application.subscription
+        cost_subscription = current_application_settings.subscription_cost.to_f
+        total_cost = cost_lodging + cost_partner + (has_subscription ? cost_subscription : 0)
+        total_paid = Payment.current_conference_payments.where(user_id: current_user, transaction_status: '1').pluck(:total_amount).map(&:to_f).sum / 100
+        total_cost - total_paid
+      end
+
       def url_params
         params.permit(:amount, :transactionType, :transactionStatus, :transactionId, :transactionTotalAmount, :transactionDate, :transactionAcountType, :transactionResultCode, :transactionResultMessage, :orderNumber, :timestamp, :hash, :conf_year)
       end

app/views/payments/payment_show.html.erb

@@ -61,10 +61,13 @@
                 <div class="input-group-prepend">
                   <span class="input-group-text">$</span>
                 </div>
-                <%= f.number_field :amount, value: "#{@balance_due.to_i}", 
-                    within: 1..2000, required: true, class: 'form-control' %>
+                <%= f.number_field :amount, value: @max_payment_amount,
+                    within: 1..@max_payment_amount, required: true, class: 'form-control' %>
                 <%= f.submit "Pay Now", class: 'btn btn-sm btn-success ml-2' %>
               </div>
+              <small class="form-text text-muted mt-2">
+                The amount is prefilled with your full balance due, but you can enter a different amount to make a partial payment.
+              </small>
             </div>
           </div>
         <% end %>

spec/controllers/payments_controller_spec.rb

@@ -168,14 +168,22 @@
       end
 
       context 'with different amounts' do
-        it 'handles decimal amounts correctly' do
+        it 'rejects decimal amounts' do
           post :make_payment, params: { amount: '50.50' }
-          expect(response.location).to include('amountDue=5000') # 50.50.to_i * 100 = 50 * 100
+          expect(response).to redirect_to(all_payments_path)
+          expect(flash[:alert]).to eq('Please enter a valid payment amount.')
         end
 
-        it 'handles zero amount' do
+        it 'rejects zero amount' do
           post :make_payment, params: { amount: '0' }
-          expect(response.location).to include('amountDue=0')
+          expect(response).to redirect_to(all_payments_path)
+          expect(flash[:alert]).to eq('Please enter a valid payment amount.')
+        end
+
+        it 'rejects amount above current balance due' do
+          post :make_payment, params: { amount: '500' }
+          expect(response).to redirect_to(all_payments_path)
+          expect(flash[:alert]).to eq('Please enter a valid payment amount.')
         end
       end
     end

spec/requests/payments_spec.rb

@@ -33,16 +33,22 @@
       before do
         sign_in user
         allow_any_instance_of(PaymentsController).to receive(:user_has_payments?).and_return(true)
+        allow_any_instance_of(PaymentsController).to receive(:payments_open?).and_return(true)
+        allow_any_instance_of(PaymentsController).to receive(:current_application) do |controller|
+          controller.instance_variable_set(:@current_application, application)
+        end
+        create(:lodging, description: application.lodging_selection, cost: 100.0)
+        application.update!(partner_registration: create(:partner_registration, cost: 0.0), subscription: false)
+        create(:payment, :current_conference, user: user, transaction_status: '1', total_amount: '3000')
 
-        mock_application = instance_double(Application,
-          lodging_selection: "Standard",
-          partner_registration: instance_double(PartnerRegistration, cost: 0.0),
-          subscription: false
+        allow_any_instance_of(PaymentsController).to receive(:current_application_settings).and_return(
+          double(subscription_cost: 25.0, payments_directions: 'Payment instructions', allow_payments: true)
         )
-        allow_any_instance_of(PaymentsController).to receive(:current_application).and_return(mock_application)
+      end
 
-        allow(Payment).to receive_message_chain(:current_conference_payments, :where, :pluck).and_return([1000, 2000])
-        allow_any_instance_of(PaymentsController).to receive(:current_application_settings).and_return(double(subscription_cost: 25.0))
+      it "renders helper text explaining partial payments" do
+        get all_payments_path
+        expect(response.body).to include("The amount is prefilled with your full balance due, but you can enter a different amount to make a partial payment.")
       end
     end
   end
@@ -58,13 +64,71 @@
     context "when user is signed in" do
       before do
         sign_in user
+        allow_any_instance_of(PaymentsController).to receive(:current_application) do |controller|
+          controller.instance_variable_set(:@current_application, application)
+        end
+        create(:lodging, description: application.lodging_selection, cost: 300.0)
+        application.update!(partner_registration: create(:partner_registration, cost: 0.0), subscription: false)
+        allow_any_instance_of(PaymentsController).to receive(:current_application_settings).and_return(
+          double(subscription_cost: 25.0)
+        )
+        allow_any_instance_of(PaymentsController).to receive(:current_balance_due).and_return(300.0)
         allow_any_instance_of(PaymentsController).to receive(:generate_hash).and_return("https://payment-url.example.com")
       end
 
       it "redirects to the payment URL" do
         post make_payment_path, params: { amount: "100" }
         expect(response).to redirect_to("https://payment-url.example.com")
       end
+
+      it "supports paying the full balance amount" do
+        expect_any_instance_of(PaymentsController)
+          .to receive(:generate_hash).with(user, 200).and_return("https://payment-url.example.com")
+
+        post make_payment_path, params: { amount: "200" }
+        expect(response).to redirect_to("https://payment-url.example.com")
+      end
+
+      it "supports paying a partial balance amount" do
+        expect_any_instance_of(PaymentsController)
+          .to receive(:generate_hash).with(user, 75).and_return("https://payment-url.example.com")
+
+        post make_payment_path, params: { amount: "75" }
+        expect(response).to redirect_to("https://payment-url.example.com")
+      end
+
+      it "rejects missing amount input" do
+        post make_payment_path, params: { amount: "" }
+        expect(response).to redirect_to(all_payments_path)
+        expect(flash[:alert]).to eq("Please enter a valid payment amount.")
+      end
+
+      it "rejects non-numeric amount input" do
+        post make_payment_path, params: { amount: "abc" }
+        expect(response).to redirect_to(all_payments_path)
+        expect(flash[:alert]).to eq("Please enter a valid payment amount.")
+      end
+
+      it "rejects non-positive amount input" do
+        post make_payment_path, params: { amount: "0" }
+        expect(response).to redirect_to(all_payments_path)
+        expect(flash[:alert]).to eq("Please enter a valid payment amount.")
+      end
+
+      it "rejects amount above balance due" do
+        post make_payment_path, params: { amount: "400" }
+        expect(response).to redirect_to(all_payments_path)
+        expect(flash[:alert]).to eq("Please enter a valid payment amount.")
+      end
+
+      it "rejects amount above MAX_PAYMENT_AMOUNT even when balance due is higher" do
+        allow_any_instance_of(PaymentsController).to receive(:current_balance_due).and_return(3000.0)
+
+        post make_payment_path, params: { amount: "2001" }
+
+        expect(response).to redirect_to(all_payments_path)
+        expect(flash[:alert]).to eq("Please enter a valid payment amount.")
+      end
     end
   end