[DT-1361]SnapshotCreateFlight: Create new steps to replace AuthorizeBillingProfileUseStep #1958
Conversation
rjohanek
requested review from
rushtong,
s-rubenstein,
snf2ye and
pshapiro4broad
and removed request for
a team,
rushtong and
s-rubenstein
|
| boolean isTdrBillingProfile; | ||
| try { | ||
| getProfileByIdNoCheck(defaultProfileId); | ||
| isTdrBillingProfile = true; |
There was a problem hiding this comment.
Up to you, but now that this is a function you can use return here and in the catch and remove isTdrBillingProfile from the function.
| // Make sure this user is authorized to use the billing profile in SAM | ||
| addStep( | ||
| new AuthorizeBillingProfileUseStep(profileService, snapshotReq.getProfileId(), userReq)); | ||
| if (isTdrBillingProfile) { |
There was a problem hiding this comment.
This now duplicates the code in DatasetCreateFlight. If we wanted to avoid this duplication, one approach would be to put both cases inside AuthorizeBillingProfileUseStep and have it also accept rawlsService and isTdrBillingProfile.
Another benefit (or shortcoming, depending on how you look at it) of this refactoring is that changing AuthorizeBillingProfileUseStep means that we'd have to look at all cases where this step is used and fix them as part of this work.
| if (tdrBillingProfileFallback) { | ||
| assertThat( | ||
| FlightTestUtils.getStepNames(flight), | ||
| containsInRelativeOrder("AuthorizeBillingProfileUseStep")); |
There was a problem hiding this comment.
To test a single element in a collection, use hasItem()
| containsInRelativeOrder("AuthorizeBillingProfileUseStep")); | |
| hasItem("AuthorizeBillingProfileUseStep")); |
Jira ticket: https://broadworkbench.atlassian.net/browse/DT-1361
Addresses
As a part of our billing migration to rawls, we want to have a period of transition where some billing profiles live in TDR while others have been migrated to rawls. So, we need to handle either case. If it is the rawls case, we need to add new logic to check if the user has access. - Same as Shelby's PR: #1952 which does this work for the DatasetCreateFlight, while this affects the SnapshotCreateFlight
Summary of changes
Update Snapshot Service to check if there is a TDR billing profile and pass it into the flight. In the flight, only run AuthorizeBillingProfileUseStep if there is a TDR billing profile, otherwise only run AuthorizeRawlsBillingProjectsUseStep.
Testing Strategy
Unit tests and manual testing as described in: #1952
To do
Manual Test for Terra billing profile showed there is an issue because the profile model saved in the map during AuthorizeBillingProfileUseStep is used later on in the flight. This is an issue because it is not populated in the Rawls only step, AuthorizeRawlsBillingProjectsUseStep, this causes a null pointer later on.
Error: response status is 500Response bodyDownload{ "message": "Cannot invoke \"bio.terra.model.BillingProfileModel.getBillingAccountId()\" because \"profileModel\" is null", "errorDetail": [ "Cannot invoke \"bio.terra.model.BillingProfileModel.getBillingAccountId()\" because \"profileModel\" is null" ] }