account tab session

app/controllers/application_controller.rb

@@ -1,5 +1,8 @@
 class ApplicationController < ActionController::Base
-  include Onboardable, Localize, AutoSync, Authentication, Invitable, SelfHostable, StoreLocation, Impersonatable, Breadcrumbable, FeatureGuardable, Notifiable
+  include Onboardable, Localize, AutoSync, Authentication, Invitable,
+          SelfHostable, StoreLocation, Impersonatable, Breadcrumbable,
+          FeatureGuardable, Notifiable, AccountGroupable
+
   include Pagy::Backend
 
   before_action :detect_os

app/controllers/concerns/account_groupable.rb

@@ -0,0 +1,44 @@
+module AccountGroupable
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :set_account_group_tab
+  end
+
+  def set_account_group_tab
+    last_selected_tab = session[:account_group_tab] || "asset"
+
+    selected_tab = if account_group_tab_param
+      account_group_tab_param
+    elsif on_asset_page?
+      "asset"
+    elsif on_liability_page?
+      "liability"
+    else
+      last_selected_tab
+    end
+
+    session[:account_group_tab] = selected_tab
+    @account_group_tab = selected_tab
+  end
+
+  private
+    def account_group_tab_param
+      valid_tabs = %w[asset liability all]
+      params[:account_group_tab].in?(valid_tabs) ? params[:account_group_tab] : nil
+    end
+
+    def on_asset_page?
+      accountable_controller_names_for("asset").include?(controller_name)
+    end
+
+    def on_liability_page?
+      accountable_controller_names_for("liability").include?(controller_name)
+    end
+
+    def accountable_controller_names_for(classification)
+      Accountable::TYPES.map(&:constantize)
+                        .select { |a| a.classification == classification }
+                        .map { |a| a.model_name.plural }
+    end
+end

app/javascript/controllers/sidebar_tabs_controller.js

@@ -1,16 +0,0 @@
-import { Controller } from "@hotwired/stimulus";
-
-// Connects to data-controller="sidebar-tabs"
-export default class extends Controller {
-  static targets = ["account"];
-
-  select(event) {
-    this.accountTargets.forEach((account) => {
-      if (account.contains(event.target)) {
-        account.classList.add("bg-container");
-      } else {
-        account.classList.remove("bg-container");
-      }
-    });
-  }
-}

app/models/account.rb

@@ -61,10 +61,6 @@ class Account < ApplicationRecord
     end
   end
 
-  def syncing?
-    true
-  end
-
   def institution_domain
     url_string = plaid_account&.plaid_item&.institution_url
     return nil unless url_string.present?

app/models/account/syncer.rb

@@ -11,9 +11,8 @@ class Account::Syncer
   end
 
   def perform_post_sync
-    account.family.remove_syncing_notice!
-    account.accountable.post_sync
     account.family.auto_match_transfers!
+    account.family.broadcast_refresh
   end
 
   private

app/models/concerns/accountable.rb

@@ -68,15 +68,6 @@ module Accountable
     end
   end
 
-  def post_sync
-    broadcast_replace_to(
-      account,
-      target: "chart_account_#{account.id}",
-      partial: "accounts/show/chart",
-      locals: { account: account }
-    )
-  end
-
   def display_name
     self.class.display_name
   end

app/models/family.rb

@@ -35,10 +35,6 @@ class Family < ApplicationRecord
   validates :locale, inclusion: { in: I18n.available_locales.map(&:to_s) }
   validates :date_format, inclusion: { in: DATE_FORMATS.map(&:last) }
 
-  def remove_syncing_notice!
-    broadcast_remove target: "syncing-notice"
-  end
-
   # If any accounts or plaid items are syncing, the family is also syncing, even if a formal "Family Sync" is not running.
   def syncing?
     Sync.joins("LEFT JOIN plaid_items ON plaid_items.id = syncs.syncable_id AND syncs.syncable_type = 'PlaidItem'")

app/models/sync.rb

@@ -26,11 +26,11 @@ class Sync < ApplicationRecord
       transitions from: :pending, to: :syncing
     end
 
-    event :complete, after_commit: :handle_finalization do
+    event :complete do
       transitions from: :syncing, to: :completed
     end
 
-    event :fail, after_commit: :handle_finalization do
+    event :fail do
       transitions from: :syncing, to: :failed
     end
   end
@@ -46,23 +46,30 @@ class Sync < ApplicationRecord
     Rails.logger.tagged("Sync", id, syncable_type, syncable_id) do
       start!
 
+      sleep 10
+
       begin
         syncable.perform_sync(self)
-        attempt_finalization
       rescue => e
-        fail!
-        handle_error(e)
+        fail_and_report_error(e)
+      ensure
+        finalize_if_all_children_finalized
       end
     end
   end
 
-  # If the sync doesn't have any in-progress children, finalize it.
-  def attempt_finalization
+  # Finalizes the current sync AND parent (if it exists)
+  def finalize_if_all_children_finalized
     Sync.transaction do
       lock!
 
