Fix attribute locking namespace conflict, duplicate syncs

app/controllers/transactions_controller.rb

@@ -61,7 +61,7 @@ class TransactionsController < ApplicationController
     if @entry.save
       @entry.sync_account_later
       @entry.lock_saved_attributes!
-      @entry.transaction.lock!(:tag_ids) if @entry.transaction.tags.any?
+      @entry.transaction.lock_attr!(:tag_ids) if @entry.transaction.tags.any?
 
       flash[:notice] = "Transaction created"
 
@@ -88,7 +88,7 @@ class TransactionsController < ApplicationController
 
       @entry.sync_account_later
       @entry.lock_saved_attributes!
-      @entry.transaction.lock!(:tag_ids) if @entry.transaction.tags.any?
+      @entry.transaction.lock_attr!(:tag_ids) if @entry.transaction.tags.any?
 
       respond_to do |format|
         format.html { redirect_back_or_to account_path(@entry.account), notice: "Transaction updated" }

app/models/concerns/enrichable.rb

@@ -42,17 +42,17 @@ module Enrichable
     !locked?(attr)
   end
 
-  def lock!(attr)
+  def lock_attr!(attr)
     update!(locked_attributes: locked_attributes.merge(attr.to_s => Time.current))
   end
 
-  def unlock!(attr)
+  def unlock_attr!(attr)
     update!(locked_attributes: locked_attributes.except(attr.to_s))
   end
 
   def lock_saved_attributes!
     saved_changes.keys.reject { |attr| ignored_enrichable_attributes.include?(attr) }.each do |attr|
-      lock!(attr)
+      lock_attr!(attr)
     end
   end
 

app/models/concerns/syncable.rb

@@ -9,20 +9,28 @@ module Syncable
     raise NotImplementedError, "Subclasses must implement the syncing? method"
   end
 
+  # Schedules a sync for syncable.  If there is an existing sync pending/syncing for this syncable,
+  # we do not create a new sync, and attempt to expand the sync window if needed.
   def sync_later(parent_sync: nil, window_start_date: nil, window_end_date: nil)
     Sync.transaction do
-      # Since we're scheduling a new sync, mark old syncs for this syncable as stale
-      self.syncs.incomplete.find_each(&:mark_stale!)
+      with_lock do
+        sync = self.syncs.incomplete.first
 
-      new_sync = self.syncs.create!(
-        parent: parent_sync,
-        window_start_date: window_start_date,
-        window_end_date: window_end_date
-      )
+        if sync
+          Rails.logger.info("There is an existing sync, expanding window if needed (#{sync.id})")
+          sync.expand_window_if_needed(window_start_date, window_end_date)
+        else
+          sync = self.syncs.create!(
+            parent: parent_sync,
+            window_start_date: window_start_date,
+            window_end_date: window_end_date
+          )
 
-      SyncJob.perform_later(new_sync)
+          SyncJob.perform_later(sync)
+        end
 
-      new_sync
+        sync
+      end
     end
   end
 

app/models/entry.rb

@@ -85,7 +85,7 @@ class Entry < ApplicationRecord
           entry.update! bulk_attributes
 
           entry.lock_saved_attributes!
-          entry.entryable.lock!(:tag_ids) if entry.transaction? && entry.transaction.tags.any?
+          entry.entryable.lock_attr!(:tag_ids) if entry.transaction? && entry.transaction.tags.any?
         end
       end
 

app/models/family/auto_categorizer.rb

@@ -27,7 +27,7 @@ class Family::AutoCategorizer
     end
 
     scope.each do |transaction|
-      transaction.lock!(:category_id)
+      transaction.lock_attr!(:category_id)
 
       auto_categorization = result.data.find { |c| c.transaction_id == transaction.id }
 

app/models/family/auto_merchant_detector.rb

@@ -27,7 +27,7 @@ class Family::AutoMerchantDetector
     end
 
     scope.each do |transaction|
-      transaction.lock!(:merchant_id)
+      transaction.lock_attr!(:merchant_id)
 
       auto_detection = result.data.find { |c| c.transaction_id == transaction.id }
 

app/models/sync.rb

@@ -95,6 +95,29 @@ class Sync < ApplicationRecord
     parent&.finalize_if_all_children_finalized
   end
 
+  # If a sync is pending, we can adjust the window if new syncs are created with a wider window.
+  def expand_window_if_needed(new_window_start_date, new_window_end_date)
+    return unless pending?
+    return if self.window_start_date.nil? && self.window_end_date.nil? # already as wide as possible
+
+    earliest_start_date = if self.window_start_date && new_window_start_date
+      [self.window_start_date, new_window_start_date].min
+    else
+      nil
+    end
+
+    latest_end_date = if self.window_end_date && new_window_end_date
+      [self.window_end_date, new_window_end_date].max
+    else
+      nil
+    end
+
+    update(
+      window_start_date: earliest_start_date,
+      window_end_date: latest_end_date
+    )
+  end
+
   private
     def log_status_change
       Rails.logger.info("changing from #{aasm.from_state} to #{aasm.to_state} (event: #{aasm.current_event})")