diff --git a/app/views/accounts/show/_chart.html.erb b/app/components/UI/account/chart.html.erb similarity index 51% rename from app/views/accounts/show/_chart.html.erb rename to app/components/UI/account/chart.html.erb index 00506be2..ff54a578 100644 --- a/app/views/accounts/show/_chart.html.erb +++ b/app/components/UI/account/chart.html.erb @@ -1,32 +1,28 @@ -<%# locals: (account:, tooltip: nil, chart_view: nil, **args) %> - -<% period = @period || Period.last_30_days %> -<% default_value_title = account.asset? ? t(".balance") : t(".owed") %> -
- <%= tag.p account.investment? ? "Total value" : default_value_title, class: "text-sm font-medium text-secondary" %> + <%= tag.p title, class: "text-sm font-medium text-secondary" %> <% if account.investment? %> - <%= render "investments/value_tooltip", balance: account.balance_money, holdings: account.balance_money - account.cash_balance_money, cash: account.cash_balance_money %> + <%= render "investments/value_tooltip", balance: account.balance_money, holdings: holdings_value_money, cash: account.cash_balance_money %> <% end %>
- <%= tag.p format_money(account.balance_money), class: "text-primary text-3xl font-medium truncate" %> - <% if account.currency != Current.family.currency %> - <%= tag.p format_money(account.balance_money.exchange_to(Current.family.currency, fallback_rate: 1)), class: "text-sm font-medium text-secondary" %> + <%= tag.p view_balance_money.format, class: "text-primary text-3xl font-medium truncate" %> + + <% if converted_balance_money %> + <%= tag.p converted_balance_money.format, class: "text-sm font-medium text-secondary" %> <% end %>
- <%= form_with url: request.path, method: :get, data: { controller: "auto-submit-form" } do |form| %> + <%= form_with url: account_path(account), method: :get, data: { controller: "auto-submit-form" } do |form| %>
- <% if chart_view.present? %> + <% if account.investment? %> <%= form.select :chart_view, [["Total value", "balance"], ["Holdings", "holdings_balance"], ["Cash", "cash_balance"]], - { selected: chart_view }, + { selected: view }, class: "bg-container border border-secondary rounded-lg text-sm pr-7 cursor-pointer text-primary focus:outline-hidden focus:ring-0", data: { "auto-submit-form-target": "auto" } %> <% end %> @@ -40,7 +36,23 @@ <% end %>
- <%= turbo_frame_tag dom_id(account, :chart_details), src: chart_account_path(account, period: period.key, chart_view: chart_view) do %> - <%= render "accounts/chart_loader" %> + <%= turbo_frame_tag dom_id(@account, :chart_details) do %> +
+ <%= render partial: "shared/trend_change", locals: { trend: trend, comparison_label: period.comparison_label } %> +
+ +
+ <% if series.any? %> +
+ <% else %> +
+

No data available

