Update properties controller to use new creational and update balance methods

test/controllers/properties_controller_test.rb

@@ -8,18 +8,15 @@ class PropertiesControllerTest < ActionDispatch::IntegrationTest
     @account = accounts(:property)
   end
 
-  test "creates property in draft status and redirects to balances step" do
+  test "creates property in draft status with initial balance information and redirects to details step" do
     assert_difference -> { Account.count } => 1 do
       post properties_path, params: {
         account: {
           name: "New Property",
-          subtype: "house",
-          accountable_type: "Property",
-          accountable_attributes: {
-            year_built: 1990,
-            area_value: 1200,
-            area_unit: "sqft"
-          }
+          purchase_price: "250000",
+          purchase_date: "2023-01-01",
+          current_estimated_value: "300000",
+          currency: "USD"
         }
       }
     end
@@ -27,38 +24,47 @@ class PropertiesControllerTest < ActionDispatch::IntegrationTest
     created_account = Account.order(:created_at).last
     assert created_account.accountable.is_a?(Property)
     assert_equal "draft", created_account.status
-    assert_equal 0, created_account.balance
-    assert_equal 1990, created_account.accountable.year_built
-    assert_equal 1200, created_account.accountable.area_value
-    assert_equal "sqft", created_account.accountable.area_unit
-    assert_redirected_to balances_property_path(created_account)
+    assert_equal "New Property", created_account.name
+    assert_equal 300_000, created_account.balance
+    assert_equal 0, created_account.cash_balance
+    assert_equal "USD", created_account.currency
+
+    # Check opening balance was set
+    opening_valuation = created_account.valuations.opening_anchor.first
+    assert_not_nil opening_valuation
+    assert_equal 250_000, opening_valuation.balance
+    assert_equal Date.parse("2023-01-01"), opening_valuation.entry.date
+
+    assert_redirected_to details_property_path(created_account)
   end
 
-  test "updates property overview" do
+  test "updates property overview with balance information" do
     assert_no_difference [ "Account.count", "Property.count" ] do
       patch property_path(@account), params: {
         account: {
           name: "Updated Property",
-          subtype: "condo"
+          current_estimated_value: "350000",
+          currency: "USD"
         }
       }
     end
 
     @account.reload
     assert_equal "Updated Property", @account.name
-    assert_equal "condo", @account.subtype
+    assert_equal 350_000, @account.balance
+    assert_equal 0, @account.cash_balance
 
-    # If account is active, it renders edit view; otherwise redirects to balances
+    # If account is active, it renders edit view; otherwise redirects to details
     if @account.active?
       assert_response :success
     else
-      assert_redirected_to balances_property_path(@account)
+      assert_redirected_to details_property_path(@account)
     end
   end
 
   # Tab view tests
-  test "shows balances tab" do
-    get balances_property_path(@account)
+  test "shows details tab" do
+    get details_property_path(@account)
     assert_response :success
   end
 
@@ -68,22 +74,26 @@ class PropertiesControllerTest < ActionDispatch::IntegrationTest
   end
 
   # Tab update tests
