[PM 27122] Individual subscription page for self-hosted customers

[cyprain-okeke]

🎟️ Tracking

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

📔 Objective

Improves the self-hosted individual subscription page by:

1. Updating license upload dialog text when opened from the user subscription page
2. Showing the "active" badge only when the subscription is active based on expiration date

Changes
1. License upload dialog text updates

• When opened from the user subscription page:
  • Dialog title: "Upload license file" → "Upload license"
  • Label: "Upload your license file" → "Upload your Premium license file"
• Added UpdateLicenseDialogData interface with fromUserSubscription flag
• Updated UpdateLicenseDialogComponent to accept config and conditionally render text
• Updated user-subscription.component.ts to pass the flag when opening the dialog

2. Active badge conditional display

• Added isSubscriptionActive getter to check if subscription is active
  • Badge shows only when:
  • sub.expiration exists
• Expiration date is in the future
• Badge is hidden if there's no expiration date or if the subscription has expired

📸 Screenshots

![Uploading image.png…]()

⏰ 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

[claude]

Claude finished @cyprain-okeke's task —— View job

---

Code Review Complete

Summary

This PR improves the self-hosted individual subscription page with contextual license upload dialog text and conditional active badge display. The implementation is mostly solid, but has several findings requiring attention.

Findings

Finding 1: Incomplete i18n message causes misleading text in user subscription view

Finding 2: Duplicate i18n keys present in messages.json

Finding 3: Legacy structural directives used instead of modern control flow syntax

Finding 4: Inconsistent i18n pattern between components

Finding 5: Incomplete test coverage (8.33% patch coverage with 55 lines missing)

---

Details

Finding 1: Incomplete i18n message causes misleading text ❌

In user-subscription.component.html:115-119, the text "You'll need to update your license file" is incomplete and grammatically incorrect:

<p bitTypography="body1" class="tw-m-0" *ngIf="sub.expiration">
  {{ "youNeedToUpdateLicenseFile" | i18n }}
  <strong>{{ sub.expiration | date: "MMMM d, y" }}</strong>.
</p>

The i18n key youNeedToUpdateLicenseFile translates to "You'll need to update your license file" but the sentence structure is broken. The strong tag with the date appears after this message with only a period, making it read: "You'll need to update your license file November 27, 2028."

This appears to be a misuse of the i18n keys. Looking at messages.json, there are two related keys:

• youNeedToUpdateLicenseFile: "You'll need to update your license file"
• youNeedToUpdateLicenseFileDate: "$DATE$."

The template should either:

1. Use a single i18n key with a placeholder for the date, OR
2. Properly construct the sentence with "by" or "before": "You'll need to update your license file by November 27, 2028."

Recommendation: Update the template to properly use the i18n pattern with date placeholder, or revise the text to read more naturally (e.g., "You'll need to update your license file by November 27, 2028").

---

Finding 2: Duplicate i18n keys in messages.json ⚠️

The messages.json file contains multiple keys related to license upload with overlapping purposes:

• Line 3397: uploadLicenseFilePremium
• Line 3400: uploadLicenseFileOrg
• Line 8770: uploadLicense (new)
• Line 12203: uploadLicenseFile (new)

Both uploadLicense and uploadLicenseFile appear to serve the same purpose (dialog title), but the conditional logic in update-license-dialog.component.html:4 uses both:

[title]="(fromUserSubscription ? 'uploadLicense' : 'uploadLicenseFile') | i18n"

While this works, it creates maintenance burden. Consider whether both keys are necessary or if one key would suffice with better naming.

---

Finding 3: Legacy structural directives instead of modern control flow 🎨

The codebase uses legacy Angular structural directives (*ngIf, *ngFor) throughout the modified templates:

self-hosted-premium.component.html:

• Line 2: <bit-section *ngIf="shouldShowUpgradeView$ | async">

user-subscription.component.html:

• Lines 1, 9, 13, 20, 35, 48, 57, 58, 94, 110, 115, 146, 166, 178: Multiple *ngIf usages

update-license-dialog.component.html:

• Line 43: <button ... *ngIf="showCancel">

While the CLAUDE.md acknowledges this is part of an active modernization effort and shouldn't be mandated for stable code, this PR introduces a brand new component (self-hosted-premium.component.ts) that uses the legacy syntax. New components should use modern control flow syntax (@if, @for) unless there's a specific reason not to.

Recommendation: Since self-hosted-premium.component.ts is a new standalone component (created in this PR), consider using modern control flow syntax (@if) instead of *ngIf directives. This aligns with Angular modernization best practices without requiring refactoring of existing stable code.

---

Finding 4: Inconsistent i18n pattern between components 💭

The new self-hosted premium component defines feature lists directly in the constructor using i18nService.t():

protected premiumFeatures = [
  this.i18nService.t("builtInAuthenticator"),
  this.i18nService.t("secureFileStorage"),
  // ...
];

This pattern differs from the reactive i18n pipe pattern used extensively in the templates (e.g., {{ "premium" | i18n }}). While both approaches work, mixing them creates inconsistency.

Question: Is there a specific reason for using the imperative i18nService.t() approach for feature lists instead of handling this in the template with the pipe? This might impact i18n reactivity if the language changes at runtime.

---

Finding 5: Incomplete test coverage ⚠️

Codecov reports 8.33% patch coverage with 55 lines missing coverage:

• self-hosted-premium.component.ts: 40 missing lines (0% coverage)
• update-license-dialog.component.ts: 10 missing lines (33.33% coverage)
• user-subscription.component.ts: 4 missing lines (0% coverage)
• individual-billing.module.ts: 1 missing line (0% coverage)

The PR checklist states "Written new unit and / or integration tests where applicable" is checked, but the new self-hosted-premium.component.ts appears to have no test coverage at all.

Recommendation: Add unit tests for:

1. The new SelfHostedPremiumComponent (especially the redirect logic in constructor)
2. The new dialog opening and URL generation methods
3. The isSubscriptionActive getter in UserSubscriptionComponent
4. The conditional dialog text logic in UpdateLicenseDialogComponent

---

Positive Notes

• ✅ Security scan passed (Checkmarx)
• ✅ Proper use of standalone components for new code
• ✅ Correct use of Tailwind CSS with tw- prefix throughout
• ✅ Observable streams follow ADR-0003 pattern with async pipe
• ✅ Good use of takeUntilDestroyed() for subscription cleanup
• ✅ Proper separation of concerns with dialog data interface

---

[github-actions]

Checkmarx One – Scan Summary & Details – 73441519-4e8c-4acb-ac06-a6031ed411e1

New Issues (1)

Checkmarx found the following issues in this Pull Request

Severity	Issue	Source File / Package	Checkmarx Insight
	Missing_HSTS_Header	/apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.ts: 143	details The web-application does not define an HSTS header, leaving it vulnerable to attack. ID: alhjfubOVRYrjEwL%2BQtMJ8mzlvY%3D Attack Vector

[codecov]

Codecov Report

❌ Patch coverage is 8.33333% with 55 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.24%. Comparing base (81453ed) to head (f0c2ef6).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...ndividual/premium/self-hosted-premium.component.ts	0.00%	40 Missing ⚠️
.../billing/shared/update-license-dialog.component.ts	33.33%	10 Missing ⚠️
.../billing/individual/user-subscription.component.ts	0.00%	4 Missing ⚠️
...pp/billing/individual/individual-billing.module.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #17517      +/-   ##
==========================================
- Coverage   41.25%   41.24%   -0.01%     
==========================================
  Files        3546     3546              
  Lines      102040   102079      +39     
  Branches    15308    15315       +7     
==========================================
+ Hits        42099    42106       +7     
- Misses      58177    58209      +32     
  Partials     1764     1764              

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

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.