+
+ <% end %> +
<% end %>
diff --git a/app/components/UI/account/chart.rb b/app/components/UI/account/chart.rb new file mode 100644 index 00000000..1e58529a --- /dev/null +++ b/app/components/UI/account/chart.rb @@ -0,0 +1,72 @@ +class UI::Account::Chart < ApplicationComponent + attr_reader :account + + def initialize(account:, period: nil, view: nil) + @account = account + @period = period + @view = view + end + + def period + @period ||= Period.last_30_days + end + + def holdings_value_money + account.balance_money - account.cash_balance_money + end + + def view_balance_money + case view + when "balance" + account.balance_money + when "holdings_balance" + holdings_value_money + when "cash_balance" + account.cash_balance_money + end + end + + def title + case account.accountable_type + when "Investment", "Crypto" + case view + when "balance" + "Total account value" + when "holdings_balance" + "Holdings value" + when "cash_balance" + "Cash value" + end + when "Property", "Vehicle" + "Estimated #{account.accountable_type.humanize.downcase} value" + when "CreditCard", "OtherLiability" + "Debt balance" + when "Loan" + "Remaining principal balance" + else + "Balance" + end + end + + def foreign_currency? + account.currency != account.family.currency + end + + def converted_balance_money + return nil unless foreign_currency? + + account.balance_money.exchange_to(account.family.currency, fallback_rate: 1) + end + + def view + @view ||= "balance" + end + + def series + account.balance_series(period: period, view: view) + end + + def trend + series.trend + end +end diff --git a/app/components/UI/account_page.html.erb b/app/components/UI/account_page.html.erb new file mode 100644 index 00000000..7b02d30f --- /dev/null +++ b/app/components/UI/account_page.html.erb @@ -0,0 +1,29 @@ +<%= turbo_stream_from account %> + +<%= turbo_frame_tag dom_id(account, :container) do %> + <%= tag.div class: "space-y-4 pb-32" do %> + <%= render "accounts/show/header", account: account, title: title, subtitle: subtitle %> + + <%= render UI::Account::Chart.new(account: account, period: chart_period, view: chart_view) %> + +
+ <% if tabs.count > 1 %> + <%= render TabsComponent.new(active_tab: active_tab, url_param_key: "tab") do |tabs_container| %> + <% tabs_container.with_nav(classes: "max-w-fit") do |nav| %> + <% tabs.each do |tab| %> + <% nav.with_btn(id: tab, label: tab.to_s.humanize, classes: "px-6") %> + <% end %> + <% end %> + + <% tabs.each do |tab| %> + <% tabs_container.with_panel(tab_id: tab) do %> + <%= render tab_partial_name(tab), account: account %> + <% end %> + <% end %> + <% end %> + <% else %> + <%= render tab_partial_name(tabs.first), account: account %> + <% end %> +
+ <% end %> +<% end %> diff --git a/app/components/UI/account_page.rb b/app/components/UI/account_page.rb new file mode 100644 index 00000000..012f9240 --- /dev/null +++ b/app/components/UI/account_page.rb @@ -0,0 +1,45 @@ +class UI::AccountPage < ApplicationComponent + attr_reader :account, :chart_view, :chart_period + + def initialize(account:, chart_view: nil, chart_period: nil, active_tab: nil) + @account = account + @chart_view = chart_view + @chart_period = chart_period + @active_tab = active_tab + end + + def title + account.name + end + + def subtitle + return nil unless account.property? + + account.property.address + end + + def active_tab + tabs.find { |tab| tab == @active_tab } || tabs.first + end + + def tabs + case account.accountable_type + when "Investment" + [ :activity, :holdings ] + when "Property", "Vehicle", "Loan" + [ :activity, :overview ] + else + [ :activity ] + end + end + + def tab_partial_name(tab) + case tab + when :activity + "accounts/show/activity" + when :holdings, :overview + # Accountable is responsible for implementing the partial in the correct folder + "#{account.accountable_type.downcase.pluralize}/tabs/#{tab}" + end + end +end diff --git a/app/components/application_component.rb b/app/components/application_component.rb new file mode 100644 index 00000000..37cb953d --- /dev/null +++ b/app/components/application_component.rb @@ -0,0 +1,4 @@ +class ApplicationComponent < ViewComponent::Base + # These don't work as expected with helpers.turbo_frame_tag, etc., so we include them here + include Turbo::FramesHelper, Turbo::StreamsHelper +end diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index ff532cfb..16f9e853 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 toggle_active] + before_action :set_account, only: %i[sync sparkline toggle_active show destroy] include Periodable def index @@ -25,11 +25,6 @@ class AccountsController < ApplicationController redirect_to account_path(@account) end - def chart - @chart_view = params[:chart_view] || "balance" - render layout: "application" - end - def sparkline etag_key = @account.family.build_cache_key("#{@account.id}_sparkline", invalidate_on_data_updates: true) @@ -50,6 +45,15 @@ class AccountsController < ApplicationController redirect_to accounts_path end + def destroy + if @account.linked? + redirect_to account_path(@account), alert: "Cannot delete a linked account" + else + @account.destroy_later + redirect_to accounts_path, notice: "Account scheduled for deletion" + end + end + private def family Current.family diff --git a/app/controllers/concerns/accountable_resource.rb b/app/controllers/concerns/accountable_resource.rb index 8bd624dc..3b06ff16 100644 --- a/app/controllers/concerns/accountable_resource.rb +++ b/app/controllers/concerns/accountable_resource.rb @@ -4,7 +4,7 @@ module AccountableResource included do include Periodable - before_action :set_account, only: [ :show, :edit, :update, :destroy ] + before_action :set_account, only: [ :show, :edit, :update ] before_action :set_link_options, only: :new end @@ -61,16 +61,7 @@ module AccountableResource end @account.lock_saved_attributes! - redirect_back_or_to @account, notice: t("accounts.update.success", type: accountable_type.name.underscore.humanize) - end - - def destroy - if @account.linked? - redirect_to account_path(@account), alert: "Cannot delete a linked account" - else - @account.destroy_later - redirect_to accounts_path, notice: t("accounts.destroy.success", type: accountable_type.name.underscore.humanize) - end + redirect_back_or_to account_path(@account), notice: t("accounts.update.success", type: accountable_type.name.underscore.humanize) end private diff --git a/app/views/accounts/chart.html.erb b/app/views/accounts/chart.html.erb deleted file mode 100644 index 11dcbaac..00000000 --- a/app/views/accounts/chart.html.erb +++ /dev/null @@ -1,22 +0,0 @@ -<% series = @account.balance_series(period: @period, view: @chart_view) %> -<% trend = series.trend %> - -<%= turbo_frame_tag dom_id(@account, :chart_details) do %> -
- <%= render partial: "shared/trend_change", locals: { trend: trend, comparison_label: @period.comparison_label } %> -
- -
- <% if series.any? %> -
- <% else %> -
-

