diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 60dc96853..614683b0f 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -11,15 +11,15 @@ class AccountsController < ApplicationController .listable_manual .where(id: @accessible_account_ids) .order(:name) - @plaid_items = family.plaid_items.ordered.includes(:syncs, :plaid_accounts) - @simplefin_items = family.simplefin_items.ordered.includes(:syncs) - @lunchflow_items = family.lunchflow_items.ordered.includes(:syncs, :lunchflow_accounts) - @enable_banking_items = family.enable_banking_items.ordered.includes(:syncs) - @coinstats_items = family.coinstats_items.ordered.includes(:coinstats_accounts, :accounts, :syncs) - @mercury_items = family.mercury_items.ordered.includes(:syncs, :mercury_accounts) - @coinbase_items = family.coinbase_items.ordered.includes(:coinbase_accounts, :accounts, :syncs) - @snaptrade_items = family.snaptrade_items.ordered.includes(:syncs, :snaptrade_accounts) - @indexa_capital_items = family.indexa_capital_items.ordered.includes(:syncs, :indexa_capital_accounts) + @plaid_items = visible_provider_items(family.plaid_items.ordered.includes(:syncs, :plaid_accounts)) + @simplefin_items = visible_provider_items(family.simplefin_items.ordered.includes(:syncs)) + @lunchflow_items = visible_provider_items(family.lunchflow_items.ordered.includes(:syncs, :lunchflow_accounts)) + @enable_banking_items = visible_provider_items(family.enable_banking_items.ordered.includes(:syncs)) + @coinstats_items = visible_provider_items(family.coinstats_items.ordered.includes(:coinstats_accounts, :accounts, :syncs)) + @mercury_items = visible_provider_items(family.mercury_items.ordered.includes(:syncs, :mercury_accounts)) + @coinbase_items = visible_provider_items(family.coinbase_items.ordered.includes(:coinbase_accounts, :accounts, :syncs)) + @snaptrade_items = visible_provider_items(family.snaptrade_items.ordered.includes(:syncs, :snaptrade_accounts)) + @indexa_capital_items = visible_provider_items(family.indexa_capital_items.ordered.includes(:syncs, :indexa_capital_accounts)) # Build sync stats maps for all providers build_sync_stats_maps @@ -220,6 +220,13 @@ class AccountsController < ApplicationController end end + def visible_provider_items(items) + items.select do |item| + Current.user.admin? || + (item.respond_to?(:accounts) && (item.accounts.map(&:id) & @accessible_account_ids).any?) + end + end + # Builds sync stats maps for all provider types to avoid N+1 queries in views def build_sync_stats_maps # SimpleFIN sync stats diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index dea9cbe6d..e5e4cad3c 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -9,7 +9,7 @@ class Api::V1::AccountsController < Api::V1::BaseController def index # Test with Pagy pagination family = current_resource_owner.family - accounts_query = family.accounts.visible.alphabetically + accounts_query = family.accounts.accessible_by(current_resource_owner).visible.alphabetically # Handle pagination with Pagy @pagy, @accounts = pagy( diff --git a/app/controllers/api/v1/merchants_controller.rb b/app/controllers/api/v1/merchants_controller.rb index da6aa0ed3..c11c7246c 100644 --- a/app/controllers/api/v1/merchants_controller.rb +++ b/app/controllers/api/v1/merchants_controller.rb @@ -22,10 +22,15 @@ module Api # @return [Array] JSON array of merchant objects def index family = current_resource_owner.family + user = current_resource_owner # Single query with OR conditions - more efficient than Ruby deduplication family_merchant_ids = family.merchants.select(:id) - provider_merchant_ids = family.transactions.select(:merchant_id) + accessible_account_ids = family.accounts.accessible_by(user).select(:id) + provider_merchant_ids = Transaction.joins(:entry) + .where(entries: { account_id: accessible_account_ids }) + .where.not(merchant_id: nil) + .select(:merchant_id) @merchants = Merchant .where(id: family_merchant_ids) @@ -48,10 +53,11 @@ module Api # @return [Hash] JSON merchant object or error def show family = current_resource_owner.family + user = current_resource_owner @merchant = family.merchants.find_by(id: params[:id]) || Merchant.joins(transactions: :entry) - .where(entries: { account_id: family.accounts.select(:id) }) + .where(entries: { account_id: family.accounts.accessible_by(user).select(:id) }) .distinct .find_by(id: params[:id]) diff --git a/app/controllers/api/v1/transactions_controller.rb b/app/controllers/api/v1/transactions_controller.rb index 4af4ca5c2..b5942b7fc 100644 --- a/app/controllers/api/v1/transactions_controller.rb +++ b/app/controllers/api/v1/transactions_controller.rb @@ -10,7 +10,9 @@ class Api::V1::TransactionsController < Api::V1::BaseController def index family = current_resource_owner.family + accessible_account_ids = family.accounts.accessible_by(current_resource_owner).select(:id) transactions_query = family.transactions.visible + .joins(:entry).where(entries: { account_id: accessible_account_ids }) # Apply filters transactions_query = apply_filters(transactions_query) @@ -76,7 +78,7 @@ class Api::V1::TransactionsController < Api::V1::BaseController return end - account = family.accounts.find(transaction_params[:account_id]) + account = family.accounts.writable_by(current_resource_owner).find(transaction_params[:account_id]) @entry = account.entries.new(entry_params_for_create) if @entry.save @@ -177,7 +179,10 @@ end def set_transaction family = current_resource_owner.family - @transaction = family.transactions.find(params[:id]) + @transaction = family.transactions + .joins(entry: :account) + .merge(Account.accessible_by(current_resource_owner)) + .find(params[:id]) @entry = @transaction.entry rescue ActiveRecord::RecordNotFound render json: { diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 79c92d76c..7604ed654 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,7 +1,7 @@ class ApplicationController < ActionController::Base include RestoreLayoutPreferences, Onboardable, Localize, AutoSync, Authentication, Invitable, SelfHostable, StoreLocation, Impersonatable, Breadcrumbable, - FeatureGuardable, Notifiable, SafePagination + FeatureGuardable, Notifiable, SafePagination, AccountAuthorizable include Pundit::Authorization include Pagy::Backend @@ -40,6 +40,17 @@ class ApplicationController < ActionController::Base session[:pending_invitation_token] = token if invitation end + def require_admin! + return if Current.user&.admin? + + respond_to do |format| + format.html { redirect_to accounts_path, alert: t("shared.require_admin") } + format.turbo_stream { head :forbidden } + format.json { head :forbidden } + format.any { head :forbidden } + end + end + def detect_os user_agent = request.user_agent @os = case user_agent diff --git a/app/controllers/budgets_controller.rb b/app/controllers/budgets_controller.rb index 837306ce7..db04a4720 100644 --- a/app/controllers/budgets_controller.rb +++ b/app/controllers/budgets_controller.rb @@ -53,12 +53,12 @@ class BudgetsController < ApplicationController def set_budget start_date = Budget.param_to_date(params[:month_year], family: Current.family) - @budget = Budget.find_or_bootstrap(Current.family, start_date: start_date) + @budget = Budget.find_or_bootstrap(Current.family, start_date: start_date, user: Current.user) raise ActiveRecord::RecordNotFound unless @budget end def redirect_to_current_month_budget - current_budget = Budget.find_or_bootstrap(Current.family, start_date: Date.current) + current_budget = Budget.find_or_bootstrap(Current.family, start_date: Date.current, user: Current.user) redirect_to budget_path(current_budget) end end diff --git a/app/controllers/coinbase_items_controller.rb b/app/controllers/coinbase_items_controller.rb index b3a1578b9..498853574 100644 --- a/app/controllers/coinbase_items_controller.rb +++ b/app/controllers/coinbase_items_controller.rb @@ -1,5 +1,6 @@ class CoinbaseItemsController < ApplicationController before_action :set_coinbase_item, only: [ :show, :edit, :update, :destroy, :sync, :setup_accounts, :complete_account_setup ] + before_action :require_admin!, only: [ :new, :create, :preload_accounts, :select_accounts, :link_accounts, :select_existing_account, :link_existing_account, :edit, :update, :destroy, :sync, :setup_accounts, :complete_account_setup ] def index @coinbase_items = Current.family.coinbase_items.ordered diff --git a/app/controllers/coinstats_items_controller.rb b/app/controllers/coinstats_items_controller.rb index 9e47d8f1a..8459d458b 100644 --- a/app/controllers/coinstats_items_controller.rb +++ b/app/controllers/coinstats_items_controller.rb @@ -1,5 +1,6 @@ class CoinstatsItemsController < ApplicationController before_action :set_coinstats_item, only: [ :show, :edit, :update, :destroy, :sync ] + before_action :require_admin!, only: [ :new, :create, :edit, :update, :destroy, :sync, :link_wallet ] def index @coinstats_items = Current.family.coinstats_items.ordered diff --git a/app/controllers/concerns/account_authorizable.rb b/app/controllers/concerns/account_authorizable.rb new file mode 100644 index 000000000..70bbdd9a4 --- /dev/null +++ b/app/controllers/concerns/account_authorizable.rb @@ -0,0 +1,29 @@ +module AccountAuthorizable + extend ActiveSupport::Concern + + included do + include StreamExtensions + end + + private + + def require_account_permission!(account, level = :write, redirect_path: nil) + permission = account.permission_for(Current.user) + + allowed = case level + when :write then permission.in?([ :owner, :full_control ]) + when :annotate then permission.in?([ :owner, :full_control, :read_write ]) + when :owner then permission == :owner + else false + end + + return true if allowed + + path = redirect_path || account_path(account) + respond_to do |format| + format.html { redirect_back_or_to path, alert: t("accounts.not_authorized") } + format.turbo_stream { stream_redirect_back_or_to(path, alert: t("accounts.not_authorized")) } + end + false + end +end diff --git a/app/controllers/concerns/accountable_resource.rb b/app/controllers/concerns/accountable_resource.rb index fcf51dbe9..23fd76107 100644 --- a/app/controllers/concerns/accountable_resource.rb +++ b/app/controllers/concerns/accountable_resource.rb @@ -46,7 +46,6 @@ module AccountableResource opening_balance_date: opening_balance_date ) @account.lock_saved_attributes! - @account.auto_share_with_family! if Current.family.share_all_by_default? end redirect_to account_params[:return_to].presence || @account, notice: t("accounts.create.success", type: accountable_type.name.underscore.humanize) @@ -96,14 +95,7 @@ module AccountableResource def set_manageable_account @account = Current.user.accessible_accounts.find(params[:id]) - permission = @account.permission_for(Current.user) - unless permission.in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_to account_path(@account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_to(account_path(@account), alert: t("accounts.not_authorized")) } - end - nil - end + require_account_permission!(@account) end def account_params diff --git a/app/controllers/concerns/entryable_resource.rb b/app/controllers/concerns/entryable_resource.rb index 11b34d83b..13f5798d0 100644 --- a/app/controllers/concerns/entryable_resource.rb +++ b/app/controllers/concerns/entryable_resource.rb @@ -31,13 +31,7 @@ module EntryableResource end def destroy - unless can_edit_entry? - respond_to do |format| - format.html { redirect_back_or_to account_path(@entry.account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(@entry.account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(@entry.account) @entry.destroy! @entry.sync_account_later diff --git a/app/controllers/enable_banking_items_controller.rb b/app/controllers/enable_banking_items_controller.rb index 63c23601a..6cb8b1fc8 100644 --- a/app/controllers/enable_banking_items_controller.rb +++ b/app/controllers/enable_banking_items_controller.rb @@ -1,6 +1,7 @@ class EnableBankingItemsController < ApplicationController include EnableBankingItems::MapsHelper before_action :set_enable_banking_item, only: [ :update, :destroy, :sync, :select_bank, :authorize, :reauthorize, :setup_accounts, :complete_account_setup, :new_connection ] + before_action :require_admin!, only: [ :new, :create, :link_accounts, :select_existing_account, :link_existing_account, :update, :destroy, :sync, :select_bank, :authorize, :reauthorize, :setup_accounts, :complete_account_setup, :new_connection ] skip_before_action :verify_authenticity_token, only: [ :callback ] def new diff --git a/app/controllers/family_merchants_controller.rb b/app/controllers/family_merchants_controller.rb index f89a9b7e7..04e8fc15d 100644 --- a/app/controllers/family_merchants_controller.rb +++ b/app/controllers/family_merchants_controller.rb @@ -6,7 +6,7 @@ class FamilyMerchantsController < ApplicationController # Show all merchants for this family @family_merchants = Current.family.merchants.alphabetically - @provider_merchants = Current.family.assigned_merchants.where(type: "ProviderMerchant").alphabetically + @provider_merchants = Current.family.assigned_merchants_for(Current.user).where(type: "ProviderMerchant").alphabetically # Show recently unlinked ProviderMerchants (within last 30 days) # Exclude merchants that are already assigned to transactions (they appear in provider_merchants) diff --git a/app/controllers/holdings_controller.rb b/app/controllers/holdings_controller.rb index 98e30a03c..d51fbc411 100644 --- a/app/controllers/holdings_controller.rb +++ b/app/controllers/holdings_controller.rb @@ -2,7 +2,7 @@ class HoldingsController < ApplicationController include StreamExtensions before_action :set_holding, only: %i[show update destroy unlock_cost_basis remap_security reset_security sync_prices] - before_action :require_holding_write_permission!, only: %i[update destroy unlock_cost_basis remap_security reset_security] + before_action :require_holding_write_permission!, only: %i[update destroy unlock_cost_basis remap_security reset_security sync_prices] def index @account = accessible_accounts.find(params[:account_id]) @@ -147,13 +147,7 @@ class HoldingsController < ApplicationController end def require_holding_write_permission! - permission = @holding.account.permission_for(Current.user) - unless permission.in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(@holding.account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(@holding.account), alert: t("accounts.not_authorized")) } - end - end + require_account_permission!(@holding.account) end def holding_params diff --git a/app/controllers/import/uploads_controller.rb b/app/controllers/import/uploads_controller.rb index 3cf58ffda..0212bc850 100644 --- a/app/controllers/import/uploads_controller.rb +++ b/app/controllers/import/uploads_controller.rb @@ -19,7 +19,7 @@ class Import::UploadsController < ApplicationController elsif @import.is_a?(SureImport) update_sure_import_upload elsif csv_valid?(csv_str) - @import.account = accessible_accounts.find_by(id: import_account_id) + @import.account = import_account_id.present? ? accessible_accounts.find(import_account_id) : nil @import.assign_attributes(raw_file_str: csv_str, col_sep: upload_params[:col_sep]) @import.save!(validate: false) diff --git a/app/controllers/indexa_capital_items_controller.rb b/app/controllers/indexa_capital_items_controller.rb index ddc4145e8..4b37eea8a 100644 --- a/app/controllers/indexa_capital_items_controller.rb +++ b/app/controllers/indexa_capital_items_controller.rb @@ -4,6 +4,7 @@ class IndexaCapitalItemsController < ApplicationController ALLOWED_ACCOUNTABLE_TYPES = %w[Depository CreditCard Investment Loan OtherAsset OtherLiability Crypto Property Vehicle].freeze before_action :set_indexa_capital_item, only: [ :show, :edit, :update, :destroy, :sync, :setup_accounts, :complete_account_setup ] + before_action :require_admin!, only: [ :new, :create, :preload_accounts, :select_accounts, :link_accounts, :select_existing_account, :link_existing_account, :edit, :update, :destroy, :sync, :setup_accounts, :complete_account_setup ] def index @indexa_capital_items = Current.family.indexa_capital_items.ordered @@ -290,6 +291,7 @@ class IndexaCapitalItemsController < ApplicationController accountable: accountable_class.new ) + account.auto_share_with_family! if Current.family.share_all_by_default? indexa_capital_account.ensure_account_provider!(account) account end @@ -303,12 +305,15 @@ class IndexaCapitalItemsController < ApplicationController accountable_attrs[:subtype] = config[:subtype] end - Current.family.accounts.create!( + account = Current.family.accounts.create!( name: indexa_capital_account.name, balance: config[:balance].present? ? config[:balance].to_d : (indexa_capital_account.current_balance || 0), currency: indexa_capital_account.currency || "EUR", accountable: accountable_class.new(accountable_attrs) ) + + account.auto_share_with_family! if Current.family.share_all_by_default? + account end def infer_accountable_type(account_type, subtype = nil) diff --git a/app/controllers/lunchflow_items_controller.rb b/app/controllers/lunchflow_items_controller.rb index 2208090e7..fce4c5750 100644 --- a/app/controllers/lunchflow_items_controller.rb +++ b/app/controllers/lunchflow_items_controller.rb @@ -1,5 +1,6 @@ class LunchflowItemsController < ApplicationController before_action :set_lunchflow_item, only: [ :show, :edit, :update, :destroy, :sync, :setup_accounts, :complete_account_setup ] + before_action :require_admin!, only: [ :new, :create, :preload_accounts, :select_accounts, :link_accounts, :select_existing_account, :link_existing_account, :edit, :update, :destroy, :sync, :setup_accounts, :complete_account_setup ] def index @lunchflow_items = Current.family.lunchflow_items.active.ordered diff --git a/app/controllers/mercury_items_controller.rb b/app/controllers/mercury_items_controller.rb index 0c72e3cd6..d02948c74 100644 --- a/app/controllers/mercury_items_controller.rb +++ b/app/controllers/mercury_items_controller.rb @@ -1,5 +1,6 @@ class MercuryItemsController < ApplicationController before_action :set_mercury_item, only: [ :show, :edit, :update, :destroy, :sync, :setup_accounts, :complete_account_setup ] + before_action :require_admin!, only: [ :new, :create, :preload_accounts, :select_accounts, :link_accounts, :select_existing_account, :link_existing_account, :edit, :update, :destroy, :sync, :setup_accounts, :complete_account_setup ] def index @mercury_items = Current.family.mercury_items.active.ordered diff --git a/app/controllers/pending_duplicate_merges_controller.rb b/app/controllers/pending_duplicate_merges_controller.rb index 3e1d68786..80c592f3b 100644 --- a/app/controllers/pending_duplicate_merges_controller.rb +++ b/app/controllers/pending_duplicate_merges_controller.rb @@ -17,11 +17,7 @@ class PendingDuplicateMergesController < ApplicationController end def create - permission = @transaction.entry.account.permission_for(Current.user) - unless permission.in?([ :owner, :full_control ]) - redirect_back_or_to transactions_path, alert: t("accounts.not_authorized") - return - end + return unless require_account_permission!(@transaction.entry.account, :annotate, redirect_path: transactions_path) # Manually merge the pending transaction with the selected posted transaction unless merge_params[:posted_entry_id].present? diff --git a/app/controllers/plaid_items_controller.rb b/app/controllers/plaid_items_controller.rb index f2846d04e..bfe8c5bd5 100644 --- a/app/controllers/plaid_items_controller.rb +++ b/app/controllers/plaid_items_controller.rb @@ -1,5 +1,6 @@ class PlaidItemsController < ApplicationController before_action :set_plaid_item, only: %i[edit destroy sync] + before_action :require_admin!, only: %i[new create select_existing_account link_existing_account edit destroy sync] def new region = params[:region] == "eu" ? :eu : :us diff --git a/app/controllers/properties_controller.rb b/app/controllers/properties_controller.rb index 563738efb..e937c4fb6 100644 --- a/app/controllers/properties_controller.rb +++ b/app/controllers/properties_controller.rb @@ -107,12 +107,6 @@ class PropertiesController < ApplicationController end def require_property_write_permission! - permission = @account.permission_for(Current.user) - unless permission.in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(@account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(@account), alert: t("accounts.not_authorized")) } - end - end + require_account_permission!(@account) end end diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 83c9b0f76..47202ec48 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -300,7 +300,7 @@ class ReportsController < ApplicationController # Only calculate if we're looking at current month return nil unless @period_type == :monthly && @start_date.beginning_of_month.to_date == Date.current.beginning_of_month.to_date - budget = Budget.find_or_bootstrap(Current.family, start_date: @start_date.beginning_of_month.to_date) + budget = Budget.find_or_bootstrap(Current.family, start_date: @start_date.beginning_of_month.to_date, user: Current.user) return 0 if budget.nil? || budget.allocated_spending.zero? (budget.actual_spending / budget.allocated_spending * 100).round(1) diff --git a/app/controllers/simplefin_items_controller.rb b/app/controllers/simplefin_items_controller.rb index 2aeb01f5d..32ba1deb4 100644 --- a/app/controllers/simplefin_items_controller.rb +++ b/app/controllers/simplefin_items_controller.rb @@ -1,6 +1,7 @@ class SimplefinItemsController < ApplicationController include SimplefinItems::MapsHelper before_action :set_simplefin_item, only: [ :show, :edit, :update, :destroy, :sync, :balances, :setup_accounts, :complete_account_setup ] + before_action :require_admin!, only: [ :new, :create, :select_existing_account, :link_existing_account, :edit, :update, :destroy, :sync, :balances, :setup_accounts, :complete_account_setup ] def index @simplefin_items = Current.family.simplefin_items.active.ordered diff --git a/app/controllers/snaptrade_items_controller.rb b/app/controllers/snaptrade_items_controller.rb index 166878370..c16eeb011 100644 --- a/app/controllers/snaptrade_items_controller.rb +++ b/app/controllers/snaptrade_items_controller.rb @@ -1,5 +1,6 @@ class SnaptradeItemsController < ApplicationController before_action :set_snaptrade_item, only: [ :show, :edit, :update, :destroy, :sync, :connect, :setup_accounts, :complete_account_setup, :connections, :delete_connection, :delete_orphaned_user ] + before_action :require_admin!, only: [ :new, :create, :preload_accounts, :select_accounts, :link_accounts, :select_existing_account, :link_existing_account, :edit, :update, :destroy, :sync, :connect, :callback, :setup_accounts, :complete_account_setup, :connections, :delete_connection, :delete_orphaned_user ] def index @snaptrade_items = Current.family.snaptrade_items.ordered diff --git a/app/controllers/splits_controller.rb b/app/controllers/splits_controller.rb index c7b264af5..a62540e9d 100644 --- a/app/controllers/splits_controller.rb +++ b/app/controllers/splits_controller.rb @@ -87,10 +87,7 @@ class SplitsController < ApplicationController end def require_split_write_permission! - permission = @entry.account.permission_for(Current.user) - unless permission.in?([ :owner, :full_control ]) - redirect_back_or_to transactions_path, alert: t("accounts.not_authorized") - end + require_account_permission!(@entry.account, redirect_path: transactions_path) end def resolve_to_parent! diff --git a/app/controllers/trades_controller.rb b/app/controllers/trades_controller.rb index 8dc280e70..c8b76af02 100644 --- a/app/controllers/trades_controller.rb +++ b/app/controllers/trades_controller.rb @@ -17,13 +17,7 @@ class TradesController < ApplicationController def create @account = accessible_accounts.find(params[:account_id]) - unless @account.permission_for(Current.user).in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(@account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(@account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(@account) @model = Trade::CreateForm.new(create_params.merge(account: @account)).create @@ -46,13 +40,7 @@ class TradesController < ApplicationController end def update - unless can_edit_entry? - respond_to do |format| - format.html { redirect_back_or_to account_path(@entry.account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(@entry.account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(@entry.account) if @entry.update(update_entry_params) @entry.lock_saved_attributes! @@ -86,13 +74,7 @@ class TradesController < ApplicationController end def unlock - unless @entry.account.permission_for(Current.user).in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(@entry.account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(@entry.account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(@entry.account) @entry.unlock_for_sync! flash[:notice] = t("entries.unlock.success") diff --git a/app/controllers/transaction_categories_controller.rb b/app/controllers/transaction_categories_controller.rb index 1acffed57..6a8ff353a 100644 --- a/app/controllers/transaction_categories_controller.rb +++ b/app/controllers/transaction_categories_controller.rb @@ -3,12 +3,7 @@ class TransactionCategoriesController < ApplicationController def update @entry = Current.accessible_entries.transactions.find(params[:transaction_id]) - - permission = @entry.account.permission_for(Current.user) - unless permission.in?([ :owner, :full_control, :read_write ]) - redirect_back_or_to transaction_path(@entry), alert: t("accounts.not_authorized") - return - end + return unless require_account_permission!(@entry.account, :annotate, redirect_path: transaction_path(@entry)) @entry.update!(entry_params) diff --git a/app/controllers/transactions/bulk_deletions_controller.rb b/app/controllers/transactions/bulk_deletions_controller.rb index 37a85ed88..2cf1fc2df 100644 --- a/app/controllers/transactions/bulk_deletions_controller.rb +++ b/app/controllers/transactions/bulk_deletions_controller.rb @@ -16,15 +16,7 @@ class Transactions::BulkDeletionsController < ApplicationController params.require(:bulk_delete).permit(entry_ids: []) end - # Accounts where the user can delete entries (owner or full_control) def writable_accounts - Current.family.accounts - .left_joins(:account_shares) - .where( - "accounts.owner_id = :uid OR (account_shares.user_id = :uid AND account_shares.permission = :perm)", - uid: Current.user.id, - perm: "full_control" - ) - .distinct + Current.family.accounts.writable_by(Current.user) end end diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 43c4c9ff0..e987d9aa1 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -93,13 +93,7 @@ class TransactionsController < ApplicationController def create account = Current.user.accessible_accounts.find(params.dig(:entry, :account_id)) - unless account.permission_for(Current.user).in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(account) @entry = account.entries.new(entry_params) @@ -167,13 +161,7 @@ class TransactionsController < ApplicationController def merge_duplicate transaction = accessible_transactions.includes(entry: :account).find(params[:id]) - unless transaction.entry.account.permission_for(Current.user).in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(transaction.entry.account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(transaction.entry.account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(transaction.entry.account) if transaction.merge_with_duplicate! flash[:notice] = t("transactions.merge_duplicate.success") @@ -191,13 +179,7 @@ class TransactionsController < ApplicationController def dismiss_duplicate transaction = accessible_transactions.includes(entry: :account).find(params[:id]) - unless transaction.entry.account.permission_for(Current.user).in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(transaction.entry.account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(transaction.entry.account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(transaction.entry.account) if transaction.dismiss_duplicate_suggestion! flash[:notice] = t("transactions.dismiss_duplicate.success") @@ -216,13 +198,7 @@ class TransactionsController < ApplicationController @transaction = accessible_transactions.includes(entry: :account).find(params[:id]) @entry = @transaction.entry - unless @entry.account.permission_for(Current.user).in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(@entry.account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(@entry.account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(@entry.account) unless @entry.account.investment? flash[:alert] = t("transactions.convert_to_trade.errors.not_investment_account") @@ -237,13 +213,7 @@ class TransactionsController < ApplicationController @transaction = accessible_transactions.includes(entry: :account).find(params[:id]) @entry = @transaction.entry - unless @entry.account.permission_for(Current.user).in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(@entry.account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(@entry.account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(@entry.account) # Pre-transaction validations unless @entry.account.investment? @@ -319,13 +289,7 @@ class TransactionsController < ApplicationController end def unlock - unless @entry.account.permission_for(Current.user).in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(@entry.account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(@entry.account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(@entry.account) @entry.unlock_for_sync! flash[:notice] = t("entries.unlock.success") @@ -336,13 +300,7 @@ class TransactionsController < ApplicationController def mark_as_recurring transaction = accessible_transactions.includes(entry: :account).find(params[:id]) - unless transaction.entry.account.permission_for(Current.user).in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(transaction.entry.account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(transaction.entry.account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(transaction.entry.account) # Check if a recurring transaction already exists for this pattern existing = Current.family.recurring_transactions.find_by( diff --git a/app/controllers/transfer_matches_controller.rb b/app/controllers/transfer_matches_controller.rb index 5792db0be..341688d5d 100644 --- a/app/controllers/transfer_matches_controller.rb +++ b/app/controllers/transfer_matches_controller.rb @@ -2,16 +2,15 @@ class TransferMatchesController < ApplicationController before_action :set_entry def new - @accounts = accessible_accounts.visible.alphabetically.where.not(id: @entry.account_id) + @accounts = Current.family.accounts.writable_by(Current.user).visible.alphabetically.where.not(id: @entry.account_id) @transfer_match_candidates = @entry.transaction.transfer_match_candidates end def create - permission = @entry.account.permission_for(Current.user) - unless permission.in?([ :owner, :full_control ]) - redirect_back_or_to transactions_path, alert: t("accounts.not_authorized") - return - end + return unless require_account_permission!(@entry.account, redirect_path: transactions_path) + + target_account = resolve_target_account + return unless require_account_permission!(target_account, redirect_path: transactions_path) @transfer = build_transfer Transfer.transaction do @@ -45,6 +44,14 @@ class TransferMatchesController < ApplicationController params.require(:transfer_match).permit(:method, :matched_entry_id, :target_account_id) end + def resolve_target_account + if transfer_match_params[:method] == "new" + accessible_accounts.find(transfer_match_params[:target_account_id]) + else + Current.accessible_entries.find(transfer_match_params[:matched_entry_id]).account + end + end + def build_transfer if transfer_match_params[:method] == "new" target_account = accessible_accounts.find(transfer_match_params[:target_account_id]) diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb index b7c571d85..0a000fad6 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -24,14 +24,8 @@ class TransfersController < ApplicationController source_account = accessible_accounts.find(transfer_params[:from_account_id]) destination_account = accessible_accounts.find(transfer_params[:to_account_id]) - unless source_account.permission_for(Current.user).in?([ :owner, :full_control ]) && - destination_account.permission_for(Current.user).in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to transactions_path, alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(transactions_path, alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(source_account, redirect_path: transactions_path) + return unless require_account_permission!(destination_account, redirect_path: transactions_path) @transfer = Transfer::Creator.new( family: Current.family, @@ -55,14 +49,7 @@ class TransfersController < ApplicationController def update outflow_account = @transfer.outflow_transaction.entry.account - permission = outflow_account.permission_for(Current.user) - unless permission.in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to transactions_url, alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(transactions_url, alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(outflow_account, redirect_path: transactions_url) Transfer.transaction do update_transfer_status @@ -76,16 +63,8 @@ class TransfersController < ApplicationController end def destroy - # Require write permission on at least the outflow account outflow_account = @transfer.outflow_transaction.entry.account - permission = outflow_account.permission_for(Current.user) - unless permission.in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to transactions_url, alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(transactions_url, alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(outflow_account, redirect_path: transactions_url) @transfer.destroy! redirect_back_or_to transactions_url, notice: t(".success") diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 937452e95..ebf250d03 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -106,10 +106,13 @@ class UsersController < ApplicationController end def user_params + family_attrs = [ :name, :currency, :country, :date_format, :timezone, :locale, :month_start_day, :id ] + family_attrs.push(:moniker, :default_account_sharing) if Current.user.admin? + params.require(:user).permit( :first_name, :last_name, :email, :profile_image, :redirect_to, :delete_profile_image, :onboarded_at, :show_sidebar, :default_period, :default_account_order, :show_ai_sidebar, :ai_enabled, :theme, :set_onboarding_preferences_at, :set_onboarding_goals_at, :locale, - family_attributes: [ :name, :currency, :country, :date_format, :timezone, :locale, :month_start_day, :moniker, :default_account_sharing, :id ], + family_attributes: family_attrs, goals: [] ) end diff --git a/app/controllers/valuations_controller.rb b/app/controllers/valuations_controller.rb index e8c08736c..32ec2df74 100644 --- a/app/controllers/valuations_controller.rb +++ b/app/controllers/valuations_controller.rb @@ -3,14 +3,7 @@ class ValuationsController < ApplicationController def confirm_create @account = accessible_accounts.find(params.dig(:entry, :account_id)) - - unless @account.permission_for(Current.user).in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(@account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(@account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(@account) @entry = @account.entries.build(entry_params.merge(currency: @account.currency)) @@ -25,14 +18,7 @@ class ValuationsController < ApplicationController def confirm_update @entry = Current.accessible_entries.find(params[:id]) - - unless @entry.account.permission_for(Current.user).in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(@entry.account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(@entry.account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(@entry.account) @account = @entry.account @entry.assign_attributes(entry_params.merge(currency: @account.currency)) @@ -49,14 +35,7 @@ class ValuationsController < ApplicationController def create account = accessible_accounts.find(params.dig(:entry, :account_id)) - - unless account.permission_for(Current.user).in?([ :owner, :full_control ]) - respond_to do |format| - format.html { redirect_back_or_to account_path(account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(account) result = account.create_reconciliation( balance: entry_params[:amount], @@ -75,13 +54,7 @@ class ValuationsController < ApplicationController end def update - unless can_edit_entry? - respond_to do |format| - format.html { redirect_back_or_to account_path(@entry.account), alert: t("accounts.not_authorized") } - format.turbo_stream { stream_redirect_back_or_to(account_path(@entry.account), alert: t("accounts.not_authorized")) } - end - return - end + return unless require_account_permission!(@entry.account) # Notes updating is independent of reconciliation, just a simple CRUD operation @entry.update!(notes: entry_params[:notes]) if entry_params[:notes].present? diff --git a/app/jobs/simplefin_item/balances_only_job.rb b/app/jobs/simplefin_item/balances_only_job.rb index a34602bcd..17f15c87c 100644 --- a/app/jobs/simplefin_item/balances_only_job.rb +++ b/app/jobs/simplefin_item/balances_only_job.rb @@ -33,25 +33,10 @@ class SimplefinItem::BalancesOnlyJob < ApplicationJob target_id = ActionView::RecordIdentifier.dom_id(item) Turbo::StreamsChannel.broadcast_replace_to(item.family, target: target_id, html: card_html) - # Also refresh Manual Accounts so the CTA state and duplicates clear without refresh - begin - manual_accounts = item.family.accounts - .visible_manual - .order(:name) - if manual_accounts.any? - manual_html = ApplicationController.render( - partial: "accounts/index/manual_accounts", - formats: [ :html ], - locals: { accounts: manual_accounts } - ) - Turbo::StreamsChannel.broadcast_update_to(item.family, target: "manual-accounts", html: manual_html) - else - manual_html = ApplicationController.render(inline: '
') - Turbo::StreamsChannel.broadcast_replace_to(item.family, target: "manual-accounts", html: manual_html) - end - rescue => inner - Rails.logger.warn("SimpleFin BalancesOnlyJob manual-accounts broadcast failed: #{inner.class} - #{inner.message}") - end + # Broadcast a refresh signal instead of rendered HTML. Each user's browser + # re-fetches via their own authenticated request, so the manual accounts + # list is correctly scoped to the current user. + item.family.broadcast_refresh rescue => e Rails.logger.warn("SimpleFin BalancesOnlyJob broadcast failed: #{e.class} - #{e.message}") end diff --git a/app/models/account.rb b/app/models/account.rb index 54e44b27b..0224f967d 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -1,9 +1,10 @@ class Account < ApplicationRecord include AASM, Syncable, Monetizable, Chartable, Linkable, Enrichable, Anchorable, Reconcileable, TaxTreatable - before_validation :assign_default_owner, on: :create + before_validation :assign_default_owner, if: -> { owner_id.blank? } validates :name, :balance, :currency, presence: true + validate :owner_belongs_to_family, if: -> { owner_id.present? && family_id.present? } belongs_to :family belongs_to :owner, class_name: "User", optional: true @@ -48,6 +49,13 @@ class Account < ApplicationRecord .distinct } + # Accounts a user can write to (owned or shared with full_control) + scope :writable_by, ->(user) { + left_joins(:account_shares) + .where("accounts.owner_id = :uid OR (account_shares.user_id = :uid AND account_shares.permission = 'full_control')", uid: user.id) + .distinct + } + # Accounts that count in a user's financial calculations scope :included_in_finances_for, ->(user) { left_joins(:account_shares) @@ -119,6 +127,8 @@ class Account < ApplicationRecord date: opening_balance_date ) raise result.error if result.error + + account.auto_share_with_family! if account.family.share_all_by_default? end # Skip initial sync for linked accounts - the provider sync will handle balance creation @@ -375,6 +385,8 @@ class Account < ApplicationRecord end def shared_with?(user) + return false if user.nil? + owned_by?(user) || if account_shares.loaded? account_shares.any? { |s| s.user_id == user.id } @@ -413,6 +425,16 @@ class Account < ApplicationRecord def assign_default_owner return if owner.present? - self.owner = Current.user || family&.users&.find_by(role: %w[admin super_admin]) || family&.users&.order(:created_at)&.first + + if Current.user.present? && Current.user.family_id == family_id + self.owner = Current.user + else + self.owner = family&.users&.find_by(role: %w[admin super_admin]) || family&.users&.order(:created_at)&.first + end + end + + def owner_belongs_to_family + return if User.where(id: owner_id, family_id: family_id).exists? + errors.add(:owner, :invalid, message: "must belong to the same family as the account") end end diff --git a/app/models/assistant/function.rb b/app/models/assistant/function.rb index 97f069ccc..4e918cb54 100644 --- a/app/models/assistant/function.rb +++ b/app/models/assistant/function.rb @@ -56,7 +56,7 @@ class Assistant::Function end def family_account_names - @family_account_names ||= family.accounts.visible.pluck(:name) + @family_account_names ||= user.accessible_accounts.visible.pluck(:name) end def family_category_names diff --git a/app/models/assistant/function/get_accounts.rb b/app/models/assistant/function/get_accounts.rb index 777b81493..13706d215 100644 --- a/app/models/assistant/function/get_accounts.rb +++ b/app/models/assistant/function/get_accounts.rb @@ -12,7 +12,7 @@ class Assistant::Function::GetAccounts < Assistant::Function def call(params = {}) { as_of_date: Date.current, - accounts: family.accounts.includes(:balances, :account_providers).map do |account| + accounts: user.accessible_accounts.includes(:balances, :account_providers).map do |account| { name: account.name, balance: account.balance, diff --git a/app/models/assistant/function/get_balance_sheet.rb b/app/models/assistant/function/get_balance_sheet.rb index b0e1f0e03..1d8846876 100644 --- a/app/models/assistant/function/get_balance_sheet.rb +++ b/app/models/assistant/function/get_balance_sheet.rb @@ -27,15 +27,15 @@ class Assistant::Function::GetBalanceSheet < Assistant::Function oldest_account_start_date: family.oldest_entry_date, currency: family.currency, net_worth: { - current: family.balance_sheet.net_worth_money.format, + current: family.balance_sheet(user: user).net_worth_money.format, monthly_history: historical_data(period) }, assets: { - current: family.balance_sheet.assets.total_money.format, + current: family.balance_sheet(user: user).assets.total_money.format, monthly_history: historical_data(period, classification: "asset") }, liabilities: { - current: family.balance_sheet.liabilities.total_money.format, + current: family.balance_sheet(user: user).liabilities.total_money.format, monthly_history: historical_data(period, classification: "liability") }, insights: insights_data @@ -44,7 +44,7 @@ class Assistant::Function::GetBalanceSheet < Assistant::Function private def historical_data(period, classification: nil) - scope = family.accounts.visible + scope = user.accessible_accounts.visible scope = scope.where(classification: classification) if classification.present? if period.start_date == Date.current @@ -65,8 +65,8 @@ class Assistant::Function::GetBalanceSheet < Assistant::Function end def insights_data - assets = family.balance_sheet.assets.total - liabilities = family.balance_sheet.liabilities.total + assets = family.balance_sheet(user: user).assets.total + liabilities = family.balance_sheet(user: user).liabilities.total ratio = liabilities.zero? ? 0 : (liabilities / assets.to_f) { diff --git a/app/models/assistant/function/get_holdings.rb b/app/models/assistant/function/get_holdings.rb index 4cc277072..d0025fb54 100644 --- a/app/models/assistant/function/get_holdings.rb +++ b/app/models/assistant/function/get_holdings.rb @@ -151,7 +151,7 @@ class Assistant::Function::GetHoldings < Assistant::Function end def investment_accounts - family.accounts.visible.where(accountable_type: SUPPORTED_ACCOUNT_TYPES) + user.accessible_accounts.visible.where(accountable_type: SUPPORTED_ACCOUNT_TYPES) end def investment_account_names diff --git a/app/models/balance_sheet/account_totals.rb b/app/models/balance_sheet/account_totals.rb index 10f6f94ee..d03340423 100644 --- a/app/models/balance_sheet/account_totals.rb +++ b/app/models/balance_sheet/account_totals.rb @@ -27,7 +27,7 @@ class BalanceSheet::AccountTotals def visible_accounts @visible_accounts ||= begin - scope = family.accounts.visible.with_attached_logo + scope = family.accounts.visible.with_attached_logo.includes(:account_shares) scope = scope.accessible_by(user) if user scope end diff --git a/app/models/budget.rb b/app/models/budget.rb index 6c994ee1e..64fcdb73d 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -3,6 +3,8 @@ class Budget < ApplicationRecord PARAM_DATE_FORMAT = "%b-%Y" + attr_accessor :current_user + belongs_to :family has_many :budget_categories, -> { includes(:category) }, dependent: :destroy @@ -38,7 +40,7 @@ class Budget < ApplicationRecord end end - def find_or_bootstrap(family, start_date:) + def find_or_bootstrap(family, start_date:, user: nil) return nil unless budget_date_valid?(start_date, family: family) Budget.transaction do @@ -58,6 +60,7 @@ class Budget < ApplicationRecord b.currency = family.currency end + budget.current_user = user budget.sync_budget_categories budget @@ -107,7 +110,11 @@ class Budget < ApplicationRecord end def transactions - family.transactions.visible.in_period(period) + scope = family.transactions.visible.in_period(period) + if current_user + scope = scope.joins(:entry).where(entries: { account_id: family.accounts.accessible_by(current_user).select(:id) }) + end + scope end def name @@ -298,7 +305,7 @@ class Budget < ApplicationRecord private def income_statement - @income_statement ||= family.income_statement + @income_statement ||= family.income_statement(user: current_user) end def net_totals diff --git a/app/models/family.rb b/app/models/family.rb index 0ba8dc8b2..f5eceb93f 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -105,6 +105,29 @@ class Family < ApplicationRecord Merchant.where(id: (assigned_ids + recently_unlinked_ids + family_merchant_ids).uniq) end + def assigned_merchants_for(user) + merchant_ids = Transaction.joins(:entry) + .where(entries: { account_id: accounts.accessible_by(user).select(:id) }) + .where.not(merchant_id: nil) + .distinct + .pluck(:merchant_id) + Merchant.where(id: merchant_ids) + end + + def available_merchants_for(user) + assigned_ids = Transaction.joins(:entry) + .where(entries: { account_id: accounts.accessible_by(user).select(:id) }) + .where.not(merchant_id: nil) + .distinct + .pluck(:merchant_id) + recently_unlinked_ids = FamilyMerchantAssociation + .where(family: self) + .recently_unlinked + .pluck(:merchant_id) + family_merchant_ids = merchants.pluck(:id) + Merchant.where(id: (assigned_ids + recently_unlinked_ids + family_merchant_ids).uniq) + end + def auto_categorize_transactions_later(transactions, rule_run_id: nil) AutoCategorizeJob.perform_later(self, transaction_ids: transactions.pluck(:id), rule_run_id: rule_run_id) end diff --git a/app/models/income_statement.rb b/app/models/income_statement.rb index 080d350ad..f885e3f81 100644 --- a/app/models/income_statement.rb +++ b/app/models/income_statement.rb @@ -176,14 +176,14 @@ class IncomeStatement def family_stats(interval: "month") @family_stats ||= {} @family_stats[interval] ||= Rails.cache.fetch([ - "income_statement", "family_stats", family.id, user&.id, interval, family.entries_cache_version + "income_statement", "family_stats", family.id, user&.id, interval, included_account_ids_hash, family.entries_cache_version ]) { FamilyStats.new(family, interval:, account_ids: included_account_ids).call } end def category_stats(interval: "month") @category_stats ||= {} @category_stats[interval] ||= Rails.cache.fetch([ - "income_statement", "category_stats", family.id, user&.id, interval, family.entries_cache_version + "income_statement", "category_stats", family.id, user&.id, interval, included_account_ids_hash, family.entries_cache_version ]) { CategoryStats.new(family, interval:, account_ids: included_account_ids).call } end @@ -191,12 +191,15 @@ class IncomeStatement @included_account_ids ||= user ? user.finance_accounts.pluck(:id) : nil end + def included_account_ids_hash + @included_account_ids_hash ||= included_account_ids ? Digest::MD5.hexdigest(included_account_ids.sort.join(",")) : nil + end + def totals_query(transactions_scope:, date_range:) sql_hash = Digest::MD5.hexdigest(transactions_scope.to_sql) - account_ids_hash = included_account_ids ? Digest::MD5.hexdigest(included_account_ids.sort.join(",")) : nil Rails.cache.fetch([ - "income_statement", "totals_query", "v2", family.id, user&.id, account_ids_hash, sql_hash, family.entries_cache_version + "income_statement", "totals_query", "v2", family.id, user&.id, included_account_ids_hash, sql_hash, date_range.begin, date_range.end, family.entries_cache_version ]) { Totals.new(family, transactions_scope: transactions_scope, date_range: date_range, included_account_ids: included_account_ids).call } end diff --git a/app/models/income_statement/category_stats.rb b/app/models/income_statement/category_stats.rb index 7fd2552c6..f9757da16 100644 --- a/app/models/income_statement/category_stats.rb +++ b/app/models/income_statement/category_stats.rb @@ -6,6 +6,8 @@ class IncomeStatement::CategoryStats end def call + return [] if @account_ids&.empty? + ActiveRecord::Base.connection.select_all(sanitized_query_sql).map do |row| StatRow.new( category_id: row["category_id"], @@ -50,7 +52,7 @@ class IncomeStatement::CategoryStats end def scope_to_account_ids_sql - return "" if @account_ids.blank? + return "" if @account_ids.nil? ActiveRecord::Base.sanitize_sql([ "AND a.id IN (?)", @account_ids ]) end diff --git a/app/models/income_statement/family_stats.rb b/app/models/income_statement/family_stats.rb index a8616ef96..d172d4ebf 100644 --- a/app/models/income_statement/family_stats.rb +++ b/app/models/income_statement/family_stats.rb @@ -6,6 +6,8 @@ class IncomeStatement::FamilyStats end def call + return [] if @account_ids&.empty? + ActiveRecord::Base.connection.select_all(sanitized_query_sql).map do |row| StatRow.new( classification: row["classification"], @@ -49,7 +51,7 @@ class IncomeStatement::FamilyStats end def scope_to_account_ids_sql - return "" if @account_ids.blank? + return "" if @account_ids.nil? ActiveRecord::Base.sanitize_sql([ "AND a.id IN (?)", @account_ids ]) end diff --git a/app/models/plaid_account/processor.rb b/app/models/plaid_account/processor.rb index 4faead9d9..4f2af022d 100644 --- a/app/models/plaid_account/processor.rb +++ b/app/models/plaid_account/processor.rb @@ -74,8 +74,11 @@ class PlaidAccount::Processor cash_balance: balance_calculator.cash_balance ) + new_account = account.new_record? account.save! + account.auto_share_with_family! if new_account && account.family.share_all_by_default? + # Create account provider link if it doesn't exist unless account_provider AccountProvider.find_or_create_by!( diff --git a/app/models/rule/condition_filter/transaction_account.rb b/app/models/rule/condition_filter/transaction_account.rb index dcdabdf65..1ec94b515 100644 --- a/app/models/rule/condition_filter/transaction_account.rb +++ b/app/models/rule/condition_filter/transaction_account.rb @@ -4,7 +4,7 @@ class Rule::ConditionFilter::TransactionAccount < Rule::ConditionFilter end def options - family.accounts.alphabetically.pluck(:name, :id) + family.accounts.accessible_by(Current.user).alphabetically.pluck(:name, :id) end def apply(scope, operator, value) diff --git a/app/models/rule/condition_filter/transaction_merchant.rb b/app/models/rule/condition_filter/transaction_merchant.rb index cc6f07644..ae28abc91 100644 --- a/app/models/rule/condition_filter/transaction_merchant.rb +++ b/app/models/rule/condition_filter/transaction_merchant.rb @@ -4,7 +4,7 @@ class Rule::ConditionFilter::TransactionMerchant < Rule::ConditionFilter end def options - family.available_merchants.alphabetically.pluck(:name, :id) + family.available_merchants_for(Current.user).alphabetically.pluck(:name, :id) end def prepare(scope) diff --git a/app/models/simplefin_item/syncer.rb b/app/models/simplefin_item/syncer.rb index e8276a895..650ce4b69 100644 --- a/app/models/simplefin_item/syncer.rb +++ b/app/models/simplefin_item/syncer.rb @@ -172,28 +172,10 @@ class SimplefinItem::Syncer target_id = ActionView::RecordIdentifier.dom_id(simplefin_item) Turbo::StreamsChannel.broadcast_replace_to(simplefin_item.family, target: target_id, html: card_html) - # Also refresh the Manual Accounts group so duplicates clear without a full page reload - begin - manual_accounts = simplefin_item.family.accounts - .visible_manual - .order(:name) - if manual_accounts.any? - manual_html = ApplicationController.render( - partial: "accounts/index/manual_accounts", - formats: [ :html ], - locals: { accounts: manual_accounts } - ) - Turbo::StreamsChannel.broadcast_update_to(simplefin_item.family, target: "manual-accounts", html: manual_html) - else - manual_html = ApplicationController.render(inline: '
') - Turbo::StreamsChannel.broadcast_replace_to(simplefin_item.family, target: "manual-accounts", html: manual_html) - end - rescue => inner - Rails.logger.warn("SimplefinItem::Syncer manual-accounts broadcast failed: #{inner.class} - #{inner.message}") - end - - # Intentionally do not broadcast modal reloads here to avoid unexpected auto-pop after sync. - # Modal opening is controlled explicitly via controller redirects with actionable conditions. + # Broadcast a refresh signal instead of rendered HTML. Each user's browser + # re-fetches via their own authenticated request, so the manual accounts + # list is correctly scoped to the current user. + simplefin_item.family.broadcast_refresh rescue => e Rails.logger.warn("SimplefinItem::Syncer broadcast failed: #{e.class} - #{e.message}") end diff --git a/app/views/account_sharings/show.html.erb b/app/views/account_sharings/show.html.erb index 0eb8524f7..b53a6bdff 100644 --- a/app/views/account_sharings/show.html.erb +++ b/app/views/account_sharings/show.html.erb @@ -24,9 +24,10 @@ <%= member.display_name %>
+ <% selected_permission = share&.permission || "read_only" %>