Commit Graph

4 Commits

Author SHA1 Message Date
Víctor Falcón d55c3f41df
fix(security): scope job-status endpoints to owner + feature-area fixes (#627)
## Summary

Starts from a security finding on the background-job status endpoints
and folds in the most important issues surfaced by a follow-up review of
the same feature area (categorization backfill, bulk rule re-evaluation,
automation-rule apply).

Each change is its own commit.

## Changes

### 1. `fix(security)` — scope job-status cache keys to the owning user
The categorization, bulk re-evaluation, and apply status endpoints
looked jobs up by a bare job UUID with no user scoping. Any
authenticated user who obtained another user's job id could poll its
progress payload. Cache keys now include the owning user's id, so a
status request keyed by the polling user's id resolves only that user's
own jobs — a mismatched owner falls through to the existing 404. No
ownership store or extra lookup. Cross-user isolation tests added for
all three endpoints.

### 2. `fix(automation-rules)` — re-check `only_uncategorized` at apply
time
The apply flow cached a match snapshot for up to 15 min (keyed only by
`rule.updated_at`) and applied the rule's category to every id in it
without re-checking eligibility. A transaction categorized *after* the
snapshot (by the user, a sibling rule, or a concurrent AI backfill) was
silently re-categorized and stamped `category_source=Rule`. Now
re-filtered through `shouldSkipForOnlyUncategorized()` at apply time;
skipped rows are no longer counted as changed.

### 3. `perf(automation-rules)` — memoize rule set per user in
`applyRules()`
Bulk re-evaluation calls `applyRules()` once per transaction, and each
call re-queried the user's whole rule set + labels — an N+1 scaling with
transaction count (the sibling apply job already loaded rules once).
Memoized per user for the service instance's lifetime (resolved fresh
per job, so rules created mid-run are intentionally not seen).
Query-count test asserts one `automation_rules` query regardless of
transaction count.

### 4. `fix(transactions)` — stop polling when the job never starts
The re-evaluate and apply pollers rescheduled on any non-terminal
status, including `pending`. If the queue worker is down the job never
runs, `failed()` never fires, and the client polled for the full
hour-long TTL with a stuck spinner. Now gives up after 30 consecutive
`pending` ticks (~30s), mirroring the guard the AI-categorization poller
already has. Long `processing` runs are unaffected.

### 5. `test(automation-rules)` — cover apply job execution and failure
branches
`ApplySingleAutomationRuleJob` was only asserted to be *pushed*; its
`handle()` body and `failed()` branch had no coverage, and
`ReEvaluateTransactionRulesJob::failed()` was untested too. Added direct
`handle()`/`failed()` tests pinning the progress-cache payloads.

## Deferred follow-ups (surfaced by review, not in this PR)

