diff --git a/app/Jobs/ReassignTransactionsToBudgets.php b/app/Jobs/ReassignTransactionsToBudgets.php new file mode 100644 index 00000000..35ef64ae --- /dev/null +++ b/app/Jobs/ReassignTransactionsToBudgets.php @@ -0,0 +1,46 @@ + $transactionIds + * @param bool $notify set to false for bulk applies, where the budget + * emails would describe limits crossed long ago + */ + public function __construct( + public array $transactionIds, + public bool $notify = true, + ) {} + + public function handle(BudgetTransactionService $service): void + { + Transaction::query() + ->whereIn('id', $this->transactionIds) + ->with('labels') + ->chunkById(200, function (Collection $transactions) use ($service): void { + foreach ($transactions as $transaction) { + $service->assignTransaction($transaction, notify: $this->notify); + } + }); + } +} diff --git a/app/Mcp/Tools/LabelTransaction.php b/app/Mcp/Tools/LabelTransaction.php index 1c087b66..38058b13 100644 --- a/app/Mcp/Tools/LabelTransaction.php +++ b/app/Mcp/Tools/LabelTransaction.php @@ -2,6 +2,7 @@ namespace App\Mcp\Tools; +use App\Jobs\ReassignTransactionsToBudgets; use App\Models\User; use Illuminate\Contracts\JsonSchema\JsonSchema; use Laravel\Mcp\Request; @@ -44,6 +45,10 @@ class LabelTransaction extends WriteTool $transaction->labels()->detach($remove->pluck('id')->all()); } + // Pivot writes fire no model event, so nothing would re-derive which + // budgets now track this transaction by label. + ReassignTransactionsToBudgets::dispatch([$transaction->id]); + return $this->json(['transaction' => $this->presentTransaction($transaction->refresh())]); } } diff --git a/app/Services/AutomationRuleService.php b/app/Services/AutomationRuleService.php index 9a31d76e..11c84ca9 100644 --- a/app/Services/AutomationRuleService.php +++ b/app/Services/AutomationRuleService.php @@ -3,6 +3,7 @@ namespace App\Services; use App\Enums\CategorySource; +use App\Jobs\ReassignTransactionsToBudgets; use App\Models\AutomationRule; use App\Models\LabelTransaction; use App\Models\Transaction; @@ -40,8 +41,8 @@ class AutomationRuleService $transactionData = $this->prepareTransactionData($transaction); $matchedRule = $this->evaluateRules($rules, $transactionData); - if ($matchedRule) { - $this->applyActions($transaction, $matchedRule); + if ($matchedRule && $this->applyActions($transaction, $matchedRule)) { + ReassignTransactionsToBudgets::dispatch([$transaction->id]); } } @@ -98,75 +99,130 @@ class AutomationRuleService } } - $changedTransactionIds = []; + $changedTransactionIds = array_values(array_unique(array_merge( + $this->applyCategoryInBulk($transactions, $rule), + $this->applyNoteInBulk($transactions, $rule), + $this->applyLabelsInBulk($transactions, $rule), + ))); - if ($rule->action_category_id !== null) { - $categoryTransactionIds = $transactions - ->filter(fn (Transaction $transaction): bool => $transaction->category_id !== $rule->action_category_id) - ->pluck('id') - ->all(); - - if ($categoryTransactionIds !== []) { - Transaction::query() - ->whereIn('id', $categoryTransactionIds) - ->update([ - 'category_id' => $rule->action_category_id, - 'category_source' => CategorySource::Rule->value, - 'categorized_by_rule_id' => $rule->id, - 'updated_at' => now(), - ]); - - foreach ($categoryTransactionIds as $transactionId) { - $changedTransactionIds[$transactionId] = true; - } - } - } - - if ($rule->action_note && $rule->action_note_iv === null) { - foreach ($transactions as $transaction) { - $existingNotes = $transaction->notes ?? ''; - - if ($this->noteAlreadyPresent($existingNotes, $rule->action_note)) { - continue; - } - - $transaction->notes = $existingNotes - ? $existingNotes."\n".$rule->action_note - : $rule->action_note; - $transaction->saveQuietly(); - $changedTransactionIds[$transaction->id] = true; - } - } - - $labelIds = $rule->labels->pluck('id')->all(); - if ($labelIds !== []) { - $now = now(); - $labelTransactionRows = []; - - foreach ($transactions as $transaction) { - $transactionLabelIds = $transaction->labels->pluck('id')->all(); - $missingLabelIds = array_diff($labelIds, $transactionLabelIds); - - foreach ($missingLabelIds as $labelId) { - $labelTransactionRows[] = [ - 'id' => (string) Str::uuid(), - 'label_id' => $labelId, - 'transaction_id' => $transaction->id, - 'created_at' => $now, - 'updated_at' => $now, - ]; - $changedTransactionIds[$transaction->id] = true; - } - } - - if ($labelTransactionRows !== []) { - LabelTransaction::query()->insertOrIgnore($labelTransactionRows); - } + if ($changedTransactionIds !== []) { + // One job for the whole batch: the mass update and the pivot insert + // above fire no model event, so nothing else re-derives which budgets + // now track these transactions — and dispatching per row would queue a + // job for every transaction the rule touched. + ReassignTransactionsToBudgets::dispatch($changedTransactionIds, notify: false); } return count($changedTransactionIds); } + /** + * Set the rule's category on every transaction that does not already carry + * it, in a single mass update. + * + * @param EloquentCollection $transactions + * @return array ids of the transactions changed + */ + private function applyCategoryInBulk(EloquentCollection $transactions, AutomationRule $rule): array + { + if ($rule->action_category_id === null) { + return []; + } + + $transactionIds = $transactions + ->filter(fn (Transaction $transaction): bool => $transaction->category_id !== $rule->action_category_id) + ->pluck('id') + ->all(); + + if ($transactionIds === []) { + return []; + } + + Transaction::query() + ->whereIn('id', $transactionIds) + ->update([ + 'category_id' => $rule->action_category_id, + 'category_source' => CategorySource::Rule->value, + 'categorized_by_rule_id' => $rule->id, + 'updated_at' => now(), + ]); + + return $transactionIds; + } + + /** + * Append the rule's note where it is not already present. Encrypted notes are + * skipped because applying them requires the user's key. + * + * @param EloquentCollection $transactions + * @return array ids of the transactions changed + */ + private function applyNoteInBulk(EloquentCollection $transactions, AutomationRule $rule): array + { + if (! $rule->action_note || $rule->action_note_iv !== null) { + return []; + } + + $transactionIds = []; + + foreach ($transactions as $transaction) { + $existingNotes = $transaction->notes ?? ''; + + if ($this->noteAlreadyPresent($existingNotes, $rule->action_note)) { + continue; + } + + $transaction->notes = $existingNotes + ? $existingNotes."\n".$rule->action_note + : $rule->action_note; + $transaction->saveQuietly(); + $transactionIds[] = $transaction->id; + } + + return $transactionIds; + } + + /** + * Attach the rule's labels in a single pivot insert, skipping the pairs that + * already exist. + * + * @param EloquentCollection $transactions + * @return array ids of the transactions changed + */ + private function applyLabelsInBulk(EloquentCollection $transactions, AutomationRule $rule): array + { + $labelIds = $rule->labels->pluck('id')->all(); + + if ($labelIds === []) { + return []; + } + + $now = now(); + $labelTransactionRows = []; + $transactionIds = []; + + foreach ($transactions as $transaction) { + $missingLabelIds = array_diff($labelIds, $transaction->labels->pluck('id')->all()); + + foreach ($missingLabelIds as $labelId) { + $labelTransactionRows[] = [ + 'id' => (string) Str::uuid(), + 'label_id' => $labelId, + 'transaction_id' => $transaction->id, + 'created_at' => $now, + 'updated_at' => $now, + ]; + $transactionIds[] = $transaction->id; + } + } + + if ($labelTransactionRows !== []) { + LabelTransaction::query()->insertOrIgnore($labelTransactionRows); + } + + return $transactionIds; + } + /** * Whether a transaction should be skipped when "only uncategorized" is on. * diff --git a/tests/Feature/AutomationRuleApplicationTest.php b/tests/Feature/AutomationRuleApplicationTest.php index 65fd5f5c..882d3d58 100644 --- a/tests/Feature/AutomationRuleApplicationTest.php +++ b/tests/Feature/AutomationRuleApplicationTest.php @@ -3,6 +3,7 @@ use App\Events\TransactionCreated; use App\Events\TransactionUpdated; use App\Jobs\ApplySingleAutomationRuleJob; +use App\Jobs\ReassignTransactionsToBudgets; use App\Models\Account; use App\Models\AutomationRule; use App\Models\Bank; @@ -212,7 +213,9 @@ test('apply endpoint runs synchronously when matches are below threshold', funct ->assertJsonPath('updated', 3) ->assertJsonPath('total', 3); - Queue::assertNothingPushed(); + Queue::assertNotPushed(ApplySingleAutomationRuleJob::class); + // One reassignment job for the whole batch, not one per transaction. + Queue::assertPushed(ReassignTransactionsToBudgets::class, 1); expect( Transaction::where('user_id', $this->user->id) @@ -263,7 +266,7 @@ test('apply endpoint batches category and label writes', function () { expect($transactionUpdateQueries)->toHaveCount(1) ->and($perTransactionPivotLookupQueries)->toHaveCount(0); - Queue::assertNothingPushed(); + Queue::assertNotPushed(ApplySingleAutomationRuleJob::class); $this->assertDatabaseCount('label_transaction', 5); }); diff --git a/tests/Feature/LabelBudgetReassignmentTest.php b/tests/Feature/LabelBudgetReassignmentTest.php new file mode 100644 index 00000000..ea49848b --- /dev/null +++ b/tests/Feature/LabelBudgetReassignmentTest.php @@ -0,0 +1,126 @@ +create([ + 'budget_id' => $budget->id, + 'start_date' => now()->subDays(30), + 'end_date' => now()->addDays(30), + ]); +} + +function budgetsOf(Transaction $transaction): array +{ + return BudgetTransaction::query() + ->where('transaction_id', $transaction->id) + ->pluck('budget_period_id') + ->all(); +} + +beforeEach(function () { + $this->user = User::factory()->create(); + $this->label = Label::factory()->create(['user_id' => $this->user->id, 'name' => 'Miami 26']); + + $category = Category::factory()->create([ + 'user_id' => $this->user->id, + 'type' => CategoryType::Expense, + ]); + + $this->catchAllPeriod = periodCovering(Budget::factory()->catchAll()->create(['user_id' => $this->user->id])); + $this->labelPeriod = periodCovering(Budget::factory()->forLabels($this->label)->create(['user_id' => $this->user->id])); + + // Created before it carries the label, so it lands in the catch-all — the + // exact state each of the three label paths used to leave behind. + $this->transaction = Transaction::factory()->plaintext()->create([ + 'user_id' => $this->user->id, + 'account_id' => Account::factory()->create(['user_id' => $this->user->id])->id, + 'category_id' => $category->id, + 'description' => 'Hotel Miami', + 'transaction_date' => now(), + 'amount' => -1000, + ]); + + expect(budgetsOf($this->transaction))->toBe([$this->catchAllPeriod->id]); +}); + +function labelRule(User $user, Label $label): AutomationRule +{ + $rule = AutomationRule::factory()->create([ + 'user_id' => $user->id, + 'priority' => 1, + 'rules_json' => ['in' => ['hotel', ['var' => 'description']]], + 'action_category_id' => null, + ]); + + $rule->labels()->attach($label->id); + + return $rule->load('labels'); +} + +test('a rule labelling a single transaction moves it into the label budget', function () { + labelRule($this->user, $this->label); + + app(AutomationRuleService::class)->applyRules($this->transaction); + + expect(budgetsOf($this->transaction))->toBe([$this->labelPeriod->id]); +}); + +test('applying a rule in bulk moves every labelled transaction into the label budget', function () { + $rule = labelRule($this->user, $this->label); + + $transactions = Transaction::query()->whereKey($this->transaction->id)->with('labels')->get(); + + app(AutomationRuleService::class)->applyRuleActionsToTransactions($transactions, $rule); + + expect(budgetsOf($this->transaction))->toBe([$this->labelPeriod->id]); +}); + +test('the bulk apply reassigns transactions whose category it mass-updated', function () { + $tracked = Category::factory()->create(['user_id' => $this->user->id, 'type' => CategoryType::Expense]); + $categoryPeriod = periodCovering(Budget::factory()->forCategories($tracked)->create(['user_id' => $this->user->id])); + + $rule = AutomationRule::factory()->create([ + 'user_id' => $this->user->id, + 'priority' => 1, + 'rules_json' => ['in' => ['hotel', ['var' => 'description']]], + 'action_category_id' => $tracked->id, + ]); + + $transactions = Transaction::query()->whereKey($this->transaction->id)->with('labels')->get(); + + app(AutomationRuleService::class)->applyRuleActionsToTransactions($transactions, $rule); + + expect(budgetsOf($this->transaction))->toBe([$categoryPeriod->id]); +}); + +test('the MCP tool reassigns the transaction when it attaches and when it removes a label', function () { + $this->user->withAccessToken($this->user->createToken('mcp', ['mcp:read', 'mcp:write'])->accessToken); + + WhisperMoneyServer::actingAs($this->user)->tool(LabelTransaction::class, [ + 'transaction_id' => $this->transaction->id, + 'add_label_ids' => [$this->label->id], + ])->assertOk(); + + expect(budgetsOf($this->transaction))->toBe([$this->labelPeriod->id]); + + WhisperMoneyServer::actingAs($this->user)->tool(LabelTransaction::class, [ + 'transaction_id' => $this->transaction->id, + 'remove_label_ids' => [$this->label->id], + ])->assertOk(); + + expect(budgetsOf($this->transaction))->toBe([$this->catchAllPeriod->id]); +});