Address provider settings review feedback

This commit is contained in:
Juan José Mata
2026-05-09 10:27:33 +02:00
parent fff5c97c56
commit 60b2f2b1ce
17 changed files with 88 additions and 37 deletions

View File

@@ -1,4 +1,4 @@
<div class="grid grid-cols-4 gap-2">
<div class="grid grid-cols-2 sm:grid-cols-4 gap-2">
<% tiles.each do |tile| %>
<div class="bg-container shadow-border-xs rounded-xl p-3 text-center">
<div class="text-2xl font-bold tabular-nums <%= tile[:color_class] %>"><%= tile[:count] %></div>

View File

@@ -1,7 +1,7 @@
<div class="bg-container shadow-border-xs rounded-xl p-4 flex flex-col gap-3 hover:shadow-border-sm transition-shadow">
<div class="flex items-start gap-3">
<div class="w-9 h-9 rounded-lg flex items-center justify-center shrink-0 <%= logo_bg %>">
<span class="text-xs font-bold text-white"><%= logo_text %></span>
<span class="text-xs font-bold text-inverse"><%= logo_text %></span>
</div>
<div class="flex-1 min-w-0">
<div class="flex items-center gap-2 flex-wrap">

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -17,6 +17,7 @@ class Family::Syncer
coinbase_items
coinstats_items
mercury_items
binance_items
snaptrade_items
sophtron_items
].freeze

View File

@@ -21,7 +21,7 @@
<% if meta.present? %>
<span class="text-xs text-subdued"><%= meta %></span>
<% end %>
<%= render "settings/providers/status_pill", status: status %>
<%= status %>
<%= actions if actions.present? %>
</div>
<% end %>

View File

@@ -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" },

View File

@@ -1,5 +1,5 @@
<%# locals: (title:, count: nil, description: nil, anchor: nil) %>
<div id="<%= anchor %>" class="flex items-baseline justify-between gap-3 mt-2 mb-1 px-1">
<%= tag.div id: anchor.presence, class: "flex items-baseline justify-between gap-3 mt-2 mb-1 px-1" do %>
<h2 class="text-sm font-medium text-primary flex items-baseline gap-2">
<%= title %>
<% if count %>
@@ -9,4 +9,4 @@
<% if description.present? %>
<p class="text-xs text-secondary"><%= description %></p>
<% end %>
</div>
<% end %>

View File

@@ -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] %>
<span class="inline-flex items-center gap-1.5 px-2 py-0.5 rounded-full text-xs font-medium <%= pill_class %>">
<span class="w-1.5 h-1.5 rounded-full flex-shrink-0 <%= dot_class %>"></span>
<%= t("settings.providers.status.#{status}") %>

View File

@@ -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" %>

View File

@@ -1,4 +1,4 @@
<%= content_for :page_title, "Bank Sync" %>
<%= content_for :page_title, t("settings.providers.bank_sync.page_title") %>
<div class="space-y-4">
<% if @encryption_error %>
@@ -13,9 +13,7 @@
</div>
<% else %>
<div class="flex items-start justify-between gap-4">
<p class="text-secondary">
Connect external accounts so transactions, balances and holdings flow into Sure automatically.
</p>
<p class="text-secondary"><%= t("settings.providers.bank_sync.lede") %></p>
<% 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 %>

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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