diff --git a/app/assets/tailwind/maybe-design-system.css b/app/assets/tailwind/maybe-design-system.css index f9dc6039..9bb3046d 100644 --- a/app/assets/tailwind/maybe-design-system.css +++ b/app/assets/tailwind/maybe-design-system.css @@ -334,6 +334,19 @@ } } + /* New form field structure components */ + .form-field__header { + @apply flex items-center justify-between gap-2; + } + + .form-field__body { + @apply flex flex-col gap-1; + } + + .form-field__actions { + @apply flex items-center gap-1; + } + .form-field__label { @apply block text-xs text-secondary peer-disabled:text-subdued; } @@ -347,10 +360,6 @@ @apply transition-opacity duration-300; @apply placeholder:text-subdued; - &select { - @apply pr-8; - } - @variant theme-dark { &::-webkit-calendar-picker-indicator { filter: invert(1); @@ -358,6 +367,14 @@ } } } + + select.form-field__input { + @apply pr-10 appearance-none; + background-image: url("data:image/svg+xml,%3csvg xmlns='http://www.w3.org/2000/svg' fill='none' viewBox='0 0 20 20'%3e%3cpath stroke='%236b7280' stroke-linecap='round' stroke-linejoin='round' stroke-width='1.5' d='M6 8l4 4 4-4'/%3e%3c/svg%3e"); + background-position: right -0.15rem center; + background-repeat: no-repeat; + background-size: 1.25rem 1.25rem; + } .form-field__radio { @apply text-primary; @@ -425,7 +442,5 @@ @variant theme-dark { fill: var(--color-white); } - } -} - +} \ No newline at end of file diff --git a/app/components/alert_component.html.erb b/app/components/alert_component.html.erb new file mode 100644 index 00000000..e56c9fc4 --- /dev/null +++ b/app/components/alert_component.html.erb @@ -0,0 +1,7 @@ +
+ <%= helpers.icon icon_name, size: "sm", color: icon_color, class: "shrink-0" %> + +
+ <%= message %> +
+
diff --git a/app/components/alert_component.rb b/app/components/alert_component.rb new file mode 100644 index 00000000..47c348f8 --- /dev/null +++ b/app/components/alert_component.rb @@ -0,0 +1,52 @@ +class AlertComponent < ViewComponent::Base + def initialize(message:, variant: :info) + @message = message + @variant = variant + end + + private + attr_reader :message, :variant + + def container_classes + base_classes = "flex items-start gap-3 p-4 rounded-lg border" + + variant_classes = case variant + when :info + "bg-blue-50 text-blue-700 border-blue-200 theme-dark:bg-blue-900/20 theme-dark:text-blue-400 theme-dark:border-blue-800" + when :success + "bg-green-50 text-green-700 border-green-200 theme-dark:bg-green-900/20 theme-dark:text-green-400 theme-dark:border-green-800" + when :warning + "bg-yellow-50 text-yellow-700 border-yellow-200 theme-dark:bg-yellow-900/20 theme-dark:text-yellow-400 theme-dark:border-yellow-800" + when :error, :destructive + "bg-red-50 text-red-700 border-red-200 theme-dark:bg-red-900/20 theme-dark:text-red-400 theme-dark:border-red-800" + end + + "#{base_classes} #{variant_classes}" + end + + def icon_name + case variant + when :info + "info" + when :success + "check-circle" + when :warning + "alert-triangle" + when :error, :destructive + "x-circle" + end + end + + def icon_color + case variant + when :success + "success" + when :warning + "warning" + when :error, :destructive + "destructive" + else + "blue-600" + end + end +end diff --git a/app/components/link_component.html.erb b/app/components/link_component.html.erb index c22e873a..dae100ea 100644 --- a/app/components/link_component.html.erb +++ b/app/components/link_component.html.erb @@ -1,7 +1,6 @@ <%= link_to href, **merged_opts do %> <% if icon && (icon_position != "right") %> <%= helpers.icon(icon, size: size, color: icon_color) %> - <% end %> <% unless icon_only? %> @@ -10,6 +9,5 @@ <% if icon && icon_position == "right" %> <%= helpers.icon(icon, size: size, color: icon_color) %> - <% end %> <% end %> diff --git a/app/controllers/accountable_sparklines_controller.rb b/app/controllers/accountable_sparklines_controller.rb index 959717ec..45f01a2c 100644 --- a/app/controllers/accountable_sparklines_controller.rb +++ b/app/controllers/accountable_sparklines_controller.rb @@ -32,7 +32,7 @@ class AccountableSparklinesController < ApplicationController end def account_ids - family.accounts.active.where(accountable_type: accountable.name).pluck(:id) + family.accounts.visible.where(accountable_type: accountable.name).pluck(:id) end def cache_key diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 854bd826..c1804637 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -1,5 +1,5 @@ class AccountsController < ApplicationController - before_action :set_account, only: %i[sync chart sparkline] + before_action :set_account, only: %i[sync chart sparkline toggle_active] include Periodable def index @@ -33,6 +33,15 @@ class AccountsController < ApplicationController end end + def toggle_active + if @account.active? + @account.disable! + elsif @account.disabled? + @account.enable! + end + redirect_to accounts_path + end + private def family Current.family diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 98a49fef..dea9cbe6 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -9,7 +9,7 @@ class Api::V1::AccountsController < Api::V1::BaseController def index # Test with Pagy pagination family = current_resource_owner.family - accounts_query = family.accounts.active.alphabetically + accounts_query = family.accounts.visible.alphabetically # Handle pagination with Pagy @pagy, @accounts = pagy( diff --git a/app/controllers/api/v1/transactions_controller.rb b/app/controllers/api/v1/transactions_controller.rb index b2e483bc..299ff274 100644 --- a/app/controllers/api/v1/transactions_controller.rb +++ b/app/controllers/api/v1/transactions_controller.rb @@ -10,7 +10,7 @@ class Api::V1::TransactionsController < Api::V1::BaseController def index family = current_resource_owner.family - transactions_query = family.transactions.active + transactions_query = family.transactions.visible # Apply filters transactions_query = apply_filters(transactions_query) diff --git a/app/controllers/concerns/accountable_resource.rb b/app/controllers/concerns/accountable_resource.rb index ec8bc845..9daa0ae2 100644 --- a/app/controllers/concerns/accountable_resource.rb +++ b/app/controllers/concerns/accountable_resource.rb @@ -43,9 +43,25 @@ module AccountableResource end def update - @account.update_with_sync!(account_params.except(:return_to)) - @account.lock_saved_attributes! + # Handle balance update if provided + if account_params[:balance].present? + result = @account.update_balance(balance: account_params[:balance], currency: account_params[:currency]) + unless result.success? + @error_message = result.error_message + render :edit, status: :unprocessable_entity + return + end + end + # Update remaining account attributes + update_params = account_params.except(:return_to, :balance, :currency) + unless @account.update(update_params) + @error_message = @account.errors.full_messages.join(", ") + render :edit, status: :unprocessable_entity + return + end + + @account.lock_saved_attributes! redirect_back_or_to @account, notice: t("accounts.update.success", type: accountable_type.name.underscore.humanize) end @@ -74,7 +90,7 @@ module AccountableResource def account_params params.require(:account).permit( - :name, :is_active, :balance, :subtype, :currency, :accountable_type, :return_to, + :name, :balance, :subtype, :currency, :accountable_type, :return_to, accountable_attributes: self.class.permitted_accountable_attributes ) end diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index a3694fb9..1fe96e27 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -5,7 +5,7 @@ class PagesController < ApplicationController def dashboard @balance_sheet = Current.family.balance_sheet - @accounts = Current.family.accounts.active.with_attached_logo + @accounts = Current.family.accounts.visible.with_attached_logo period_param = params[:cashflow_period] @cashflow_period = if period_param.present? diff --git a/app/controllers/properties_controller.rb b/app/controllers/properties_controller.rb index fb6048e9..8b2ec062 100644 --- a/app/controllers/properties_controller.rb +++ b/app/controllers/properties_controller.rb @@ -1,21 +1,99 @@ class PropertiesController < ApplicationController - include AccountableResource + include AccountableResource, StreamExtensions - permitted_accountable_attributes( - :id, :year_built, :area_unit, :area_value, - address_attributes: [ :line1, :line2, :locality, :region, :country, :postal_code ] - ) + before_action :set_property, only: [ :balances, :address, :update_balances, :update_address ] def new - @account = Current.family.accounts.build( - currency: Current.family.currency, - accountable: Property.new( - address: Address.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") ) + + redirect_to balances_property_path(@account) + end + + def update + if @account.update(property_params) + @success_message = "Property details updated successfully." + + if @account.active? + render :edit + else + redirect_to balances_property_path(@account) + end + else + @error_message = "Unable to update property details." + render :edit, status: :unprocessable_entity + end end def edit - @account.accountable.address ||= Address.new end + + def balances + 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." + + if @account.active? + render :balances + else + redirect_to address_property_path(@account) + end + else + @error_message = result.error_message + render :balances, status: :unprocessable_entity + end + end + + def address + @property = @account.property + @property.address ||= Address.new + end + + def update_address + if @account.property.update(address_params) + if @account.draft? + @account.activate! + + respond_to do |format| + format.html { redirect_to account_path(@account) } + format.turbo_stream { stream_redirect_to account_path(@account) } + end + else + @success_message = "Address updated successfully." + render :address + end + else + @error_message = "Unable to update address. Please check the required fields." + render :address, status: :unprocessable_entity + end + end + + private + def balance_params + params.require(:account).permit(:balance, :currency) + end + + def address_params + params.require(:property) + .permit(address_attributes: [ :line1, :line2, :locality, :region, :country, :postal_code ]) + end + + def property_params + params.require(:account) + .permit(:name, :subtype, :accountable_type, accountable_attributes: [ :id, :year_built, :area_unit, :area_value ]) + end + + def set_property + @account = Current.family.accounts.find(params[:id]) + @property = @account.property + end end diff --git a/app/controllers/transfer_matches_controller.rb b/app/controllers/transfer_matches_controller.rb index f8cd5b49..37dbc9d7 100644 --- a/app/controllers/transfer_matches_controller.rb +++ b/app/controllers/transfer_matches_controller.rb @@ -2,7 +2,7 @@ class TransferMatchesController < ApplicationController before_action :set_entry def new - @accounts = Current.family.accounts.alphabetically.where.not(id: @entry.account_id) + @accounts = Current.family.accounts.visible.alphabetically.where.not(id: @entry.account_id) @transfer_match_candidates = @entry.transaction.transfer_match_candidates end diff --git a/app/controllers/valuations_controller.rb b/app/controllers/valuations_controller.rb index a33e5457..90aa1da0 100644 --- a/app/controllers/valuations_controller.rb +++ b/app/controllers/valuations_controller.rb @@ -1,30 +1,42 @@ class ValuationsController < ApplicationController - include EntryableResource + include EntryableResource, StreamExtensions def create account = Current.family.accounts.find(params.dig(:entry, :account_id)) - @entry = account.entries.new(entry_params.merge(entryable: Valuation.new)) - if @entry.save - @entry.sync_account_later + result = account.update_balance( + balance: entry_params[:amount], + date: entry_params[:date], + currency: entry_params[:currency], + notes: entry_params[:notes] + ) - flash[:notice] = "Balance created" + if result.success? + @success_message = result.updated? ? "Balance updated" : "No changes made. Account is already up to date." respond_to do |format| - format.html { redirect_back_or_to account_path(@entry.account) } - format.turbo_stream { stream_redirect_back_or_to(account_path(@entry.account)) } + format.html { redirect_back_or_to account_path(account), notice: @success_message } + format.turbo_stream { stream_redirect_back_or_to(account_path(account), notice: @success_message) } end else + @error_message = result.error_message render :new, status: :unprocessable_entity end end def update - if @entry.update(entry_params) - @entry.sync_account_later + result = @entry.account.update_balance( + date: @entry.date, + balance: entry_params[:amount], + currency: entry_params[:currency], + notes: entry_params[:notes] + ) + + if result.success? + @entry.reload respond_to do |format| - format.html { redirect_back_or_to account_path(@entry.account), notice: "Balance updated" } + format.html { redirect_back_or_to account_path(@entry.account), notice: result.updated? ? "Balance updated" : "No changes made. Account is already up to date." } format.turbo_stream do render turbo_stream: [ turbo_stream.replace( @@ -37,6 +49,7 @@ class ValuationsController < ApplicationController end end else + @error_message = result.error_message render :show, status: :unprocessable_entity end end @@ -44,6 +57,6 @@ class ValuationsController < ApplicationController private def entry_params params.require(:entry) - .permit(:name, :date, :amount, :currency, :notes) + .permit(:date, :amount, :currency, :notes) end end diff --git a/app/helpers/styled_form_builder.rb b/app/helpers/styled_form_builder.rb index 1ddd445a..b614255d 100644 --- a/app/helpers/styled_form_builder.rb +++ b/app/helpers/styled_form_builder.rb @@ -1,42 +1,38 @@ class StyledFormBuilder < ActionView::Helpers::FormBuilder - # Fields that visually inherit from "text field" class_attribute :text_field_helpers, default: field_helpers - [ :label, :check_box, :radio_button, :fields_for, :fields, :hidden_field, :file_field ] - # Wraps "text" inputs with custom structure + base styles text_field_helpers.each do |selector| class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1 def #{selector}(method, options = {}) - merged_options = { class: "form-field__input" }.merge(options) - label = build_label(method, options) - field = super(method, merged_options) + form_options = options.slice(:label, :label_tooltip, :inline, :container_class, :required) + html_options = options.except(:label, :label_tooltip, :inline, :container_class) - build_styled_field(label, field, merged_options) + build_field(method, form_options, html_options) do |merged_options| + super(method, merged_options) + end end RUBY_EVAL end def radio_button(method, tag_value, options = {}) merged_options = { class: "form-field__radio" }.merge(options) - super(method, tag_value, merged_options) end def select(method, choices, options = {}, html_options = {}) - merged_html_options = { class: "form-field__input" }.merge(html_options) + field_options = normalize_options(options, html_options) - label = build_label(method, options.merge(required: merged_html_options[:required])) - field = super(method, choices, options, merged_html_options) - - build_styled_field(label, field, options, remove_padding_right: true) + build_field(method, field_options, html_options) do |merged_html_options| + super(method, choices, options, merged_html_options) + end end def collection_select(method, collection, value_method, text_method, options = {}, html_options = {}) - merged_html_options = { class: "form-field__input" }.merge(html_options) + field_options = normalize_options(options, html_options) - label = build_label(method, options.merge(required: merged_html_options[:required])) - field = super(method, collection, value_method, text_method, options, merged_html_options) - - build_styled_field(label, field, options, remove_padding_right: true) + build_field(method, field_options, html_options) do |merged_html_options| + super(method, collection, value_method, text_method, options, merged_html_options) + end end def money_field(amount_method, options = {}) @@ -48,22 +44,15 @@ class StyledFormBuilder < ActionView::Helpers::FormBuilder } end - # A custom styled "toggle" switch input. Underlying input is a `check_box` (uses same API) def toggle(method, options = {}, checked_value = "1", unchecked_value = "0") - if object - id = "#{object.id}_#{object_name}_#{method}" - name = "#{object_name}[#{method}]" - checked = object.send(method) - else - id = "#{method}_toggle_id" - name = method - checked = options[:checked] - end + field_id = field_id(method) + field_name = field_name(method) + checked = object ? object.send(method) : options[:checked] @template.render( ToggleComponent.new( - id: id, - name: name, + id: field_id, + name: field_name, checked: checked, disabled: options[:disabled], checked_value: checked_value, @@ -74,7 +63,6 @@ class StyledFormBuilder < ActionView::Helpers::FormBuilder end def submit(value = nil, options = {}) - # Rails superclass logic to extract the submit text value, options = nil, value if value.is_a?(Hash) value ||= submit_default_value @@ -88,16 +76,39 @@ class StyledFormBuilder < ActionView::Helpers::FormBuilder end private - def build_styled_field(label, field, options, remove_padding_right: false) - if options[:inline] - label + field - else - @template.tag.div class: [ "form-field", options[:container_class], ("pr-0" if remove_padding_right) ] do - label + field + def build_field(method, options = {}, html_options = {}, &block) + if options[:inline] || options[:label] == false + return yield({ class: "form-field__input" }.merge(html_options)) + end + + label_element = build_label(method, options) + field_element = yield({ class: "form-field__input" }.merge(html_options)) + + container_classes = [ "form-field", options[:container_class] ].compact + + @template.tag.div class: container_classes do + if options[:label_tooltip] + @template.tag.div(class: "form-field__header") do + label_element + + @template.tag.div(class: "form-field__actions") do + build_tooltip(options[:label_tooltip]) + end + end + + @template.tag.div(class: "form-field__body") do + field_element + end + else + @template.tag.div(class: "form-field__body") do + label_element + field_element + end end end end + def normalize_options(options, html_options) + options.merge(required: options[:required] || html_options[:required]) + end + def build_label(method, options) return "".html_safe unless options[:label] @@ -113,4 +124,15 @@ class StyledFormBuilder < ActionView::Helpers::FormBuilder return label(method, class: "form-field__label") if label_text == true label(method, label_text, class: "form-field__label") end + + def build_tooltip(tooltip_text) + return nil unless tooltip_text + + @template.tag.div(data: { controller: "tooltip" }) do + @template.safe_join([ + @template.icon("help-circle", size: "sm", color: "default", class: "cursor-help"), + @template.tag.div(tooltip_text, role: "tooltip", data: { tooltip_target: "tooltip" }, class: "tooltip bg-gray-700 text-sm p-2 rounded w-64 text-white") + ]) + end + end end diff --git a/app/models/account.rb b/app/models/account.rb index a87c9fa1..43e02cfd 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -1,5 +1,6 @@ class Account < ApplicationRecord include Syncable, Monetizable, Chartable, Linkable, Enrichable + include AASM validates :name, :balance, :currency, presence: true @@ -18,7 +19,7 @@ class Account < ApplicationRecord enum :classification, { asset: "asset", liability: "liability" }, validate: { allow_nil: true } - scope :active, -> { where(is_active: true) } + scope :visible, -> { where(status: [ "draft", "active" ]) } scope :assets, -> { where(classification: "asset") } scope :liabilities, -> { where(classification: "liability") } scope :alphabetically, -> { order(:name) } @@ -30,6 +31,30 @@ class Account < ApplicationRecord accepts_nested_attributes_for :accountable, update_only: true + # Account state machine + aasm column: :status, timestamps: true do + state :active, initial: true + state :draft + state :disabled + state :pending_deletion + + event :activate do + transitions from: [ :draft, :disabled ], to: :active + end + + event :disable do + transitions from: [ :draft, :active ], to: :disabled + end + + event :enable do + transitions from: :disabled, to: :active + end + + event :mark_for_deletion do + transitions from: [ :draft, :active, :disabled ], to: :pending_deletion + end + end + class << self def create_and_sync(attributes) attributes[:accountable_attributes] ||= {} # Ensure accountable is created, even if empty @@ -77,10 +102,20 @@ class Account < ApplicationRecord end def destroy_later - update!(scheduled_for_deletion: true, is_active: false) + mark_for_deletion! DestroyJob.perform_later(self) end + # Override destroy to handle error recovery for accounts + def destroy + super + rescue => e + # If destruction fails, transition back to disabled state + # This provides a cleaner recovery path than the generic scheduled_for_deletion flag + disable! if may_disable? + raise e + end + def current_holdings holdings.where(currency: currency) .where.not(qty: 0) @@ -92,49 +127,9 @@ class Account < ApplicationRecord .order(amount: :desc) end - def update_with_sync!(attributes) - should_update_balance = attributes[:balance] && attributes[:balance].to_d != balance - initial_balance = attributes.dig(:accountable_attributes, :initial_balance) - should_update_initial_balance = initial_balance && initial_balance.to_d != accountable.initial_balance - - transaction do - update!(attributes) - update_balance!(attributes[:balance]) if should_update_balance - update_inital_balance!(attributes[:accountable_attributes][:initial_balance]) if should_update_initial_balance - end - - sync_later - end - - def update_balance!(balance) - valuation = entries.valuations.find_by(date: Date.current) - - if valuation - valuation.update! amount: balance - else - entries.create! \ - date: Date.current, - name: "Balance update", - amount: balance, - currency: currency, - entryable: Valuation.new - end - end - - def update_inital_balance!(initial_balance) - valuation = first_valuation - - if valuation - valuation.update! amount: initial_balance - else - entries.create! \ - date: Date.current, - name: "Initial Balance", - amount: initial_balance, - currency: currency, - entryable: Valuation.new - end + def update_balance(balance:, date: Date.current, currency: nil, notes: nil) + Account::BalanceUpdater.new(self, balance:, currency:, date:, notes:).update end def start_date diff --git a/app/models/account/balance_updater.rb b/app/models/account/balance_updater.rb new file mode 100644 index 00000000..d006df3f --- /dev/null +++ b/app/models/account/balance_updater.rb @@ -0,0 +1,47 @@ +class Account::BalanceUpdater + def initialize(account, balance:, currency: nil, date: Date.current, notes: nil) + @account = account + @balance = balance.to_d + @currency = currency + @date = date.to_date + @notes = notes + end + + def update + return Result.new(success?: true, updated?: false) unless requires_update? + + Account.transaction do + if date == Date.current + account.balance = balance + account.currency = currency if currency.present? + account.save! + end + + valuation_entry = account.entries.valuations.find_or_initialize_by(date: date) do |entry| + entry.entryable = Valuation.new + end + + valuation_entry.amount = balance + valuation_entry.currency = currency if currency.present? + valuation_entry.name = "Manual #{account.accountable.balance_display_name} update" + valuation_entry.notes = notes if notes.present? + valuation_entry.save! + end + + account.sync_later + + Result.new(success?: true, updated?: true) + rescue => e + message = Rails.env.development? ? e.message : "Unable to update account values. Please try again." + Result.new(success?: false, updated?: false, error_message: message) + end + + private + attr_reader :account, :balance, :currency, :date, :notes + + Result = Struct.new(:success?, :updated?, :error_message) + + def requires_update? + date != Date.current || account.balance != balance || account.currency != currency + end +end diff --git a/app/models/assistant/function.rb b/app/models/assistant/function.rb index 16e3215f..97f069cc 100644 --- a/app/models/assistant/function.rb +++ b/app/models/assistant/function.rb @@ -56,7 +56,7 @@ class Assistant::Function end def family_account_names - @family_account_names ||= family.accounts.active.pluck(:name) + @family_account_names ||= family.accounts.visible.pluck(:name) end def family_category_names diff --git a/app/models/assistant/function/get_accounts.rb b/app/models/assistant/function/get_accounts.rb index b912d81d..f8c6a6db 100644 --- a/app/models/assistant/function/get_accounts.rb +++ b/app/models/assistant/function/get_accounts.rb @@ -22,7 +22,7 @@ class Assistant::Function::GetAccounts < Assistant::Function type: account.accountable_type, start_date: account.start_date, is_plaid_linked: account.plaid_account_id.present?, - is_active: account.is_active, + status: account.status, historical_balances: historical_balances(account) } end diff --git a/app/models/assistant/function/get_balance_sheet.rb b/app/models/assistant/function/get_balance_sheet.rb index 5bea4c6d..b0e1f0e0 100644 --- a/app/models/assistant/function/get_balance_sheet.rb +++ b/app/models/assistant/function/get_balance_sheet.rb @@ -44,7 +44,7 @@ class Assistant::Function::GetBalanceSheet < Assistant::Function private def historical_data(period, classification: nil) - scope = family.accounts.active + scope = family.accounts.visible scope = scope.where(classification: classification) if classification.present? if period.start_date == Date.current diff --git a/app/models/assistant/function/get_transactions.rb b/app/models/assistant/function/get_transactions.rb index 7b68def9..9b0da316 100644 --- a/app/models/assistant/function/get_transactions.rb +++ b/app/models/assistant/function/get_transactions.rb @@ -134,7 +134,7 @@ class Assistant::Function::GetTransactions < Assistant::Function def call(params = {}) search_params = params.except("order", "page") - transactions_query = family.transactions.active.search(search_params) + transactions_query = family.transactions.visible.search(search_params) pagy_query = params["order"] == "asc" ? transactions_query.chronological : transactions_query.reverse_chronological # By default, we give a small page size to force the AI to use filters effectively and save on tokens diff --git a/app/models/balance_sheet/account_totals.rb b/app/models/balance_sheet/account_totals.rb index ee3f1718..31708f24 100644 --- a/app/models/balance_sheet/account_totals.rb +++ b/app/models/balance_sheet/account_totals.rb @@ -23,8 +23,8 @@ class BalanceSheet::AccountTotals delegate_missing_to :account end - def active_accounts - @active_accounts ||= family.accounts.active.with_attached_logo + def visible_accounts + @visible_accounts ||= family.accounts.visible.with_attached_logo end def account_rows @@ -46,7 +46,7 @@ class BalanceSheet::AccountTotals def query @query ||= Rails.cache.fetch(cache_key) do - active_accounts + visible_accounts .joins(ActiveRecord::Base.sanitize_sql_array([ "LEFT JOIN exchange_rates ON exchange_rates.date = ? AND accounts.currency = exchange_rates.from_currency AND exchange_rates.to_currency = ?", Date.current, diff --git a/app/models/balance_sheet/net_worth_series_builder.rb b/app/models/balance_sheet/net_worth_series_builder.rb index c4c79971..f3e61a99 100644 --- a/app/models/balance_sheet/net_worth_series_builder.rb +++ b/app/models/balance_sheet/net_worth_series_builder.rb @@ -6,7 +6,7 @@ class BalanceSheet::NetWorthSeriesBuilder def net_worth_series(period: Period.last_30_days) Rails.cache.fetch(cache_key(period)) do builder = Balance::ChartSeriesBuilder.new( - account_ids: active_account_ids, + account_ids: visible_account_ids, currency: family.currency, period: period, favorable_direction: "up" @@ -19,8 +19,8 @@ class BalanceSheet::NetWorthSeriesBuilder private attr_reader :family - def active_account_ids - @active_account_ids ||= family.accounts.active.with_attached_logo.pluck(:id) + def visible_account_ids + @visible_account_ids ||= family.accounts.visible.with_attached_logo.pluck(:id) end def cache_key(period) diff --git a/app/models/balance_sheet/sync_status_monitor.rb b/app/models/balance_sheet/sync_status_monitor.rb index 5682bd63..475454ee 100644 --- a/app/models/balance_sheet/sync_status_monitor.rb +++ b/app/models/balance_sheet/sync_status_monitor.rb @@ -17,7 +17,7 @@ class BalanceSheet::SyncStatusMonitor def syncing_account_ids Rails.cache.fetch(cache_key) do Sync.visible - .where(syncable_type: "Account", syncable_id: family.accounts.active.pluck(:id)) + .where(syncable_type: "Account", syncable_id: family.accounts.visible.pluck(:id)) .pluck(:syncable_id) .to_set end diff --git a/app/models/budget.rb b/app/models/budget.rb index e53b6f09..f8cb991c 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -88,7 +88,7 @@ class Budget < ApplicationRecord end def transactions - family.transactions.active.in_period(period) + family.transactions.visible.in_period(period) end def name diff --git a/app/models/concerns/accountable.rb b/app/models/concerns/accountable.rb index 12ee9888..d7f2a331 100644 --- a/app/models/concerns/accountable.rb +++ b/app/models/concerns/accountable.rb @@ -72,6 +72,14 @@ module Accountable self.class.display_name end + def balance_display_name + "account value" + end + + def opening_balance_display_name + "opening balance" + end + def icon self.class.icon end diff --git a/app/models/entry.rb b/app/models/entry.rb index 0426c9f5..332cbee9 100644 --- a/app/models/entry.rb +++ b/app/models/entry.rb @@ -14,8 +14,8 @@ class Entry < ApplicationRecord validates :date, uniqueness: { scope: [ :account_id, :entryable_type ] }, if: -> { valuation? } validates :date, comparison: { greater_than: -> { min_supported_date } } - scope :active, -> { - joins(:account).where(accounts: { is_active: true }) + scope :visible, -> { + joins(:account).where(accounts: { status: [ "draft", "active" ] }) } scope :chronological, -> { diff --git a/app/models/entryable.rb b/app/models/entryable.rb index 2c93786c..cf4d2282 100644 --- a/app/models/entryable.rb +++ b/app/models/entryable.rb @@ -14,7 +14,7 @@ module Entryable scope :with_entry, -> { joins(:entry) } - scope :active, -> { with_entry.merge(Entry.active) } + scope :visible, -> { with_entry.merge(Entry.visible) } scope :in_period, ->(period) { with_entry.where(entries: { date: period.start_date..period.end_date }) diff --git a/app/models/family.rb b/app/models/family.rb index 827a2e46..1f35488f 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -100,7 +100,8 @@ class Family < ApplicationRecord [ id, key, - data_invalidation_key + data_invalidation_key, + accounts.maximum(:updated_at) ].compact.join("_") end diff --git a/app/models/family/auto_transfer_matchable.rb b/app/models/family/auto_transfer_matchable.rb index 988fda22..28d06f43 100644 --- a/app/models/family/auto_transfer_matchable.rb +++ b/app/models/family/auto_transfer_matchable.rb @@ -30,8 +30,8 @@ module Family::AutoTransferMatchable .joins("JOIN accounts inflow_accounts ON inflow_accounts.id = inflow_candidates.account_id") .joins("JOIN accounts outflow_accounts ON outflow_accounts.id = outflow_candidates.account_id") .where("inflow_accounts.family_id = ? AND outflow_accounts.family_id = ?", self.id, self.id) - .where("inflow_accounts.is_active = true") - .where("outflow_accounts.is_active = true") + .where("inflow_accounts.status IN ('draft', 'active')") + .where("outflow_accounts.status IN ('draft', 'active')") .where("inflow_candidates.entryable_type = 'Transaction' AND outflow_candidates.entryable_type = 'Transaction'") .where(" ( diff --git a/app/models/income_statement.rb b/app/models/income_statement.rb index a06806d4..cfa066fe 100644 --- a/app/models/income_statement.rb +++ b/app/models/income_statement.rb @@ -10,7 +10,7 @@ class IncomeStatement end def totals(transactions_scope: nil) - transactions_scope ||= family.transactions.active + transactions_scope ||= family.transactions.visible result = totals_query(transactions_scope: transactions_scope) @@ -62,7 +62,7 @@ class IncomeStatement end def build_period_total(classification:, period:) - totals = totals_query(transactions_scope: family.transactions.active.in_period(period)).select { |t| t.classification == classification } + totals = totals_query(transactions_scope: family.transactions.visible.in_period(period)).select { |t| t.classification == classification } classification_total = totals.sum(&:total) uncategorized_category = family.categories.uncategorized diff --git a/app/models/property.rb b/app/models/property.rb index a530f8b1..6114a9f4 100644 --- a/app/models/property.rb +++ b/app/models/property.rb @@ -42,6 +42,14 @@ class Property < ApplicationRecord Trend.new(current: account.balance_money, previous: first_valuation_amount) end + def balance_display_name + "market value" + end + + def opening_balance_display_name + "original purchase price" + end + private def first_valuation_amount account.entries.valuations.order(:date).first&.amount_money || account.balance_money diff --git a/app/models/rule/registry/transaction_resource.rb b/app/models/rule/registry/transaction_resource.rb index 15aae288..d00f2b1e 100644 --- a/app/models/rule/registry/transaction_resource.rb +++ b/app/models/rule/registry/transaction_resource.rb @@ -1,6 +1,6 @@ class Rule::Registry::TransactionResource < Rule::Registry def resource_scope - family.transactions.active.with_entry.where(entry: { date: rule.effective_date.. }) + family.transactions.visible.with_entry.where(entry: { date: rule.effective_date.. }) end def condition_filters diff --git a/app/models/transaction/search.rb b/app/models/transaction/search.rb index 71406e71..2fc17182 100644 --- a/app/models/transaction/search.rb +++ b/app/models/transaction/search.rb @@ -81,7 +81,7 @@ class Transaction::Search def apply_active_accounts_filter(query, active_accounts_only_filter) if active_accounts_only_filter - query.where(accounts: { is_active: true }) + query.where(accounts: { status: [ "draft", "active" ] }) else query end diff --git a/app/views/accounts/_account.html.erb b/app/views/accounts/_account.html.erb index 475f953e..9430d605 100644 --- a/app/views/accounts/_account.html.erb +++ b/app/views/accounts/_account.html.erb @@ -6,7 +6,7 @@ <%= render "accounts/logo", account: account, size: "md" %>
- <% if account.scheduled_for_deletion? %> + <% if account.pending_deletion? %>

