diff --git a/app/models/ledger/item.rb b/app/models/ledger/item.rb index fc0cdc794c..ec2d98f494 100644 --- a/app/models/ledger/item.rb +++ b/app/models/ledger/item.rb @@ -29,6 +29,7 @@ # index_ledger_items_on_author_id (author_id) # index_ledger_items_on_datetime (datetime) # index_ledger_items_on_linked_object (linked_object_type,linked_object_id) +# index_ledger_items_on_memo_tsvector (to_tsvector('simple'::regconfig, COALESCE(memo, ''::text))) USING gin # index_ledger_items_on_receipt_missing (id) WHERE (receipt_required AND (marked_no_or_lost_receipt_at IS NULL) AND (receipt_count = 0)) # index_ledger_items_on_short_code (short_code) UNIQUE # index_ledger_items_on_status (status) diff --git a/app/models/ledger/query.rb b/app/models/ledger/query.rb index 57d408abcd..fb6ce4fa6d 100644 --- a/app/models/ledger/query.rb +++ b/app/models/ledger/query.rb @@ -45,15 +45,21 @@ def initialize(query_hash) # dynamically-empty set (e.g. an event with no card grants) can never leak # another organization's items. def execute(ledgers: [], all_ledgers: false) - results = apply_query(relation: Ledger::Item.all, query: @query_hash) - # Strict boolean: only a literal true opts out of scoping, so a caller that # accidentally passes a truthy value (e.g. the string "false") fails closed. - unless all_ledgers == true + # Stashed on the instance (rather than threaded through every apply_* + # method) so apply_partial_predicate's $search branch can scope its own + # subquery to the same ledgers instead of searching every ledger's items. + @ledger_scope_ids = ledgers + @ledger_scope_all = (all_ledgers == true) + + results = apply_query(relation: Ledger::Item.all, query: @query_hash) + + unless @ledger_scope_all # Scope via a subquery rather than joins(...).distinct: DISTINCT breaks # under Postgres when combined with our ORDER BY and a narrowed select # list (e.g. pluck) — ORDER BY expressions must appear in the select list. - results = results.where(id: Ledger::Mapping.where(ledger_id: ledgers).select(:ledger_item_id)) + results = results.where(id: Ledger::Mapping.where(ledger_id: @ledger_scope_ids).select(:ledger_item_id)) end # Pending items sort first regardless of datetime. A CASE (rather than @@ -256,7 +262,12 @@ def apply_partial_predicate(relation, operator, raw_key, operand) # full-text search, supported only on the memo column. raise Ledger::Query::Error.new("$search is only supported on the memo field") unless key == "memo" - relation.where(id: Ledger::Item.search_memo(operand).select(:id)) + matches = Ledger::Item.search_memo(operand) + # Scope the search itself to the same ledgers as the overall query, + # rather than full-text searching every ledger's items — mirrors the + # scoping applied to `results` in execute(). + matches = matches.where(id: Ledger::Mapping.where(ledger_id: @ledger_scope_ids).select(:ledger_item_id)) unless @ledger_scope_all + relation.where(id: matches.select(:id)) else raise Ledger::Query::Error.new("Unsupported comparison operator: #{operator}") end diff --git a/db/migrate/20260718010000_add_gin_index_for_ledger_item_memo_search.rb b/db/migrate/20260718010000_add_gin_index_for_ledger_item_memo_search.rb new file mode 100644 index 0000000000..5471d34c92 --- /dev/null +++ b/db/migrate/20260718010000_add_gin_index_for_ledger_item_memo_search.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddGinIndexForLedgerItemMemoSearch < ActiveRecord::Migration[8.0] + disable_ddl_transaction! + + def change + # Expression must match exactly what Ledger::Item's pg_search_scope(:search_memo) + # generates (dictionary "simple") for Postgres to use this index for that search. + add_index :ledger_items, "to_tsvector('simple', coalesce(memo, ''))", + using: :gin, + name: "index_ledger_items_on_memo_tsvector", + algorithm: :concurrently + end + +end diff --git a/db/schema.rb b/db/schema.rb index 72d9ac95f5..11581d8ebf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,7 +12,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2026_07_16_120000) do +ActiveRecord::Schema[8.0].define(version: 2026_07_18_010000) do create_schema "google_sheets" # These are extensions that must be enabled in order to support this database @@ -1618,6 +1618,7 @@ t.string "status" t.text "system_memo" t.datetime "updated_at", null: false + t.index "to_tsvector('simple'::regconfig, COALESCE(memo, ''::text))", name: "index_ledger_items_on_memo_tsvector", using: :gin t.index ["amount_cents"], name: "index_ledger_items_on_amount_cents" t.index ["author_id"], name: "index_ledger_items_on_author_id" t.index ["datetime"], name: "index_ledger_items_on_datetime" diff --git a/spec/models/ledger/query_spec.rb b/spec/models/ledger/query_spec.rb index 9b00885518..166213edc6 100644 --- a/spec/models/ledger/query_spec.rb +++ b/spec/models/ledger/query_spec.rb @@ -395,6 +395,19 @@ def ids_of(*items) end end + describe "$search" do + it "matches items by memo full-text search" do + result = execute_query({ memo: { "$search" => "alpha" } }) + + expect(result.pluck(:id)).to match_array(ids_of(item_b, item_c)) + end + + it "raises when $search is used on a field other than memo" do + query = { amount_cents: { "$search" => "100" } } + expect { described_class.new(query).execute }.to raise_error(Ledger::Query::Error, /\$search is only supported on the memo field/) + end + end + describe "error handling" do it "raises on unsupported logical operator" do query = { "$xor" => [{ amount_cents: 100 }] } @@ -509,6 +522,34 @@ def create_other_ledger_item(**attrs) expect(result.to_sql).to match(/ledger_mappings/) expect(result.pluck(:id)).to be_empty end + + it "only returns matches from the requested ledger when combined with $search" do + other_payment_item = create_other_ledger_item(amount_cents: 400, memo: "other payment") + + result = execute_query({ memo: { "$search" => "payment" } }) + + expect(result.pluck(:id)).to match_array(ids_of(item_b, item_e, item_f, item_g)) + expect(result.pluck(:id)).not_to include(other_payment_item.id) + end + + it "includes $search matches from every requested ledger when multiple are given" do + other_payment_item = create_other_ledger_item(amount_cents: 400, memo: "other payment") + + result = described_class.new({ memo: { "$search" => "payment" } }).execute(ledgers: [test_ledger.id, other_ledger.id]) + + expect(result.pluck(:id)).to match_array(ids_of(item_b, item_e, item_f, item_g, other_payment_item)) + end + + it "scopes the $search subquery itself to the requested ledgers, not just the final result" do + # The $search subquery is a separate query.rb code path from every other + # predicate (it wraps pg_search's own scope rather than a plain `where`), + # so it needs its own assertion that the ledger scope reaches it: a + # `FROM "ledger_mappings"` subquery for the search itself, plus one for + # the outer result set — not just the outer one alone. + result = execute_query({ memo: { "$search" => "payment" } }) + + expect(result.to_sql.scan(/FROM "ledger_mappings"/).count).to eq(2) + end end describe "complex queries" do