QT/sure
1
0
Fork 0
mirror of https://github.com/we-promise/sure.git synced 2026-04-19 12:04:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fixes & Improvements (#316)

Browse Source 
* Some improvements

- Fix issue with lunch flow accounts that were imported
- Remove the period comparison section from reports

* Add cleanup migration

* FIX for dynamic config

* Fix linter

* FIX settings setter

Reuse the base class’ atomic setter to leverage its locking and cache invalidation.

* Make upsert atomic

* Remove migration file

Signed-off-by: soky srm <sokysrm@gmail.com>

* Delete db/migrate/20251111094448_migrate_dynamic_fields_to_individual_entries.rb

Signed-off-by: soky srm <sokysrm@gmail.com>

* Fix cache reset

* Revert "Remove migration file"

This reverts commit 1f2a21ef58.

* Revert "Delete db/migrate/20251111094448_migrate_dynamic_fields_to_individual_entries.rb"

This reverts commit 29dcaaafb2.

* Fix Plaid initialiser

---------

Signed-off-by: soky srm <sokysrm@gmail.com>
This commit is contained in:
soky srm
2025-11-11 19:51:07 +01:00
committed by GitHub
parent 7851e965ea
commit fad241c416
19 changed files with 215 additions and 391 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
app/controllers/reports_controller.rb
View File

@@ -25,9 +25,6 @@ class ReportsController < ApplicationController
# Calculate summary metrics
@summary_metrics = build_summary_metrics
# Build comparison data
@comparison_data = build_comparison_data
# Build trend data (last 6 months)
@trends_data = build_trends_data
@@ -195,25 +192,6 @@ class ReportsController < ApplicationController
nil
end
def build_comparison_data
currency_symbol = Money::Currency.new(Current.family.currency).symbol
# Totals are BigDecimal amounts in dollars - pass directly to Money.new()
{
current: {
income: @current_income_totals.total,
expenses: @current_expense_totals.total,
net: @current_income_totals.total - @current_expense_totals.total
},
previous: {
income: @previous_income_totals.total,
expenses: @previous_expense_totals.total,
net: @previous_income_totals.total - @previous_expense_totals.total
},
currency_symbol: currency_symbol
}
end
def build_trends_data
# Generate month-by-month data based on the current period filter
trends = []

28
app/controllers/settings/providers_controller.rb
View File

@@ -28,9 +28,6 @@ class Settings::ProvidersController < ApplicationController
updated_fields = []
# This hash will store only the updates for dynamic (non-declared) fields
dynamic_updates = {}
# Perform all updates within a transaction for consistency
Setting.transaction do
provider_params.each do |param_key, param_value|
@@ -57,32 +54,13 @@ class Settings::ProvidersController < ApplicationController
# This is safe and uses the proper setter.
Setting.public_send("#{key_str}=", value)
else
# If it's a dynamic field, add it to our batch hash
# to avoid the Read-Modify-Write conflict.
dynamic_updates[key_str] = value
# If it's a dynamic field, set it as an individual entry
# Each field is stored independently, preventing race conditions
Setting[key_str] = value
end
updated_fields << param_key
end
# Now, if we have any dynamic updates, apply them all at once
if dynamic_updates.any?
# 1. READ the current hash once
current_dynamic = Setting.dynamic_fields.dup
# 2. MODIFY by merging changes
# Treat nil values as deletions to keep the hash clean
dynamic_updates.each do |key, value|
if value.nil?
current_dynamic.delete(key)
else
current_dynamic[key] = value
end
end
# 3. WRITE the complete, merged hash back once
Setting.dynamic_fields = current_dynamic
end
end
if updated_fields.any?

8
app/models/lunchflow_item/importer.rb
View File

@@ -29,8 +29,12 @@ class LunchflowItem::Importer
accounts_failed = 0
if accounts_data[:accounts].present?
# Get all existing lunchflow account IDs for this item (normalize to strings for comparison)
existing_account_ids = lunchflow_item.lunchflow_accounts.pluck(:account_id).map(&:to_s)
# Get only linked lunchflow account IDs (ones actually imported/used by the user)
# This prevents updating orphaned accounts from old behavior that saved everything
existing_account_ids = lunchflow_item.lunchflow_accounts
.joins(:account_provider)
.pluck(:account_id)
.map(&:to_s)
accounts_data[:accounts].each do |account_data|
account_id = account_data[:id]&.to_s

8
app/models/provider/configurable.rb
View File

