Skip to content

fix: remove incorrect cents-to-dollars conversion in notification currency display#22

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1776892397-fix-notification-currency
Open

fix: remove incorrect cents-to-dollars conversion in notification currency display#22
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1776892397-fix-notification-currency

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot commented Apr 22, 2026

Summary

Order confirmation notification emails were displaying amounts off by a factor of 100 — a $149.99 order showed as $1.50. The FormatCurrency method in NotificationRenderer divided the amount by 100m, assuming the OrderPlacedEvent.TotalAmount was in cents. It is not — the shared contract, HTTP DTO, and domain entity all pass the value as a dollar-denominated decimal. Removed the erroneous division.

Additionally fixes broken project reference paths in Notification.API.csproj (..\..\Shared\..\..\..\Shared\) and updates global.json to match the available .NET 10 RC SDK.

Before → After:

Before: $1.50
After: $149.99

Review & Testing Checklist for Human

  • Verify no other event producer sends TotalAmount in cents. The HTTP endpoint (OrderPlacedEventDto) clearly uses dollars, but if there are RabbitMQ producers elsewhere that actually send cents, this fix would break those. Search for all usages of OrderPlacedEvent and TotalAmount across services.
  • global.json change: SDK version was pinned to 10.0.100-rc.2.25502.107 with rollForward: latestMajor to get the project building. Confirm this is acceptable for your team/CI — you may want to revert to 10.0.100 with latestFeature if your CI has the stable SDK.
  • csproj path fix: The Shared.* project references went from ..\..\Shared\ to ..\..\..\Shared\. Verify this resolves correctly in both local builds and Docker builds (Dockerfiles may use a different build context).
  • Recommended test plan: 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"}, then open the returned preview URL and confirm the total shows $149.99.

Notes

  • No test projects exist in this repo, so the fix has no automated test coverage — verified manually only.
  • ROOT_CAUSE_ANALYSIS.md added at repo root documenting the bug, root cause, and fix.

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


Open in Devin Review

…rency display

The FormatCurrency method in NotificationRenderer divided the amount by 100,
assuming the OrderPlacedEvent.TotalAmount was in cents. However, the shared
contract defines TotalAmount as a decimal in dollars. This caused a 49.99
order to display as .50 in email previews.

Also fixes project reference paths in Notification.API.csproj (Shared.*
projects were unreachable due to incorrect relative paths).
@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 found 1 potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment thread src/global.json
Comment on lines +3 to +4
"version": "10.0.100-rc.2.25502.107",
"rollForward": "latestMajor"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🟡 rollForward: latestMajor allows building with unintended SDK major versions

The rollForward policy was changed from latestFeature to latestMajor, and the version was pinned to a specific pre-release SDK (10.0.100-rc.2.25502.107). The latestMajor policy is the most permissive roll-forward option — it means the .NET SDK resolver will use any installed SDK version at or above the specified one, including future major versions (e.g., .NET 11, 12). Combined with targeting net10.0 in all .csproj files, this could lead to non-reproducible builds across developer machines and CI. The original latestFeature policy was more appropriate as it constrained resolution to the 10.0.1xx feature band. This appears to be a local workaround for having an RC SDK installed that shouldn't have been committed.

Suggested change
"version": "10.0.100-rc.2.25502.107",
"rollForward": "latestMajor"
"version": "10.0.100",
"rollForward": "latestFeature"
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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