Skip to content

Fix notification email showing wrong order total ($149.99 displayed as $1.50)#10

Open
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1775541027-fix-notification-amount-formatting
Open

Fix notification email showing wrong order total ($149.99 displayed as $1.50)#10
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1775541027-fix-notification-amount-formatting

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot commented Apr 7, 2026

Summary

Order confirmation notification emails were displaying incorrect totals — a $149.99 order appeared as $1.50 in the email preview.

Root cause: NotificationRenderer.FormatCurrency() divided the amount by 100, assuming TotalAmount was transmitted in cents. However, OrderPlacedEvent.TotalAmount (defined in Shared.Contracts) is a decimal already representing dollars. The division produced 149.99 / 100 = 1.4999, which formatted as $1.50.

Fix: Remove the erroneous /100m division and the misleading comment. The amount is now formatted directly via ToString("C2").

Before / After

Before (bug) After (fix)
before after

Review & Testing Checklist for Human

  • Verify no other event producers send TotalAmount in cents. The removed comment claimed cents were intentional. Confirm that all producers of OrderPlacedEvent (including any RabbitMQ publishers not in this repo) send dollar amounts, not cent amounts. If any producer does send cents, this fix is incorrect and the producer needs to change instead.
  • Reproduce locally: POST to /api/notification/events/order-placed with {"orderId":"11111111-1111-1111-1111-111111111111","customerId":"22222222-2222-2222-2222-222222222222","totalAmount":149.99,"placedAt":"2026-03-17T12:00:00Z"} and verify the preview shows $149.99.
  • Test edge cases: Try amounts like 0.01, 0.99, 1000.00, and 0 to ensure formatting is correct across the board.

Notes

  • No unit tests existed for FormatCurrency; consider adding tests to prevent regressions.
  • ToString("C2") is locale-dependent — it uses the server's current culture for the currency symbol. This is pre-existing behavior, not introduced by this change.

Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/ed836c8f8ffb4732b029cbd48d7cad1e


Open with Devin

Root cause: NotificationRenderer.FormatCurrency() incorrectly divided the
amount by 100, assuming TotalAmount was transmitted in cents. However, the
OrderPlacedEvent.TotalAmount is already in dollars (decimal), so the division
produced wrong values (e.g. $149.99 / 100 = $1.4999, displayed as $1.50).

Fix: Remove the erroneous division by 100. The amount is formatted directly
using ToString("C2") since it is already in the correct dollar unit.
@devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

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.

0 participants