<%= account.name %> @@ -16,31 +16,45 @@

<% else %> - <%= link_to account.name, account, class: [(account.is_active ? "text-primary" : "text-subdued"), "text-sm font-medium hover:underline"], data: { turbo_frame: "_top" } %> + <%= link_to account.name, account, class: [(account.active? ? "text-primary" : "text-subdued"), "text-sm font-medium hover:underline"], data: { turbo_frame: "_top" } %> <% if account.long_subtype_label %>

<%= account.long_subtype_label %>

<% end %> <% end %>
- <% unless account.scheduled_for_deletion? %> + <% unless account.pending_deletion? %> <%= link_to edit_account_path(account, return_to: return_to), data: { turbo_frame: :modal }, class: "group-hover/account:flex hidden hover:opacity-80 items-center justify-center" do %> <%= icon("pencil-line", size: "sm") %> <% end %> <% end %>
- <% if account.syncing? %> + <% if account.draft? %> + + <% elsif account.syncing? %>
<% else %> -

"> +

"> <%= format_money account.balance_money %>

<% end %> - <% unless account.scheduled_for_deletion? %> - <%= styled_form_with model: account, data: { turbo_frame: "_top", controller: "auto-submit-form" } do |f| %> - <%= f.toggle :is_active, { data: { auto_submit_form_target: "auto" } } %> + <% if account.draft? %> + <%= render LinkComponent.new( + text: "Complete setup", + href: edit_account_path(account, return_to: return_to), + variant: :outline, + frame: :modal + ) %> + <% elsif account.active? || account.disabled? %> + <%= form_with model: account, url: toggle_active_account_path(account), method: :patch, data: { turbo_frame: "_top", controller: "auto-submit-form" } do |f| %> + <%= render ToggleComponent.new( + id: "account_#{account.id}_active", + name: "active", + checked: account.active?, + data: { auto_submit_form_target: "auto" } + ) %> <% end %> <% end %>
diff --git a/app/views/accounts/_form.html.erb b/app/views/accounts/_form.html.erb index 8a67d20f..ef2e0af5 100644 --- a/app/views/accounts/_form.html.erb +++ b/app/views/accounts/_form.html.erb @@ -1,5 +1,9 @@ <%# locals: (account:, url:) %> +<% if @error_message.present? %> + <%= render AlertComponent.new(message: @error_message, variant: :error) %> +<% end %> + <%= styled_form_with model: account, url: url, scope: :account, data: { turbo: false }, class: "flex flex-col gap-4 justify-between grow text-primary" do |form| %>
<%= form.hidden_field :accountable_type %> diff --git a/app/views/accounts/show/_header.html.erb b/app/views/accounts/show/_header.html.erb index ea0514a5..51d3b253 100644 --- a/app/views/accounts/show/_header.html.erb +++ b/app/views/accounts/show/_header.html.erb @@ -12,7 +12,18 @@
-

