clients
clients copied to clipboard
[PM-5971] Fix Payment Method Warning Bugs
Type of change
- [X] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
Objective
This PR refactors the implementation of the payment method warning banners to continuously poll the /organizations/{organizationId}/billing-status endpoint for whether the active user's organizations risk subscription failure. It uses the responses from those endpoints to update an ActiveUserState observable used to determine whether or not to display the payment method warning banners.
Key Code Changes
Rendering
- organization-layout.component.*: Render newly refactored component if FF is on.
- user-layout.component.*: Render newly refactored component if FF is on.
- providers-layout.componen.*: Render newly refactored component if FF is on.
- setup.component.*: Render newly refactored component if FF is on.
- payment-method-warning-banners.component.*: Consume the new
paymentMethodWarningService.paymentMethodWarnings$observable and display the resulting banners accordingly.
Business Logic
- organization-api.service.*: Rename endpoint method to target
/billing-statusper https://github.com/bitwarden/server/pull/3790. - payment-method-warning.service.*: Main bulk of the new logic around fetching each organization's billing status on a timer and updating the
ActiveUserStateobservable. - app.component.ts: Clear session storage from payment-method-warning.service when user logs out.
- adjust-payment.component.ts: Hide the payment method warning banner when a user adds a payment method.
- feature-flag.enum.ts: Add new feature flat to control display.
Unrelated Refactor
- organization-billing.service.*: Move request models to their own file. Refactor existing methods.
- secrets-manager-trial-billing-step.component.ts: Use new
organizationBillingServicemethod name. - secrets-manager-trial-free-stepper.component.ts: Use new
organizationBillingServicemethod name.
Screenshots
https://github.com/bitwarden/clients/assets/144709477/f23e89e4-e1d9-4416-9912-8a9db0b069b6
Before you submit
- Please add unit tests where it makes sense to do so (encouraged but not required)
- If this change requires a documentation update - notify the documentation team
- If this change has particular deployment requirements - notify the DevOps team
- Ensure that all UI additions follow WCAG AA requirements
Codecov Report
Attention: Patch coverage is 56.70103% with 42 lines in your changes are missing coverage. Please review.
Project coverage is 24.96%. Comparing base (
c8e36b6) to head (b52e077).
Additional details and impacted files
@@ Coverage Diff @@
## main #7923 +/- ##
==========================================
+ Coverage 24.92% 24.96% +0.04%
==========================================
Files 2239 2242 +3
Lines 65596 65637 +41
Branches 12376 12379 +3
==========================================
+ Hits 16347 16388 +41
Misses 47916 47916
Partials 1333 1333
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Checkmarx One – Scan Summary & Details – 1f656889-8e4f-48d7-94f5-de49f1870d4f
No New Or Fixed Issues Found
@MGibson1 Really appreciate the thorough review; learning a lot with this one that's very helpful. I think I resolved the remaining feedback. I also added tests for the PaymentMethodWarningsService.
Hi @eliykat, thank you for the review. I've addressed all of your feedback here: https://github.com/bitwarden/clients/pull/7923/commits/c14dfc83918b4c839bad531eed9f5f046973f5f0 except for the part about the standalone component where you asked @MGibson1 for input.
In the interest of not holding this open too long, would it be possible to leave the component as is and open a ticket to refactor it into a standalone component if need be down the line? Thanks in advance!
@MGibson1 @eliykat @cyprain-okeke @r-tome Thank you all very much for your help and reviews here. I think I've resolved the remainder of the feedback and resolved the conflicts related to the wide-scale layout changes.
If I could one more round of re-reviews from each of you to wrap this up when you have the chance, I'd greatly appreciate it.