Isolate infinite loop bug, add timeout to actions (#583)

* Isolate infinite loop bug, add timeout to actions

* Increase timeout to allow for temporary failure

* Set correct timeout, implement temporary fix

* Trigger syncs at controller layer

.github/workflows/ci.yml

@@ -8,7 +8,7 @@ on:
 jobs:
   scan_ruby:
     runs-on: ubuntu-latest
-
+    timeout-minutes: 10
     steps:
       - name: Checkout code
         uses: actions/checkout@v4
@@ -24,7 +24,7 @@ jobs:
 
   scan_js:
     runs-on: ubuntu-latest
-
+    timeout-minutes: 10
     steps:
       - name: Checkout code
         uses: actions/checkout@v4
@@ -40,6 +40,7 @@ jobs:
 
   lint:
     runs-on: ubuntu-latest
+    timeout-minutes: 10
     steps:
       - name: Checkout code
         uses: actions/checkout@v4
@@ -55,6 +56,7 @@ jobs:
 
   test:
     runs-on: ubuntu-latest
+    timeout-minutes: 10
 
     services:
       postgres:

app/controllers/transactions_controller.rb

@@ -63,6 +63,7 @@ class TransactionsController < ApplicationController
 
     respond_to do |format|
       if @transaction.save
+        @transaction.account.sync_later
         format.html { redirect_to transactions_url, notice: t(".success") }
       else
         format.html { render :new, status: :unprocessable_entity }
@@ -73,6 +74,8 @@ class TransactionsController < ApplicationController
   def update
     respond_to do |format|
       if @transaction.update(transaction_params)
+        @transaction.account.sync_later
+
         format.html { redirect_to transaction_url(@transaction), notice: t(".success") }
         format.turbo_stream do
           render turbo_stream: [
@@ -88,6 +91,7 @@ class TransactionsController < ApplicationController
 
   def destroy
     @transaction.destroy!
+    @transaction.account.sync_later
 
     respond_to do |format|
       format.html { redirect_to transactions_url, notice: t(".success") }

app/controllers/valuations_controller.rb

@@ -7,6 +7,8 @@ class ValuationsController < ApplicationController
     # TODO: placeholder logic until we have a better abstraction for trends
     @valuation = @account.valuations.new(valuation_params.merge(currency: Current.family.currency))
     if @valuation.save
+      @valuation.account.sync_later
+
       respond_to do |format|
         format.html { redirect_to account_path(@account), notice: "Valuation created" }
         format.turbo_stream
@@ -30,6 +32,8 @@ class ValuationsController < ApplicationController
   def update
     @valuation = Valuation.find(params[:id])
     if @valuation.update(valuation_params)
+      @valuation.account.sync_later
+
       redirect_to account_path(@valuation.account), notice: "Valuation updated"
     else
       render :edit, status: :unprocessable_entity
@@ -42,7 +46,8 @@ class ValuationsController < ApplicationController
   def destroy
     @valuation = Valuation.find(params[:id])
     @account = @valuation.account
-    @valuation.destroy
+    @valuation.destroy!
+    @account.sync_later
 
     respond_to do |format|
       format.html { redirect_to account_path(@account), notice: "Valuation deleted" }

app/models/account.rb

@@ -6,9 +6,10 @@ class Account < ApplicationRecord
 
   broadcasts_refreshes
   belongs_to :family
-  has_many :balances
-  has_many :valuations
-  has_many :transactions
+
+  has_many :balances, dependent: :destroy
+  has_many :valuations, dependent: :destroy
+  has_many :transactions, dependent: :destroy
 
   monetize :balance
 

app/models/transaction.rb

@@ -6,8 +6,6 @@ class Transaction < ApplicationRecord
 
   validates :name, :date, :amount, :account, presence: true
 
-  after_commit :sync_account
-
   monetize :amount
 
   scope :inflows, -> { where("amount > 0") }
@@ -42,10 +40,4 @@ class Transaction < ApplicationRecord
 
     filters
   end
-
-  private
-
-    def sync_account
-      self.account.sync_later
-    end
 end

app/models/valuation.rb

@@ -3,7 +3,6 @@ class Valuation < ApplicationRecord
 
   belongs_to :account
   validates :account, :date, :value, presence: true
-  after_commit :sync_account
   monetize :value
 
   scope :in_period, ->(period) { period.date_range.nil? ? all : where(date: period.date_range) }
@@ -11,9 +10,4 @@ class Valuation < ApplicationRecord
   def self.to_series
     TimeSeries.from_collection all, :value_money
   end
-
-  private
-    def sync_account
-      self.account.sync_later
-    end
 end

test/models/account_test.rb

@@ -110,4 +110,22 @@ class AccountTest < ActiveSupport::TestCase
     # We know EUR -> NZD exchange rate is not available in fixtures
     assert_equal 0, account.series(currency: "NZD").values.count
   end
+
+  test "should destroy dependent transactions" do
+    assert_difference("Transaction.count", -@account.transactions.count) do
+      @account.destroy
+    end
+  end
+
+  test "should destroy dependent balances" do
+    assert_difference("Account::Balance.count", -@account.balances.count) do
+      @account.destroy
+    end
+  end
+
+  test "should destroy dependent valuations" do
+    assert_difference("Valuation.count", -@account.valuations.count) do
+      @account.destroy
+    end
+  end
 end