@@ -1,8 +1,8 @@
# Module for providers to declare their configuration requirements
#
# Providers can declare their own configuration fields without needing to modify
# the Setting model. Settings are stored dynamically using RailsSettings::Base's
# hash-style access (Setting[:key] = value).
# the Setting model. Settings are stored dynamically as individual entries using
# RailsSettings::Base's bracket-style access (Setting[:key] = value).
#
# Configuration fields are automatically registered and displayed in the UI at
# /settings/providers. The system checks Setting storage first, then ENV variables,
@@ -186,8 +186,8 @@ module Provider::Configurable
# Get the value for this field (Setting -> ENV -> default)
def value
# First try Setting using dynamic hash-style access
# This works even without explicit field declarations in Setting model
# First try Setting using dynamic bracket-style access
# Each field is stored as an individual entry without explicit field declarations
setting_value = Setting[setting_key]
return normalize_value(setting_value) if setting_value.present?

43
app/models/setting.rb
View File

@@ -11,9 +11,8 @@ class Setting < RailsSettings::Base
field :openai_model, type: :string, default: ENV["OPENAI_MODEL"]
field :brand_fetch_client_id, type: :string, default: ENV["BRAND_FETCH_CLIENT_ID"]
# Single hash field for all dynamic provider credentials and other dynamic settings
# This allows unlimited dynamic fields without declaring them upfront
field :dynamic_fields, type: :hash, default: {}
# Dynamic fields are now stored as individual entries with "dynamic:" prefix
# This prevents race conditions and ensures each field is independently managed
# Onboarding and app settings
ONBOARDING_STATES = %w[open closed invite_only].freeze
@@ -50,7 +49,7 @@ class Setting < RailsSettings::Base
end
# Support dynamic field access via bracket notation
# First checks if it's a declared field, then falls back to dynamic_fields hash
# First checks if it's a declared field, then falls back to individual dynamic entries
def [](key)
key_str = key.to_s
@@ -58,8 +57,8 @@ class Setting < RailsSettings::Base
if respond_to?(key_str)
public_send(key_str)
else
# Fall back to dynamic_fields hash
dynamic_fields[key_str]
# Fall back to individual dynamic entry lookup
find_by(var: dynamic_key_name(key_str))&.value
end
end
@@ -70,21 +69,26 @@ class Setting < RailsSettings::Base
if respond_to?("#{key_str}=")
public_send("#{key_str}=", value)
else
# Otherwise, manage in dynamic_fields hash
current_dynamic = dynamic_fields.dup
# Store as individual dynamic entry
dynamic_key = dynamic_key_name(key_str)
if value.nil?
current_dynamic.delete(key_str) # treat nil as delete
where(var: dynamic_key).destroy_all
clear_cache
else
current_dynamic[key_str] = value
# Use upsert for atomic insert/update to avoid race conditions
upsert({ var: dynamic_key, value: value.to_yaml }, unique_by: :var)
clear_cache
end
self.dynamic_fields = current_dynamic # persists & busts cache
end
end
# Check if a dynamic field exists (useful to distinguish nil value vs missing key)
def key?(key)
key_str = key.to_s
respond_to?(key_str) || dynamic_fields.key?(key_str)
return true if respond_to?(key_str)
# Check if dynamic entry exists
where(var: dynamic_key_name(key_str)).exists?
end
# Delete a dynamic field
@@ -92,16 +96,23 @@ class Setting < RailsSettings::Base
key_str = key.to_s
return nil if respond_to?(key_str) # Can't delete declared fields
current_dynamic = dynamic_fields.dup
value = current_dynamic.delete(key_str)
self.dynamic_fields = current_dynamic
dynamic_key = dynamic_key_name(key_str)
value = self[key_str]
where(var: dynamic_key).destroy_all
clear_cache
value
end
# List all dynamic field keys (excludes declared fields)
def dynamic_keys
dynamic_fields.keys
where("var LIKE ?", "dynamic:%").pluck(:var).map { |var| var.sub(/^dynamic:/, "") }
end
private
def dynamic_key_name(key_str)
"dynamic:#{key_str}"
end
end
# Validates OpenAI configuration requires model when custom URI base is set

4
app/views/lunchflow_items/select_accounts.html.erb
View File

