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
This commit is contained in:
parent
2f583c0113
commit
552aa59aaf
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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';
|
||||
|
|
|
|||
|
|
@ -0,0 +1,84 @@
|
|||
<?php
|
||||
|
||||
namespace App\Jobs;
|
||||
|
||||
use App\Enums\DripEmailType;
|
||||
use App\Enums\TransactionSource;
|
||||
use App\Mail\BankTransactionsSyncedEmail;
|
||||
use App\Models\Transaction;
|
||||
use App\Models\User;
|
||||
use App\Models\UserMailLog;
|
||||
use Illuminate\Bus\Queueable;
|
||||
use Illuminate\Contracts\Queue\ShouldBeUnique;
|
||||
use Illuminate\Contracts\Queue\ShouldQueue;
|
||||
use Illuminate\Foundation\Bus\Dispatchable;
|
||||
use Illuminate\Queue\InteractsWithQueue;
|
||||
use Illuminate\Queue\SerializesModels;
|
||||
use Illuminate\Support\Facades\Mail;
|
||||
|
||||
class SendDailyBankTransactionsSyncedEmailJob implements ShouldBeUnique, ShouldQueue
|
||||
{
|
||||
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
|
||||
|
||||
public int $tries = 5;
|
||||
|
||||
/**
|
||||
* @var array<int, int>
|
||||
*/
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -1,6 +1,9 @@
|
|||
<?php
|
||||
|
||||
use App\Enums\BankingConnectionStatus;
|
||||
use App\Enums\DripEmailType;
|
||||
use App\Enums\TransactionSource;
|
||||
use App\Jobs\SendDailyBankTransactionsSyncedEmailJob;
|
||||
use App\Jobs\SyncBankingConnectionJob;
|
||||
use App\Jobs\SyncBinanceHistoricalBalancesJob;
|
||||
use App\Mail\BankingConnectionAuthFailedEmail;
|
||||
|
|
@ -10,8 +13,10 @@ use App\Models\Bank;
|
|||
use App\Models\BankingConnection;
|
||||
use App\Models\Transaction;
|
||||
use App\Models\User;
|
||||
use App\Models\UserMailLog;
|
||||
use App\Services\Banking\BalanceSyncService;
|
||||
use App\Services\Banking\TransactionSyncService;
|
||||
use Carbon\Carbon;
|
||||
use GuzzleHttp\Psr7\Response;
|
||||
use Illuminate\Contracts\Queue\Job;
|
||||
use Illuminate\Http\Client\RequestException;
|
||||
|
|
@ -132,7 +137,7 @@ test('mixed linked and new accounts in same connection', function () {
|
|||
});
|
||||
|
||||
test('sends email when new transactions are synced on subsequent sync', function () {
|
||||
Mail::fake();
|
||||
Queue::fake();
|
||||
|
||||
$user = User::factory()->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 () {
|
||||
|
|
|
|||
Loading…
Reference in New Issue