"><%= title || account.name %>

+
+

"><%= title || account.name %>

+ <% if account.draft? %> + <%= render LinkComponent.new( + text: "Complete setup", + href: edit_account_path(account), + variant: :outline, + size: :sm, + frame: :modal + ) %> + <% end %> +
<% if subtitle.present? %>

<%= subtitle %>

<% end %> diff --git a/app/views/import/uploads/show.html.erb b/app/views/import/uploads/show.html.erb index 9f523673..1f390617 100644 --- a/app/views/import/uploads/show.html.erb +++ b/app/views/import/uploads/show.html.erb @@ -22,7 +22,7 @@ <%= form.select :col_sep, Import::SEPARATORS, label: true %> <% if @import.type == "TransactionImport" || @import.type == "TradeImport" %> - <%= form.select :account_id, @import.family.accounts.pluck(:name, :id), { label: "Account (optional)", include_blank: "Multi-account import", selected: @import.account_id } %> + <%= form.select :account_id, @import.family.accounts.visible.pluck(:name, :id), { label: "Account (optional)", include_blank: "Multi-account import", selected: @import.account_id } %> <% end %>
@@ -54,7 +54,7 @@ <%= form.select :col_sep, Import::SEPARATORS, label: true %> <% if @import.type == "TransactionImport" || @import.type == "TradeImport" %> - <%= form.select :account_id, @import.family.accounts.pluck(:name, :id), { label: "Account (optional)", include_blank: "Multi-account import", selected: @import.account_id } %> + <%= form.select :account_id, @import.family.accounts.visible.pluck(:name, :id), { label: "Account (optional)", include_blank: "Multi-account import", selected: @import.account_id } %> <% end %> <%= form.text_area :raw_file_str, diff --git a/app/views/properties/_form_alert.html.erb b/app/views/properties/_form_alert.html.erb new file mode 100644 index 00000000..934fa374 --- /dev/null +++ b/app/views/properties/_form_alert.html.erb @@ -0,0 +1,7 @@ +<%# locals: (notice: nil, error: nil) %> + +<% if notice.present? %> + <%= render AlertComponent.new(message: notice, variant: :success) %> +<% elsif error.present? %> + <%= render AlertComponent.new(message: error, variant: :error) %> +<% end %> diff --git a/app/views/properties/_form_tab.html.erb b/app/views/properties/_form_tab.html.erb new file mode 100644 index 00000000..f23c0937 --- /dev/null +++ b/app/views/properties/_form_tab.html.erb @@ -0,0 +1,16 @@ +<%# locals: (label:, href: nil, active: false) %> + +<% classes = class_names( + "flex items-center px-3 py-2 rounded-lg text-sm font-medium", + active ? "bg-surface-inset text-primary" : "text-secondary", +) %> + +<% if href.present? %> + <%= link_to href, data: { turbo_frame: :modal }, class: class_names(classes, "cursor-pointer hover:bg-surface-inset-hover hover:text-primary") do %> + <%= label %> + <% end %> +<% else %> + <%= tag.span class: classes do %> + <%= label %> + <% end %> +<% end %> diff --git a/app/views/properties/_form_tabs.html.erb b/app/views/properties/_form_tabs.html.erb new file mode 100644 index 00000000..0693c7c9 --- /dev/null +++ b/app/views/properties/_form_tabs.html.erb @@ -0,0 +1,7 @@ +<%# locals: (account:, active_tab:) %> + +
+ <%= 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: "Address", href: account.new_record? ? nil : address_property_path(@account), active: active_tab == "address" %> +
diff --git a/app/views/properties/_overview_fields.html.erb b/app/views/properties/_overview_fields.html.erb new file mode 100644 index 00000000..cd263ae9 --- /dev/null +++ b/app/views/properties/_overview_fields.html.erb @@ -0,0 +1,35 @@ +<%# locals: (form:) %> + +
+ <%= form.text_field :name, + label: "Name", + placeholder: "Vacation home", + required: true %> + + <%= form.select :subtype, + Property::SUBTYPES.map { |k, v| [v[:long], k] }, + { prompt: "Select type", label: "Property type" }, required: true %> + + <%= form.hidden_field :accountable_type, value: "Property" %> + + <%= form.fields_for :accountable do |property_form| %> +
+ <%= property_form.number_field :year_built, + label: "Year Built (optional)", + placeholder: "1990", + min: 1800, + max: Time.current.year %> +
+ +
+ <%= property_form.number_field :area_value, + label: "Area (optional)", + placeholder: "1200", + min: 0 %> + <%= property_form.select :area_unit, + [["Square Feet", "sqft"], ["Square Meters", "sqm"]], + { label: "Area Unit" } %> +
+ <% end %> + +
diff --git a/app/views/properties/address.html.erb b/app/views/properties/address.html.erb new file mode 100644 index 00000000..765fc48a --- /dev/null +++ b/app/views/properties/address.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: "address" %> + + +
+ <%= styled_form_with model: @property, url: update_address_property_path(@account), method: :patch, data: { turbo_frame: @property.address.persisted? ? nil : :_top } do |form| %> +
+ <%= render "properties/form_alert", notice: @success_message, error: @error_message %> + + <%= form.fields_for :address do |address_form| %> + <%= address_form.text_field :line1, + label: "Address Line 1", + placeholder: "123 Main Street" %> + +
+ <%= address_form.text_field :locality, + label: "City", + placeholder: "San Francisco" %> + <%= address_form.text_field :region, + label: "State/Region", + placeholder: "CA" %> +
+ +
+ <%= address_form.text_field :postal_code, + label: "Postal Code", + placeholder: "12345" %> + <%= address_form.text_field :country, + label: "Country", + placeholder: "USA" %> +
+ <% end %> +
+ + +
+ <%= render ButtonComponent.new( + text: "Save", + variant: "primary", + ) %> +
+ <% end %> +
+
+ <% end %> +<% end %> diff --git a/app/views/properties/balances.html.erb b/app/views/properties/balances.html.erb new file mode 100644 index 00000000..c9141771 --- /dev/null +++ b/app/views/properties/balances.html.erb @@ -0,0 +1,30 @@ +<%= 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/edit.html.erb b/app/views/properties/edit.html.erb index 9681e031..fd75d929 100644 --- a/app/views/properties/edit.html.erb +++ b/app/views/properties/edit.html.erb @@ -1,6 +1,27 @@ <%= render DialogComponent.new do |dialog| %> - <% dialog.with_header(title: t(".edit", account: @account.name)) %> + <% dialog.with_header(title: "Enter property manually") %> <% dialog.with_body do %> - <%= render "form", account: @account, url: property_path(@account) %> +
+ + <%= render "properties/form_tabs", account: @account, active_tab: "overview" %> + + +
+ <%= 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 ButtonComponent.new( + text: @account.active? ? "Save" : "Next", + variant: "primary", + ) %> +
+ <% end %> +
+
<% end %> <% end %> diff --git a/app/views/properties/new.html.erb b/app/views/properties/new.html.erb index d888f198..73dae294 100644 --- a/app/views/properties/new.html.erb +++ b/app/views/properties/new.html.erb @@ -1,6 +1,27 @@ <%= render DialogComponent.new do |dialog| %> - <% dialog.with_header(title: t(".title")) %> + <% dialog.with_header(title: "Enter property manually") %> <% dialog.with_body do %> - <%= render "properties/form", account: @account, url: properties_path(return_to: params[:return_to]) %> +
+ + <%= render "properties/form_tabs", account: @account, active_tab: "overview" %> + + +
+ <%= 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 ButtonComponent.new( + text: "Next", + variant: "primary", + ) %> +
+ <% end %> +
+
<% end %> <% end %> diff --git a/app/views/shared/_money_field.html.erb b/app/views/shared/_money_field.html.erb index 3f427c71..05929401 100644 --- a/app/views/shared/_money_field.html.erb +++ b/app/views/shared/_money_field.html.erb @@ -1,62 +1,85 @@ <%# locals: (form:, amount_method:, currency_method:, **options) %> <% currency_value = if options[:currency_value_override].present? - options[:currency_value_override] - elsif form.object && form.object.respond_to?(currency_method) - form.object.public_send(currency_method) - end + options[:currency_value_override] + elsif form.object && form.object.respond_to?(currency_method) + form.object.public_send(currency_method) + end currency = Money::Currency.new(currency_value || options[:default_currency] || "USD") %> -
- <%= form.label options[:label] || t(".label"), class: "form-field__label" do %> - <%= options[:label] || t(".label") %> - <% if options[:required] %> - * - <% end %> +
+ <% if options[:label_tooltip] %> +
+ <%= form.label options[:label] || t(".label"), class: "form-field__label" do %> + <%= options[:label] || t(".label") %> + <% if options[:required] %> + * + <% end %> + <% end %> +
+
+ <%= icon "help-circle", size: "sm", color: "default", class: "cursor-help" %> + +
+
+
<% end %> -
-
- - <%= currency.symbol %> - - - <%= form.number_field amount_method, - class: "form-field__input", - inline: true, - placeholder: "100", - value: if options[:value] - sprintf("%.#{currency.default_precision}f", options[:value]) - elsif form.object && form.object.respond_to?(amount_method) - val = form.object.public_send(amount_method) - sprintf("%.#{currency.default_precision}f", val) if val.present? - end, - min: options[:min] || -99999999999999, - max: options[:max] || 99999999999999, - step: currency.step, - disabled: options[:disabled], - data: { - "money-field-target": "amount", - "auto-submit-form-target": ("auto" if options[:auto_submit]) - }.compact, - required: options[:required] %> -
- - <% unless options[:hide_currency] %> -
- <%= form.select currency_method, - Money::Currency.as_options.map(&:iso_code), - { inline: true, selected: currency.iso_code }, - { - class: "w-fit pr-5 disabled:text-subdued form-field__input", - disabled: options[:disable_currency], - data: { - "money-field-target": "currency", - action: "change->money-field#handleCurrencyChange", - "auto-submit-form-target": ("auto" if options[:auto_submit]) - }.compact - } %> -
+
+ <% unless options[:label_tooltip] %> + <%= form.label options[:label] || t(".label"), class: "form-field__label" do %> + <%= options[:label] || t(".label") %> + <% if options[:required] %> + * + <% end %> + <% end %> <% end %> + +
+
+ + <%= currency.symbol %> + + + <%= form.number_field amount_method, + class: "form-field__input", + inline: true, + placeholder: "100", + value: if options[:value] + sprintf("%.#{currency.default_precision}f", options[:value]) + elsif form.object && form.object.respond_to?(amount_method) + val = form.object.public_send(amount_method) + sprintf("%.#{currency.default_precision}f", val) if val.present? + end, + min: options[:min] || -99999999999999, + max: options[:max] || 99999999999999, + step: currency.step, + disabled: options[:disabled], + data: { + "money-field-target": "amount", + "auto-submit-form-target": ("auto" if options[:auto_submit]) + }.compact, + required: options[:required] %> +
+ + <% unless options[:hide_currency] %> +
+ <%= form.select currency_method, + Money::Currency.as_options.map(&:iso_code), + { inline: true, selected: currency.iso_code }, + { + class: "w-fit pr-5 disabled:text-subdued form-field__input", + disabled: options[:disable_currency], + data: { + "money-field-target": "currency", + action: "change->money-field#handleCurrencyChange", + "auto-submit-form-target": ("auto" if options[:auto_submit]) + }.compact + } %> +
+ <% end %> +
diff --git a/app/views/trades/_form.html.erb b/app/views/trades/_form.html.erb index ddf9a26b..57e22907 100644 --- a/app/views/trades/_form.html.erb +++ b/app/views/trades/_form.html.erb @@ -45,7 +45,7 @@ <% end %> <% if %w[deposit withdrawal].include?(type) %> - <%= form.collection_select :transfer_account_id, Current.family.accounts.alphabetically, :id, :name, { prompt: t(".account_prompt"), label: t(".account") } %> + <%= form.collection_select :transfer_account_id, Current.family.accounts.visible.alphabetically, :id, :name, { prompt: t(".account_prompt"), label: t(".account") } %> <% end %> <% if %w[buy sell].include?(type) %> diff --git a/app/views/valuations/_form.html.erb b/app/views/valuations/_form.html.erb index 287d057f..5429f3a7 100644 --- a/app/views/valuations/_form.html.erb +++ b/app/views/valuations/_form.html.erb @@ -1,10 +1,10 @@ -<%# locals: (entry:) %> +<%# locals: (entry:, error_message:) %> <%= styled_form_with model: entry, url: valuations_path, class: "space-y-4" do |form| %> <%= form.hidden_field :account_id %> - <% if entry.errors.any? %> - <%= render "shared/form_errors", model: entry %> + <% if error_message.present? %> + <%= render AlertComponent.new(message: error_message, variant: :error) %> <% end %>
diff --git a/app/views/valuations/_header.html.erb b/app/views/valuations/_header.html.erb index 1f19967a..ce7f138f 100644 --- a/app/views/valuations/_header.html.erb +++ b/app/views/valuations/_header.html.erb @@ -2,7 +2,7 @@ <%= tag.header class: "mb-4 space-y-1", id: dom_id(entry, :header) do %> - <%= t(".balance") %> + <%= entry.name %>
diff --git a/app/views/valuations/_valuation.html.erb b/app/views/valuations/_valuation.html.erb index 416f1775..176f21be 100644 --- a/app/views/valuations/_valuation.html.erb +++ b/app/views/valuations/_valuation.html.erb @@ -25,15 +25,7 @@
-
- <% if balance_trend&.trend %> - <%= tag.span format_money(balance_trend.trend.value), style: "color: #{balance_trend.trend.color}" %> - <% else %> - <%= tag.span "--", class: "text-gray-400" %> - <% end %> -
- -
+
<%= tag.p format_money(entry.amount_money), class: "font-medium text-sm text-primary" %>
diff --git a/app/views/valuations/new.html.erb b/app/views/valuations/new.html.erb index 191ee5f5..82102f16 100644 --- a/app/views/valuations/new.html.erb +++ b/app/views/valuations/new.html.erb @@ -1,6 +1,6 @@ <%= render DialogComponent.new do |dialog| %> <% dialog.with_header(title: t(".title")) %> <% dialog.with_body do %> - <%= render "form", entry: @entry %> + <%= render "form", entry: @entry, error_message: @error_message %> <% end %> <% end %> diff --git a/app/views/valuations/show.html.erb b/app/views/valuations/show.html.erb index bd7d8d9e..6e96dcaa 100644 --- a/app/views/valuations/show.html.erb +++ b/app/views/valuations/show.html.erb @@ -6,17 +6,18 @@ <% end %> <% dialog.with_body do %> + <% if @error_message.present? %> +
+ <%= render AlertComponent.new(message: @error_message, variant: :error) %> +
+ <% end %> + <% dialog.with_section(title: t(".overview"), open: true) do %>
<%= styled_form_with model: entry, url: entry_path(entry), class: "space-y-2", data: { controller: "auto-submit-form" } do |f| %> - <%= f.text_field :name, - label: t(".name_label"), - placeholder: t(".name_placeholder"), - "data-auto-submit-form-target": "auto" %> - <%= f.date_field :date, label: t(".date_label"), max: Date.current, diff --git a/config/brakeman.ignore b/config/brakeman.ignore index 4d872a49..5ca90443 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -1,5 +1,28 @@ { "ignored_warnings": [ + { + "warning_type": "Mass Assignment", + "warning_code": 105, + "fingerprint": "85e2c11853dd6c69b1953a6ec3ad661cd0ce3df55e4e5beff92365b6ed601171", + "check_name": "PermitAttributes", + "message": "Potentially dangerous key allowed for mass assignment", + "file": "app/controllers/api/v1/transactions_controller.rb", + "line": 255, + "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", + "code": "params.require(:transaction).permit(:account_id, :date, :amount, :name, :description, :notes, :currency, :category_id, :merchant_id, :nature, :tag_ids => ([]))", + "render_path": null, + "location": { + "type": "method", + "class": "Api::V1::TransactionsController", + "method": "transaction_params" + }, + "user_input": ":account_id", + "confidence": "High", + "cwe_id": [ + 915 + ], + "note": "account_id is properly validated in create action - line 79 ensures account belongs to user's family: family.accounts.find(transaction_params[:account_id])" + }, { "warning_type": "Mass Assignment", "warning_code": 105, @@ -26,13 +49,13 @@ { "warning_type": "Dangerous Eval", "warning_code": 13, - "fingerprint": "c193307bb82f931950d3bf2855f82f9a7f50d94c5bd950ee2803cb8a8abe5253", + "fingerprint": "c154514a0f86341473e4abf35e77721495b326c7855e4967d284b4942371819c", "check_name": "Evaluation", "message": "Dynamic string evaluated as code", "file": "app/helpers/styled_form_builder.rb", - "line": 7, + "line": 5, "link": "https://brakemanscanner.org/docs/warning_types/dangerous_eval/", - "code": "class_eval(\" def #{selector}(method, options = {})\\n merged_options = { class: \\\"form-field__input\\\" }.merge(options)\\n label = build_label(method, options)\\n field = super(method, merged_options)\\n\\n build_styled_field(label, field, merged_options)\\n end\\n\", \"app/helpers/styled_form_builder.rb\", (7 + 1))", + "code": "class_eval(\" def #{selector}(method, options = {})\\n form_options = options.slice(:label, :label_tooltip, :inline, :container_class, :required)\\n html_options = options.except(:label, :label_tooltip, :inline, :container_class)\\n\\n build_field(method, form_options, html_options) do |merged_options|\\n super(method, merged_options)\\n end\\n end\\n\", \"app/helpers/styled_form_builder.rb\", (5 + 1))", "render_path": null, "location": { "type": "method", @@ -45,7 +68,7 @@ 913, 95 ], - "note": "This is safe as 'selector' comes from a predefined list of Rails form helpers (StyledFormBuilder.text_field_helpers)." + "note": "Uses similar pattern to Rails internal form builder" }, { "warning_type": "Dynamic Render Path", @@ -80,29 +103,6 @@ 22 ], "note": "" - }, - { - "warning_type": "Mass Assignment", - "warning_code": 105, - "fingerprint": "85e2c11853dd6c69b1953a6ec3ad661cd0ce3df55e4e5beff92365b6ed601171", - "check_name": "PermitAttributes", - "message": "Potentially dangerous key allowed for mass assignment", - "file": "app/controllers/api/v1/transactions_controller.rb", - "line": 255, - "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", - "code": "params.require(:transaction).permit(:account_id, :date, :amount, :name, :description, :notes, :currency, :category_id, :merchant_id, :nature, :tag_ids => ([]))", - "render_path": null, - "location": { - "type": "method", - "class": "Api::V1::TransactionsController", - "method": "transaction_params" - }, - "user_input": ":account_id", - "confidence": "High", - "cwe_id": [ - 915 - ], - "note": "account_id is properly validated in create action - line 79 ensures account belongs to user's family: family.accounts.find(transaction_params[:account_id])" } ], "brakeman_version": "7.0.2" diff --git a/config/routes.rb b/config/routes.rb index 43ca807a..f0414287 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -108,14 +108,6 @@ Rails.application.routes.draw do resources :mappings, only: :update, module: :import end - resources :accounts, only: %i[index new], shallow: true do - member do - post :sync - get :chart - get :sparkline - end - end - resources :holdings, only: %i[index new show destroy] resources :trades, only: %i[show new create update destroy] resources :valuations, only: %i[show new create update destroy] @@ -155,18 +147,36 @@ Rails.application.routes.draw do end end + resources :accounts, only: %i[index new], shallow: true do + member do + post :sync + get :chart + get :sparkline + patch :toggle_active + end + end + # Convenience routes for polymorphic paths # Example: account_path(Account.new(accountable: Depository.new)) => /depositories/123 direct :account do |model, options| route_for model.accountable_name, model, options end + direct :edit_account do |model, options| route_for "edit_#{model.accountable_name}", model, options end resources :depositories, except: :index resources :investments, except: :index - resources :properties, except: :index + resources :properties, except: :index do + member do + get :balances + patch :update_balances + + get :address + patch :update_address + end + end resources :vehicles, except: :index resources :credit_cards, except: :index resources :loans, except: :index diff --git a/db/migrate/20250701161640_add_account_status.rb b/db/migrate/20250701161640_add_account_status.rb new file mode 100644 index 00000000..afa4ede3 --- /dev/null +++ b/db/migrate/20250701161640_add_account_status.rb @@ -0,0 +1,41 @@ +class AddAccountStatus < ActiveRecord::Migration[7.2] + def up + add_column :accounts, :status, :string, default: "active" + change_column_null :entries, :amount, false + + # Migrate existing data + execute <<-SQL + UPDATE accounts + SET status = CASE + WHEN scheduled_for_deletion = true THEN 'pending_deletion' + WHEN is_active = true THEN 'active' + WHEN is_active = false THEN 'disabled' + ELSE 'draft' + END + SQL + + remove_column :accounts, :is_active + remove_column :accounts, :scheduled_for_deletion + end + + def down + add_column :accounts, :is_active, :boolean, default: true, null: false + add_column :accounts, :scheduled_for_deletion, :boolean, default: false + + # Restore the original boolean fields based on status + execute <<-SQL + UPDATE accounts + SET is_active = CASE + WHEN status = 'active' THEN true + WHEN status IN ('disabled', 'pending_deletion') THEN false + ELSE false + END, + scheduled_for_deletion = CASE + WHEN status = 'pending_deletion' THEN true + ELSE false + END + SQL + + remove_column :accounts, :status + end +end diff --git a/db/schema.rb b/db/schema.rb index 8a484295..f1bc9daf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2025_06_23_162207) do +ActiveRecord::Schema[7.2].define(version: 2025_07_01_161640) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -29,13 +29,12 @@ ActiveRecord::Schema[7.2].define(version: 2025_06_23_162207) do t.uuid "accountable_id" t.decimal "balance", precision: 19, scale: 4 t.string "currency" - t.boolean "is_active", default: true, null: false - t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY (ARRAY[('Loan'::character varying)::text, ('CreditCard'::character varying)::text, ('OtherLiability'::character varying)::text])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true + t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY ((ARRAY['Loan'::character varying, 'CreditCard'::character varying, 'OtherLiability'::character varying])::text[])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true t.uuid "import_id" t.uuid "plaid_account_id" - t.boolean "scheduled_for_deletion", default: false t.decimal "cash_balance", precision: 19, scale: 4, default: "0.0" t.jsonb "locked_attributes", default: {} + t.string "status", default: "active" t.index ["accountable_id", "accountable_type"], name: "index_accounts_on_accountable_id_and_accountable_type" t.index ["accountable_type"], name: "index_accounts_on_accountable_type" t.index ["family_id", "accountable_type"], name: "index_accounts_on_family_id_and_accountable_type" @@ -205,7 +204,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_06_23_162207) do t.uuid "account_id", null: false t.string "entryable_type" t.uuid "entryable_id" - t.decimal "amount", precision: 19, scale: 4 + t.decimal "amount", precision: 19, scale: 4, null: false t.string "currency" t.date "date" t.string "name", null: false @@ -216,12 +215,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_06_23_162207) do t.boolean "excluded", default: false t.string "plaid_id" t.jsonb "locked_attributes", default: {} - t.index ["account_id", "date"], name: "index_entries_on_account_id_and_date" t.index ["account_id"], name: "index_entries_on_account_id" - t.index ["amount"], name: "index_entries_on_amount" - t.index ["date"], name: "index_entries_on_date" - t.index ["entryable_id", "entryable_type"], name: "index_entries_on_entryable" - t.index ["excluded"], name: "index_entries_on_excluded" t.index ["import_id"], name: "index_entries_on_import_id" end @@ -232,7 +226,6 @@ ActiveRecord::Schema[7.2].define(version: 2025_06_23_162207) do t.date "date", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["date", "from_currency", "to_currency"], name: "index_exchange_rates_on_date_and_currencies" t.index ["from_currency", "to_currency", "date"], name: "index_exchange_rates_on_base_converted_date_unique", unique: true t.index ["from_currency"], name: "index_exchange_rates_on_from_currency" t.index ["to_currency"], name: "index_exchange_rates_on_to_currency" @@ -691,7 +684,6 @@ ActiveRecord::Schema[7.2].define(version: 2025_06_23_162207) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["tag_id"], name: "index_taggings_on_tag_id" - t.index ["taggable_id", "taggable_type"], name: "index_taggings_on_taggable_id_and_type" t.index ["taggable_type", "taggable_id"], name: "index_taggings_on_taggable" end diff --git a/test/components/previews/alert_component_preview.rb b/test/components/previews/alert_component_preview.rb new file mode 100644 index 00000000..34abcb37 --- /dev/null +++ b/test/components/previews/alert_component_preview.rb @@ -0,0 +1,7 @@ +class AlertComponentPreview < Lookbook::Preview + # @param message text + # @param variant select [info, success, warning, error] + def default(message: "This is an alert message.", variant: :info) + render AlertComponent.new(message: message, variant: variant.to_sym) + end +end diff --git a/test/controllers/api/v1/accounts_controller_test.rb b/test/controllers/api/v1/accounts_controller_test.rb index 0af38702..9d95b454 100644 --- a/test/controllers/api/v1/accounts_controller_test.rb +++ b/test/controllers/api/v1/accounts_controller_test.rb @@ -81,7 +81,7 @@ end test "should only return active accounts" do # Make one account inactive inactive_account = accounts(:depository) - inactive_account.update!(is_active: false) + inactive_account.disable! access_token = Doorkeeper::AccessToken.create!( application: @oauth_app, diff --git a/test/controllers/properties_controller_test.rb b/test/controllers/properties_controller_test.rb index 43053373..b5f1305f 100644 --- a/test/controllers/properties_controller_test.rb +++ b/test/controllers/properties_controller_test.rb @@ -8,72 +8,169 @@ class PropertiesControllerTest < ActionDispatch::IntegrationTest @account = accounts(:property) end - test "creates with property details" do - assert_difference -> { Account.count } => 1, - -> { Property.count } => 1, - -> { Valuation.count } => 2, - -> { Entry.count } => 2 do + test "creates property in draft status and redirects to balances step" do + assert_difference -> { Account.count } => 1 do post properties_path, params: { account: { - name: "Property", - balance: 500000, - currency: "USD", + name: "New Property", + subtype: "house", accountable_type: "Property", accountable_attributes: { - year_built: 2002, - area_value: 1000, - area_unit: "sqft", - address_attributes: { - line1: "123 Main St", - line2: "Apt 1", - locality: "Los Angeles", - region: "CA", # ISO3166-2 code - country: "US", # ISO3166-1 Alpha-2 code - postal_code: "90001" - } + year_built: 1990, + area_value: 1200, + area_unit: "sqft" } } } end created_account = Account.order(:created_at).last - - assert created_account.accountable.year_built.present? - assert created_account.accountable.address.line1.present? - - assert_redirected_to created_account - assert_equal "Property account created", flash[:notice] - assert_enqueued_with(job: SyncJob) + 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) end - test "updates with property details" do + test "updates property overview" do assert_no_difference [ "Account.count", "Property.count" ] do - patch account_path(@account), params: { + patch property_path(@account), params: { account: { name: "Updated Property", - balance: 500000, - currency: "USD", - accountable_type: "Property", - accountable_attributes: { - id: @account.accountable_id, - year_built: 2002, - area_value: 1000, - area_unit: "sqft", - address_attributes: { - line1: "123 Main St", - line2: "Apt 1", - locality: "Los Angeles", - region: "CA", # ISO3166-2 code - country: "US", # ISO3166-1 Alpha-2 code - postal_code: "90001" - } - } + subtype: "condo" } } end - assert_redirected_to @account - assert_equal "Property account updated", flash[:notice] - assert_enqueued_with(job: SyncJob) + @account.reload + assert_equal "Updated Property", @account.name + assert_equal "condo", @account.subtype + + # If account is active, it renders edit view; otherwise redirects to balances + if @account.active? + assert_response :success + else + assert_redirected_to balances_property_path(@account) + end + end + + # Tab view tests + test "shows balances tab" do + get balances_property_path(@account) + assert_response :success + end + + test "shows address tab" do + get address_property_path(@account) + assert_response :success + 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: { + account: { + balance: 600000, + currency: "EUR" + } + } + + # If account is active, it renders balances view; otherwise redirects to address + if @account.reload.active? + assert_response :success + else + assert_redirected_to address_property_path(@account) + end + end + + test "updates address tab" do + patch update_address_property_path(@account), params: { + property: { + address_attributes: { + line1: "456 New Street", + locality: "San Francisco", + region: "CA", + country: "US", + postal_code: "94102" + } + } + } + + @account.reload + assert_equal "456 New Street", @account.accountable.address.line1 + assert_equal "San Francisco", @account.accountable.address.locality + + # If account is draft, it activates and redirects; otherwise renders address + if @account.draft? + assert_redirected_to account_path(@account) + else + assert_response :success + 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) + + patch update_address_property_path(@account), params: { + property: { + address_attributes: { + line1: "123 Test St" + } + } + } + + assert_response :unprocessable_entity + end + + test "address update activates draft account" do + # Create a draft property account + draft_account = Account.create!( + family: @user.family, + name: "Draft Property", + accountable: Property.new, + status: "draft", + balance: 500000, + currency: "USD" + ) + + assert draft_account.draft? + + patch update_address_property_path(draft_account), params: { + property: { + address_attributes: { + line1: "789 Activate St", + locality: "New York", + region: "NY", + country: "US", + postal_code: "10001" + } + } + } + + draft_account.reload + assert draft_account.active? + assert_redirected_to account_path(draft_account) end end diff --git a/test/controllers/valuations_controller_test.rb b/test/controllers/valuations_controller_test.rb index 34b2515f..d9ee00b3 100644 --- a/test/controllers/valuations_controller_test.rb +++ b/test/controllers/valuations_controller_test.rb @@ -8,35 +8,24 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest @entry = entries(:valuation) end - test "error when valuation already exists for date" do - assert_no_difference [ "Entry.count", "Valuation.count" ] do - post valuations_url(@entry.account), params: { - entry: { - account_id: @entry.account_id, - amount: 19800, - date: @entry.date, - currency: "USD" - } - } - end - - assert_response :unprocessable_entity - end - test "creates entry with basic attributes" do + account = accounts(:investment) + assert_difference [ "Entry.count", "Valuation.count" ], 1 do post valuations_url, params: { entry: { - name: "New entry", - amount: 10000, + amount: account.balance + 100, currency: "USD", - date: Date.current, - account_id: @entry.account_id + date: Date.current.to_s, + account_id: account.id } } end created_entry = Entry.order(created_at: :desc).first + assert_equal "Manual account value update", created_entry.name + assert_equal Date.current, created_entry.date + assert_equal account.balance + 100, created_entry.amount_money.to_f assert_enqueued_with job: SyncJob @@ -47,7 +36,6 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest assert_no_difference [ "Entry.count", "Valuation.count" ] do patch valuation_url(@entry), params: { entry: { - name: "Updated entry", amount: 20000, currency: "USD", date: Date.current diff --git a/test/fixtures/accounts.yml b/test/fixtures/accounts.yml index ec8668e5..8692522e 100644 --- a/test/fixtures/accounts.yml +++ b/test/fixtures/accounts.yml @@ -5,6 +5,7 @@ other_asset: currency: USD accountable_type: OtherAsset accountable: one + status: active other_liability: family: dylan_family @@ -13,6 +14,7 @@ other_liability: currency: USD accountable_type: OtherLiability accountable: one + status: active depository: family: dylan_family @@ -21,6 +23,7 @@ depository: currency: USD accountable_type: Depository accountable: one + status: active connected: family: dylan_family @@ -31,6 +34,7 @@ connected: accountable_type: Depository accountable: two plaid_account: one + status: active credit_card: family: dylan_family @@ -39,6 +43,7 @@ credit_card: currency: USD accountable_type: CreditCard accountable: one + status: active investment: family: dylan_family @@ -48,6 +53,7 @@ investment: currency: USD accountable_type: Investment accountable: one + status: active loan: family: dylan_family @@ -56,6 +62,7 @@ loan: currency: USD accountable_type: Loan accountable: one + status: active property: family: dylan_family @@ -64,6 +71,7 @@ property: currency: USD accountable_type: Property accountable: one + status: active vehicle: family: dylan_family @@ -72,6 +80,7 @@ vehicle: currency: USD accountable_type: Vehicle accountable: one + status: active crypto: family: dylan_family @@ -80,3 +89,4 @@ crypto: currency: USD accountable_type: Crypto accountable: one + status: active diff --git a/test/interfaces/accountable_resource_interface_test.rb b/test/interfaces/accountable_resource_interface_test.rb index 551995de..ad5f5079 100644 --- a/test/interfaces/accountable_resource_interface_test.rb +++ b/test/interfaces/accountable_resource_interface_test.rb @@ -26,66 +26,4 @@ module AccountableResourceInterfaceTest assert_enqueued_with job: DestroyJob assert_equal "#{@account.accountable_name.underscore.humanize} account scheduled for deletion", flash[:notice] end - - test "updates basic account balances" do - assert_no_difference [ "Account.count", "@account.accountable_class.count" ] do - patch account_url(@account), params: { - account: { - name: "Updated name", - balance: 10000, - currency: "USD" - } - } - end - - assert_redirected_to @account - assert_equal "#{@account.accountable_name.underscore.humanize} account updated", flash[:notice] - end - - test "creates with basic attributes" do - assert_difference [ "Account.count", "@account.accountable_class.count" ], 1 do - post "/#{@account.accountable_name.pluralize}", params: { - account: { - accountable_type: @account.accountable_class, - name: "New accountable", - balance: 10000, - currency: "USD", - subtype: "checking" - } - } - end - - assert_redirected_to Account.order(:created_at).last - assert_equal "#{@account.accountable_name.humanize} account created", flash[:notice] - end - - test "updates account balance by creating new valuation if balance has changed" do - assert_difference [ "Entry.count", "Valuation.count" ], 1 do - patch account_url(@account), params: { - account: { - balance: 12000 - } - } - end - - assert_redirected_to @account - assert_enqueued_with job: SyncJob - assert_equal "#{@account.accountable_name.humanize} account updated", flash[:notice] - end - - test "updates account balance by editing existing valuation for today" do - @account.entries.create! date: Date.current, amount: 6000, currency: "USD", name: "Balance update", entryable: Valuation.new - - assert_no_difference [ "Entry.count", "Valuation.count" ] do - patch account_url(@account), params: { - account: { - balance: 12000 - } - } - end - - assert_redirected_to @account - assert_enqueued_with job: SyncJob - assert_equal "#{@account.accountable_name.humanize} account updated", flash[:notice] - end end diff --git a/test/models/account/entry_test.rb b/test/models/account/entry_test.rb index edd55e68..1cc6b478 100644 --- a/test/models/account/entry_test.rb +++ b/test/models/account/entry_test.rb @@ -67,21 +67,21 @@ class EntryTest < ActiveSupport::TestCase assert_equal 0, family.entries.search(params).size end - test "active scope only returns entries from active accounts" do + test "visible scope only returns entries from visible accounts" do # Create transactions for all account types - active_transaction = create_transaction(account: accounts(:depository), name: "Active transaction") - inactive_transaction = create_transaction(account: accounts(:credit_card), name: "Inactive transaction") + visible_transaction = create_transaction(account: accounts(:depository), name: "Visible transaction") + invisible_transaction = create_transaction(account: accounts(:credit_card), name: "Invisible transaction") # Update account statuses - accounts(:credit_card).update!(is_active: false) + accounts(:credit_card).disable! # Test the scope - active_entries = Entry.active + visible_entries = Entry.visible # Should include entry from active account - assert_includes active_entries, active_transaction + assert_includes visible_entries, visible_transaction - # Should not include entry from inactive account - assert_not_includes active_entries, inactive_transaction + # Should not include entry from disabled account + assert_not_includes visible_entries, invisible_transaction end end diff --git a/test/models/balance_sheet_test.rb b/test/models/balance_sheet_test.rb index c9478979..04906afa 100644 --- a/test/models/balance_sheet_test.rb +++ b/test/models/balance_sheet_test.rb @@ -39,7 +39,7 @@ class BalanceSheetTest < ActiveSupport::TestCase create_account(balance: 10000, accountable: Depository.new) other_liability = create_account(balance: 5000, accountable: OtherLiability.new) - other_liability.update!(is_active: false) + other_liability.disable! assert_equal 10000 - 1000, BalanceSheet.new(@family).net_worth assert_equal 10000, BalanceSheet.new(@family).assets.total diff --git a/test/models/family/auto_transfer_matchable_test.rb b/test/models/family/auto_transfer_matchable_test.rb index 6f03ad85..6fdad008 100644 --- a/test/models/family/auto_transfer_matchable_test.rb +++ b/test/models/family/auto_transfer_matchable_test.rb @@ -89,7 +89,7 @@ class Family::AutoTransferMatchableTest < ActiveSupport::TestCase end test "does not consider inactive accounts when matching transfers" do - @depository.update!(is_active: false) + @depository.disable! outflow = create_transaction(date: Date.current, account: @depository, amount: 500) inflow = create_transaction(date: Date.current, account: @credit_card, amount: -500) diff --git a/test/system/accounts_test.rb b/test/system/accounts_test.rb index c3cf8f45..e910a3ac 100644 --- a/test/system/accounts_test.rb +++ b/test/system/accounts_test.rb @@ -23,15 +23,40 @@ class AccountsTest < ApplicationSystemTestCase end test "can create property account" do - assert_account_created "Property" do - fill_in "Year built", with: 2005 - fill_in "Living area", with: 2250 - fill_in "Street address", with: "123 Main St" - fill_in "City", with: "San Francisco" - fill_in "State/Province", with: "CA" - fill_in "ZIP/Postal code", with: "94101" - fill_in "Country", with: "US" - end + # Step 1: Select property type and enter basic details + click_link "Property" + + account_name = "[system test] Property Account" + fill_in "Name*", with: account_name + select "Single Family Home", from: "Property type*" + fill_in "Year Built (optional)", with: 2005 + fill_in "Area (optional)", with: 2250 + + click_button "Next" + + # Step 2: Enter balance information + assert_text "Value" + fill_in "account[balance]", with: 500000 + click_button "Next" + + # Step 3: Enter address information + assert_text "Address" + fill_in "Address Line 1", with: "123 Main St" + fill_in "City", with: "San Francisco" + fill_in "State/Region", with: "CA" + fill_in "Postal Code", with: "94101" + fill_in "Country", with: "US" + + click_button "Save" + + # Verify account was created and is now active + assert_text account_name + + created_account = Account.order(:created_at).last + assert_equal "active", created_account.status + assert_equal 500000, created_account.balance + assert_equal "123 Main St", created_account.property.address.line1 + assert_equal "San Francisco", created_account.property.address.locality end test "can create vehicle account" do