<%= t(".data_not_available") %>

-
- <% end %> -
-<% end %> diff --git a/app/views/accounts/show.html.erb b/app/views/accounts/show.html.erb index 1d9e7e19..69ce322d 100644 --- a/app/views/accounts/show.html.erb +++ b/app/views/accounts/show.html.erb @@ -1,25 +1,6 @@ -<%= turbo_stream_from @account %> - -<%= turbo_frame_tag dom_id(@account, :container) do %> - <%= tag.div class: "space-y-4 pb-32" do %> - <% if header.present? %> - <%= header %> - <% else %> - <%= render "accounts/show/header", account: account %> - <% end %> - - <% if chart.present? %> - <%= chart %> - <% else %> - <%= render "accounts/show/chart", account: account, chart_view: chart_view %> - <% end %> - -
- <% if tabs.present? %> - <%= tabs %> - <% else %> - <%= render "accounts/show/activity", account: account %> - <% end %> -
- <% end %> -<% end %> +<%= render UI::AccountPage.new( + account: @account, + chart_view: @chart_view, + chart_period: @period, + active_tab: @active_tab + ) %> diff --git a/app/views/accounts/show/_header.html.erb b/app/views/accounts/show/_header.html.erb index 51d3b253..06bcc3e2 100644 --- a/app/views/accounts/show/_header.html.erb +++ b/app/views/accounts/show/_header.html.erb @@ -1,36 +1,30 @@ -<%# locals: (account:, title: nil, subtitle: nil) %> +<%# locals: (account:, title:, subtitle: nil) %>
- <% content = yield %> +
+ <%= render "accounts/logo", account: account %> - <% if content.present? %> - <%= content %> - <% else %> -
- <%= render "accounts/logo", account: account %> - -
-
-
-

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