@@ -16,10 +16,10 @@
<div class="space-y-2">
<% @available_accounts.each do |account| %>
<% has_blank_name = account[:name].blank? %>
<label class="flex items-start gap-3 p-3 border <%= has_blank_name ? 'border-error bg-error/5' : 'border-primary' %> rounded-lg <%= has_blank_name ? 'cursor-not-allowed opacity-60' : 'hover:bg-subtle cursor-pointer' %> transition-colors">
<label class="flex items-start gap-3 p-3 border <%= has_blank_name ? "border-error bg-error/5" : "border-primary" %> rounded-lg <%= has_blank_name ? "cursor-not-allowed opacity-60" : "hover:bg-subtle cursor-pointer" %> transition-colors">
<%= check_box_tag "account_ids[]", account[:id], false, disabled: has_blank_name, class: "mt-1" %>
<div class="flex-1">
<div class="font-medium text-sm <%= has_blank_name ? 'text-error' : 'text-primary' %>">
<div class="font-medium text-sm <%= has_blank_name ? "text-error" : "text-primary" %>">
<% if has_blank_name %>
<%= t(".no_name_placeholder") %>
<% else %>

4
app/views/lunchflow_items/select_existing_account.html.erb
View File

@@ -16,10 +16,10 @@
<div class="space-y-2">
<% @available_accounts.each do |account| %>
<% has_blank_name = account[:name].blank? %>
<label class="flex items-start gap-3 p-3 border <%= has_blank_name ? 'border-error bg-error/5' : 'border-primary' %> rounded-lg <%= has_blank_name ? 'cursor-not-allowed opacity-60' : 'hover:bg-subtle cursor-pointer' %> transition-colors">
<label class="flex items-start gap-3 p-3 border <%= has_blank_name ? "border-error bg-error/5" : "border-primary" %> rounded-lg <%= has_blank_name ? "cursor-not-allowed opacity-60" : "hover:bg-subtle cursor-pointer" %> transition-colors">
<%= radio_button_tag "lunchflow_account_id", account[:id], false, disabled: has_blank_name, class: "mt-1" %>
<div class="flex-1">
<div class="font-medium text-sm <%= has_blank_name ? 'text-error' : 'text-primary' %>">
<div class="font-medium text-sm <%= has_blank_name ? "text-error" : "text-primary" %>">
<% if has_blank_name %>
<%= t(".no_name_placeholder") %>
<% else %>

250
app/views/reports/_comparison_chart.html.erb
View File

