diff --git a/app/controllers/properties_controller.rb b/app/controllers/properties_controller.rb index 8b2ec062..8ccdf718 100644 --- a/app/controllers/properties_controller.rb +++ b/app/controllers/properties_controller.rb @@ -1,31 +1,49 @@ class PropertiesController < ApplicationController include AccountableResource, StreamExtensions - before_action :set_property, only: [ :balances, :address, :update_balances, :update_address ] + before_action :set_property, only: [ :edit, :update, :details, :update_details, :address, :update_address ] def new @account = Current.family.accounts.build(accountable: Property.new) end def create - @account = Current.family.accounts.create!( - property_params.merge(currency: Current.family.currency, balance: 0, status: "draft") + @account = Current.family.create_property_account!( + name: property_params[:name], + current_value: property_params[:current_estimated_value].to_d, + purchase_price: property_params[:purchase_price].present? ? property_params[:purchase_price].to_d : nil, + purchase_date: property_params[:purchase_date], + currency: property_params[:currency] || Current.family.currency, + draft: true ) - redirect_to balances_property_path(@account) + redirect_to details_property_path(@account) end def update - if @account.update(property_params) + form = Account::OverviewForm.new( + account: @account, + name: property_params[:name], + currency: property_params[:currency], + opening_balance: property_params[:purchase_price], + opening_cash_balance: property_params[:purchase_price].present? ? "0" : nil, + opening_date: property_params[:purchase_date], + current_balance: property_params[:current_estimated_value], + current_cash_balance: property_params[:current_estimated_value].present? ? "0" : nil + ) + + result = form.save + + if result.success? @success_message = "Property details updated successfully." if @account.active? render :edit else - redirect_to balances_property_path(@account) + redirect_to details_property_path(@account) end else - @error_message = "Unable to update property details." + @error_message = result.error || "Unable to update property details." render :edit, status: :unprocessable_entity end end @@ -33,26 +51,25 @@ class PropertiesController < ApplicationController def edit end - def balances + def details end - def update_balances - result = @account.update_balance(balance: balance_params[:balance], currency: balance_params[:currency]) - - if result.success? - @success_message = result.updated? ? "Balance updated successfully." : "No changes made. Account is already up to date." + def update_details + if @account.update(details_params) + @success_message = "Property details updated successfully." if @account.active? - render :balances + render :details else redirect_to address_property_path(@account) end else - @error_message = result.error_message - render :balances, status: :unprocessable_entity + @error_message = "Unable to update property details." + render :details, status: :unprocessable_entity end end + def address @property = @account.property @property.address ||= Address.new @@ -78,8 +95,9 @@ class PropertiesController < ApplicationController end private - def balance_params - params.require(:account).permit(:balance, :currency) + def details_params + params.require(:account) + .permit(:subtype, accountable_attributes: [ :id, :year_built, :area_unit, :area_value ]) end def address_params @@ -89,7 +107,9 @@ class PropertiesController < ApplicationController def property_params params.require(:account) - .permit(:name, :subtype, :accountable_type, accountable_attributes: [ :id, :year_built, :area_unit, :area_value ]) + .permit(:name, :currency, :purchase_price, :purchase_date, :current_estimated_value, + :subtype, :accountable_type, + accountable_attributes: [ :id, :year_built, :area_unit, :area_value ]) end def set_property diff --git a/app/javascript/controllers/money_field_controller.js b/app/javascript/controllers/money_field_controller.js index 2aab2d16..f41ae874 100644 --- a/app/javascript/controllers/money_field_controller.js +++ b/app/javascript/controllers/money_field_controller.js @@ -5,10 +5,15 @@ import { CurrenciesService } from "services/currencies_service"; // when currency select change, update the input value with the correct placeholder and step export default class extends Controller { static targets = ["amount", "currency", "symbol"]; + static values = { syncCurrency: Boolean }; handleCurrencyChange(e) { const selectedCurrency = e.target.value; this.updateAmount(selectedCurrency); + + if (this.syncCurrencyValue) { + this.syncOtherMoneyFields(selectedCurrency); + } } updateAmount(currency) { @@ -24,4 +29,28 @@ export default class extends Controller { this.symbolTarget.innerText = currency.symbol; }); } + + syncOtherMoneyFields(selectedCurrency) { + // Find the form this money field belongs to + const form = this.element.closest("form"); + if (!form) return; + + // Find all other money field controllers in the same form + const allMoneyFields = form.querySelectorAll('[data-controller~="money-field"]'); + + allMoneyFields.forEach(field => { + // Skip the current field + if (field === this.element) return; + + // Get the controller instance + const controller = this.application.getControllerForElementAndIdentifier(field, "money-field"); + if (!controller) return; + + // Update the currency select if it exists + if (controller.hasCurrencyTarget) { + controller.currencyTarget.value = selectedCurrency; + controller.updateAmount(selectedCurrency); + } + }); + } } diff --git a/app/models/account/overview_form.rb b/app/models/account/overview_form.rb new file mode 100644 index 00000000..089711c8 --- /dev/null +++ b/app/models/account/overview_form.rb @@ -0,0 +1,87 @@ +class Account::OverviewForm + include ActiveModel::Model + + attr_accessor :account, :name, :currency, :opening_date + attr_reader :opening_balance, :opening_cash_balance, :current_balance, :current_cash_balance + + Result = Struct.new(:success?, :updated?, :error, keyword_init: true) + CurrencyUpdateError = Class.new(StandardError) + + def opening_balance=(value) + @opening_balance = value.nil? ? nil : value.to_d + end + + def opening_cash_balance=(value) + @opening_cash_balance = value.nil? ? nil : value.to_d + end + + def current_balance=(value) + @current_balance = value.nil? ? nil : value.to_d + end + + def current_cash_balance=(value) + @current_cash_balance = value.nil? ? nil : value.to_d + end + + def save + # Validate that balance fields are properly paired + if (!opening_balance.nil? && opening_cash_balance.nil?) || + (opening_balance.nil? && !opening_cash_balance.nil?) + raise ArgumentError, "Both opening_balance and opening_cash_balance must be provided together" + end + + if (!current_balance.nil? && current_cash_balance.nil?) || + (current_balance.nil? && !current_cash_balance.nil?) + raise ArgumentError, "Both current_balance and current_cash_balance must be provided together" + end + + updated = false + sync_required = false + + Account.transaction do + # Update name if provided + if name.present? && name != account.name + account.update!(name: name) + updated = true + end + + # Update currency if provided + if currency.present? && currency != account.currency + account.update_currency!(currency) + updated = true + sync_required = true + end + + # Update opening balance if provided (already validated that both are present) + if !opening_balance.nil? + account.set_or_update_opening_balance!( + balance: opening_balance, + cash_balance: opening_cash_balance, + date: opening_date # optional + ) + updated = true + sync_required = true + end + + # Update current balance if provided (already validated that both are present) + if !current_balance.nil? + account.update_current_balance!( + balance: current_balance, + cash_balance: current_cash_balance + ) + updated = true + sync_required = true + end + end + + # Only sync if transaction succeeded and sync is required + account.sync_later if sync_required + + Result.new(success?: true, updated?: updated) + rescue ArgumentError => e + # Re-raise ArgumentError as it's a developer error + raise e + rescue => e + Result.new(success?: false, updated?: false, error: e.message) + end +end diff --git a/app/models/account/reconcileable.rb b/app/models/account/reconcileable.rb index 6b6f1adc..dc6df605 100644 --- a/app/models/account/reconcileable.rb +++ b/app/models/account/reconcileable.rb @@ -17,39 +17,63 @@ module Account::Reconcileable balance - cash_balance end + def opening_balance + @opening_balance ||= opening_anchor_valuation&.balance + end + + def opening_cash_balance + @opening_cash_balance ||= opening_anchor_valuation&.cash_balance + end + + def opening_date + @opening_date ||= opening_anchor_valuation&.entry&.date + end + def reconcile_balance!(balance:, cash_balance:, date:) raise InvalidBalanceError, "Cash balance cannot exceed balance" if cash_balance > balance raise InvalidBalanceError, "Linked accounts cannot be reconciled" if linked? - existing_valuation = valuations.joins(:entry).where(kind: "recon", entry: { date: Date.current }).first + existing_valuation = valuations.joins(:entry).where(kind: "recon", entry: { date: date }).first - if existing_valuation.present? - existing_valuation.update!( - balance: balance, - cash_balance: cash_balance - ) - else - entries.create!( - date: date, - name: Valuation::Name.new("recon", self.accountable_type), - amount: balance, - currency: self.currency, - entryable: Valuation.new( - kind: "recon", + transaction do + if existing_valuation.present? + existing_valuation.update!( balance: balance, cash_balance: cash_balance ) - ) + else + entries.create!( + date: date, + name: Valuation::Name.new("recon", self.accountable_type), + amount: balance, + currency: self.currency, + entryable: Valuation.new( + kind: "recon", + balance: balance, + cash_balance: cash_balance + ) + ) + end + + # Update cached balance fields on account when reconciling for current date + if date == Date.current + update!(balance: balance, cash_balance: cash_balance) + end end end - def update_current_balance(balance:, cash_balance:) + def update_current_balance!(balance:, cash_balance:) raise InvalidBalanceError, "Cash balance cannot exceed balance" if cash_balance > balance - if opening_anchor_valuation.present? && valuations.where(kind: "recon").empty? - adjust_opening_balance_with_delta(balance:, cash_balance:) - else - reconcile_balance!(balance:, cash_balance:, date: Date.current) + transaction do + if opening_anchor_valuation.present? && valuations.where(kind: "recon").empty? + adjust_opening_balance_with_delta(balance:, cash_balance:) + else + reconcile_balance!(balance:, cash_balance:, date: Date.current) + end + + # Always update cached balance fields when updating current balance + update!(balance: balance, cash_balance: cash_balance) end end @@ -100,7 +124,7 @@ module Account::Reconcileable private def opening_anchor_valuation - valuations.opening_anchor.first + @opening_anchor_valuation ||= valuations.opening_anchor.includes(:entry).first end def current_anchor_valuation diff --git a/app/models/family/account_creatable.rb b/app/models/family/account_creatable.rb index 8ab6985c..9aa575d9 100644 --- a/app/models/family/account_creatable.rb +++ b/app/models/family/account_creatable.rb @@ -1,7 +1,7 @@ module Family::AccountCreatable extend ActiveSupport::Concern - def create_property_account!(name:, current_value:, purchase_price: nil, purchase_date: nil, currency: nil) + def create_property_account!(name:, current_value:, purchase_price: nil, purchase_date: nil, currency: nil, draft: false) create_manual_account!( name: name, balance: current_value, @@ -9,11 +9,12 @@ module Family::AccountCreatable accountable_type: Property, opening_balance: purchase_price, opening_date: purchase_date, - currency: currency + currency: currency, + draft: draft ) end - def create_vehicle_account!(name:, current_value:, purchase_price: nil, purchase_date: nil, currency: nil) + def create_vehicle_account!(name:, current_value:, purchase_price: nil, purchase_date: nil, currency: nil, draft: false) create_manual_account!( name: name, balance: current_value, @@ -21,23 +22,25 @@ module Family::AccountCreatable accountable_type: Vehicle, opening_balance: purchase_price, opening_date: purchase_date, - currency: currency + currency: currency, + draft: draft ) end - def create_depository_account!(name:, current_balance:, opening_date: nil, currency: nil) + def create_depository_account!(name:, current_balance:, opening_date: nil, currency: nil, draft: false) create_manual_account!( name: name, balance: current_balance, cash_balance: current_balance, accountable_type: Depository, opening_date: opening_date, - currency: currency + currency: currency, + draft: draft ) end # Investment account values are built up by adding holdings / trades, not by initializing a "balance" - def create_investment_account!(name:, currency: nil) + def create_investment_account!(name:, currency: nil, draft: false) create_manual_account!( name: name, balance: 0, @@ -45,11 +48,12 @@ module Family::AccountCreatable accountable_type: Investment, opening_balance: 0, # Investment accounts start empty opening_cash_balance: 0, - currency: currency + currency: currency, + draft: draft ) end - def create_other_asset_account!(name:, current_value:, purchase_price: nil, purchase_date: nil, currency: nil) + def create_other_asset_account!(name:, current_value:, purchase_price: nil, purchase_date: nil, currency: nil, draft: false) create_manual_account!( name: name, balance: current_value, @@ -57,11 +61,12 @@ module Family::AccountCreatable accountable_type: OtherAsset, opening_balance: purchase_price, opening_date: purchase_date, - currency: currency + currency: currency, + draft: draft ) end - def create_other_liability_account!(name:, current_debt:, original_debt: nil, origination_date: nil, currency: nil) + def create_other_liability_account!(name:, current_debt:, original_debt: nil, origination_date: nil, currency: nil, draft: false) create_manual_account!( name: name, balance: current_debt, @@ -69,12 +74,13 @@ module Family::AccountCreatable accountable_type: OtherLiability, opening_balance: original_debt, opening_date: origination_date, - currency: currency + currency: currency, + draft: draft ) end # For now, crypto accounts are very simple; we just track overall value - def create_crypto_account!(name:, current_value:, currency: nil) + def create_crypto_account!(name:, current_value:, currency: nil, draft: false) create_manual_account!( name: name, balance: current_value, @@ -82,11 +88,12 @@ module Family::AccountCreatable accountable_type: Crypto, opening_balance: current_value, opening_cash_balance: current_value, - currency: currency + currency: currency, + draft: draft ) end - def create_credit_card_account!(name:, current_debt:, opening_date: nil, currency: nil) + def create_credit_card_account!(name:, current_debt:, opening_date: nil, currency: nil, draft: false) create_manual_account!( name: name, balance: current_debt, @@ -94,11 +101,12 @@ module Family::AccountCreatable accountable_type: CreditCard, opening_balance: 0, # Credit cards typically start with no debt opening_date: opening_date, - currency: currency + currency: currency, + draft: draft ) end - def create_loan_account!(name:, current_principal:, original_principal: nil, origination_date: nil, currency: nil) + def create_loan_account!(name:, current_principal:, original_principal: nil, origination_date: nil, currency: nil, draft: false) create_manual_account!( name: name, balance: current_principal, @@ -106,7 +114,8 @@ module Family::AccountCreatable accountable_type: Loan, opening_balance: original_principal, opening_date: origination_date, - currency: currency + currency: currency, + draft: draft ) end @@ -128,14 +137,15 @@ module Family::AccountCreatable private - def create_manual_account!(name:, balance:, cash_balance:, accountable_type:, opening_balance: nil, opening_cash_balance: nil, opening_date: nil, currency: nil) + def create_manual_account!(name:, balance:, cash_balance:, accountable_type:, opening_balance: nil, opening_cash_balance: nil, opening_date: nil, currency: nil, draft: false) Family.transaction do account = accounts.create!( name: name, balance: balance, cash_balance: cash_balance, currency: currency.presence || self.currency, - accountable: accountable_type.new + accountable: accountable_type.new, + status: draft ? "draft" : "active" ) account.set_or_update_opening_balance!( diff --git a/app/models/property.rb b/app/models/property.rb index 6114a9f4..4868e42d 100644 --- a/app/models/property.rb +++ b/app/models/property.rb @@ -52,6 +52,7 @@ class Property < ApplicationRecord private def first_valuation_amount + return nil unless account account.entries.valuations.order(:date).first&.amount_money || account.balance_money end end diff --git a/app/views/properties/_form_tabs.html.erb b/app/views/properties/_form_tabs.html.erb index 0693c7c9..9e8b2e6b 100644 --- a/app/views/properties/_form_tabs.html.erb +++ b/app/views/properties/_form_tabs.html.erb @@ -2,6 +2,6 @@
<%= render "properties/form_tab", label: "Overview", href: account.new_record? ? nil : edit_property_path(@account), active: active_tab == "overview" %> - <%= render "properties/form_tab", label: "Value", href: account.new_record? ? nil : balances_property_path(@account), active: active_tab == "value" %> + <%= render "properties/form_tab", label: "Details", href: account.new_record? ? nil : details_property_path(@account), active: active_tab == "details" %> <%= render "properties/form_tab", label: "Address", href: account.new_record? ? nil : address_property_path(@account), active: active_tab == "address" %>
diff --git a/app/views/properties/_property_overview_fields.html.erb b/app/views/properties/_property_overview_fields.html.erb new file mode 100644 index 00000000..401f618b --- /dev/null +++ b/app/views/properties/_property_overview_fields.html.erb @@ -0,0 +1,30 @@ +<%# locals: (form:, account:) %> + +
+ <%= form.text_field :name, + label: "Name", + placeholder: "Vacation home", + required: true %> + + <%= form.money_field :current_estimated_value, + label: "Current estimated value", + label_tooltip: "The estimated market value of your property. This number can often be found on sites like Zillow or Redfin, and is never an exact number.", + placeholder: Money.new(0, form.object.currency || Current.family.currency), + value: account.balance, + required: true, + sync_currency: true %> + + <%= form.money_field :purchase_price, + label: "Purchase price", + label_tooltip: "The amount you paid when you purchased the property. Leave blank if unknown.", + placeholder: Money.new(0, form.object.currency || Current.family.currency), + value: account.opening_balance, + sync_currency: true %> + + <%= form.date_field :purchase_date, + label: "Purchase date", + label_tooltip: "The date you purchased the property. This helps track your property's value over time.", + value: account.opening_date %> + + <%= form.hidden_field :current_cash_balance, value: 0 %> +
diff --git a/app/views/properties/balances.html.erb b/app/views/properties/balances.html.erb deleted file mode 100644 index c9141771..00000000 --- a/app/views/properties/balances.html.erb +++ /dev/null @@ -1,30 +0,0 @@ -<%= render DialogComponent.new do |dialog| %> - <% dialog.with_header(title: "Enter property manually") %> - <% dialog.with_body do %> -
- <%= render "properties/form_tabs", account: @account, active_tab: "value" %> - - -
- <%= styled_form_with model: @account, url: update_balances_property_path(@account), method: :patch do |form| %> -
- <%= render "properties/form_alert", notice: @success_message, error: @error_message %> - - <%= form.money_field :balance, - label: "Estimated market value", - label_tooltip: "The estimated market value of your property. This number can often be found on sites like Zillow or Redfin, and is never an exact number.", - placeholder: "0" %> -
- - -
- <%= render ButtonComponent.new( - text: @account.active? ? "Save" : "Next", - variant: "primary", - ) %> -
- <% end %> -
-
- <% end %> -<% end %> diff --git a/app/views/properties/details.html.erb b/app/views/properties/details.html.erb new file mode 100644 index 00000000..65a6a009 --- /dev/null +++ b/app/views/properties/details.html.erb @@ -0,0 +1,50 @@ +<%= render DialogComponent.new do |dialog| %> + <% dialog.with_header(title: "Enter property manually") %> + <% dialog.with_body do %> +
+ + <%= render "properties/form_tabs", account: @account, active_tab: "details" %> + + +
+ <%= styled_form_with model: @account, url: update_details_property_path(@account), method: :patch do |form| %> +
+ <%= render "properties/form_alert", notice: @success_message, error: @error_message %> + + <%= form.select :subtype, + options_for_select(Property::SUBTYPES.map { |k, v| [v[:long], k] }, @account.subtype), + { label: "Property type" }, + { class: "form-field__input" } %> + + <%= form.fields_for :accountable do |property_form| %> +
+ <%= property_form.number_field :area_value, + label: "Area", + placeholder: "1200", + min: 0 %> + <%= property_form.select :area_unit, + [["Square Feet", "sqft"], ["Square Meters", "sqm"]], + { label: "Area unit", selected: @account.accountable.area_unit } %> +
+ + <%= property_form.number_field :year_built, + label: "Year built", + placeholder: "2000", + step: 1, + min: 1800, + max: Date.current.year %> + <% end %> +
+ + +
+ <%= render ButtonComponent.new( + text: @account.active? ? "Save" : "Next", + variant: "primary", + ) %> +
+ <% end %> +
+
+ <% end %> +<% end %> diff --git a/app/views/properties/edit.html.erb b/app/views/properties/edit.html.erb index fd75d929..7e5de7ac 100644 --- a/app/views/properties/edit.html.erb +++ b/app/views/properties/edit.html.erb @@ -10,7 +10,7 @@ <%= styled_form_with model: @account, url: property_path(@account), method: :patch do |form| %>
<%= render "properties/form_alert", notice: @success_message, error: @error_message %> - <%= render "properties/overview_fields", form: form %> + <%= render "properties/property_overview_fields", form: form, account: @account %>
diff --git a/app/views/properties/new.html.erb b/app/views/properties/new.html.erb index 73dae294..ee1e08c9 100644 --- a/app/views/properties/new.html.erb +++ b/app/views/properties/new.html.erb @@ -10,7 +10,7 @@ <%= styled_form_with model: @account, url: properties_path do |form| %>
<%= render "properties/form_alert", notice: @success_message, error: @error_message %> - <%= render "properties/overview_fields", form: form %> + <%= render "properties/property_overview_fields", form: form, account: @account %>
diff --git a/app/views/shared/_money_field.html.erb b/app/views/shared/_money_field.html.erb index 05929401..59449936 100644 --- a/app/views/shared/_money_field.html.erb +++ b/app/views/shared/_money_field.html.erb @@ -7,7 +7,7 @@ end currency = Money::Currency.new(currency_value || options[:default_currency] || "USD") %> -
+
> <% if options[:label_tooltip] %>
<%= form.label options[:label] || t(".label"), class: "form-field__label" do %> diff --git a/config/routes.rb b/config/routes.rb index f0414287..09ecc211 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -170,8 +170,8 @@ Rails.application.routes.draw do resources :investments, except: :index resources :properties, except: :index do member do - get :balances - patch :update_balances + get :details + patch :update_details get :address patch :update_address diff --git a/test/controllers/properties_controller_test.rb b/test/controllers/properties_controller_test.rb index b5f1305f..9295a0bd 100644 --- a/test/controllers/properties_controller_test.rb +++ b/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) diff --git a/test/models/account/overview_form_test.rb b/test/models/account/overview_form_test.rb new file mode 100644 index 00000000..351cf682 --- /dev/null +++ b/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 diff --git a/test/models/account/reconcileable_test.rb b/test/models/account/reconcileable_test.rb index 334e907d..39fa7ade 100644 --- a/test/models/account/reconcileable_test.rb +++ b/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