diff --git a/app/Listeners/CategorizeTransactionWithAi.php b/app/Listeners/CategorizeTransactionWithAi.php index 7fa872d8..4cae0489 100644 --- a/app/Listeners/CategorizeTransactionWithAi.php +++ b/app/Listeners/CategorizeTransactionWithAi.php @@ -3,6 +3,7 @@ namespace App\Listeners; use App\Events\TransactionCreated; +use App\Models\Transaction; use App\Services\Ai\AiCategorizationGate; use App\Services\Ai\AiCategorizer; use Illuminate\Contracts\Queue\ShouldQueue; @@ -31,22 +32,44 @@ class CategorizeTransactionWithAi implements ShouldQueue return (string) config('ai_categorization.queue'); } + /** + * Avoid publishing a job that would immediately no-op. In the common case the + * transaction is already categorized by the synchronous automation rules that + * run before this listener, or the user simply isn't AI-eligible, so queuing + * would only cost a wasted jobs-table insert and a wasted worker dequeue. + */ + public function shouldQueue(TransactionCreated $event): bool + { + return $this->shouldCategorize($event->transaction); + } + public function handle(TransactionCreated $event): void { $transaction = $event->transaction; - if ($transaction->category_id !== null) { + // Re-check at run time: eligibility or the category may have changed while + // the job waited in the queue. + if (! $this->shouldCategorize($transaction)) { return; } + $this->categorizer->run($transaction->user, new Collection([$transaction])); + } + + private function shouldCategorize(Transaction $transaction): bool + { + if ($transaction->category_id !== null) { + return false; + } + if ($transaction->description_iv !== null) { - return; + return false; } $user = $transaction->user; if ($user === null) { - return; + return false; } // Transactions imported during onboarding are deliberately skipped here: @@ -54,13 +77,9 @@ class CategorizeTransactionWithAi implements ShouldQueue // end of onboarding, and whatever is left is categorized in a single batch // pass once onboarding completes (CategorizeOnboardingTransactionsJob). if (! $user->isOnboarded()) { - return; + return false; } - if (! $this->gate->allows($user)) { - return; - } - - $this->categorizer->run($user, new Collection([$transaction])); + return $this->gate->allows($user); } } diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 6de7b220..9de64c83 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -3,29 +3,17 @@ namespace App\Providers; use App\Contracts\BankingProviderInterface; -use App\Events\TransactionCreated; -use App\Events\TransactionDeleted; -use App\Events\TransactionUpdated; use App\Http\Responses\RegisterResponse; -use App\Listeners\ApplyAutomationRules; -use App\Listeners\AssignTransactionToBudget; -use App\Listeners\CategorizeTransactionWithAi; -use App\Listeners\PostStripeEventToDiscord; -use App\Listeners\UnassignTransactionFromBudget; -use App\Listeners\UpdateLastLoggedInAt; use App\Services\Ai\Contracts\RuleSuggestionGenerator; use App\Services\Ai\Contracts\TransactionMatcher; use App\Services\Ai\LaravelAiRuleSuggestionGenerator; use App\Services\Ai\UncategorizedTransactionMatcher; use App\Services\Banking\EnableBankingProvider; use App\Services\Discord\DiscordWebhook; -use Illuminate\Auth\Events\Login; use Illuminate\Cache\RateLimiting\Limit; -use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\RateLimiter; use Illuminate\Support\ServiceProvider; use Laravel\Cashier\Cashier; -use Laravel\Cashier\Events\WebhookReceived; use Laravel\Fortify\Contracts\RegisterResponse as RegisterResponseContract; class AppServiceProvider extends ServiceProvider @@ -59,14 +47,11 @@ class AppServiceProvider extends ServiceProvider */ public function boot(): void { - Event::listen(TransactionCreated::class, ApplyAutomationRules::class); - Event::listen(TransactionCreated::class, AssignTransactionToBudget::class); - Event::listen(TransactionCreated::class, CategorizeTransactionWithAi::class); - Event::listen(TransactionUpdated::class, AssignTransactionToBudget::class); - Event::listen(TransactionDeleted::class, UnassignTransactionFromBudget::class); - Event::listen(WebhookReceived::class, PostStripeEventToDiscord::class); - Event::listen(Login::class, UpdateLastLoggedInAt::class); - + // Event listeners are registered automatically via Laravel's event + // discovery of the App\Listeners directory (matched by their handle() + // type-hint). Do not also register them explicitly here — doing both + // registers every listener twice, so each queued listener is dispatched + // twice per event. RateLimiter::for('emails', function (object $job): Limit { return Limit::perSecond(30); }); diff --git a/tests/Feature/Ai/CategorizeTransactionWithAiListenerTest.php b/tests/Feature/Ai/CategorizeTransactionWithAiListenerTest.php new file mode 100644 index 00000000..8a49dc65 --- /dev/null +++ b/tests/Feature/Ai/CategorizeTransactionWithAiListenerTest.php @@ -0,0 +1,63 @@ +onboarded()->create(); + $user->recordAiConsent(); + + return $user; +} + +function aiCategorizationJobsQueued(): int +{ + return Queue::pushed(CallQueuedListener::class) + ->filter(fn (CallQueuedListener $job): bool => $job->class === CategorizeTransactionWithAi::class) + ->count(); +} + +it('queues an AI categorization job for an eligible, uncategorized transaction', function () { + $user = aiEligibleUser(); + + Queue::fake(); + + Transaction::factory()->plaintext()->create([ + 'user_id' => $user->id, + 'category_id' => null, + ]); + + expect(aiCategorizationJobsQueued())->toBe(1); +}); + +it('does not queue an AI categorization job when the user has no AI consent', function () { + $user = User::factory()->onboarded()->create(); + + Queue::fake(); + + Transaction::factory()->plaintext()->create([ + 'user_id' => $user->id, + 'category_id' => null, + ]); + + expect(aiCategorizationJobsQueued())->toBe(0); +}); + +it('does not queue an AI categorization job when the transaction is already categorized', function () { + $user = aiEligibleUser(); + $category = Category::factory()->for($user)->create(); + + Queue::fake(); + + Transaction::factory()->plaintext()->create([ + 'user_id' => $user->id, + 'category_id' => $category->id, + ]); + + expect(aiCategorizationJobsQueued())->toBe(0); +}); diff --git a/tests/Feature/Events/TransactionListenerRegistrationTest.php b/tests/Feature/Events/TransactionListenerRegistrationTest.php new file mode 100644 index 00000000..c5e47067 --- /dev/null +++ b/tests/Feature/Events/TransactionListenerRegistrationTest.php @@ -0,0 +1,33 @@ +filter(fn (CallQueuedListener $job): bool => $job->class === $listener) + ->count(); +} + +it('registers each transaction listener exactly once', function (string $event, string $listener): void { + $user = User::factory()->onboarded()->create(); + $transaction = Transaction::factory()->plaintext()->create(['user_id' => $user->id]); + + Queue::fake(); + + event(new $event($transaction)); + + expect(queuedListenerCount($listener))->toBe(1); +})->with([ + 'created dispatches budget assignment once' => [TransactionCreated::class, AssignTransactionToBudget::class], + 'updated dispatches budget assignment once' => [TransactionUpdated::class, AssignTransactionToBudget::class], + 'deleted dispatches budget unassignment once' => [TransactionDeleted::class, UnassignTransactionFromBudget::class], +]);