+      # If this is the "parent" and there are still children running, don't finalize.
       return unless all_children_finalized?
 
+      # If we make it here, the sync is finalized.  Run post-sync, regardless of failure/success.
+      perform_post_sync
+
+      if syncing?
         if has_failed_children?
           fail!
         else
@@ -71,6 +78,10 @@ class Sync < ApplicationRecord
       end
     end
 
+    # If this sync has a parent, try to finalize it so the child status propagates up the chain.
+    parent&.finalize_if_all_children_finalized
+  end
+
   private
     def log_status_change
       Rails.logger.info("changing from #{aasm.from_state} to #{aasm.to_state} (event: #{aasm.current_event})")
@@ -84,17 +95,15 @@ class Sync < ApplicationRecord
       children.incomplete.empty?
     end
 
-    # Once sync finalizes, notify its parent and run its post-sync logic.
-    def handle_finalization
+    def perform_post_sync
       syncable.perform_post_sync
-
-      if parent
-        parent.attempt_finalization
-      end
+    rescue => e
+      fail_and_report_error(e)
     end
 
-    def handle_error(error)
-      update!(error: error.message)
+    def fail_and_report_error(error)
+      fail!
+      update(error: error.message)
       Sentry.capture_exception(error) do |scope|
         scope.set_tags(sync_id: id)
       end

app/views/accounts/_account_sidebar_tabs.html.erb

@@ -1,6 +1,6 @@
-<%# locals: (family:, active_account_group_tab:) %>
+<%# locals: (family:, active_account_group_tab: "assets") %>
 
-<div>
+<div id="account-sidebar-tabs">
   <% if family.missing_data_provider? %>
     <details class="group bg-yellow-tint-10 rounded-lg p-2 text-yellow-600 mb-3 text-xs">
       <summary class="flex items-center justify-between gap-2">
@@ -21,15 +21,14 @@
     </details>
   <% end %>
 
-  <div data-controller="sidebar-tabs">
-    <%= render TabsComponent.new(active_tab: active_account_group_tab, url_param_key: "account_group_tab", testid: "account-sidebar-tabs") do |tabs| %>
+  <%= render TabsComponent.new(active_tab: active_account_group_tab, testid: "account-sidebar-tabs") do |tabs| %>
     <% tabs.with_nav do |nav| %>
-        <% nav.with_btn(id: "assets", label: "Assets") %>
-        <% nav.with_btn(id: "debts", label: "Debts") %>
+      <% nav.with_btn(id: "asset", label: "Assets") %>
+      <% nav.with_btn(id: "liability", label: "Debts") %>
       <% nav.with_btn(id: "all", label: "All") %>
     <% end %>
 
-      <% tabs.with_panel(tab_id: "assets") do %>
+    <% tabs.with_panel(tab_id: "asset") do %>
       <div class="space-y-2">
         <%= render LinkComponent.new(
           text: "New asset",
@@ -40,6 +39,7 @@
           full_width: true,
           class: "justify-start"
         ) %>
+
         <div>
           <% family.balance_sheet.account_groups("asset").each do |group| %>
             <%= render "accounts/accountable_group", account_group: group %>
@@ -48,7 +48,7 @@
       </div>
     <% end %>
 
-      <% tabs.with_panel(tab_id: "debts") do %>
+    <% tabs.with_panel(tab_id: "liability") do %>
       <div class="space-y-2">
         <%= render LinkComponent.new(
             text: "New debt",
@@ -59,6 +59,7 @@
             full_width: true,
             class: "justify-start"
           ) %>
+
         <div>
           <% family.balance_sheet.account_groups("liability").each do |group| %>
             <%= render "accounts/accountable_group", account_group: group %>
@@ -78,6 +79,7 @@
             frame: :modal,
             class: "justify-start"
           ) %>
+
         <div>
           <% family.balance_sheet.account_groups.each do |group| %>
             <%= render "accounts/accountable_group", account_group: group %>
@@ -86,5 +88,4 @@
       </div>
     <% end %>
   <% end %>
-  </div>
 </div>

app/views/accounts/_accountable_group.html.erb

@@ -28,7 +28,6 @@
                     "block flex items-center gap-2 px-3 py-2 rounded-lg",
                     page_active?(account_path(account)) ? "bg-container" : "hover:bg-surface-hover"
                   ),
-                  data: { sidebar_tabs_target: "account", action: "click->sidebar-tabs#select" },
                   title: account.name do %>
         <%= render "accounts/logo", account: account, size: "sm", color: account_group.color %>
 
@@ -37,7 +36,7 @@
           <%= tag.p account.short_subtype_label, class: "text-sm text-secondary truncate" %>
         </div>
 
-        <% if account_group.syncing? %>
+        <% if account.syncing? %>
           <div class="ml-auto text-right grow h-10">
             <div class="space-y-1">
               <div class="h-5 w-24 bg-loader rounded ml-auto"></div>

app/views/accounts/_chart_loader.html.erb

@@ -2,6 +2,6 @@
   <div class="bg-loader rounded-md h-5 w-32"></div>
 </div>
 
-<div class="p-4 h-52 flex items-center justify-center">
+<div class="p-4 h-60 flex items-center justify-center">
   <div class="bg-loader rounded-md h-full w-full"></div>
 </div>

app/views/layouts/application.html.erb

@@ -80,7 +80,7 @@
         <%= yield :sidebar %>
       <% else %>
         <div class="h-full flex flex-col">
-          <div class="overflow-y-auto grow" id="account-sidebar-tabs" data-turbo-permanent>
+          <div class="overflow-y-auto grow">
             <%= render "accounts/account_sidebar_tabs", family: Current.family, active_account_group_tab: params[:account_group_tab] || "assets" %>
           </div>
 

config/initializers/mini_profiler.rb

@@ -1,4 +1,4 @@
 Rails.application.configure do
-  Rack::MiniProfiler.config.skip_paths = [ "/design-system" ]
-  Rack::MiniProfiler.config.max_traces_to_show = 30
+  Rack::MiniProfiler.config.skip_paths = [ "/design-system", "/assets", "/cable", "/manifest", "/favicon.ico", "/hotwire-livereload", "/logo-pwa.png" ]
+  Rack::MiniProfiler.config.max_traces_to_show = 50
 end

test/models/sync_test.rb

@@ -68,8 +68,92 @@ class SyncTest < ActiveSupport::TestCase
 
     account_sync.perform
 
-    assert_equal "completed", family_sync.reload.status
     assert_equal "completed", plaid_item_sync.reload.status
     assert_equal "completed", account_sync.reload.status
+    assert_equal "completed", family_sync.reload.status
+  end
+
+  test "failures propagate up the chain" do
+    family = families(:dylan_family)
+    plaid_item = plaid_items(:one)
+    account = accounts(:connected)
+
+    family_sync = Sync.create!(syncable: family)
+    plaid_item_sync = Sync.create!(syncable: plaid_item, parent: family_sync)
+    account_sync = Sync.create!(syncable: account, parent: plaid_item_sync)
+
+    assert_equal "pending", family_sync.status
+    assert_equal "pending", plaid_item_sync.status
+    assert_equal "pending", account_sync.status
+
+    family.expects(:perform_sync).with(family_sync).once
+
+    family_sync.perform
+
+    assert_equal "syncing", family_sync.reload.status
+
+    plaid_item.expects(:perform_sync).with(plaid_item_sync).once
+
+    plaid_item_sync.perform
+
+    assert_equal "syncing", family_sync.reload.status
+    assert_equal "syncing", plaid_item_sync.reload.status
+
+    # This error should "bubble up" to the PlaidItem and Family sync results
+    account.expects(:perform_sync).with(account_sync).raises(StandardError.new("test account sync error"))
+
+    # Since these are accessed through `parent`, they won't necessarily be the same
+    # instance we configured above
+    Account.any_instance.expects(:perform_post_sync).once
+    PlaidItem.any_instance.expects(:perform_post_sync).once
+    Family.any_instance.expects(:perform_post_sync).once
+
+    account_sync.perform
+
+    assert_equal "failed", plaid_item_sync.reload.status
+    assert_equal "failed", account_sync.reload.status
+    assert_equal "failed", family_sync.reload.status
+  end
+
+  test "parent failure should not change status if child succeeds" do
+    family = families(:dylan_family)
+    plaid_item = plaid_items(:one)
+    account = accounts(:connected)
+
+    family_sync = Sync.create!(syncable: family)
+    plaid_item_sync = Sync.create!(syncable: plaid_item, parent: family_sync)
+    account_sync = Sync.create!(syncable: account, parent: plaid_item_sync)
+
+    assert_equal "pending", family_sync.status
+    assert_equal "pending", plaid_item_sync.status
+    assert_equal "pending", account_sync.status
+
+    family.expects(:perform_sync).with(family_sync).raises(StandardError.new("test family sync error"))
+
+    family_sync.perform
+
+    assert_equal "failed", family_sync.reload.status
+
+    plaid_item.expects(:perform_sync).with(plaid_item_sync).raises(StandardError.new("test plaid item sync error"))
+
+    plaid_item_sync.perform
+
+    assert_equal "failed", family_sync.reload.status
+    assert_equal "failed", plaid_item_sync.reload.status
+
+    # Leaf level sync succeeds, but shouldn't change the status of the already-failed parent syncs
+    account.expects(:perform_sync).with(account_sync).once
+
+    # Since these are accessed through `parent`, they won't necessarily be the same
+    # instance we configured above
+    Account.any_instance.expects(:perform_post_sync).once
+    PlaidItem.any_instance.expects(:perform_post_sync).once
+    Family.any_instance.expects(:perform_post_sync).once
+
+    account_sync.perform
+
+    assert_equal "failed", plaid_item_sync.reload.status
+    assert_equal "failed", family_sync.reload.status
+    assert_equal "completed", account_sync.reload.status
   end
 end