[PM-23717] premium renewal email

[kdenney]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-23717

📔 Objective

Premium email for milestone 2.

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[github-actions]

Checkmarx One – Scan Summary & Details – 72e2b092-0c11-4b01-b2d9-e539d75ee080

Great job! No new security vulnerabilities introduced in this pull request

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.41%. Comparing base (b3573c1) to head (8efe79e).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6672      +/-   ##
==========================================
- Coverage   57.24%   53.41%   -3.84%     
==========================================
  Files        1910     1911       +1     
  Lines       85156    85129      -27     
  Branches     7651     7642       -9     
==========================================
- Hits        48748    45469    -3279     
- Misses      34576    37909    +3333     
+ Partials     1832     1751      -81     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sbrown-livefront]

This looks good!

My only question for this and the family updates is I think we should add test edge cases for the coupon and percentage off exceptions just to cover the code paths.

[claude]

Claude finished @kdenney's task —— View job

---

Code Review Complete ✅

Review Summary

I've completed a comprehensive review of the premium renewal email implementation for Milestone 2. Overall, the implementation is solid and well-tested. The code follows proper patterns, includes comprehensive test coverage including edge cases, and the error messages are correct.

---

✅ Strengths

1. Excellent Test Coverage - The reviewer (sbrown-livefront) requested test edge cases for coupon and percentage off exceptions, and they have been properly implemented:

   • HandleAsync_WhenMilestone2Enabled_AndCouponNotFound_LogsErrorAndSendsTraditionalEmail (line 2256)
   • HandleAsync_WhenMilestone2Enabled_AndCouponPercentOffIsNull_LogsErrorAndSendsTraditionalEmail (line 2285)

2. Security Compliance - No PII or sensitive data exposed in error messages or logs ✅

3. Proper Error Handling - Exceptions are thrown with clear, descriptive messages that include the coupon ID for debugging

4. Feature Flag Protection - Implementation properly gated behind PM23341_Milestone_2 feature flag

5. Email Templates - Both HTML and text versions provided with proper formatting and internationalization considerations

6. Zero-Knowledge Principles - No vault data is accessed or compromised ✅

---

📝 Review Comments

src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs

Lines 610-618: Error messages are correct ✅
The error messages properly reference Milestone2SubscriptionDiscount (the previous review comments about wrong coupon IDs appear to have been fixed already).

Lines 621-631: Calculation and formatting looks good ✅

• Discount calculation is correct: premiumPlan.Seat.Price * (100 - coupon.PercentOff.Value) / 100
• Currency formatting uses proper culture info: CultureInfo("en-US")
• Both base and discounted prices are formatted consistently

test/Billing.Test/Services/UpcomingInvoiceHandlerTests.cs

Lines 2256-2282: Coupon not found edge case ✅
Properly tests the scenario when the coupon doesn't exist, verifying:

• Error is logged with correct exception type and message
• Premium renewal email is NOT sent
• Traditional invoice email IS sent as fallback

Lines 2285-2366: Coupon PercentOff null edge case ✅
Properly tests the scenario when coupon exists but PercentOff is null, verifying:

• Error is logged with correct exception message
• Premium renewal email is NOT sent
• Traditional invoice email IS sent as fallback

Lines 186-285: Happy path test ✅
Comprehensive test verifying:

• Subscription update with correct price and discount
• Email sent with properly calculated prices
• All dependencies called correctly

Email Templates

premium-renewal.mjml: Clean MJML structure following Bitwarden patterns ✅

PremiumRenewalMailView.html.hbs: Properly compiled HTML email template ✅

PremiumRenewalMailView.text.hbs: Clean text version for email clients ✅

PremiumRenewalMailView.cs:

• Subject line is clear and informative ✅
• Required properties properly defined ✅

---

🎯 Response to Reviewer Feedback

@sbrown-livefront's request: "I think we should add test edge cases for the coupon and percentage off exceptions just to cover the code paths."

✅ ADDRESSED - Both edge case tests have been implemented:

1. Test for when coupon is not found
2. Test for when coupon.PercentOff is null

Both tests verify that:

• Errors are properly logged
• The system gracefully falls back to sending the traditional invoice email
• The premium renewal email is not sent when there's an error

---

✨ Final Assessment

This PR is ready for merge from a code quality perspective:

• ✅ Comprehensive test coverage including requested edge cases
• ✅ Security best practices followed
• ✅ No PII exposure
• ✅ Proper error handling with clear messages
• ✅ Feature flag protection
• ✅ Email templates well-structured
• ✅ Code follows established patterns (matches Families renewal implementation)
• ✅ Proper internationalization considerations

No blocking issues found.

---

[kdenney]

This looks good!

My only question for this and the family updates is I think we should add test edge cases for the coupon and percentage off exceptions just to cover the code paths.

Good call! I've added unit tests for these edge cases.

[sbrown-livefront]

🔥