Skip to content

Fix order amount display: remove incorrect /100 division in FormatCurrency#12

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

Fix order amount display: remove incorrect /100 division in FormatCurrency#12
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1775541219-fix-notification-amount-display

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

Summary

Order confirmation notification emails displayed incorrect amounts after the microservice decomposition. A $149.99 order was rendered as $1.50 in the email preview.

Root cause: NotificationRenderer.FormatCurrency() divided the amount by 100, assuming OrderPlacedEvent.TotalAmount was transmitted in cents. In reality, TotalAmount is already in dollars (the shared contract OrderPlacedEvent defines it as decimal with no cents convention). So 149.99 / 100 = 1.4999$1.50.

Fix: Remove the erroneous / 100m division and format the amount directly.

Also fixes incorrect relative paths for Shared project references in Notification.API.csproj (../../Shared../../../Shared), which prevented the project from building standalone outside the solution.

Before / After

Before (bug) After (fix)
before after

Review & Testing Checklist for Human

  • Verify no upstream producer sends amounts in cents. The old comment explicitly claimed "TotalAmount is transmitted in cents." Confirm with the Order service team that TotalAmount is indeed in dollars — if any producer sends cents, this fix would be wrong.
  • Verify the csproj path fix doesn't break Docker/CI builds. The old ../../Shared paths were wrong for standalone builds but may have been compensated by the solution-level build or Dockerfile COPY layout. Confirm CI still resolves Shared project references correctly with the new ../../../Shared paths.
  • Test end-to-end: Run the notification service 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.

Notes

  • Demo recording attached showing before/after behavior: recording

View original video (rec-89ec476ec3d343e397607c18529a2fc9-edited.mp4)

  • No unit tests existed for FormatCurrency; consider adding one to prevent regression.

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

…rency

Root cause: NotificationRenderer.FormatCurrency() divided the amount by 100,
assuming TotalAmount was in cents. However, OrderPlacedEvent.TotalAmount is
already in dollars (e.g. 149.99). This caused 149.99/100 = 1.4999, which
displayed as $1.50 instead of $149.99.

Also fixes incorrect relative paths for Shared project references in the
Notification.API.csproj (../../Shared -> ../../../Shared).
@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

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