- **Consolidate the three frontend pollers onto `usePollJobStatus`** —
they hand-roll `setTimeout` loops and don't tear down on unmount (can
`setState`/`onClose` after the dialog closes). The hook already exists
and is unmount-safe; routing all three through it would also make the
`pending` cap unit-testable. The cap in change #4 currently mirrors the
already-shipped AI poller and is not separately unit-tested.
- **`noteAlreadyPresent()` uses a substring match**
(`AutomationRuleService`) — a rule note that is a substring of an
existing note is silently not appended. Should compare note lines
exactly.
- **No dedup/lock on concurrent apply/re-evaluate jobs** — note appends
are a non-atomic read-then-write, so two concurrent jobs can duplicate a
note. Category/label writes are already idempotent.
- **Transient 404 → false "failed"** — a mid-run cache
eviction/TTL-expiry (or the deploy window of change #1, where an
in-flight job's pre-deploy key is orphaned) makes the poller surface a
false failure. Retry a few times on transient errors before giving up.
- **`matches()` pagination** advances `next_offset` by fetched rows, not
page-window size — deleted ids can stall infinite scroll before reaching
`total`.
- **"Apply to N" count / encrypted-skip counts** can be stale or
under-report vs what's actually processed.
- **Job-trait convention drift** between the three jobs (modern
`Queueable` vs legacy trait stack).

## Testing
- `vendor/bin/pint --test` — pass
- `bun run format` / `bun run lint` — pass (one pre-existing unrelated
warning in `chart.tsx`)
- Affected Pest suites (apply, re-evaluate, evaluation, rule,
categorization, apply-rule-suggestions) — 94 tests pass
2026-07-03 14:49:32 +02:00
Víctor Falcón 1880333b1c
chore: upgrade Laravel 12 to 13 (#242)
## Summary

- Upgrade `laravel/framework` from v12 to **v13.1.1** and update all 52
dependencies to their latest versions
- Bump `laravel/tinker` from v2 to **v3.0** (required for Laravel 13
compatibility)
- Address Laravel 13 breaking change: add `serializable_classes =>
false` to `config/cache.php`
- Fix cached `Collection` in `routes/web.php` — converted to plain array
via `->toArray()` for serialization safety

## Changes

| File | What changed |
|------|-------------|
| `composer.json` | Bumped `php ^8.3`, `laravel/framework ^13.0`,
`laravel/tinker ^3.0` |
| `composer.lock` | 52 packages updated, 1 removed
(`symfony/polyfill-php83`) |
| `config/cache.php` | Added `serializable_classes => false` |
| `routes/web.php` | Cached query result uses `->toArray()`, fallback
changed from `collect()` to `[]` |

## Testing

Full test suite passing: **919 tests, 3792 assertions, 0 failures**
2026-03-25 12:56:33 +00:00
Víctor Falcón 0d9fc5a2b9
feat(transactions): re-add select all matching filters to bulk actions bar (#169)
## 🚪 Why?

### Problem
The bulk actions bar lost the \"Select all matching filters\"
capability, limiting users to acting only on the currently
visible/selected page of transactions. Users had no way to apply bulk
operations (category, labels, re-evaluate rules) to all transactions
matching their active filters.

## 🔑 What?

### Changes
- Re-adds a \"Select all matching filters\" icon button (with tooltip)
to the bulk actions bar — clicking it switches into a \"selecting all\"
mode
- When selecting all: the count label is replaced by a `CheckCheck` icon
with tooltip \"All matching filters\"; the Delete action is hidden
(backend-only safety)
- Bulk category and label updates in \"select all\" mode send `filters`
to `PATCH /transactions/bulk` and show a loading → success toast with
the updated count
- Re-evaluate rules in \"select all\" mode sends `filters` to the
re-evaluate bulk endpoint instead of `transaction_ids`
- Extends `BulkReEvaluateRulesRequest` and
`ReEvaluateTransactionRulesJob` to accept and apply a `filters` param
- Adds Spanish translations for new strings
- Wraps transaction count text in `whitespace-nowrap` to prevent it
splitting across two lines

##  Verification

### Tests
- 2 new tests added in
`tests/Feature/ReEvaluateTransactionRulesTest.php`: verifies controller
passes filters to job, and job correctly scopes transactions by filters
- All 16 tests in `ReEvaluateTransactionRulesTest` pass
2026-03-01 10:27:43 +00:00
Víctor Falcón eda72d4304
feat(rules): move automation rule evaluation to the backend (#168)
## 🚪 Why?

### Problem

Automation rule evaluation was happening entirely on the frontend,
relying on the client to fetch rules, loop over transactions, and apply
category/label/note changes. This meant evaluation was tied to
browser-side decryption logic, couldn't be triggered server-side, and
had no reliable progress reporting for bulk operations.

## 🔑 What?

### Changes

- **Single re-evaluation**: `POST
/transactions/{transaction}/re-evaluate-rules` applies all matching
rules immediately and returns the updated transaction; the frontend
updates local state from the response.
- **Bulk re-evaluation**: `POST /transactions/re-evaluate-rules`
dispatches a queued job and returns a `job_id`; the frontend polls `GET
/transactions/re-evaluate-rules/status/{jobId}` ~every second, updates a
progress toast, and refreshes the list when done.
- **Note support**: `AutomationRuleService` now applies plain
(unencrypted) `action_note` actions; encrypted notes are skipped since
the backend cannot decrypt them.
- **Dirty-only saves**: `AutomationRuleService::applyActions()` now
tracks whether any field changed and only calls `saveQuietly()` when
needed, avoiding unnecessary writes.
- **Encrypted transactions skipped**: Transactions with `description_iv`
are excluded from bulk queries and silently skipped on single
re-evaluation (existing behavior preserved).
- **Frontend cleanup**: Removed `evaluateRules`,
`appendNoteIfNotPresent`, and related crypto imports from
`transaction-list.tsx`, `index.tsx`,
`use-re-evaluate-all-transactions.tsx`, and
`transaction-actions-menu.tsx`.

##  Verification

### Tests

- New: `tests/Feature/ReEvaluateTransactionRulesTest.php` — 14 passing
tests covering:
  - Single re-evaluation applies category, label, and note
  - Single re-evaluation skips encrypted transactions
  - Bulk job dispatched and returns 202 with job_id
  - Bulk status polling returns `processing` and `done`
- Partial bulk (specific transaction_ids) only processes selected
transactions
  - Encrypted transactions excluded from bulk processing
  - Unauthenticated requests are rejected (401)

### Manual Verification

- Row dropdown "Re-evaluate rules" calls backend and updates the row in
place
- Bulk "Re-evaluate All Expenses" dispatches job, shows progress toast,
refreshes list on completion
- Encrypted transactions are silently skipped in both flows
2026-03-01 10:37:12 +01:00