- <% if account.draft? %> - <%= render LinkComponent.new( +
+
+
+

"><%= title %>

+ <% 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 %>
+ <% if subtitle.present? %> +

<%= subtitle %>

+ <% end %>
- <% end %> +
<% if Rails.env.development? || self_hosted? %> diff --git a/app/views/credit_cards/show.html.erb b/app/views/credit_cards/show.html.erb deleted file mode 100644 index 4ba5252b..00000000 --- a/app/views/credit_cards/show.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -<%= render "accounts/show/template", - account: @account, - tabs: render("accounts/show/tabs", account: @account, tabs: [ - { key: "activity", contents: render("accounts/show/activity", account: @account) }, - { key: "overview", contents: render("credit_cards/overview", account: @account) }, - ]) %> diff --git a/app/views/cryptos/show.html.erb b/app/views/cryptos/show.html.erb deleted file mode 100644 index 0c3e6e97..00000000 --- a/app/views/cryptos/show.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render "accounts/show/template", account: @account %> diff --git a/app/views/depositories/show.html.erb b/app/views/depositories/show.html.erb deleted file mode 100644 index 0c3e6e97..00000000 --- a/app/views/depositories/show.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render "accounts/show/template", account: @account %> diff --git a/app/views/investments/show.html.erb b/app/views/investments/show.html.erb deleted file mode 100644 index 84be8629..00000000 --- a/app/views/investments/show.html.erb +++ /dev/null @@ -1,7 +0,0 @@ -<%= render "accounts/show/template", - account: @account, - chart_view: @chart_view, - tabs: render("accounts/show/tabs", account: @account, tabs: [ - { key: "activity", contents: render("accounts/show/activity", account: @account) }, - { key: "holdings", contents: render("investments/holdings_tab", account: @account) }, - ]) %> diff --git a/app/views/investments/_holdings_tab.html.erb b/app/views/investments/tabs/_holdings.html.erb similarity index 100% rename from app/views/investments/_holdings_tab.html.erb rename to app/views/investments/tabs/_holdings.html.erb diff --git a/app/views/loans/show.html.erb b/app/views/loans/show.html.erb deleted file mode 100644 index 55bffa52..00000000 --- a/app/views/loans/show.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -<%= render "accounts/show/template", - account: @account, - tabs: render("accounts/show/tabs", account: @account, tabs: [ - { key: "activity", contents: render("accounts/show/activity", account: @account) }, - { key: "overview", contents: render("loans/overview", account: @account) }, - ]) %> diff --git a/app/views/loans/_overview.html.erb b/app/views/loans/tabs/_overview.html.erb similarity index 100% rename from app/views/loans/_overview.html.erb rename to app/views/loans/tabs/_overview.html.erb diff --git a/app/views/other_assets/show.html.erb b/app/views/other_assets/show.html.erb deleted file mode 100644 index 0c3e6e97..00000000 --- a/app/views/other_assets/show.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render "accounts/show/template", account: @account %> diff --git a/app/views/other_liabilities/show.html.erb b/app/views/other_liabilities/show.html.erb deleted file mode 100644 index 0c3e6e97..00000000 --- a/app/views/other_liabilities/show.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render "accounts/show/template", account: @account %> diff --git a/app/views/properties/show.html.erb b/app/views/properties/show.html.erb deleted file mode 100644 index e6f8c34f..00000000 --- a/app/views/properties/show.html.erb +++ /dev/null @@ -1,7 +0,0 @@ -<%= render "accounts/show/template", - account: @account, - header: render("accounts/show/header", account: @account, subtitle: @account.property.address), - tabs: render("accounts/show/tabs", account: @account, tabs: [ - { key: "activity", contents: render("accounts/show/activity", account: @account) }, - { key: "overview", contents: render("properties/overview", account: @account) }, - ]) %> diff --git a/app/views/properties/_overview.html.erb b/app/views/properties/tabs/_overview.html.erb similarity index 100% rename from app/views/properties/_overview.html.erb rename to app/views/properties/tabs/_overview.html.erb diff --git a/app/views/vehicles/show.html.erb b/app/views/vehicles/show.html.erb deleted file mode 100644 index 555cd926..00000000 --- a/app/views/vehicles/show.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -<%= render "accounts/show/template", - account: @account, - tabs: render("accounts/show/tabs", account: @account, tabs: [ - { key: "activity", contents: render("accounts/show/activity", account: @account) }, - { key: "overview", contents: render("vehicles/overview", account: @account) }, - ]) %> diff --git a/app/views/vehicles/_overview.html.erb b/app/views/vehicles/tabs/_overview.html.erb similarity index 100% rename from app/views/vehicles/_overview.html.erb rename to app/views/vehicles/tabs/_overview.html.erb diff --git a/config/routes.rb b/config/routes.rb index 3dd1d7f1..0c89bb0b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -150,10 +150,9 @@ Rails.application.routes.draw do end end - resources :accounts, only: %i[index new], shallow: true do + resources :accounts, only: %i[index new show destroy], shallow: true do member do post :sync - get :chart get :sparkline patch :toggle_active end @@ -161,17 +160,13 @@ Rails.application.routes.draw do # 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 do + resources :depositories, except: [ :index, :show ] + resources :investments, except: [ :index, :show ] + resources :properties, except: [ :index, :show ] do member do get :balances patch :update_balances @@ -180,12 +175,12 @@ Rails.application.routes.draw do patch :update_address end end - resources :vehicles, except: :index - resources :credit_cards, except: :index - resources :loans, except: :index - resources :cryptos, except: :index - resources :other_assets, except: :index - resources :other_liabilities, except: :index + resources :vehicles, except: [ :index, :show ] + resources :credit_cards, except: [ :index, :show ] + resources :loans, except: [ :index, :show ] + resources :cryptos, except: [ :index, :show ] + resources :other_assets, except: [ :index, :show ] + resources :other_liabilities, except: [ :index, :show ] resources :securities, only: :index diff --git a/test/controllers/accounts_controller_test.rb b/test/controllers/accounts_controller_test.rb index ba0b937e..ec26ef49 100644 --- a/test/controllers/accounts_controller_test.rb +++ b/test/controllers/accounts_controller_test.rb @@ -11,18 +11,25 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest assert_response :success end + test "should get show" do + get account_url(@account) + assert_response :success + end + test "should sync account" do post sync_account_url(@account) assert_redirected_to account_url(@account) end - test "should get chart" do - get chart_account_url(@account) - assert_response :success - end - test "should get sparkline" do get sparkline_account_url(@account) assert_response :success end + + test "destroys account" do + delete account_url(@account) + assert_redirected_to accounts_path + assert_enqueued_with job: DestroyJob + assert_equal "Account scheduled for deletion", flash[:notice] + end end diff --git a/test/controllers/credit_cards_controller_test.rb b/test/controllers/credit_cards_controller_test.rb index 5fb0ec52..d19db651 100644 --- a/test/controllers/credit_cards_controller_test.rb +++ b/test/controllers/credit_cards_controller_test.rb @@ -48,7 +48,7 @@ class CreditCardsControllerTest < ActionDispatch::IntegrationTest test "updates with credit card details" do assert_no_difference [ "Account.count", "CreditCard.count" ] do - patch account_path(@account), params: { + patch credit_card_path(@account), params: { account: { name: "Updated Credit Card", balance: 2000, diff --git a/test/controllers/loans_controller_test.rb b/test/controllers/loans_controller_test.rb index e12a2705..47809400 100644 --- a/test/controllers/loans_controller_test.rb +++ b/test/controllers/loans_controller_test.rb @@ -46,7 +46,7 @@ class LoansControllerTest < ActionDispatch::IntegrationTest test "updates with loan details" do assert_no_difference [ "Account.count", "Loan.count" ] do - patch account_path(@account), params: { + patch loan_path(@account), params: { account: { name: "Updated Loan", balance: 45000, diff --git a/test/controllers/vehicles_controller_test.rb b/test/controllers/vehicles_controller_test.rb index 37cea18d..55aa89cf 100644 --- a/test/controllers/vehicles_controller_test.rb +++ b/test/controllers/vehicles_controller_test.rb @@ -45,7 +45,7 @@ class VehiclesControllerTest < ActionDispatch::IntegrationTest test "updates with vehicle details" do assert_no_difference [ "Account.count", "Vehicle.count" ] do - patch account_path(@account), params: { + patch vehicle_path(@account), params: { account: { name: "Updated Vehicle", balance: 28000, @@ -64,7 +64,7 @@ class VehiclesControllerTest < ActionDispatch::IntegrationTest } end - assert_redirected_to @account + assert_redirected_to account_path(@account) assert_equal "Vehicle account updated", flash[:notice] assert_enqueued_with(job: SyncJob) end diff --git a/test/interfaces/accountable_resource_interface_test.rb b/test/interfaces/accountable_resource_interface_test.rb index ad5f5079..ef318cb0 100644 --- a/test/interfaces/accountable_resource_interface_test.rb +++ b/test/interfaces/accountable_resource_interface_test.rb @@ -15,15 +15,4 @@ module AccountableResourceInterfaceTest assert_response :success end - test "renders accountable page" do - get account_url(@account) - assert_response :success - end - - test "destroys account" do - delete account_url(@account) - assert_redirected_to accounts_path - assert_enqueued_with job: DestroyJob - assert_equal "#{@account.accountable_name.underscore.humanize} account scheduled for deletion", flash[:notice] - end end