From f9bdd0c79d1cc93716cfc44c6d5e18e825fdc5da Mon Sep 17 00:00:00 2001
From: jonkaplan <jonwiththewind@gmail.com>
Date: Fri, 31 May 2024 16:23:24 -0400
Subject: [PATCH 1/2] More resilient add_funds

- Use rails helper
- Add tests/proper logic for error rendering and for empty add_fund_amount
---
 app/controllers/admin/students_controller.rb  |  2 +-
 app/views/admin/students/_form.html.erb       |  4 ++--
 .../admin/students_controller_test.rb         | 22 +++++++++++++++++++
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/app/controllers/admin/students_controller.rb b/app/controllers/admin/students_controller.rb
index 42c3d28..fc3988e 100644
--- a/app/controllers/admin/students_controller.rb
+++ b/app/controllers/admin/students_controller.rb
@@ -47,7 +47,7 @@ def update
     private
 
     def create_portfolio_transaction!
-      return unless fund_amount
+      return unless fund_amount.present?
 
       PortfolioTransaction.create!(portfolio: requested_resource.portfolio, amount: fund_amount) if fund_amount
     end
diff --git a/app/views/admin/students/_form.html.erb b/app/views/admin/students/_form.html.erb
index 25fb1d1..66204cd 100644
--- a/app/views/admin/students/_form.html.erb
+++ b/app/views/admin/students/_form.html.erb
@@ -52,11 +52,11 @@ and renders all form fields for a resource's editable attributes.
         </div>
       <% end %>
     </fieldset>
-    <% if action_name == 'edit' %>
+    <% if action_name == 'edit' || action_name == 'update' %>
       <div class="field-unit">
         <div class="field-unit__label"><label>Cash balance</label></div>
         <div class="field-unit__field">
-          <%= sprintf('%.2f', page.resource&.portfolio&.cash_balance || 0)%>
+          <%= number_to_currency(page.resource&.portfolio&.cash_balance || 0)%>
         </div>
       </div>
       <div class="field-unit">
diff --git a/test/controllers/admin/students_controller_test.rb b/test/controllers/admin/students_controller_test.rb
index 759283f..d711195 100644
--- a/test/controllers/admin/students_controller_test.rb
+++ b/test/controllers/admin/students_controller_test.rb
@@ -14,6 +14,18 @@ class Admin::StudentsControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to admin_student_url(@student)
   end
 
+  test 'should not update with an error' do
+    @student.update_attribute(:username, 'testingusername')
+
+    patch admin_student_url(@student), params: { student: { username: '' } }
+
+    @student.reload
+
+    assert_equal 'testingusername', @student.username
+    assert_response :unprocessable_entity
+    assert_select 'div#error_explanation'
+  end
+
   test 'given a add_fund_amount, creates a transaction' do
     assert_difference('PortfolioTransaction.count', 1) do
       patch admin_student_url(@student), params: { student: { add_fund_amount: '10.50' } }
@@ -26,4 +38,14 @@ class Admin::StudentsControllerTest < ActionDispatch::IntegrationTest
 
     assert_redirected_to admin_student_url(@student)
   end
+
+  test 'given an empty add_fund_amount, does not create a transaction' do
+    assert_difference('PortfolioTransaction.count', 0) do
+      patch admin_student_url(@student), params: { student: { add_fund_amount: '' } }
+    end
+
+    transaction = PortfolioTransaction.last
+
+    assert_equal nil, transaction.amount
+  end
 end

From cb649eacf930fd0a0e657d0032422dc7f545adcb Mon Sep 17 00:00:00 2001
From: jonkaplan <jonwiththewind@gmail.com>
Date: Sat, 1 Jun 2024 08:38:27 -0400
Subject: [PATCH 2/2] Make transaction explicitly a deposit

- Fix spec to be more explicit
---
 app/controllers/admin/students_controller.rb       | 6 +++++-
 test/controllers/admin/students_controller_test.rb | 5 +----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/app/controllers/admin/students_controller.rb b/app/controllers/admin/students_controller.rb
index fc3988e..b9fa355 100644
--- a/app/controllers/admin/students_controller.rb
+++ b/app/controllers/admin/students_controller.rb
@@ -49,7 +49,11 @@ def update
     def create_portfolio_transaction!
       return unless fund_amount.present?
 
-      PortfolioTransaction.create!(portfolio: requested_resource.portfolio, amount: fund_amount) if fund_amount
+      PortfolioTransaction.create!(
+        portfolio: requested_resource.portfolio,
+        amount: fund_amount,
+        transaction_type: 'deposit'
+      )
     end
 
     def fund_amount
diff --git a/test/controllers/admin/students_controller_test.rb b/test/controllers/admin/students_controller_test.rb
index d711195..9d85b4e 100644
--- a/test/controllers/admin/students_controller_test.rb
+++ b/test/controllers/admin/students_controller_test.rb
@@ -33,6 +33,7 @@ class Admin::StudentsControllerTest < ActionDispatch::IntegrationTest
 
     transaction = PortfolioTransaction.last
 
+    assert_equal 'deposit', transaction.transaction_type
     assert_equal 10.50, transaction.amount
     assert_equal @student.reload.portfolio.portfolio_transactions.last, transaction
 
@@ -43,9 +44,5 @@ class Admin::StudentsControllerTest < ActionDispatch::IntegrationTest
     assert_difference('PortfolioTransaction.count', 0) do
       patch admin_student_url(@student), params: { student: { add_fund_amount: '' } }
     end
-
-    transaction = PortfolioTransaction.last
-
-    assert_equal nil, transaction.amount
   end
 end