Skip to content

Conversation

polypixeldev
Copy link
Member

@polypixeldev polypixeldev commented Aug 19, 2025

Summary of the problem

It'd be useful to be able to filter by merchant for card transactions on the ledger!

Note: this PR originally included a category filter as well, but with #11298 soon merged, it will be reimplemented in a later PR using TransactionCategory

Closes #11330

Describe your changes

Modifies both TransactionGroupingEngine::Transaction::All and PendingTransactionEngine::PendingTransaction::All to accept the merchants filter, which is applied using SQL.

Merchants for the current event are loaded lazily with a Turbo frame since some events have very large lists of merchants that they've used.

Screenshot From 2025-08-18 22-10-58 image

@polypixeldev polypixeldev requested review from a team as code owners August 19, 2025 02:12
@polypixeldev polypixeldev changed the title [Ledger] Add filters for categories and merchants [Ledger] Add filters for merchants Aug 19, 2025
Comment on lines 115 to 119
<div data-tabs-target="tab" id="merchant">
<p class="italic text-center">Merchants are the company that charges a credit card. Non-card transactions will not be included when this filter is applied.</p>
<%= turbo_frame_tag :merchants_filter, src: event_merchants_filter_path(@event, merchant: @merchant), loading: :lazy, target: "event_#{@event.id}_ledger" %>
</div>
<div data-tabs-target="tab" id="receipts">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for using a Turbo frame!

Comment on lines 1221 to 1246
def merchants_list
settled_merchants = @event.canonical_transactions.map do |ct|
rst = ct.raw_stripe_transaction

if rst.present?
merchant_data = rst.stripe_transaction["merchant_data"]
yp_merchant = YellowPages::Merchant.lookup(network_id: merchant_data["network_id"])
{ id: merchant_data["network_id"], name: yp_merchant.name || merchant_data["name"].titleize }
else
nil
end
end.select(&:present?)

pending_merchants = @event.canonical_pending_transactions.map do |cpt|
rpst = cpt.raw_pending_stripe_transaction

if rpst.present?
merchant_data = rpst.stripe_transaction["merchant_data"]
yp_merchant = YellowPages::Merchant.lookup(network_id: merchant_data["network_id"])
{ id: merchant_data["network_id"], name: yp_merchant.name || merchant_data["name"].titleize }
else
nil
end
end.select(&:present?)

settled_merchants.concat(pending_merchants)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this should belong in the model

ActiveRecord::Base.sanitize_sql_array(["and raw_stripe_transactions.stripe_transaction->'merchant_data'->>'network_id' = ?", @merchant])
end

def stripe_joins_for(type)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you! I like this naming scheme

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

Copy link
Member

@sampoder sampoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome work here! only real ask is moving as much of the merchant calculation as possible to the model

@polypixeldev polypixeldev requested a review from sampoder August 21, 2025 02:34
@sampoder
Copy link
Member

Screenshot 2025-08-30 at 11 45 17 PM

I think we should cut this copy - it feels very long and clogs up the UI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Ledger] Allow filtering card transactions by merchant and category
2 participants