Skip to content

Fix incorrect currency formatting in notification emails ($149.99 displayed as $1.50)#17

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

Fix incorrect currency formatting in notification emails ($149.99 displayed as $1.50)#17
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1775543288-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 displayed incorrect amounts (e.g. a $149.99 order showed as $1.50).

Root cause: FormatCurrency() in NotificationRenderer.cs divided the amount by 100, based on a stale comment claiming TotalAmount was transmitted in cents. In reality, the OrderPlacedEvent shared contract defines TotalAmount as a decimal in dollars. The division converted $149.99 → $1.4999 → $1.50.

Fix: Remove the erroneous / 100m division so the dollar amount is formatted directly.

Before / After

Before (bug) After (fix)
before after

Recording: Reproduction and fix verification walkthrough:

recording

View original video (rec-26d7d7d3290a474f816e42e5881d0dd8-edited.mp4)

Review & Testing Checklist for Human

  • Verify no other producer sends TotalAmount in cents. The stale comment ("transmitted in cents") may reflect a previous contract version. Check the Order service and any other event publishers to confirm they populate TotalAmount as dollars, not cents. If any producer does send cents, this fix would break that flow.
  • Reproduce end-to-end: Run the notification service, POST {"orderId": "11111111-1111-1111-1111-111111111111", "customerId": "22222222-2222-2222-2222-222222222222", "totalAmount": 149.99, "placedAt": "2026-03-17T12:00:00Z"} to /api/notification/events/order-placed, and confirm the preview shows $149.99.
  • Consider adding a unit test for FormatCurrency to guard against regression, since the misleading comment made this bug easy to introduce in the first place.

Notes

  • ToString("C2") uses the server's current culture for currency symbol/formatting — this was already the case before the fix and is unchanged.

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


Open with Devin

Root cause: FormatCurrency() was dividing the amount by 100, assuming
TotalAmount was transmitted in cents. However, the OrderPlacedEvent
shared contract defines TotalAmount as a decimal in dollars (e.g. 149.99).
The erroneous division converted $149.99 to $1.50 in email previews.

Fix: Remove the /100 division since the amount is already in dollars.
@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