From 315c4bf1ec2c5684711c298caaad0fe6f69c383e Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 4 Apr 2024 23:00:12 +0200 Subject: [PATCH] Account Sync should happen at login, 1x per day, OR on-demand (#594) * Add last_sync_date to accounts table * Always sync Account after Valuation or Transaction creation, update, or deletion. Skip sync if user clicks "sync" button without changing anything * Sync user accounts daily based on last_login_at --- app/controllers/accounts_controller.rb | 9 +++------ app/controllers/application_controller.rb | 10 ++++++++++ app/controllers/concerns/authentication.rb | 6 ++++++ app/models/account/syncable.rb | 13 ++++++++++++- app/models/family.rb | 4 ++++ ...20240401213443_add_last_sync_date_to_accounts.rb | 5 +++++ .../20240403192649_add_last_login_at_to_users.rb | 5 +++++ db/schema.rb | 4 +++- 8 files changed, 48 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20240401213443_add_last_sync_date_to_accounts.rb create mode 100644 db/migrate/20240403192649_add_last_login_at_to_users.rb diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 314f9722..c3a28c55 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -20,7 +20,7 @@ class AccountsController < ApplicationController def update if @account.update(account_params.except(:accountable_type)) - @account.sync_later if account_params[:is_active] == "1" + @account.sync_later if account_params[:is_active] == "1" && @account.can_sync? respond_to do |format| format.html { redirect_to accounts_path, notice: t(".success") } @@ -53,15 +53,12 @@ class AccountsController < ApplicationController end def sync - @account.sync_later + @account.sync_later if @account.can_sync? respond_to do |format| format.html { redirect_to account_path(@account), notice: t(".success") } format.turbo_stream do - render turbo_stream: [ - turbo_stream.append("notification-tray", partial: "shared/notification", locals: { type: "success", content: t(".success") }), - turbo_stream.replace("sync_message", partial: "accounts/sync_message", locals: { is_syncing: true }) - ] + render turbo_stream: turbo_stream.append("notification-tray", partial: "shared/notification", locals: { type: "success", content: t(".success") }) end end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8166a181..f8d4686a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,6 +2,8 @@ class ApplicationController < ActionController::Base include Authentication include Pagy::Backend + before_action :sync_accounts + default_form_builder ApplicationFormBuilder # Only allow modern browsers supporting webp images, web push, badges, import maps, CSS nesting, and CSS :has. @@ -13,4 +15,12 @@ class ApplicationController < ActionController::Base ENV["HOSTED"] == "true" end helper_method :hosted_app? + + def sync_accounts + return if Current.user.blank? + + if Current.user.last_login_at.nil? || Current.user.last_login_at.before?(Date.current.beginning_of_day) + Current.family.sync_accounts + end + end end diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index 51ebafe6..64d0a8c0 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -3,11 +3,13 @@ module Authentication included do before_action :authenticate_user! + after_action :set_last_login_at end class_methods do def skip_authentication(**options) skip_before_action :authenticate_user!, **options + skip_after_action :set_last_login_at, **options end end @@ -31,4 +33,8 @@ module Authentication Current.user = nil reset_session end + + def set_last_login_at + Current.user.update(last_login_at: DateTime.now) + end end diff --git a/app/models/account/syncable.rb b/app/models/account/syncable.rb index 5b47cfd7..19c15337 100644 --- a/app/models/account/syncable.rb +++ b/app/models/account/syncable.rb @@ -14,12 +14,23 @@ module Account::Syncable calculator.calculate self.balances.upsert_all(calculator.daily_balances, unique_by: :index_account_balances_on_account_id_date_currency_unique) self.balances.where("date < ?", effective_start_date).delete_all - update!(status: "ok") + update!(status: "ok", last_sync_date: Date.today) rescue => e update!(status: "error") Rails.logger.error("Failed to sync account #{id}: #{e.message}") end + def can_sync? + # Skip account sync if account is not active or the sync process is already running + return false unless is_active + return false if syncing? + # If last_sync_date is blank (i.e. the account has never been synced before) allow syncing + return true if last_sync_date.blank? + + # If last_sync_date is not today, allow syncing + last_sync_date != Date.today + end + # The earliest date we can calculate a balance for def effective_start_date first_valuation_date = self.valuations.order(:date).pluck(:date).first diff --git a/app/models/family.rb b/app/models/family.rb index 3cf10dbb..27b1b526 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -43,4 +43,8 @@ class Family < ApplicationRecord def liabilities Money.new(accounts.active.liabilities.map { |account| account.balance_money.exchange_to(currency) || 0 }.sum, currency) end + + def sync_accounts + accounts.each { |account| account.sync_later if account.can_sync? } + end end diff --git a/db/migrate/20240401213443_add_last_sync_date_to_accounts.rb b/db/migrate/20240401213443_add_last_sync_date_to_accounts.rb new file mode 100644 index 00000000..004eb380 --- /dev/null +++ b/db/migrate/20240401213443_add_last_sync_date_to_accounts.rb @@ -0,0 +1,5 @@ +class AddLastSyncDateToAccounts < ActiveRecord::Migration[7.2] + def change + add_column :accounts, :last_sync_date, :date + end +end diff --git a/db/migrate/20240403192649_add_last_login_at_to_users.rb b/db/migrate/20240403192649_add_last_login_at_to_users.rb new file mode 100644 index 00000000..9ec54b42 --- /dev/null +++ b/db/migrate/20240403192649_add_last_login_at_to_users.rb @@ -0,0 +1,5 @@ +class AddLastLoginAtToUsers < ActiveRecord::Migration[7.2] + def change + add_column :users, :last_login_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index a3110426..b1be2d79 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_03_25_064211) do +ActiveRecord::Schema[7.2].define(version: 2024_04_03_192649) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -90,6 +90,7 @@ ActiveRecord::Schema[7.2].define(version: 2024_03_25_064211) do t.enum "status", default: "ok", null: false, enum_type: "account_status" t.jsonb "sync_warnings", default: "[]", null: false t.jsonb "sync_errors", default: "[]", null: false + t.date "last_sync_date" t.index ["accountable_type"], name: "index_accounts_on_accountable_type" t.index ["family_id"], name: "index_accounts_on_family_id" end @@ -232,6 +233,7 @@ ActiveRecord::Schema[7.2].define(version: 2024_03_25_064211) do t.string "password_digest" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.datetime "last_login_at" t.index ["email"], name: "index_users_on_email", unique: true t.index ["family_id"], name: "index_users_on_family_id" end