@@ -1,250 +0,0 @@
<%
currency = Current.family.currency
# Helper to calculate percentage change and determine if it's good or bad
def comparison_class(current, previous, inverse: false)
return "text-primary" if previous.zero?
change = current - previous
is_positive_change = change > 0
# For expenses, lower is better (inverse logic)
is_good = inverse ? !is_positive_change : is_positive_change
is_good ? "text-green-600" : "text-gray-600"
end
def percentage_change(current, previous)
return 0 if previous.zero?
((current - previous) / previous.abs * 100).round(1)
end
%>
<div>
<div class="flex items-center justify-between mb-6">
<h2 class="text-lg font-medium text-primary">
<%= t("reports.comparison.title") %>
</h2>
<p class="text-sm text-tertiary">
<%= t("reports.comparison.currency", symbol: comparison_data[:currency_symbol]) %>
</p>
</div>
<div class="space-y-6">
<%# Income Comparison %>
<div>
<div class="flex items-center justify-between mb-3">
<h3 class="text-sm font-medium text-secondary flex items-center gap-2">
<%= icon("trending-up", class: "w-4 h-4 text-success") %>
<%= t("reports.comparison.income") %>
</h3>
</div>
<div class="flex items-center justify-between mb-2">
<div class="flex items-center gap-3">
<span class="text-2xl font-semibold <%= comparison_class(comparison_data[:current][:income], comparison_data[:previous][:income]) %>">
<%= Money.new(comparison_data[:current][:income], currency).format %>
</span>
<% change = percentage_change(comparison_data[:current][:income], comparison_data[:previous][:income]) %>
<% if change != 0 %>
<% income_improved = comparison_data[:current][:income] > comparison_data[:previous][:income] %>
<div class="flex items-center gap-1.5">
<span class="text-sm font-medium <%= comparison_class(comparison_data[:current][:income], comparison_data[:previous][:income]) %>">
<%= change >= 0 ? "+" : "" %><%= change %>%
</span>
<span class="inline-flex items-center gap-1 px-2 py-0.5 rounded-full text-xs font-medium <%= income_improved ? 'bg-green-100 text-green-700' : 'bg-gray-100 text-gray-600' %>">
<%= icon(income_improved ? "trending-up" : "trending-down", class: "w-3 h-3") %>
<%= t(income_improved ? "reports.comparison.status.improved" : "reports.comparison.status.decreased") %>
</span>
</div>
<% end %>
</div>
<span class="text-sm text-tertiary">
<%= t("reports.comparison.previous") %>: <%= Money.new(comparison_data[:previous][:income], currency).format %>
</span>
</div>
<%# Overlapping bars %>
<div class="relative h-10">
<%
current_income_abs = comparison_data[:current][:income].to_f.abs
previous_income_abs = comparison_data[:previous][:income].to_f.abs
max_income = [current_income_abs, previous_income_abs].max
if max_income > 0
current_width = [3, (current_income_abs / max_income * 100)].max
previous_width = [3, (previous_income_abs / max_income * 100)].max
else
current_width = 0
previous_width = 0
end
# Income: green if increased, gray/primary if decreased
income_increased = comparison_data[:current][:income] >= comparison_data[:previous][:income]
income_bar_color = income_increased ? "bg-green-500" : "bg-gray-600"
income_bg_color = income_increased ? "bg-green-200" : "bg-gray-300"
%>
<% if previous_width > 0 || current_width > 0 %>
<%# Previous period bar (background) %>
<% if previous_width > 0 %>
<div class="absolute top-0 left-0 h-10 <%= income_bg_color %> rounded-lg transition-all duration-500"
style="width: <%= previous_width %>%"></div>
<% end %>
<%# Current period bar (foreground) %>
<% if current_width > 0 %>
<div class="absolute top-2 left-0 h-6 <%= income_bar_color %> rounded-lg transition-all duration-500"
style="width: <%= current_width %>%"></div>
<% end %>
<% else %>
<div class="flex items-center justify-center h-10 text-sm text-tertiary">
<%= t("reports.comparison.no_data") %>
</div>
<% end %>
</div>
</div>
<%# Expenses Comparison %>
<div>
<div class="flex items-center justify-between mb-3">
<h3 class="text-sm font-medium text-secondary flex items-center gap-2">
<%= icon("trending-down", class: "w-4 h-4 text-danger") %>
<%= t("reports.comparison.expenses") %>
</h3>
</div>
<div class="flex items-center justify-between mb-2">
<div class="flex items-center gap-3">
<span class="text-2xl font-semibold <%= comparison_class(comparison_data[:current][:expenses], comparison_data[:previous][:expenses], inverse: true) %>">
<%= Money.new(comparison_data[:current][:expenses], currency).format %>
</span>
<% change = percentage_change(comparison_data[:current][:expenses], comparison_data[:previous][:expenses]) %>
<% if change != 0 %>
<% expenses_improved = comparison_data[:current][:expenses] < comparison_data[:previous][:expenses] %>
<div class="flex items-center gap-1.5">
<span class="text-sm font-medium <%= comparison_class(comparison_data[:current][:expenses], comparison_data[:previous][:expenses], inverse: true) %>">
<%= change >= 0 ? "+" : "" %><%= change %>%
</span>
<span class="inline-flex items-center gap-1 px-2 py-0.5 rounded-full text-xs font-medium <%= expenses_improved ? 'bg-green-100 text-green-700' : 'bg-gray-100 text-gray-600' %>">
<%= icon(expenses_improved ? "trending-down" : "trending-up", class: "w-3 h-3") %>
<%= t(expenses_improved ? "reports.comparison.status.reduced" : "reports.comparison.status.increased") %>
</span>
</div>
<% end %>
</div>
<span class="text-sm text-tertiary">
<%= t("reports.comparison.previous") %>: <%= Money.new(comparison_data[:previous][:expenses], currency).format %>
</span>
</div>
<%# Overlapping bars %>
<div class="relative h-10">
<%
current_expenses_abs = comparison_data[:current][:expenses].to_f.abs
previous_expenses_abs = comparison_data[:previous][:expenses].to_f.abs
max_expenses = [current_expenses_abs, previous_expenses_abs].max
if max_expenses > 0
current_width = [3, (current_expenses_abs / max_expenses * 100)].max
previous_width = [3, (previous_expenses_abs / max_expenses * 100)].max
else
current_width = 0
previous_width = 0
end
# Expenses: green if decreased (inverse logic), gray/primary if increased
expenses_decreased = comparison_data[:current][:expenses] <= comparison_data[:previous][:expenses]
expenses_bar_color = expenses_decreased ? "bg-green-500" : "bg-gray-600"
expenses_bg_color = expenses_decreased ? "bg-green-200" : "bg-gray-300"
%>
<% if previous_width > 0 || current_width > 0 %>
<%# Previous period bar (background) %>
<% if previous_width > 0 %>
<div class="absolute top-0 left-0 h-10 <%= expenses_bg_color %> rounded-lg transition-all duration-500"
style="width: <%= previous_width %>%"></div>
<% end %>
<%# Current period bar (foreground) %>
<% if current_width > 0 %>
<div class="absolute top-2 left-0 h-6 <%= expenses_bar_color %> rounded-lg transition-all duration-500"
style="width: <%= current_width %>%"></div>
<% end %>
<% else %>
<div class="flex items-center justify-center h-10 text-sm text-tertiary">
<%= t("reports.comparison.no_data") %>
</div>
<% end %>
</div>
</div>
<%# Net Savings Comparison %>
<div>
<div class="flex items-center justify-between mb-3">
<h3 class="text-sm font-medium text-secondary flex items-center gap-2">
<%= icon("piggy-bank", class: "w-4 h-4 text-primary") %>
<%= t("reports.comparison.net_savings") %>
</h3>
</div>
<div class="flex items-center justify-between mb-2">
<div class="flex items-center gap-3">
<span class="text-2xl font-semibold <%= comparison_class(comparison_data[:current][:net], comparison_data[:previous][:net]) %>">
<%= Money.new(comparison_data[:current][:net], currency).format %>
</span>
<% change = percentage_change(comparison_data[:current][:net], comparison_data[:previous][:net]) %>
<% if change != 0 %>
<% net_improved = comparison_data[:current][:net] > comparison_data[:previous][:net] %>
<div class="flex items-center gap-1.5">
<span class="text-sm font-medium <%= comparison_class(comparison_data[:current][:net], comparison_data[:previous][:net]) %>">
<%= change >= 0 ? "+" : "" %><%= change %>%
</span>
<span class="inline-flex items-center gap-1 px-2 py-0.5 rounded-full text-xs font-medium <%= net_improved ? 'bg-green-100 text-green-700' : 'bg-gray-100 text-gray-600' %>">
<%= icon(net_improved ? "trending-up" : "trending-down", class: "w-3 h-3") %>
<%= t(net_improved ? "reports.comparison.status.improved" : "reports.comparison.status.decreased") %>
</span>
</div>
<% end %>
</div>
<span class="text-sm text-tertiary">
<%= t("reports.comparison.previous") %>: <%= Money.new(comparison_data[:previous][:net], currency).format %>
</span>
</div>
<%# Overlapping bars %>
<div class="relative h-10">
<%
current_net_abs = comparison_data[:current][:net].to_f.abs
previous_net_abs = comparison_data[:previous][:net].to_f.abs
max_net = [current_net_abs, previous_net_abs].max
if max_net > 0
current_width = [3, (current_net_abs / max_net * 100)].max
previous_width = [3, (previous_net_abs / max_net * 100)].max
else
current_width = 0
previous_width = 0
end
# Net Savings: green if improved (increased), gray/primary if got worse
net_improved = comparison_data[:current][:net] >= comparison_data[:previous][:net]
net_bar_color = net_improved ? "bg-green-500" : "bg-gray-600"
net_bg_color = net_improved ? "bg-green-200" : "bg-gray-300"
%>
<% if previous_width > 0 || current_width > 0 %>
<%# Previous period bar (background) %>
<% if previous_width > 0 %>
<div class="absolute top-0 left-0 h-10 <%= net_bg_color %> rounded-lg transition-all duration-500"
style="width: <%= previous_width %>%"></div>
<% end %>
<%# Current period bar (foreground) %>
<% if current_width > 0 %>
<div class="absolute top-2 left-0 h-6 <%= net_bar_color %> rounded-lg transition-all duration-500"
style="width: <%= current_width %>%"></div>
<% end %>
<% else %>
<div class="flex items-center justify-center h-10 text-sm text-tertiary">
<%= t("reports.comparison.no_data") %>
</div>
<% end %>
</div>
</div>
</div>
</div>

