-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PM-12601] Add discount to MSP during creation in Admin Portal #5391
base: main
Are you sure you want to change the base?
[PM-12601] Add discount to MSP during creation in Admin Portal #5391
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5391 +/- ##
=======================================
Coverage 44.56% 44.56%
=======================================
Files 1505 1505
Lines 69896 69912 +16
Branches 6285 6287 +2
=======================================
+ Hits 31150 31159 +9
- Misses 37414 37421 +7
Partials 1332 1332 ☔ View full report in Codecov by Sentry. |
New Issues (2)Checkmarx found the following issues in this Pull Request
Fixed Issues (6)Great job! The following issues were fixed in this Pull Request
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there's a failing required test related to missing EF migrations.
@BTreston Completely missed that! Thank you, I've added the migrations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small linter error, otherwise looks good
|
||
#nullable disable | ||
|
||
namespace Bit.MySqlMigrations.Migrations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe a semi-colon at the end here will fix these linter errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gah sorry about that I forgot to run dotnet format
for the migrations. Should be good now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AC code looks good, thank you!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-12601
📔 Objective
This PR adds a selectable discount type to the MSP creation screen in the Bitwarden Admin Portal. The select list options are mapped to Stripe
Coupon
IDs and those IDs are then stored in the database on the newly created, nullableProvider.DiscountId
column.Then, when the Stripe
Customer
for the Provider is created, the service checks for the existence of aDiscountId
and uses that to attach the related StripeCoupon
to the Provider'sCustomer
.After this point, the remainder of the platform's functionality with relation to Provider discounts is driven off the
Coupon
attached to the Provider's customer.I chose to forego a feature flag for this feature because it requires a database migration anyway and the actual code change is all just small additions that doesn't disrupt existing logic.
📸 Screenshots
Screen.Recording.2025-02-11.at.11.33.58.AM.mov
⏰ Reminders before review
🦮 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