Show negative enterprise fees in shopfront price breakdown

[David-OFN-CA]

What? Why?

• Closes Negative enterprise fees are not listed in the shop pie chart #14017

Negative enterprise fees (used as discounts by some enterprises) calculated correctly on orders and appeared correctly in reports, but were silently excluded from the shopfront price-breakdown pie chart.

Root cause: EnterpriseFeeCalculator#indexed_fees_by_type_for and #fees_by_type_for both filtered results with .select { |_fee_type, amount| amount > 0 }, dropping any fee type whose net amount was negative before the data reached the variant serializer and the Angular pie-chart template.

Fix: change both guards to .reject { |_fee_type, amount| amount.zero? } — this preserves negative values while still excluding true-zero entries.

(Note: PR opened by an AI agent on behalf of @David-OFN-CA)

What should we test?

1. Create an enterprise fee with a negative amount (e.g. -20% flat rate) and add it to an order cycle.
2. Visit the shop, click the pie chart icon on a variant that has the fee applied.
3. Before this fix: the negative fee is absent from the breakdown.
4. After this fix: the negative fee appears in the breakdown (e.g. as "Admin: -$2.00").
5. Confirm that fees with a 0 value are still excluded from the pie chart.
6. Confirm orders with only positive fees are unaffected.

Release notes

Changelog Category (reviewers may add a label for the release notes):

• User facing changes
• API changes (V0, V1, DFC or Webhook)
• Technical changes only
• Feature toggled

Screenshot

[rioug]

I think we can improve the test, currently we are not testing the whole logic, see comments. But otherwise it looks good 👍

[David-OFN-CA]

Thanks @rioug — added in 2df620e. Since zero-exclusion and negative-inclusion were already covered in separate examples, I added a combined test to each computation block (regular + indexed) that sets one fee to 0 and another to -1.23 in the same breakdown, then asserts the zero is dropped while the negative is kept. That exercises the full reject { amount.zero? } discrimination in a single assertion, so the whole logic is covered explicitly. All 30 examples pass locally, rubocop clean.

[David-OFN-CA]

Hi @rioug — this is ready for another look when you have a moment. 🙏

I've addressed your request to cover the zero-fee logic in both computation blocks. Each block (regular and indexed) now has:

• "filters out zero fees" — sets a fee to 0 and asserts it's excluded (the direct check you asked for).
• "drops zero fees while keeping negative ones in the same breakdown" (2df620e535) — sets one fee to -1.23 and another to 0 in the same breakdown, then asserts the negative is kept while the zero is dropped.

That second test is the meaningful regression guard here: it's the one case that would still fail under the old select { amount > 0 } (which dropped negatives too) and only passes under reject(&:zero?). So it pins exactly the behaviour change this PR introduces.

CI is green. Could you re-review when you get a chance? Thanks!

[rioug]

Thanks @David-OFN-CA ! It looks good.

[dacook]

Great, thanks for fixing.

[dacook]

I would suggest for next time, to set these fee amounts using let(), rather than updating here. Generally we try to set up all conditions for a test outside the it block. This helps clarify what the context is, and is also more efficient (this test will create an admin fee of 1.23, then update it to -1.23, which results in an unnecessary extra database interaction).
I'm not sure if I explained that well, so this is what I mean:

  context "with both zero and negative fees" do
    let!(:ef_admin) { create(:enterprise_fee, fee_type: 'admin', amount: -1.23, name: "Admin") }
    let!(:ef_sales) { create(:enterprise_fee, fee_type: 'sales', amount: 0, name: "Sales") }

    it "drops zero fees while keeping negative ones in the same breakdown"

(this replaces the ef_admin and ef_sales that were defined above)

@David-OFN-CA I'm wondering are you able to feed this feedback back to your AI agent to learn from?

[AMEA-LYON]

All tests successfull !! Congratulations !