9
app/views/reports/index.html.erb
View File

@@ -87,15 +87,6 @@
} %>
</section>
<%# Comparison Chart %>
<section class="bg-container py-4 px-6 rounded-xl shadow-border-xs">
<%= render partial: "reports/comparison_chart", locals: {
comparison_data: @comparison_data,
period_type: @period_type,
start_date: @start_date
} %>
</section>
<%# Trends & Insights %>
<section class="bg-container py-4 px-6 rounded-xl shadow-border-xs">
<%= render partial: "reports/trends_insights", locals: {

2
app/views/settings/providers/_provider_form.html.erb
View File

@@ -73,7 +73,7 @@
</div>
<% end %>
<% # Show configuration status %>
<%# Show configuration status %>
<% if configuration.configured? %>
<div class="flex items-center gap-2 mt-4">
<div class="w-2 h-2 bg-green-500 rounded-full"></div>

6
config/initializers/plaid.rb
View File

@@ -6,10 +6,16 @@ end
# Load Plaid configuration from adapters after initialization
Rails.application.config.after_initialize do
# Skip if database is not ready (e.g., during db:create)
next unless ActiveRecord::Base.connection.table_exists?("settings")
# Ensure provider adapters are loaded
Provider::Factory.ensure_adapters_loaded
# Reload configurations from settings/ENV
Provider::PlaidAdapter.reload_configuration # US region
Provider::PlaidEuAdapter.reload_configuration # EU region
rescue ActiveRecord::NoDatabaseError, PG::ConnectionBad
# Database doesn't exist yet, skip initialization
nil
end

14
config/locales/views/reports/en.yml
View File

@@ -24,20 +24,6 @@ en:
income_minus_expenses: Income minus expenses
of_budget_used: of budget used
no_budget_data: No budget data for this period
comparison:
title: Period Comparison
currency: "Currency: %{symbol}"
income: Income
expenses: Expenses
net_savings: Net Savings
current: Current Period
previous: Previous Period
no_data: No data available
status:
improved: Improved
decreased: Decreased
reduced: Reduced
increased: Increased
budget_performance:
title: Budget Performance
spent: Spent

28
db/migrate/20251111084519_remove_orphaned_lunchflow_accounts.rb Normal file
View File

@@ -0,0 +1,28 @@
class RemoveOrphanedLunchflowAccounts < ActiveRecord::Migration[7.2]
def up
# Find all LunchflowAccount records that don't have an associated account_provider
# These are "orphaned" accounts that were created during sync but never actually
# imported/linked by the user due to old behavior that saved all accounts
orphaned_accounts = LunchflowAccount.left_outer_joins(:account_provider)
.where(account_providers: { id: nil })
orphaned_count = orphaned_accounts.count
if orphaned_count > 0
Rails.logger.info "Removing #{orphaned_count} orphaned LunchflowAccount records (not linked via account_provider)"
# Delete orphaned accounts
orphaned_accounts.destroy_all
Rails.logger.info "Successfully removed #{orphaned_count} orphaned LunchflowAccount records"
else
Rails.logger.info "No orphaned LunchflowAccount records found to remove"
end
end
def down
# Cannot restore orphaned accounts that were deleted
# These were unused accounts from old behavior that shouldn't have been created
Rails.logger.info "Cannot restore orphaned LunchflowAccount records (data migration is irreversible)"
end
end

35
db/migrate/20251111094448_migrate_dynamic_fields_to_individual_entries.rb Normal file
View File

@@ -0,0 +1,35 @@
class MigrateDynamicFieldsToIndividualEntries < ActiveRecord::Migration[7.2]
def up
# Find the dynamic_fields setting record
dynamic_fields_record = Setting.find_by(var: "dynamic_fields")
return unless dynamic_fields_record
# Parse the hash and create individual entries
dynamic_fields_hash = dynamic_fields_record.value || {}
dynamic_fields_hash.each do |key, value|
Setting.create!(
var: "dynamic:#{key}",
value: value
)
end
# Delete the old dynamic_fields record
dynamic_fields_record.destroy
end
def down
# Collect all dynamic: entries back into a hash
dynamic_fields_hash = {}
Setting.where("var LIKE ?", "dynamic:%").find_each do |record|
key = record.var.sub(/^dynamic:/, "")
dynamic_fields_hash[key] = record.value
record.destroy
end
# Recreate the dynamic_fields record with the hash
Setting.create!(
var: "dynamic_fields",
value: dynamic_fields_hash
) if dynamic_fields_hash.any?
end
end

6
db/schema.rb generated
View File

@@ -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_11_10_104411) do
ActiveRecord::Schema[7.2].define(version: 2025_11_11_094448) do
# These are extensions that must be enabled in order to support this database
enable_extension "pgcrypto"
enable_extension "plpgsql"
@@ -687,6 +687,10 @@ ActiveRecord::Schema[7.2].define(version: 2025_11_10_104411) do
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.string "name"
t.boolean "manual", default: false, null: false
t.decimal "expected_amount_min", precision: 19, scale: 4
t.decimal "expected_amount_max", precision: 19, scale: 4
t.decimal "expected_amount_avg", precision: 19, scale: 4
t.index ["family_id", "merchant_id", "amount", "currency"], name: "idx_recurring_txns_merchant", unique: true, where: "(merchant_id IS NOT NULL)"
t.index ["family_id", "name", "amount", "currency"], name: "idx_recurring_txns_name", unique: true, where: "((name IS NOT NULL) AND (merchant_id IS NULL))"
t.index ["family_id", "status"], name: "index_recurring_transactions_on_family_id_and_status"

