Skip to content

[PM-29061] Remove Feature Flag 24996#6744

Merged
sbrown-livefront merged 14 commits intomainfrom
billing/pm-29061/remove-ff-24996
Jan 6, 2026
Merged

[PM-29061] Remove Feature Flag 24996#6744
sbrown-livefront merged 14 commits intomainfrom
billing/pm-29061/remove-ff-24996

Conversation

@sbrown-livefront
Copy link
Copy Markdown
Collaborator

@sbrown-livefront sbrown-livefront commented Dec 16, 2025

🎟️ Tracking

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

📔 Objective

This PR removes Feature Flag PM-24996-implement-upgrade-from-free-dialog and associated code.

Note: The old endpoint for upgrading to premium has also been removed. This cannot be merged, until https://bitwarden.atlassian.net/browse/PM-29079 is merged because the current CloudHostedPremiumComponent uses this endpoint.

Update: https://bitwarden.atlassian.net/browse/PM-29079 is merged

📸 Screenshots

⏰ 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
Copy link
Copy Markdown
Contributor

claude Bot commented Dec 16, 2025

Claude finished @sbrown-livefront's task in 2m 59s —— View job


Code Review Complete

Overall Assessment: REQUEST CHANGES

Summary:
The feature flag removal is clean and thorough, but there's one piece of cleanup needed.

Issue Found:

🌱 Dead Code - Orphaned Method

  • Location: src/Api/Billing/Controllers/AccountsController.cs:135-139
  • Issue: The private method GetOrganizationIdsClaimingUserAsync is no longer called after removing PostPremiumAsync
  • Recommendation: Remove this method to avoid accumulating dead code

What's Good:

  • ✓ Feature flag completely removed - no remaining references
  • ✓ Test constructor properly updated to match controller signature
  • ✓ VNext endpoints now correctly unrestricted
  • ✓ No breaking changes to active endpoints
  • ✓ Proper authorization still in place on VNext endpoints
  • ✓ PR correctly marked [DNM] with dependency noted

Verified:

  • No security concerns with the changes
  • Test coverage appropriately maintained
  • Models (PremiumRequestModel, PaymentResponseModel) still have valid usage elsewhere
  • All imports and dependencies cleaned up correctly

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Dec 16, 2025

Overall Assessment: REQUEST CHANGES

Critical Issues:

  • test/Api.Test/Billing/Controllers/AccountsControllerTests.cs:48-54 - Test constructor signature mismatch will cause build failure

See inline comments for details.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 16, 2025

Logo
Checkmarx One – Scan Summary & Details634b0d7a-7755-49ac-87db-d3a467c24c35

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

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.97%. Comparing base (5e735c8) to head (240272f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6744   +/-   ##
=======================================
  Coverage   54.97%   54.97%           
=======================================
  Files        1930     1930           
  Lines       85507    85472   -35     
  Branches     7654     7650    -4     
=======================================
- Hits        47005    46989   -16     
+ Misses      36716    36696   -20     
- Partials     1786     1787    +1     

☔ 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 sbrown-livefront changed the title [DNM] Remove Feature Flag 24996 [PM-29061] Remove Feature Flag 24996 Dec 29, 2025
@sbrown-livefront sbrown-livefront enabled auto-merge (squash) January 6, 2026 20:27
@sbrown-livefront sbrown-livefront merged commit 530d946 into main Jan 6, 2026
40 of 41 checks passed
@sbrown-livefront sbrown-livefront deleted the billing/pm-29061/remove-ff-24996 branch January 6, 2026 20:51
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.

3 participants