fix(transactions): include enable_banking in pending/confirmed status filter

[web-dev0521]

Summary

The transaction status filter (Pending / Confirmed) hardcoded only simplefin, plaid, and lunchflow in its SQL, even though Transaction::PENDING_PROVIDERS also includes enable_banking. As a result,
for Enable Banking users:

• "Pending" filter → 0 results — Enable Banking pending transactions never matched the filter SQL.
• "Confirmed" filter leaked pending txns — the confirmed condition didn't check enable_banking, so those transactions passed through.

This is the same class of bug that was previously fixed for lunchflow in #859.

Fixes #1668.

Changes

• Transaction::Search#apply_status_filter now delegates to the existing pending / excluding_pending model scopes instead of hardcoded SQL.
• EntrySearch#apply_status_filter now interpolates Transaction::PENDING_CHECK_SQL (already aliased to t) into its EXISTS subqueries.
• Both paths are now sourced from Transaction::PENDING_PROVIDERS, so adding a future provider can't silently reintroduce the bug.
• Added a regression test that creates one pending transaction per provider in PENDING_PROVIDERS and asserts each is matched by "Pending" and excluded from "Confirmed".

Test plan

• bin/rails test test/models/transaction/search_test.rb
• Manually: with an Enable Banking account that has pending transactions, filter Transactions by Status: Pending → pending txns appear; filter by Status: Confirmed → pending txns are excluded.

Summary by CodeRabbit

• Bug Fixes

  • Improved transaction/entry status filtering so pending and confirmed states are applied reliably and consistently across supported providers.

• Tests

  • Added regression tests verifying status filters correctly include pending items and exclude them from confirmed results for all supported providers.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5ed966ad-e230-4d48-8672-94a8d7e56bdf

📥 Commits

Reviewing files that changed from the base of the PR and between bd7170d and e8c6472.

📒 Files selected for processing (1)

• test/models/account/entry_test.rb

---

📝 Walkthrough

Walkthrough

Centralizes pending detection into Transaction::PENDING_CHECK_SQL and delegates status filtering to model scopes; updates EntrySearch to use the shared predicate and adds regression tests that verify pending/confirmed filtering across all providers.

Changes

Status Filter Refactoring

Layer / File(s)	Summary
Refactor Transaction::Search to delegate to model scopes app/models/transaction/search.rb	apply_status_filter replaces inline provider-specific SQL with a case that applies query.pending for ["pending"] and query.excluding_pending for ["confirmed"].
Consolidate pending check logic in EntrySearch app/models/entry_search.rb	pending_condition and the NOT EXISTS confirmed_condition now interpolate the shared Transaction::PENDING_CHECK_SQL (aliasing transactions as t) instead of hardcoding provider JSON path checks.
Add regression tests for status filter across providers test/models/transaction/search_test.rb, test/models/account/entry_test.rb	New tests create one pending transaction/entry per provider in Transaction::PENDING_PROVIDERS and assert that status: ["pending"] includes those and status: ["confirmed"] excludes them (and vice versa for a confirmed record).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• we-promise/sure#859: Both PRs modify SQL predicates in status filtering; this PR consolidates provider checks into shared logic, superseding provider-specific checks introduced earlier.
• we-promise/sure#1783: Related to handling of provider-specific pending flags and pending→posted auto-claim behavior touching the same transaction.extra flags.

Suggested labels

contributor:verified, pr:verified

Suggested reviewers

• jjmata

Poem

🐰 In code where statuses once split apart,
I stitched the checks to share a single heart.
Pending flags now follow one neat line,
Tests hop through providers — each one fine.
With carrots and bytes, our filters shine.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: including enable_banking in the pending/confirmed status filter.
Linked Issues check	✅ Passed	The code changes address all objectives from #1668: the status filter now works correctly for all providers including enable_banking by delegating to model scopes and interpolating shared SQL predicates.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the status filter for pending/confirmed transactions across all providers as specified in #1668.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[jjmata]

Good fix and the right architectural direction — delegating to pending/excluding_pending scopes means any future provider added to PENDING_PROVIDERS is automatically covered in both filter paths without touching search code.

One thing to verify: EntrySearch#apply_status_filter now embeds Transaction::PENDING_CHECK_SQL directly in the EXISTS subquery that already aliases transactions t. Double-check that PENDING_CHECK_SQL is written to reference the table as t (or is alias-agnostic) so the interpolation doesn't produce a broken query. If the constant uses transactions.extra unqualified, it would conflict with the t alias in the subquery context. Worth adding a quick integration test for the EntrySearch path (the existing regression test covers Transaction::Search — are both paths exercised?).

---

Generated by Claude Code

[web-dev0521]

Hi, @jjmata ,
Could you please review this PR?