-  test "updates balances tab" do
-    original_balance = @account.balance
-
-    # Mock the update_balance method to return a successful result
-    Account::BalanceUpdater::Result.any_instance.stubs(:success?).returns(true)
-    Account::BalanceUpdater::Result.any_instance.stubs(:updated?).returns(true)
-
-    patch update_balances_property_path(@account), params: {
+  test "updates property details" do
+    patch update_details_property_path(@account), params: {
       account: {
-        balance: 600000,
-        currency: "EUR"
+        subtype: "condo",
+        accountable_attributes: {
+          year_built: 2005,
+          area_value: 1500,
+          area_unit: "sqft"
+        }
       }
     }
 
-    # If account is active, it renders balances view; otherwise redirects to address
-    if @account.reload.active?
+    @account.reload
+    assert_equal "condo", @account.subtype
+    assert_equal 2005, @account.accountable.year_built
+    assert_equal 1500, @account.accountable.area_value
+    assert_equal "sqft", @account.accountable.area_unit
+
+    # If account is active, it renders details view; otherwise redirects to address
+    if @account.active?
       assert_response :success
     else
       assert_redirected_to address_property_path(@account)
@@ -115,20 +125,6 @@ class PropertiesControllerTest < ActionDispatch::IntegrationTest
     end
   end
 
-  test "balances update handles validation errors" do
-    # Mock update_balance to return a failure result
-    Account::BalanceUpdater::Result.any_instance.stubs(:success?).returns(false)
-    Account::BalanceUpdater::Result.any_instance.stubs(:error_message).returns("Invalid balance")
-
-    patch update_balances_property_path(@account), params: {
-      account: {
-        balance: 600000,
-        currency: "EUR"
-      }
-    }
-
-    assert_response :unprocessable_entity
-  end
 
   test "address update handles validation errors" do
     Property.any_instance.stubs(:update).returns(false)

test/models/account/overview_form_test.rb

@@ -0,0 +1,139 @@
+require "test_helper"
+
+class Account::OverviewFormTest < ActiveSupport::TestCase
+  setup do
+    @account = accounts(:property)
+  end
+
+  test "initializes with account and attributes" do
+    form = Account::OverviewForm.new(
+      account: @account,
+      name: "Updated Property"
+    )
+
+    assert_equal @account, form.account
+    assert_equal "Updated Property", form.name
+  end
+
+  test "save returns result with success and updated status" do
+    form = Account::OverviewForm.new(account: @account)
+    result = form.save
+
+    assert result.success?
+    assert_not result.updated?
+  end
+
+  test "updates account name when provided" do
+    form = Account::OverviewForm.new(
+      account: @account,
+      name: "New Property Name"
+    )
+
+    @account.expects(:update!).with(name: "New Property Name").once
+    @account.expects(:sync_later).never  # Name change should not trigger sync
+
+    result = form.save
+
+    assert result.success?
+    assert result.updated?
+  end
+
+  test "updates currency and triggers sync" do
+    form = Account::OverviewForm.new(
+      account: @account,
+      currency: "EUR"
+    )
+
+    @account.expects(:update_currency!).with("EUR").once
+    @account.expects(:sync_later).once  # Currency change should trigger sync
+
+    result = form.save
+
+    assert result.success?
+    assert result.updated?
+  end
+
+  test "calls sync_later only once for multiple balance-related changes" do
+    form = Account::OverviewForm.new(
+      account: @account,
+      currency: "EUR",
+      opening_balance: 100_000,
+      opening_cash_balance: 0,
+      current_balance: 150_000,
+      current_cash_balance: 0
+    )
+
+    @account.expects(:update_currency!).with("EUR").once
+    @account.expects(:set_or_update_opening_balance!).once
+    @account.expects(:update_current_balance!).once
+    @account.expects(:sync_later).once  # Should only be called once despite multiple changes
+
+    result = form.save
+
+    assert result.success?
+    assert result.updated?
+  end
+
+  test "does not call sync_later when transaction fails" do
+    form = Account::OverviewForm.new(
+      account: @account,
+      name: "New Name",
+      opening_balance: 100_000,
+      opening_cash_balance: 0
+    )
+
+    # Simulate a validation error on opening balance update
+    @account.expects(:update!).with(name: "New Name").once
+    @account.expects(:set_or_update_opening_balance!).raises(Account::Reconcileable::InvalidBalanceError.new("Cash balance cannot exceed balance"))
+    @account.expects(:sync_later).never  # Should NOT sync if any update fails
+
+    result = form.save
+
+    assert_not result.success?
+    assert_not result.updated?
+    assert_equal "Cash balance cannot exceed balance", result.error
+  end
+
+  test "raises ArgumentError when balance fields are not properly paired" do
+    # Opening balance without cash balance
+    form = Account::OverviewForm.new(
+      account: @account,
+      opening_balance: 100_000
+    )
+
+    # Debug what values we have
+    assert_equal 100_000.to_d, form.opening_balance
+    assert_nil form.opening_cash_balance
+
+    error = assert_raises(ArgumentError) do
+      form.save
+    end
+    assert_equal "Both opening_balance and opening_cash_balance must be provided together", error.message
+
+    # Current cash balance without balance
+    form = Account::OverviewForm.new(
+      account: @account,
+      current_cash_balance: 0
+    )
+
+    error = assert_raises(ArgumentError) do
+      form.save
+    end
+    assert_equal "Both current_balance and current_cash_balance must be provided together", error.message
+  end
+
+  test "converts string balance values to decimals" do
+    form = Account::OverviewForm.new(
+      account: @account,
+      opening_balance: "100000.50",
+      opening_cash_balance: "0",
+      current_balance: "150000.75",
+      current_cash_balance: "5000.25"
+    )
+
+    assert_equal 100000.50.to_d, form.opening_balance
+    assert_equal 0.to_d, form.opening_cash_balance
+    assert_equal 150000.75.to_d, form.current_balance
+    assert_equal 5000.25.to_d, form.current_cash_balance
+  end
+end

test/models/account/reconcileable_test.rb

@@ -64,7 +64,7 @@ class Account::ReconcileableTest < ActiveSupport::TestCase
 
     # No new valuation is appended; we're just adjusting the opening valuation anchor
     assert_no_difference "account.entries.count" do
-      account.update_current_balance(balance: 1000, cash_balance: 1000)
+      account.update_current_balance!(balance: 1000, cash_balance: 1000)
     end
 
     opening_valuation = account.valuations.first
@@ -113,7 +113,7 @@ class Account::ReconcileableTest < ActiveSupport::TestCase
     assert_equal 2, account.valuations.count
 
     # Here, we assume user is once again "overriding" the balance to 1400
-    account.update_current_balance(balance: 1400, cash_balance: 1400)
+    account.update_current_balance!(balance: 1400, cash_balance: 1400)
 
     most_recent_valuation = account.valuations.joins(:entry).order("entries.date DESC").first