[AC-2576] Replace Billing commands and queries with services #4070
+3,451
−3,425
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rvice, move to commercial
|
New Issues
Fixed Issues
|
r-tome
previously approved these changes
May 13, 2024
|
|
||
| organizationRepository.GetOwnerEmailAddressesById(organization.Id).Returns(organizationOwnerEmails); | ||
| organizationRepository.GetOwnerEmailAddressesById(organization.Id).Returns([ | ||
| "a@b.com", |
There was a problem hiding this comment.
Nit: I prefer to use the domain example.com that is reserved for testing purposes
jlf0dev
previously approved these changes
May 13, 2024
cturnbull-bitwarden
previously approved these changes
May 13, 2024
amorask-bitwarden
dismissed stale reviews from cturnbull-bitwarden and r-tome
via
734e77c
cturnbull-bitwarden
previously approved these changes
May 14, 2024
amorask-bitwarden
dismissed stale reviews from cturnbull-bitwarden, r-tome, and jlf0dev
via
e5c477e
cturnbull-bitwarden
previously approved these changes
May 14, 2024
r-tome
previously approved these changes
May 15, 2024
amorask-bitwarden
dismissed stale reviews from r-tome and cturnbull-bitwarden
via
5d77b73
cturnbull-bitwarden
previously approved these changes
May 20, 2024
r-tome
previously approved these changes
May 20, 2024
…ormation (#4107) * Move the gettax and paymentmethod from stripepayment class Signed-off-by: Cy Okeke <cokeke@bitwarden.com> * Add the method to retrieve the tax and payment details Signed-off-by: Cy Okeke <cokeke@bitwarden.com> * Add unit tests for the paymentInformation method Signed-off-by: Cy Okeke <cokeke@bitwarden.com> * Add the endpoint to retrieve paymentinformation Signed-off-by: Cy Okeke <cokeke@bitwarden.com> * Add unit tests to the SubscriberService Signed-off-by: Cy Okeke <cokeke@bitwarden.com> * Remove the getTaxInfoAsync update reference Signed-off-by: Cy Okeke <cokeke@bitwarden.com> --------- Signed-off-by: Cy Okeke <cokeke@bitwarden.com>
cyprain-okeke
dismissed stale reviews from r-tome and cturnbull-bitwarden
via
bf5246c
cturnbull-bitwarden
previously approved these changes
May 22, 2024
r-tome
previously approved these changes
May 22, 2024
amorask-bitwarden
dismissed stale reviews from r-tome and cturnbull-bitwarden
via
d6dbbdf
cturnbull-bitwarden
approved these changes
May 23, 2024
r-tome
approved these changes
May 23, 2024
jlf0dev
approved these changes
May 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of change
Objective
I wrote this PR due to ongoing conversations in the #code channel regarding how Bitwarden handles "CQRS" and the issue of having commands invoke other commands. Since Billing as a business function is more of an intermediary layer that processes side effects of other user actions, we've decided to refactor the business logic we've been writing to be service-oriented rather than formatted as commands or queries.
(Sorry for the insane commit chain. I've outlined the changes by team below)
This resulted in the movement of the following logic:
AssignSeatsToClientOrganizationCommand->Commercial.ProviderBillingServiceCancelSubscriptionCommand->SubscriberServiceCreateCustomerCommand->Commercial.ProviderBillingServiceRemovePaymentMethodCommand->SubscriberServiceScaleSeatsCommand->Commercial.ProviderBillingServiceStartSubscriptionCommand->Commercial.ProviderBillingServiceProviderBillingQueries->Commercial.ProviderBillingServiceOrganizationBillingQueries->OrganizationBillingServiceSubscriberQueries->SubscriberService@bitwarden/team-admin-console-dev:
As far as changes to your domain go, the big one is in
RemoveOrganizationFromProviderCommand. SinceRemovePaymentMethodis no longer a command, we can now invoke it from its service within this command. That update is in this PR and I've added tests for it.The updates to your API Controllers are just wiring up the new services with the same logic instead of the commands and queries. The only actual change to billing business logic represented in AC code is in the API
ProvidersController. You'll seeStartSubscriptionCommandhas been split into 2 methods:CreateCustomerandStartSubscription. They do the same thing as before, but I wanted method control to be more granular.@bitwarden/team-auth-dev:
I think you only have one file changed in here and it's just replacing the previous command invocation for cancelling a subscription with the new service method; should be pretty straight forward.
@bitwarden/team-billing-dev:
If needed, we can sync as a team on the changes. Jet let me know.