diff --git a/app/components/settings/health_summary.html.erb b/app/components/settings/health_summary.html.erb index 646c89521..7112e9f61 100644 --- a/app/components/settings/health_summary.html.erb +++ b/app/components/settings/health_summary.html.erb @@ -1,4 +1,4 @@ -
+
<% tiles.each do |tile| %>
<%= tile[:count] %>
diff --git a/app/components/settings/provider_card.html.erb b/app/components/settings/provider_card.html.erb index 2a286a8bc..50ab60d58 100644 --- a/app/components/settings/provider_card.html.erb +++ b/app/components/settings/provider_card.html.erb @@ -1,7 +1,7 @@
- <%= logo_text %> + <%= logo_text %>
diff --git a/app/components/settings/provider_card.rb b/app/components/settings/provider_card.rb index 2f9e2bc5e..1898352d1 100644 --- a/app/components/settings/provider_card.rb +++ b/app/components/settings/provider_card.rb @@ -1,5 +1,8 @@ class Settings::ProviderCard < ApplicationComponent - MATURITY_LABELS = { beta: "Beta", alpha: "Alpha" }.freeze + MATURITY_LABELS = { + beta: "settings.providers.maturity.beta", + alpha: "settings.providers.maturity.alpha" + }.freeze def initialize(provider_key:, name:, tagline: nil, region: nil, kind: nil, tier: nil, maturity: :stable, logo_bg: "bg-gray-500", logo_text: nil) @@ -15,7 +18,8 @@ class Settings::ProviderCard < ApplicationComponent end def maturity_label - MATURITY_LABELS[@maturity] + label_key = MATURITY_LABELS[@maturity] + t(label_key) if label_key end def meta_line diff --git a/app/controllers/settings/providers_controller.rb b/app/controllers/settings/providers_controller.rb index 3549f6321..9cd689d51 100644 --- a/app/controllers/settings/providers_controller.rb +++ b/app/controllers/settings/providers_controller.rb @@ -79,12 +79,17 @@ class Settings::ProvidersController < ApplicationController def sync_all family = Current.family + now = Time.current - if family.last_sync_all_attempted_at.present? && family.last_sync_all_attempted_at > 30.seconds.ago + updated_count = Family + .where(id: family.id) + .where("last_sync_all_attempted_at IS NULL OR last_sync_all_attempted_at <= ?", 30.seconds.ago) + .update_all(last_sync_all_attempted_at: now, updated_at: now) + + if updated_count.zero? return redirect_to settings_providers_path, notice: t("settings.providers.sync_all_recently") end - family.update!(last_sync_all_attempted_at: Time.current) SyncAllProvidersJob.perform_later(family.id) redirect_to settings_providers_path, notice: t("settings.providers.sync_all_in_progress") end @@ -141,7 +146,9 @@ class Settings::ProvidersController < ApplicationController end def ensure_admin - redirect_to settings_providers_path, alert: "Not authorized" unless Current.user.admin? + return if Current.user.admin? + + redirect_to root_path, alert: t("settings.providers.not_authorized") end # Reload provider configurations after settings update diff --git a/app/helpers/settings_helper.rb b/app/helpers/settings_helper.rb index 3e73933b9..ea2418834 100644 --- a/app/helpers/settings_helper.rb +++ b/app/helpers/settings_helper.rb @@ -2,7 +2,7 @@ module SettingsHelper SETTINGS_ORDER = [ # General section { name: "Accounts", path: :accounts_path }, - { name: "Bank Sync", path: :settings_providers_path }, + { name: "Bank Sync", path: :settings_providers_path, condition: :admin_user? }, { name: "Preferences", path: :settings_preferences_path }, { name: "Appearance", path: :settings_appearance_path }, { name: "Profile Info", path: :settings_profile_path }, @@ -49,6 +49,19 @@ module SettingsHelper render partial: "settings/section", locals: { title: title, subtitle: subtitle, content: content, collapsible: collapsible, open: open, auto_open_param: auto_open_param, status: status, meta: meta, actions: actions, badge: badge } end + def status_pill_classes(status) + case status.to_s.to_sym + when :ok + { dot: "bg-success", pill: "bg-success/10 text-success" } + when :warn + { dot: "bg-warning", pill: "bg-warning/10 text-warning" } + when :err + { dot: "bg-destructive", pill: "bg-destructive/10 text-destructive" } + else + { dot: "bg-gray-400", pill: "bg-gray-100 text-secondary" } + end + end + def provider_summary(provider_key) key = provider_key.to_s.downcase diff --git a/app/jobs/sync_all_providers_job.rb b/app/jobs/sync_all_providers_job.rb index c13e6c151..431b77cad 100644 --- a/app/jobs/sync_all_providers_job.rb +++ b/app/jobs/sync_all_providers_job.rb @@ -3,7 +3,7 @@ class SyncAllProvidersJob < ApplicationJob sidekiq_options lock: :until_executed, lock_args: ->(args) { [ args.first ] }, on_conflict: :log def perform(family_id) - family = Family.find(family_id) - family.sync_later + family = Family.find_by(id: family_id) + family&.sync_later end end diff --git a/app/models/family/syncer.rb b/app/models/family/syncer.rb index 3eace0b06..6b909ebcb 100644 --- a/app/models/family/syncer.rb +++ b/app/models/family/syncer.rb @@ -17,6 +17,7 @@ class Family::Syncer coinbase_items coinstats_items mercury_items + binance_items snaptrade_items sophtron_items ].freeze diff --git a/app/views/settings/_section.html.erb b/app/views/settings/_section.html.erb index fcbe77f00..0c05833e8 100644 --- a/app/views/settings/_section.html.erb +++ b/app/views/settings/_section.html.erb @@ -21,7 +21,7 @@ <% if meta.present? %> <%= meta %> <% end %> - <%= render "settings/providers/status_pill", status: status %> + <%= status %> <%= actions if actions.present? %>
<% end %> diff --git a/app/views/settings/_settings_nav.html.erb b/app/views/settings/_settings_nav.html.erb index ce249174a..94df67ae2 100644 --- a/app/views/settings/_settings_nav.html.erb +++ b/app/views/settings/_settings_nav.html.erb @@ -4,7 +4,7 @@ nav_sections = [ header: t(".general_section_title"), items: [ { label: t(".accounts_label"), path: accounts_path, icon: "layers" }, - { label: t(".bank_sync_label"), path: settings_providers_path, icon: "banknote" }, + { label: t(".bank_sync_label"), path: settings_providers_path, icon: "banknote", if: Current.user&.admin? }, { label: t(".preferences_label"), path: settings_preferences_path, icon: "bolt" }, { label: t(".appearance_label"), path: settings_appearance_path, icon: "palette" }, { label: t(".profile_label"), path: settings_profile_path, icon: "circle-user" }, diff --git a/app/views/settings/providers/_group_heading.html.erb b/app/views/settings/providers/_group_heading.html.erb index 5b234c4f2..e7246c4c2 100644 --- a/app/views/settings/providers/_group_heading.html.erb +++ b/app/views/settings/providers/_group_heading.html.erb @@ -1,5 +1,5 @@ <%# locals: (title:, count: nil, description: nil, anchor: nil) %> -
+<%= tag.div id: anchor.presence, class: "flex items-baseline justify-between gap-3 mt-2 mb-1 px-1" do %>

<%= title %> <% if count %> @@ -9,4 +9,4 @@ <% if description.present? %>

<%= description %>

<% end %> -

+<% end %> diff --git a/app/views/settings/providers/_status_pill.html.erb b/app/views/settings/providers/_status_pill.html.erb index 7b436bf1f..1f46b9216 100644 --- a/app/views/settings/providers/_status_pill.html.erb +++ b/app/views/settings/providers/_status_pill.html.erb @@ -1,12 +1,6 @@ <%# locals: (status:) %> -<% - dot_class, pill_class = case status.to_sym - when :ok then [ "bg-success", "bg-success/10 text-success" ] - when :warn then [ "bg-warning", "bg-warning/10 text-warning" ] - when :err then [ "bg-destructive", "bg-destructive/10 text-destructive" ] - else [ "bg-gray-400", "bg-gray-100 text-secondary" ] - end -%> +<% classes = status_pill_classes(status) %> +<% dot_class, pill_class = classes[:dot], classes[:pill] %> <%= t("settings.providers.status.#{status}") %> diff --git a/app/views/settings/providers/_sync_button.html.erb b/app/views/settings/providers/_sync_button.html.erb index 039fcb626..ba97b2ce1 100644 --- a/app/views/settings/providers/_sync_button.html.erb +++ b/app/views/settings/providers/_sync_button.html.erb @@ -1,9 +1,11 @@ <%# locals: (provider_key:, last_synced_at: nil) %> <% recently_synced = last_synced_at.present? && last_synced_at > 60.seconds.ago %> +<% button_label = recently_synced ? t("settings.providers.recently_synced") : t("settings.providers.sync_provider") %> <%= button_to sync_provider_settings_providers_path(provider_key: provider_key), method: :post, disabled: recently_synced, - title: recently_synced ? t("settings.providers.recently_synced") : t("settings.providers.sync_provider"), + title: button_label, + aria: { label: button_label }, class: "inline-flex items-center p-1 rounded text-secondary hover:text-primary hover:bg-alpha-black-50 transition-colors disabled:opacity-40 disabled:cursor-not-allowed", form: { onclick: "event.stopPropagation()", class: "inline-flex" } do %> <%= icon "refresh-cw", class: "w-3.5 h-3.5" %> diff --git a/app/views/settings/providers/show.html.erb b/app/views/settings/providers/show.html.erb index b0a2e06e4..0b4ae09c7 100644 --- a/app/views/settings/providers/show.html.erb +++ b/app/views/settings/providers/show.html.erb @@ -1,4 +1,4 @@ -<%= content_for :page_title, "Bank Sync" %> +<%= content_for :page_title, t("settings.providers.bank_sync.page_title") %>
<% if @encryption_error %> @@ -13,9 +13,7 @@
<% else %>
-

- Connect external accounts so transactions, balances and holdings flow into Sure automatically. -

+

<%= t("settings.providers.bank_sync.lede") %>

<% if @connected.any? || @needs_attention.any? %> <% sync_all_disabled = Current.family.last_sync_all_attempted_at.present? && Current.family.last_sync_all_attempted_at > 30.seconds.ago %> <%= button_to sync_all_settings_providers_path, @@ -44,13 +42,15 @@ <% all_connections.each do |entry| %> <% auto_open = [ :warn, :err ].include?(entry[:summary][:status]) || all_connections.size == 1 %> <% sync_action = entry[:partial].present? ? render("settings/providers/sync_button", provider_key: entry[:provider_key], last_synced_at: entry[:summary][:last_synced_at]) : nil %> - <% maturity_label = Settings::ProviderCard::MATURITY_LABELS[entry[:maturity]] %> + <% maturity_label_key = Settings::ProviderCard::MATURITY_LABELS[entry[:maturity]] %> + <% maturity_label = maturity_label_key ? t(maturity_label_key) : nil %> <% maturity_badge = maturity_label ? content_tag(:span, maturity_label, class: "text-xs font-medium px-1.5 py-0.5 rounded-full bg-alpha-black-50 text-secondary") : nil %> + <% status_pill = render("settings/providers/status_pill", status: entry[:summary][:status]) %> <%= settings_section title: entry[:title], collapsible: true, open: auto_open, auto_open_param: entry[:auto_open_param], - status: entry[:summary][:status], + status: status_pill, meta: entry[:summary][:meta], actions: sync_action, badge: maturity_badge do %> diff --git a/config/locales/views/settings/en.yml b/config/locales/views/settings/en.yml index c47fd2444..028cb2f79 100644 --- a/config/locales/views/settings/en.yml +++ b/config/locales/views/settings/en.yml @@ -186,11 +186,18 @@ en: choose_label: (optional) change: Change photo providers: + not_authorized: Not authorized + bank_sync: + page_title: Bank Sync + lede: Connect external accounts so transactions, balances and holdings flow into Sure automatically. status: ok: Connected warn: Action needed err: Error off: Not configured + maturity: + beta: Beta + alpha: Alpha connect: Connect groups: your_connections: Your connections diff --git a/test/controllers/settings/providers_controller_test.rb b/test/controllers/settings/providers_controller_test.rb index 3a4ab28ee..00b98e893 100644 --- a/test/controllers/settings/providers_controller_test.rb +++ b/test/controllers/settings/providers_controller_test.rb @@ -325,6 +325,17 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest assert_match(/Syncing all connected providers/i, response.body) end + test "POST sync_all respects recent sync throttle" do + families(:dylan_family).update_column(:last_sync_all_attempted_at, Time.current) + + assert_no_enqueued_jobs only: SyncAllProvidersJob do + post sync_all_settings_providers_path + end + + assert_redirected_to settings_providers_path + assert_equal I18n.t("settings.providers.sync_all_recently"), flash[:notice] + end + test "POST sync for simplefin without an active Simplefin sync enqueues SyncJob" do item = SimplefinItem.create!( family: families(:dylan_family), @@ -352,7 +363,7 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest setting: { plaid_client_id: "test" } } - assert_redirected_to settings_providers_path + assert_redirected_to root_path assert_equal "Not authorized", flash[:alert] # Value should not have changed diff --git a/test/models/family/syncer_test.rb b/test/models/family/syncer_test.rb index baada992e..1a2aa33e5 100644 --- a/test/models/family/syncer_test.rb +++ b/test/models/family/syncer_test.rb @@ -5,11 +5,12 @@ class Family::SyncerTest < ActiveSupport::TestCase @family = families(:dylan_family) end - test "syncs plaid items and manual accounts" do + test "syncs provider items and manual accounts" do family_sync = syncs(:family) manual_accounts_count = @family.accounts.manual.count - items_count = @family.plaid_items.count + plaid_items_count = @family.plaid_items.count + binance_items_count = @family.binance_items.syncable.count syncer = Family::Syncer.new(@family) @@ -19,9 +20,14 @@ class Family::SyncerTest < ActiveSupport::TestCase .times(manual_accounts_count) PlaidItem.any_instance - .expects(:sync_later) - .with(parent_sync: family_sync, window_start_date: nil, window_end_date: nil) - .times(items_count) + .expects(:sync_later) + .with(parent_sync: family_sync, window_start_date: nil, window_end_date: nil) + .times(plaid_items_count) + + BinanceItem.any_instance + .expects(:sync_later) + .with(parent_sync: family_sync, window_start_date: nil, window_end_date: nil) + .times(binance_items_count) syncer.perform_sync(family_sync) @@ -61,6 +67,7 @@ class Family::SyncerTest < ActiveSupport::TestCase LunchflowItem.any_instance.stubs(:sync_later) EnableBankingItem.any_instance.stubs(:sync_later) SophtronItem.any_instance.stubs(:sync_later) + BinanceItem.any_instance.stubs(:sync_later) syncer.perform_sync(family_sync) syncer.perform_post_sync diff --git a/test/system/settings_test.rb b/test/system/settings_test.rb index 941d1bd02..68b85f297 100644 --- a/test/system/settings_test.rb +++ b/test/system/settings_test.rb @@ -6,8 +6,12 @@ class SettingsTest < ApplicationSystemTestCase # Base settings available to all users @settings_links = [ - [ "Accounts", accounts_path ], - [ "Bank Sync", settings_providers_path ], + [ "Accounts", accounts_path ] + ] + + @settings_links << [ "Bank Sync", settings_providers_path ] if @user.admin? + + @settings_links += [ [ "Preferences", settings_preferences_path ], [ "Profile Info", settings_profile_path ], [ "Security", settings_security_path ], @@ -87,6 +91,7 @@ class SettingsTest < ApplicationSystemTestCase # Assert that admin-only settings are not present in the navigation assert_no_selector "li", text: "AI Prompts" assert_no_selector "li", text: "API Key" + assert_no_selector "li", text: "Bank Sync" end end