diff --git a/app/controllers/family_exports_controller.rb b/app/controllers/family_exports_controller.rb index cc9226a54..c313da6af 100644 --- a/app/controllers/family_exports_controller.rb +++ b/app/controllers/family_exports_controller.rb @@ -2,7 +2,7 @@ class FamilyExportsController < ApplicationController include StreamExtensions before_action :require_admin - before_action :set_export, only: [ :download, :destroy ] + before_action :set_export, only: [ :download, :destroy, :cancel ] def new # Modal view for initiating export @@ -46,6 +46,14 @@ class FamilyExportsController < ApplicationController redirect_to family_exports_path, notice: t("family_exports.destroy.success") end + def cancel + if @export.force_fail! + redirect_to family_exports_path, notice: t(".cancelled") + else + redirect_to family_exports_path, alert: t(".not_cancellable") + end + end + private def set_export diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb index 0c13a57ae..7d36a783e 100644 --- a/app/controllers/imports_controller.rb +++ b/app/controllers/imports_controller.rb @@ -1,8 +1,8 @@ class ImportsController < ApplicationController include SettingsHelper - before_action :set_import, only: %i[show update publish destroy revert apply_template] - before_action :require_statement_import_permission!, only: %i[update publish destroy revert apply_template] + before_action :set_import, only: %i[show update publish destroy revert apply_template cancel] + before_action :require_statement_import_permission!, only: %i[update publish destroy revert apply_template cancel] def update # Handle both pdf_import[account_id] and import[account_id] param formats @@ -30,6 +30,14 @@ class ImportsController < ApplicationController redirect_back_or_to import_path(@import), alert: t(".max_rows_exceeded", max: @import.max_row_count) end + def cancel + if @import.force_fail! + redirect_to imports_path, notice: t(".cancelled") + else + redirect_to imports_path, alert: t(".not_cancellable") + end + end + def index @pagy, @imports = pagy(Current.family.imports.where(type: Import::TYPES).ordered, limit: safe_per_page) @breadcrumbs = [ diff --git a/app/controllers/syncs_controller.rb b/app/controllers/syncs_controller.rb new file mode 100644 index 000000000..97939159f --- /dev/null +++ b/app/controllers/syncs_controller.rb @@ -0,0 +1,23 @@ +class SyncsController < ApplicationController + def cancel + # for_family with resource_owner scoping: account-level syncs are only + # reachable for accounts the user can access; cross-family ids 404. + sync = Sync.for_family(Current.family, resource_owner: Current.user).find(params[:id]) + + # resource_owner only scopes the Account branch of for_family — provider + # item syncs match for every member, including accounts a restricted + # member cannot see. Provider connections are admin-managed surfaces, so + # cancelling their syncs requires admin too. Family- and account-level + # syncs stay member-cancellable (the accounts page shows those buttons + # to every member). + unless sync.syncable.is_a?(Account) || sync.syncable.is_a?(Family) || Current.user.admin? + raise ActiveRecord::RecordNotFound + end + + if sync.request_cancel! + redirect_back_or_to accounts_path, notice: t(".cancelled") + else + redirect_back_or_to accounts_path, alert: t(".not_cancellable") + end + end +end diff --git a/app/models/family/syncer.rb b/app/models/family/syncer.rb index 9952e2d55..03b799ecf 100644 --- a/app/models/family/syncer.rb +++ b/app/models/family/syncer.rb @@ -11,6 +11,10 @@ class Family::Syncer # Schedule child syncs child_syncables.each do |syncable| + # Cooperative cancellation: stop fanning out child syncs once a cancel + # has been requested (fresh read — the flag is set from another process). + break if sync.cancel_requested? + syncable.sync_later(parent_sync: sync, window_start_date: sync.window_start_date, window_end_date: sync.window_end_date) end end diff --git a/app/models/family_export.rb b/app/models/family_export.rb index 94e4fd836..0ac61aeef 100644 --- a/app/models/family_export.rb +++ b/app/models/family_export.rb @@ -12,6 +12,28 @@ class FamilyExport < ApplicationRecord scope :ordered, -> { order(created_at: :desc) } + # See Import::PRESUMED_LOST_AFTER — same dead-worker failure mode. Exports + # build in minutes; a pending/processing export idle for an hour is lost. + PRESUMED_LOST_AFTER = 1.hour + + def presumed_lost? + (pending? || processing?) && updated_at < PRESUMED_LOST_AFTER.ago + end + + # Escape hatch for exports whose background job died mid-flight; the + # with_lock re-check means a job finishing between render and click wins. + # Export generation is in-memory, so nothing is left behind — the user + # simply creates a new export. + def force_fail! + with_lock do + return false unless presumed_lost? + + update!(status: :failed) + end + + true + end + def filename "sure_export_#{created_at.strftime('%Y%m%d_%H%M%S')}.zip" end diff --git a/app/models/import.rb b/app/models/import.rb index 80b14ab26..d2565a3e8 100644 --- a/app/models/import.rb +++ b/app/models/import.rb @@ -2,6 +2,22 @@ class Import < ApplicationRecord MaxRowCountExceededError = Class.new(StandardError) MappingError = Class.new(StandardError) + # A hard-killed worker (OOM, SIGKILL during deploy) loses its in-flight job + # permanently, wedging the record in importing/reverting with no UI recourse. + # After this idle window the job is presumed lost and the user may force the + # record into a retryable terminal status. Imports finish in minutes, so an + # hour of silence dwarfs any legitimate run. + PRESUMED_LOST_AFTER = 1.hour + + # User-facing (shown as the import's error in the UI), so resolved through + # i18n at call time rather than frozen at boot. + def self.lost_error_message + I18n.t( + "imports.errors.presumed_lost", + default: "Marked as failed after the background job was presumed lost. The imported data was rolled back — you can safely try again." + ) + end + # Shared CSV upload/content limit for web and API imports, including preflight. MAX_CSV_SIZE = 10.megabytes MAX_PDF_SIZE = 25.megabytes @@ -167,6 +183,29 @@ class Import < ApplicationRecord RevertImportJob.perform_later(self) end + def presumed_lost? + (importing? || reverting?) && updated_at < PRESUMED_LOST_AFTER.ago + end + + # Escape hatch for imports whose background job died mid-flight. Only + # allowed once the record has been idle past PRESUMED_LOST_AFTER, and the + # with_lock re-check means a job finishing between page render and button + # click wins. Every import! runs in a single DB transaction, so a lost job + # rolled its data back — failing the record is safe and re-enables the + # existing "Try again" (failed) / revert-retry (revert_failed) paths. + def force_fail!(error_message = self.class.lost_error_message) + with_lock do + return false unless presumed_lost? + + update!( + status: reverting? ? :revert_failed : :failed, + error: error_message + ) + end + + true + end + def revert Import.transaction do accounts.destroy_all diff --git a/app/models/pdf_import.rb b/app/models/pdf_import.rb index 616b81644..ce2413216 100644 --- a/app/models/pdf_import.rb +++ b/app/models/pdf_import.rb @@ -29,6 +29,22 @@ class PdfImport < Import end end + # A PdfImport's importing status is a processing claim (AI extraction or + # publish). Release a lost claim back to pending so the user can re-trigger + # processing, mirroring ProcessPdfJob's own reclaim; lost reverts keep the + # base revert_failed behavior. + def force_fail!(error_message = Import.lost_error_message) + return super if reverting? + + with_lock do + return false unless presumed_lost? + + update!(status: :pending) + end + + true + end + def import! raise "Account required for PDF import" unless account.present? diff --git a/app/models/simplefin_item/syncer.rb b/app/models/simplefin_item/syncer.rb index 650ce4b69..b8e184653 100644 --- a/app/models/simplefin_item/syncer.rb +++ b/app/models/simplefin_item/syncer.rb @@ -120,14 +120,32 @@ class SimplefinItem::Syncer end def mark_completed(sync) - if sync.may_start? - sync.start! + # Re-read under a row lock before finalizing: this job holds an + # in-memory copy loaded before the run, and the sync may have been + # cancelled (Sync#request_cancel! finalized it to stale) or otherwise + # terminalized while the work ran. An unguarded complete! here would + # overwrite that terminal status and resurrect a cancelled sync. + finalized = sync.with_lock do + if sync.cancel_requested_at? || sync.terminal? + false + else + sync.start! if sync.may_start? + sync.complete! if sync.may_complete? + true + end end - if sync.may_complete? - sync.complete! - else - # If aasm not used, at least set status text - sync.update!(status: :completed) if sync.status != "completed" + + unless finalized + DebugLogEntry.capture( + category: "provider_sync", + level: "info", + message: "SimplefinItem::Syncer#mark_completed skipped: sync was #{sync.status} (cancel requested: #{sync.cancel_requested_at.present?})", + source: self.class.name, + family: simplefin_item.family, + provider_key: "simplefin", + metadata: { sync_id: sync.id, status: sync.status, cancel_requested_at: sync.cancel_requested_at } + ) + return end # After completion, compute and persist compact post-run stats for the summary panel diff --git a/app/models/sync.rb b/app/models/sync.rb index 6d34c7ecf..9c5399028 100644 --- a/app/models/sync.rb +++ b/app/models/sync.rb @@ -17,7 +17,9 @@ class Sync < ApplicationRecord scope :ordered, -> { order(created_at: :desc, id: :desc) } scope :incomplete, -> { where("syncs.status IN (?)", %w[pending syncing]) } - scope :visible, -> { incomplete.where("syncs.created_at > ?", VISIBLE_FOR.ago) } + # Cancel-requested syncs are excluded so spinners clear immediately and + # sync_later stops piggybacking new requests onto a dying sync. + scope :visible, -> { incomplete.where("syncs.created_at > ?", VISIBLE_FOR.ago).where(cancel_requested_at: nil) } after_commit :update_family_sync_timestamp, on: [ :create, :update ] @@ -157,6 +159,45 @@ class Sync < ApplicationRecord end end + # Requests cooperative cancellation of this sync tree. Only this sync + # carries the flag: pending descendants are marked stale immediately (their + # queued jobs no-op via the may_start? guard), while descendants whose jobs + # are already running finish their work honestly — finalization then + # resolves this sync to stale instead of completed. Returns false when the + # sync is already terminal. + def request_cancel! + result = with_lock do + if pending? + # Job hasn't started — safe to resolve immediately; the queued job + # will no-op via the may_start? guard. + update!(cancel_requested_at: Time.current) + mark_stale! + :cancelled_before_start + elsif syncing? + update!(cancel_requested_at: Time.current) + :cancel_requested + end + end + return false if result.nil? + + # Both paths cascade: a pending sync resolved above went terminal without + # its job ever running, so nothing else will ever call + # finalize_if_all_children_finalized for it — without this, a parent + # waiting on the cancelled child stays syncing until the 24h sweep. + # finalize_if_all_children_finalized re-reads under lock!, so it safely + # no-ops on this sync's own branch when already terminal. + cancel_pending_descendants! + finalize_if_all_children_finalized + + true + end + + # Fresh DB read — cancellation is requested from the web process while this + # sync's job holds a stale in-memory copy of the record. + def cancel_requested? + self.class.where(id: id).pick(:cancel_requested_at).present? + end + # Finalizes the current sync AND parent (if it exists) def finalize_if_all_children_finalized Sync.transaction do @@ -170,7 +211,12 @@ class Sync < ApplicationRecord return unless all_children_finalized? if syncing? - if has_failed_children? + if cancel_requested_at? + # User asked for cancellation while work was in flight. Whatever + # children completed keep their data; the tree resolves to stale + # (which also skips post-sync below). + mark_stale! + elsif has_failed_children? fail! else complete! @@ -211,6 +257,14 @@ class Sync < ApplicationRecord ) end + protected + def cancel_pending_descendants! + children.incomplete.find_each do |child| + child.with_lock { child.mark_stale! if child.pending? } + child.cancel_pending_descendants! + end + end + private def log_status_change Rails.logger.info("changing from #{aasm.from_state} to #{aasm.to_state} (event: #{aasm.current_event})") diff --git a/app/views/accounts/index.html.erb b/app/views/accounts/index.html.erb index 4d7f161b8..85d344024 100644 --- a/app/views/accounts/index.html.erb +++ b/app/views/accounts/index.html.erb @@ -8,6 +8,16 @@ disabled: Current.family.syncing?, frame: :_top ) %> + <% if (family_sync = Current.family.syncs.visible.first) %> + <%= button_to cancel_sync_path(family_sync), + method: :post, + class: "flex items-center gap-1 text-sm text-secondary hover:text-primary", + aria: { label: t(".cancel_sync") }, + data: { turbo_frame: :_top } do %> + <%= icon "circle-x", class: "w-4 h-4" %> + <%= t(".cancel_sync") %> + <% end %> + <% end %> <%= render DS::Link.new( text: t(".new_account"), href: new_account_path(return_to: accounts_path), diff --git a/app/views/family_exports/index.html.erb b/app/views/family_exports/index.html.erb index 266d8e96a..69dfbf0db 100644 --- a/app/views/family_exports/index.html.erb +++ b/app/views/family_exports/index.html.erb @@ -67,6 +67,18 @@ <% if export.processing? || export.pending? %>
+ <% if export.presumed_lost? %> + <%= button_to cancel_family_export_path(export), + method: :post, + class: "flex items-center gap-2 text-secondary hover:text-primary", + aria: { label: t("family_exports.table.row.actions.mark_failed") }, + data: { + turbo_confirm: t("family_exports.table.row.actions.confirm_mark_failed"), + turbo_frame: "_top" + } do %> + <%= icon "circle-x", class: "w-4 h-4" %> + <% end %> + <% end %>
<%= t("family_exports.exporting") %>
diff --git a/app/views/imports/index.html.erb b/app/views/imports/index.html.erb index 626e6140f..a8f6c6e70 100644 --- a/app/views/imports/index.html.erb +++ b/app/views/imports/index.html.erb @@ -86,6 +86,18 @@ <% end %> <% else %> + <% if import.presumed_lost? %> + <%= button_to cancel_import_path(import), + method: :post, + class: "flex items-center gap-2 text-secondary hover:text-primary", + aria: { label: t("imports.table.row.actions.mark_failed") }, + data: { + turbo_confirm: t("imports.table.row.actions.confirm_mark_failed") + } do %> + <%= icon "circle-x", class: "w-5 h-5" %> + <% end %> + <% end %> + <%= button_to import_path(import), method: :delete, class: "flex items-center gap-2 text-destructive hover:text-destructive-hover", diff --git a/config/locales/views/accounts/en.yml b/config/locales/views/accounts/en.yml index 43a9e65b5..398bfa38a 100644 --- a/config/locales/views/accounts/en.yml +++ b/config/locales/views/accounts/en.yml @@ -53,6 +53,7 @@ en: exclude_from_reports: Exclude from all reports index: accounts: Accounts + cancel_sync: Cancel sync manual_accounts: other_accounts: Other accounts new_account: New account diff --git a/config/locales/views/family_exports/en.yml b/config/locales/views/family_exports/en.yml index f81c29b49..12f614449 100644 --- a/config/locales/views/family_exports/en.yml +++ b/config/locales/views/family_exports/en.yml @@ -6,6 +6,9 @@ en: success: Export started. You'll be able to download it shortly. delete_confirmation: Are you sure you want to delete this export? This action cannot be undone. delete_failed_confirmation: Are you sure you want to delete this failed export? + cancel: + cancelled: Export marked as failed. You can create a new export right away. + not_cancellable: This export can't be marked as failed right now — its job may still be running. Try again once it has been inactive for an hour. destroy: success: Export deleted successfully export_not_ready: Export not ready for download @@ -40,4 +43,6 @@ en: actions: delete: Delete download: Download + mark_failed: Mark as failed + confirm_mark_failed: This export has been inactive for a while and its background job was likely interrupted. Mark it as failed? You can create a new export right away. empty: No exports yet. diff --git a/config/locales/views/imports/en.yml b/config/locales/views/imports/en.yml index 017a731c5..d5e5ce852 100644 --- a/config/locales/views/imports/en.yml +++ b/config/locales/views/imports/en.yml @@ -277,6 +277,9 @@ en: max_rows_exceeded: "Your import exceeds the maximum row count of %{max}." revert: started: "Import is reverting in the background." + cancel: + cancelled: "Import marked as failed. You can try again from the import page." + not_cancellable: "This import can't be marked as failed right now — its job may still be running. Try again once it has been inactive for an hour." apply_template: template_applied: "Template applied." no_template_found: "No template found, please manually configure your import." @@ -376,6 +379,8 @@ en: confirm_revert: This will delete transactions that were imported, but you will still be able to review and re-import your data at any time. delete: Delete view: View + mark_failed: Mark as failed + confirm_mark_failed: This import has been inactive for a while and its background job was likely interrupted. Mark it as failed so you can try again? No imported data is kept. empty: No imports yet. new: description: Import from a financial tool or upload raw data files. @@ -429,6 +434,7 @@ en: back_to_imports: Back to imports errors: custom_column_requires_inflow: "Custom column imports require an inflow column to be selected" + presumed_lost: "Marked as failed after the background job was presumed lost. The imported data was rolled back — you can safely try again." document_types: bank_statement: Bank Statement credit_card_statement: Credit Card Statement diff --git a/config/locales/views/syncs/en.yml b/config/locales/views/syncs/en.yml new file mode 100644 index 000000000..66cd17afe --- /dev/null +++ b/config/locales/views/syncs/en.yml @@ -0,0 +1,6 @@ +--- +en: + syncs: + cancel: + cancelled: Sync cancelled. Anything already synced is kept; queued work was stopped. + not_cancellable: This sync has already finished. diff --git a/config/routes.rb b/config/routes.rb index c46497896..50b4ccf55 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -237,6 +237,13 @@ Rails.application.routes.draw do resources :family_exports, only: %i[new create index destroy] do member do get :download + post :cancel + end + end + + resources :syncs, only: [] do + member do + post :cancel end end @@ -389,6 +396,7 @@ Rails.application.routes.draw do post :publish put :revert put :apply_template + post :cancel end resource :upload, only: %i[show update], module: :import diff --git a/db/migrate/20260714120000_add_cancel_requested_at_to_syncs.rb b/db/migrate/20260714120000_add_cancel_requested_at_to_syncs.rb new file mode 100644 index 000000000..91be06a8c --- /dev/null +++ b/db/migrate/20260714120000_add_cancel_requested_at_to_syncs.rb @@ -0,0 +1,5 @@ +class AddCancelRequestedAtToSyncs < ActiveRecord::Migration[7.2] + def change + add_column :syncs, :cancel_requested_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 94e46d191..19a0d2907 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: 2026_07_13_090000) do +ActiveRecord::Schema[7.2].define(version: 2026_07_14_120000) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -1968,6 +1968,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_07_13_090000) do t.date "window_start_date" t.date "window_end_date" t.text "sync_stats" + t.datetime "cancel_requested_at" t.index ["parent_id"], name: "index_syncs_on_parent_id" t.index ["status"], name: "index_syncs_on_status" t.index ["syncable_type", "syncable_id"], name: "index_syncs_on_syncable" diff --git a/test/controllers/family_exports_controller_test.rb b/test/controllers/family_exports_controller_test.rb index 217a6c170..29d961ac5 100644 --- a/test/controllers/family_exports_controller_test.rb +++ b/test/controllers/family_exports_controller_test.rb @@ -28,6 +28,37 @@ class FamilyExportsControllerTest < ActionDispatch::IntegrationTest assert_select "h2", text: "Export your data" end + test "admin can mark a lost export as failed" do + export = @family.family_exports.create! + export.update_columns(status: "processing", updated_at: 2.hours.ago) + + post cancel_family_export_path(export) + + assert_redirected_to family_exports_path + assert_equal "failed", export.reload.status + end + + test "cancel refuses an export that is not presumed lost" do + export = @family.family_exports.create! + export.update_columns(status: "processing", updated_at: 5.minutes.ago) + + post cancel_family_export_path(export) + + assert_equal I18n.t("family_exports.cancel.not_cancellable"), flash[:alert] + assert_equal "processing", export.reload.status + end + + test "non-admin cannot cancel an export" do + export = @family.family_exports.create! + export.update_columns(status: "processing", updated_at: 2.hours.ago) + + sign_in @non_admin + post cancel_family_export_path(export) + + assert_redirected_to root_path + assert_equal "processing", export.reload.status + end + test "admin can create export" do assert_enqueued_with(job: FamilyDataExportJob) do post family_exports_path diff --git a/test/controllers/imports_controller_test.rb b/test/controllers/imports_controller_test.rb index 34da55633..42ca620a8 100644 --- a/test/controllers/imports_controller_test.rb +++ b/test/controllers/imports_controller_test.rb @@ -26,6 +26,39 @@ class ImportsControllerTest < ActionDispatch::IntegrationTest assert_select "turbo-frame#modal" end + test "cancel marks a lost import as failed" do + import = imports(:transaction) + import.update_columns(status: "importing", updated_at: 2.hours.ago) + + post cancel_import_path(import) + + assert_redirected_to imports_path + assert_equal "failed", import.reload.status + assert_equal Import.lost_error_message, import.error + end + + test "cancel refuses an import that is not presumed lost" do + import = imports(:transaction) + import.update_columns(status: "importing", updated_at: 5.minutes.ago) + + post cancel_import_path(import) + + assert_equal I18n.t("imports.cancel.not_cancellable"), flash[:alert] + assert_equal "importing", import.reload.status + end + + test "cannot cancel another family's import" do + import = imports(:transaction) + import.update_columns(status: "importing", updated_at: 2.hours.ago) + + sign_in users(:empty) + + post cancel_import_path(import) + + assert_response :not_found + assert_equal "importing", import.reload.status + end + test "shows disabled account-dependent imports when family has no accounts" do sign_in users(:empty) diff --git a/test/controllers/syncs_controller_test.rb b/test/controllers/syncs_controller_test.rb new file mode 100644 index 000000000..1082b3967 --- /dev/null +++ b/test/controllers/syncs_controller_test.rb @@ -0,0 +1,54 @@ +require "test_helper" + +class SyncsControllerTest < ActionDispatch::IntegrationTest + setup do + sign_in @user = users(:family_admin) + end + + test "member can cancel their family's sync" do + sync = Sync.create!(syncable: @user.family, status: :syncing) + + post cancel_sync_path(sync) + + assert_redirected_to accounts_path + assert_not_nil sync.reload.cancel_requested_at + end + + test "cancelling a finished sync is a no-op with an alert" do + sync = Sync.create!(syncable: @user.family, status: :completed) + + post cancel_sync_path(sync) + + assert_redirected_to accounts_path + assert_equal I18n.t("syncs.cancel.not_cancellable"), flash[:alert] + assert_equal "completed", sync.reload.status + end + + test "cannot cancel another family's sync" do + other_family_sync = Sync.create!(syncable: users(:empty).family, status: :syncing) + + post cancel_sync_path(other_family_sync) + + assert_response :not_found + assert_equal "syncing", other_family_sync.reload.status + end + + test "non-admin member cannot cancel a provider item sync" do + sign_in users(:family_member) + provider_sync = Sync.create!(syncable: plaid_items(:one), status: :syncing) + + post cancel_sync_path(provider_sync) + + assert_response :not_found + assert_equal "syncing", provider_sync.reload.status + end + + test "admin can cancel a provider item sync" do + provider_sync = Sync.create!(syncable: plaid_items(:one), status: :syncing) + + post cancel_sync_path(provider_sync) + + assert_redirected_to accounts_path + assert_not_nil provider_sync.reload.cancel_requested_at + end +end diff --git a/test/models/family_export_test.rb b/test/models/family_export_test.rb index c6f86c8bc..bad6fd2b5 100644 --- a/test/models/family_export_test.rb +++ b/test/models/family_export_test.rb @@ -14,6 +14,20 @@ class FamilyExportTest < ActiveSupport::TestCase assert_equal "pending", @export.status end + test "force_fail! fails a lost export but refuses fresh or terminal ones" do + @export.update_columns(status: "processing", updated_at: 2.hours.ago) + assert @export.force_fail! + assert_equal "failed", @export.reload.status + + fresh = @family.family_exports.create! + fresh.update_columns(status: "processing", updated_at: 5.minutes.ago) + assert_not fresh.force_fail! + assert_equal "processing", fresh.reload.status + + assert_not @export.force_fail! + assert_equal "failed", @export.reload.status + end + test "can have export file attached" do @export.export_file.attach( io: StringIO.new("test content"), diff --git a/test/models/import_test.rb b/test/models/import_test.rb new file mode 100644 index 000000000..c7736cf57 --- /dev/null +++ b/test/models/import_test.rb @@ -0,0 +1,42 @@ +require "test_helper" + +class ImportTest < ActiveSupport::TestCase + test "force_fail! refuses records that have not been idle long enough" do + import = imports(:transaction) + import.update_columns(status: "importing", updated_at: 5.minutes.ago) + + assert_not import.force_fail! + assert_equal "importing", import.reload.status + end + + test "force_fail! fails a lost import and keeps reverts retryable" do + lost_import = imports(:transaction) + lost_import.update_columns(status: "importing", updated_at: 2.hours.ago) + + assert lost_import.force_fail! + assert_equal "failed", lost_import.reload.status + assert_equal Import.lost_error_message, lost_import.error + + lost_revert = imports(:trade) + lost_revert.update_columns(status: "reverting", updated_at: 2.hours.ago) + + assert lost_revert.force_fail! + assert_equal "revert_failed", lost_revert.reload.status + end + + test "force_fail! refuses terminal statuses" do + import = imports(:transaction) + import.update_columns(status: "complete", updated_at: 2.hours.ago) + + assert_not import.force_fail! + assert_equal "complete", import.reload.status + end + + test "force_fail! releases a lost PdfImport claim back to pending" do + pdf = imports(:pdf) + pdf.update_columns(status: "importing", updated_at: 2.hours.ago) + + assert pdf.force_fail! + assert_equal "pending", pdf.reload.status + end +end diff --git a/test/models/sync_test.rb b/test/models/sync_test.rb index 379750f12..08fa5aa2d 100644 --- a/test/models/sync_test.rb +++ b/test/models/sync_test.rb @@ -212,6 +212,113 @@ class SyncTest < ActiveSupport::TestCase assert_equal "provider blew up", sync.error end + test "request_cancel! resolves a pending sync immediately" do + sync = Sync.create!(syncable: accounts(:depository)) + + assert sync.request_cancel! + assert_equal "stale", sync.reload.status + + # The queued job later no-ops via the may_start? guard + accounts(:depository).expects(:perform_sync).never + sync.perform + assert_equal "stale", sync.reload.status + end + + test "cancelling a pending child finalizes its waiting parent" do + family = families(:dylan_family) + parent = Sync.create!(syncable: family, status: :syncing) + child = Sync.create!(syncable: accounts(:depository), parent: parent, status: :pending) + + Family.any_instance.expects(:perform_post_sync).once + Family.any_instance.expects(:broadcast_sync_complete).once + + assert child.request_cancel! + + # The child's queued job will no-op via may_start?, so nothing else ever + # finalizes the parent — request_cancel! itself must cascade or the + # parent hangs in syncing until the 24h sweep. + assert_equal "stale", child.reload.status + assert_equal "completed", parent.reload.status + end + + test "a late provider complete! cannot resurrect a cancelled sync" do + item = SimplefinItem.create!(family: families(:dylan_family), name: "SF Conn", access_url: "https://example.com/access") + sync = Sync.create!(syncable: item, status: :syncing) + + # Simulates the Sidekiq job's in-memory copy, loaded before cancellation + in_job_copy = Sync.find(sync.id) + + assert sync.request_cancel! + assert_equal "stale", sync.reload.status + + SimplefinItem::Syncer.new(item).send(:mark_completed, in_job_copy) + + assert_equal "stale", sync.reload.status + end + + test "request_cancel! returns false for terminal syncs" do + sync = Sync.create!(syncable: accounts(:depository), status: :completed) + + assert_not sync.request_cancel! + assert_equal "completed", sync.reload.status + end + + test "cancelling a running tree stales pending children and resolves the root to stale without post-sync" do + family = families(:dylan_family) + plaid_item = plaid_items(:one) + account = accounts(:connected) + + family_sync = Sync.create!(syncable: family, status: :syncing) + running_child = Sync.create!(syncable: plaid_item, parent: family_sync) + pending_child = Sync.create!(syncable: account, parent: running_child, status: :pending) + + running_child.start! + + assert family_sync.request_cancel! + + # Pending descendants are resolved immediately; running ones are left alone + assert_equal "stale", pending_child.reload.status + assert_equal "syncing", running_child.reload.status + assert_equal "syncing", family_sync.reload.status + + # The running child finishes honestly; the cancelled root resolves to + # stale and must not re-run family transfer matching / rules / broadcasts + PlaidItem.any_instance.expects(:perform_post_sync).once + PlaidItem.any_instance.expects(:broadcast_sync_complete).once + Family.any_instance.expects(:perform_post_sync).never + Family.any_instance.expects(:broadcast_sync_complete).never + + # Simulate the in-flight job finishing after the cancel was requested + running_child.finalize_if_all_children_finalized + + assert_equal "completed", running_child.reload.status + assert_equal "stale", family_sync.reload.status + end + + test "cancel-requested syncs are not visible and do not swallow new sync requests" do + account = accounts(:depository) + Sync.where(syncable: account).destroy_all + + sync = Sync.create!(syncable: account, status: :syncing, cancel_requested_at: Time.current) + + assert_not account.syncing? + + new_sync = nil + assert_difference "Sync.count", 1 do + new_sync = account.sync_later + end + assert_not_equal sync.id, new_sync.id + end + + test "family syncer stops scheduling children once cancel is requested" do + family = families(:dylan_family) + sync = Sync.create!(syncable: family, status: :syncing, cancel_requested_at: Time.current) + + assert_no_difference "Sync.count" do + Family::Syncer.new(family).perform_sync(sync) + end + end + test "clean marks stale incomplete rows" do stale_pending = Sync.create!( syncable: accounts(:depository),