8
test/controllers/reports_controller_test.rb
View File

@@ -76,14 +76,6 @@ class ReportsControllerTest < ActionDispatch::IntegrationTest
assert_select "h3", text: I18n.t("reports.summary.net_savings")
end
test "index builds comparison data" do
get reports_path(period_type: :monthly)
assert_response :ok
assert_select "h2", text: I18n.t("reports.comparison.title")
assert_select "h3", text: I18n.t("reports.comparison.income")
assert_select "h3", text: I18n.t("reports.comparison.expenses")
end
test "index builds trends data" do
get reports_path(period_type: :monthly)
assert_response :ok

47
test/controllers/settings/providers_controller_test.rb
View File

@@ -41,10 +41,10 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
end
test "updates dynamic provider fields using batch update" do
# plaid_client_id is a dynamic field, so it should go through dynamic_fields hash
# plaid_client_id is a dynamic field, stored as an individual entry
with_self_hosting do
# Clear any existing plaid settings
Setting.dynamic_fields = {}
Setting["plaid_client_id"] = nil
patch settings_providers_url, params: {
setting: { plaid_client_id: "test_client_id" }
@@ -52,14 +52,16 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to settings_providers_url
assert_equal "test_client_id", Setting["plaid_client_id"]
assert_equal "test_client_id", Setting.dynamic_fields["plaid_client_id"]
end
end
test "batches multiple dynamic fields from same provider atomically" do
# Test that multiple fields from Plaid are updated together in one write operation
# Test that multiple fields from Plaid are updated as individual entries
with_self_hosting do
Setting.dynamic_fields = {}
# Clear existing fields
Setting["plaid_client_id"] = nil
Setting["plaid_secret"] = nil
Setting["plaid_environment"] = nil
patch settings_providers_url, params: {
setting: {
@@ -71,7 +73,7 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to settings_providers_url
# All three should be present in dynamic_fields
# All three should be present as individual entries
assert_equal "new_client_id", Setting["plaid_client_id"]
assert_equal "new_secret", Setting["plaid_secret"]
assert_equal "production", Setting["plaid_environment"]
@@ -79,9 +81,14 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
end
test "batches dynamic fields from multiple providers atomically" do
# Test that fields from different providers are all batched together
# Test that fields from different providers are stored as individual entries
with_self_hosting do
Setting.dynamic_fields = {}
# Clear existing fields
Setting["plaid_client_id"] = nil
Setting["plaid_secret"] = nil
Setting["plaid_eu_client_id"] = nil
Setting["plaid_eu_secret"] = nil
Setting["simplefin_setup_token"] = nil
patch settings_providers_url, params: {
setting: {
@@ -108,10 +115,8 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
# Test that updating some fields doesn't overwrite other existing fields
with_self_hosting do
# Set initial fields
Setting.dynamic_fields = {
"existing_field_1" => "value1",
"plaid_client_id" => "old_client_id"
}
Setting["existing_field_1"] = "value1"
Setting["plaid_client_id"] = "old_client_id"
# Update one field and add a new one
patch settings_providers_url, params: {
@@ -162,7 +167,7 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
# Set initial values
Setting["plaid_client_id"] = "old_value"
assert_equal "old_value", Setting["plaid_client_id"]
assert Setting.dynamic_fields.key?("plaid_client_id")
assert Setting.key?("plaid_client_id")
patch settings_providers_url, params: {
setting: { plaid_client_id: " " } # Blank string with spaces
@@ -170,18 +175,18 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to settings_providers_url
assert_nil Setting["plaid_client_id"]
# Key should be removed from hash, not just set to nil
refute Setting.dynamic_fields.key?("plaid_client_id"),
"nil values should delete the key from dynamic_fields"
# Entry should be removed, not just set to nil
refute Setting.key?("plaid_client_id"),
"nil values should delete the entry"
end
end
test "handles sequential updates to different dynamic fields safely" do
# This test simulates what would happen if two requests tried to update
# different dynamic fields sequentially. With the batch update approach,
# all changes should be preserved.
# different dynamic fields sequentially. With individual entries,
# all changes should be preserved without conflicts.
with_self_hosting do
Setting.dynamic_fields = { "existing_field" => "existing_value" }
Setting["existing_field"] = "existing_value"
# Simulate first request updating plaid fields
patch settings_providers_url, params: {
@@ -266,8 +271,8 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
test "logs errors when update fails" do
with_self_hosting do
# Test that errors during update are properly logged and handled gracefully
# We'll force an error by making the dynamic_fields= setter raise
Setting.expects(:dynamic_fields=).raises(StandardError.new("Database error")).once
# We'll force an error by making the []= method raise
Setting.expects(:[]=).with("plaid_client_id", "test").raises(StandardError.new("Database error")).once
# Mock logger to verify error is logged
Rails.logger.expects(:error).with(regexp_matches(/Failed to update provider settings.*Database error/)).once

14
test/models/period_test.rb
View File

@@ -45,25 +45,11 @@ class PeriodTest < ActiveSupport::TestCase
assert_equal "Custom Period", period.label
end
test "comparison_label returns correct label for known period" do
period = Period.from_key("last_30_days")
assert_equal "vs. last month", period.comparison_label
end
test "comparison_label returns date range for unknown period" do
start_date = Date.current - 15.days
end_date = Date.current
period = Period.new(start_date: start_date, end_date: end_date)
expected = "#{start_date.strftime("%b %d, %Y")} to #{end_date.strftime("%b %d, %Y")}"
assert_equal expected, period.comparison_label
end
test "all_time period can be created" do
period = Period.from_key("all_time")
assert_equal "all_time", period.key
assert_equal "All Time", period.label
assert_equal "All", period.label_short
assert_equal "vs. beginning", period.comparison_label
end
test "all_time period uses family's oldest entry date" do

70
test/models/setting_test.rb
View File

@@ -7,6 +7,11 @@ class SettingTest < ActiveSupport::TestCase
Setting.openai_model = nil
end
teardown do
# Clean up dynamic fields after each test
Setting.where("var LIKE ?", "dynamic:%").destroy_all
end
test "validate_openai_config! passes when both uri base and model are set" do
assert_nothing_raised do
Setting.validate_openai_config!(uri_base: "https://api.example.com", model: "gpt-4")
@@ -61,4 +66,69 @@ class SettingTest < ActiveSupport::TestCase
Setting.validate_openai_config!(uri_base: "", model: nil)
end
end
# Dynamic field tests
test "can set and get dynamic fields" do
Setting["custom_key"] = "custom_value"
assert_equal "custom_value", Setting["custom_key"]
end
test "can set and get multiple dynamic fields independently" do
Setting["key1"] = "value1"
Setting["key2"] = "value2"
Setting["key3"] = "value3"
assert_equal "value1", Setting["key1"]
assert_equal "value2", Setting["key2"]
assert_equal "value3", Setting["key3"]
end
test "setting nil value deletes dynamic field" do
Setting["temp_key"] = "temp_value"
assert_equal "temp_value", Setting["temp_key"]
Setting["temp_key"] = nil
assert_nil Setting["temp_key"]
end
test "can delete dynamic field" do
Setting["delete_key"] = "delete_value"
assert_equal "delete_value", Setting["delete_key"]
value = Setting.delete("delete_key")
assert_equal "delete_value", value
assert_nil Setting["delete_key"]
end
test "key? returns true for existing dynamic field" do
Setting["exists_key"] = "exists_value"
assert Setting.key?("exists_key")
end
test "key? returns false for non-existing dynamic field" do
assert_not Setting.key?("nonexistent_key")
end
test "dynamic_keys returns all dynamic field keys" do
Setting["dynamic1"] = "value1"
Setting["dynamic2"] = "value2"
keys = Setting.dynamic_keys
assert_includes keys, "dynamic1"
assert_includes keys, "dynamic2"
end
test "declared fields take precedence over dynamic fields" do
# Try to set a declared field using bracket notation
Setting["openai_model"] = "custom-model"
assert_equal "custom-model", Setting["openai_model"]
assert_equal "custom-model", Setting.openai_model
end
test "cannot delete declared fields" do
Setting.openai_model = "test-model"
result = Setting.delete("openai_model")
assert_nil result
assert_equal "test-model", Setting.openai_model
end
end