From d41eace70fc9aa4cb7765bec6b965582b8bd499b Mon Sep 17 00:00:00 2001
From: Javi R <4920956+rameerez@users.noreply.github.com>
Date: Tue, 7 Jul 2026 04:43:29 +0100
Subject: [PATCH] Table-qualify amount/expires_at scopes so they compose with
 joins
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`credits`/`debits`/`not_expired`/`expired` used bare `where("amount > 0")`
-style predicates. A bare column name is ambiguous the moment a host joins
another table carrying the same column — and every wallet has an `amount`
on BOTH wallets_transactions and wallets_transfers, so a natural query like

    wallet.transactions.joins(:transfer).credits

(find the transactions belonging to a category of transfer — e.g. reversals)
raised PG::AmbiguousColumn; SQLite and MySQL raise the equivalent.

Route the predicates through `arel_table[:col]` so the generated SQL is
always table-qualified. This also respects the embedding `table_name`
override for free (arel_table derives from the resolved table name), so
embedded/subclassed transaction tables qualify correctly too.

Regression test reproduces the ambiguity across the transfer join and
asserts the generated SQL is qualified (DB-agnostic).
---
 CHANGELOG.md                            |  6 ++++++
 lib/wallets/models/transaction.rb       | 17 ++++++++++++----
 test/models/wallets/transaction_test.rb | 27 +++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3b26999..b6cf068 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,9 @@
+## [Unreleased]
+
+### Fixed
+
+- The `credits`, `debits`, `not_expired`, and `expired` scopes now table-qualify their column predicates (via `arel_table`) instead of a bare `where("amount > 0")`. A bare column name is ambiguous the moment a host joins another table that also has that column — and every wallet has an `amount` on **both** `wallets_transactions` and `wallets_transfers`, so a natural query like `wallet.transactions.joins(:transfer).credits` (e.g. "which of my transactions belong to a reversal transfer?") raised `PG::AmbiguousColumn` (SQLite/MySQL raise the equivalent). `arel_table` also respects the embedding `table_name` override, so embedded/subclassed transaction tables get the correct qualification for free.
+
 ## [0.2.0] - 2026-05-03
 
 ### Fixed
diff --git a/lib/wallets/models/transaction.rb b/lib/wallets/models/transaction.rb
index 4f16dfd..d2f03c0 100644
--- a/lib/wallets/models/transaction.rb
+++ b/lib/wallets/models/transaction.rb
@@ -63,12 +63,21 @@ def self.categories
 
     before_save :sync_metadata_cache
 
-    scope :credits, -> { where("amount > 0") }
-    scope :debits, -> { where("amount < 0") }
+    # Column predicates go through `arel_table` rather than a raw
+    # `where("amount > 0")` string so the generated SQL is always
+    # TABLE-QUALIFIED (`"wallets_transactions"."amount" > 0`). A bare
+    # `amount` breaks the moment a host joins another table that also has an
+    # `amount` column — every wallet has an `amount` on BOTH transactions and
+    # transfers, so `wallet.transactions.joins(:transfer).credits` raised
+    # `PG::AmbiguousColumn` (SQLite/MySQL raise the equivalent). Arel also
+    # respects the embedding `table_name` override for free, since
+    # `arel_table` is derived from the resolved table name.
+    scope :credits, -> { where(arel_table[:amount].gt(0)) }
+    scope :debits, -> { where(arel_table[:amount].lt(0)) }
     scope :recent, -> { order(created_at: :desc) }
     scope :by_category, ->(category) { where(category: category) }
-    scope :not_expired, -> { where("expires_at IS NULL OR expires_at > ?", Time.current) }
-    scope :expired, -> { where("expires_at < ?", Time.current) }
+    scope :not_expired, -> { where(arel_table[:expires_at].eq(nil).or(arel_table[:expires_at].gt(Time.current))) }
+    scope :expired, -> { where(arel_table[:expires_at].lt(Time.current)) }
 
     def metadata
       @indifferent_metadata ||= ActiveSupport::HashWithIndifferentAccess.new(super || {})
diff --git a/test/models/wallets/transaction_test.rb b/test/models/wallets/transaction_test.rb
index 8708cf4..de6ad63 100644
--- a/test/models/wallets/transaction_test.rb
+++ b/test/models/wallets/transaction_test.rb
@@ -73,4 +73,31 @@ class Wallets::TransactionTest < ActiveSupport::TestCase
   ensure
     Wallets.configuration.allow_negative_balance = original_setting
   end
+
+  test "amount scopes qualify their column so they compose with a transfer join" do
+    # Regression: `credits`/`debits` used a bare `where(\"amount > 0\")`.
+    # Both `wallets_transactions` and `wallets_transfers` have an `amount`
+    # column, so the instant a host joined the two — e.g.
+    # `wallet.transactions.joins(:transfer).credits` to find the transactions
+    # belonging to a category of transfer — the database raised an ambiguous
+    # column error (PG::AmbiguousColumn; SQLite "ambiguous column name").
+    sender = User.create!(email: "amount-scope-sender-#{SecureRandom.hex(4)}@example.com", name: "Sender").wallet(:coins)
+    recipient = User.create!(email: "amount-scope-recipient-#{SecureRandom.hex(4)}@example.com", name: "Recipient").wallet(:coins)
+    sender.credit(500, category: :top_up)
+    transfer = sender.transfer_to(recipient, 120, category: :peer_payment)
+
+    # The join is what triggered the ambiguity; both scopes must survive it.
+    sender_debit = sender.transactions.joins(:transfer).debits.sole
+    recipient_credit = recipient.transactions.joins(:transfer).credits.sole
+
+    assert_equal(-120, sender_debit.amount)
+    assert_equal transfer.id, sender_debit.transfer_id
+    assert_equal 120, recipient_credit.amount
+    assert_equal transfer.id, recipient_credit.transfer_id
+
+    # DB-agnostic proof the column is table-qualified in the generated SQL.
+    table = Wallets::Transaction.table_name
+    assert_includes Wallets::Transaction.credits.to_sql, %("#{table}"."amount")
+    assert_includes Wallets::Transaction.debits.to_sql, %("#{table}"."amount")
+  end
 end