From 552aa59aaf5e476c81d81483ca9118f872730d2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Wed, 15 Apr 2026 15:39:28 +0100 Subject: [PATCH] fix: limit bank sync emails to one per day (#290) ## Summary - move bank transaction sync emails from per-connection inline sends to a unique per-user daily job - send at most one bank sync email per user per day while still including all unreported enable-banking transactions since last reported mail - keep first-ever connection sync silent and add coverage for same-day suppression and next-day catch-up emails ## Testing - php artisan test --compact tests/Feature/OpenBanking/SyncBankingConnectionJobTest.php --- .github/workflows/automerge.yml | 12 +- app/Enums/DripEmailType.php | 1 + ...endDailyBankTransactionsSyncedEmailJob.php | 84 ++++++++++ app/Jobs/SyncBankingConnectionJob.php | 20 +-- .../SyncBankingConnectionJobTest.php | 148 +++++++++++++++--- 5 files changed, 228 insertions(+), 37 deletions(-) create mode 100644 app/Jobs/SendDailyBankTransactionsSyncedEmailJob.php diff --git a/.github/workflows/automerge.yml b/.github/workflows/automerge.yml index 9974202c..beb8933f 100644 --- a/.github/workflows/automerge.yml +++ b/.github/workflows/automerge.yml @@ -1,8 +1,8 @@ -name: Automerge +name: automerge on: workflow_run: - workflows: ["CI"] + workflows: ['CI'] types: [completed] jobs: @@ -15,7 +15,7 @@ jobs: contents: write pull-requests: write steps: - - name: Merge PR with Automerge label + - name: Merge PR with automerge label env: GH_TOKEN: ${{ secrets.MERGE_TOKEN }} REPO: ${{ github.repository }} @@ -29,10 +29,10 @@ jobs: exit 0 fi - HAS_LABEL=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json labels --jq '[.labels[].name] | map(select(. == "Automerge")) | length') + HAS_LABEL=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json labels --jq '[.labels[].name] | map(select(. == "automerge")) | length') if [ "$HAS_LABEL" -eq 0 ]; then - echo "PR #$PR_NUMBER does not have Automerge label, skipping" + echo "PR #$PR_NUMBER does not have automerge label, skipping" exit 0 fi @@ -53,5 +53,5 @@ jobs: exit 0 fi - echo "PR #$PR_NUMBER has Automerge label and all checks passed. Merging..." + echo "PR #$PR_NUMBER has automerge label and all checks passed. Merging..." gh pr merge "$PR_NUMBER" --squash --repo "$REPO" --delete-branch diff --git a/app/Enums/DripEmailType.php b/app/Enums/DripEmailType.php index 611d58d9..c92fcadc 100644 --- a/app/Enums/DripEmailType.php +++ b/app/Enums/DripEmailType.php @@ -4,6 +4,7 @@ namespace App\Enums; enum DripEmailType: string { + case BankTransactionsSynced = 'bank_transactions_synced'; case Welcome = 'welcome'; case OnboardingReminder = 'onboarding_reminder'; case PromoCode = 'promo_code'; diff --git a/app/Jobs/SendDailyBankTransactionsSyncedEmailJob.php b/app/Jobs/SendDailyBankTransactionsSyncedEmailJob.php new file mode 100644 index 00000000..842f232a --- /dev/null +++ b/app/Jobs/SendDailyBankTransactionsSyncedEmailJob.php @@ -0,0 +1,84 @@ + + */ + public $backoff = [2, 5, 10, 30]; + + public function __construct( + public User $user, + public string $reportDate, + ) { + $this->onQueue('emails'); + } + + public function handle(): void + { + $lastSentMailLog = UserMailLog::query() + ->where('user_id', $this->user->id) + ->where('email_type', DripEmailType::BankTransactionsSynced) + ->latest('sent_at') + ->first(); + + if ($lastSentMailLog?->email_identifier === $this->reportDate) { + return; + } + + $pendingTransactions = Transaction::query() + ->where('user_id', $this->user->id) + ->where('source', TransactionSource::EnableBanking) + ->when($lastSentMailLog?->sent_at, fn ($query, $lastSentAt) => $query->where('created_at', '>', $lastSentAt)) + ->whereHas('account.bankingConnection') + ->with('account.bank') + ->get(); + + if ($pendingTransactions->isEmpty()) { + return; + } + + $transactionsPerBank = $pendingTransactions + ->groupBy(fn (Transaction $transaction) => $transaction->account->bank->name ?? __('Unknown Bank')) + ->map(fn ($transactions) => $transactions->count()) + ->all(); + + Mail::to($this->user)->send(new BankTransactionsSyncedEmail( + $this->user, + $pendingTransactions->count(), + $transactionsPerBank, + )); + + UserMailLog::create([ + 'user_id' => $this->user->id, + 'email_type' => DripEmailType::BankTransactionsSynced, + 'email_identifier' => $this->reportDate, + 'sent_at' => now(), + ]); + } + + public function uniqueId(): string + { + return $this->user->id.':'.$this->reportDate; + } +} diff --git a/app/Jobs/SyncBankingConnectionJob.php b/app/Jobs/SyncBankingConnectionJob.php index b7e57b54..c77406c0 100644 --- a/app/Jobs/SyncBankingConnectionJob.php +++ b/app/Jobs/SyncBankingConnectionJob.php @@ -5,7 +5,6 @@ namespace App\Jobs; use App\Enums\BankingConnectionStatus; use App\Enums\BankingSyncLogStatus; use App\Mail\BankingConnectionAuthFailedEmail; -use App\Mail\BankTransactionsSyncedEmail; use App\Models\BankingConnection; use App\Models\BankingSyncLog; use App\Services\Banking\BalanceSyncService; @@ -82,6 +81,13 @@ class SyncBankingConnectionJob implements ShouldBeUnique, ShouldQueue $this->syncBitpanda($connection); } else { $metadata = $this->syncEnableBanking($connection, $transactionSync, $balanceSync, $isFirstSync); + + if (! $isFirstSync) { + SendDailyBankTransactionsSyncedEmailJob::dispatch( + $connection->user, + now()->toDateString(), + ); + } } $connection->update([ @@ -274,17 +280,7 @@ class SyncBankingConnectionJob implements ShouldBeUnique, ShouldQueue } } - $totalTransactions = array_sum($transactionsPerBank); - - if (! $isFirstSync && $totalTransactions > 0) { - Mail::to($connection->user)->send(new BankTransactionsSyncedEmail( - $connection->user, - $totalTransactions, - $transactionsPerBank, - )); - } - - return ['transactions_synced' => $totalTransactions, 'transactions_per_bank' => $transactionsPerBank]; + return ['transactions_synced' => array_sum($transactionsPerBank), 'transactions_per_bank' => $transactionsPerBank]; } private function friendlyErrorMessage(\Throwable $e): string diff --git a/tests/Feature/OpenBanking/SyncBankingConnectionJobTest.php b/tests/Feature/OpenBanking/SyncBankingConnectionJobTest.php index faee6ba2..a20c195d 100644 --- a/tests/Feature/OpenBanking/SyncBankingConnectionJobTest.php +++ b/tests/Feature/OpenBanking/SyncBankingConnectionJobTest.php @@ -1,6 +1,9 @@ onboarded()->create(); $bank = Bank::factory()->create(['name' => 'Test Bank']); @@ -156,15 +161,14 @@ test('sends email when new transactions are synced on subsequent sync', function $job = new SyncBankingConnectionJob($connection); $job->handle($transactionSync, $balanceSync); - Mail::assertQueued(BankTransactionsSyncedEmail::class, function ($mail) use ($user) { - return $mail->totalTransactions === 5 - && $mail->transactionsPerBank === ['Test Bank' => 5] - && $mail->hasTo($user->email); + Queue::assertPushed(SendDailyBankTransactionsSyncedEmailJob::class, function ($job) use ($user) { + return $job->user->is($user) + && $job->reportDate === now()->toDateString(); }); }); test('does not send email on first sync', function () { - Mail::fake(); + Queue::fake(); $user = User::factory()->onboarded()->create(); $bank = Bank::factory()->create(['name' => 'Test Bank']); @@ -189,11 +193,11 @@ test('does not send email on first sync', function () { $job = new SyncBankingConnectionJob($connection); $job->handle($transactionSync, $balanceSync); - Mail::assertNotQueued(BankTransactionsSyncedEmail::class); + Queue::assertNotPushed(SendDailyBankTransactionsSyncedEmailJob::class); }); -test('does not send email when zero new transactions', function () { - Mail::fake(); +test('schedules daily bank sync email check when subsequent sync imports zero new transactions', function () { + Queue::fake(); $user = User::factory()->onboarded()->create(); $connection = BankingConnection::factory()->create([ @@ -215,11 +219,11 @@ test('does not send email when zero new transactions', function () { $job = new SyncBankingConnectionJob($connection); $job->handle($transactionSync, $balanceSync); - Mail::assertNotQueued(BankTransactionsSyncedEmail::class); + Queue::assertPushed(SendDailyBankTransactionsSyncedEmailJob::class); }); test('aggregates multiple accounts under same bank', function () { - Mail::fake(); + Queue::fake(); $user = User::factory()->onboarded()->create(); $bank = Bank::factory()->create(['name' => 'Shared Bank']); @@ -250,14 +254,11 @@ test('aggregates multiple accounts under same bank', function () { $job = new SyncBankingConnectionJob($connection); $job->handle($transactionSync, $balanceSync); - Mail::assertQueued(BankTransactionsSyncedEmail::class, function ($mail) { - return $mail->totalTransactions === 6 - && $mail->transactionsPerBank === ['Shared Bank' => 6]; - }); + Queue::assertPushed(SendDailyBankTransactionsSyncedEmailJob::class); }); test('lists different banks separately in email', function () { - Mail::fake(); + Queue::fake(); $user = User::factory()->onboarded()->create(); $bankA = Bank::factory()->create(['name' => 'Bank A']); @@ -289,10 +290,119 @@ test('lists different banks separately in email', function () { $job = new SyncBankingConnectionJob($connection); $job->handle($transactionSync, $balanceSync); - Mail::assertQueued(BankTransactionsSyncedEmail::class, function ($mail) { - return $mail->totalTransactions === 8 - && $mail->transactionsPerBank === ['Bank A' => 4, 'Bank B' => 4]; + Queue::assertPushed(SendDailyBankTransactionsSyncedEmailJob::class); +}); + +test('daily bank sync email job sends pending transactions once per day', function () { + Mail::fake(); + + test()->travelTo(Carbon::parse('2026-04-15 09:00:00')); + + $user = User::factory()->onboarded()->create(); + $bankA = Bank::factory()->create(['name' => 'Bank A']); + $bankB = Bank::factory()->create(['name' => 'Bank B']); + $connection = BankingConnection::factory()->create([ + 'user_id' => $user->id, + 'last_synced_at' => now()->subDay(), + ]); + $accountA = Account::factory()->connected()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'bank_id' => $bankA->id, + ]); + $accountB = Account::factory()->connected()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'bank_id' => $bankB->id, + ]); + + Transaction::factory()->count(2)->enableBanking()->create([ + 'user_id' => $user->id, + 'account_id' => $accountA->id, + 'created_at' => now()->subMinutes(10), + 'updated_at' => now()->subMinutes(10), + ]); + Transaction::factory()->create([ + 'user_id' => $user->id, + 'account_id' => $accountB->id, + 'source' => TransactionSource::EnableBanking, + 'description_iv' => null, + 'notes_iv' => null, + 'created_at' => now()->subMinutes(5), + 'updated_at' => now()->subMinutes(5), + ]); + + $job = new SendDailyBankTransactionsSyncedEmailJob($user, now()->toDateString()); + $job->handle(); + + Mail::assertQueued(BankTransactionsSyncedEmail::class, function ($mail) use ($user) { + return $mail->totalTransactions === 3 + && $mail->transactionsPerBank === ['Bank A' => 2, 'Bank B' => 1] + && $mail->hasTo($user->email); }); + + expect(UserMailLog::query() + ->where('user_id', $user->id) + ->where('email_type', DripEmailType::BankTransactionsSynced) + ->where('email_identifier', '2026-04-15') + ->exists())->toBeTrue(); + + $job->handle(); + + Mail::assertQueued(BankTransactionsSyncedEmail::class, 1); +}); + +test('daily bank sync email job sends unreported transactions next day even when current sync imported none', function () { + Mail::fake(); + + $user = User::factory()->onboarded()->create(); + $bank = Bank::factory()->create(['name' => 'Test Bank']); + $connection = BankingConnection::factory()->create([ + 'user_id' => $user->id, + 'last_synced_at' => now()->subDays(2), + ]); + $account = Account::factory()->connected()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'bank_id' => $bank->id, + ]); + + test()->travelTo(Carbon::parse('2026-04-15 18:00:00')); + + Transaction::factory()->count(4)->enableBanking()->create([ + 'user_id' => $user->id, + 'account_id' => $account->id, + 'created_at' => now()->subHour(), + 'updated_at' => now()->subHour(), + ]); + + test()->travelTo(Carbon::parse('2026-04-16 08:00:00')); + + $job = new SendDailyBankTransactionsSyncedEmailJob($user, now()->toDateString()); + $job->handle(); + + Mail::assertQueued(BankTransactionsSyncedEmail::class, function ($mail) { + return $mail->totalTransactions === 4 + && $mail->transactionsPerBank === ['Test Bank' => 4]; + }); +}); + +test('daily bank sync email job skips when no pending transactions', function () { + Mail::fake(); + + $user = User::factory()->onboarded()->create(); + + UserMailLog::create([ + 'user_id' => $user->id, + 'email_type' => DripEmailType::BankTransactionsSynced, + 'email_identifier' => now()->subDay()->toDateString(), + 'sent_at' => now()->subDay(), + ]); + + $job = new SendDailyBankTransactionsSyncedEmailJob($user, now()->toDateString()); + $job->handle(); + + Mail::assertNothingQueued(); }); test('indexa capital sync only syncs balances, not transactions', function () {