Skip to content

[PM-33301] Add Functionality for Upgrading Using PayPal#7183

Open
sbrown-livefront wants to merge 21 commits intomainfrom
billing/pm-33301/upgrade-using-paypal-account
Open

[PM-33301] Add Functionality for Upgrading Using PayPal#7183
sbrown-livefront wants to merge 21 commits intomainfrom
billing/pm-33301/upgrade-using-paypal-account

Conversation

@sbrown-livefront
Copy link
Collaborator

@sbrown-livefront sbrown-livefront commented Mar 9, 2026

🎟️ Tracking

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

📔 Objective

This PR adds required functionality to be able to use Paypal as a payment method for upgrading.

It also pull out some of the upgrade command functionality into helper methods to improve readability of the main function.

📸 Screenshots

Upgrading with Existing PayPal Payment Method

Screen.Recording.2026-03-09.at.12.44.23.PM.mov

Upgrading with New Payment Method Changing from PayPal

Screen.Recording.2026-03-09.at.12.48.45.PM.mov

Upgrading with Bank Account Changing from PayPal

Screen.Recording.2026-03-19.at.4.10.42.PM.mov

@sbrown-livefront sbrown-livefront requested a review from a team as a code owner March 9, 2026 17:50
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2026

Logo
Checkmarx One – Scan Summary & Details1d8b2c4d-fc08-46c7-b049-b0bfd91e8671


New Issues (2) Checkmarx found the following issues in this Pull Request
# Severity Issue Source File / Package Checkmarx Insight
1 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1592
detailsMethod at line 1592 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
Attack Vector
2 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1419
detailsMethod at line 1419 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
Attack Vector

Fixed Issues (1) Great job! The following issues were fixed in this Pull Request
Severity Issue Source File / Package
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 293

@codecov
Copy link

codecov bot commented Mar 9, 2026

Codecov Report

❌ Patch coverage is 97.08029% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.64%. Comparing base (50e1847) to head (933ddd5).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...um/Commands/UpgradePremiumToOrganizationCommand.cs 98.50% 0 Missing and 2 partials ⚠️
.../Core/Billing/Subscriptions/Models/SubscriberId.cs 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7183      +/-   ##
==========================================
- Coverage   57.64%   57.64%   -0.01%     
==========================================
  Files        2033     2033              
  Lines       89597    89602       +5     
  Branches     7974     7982       +8     
==========================================
+ Hits        51650    51651       +1     
- Misses      36092    36094       +2     
- Partials     1855     1857       +2     

☔ 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.

@theMickster theMickster added the ai-review Request a Claude code review label Mar 10, 2026
@claude
Copy link
Contributor

claude bot commented Mar 10, 2026

Claude finished @sbrown-livefront's task in 3m 13s —— View job


Overall Assessment: APPROVE

Reviewed the addition of PayPal/Braintree payment support for premium-to-organization upgrades, a payment method pre-check before subscription modification, and the refactoring of the command into smaller focused methods. The SubscriberId model gains convenience properties (IsUserId, IsOrganizationId, IsProviderId). New tests cover PayPal payment, missing payment method, and unverified bank account scenarios.

Code Review Details

No findings. All previously identified critical issues (wrong subscriber type for Braintree, matching test assertion) have been addressed. The PayPal flow follows established patterns from CreatePremiumCloudHostedSubscriptionCommand, the DefaultIncomplete payment behavior logic correctly handles both PayPal and non-default payment method cases, and the payment method validation is appropriately placed before any subscription modification occurs.

cyprain-okeke
cyprain-okeke previously approved these changes Mar 10, 2026
Copy link
Contributor

@cyprain-okeke cyprain-okeke left a comment

Choose a reason for hiding this comment

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

Nice work!

@sonarqubecloud
Copy link

},
TaxExempt = TaxHelpers.DetermineTaxExemptStatus(billingAddress.Country)
});
PaymentBehavior = requiresIncomplete ? PaymentBehavior.DefaultIncomplete : null
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@amorask-bitwarden I had to change this to accommodate changes from PayPal to bank accounts when updating. Is this the correct payment behavior for bank accounts waiting to be verified? I have a video of this change on the description

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants