Skip to content

fix: correct currency formatting in notification email previews#31

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1777935407-fix-notification-amount-display
Open

fix: correct currency formatting in notification email previews#31
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1777935407-fix-notification-amount-display

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot commented May 4, 2026

Summary

Fixes a bug where order confirmation notification emails displayed incorrect amounts after the microservice decomposition. A $149.99 order was showing as $1.50 in the email preview.

Root Cause

The NotificationRenderer.FormatCurrency method in the notification-service incorrectly divided the order total by 100, based on an erroneous comment claiming that OrderPlacedEvent.TotalAmount is "transmitted in cents." In reality, the shared OrderPlacedEvent contract (Shared.Contracts/Events/OrderPlacedEvent.cs) defines TotalAmount as a decimal representing dollars, not cents.

// BEFORE (buggy) — divides dollars by 100, yielding $1.50 for a $149.99 order
var dollars = amount / 100m;
return dollars.ToString(\"C2\");

// AFTER (fixed) — formats the amount directly
return amount.ToString(\"C2\");

This also fixes broken project references to the Shared.Contracts and Shared.Infrastructure libraries (incorrect relative path depth prevented the notification-service from building independently).

Before / After

Before (Bug) After (Fix)
Before fix After fix

Review & Testing Checklist for Human

  • Run the notification-service locally and POST to http://localhost:5005/api/notification/events/order-placed with {\"totalAmount\": 149.99, ...} — verify the preview shows $149.99
  • Confirm that the OrderPlacedEvent.TotalAmount contract is consistently used as dollars (not cents) across all producing services
  • Verify the corrected project references allow dotnet build to succeed from the Notification.API directory

Notes

  • The generic currency symbol (¤) appears in the preview because the server runs with invariant culture; in production with en-US locale, it will render as $149.99.
  • The broken project references (..\\..\\Shared\\..\\..\\..\\Shared\\) were a pre-existing issue from the decomposition scaffolding that prevented local builds.

Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/7fd8885c2b024624835c9f4d1ec825d5
Requested by: @DhrovS


Open in Devin Review

The NotificationRenderer.FormatCurrency method incorrectly divided the
order total by 100, assuming TotalAmount was in cents. However, the
OrderPlacedEvent contract transmits TotalAmount in dollars (decimal).
This caused a $149.99 order to display as $1.50 in email previews.

Also fixes broken project references to Shared libraries (wrong relative
path depth prevented the service from building).
@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 2 additional findings.

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.

1 participant