Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements preparatory work for supporting pending deletion of subaccounts in the SAP BTP Terraform provider. The changes include adding a new contract_status computed field to track the deletion status of subaccounts and inverting the deletion strategy to attempt normal deletion before falling back to forced deletion.
Changes:
- Added
contract_statusas a new computed field to thebtp_subaccountresource and data source to expose the contract status from the API - Modified the deletion facade logic to first attempt a normal delete (forceDelete=false), then retry with force delete if the error indicates active resources exist
- Added
StateUpdatingto the list of pending states during subaccount deletion to handle the transitional status correctly
Reviewed changes
Copilot reviewed 16 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/btpcli/types/cis/subaccount_response_object.go | Added ContractStatus field to API response type with omitempty JSON tag and manual annotation comment |
| internal/btpcli/facade_accounts_subaccount.go | Inverted deletion logic: now attempts forceDelete=false first, retries with forceDelete=true if specific error detected |
| btp/provider/type_subaccount.go | Added ContractStatus field to all subaccount type models and mapping functions |
| btp/provider/resource_subaccount.go | Added schema definition for contract_status field; added StateUpdating to deletion pending states; includes commented PENDING_FORCED_DELETION handling code |
| btp/provider/datasource_subaccount.go | Added contract_status schema attribute to data source |
| btp/provider/datasource_subaccounts.go | Added contract_status to subaccount object type and data source schema; updated mapping logic |
| btp/provider/resource_subaccount_test.go | Updated all test assertions to verify contract_status = "ACTIVE" |
| btp/provider/datasource_subaccount_test.go | Added contract_status assertions; updated error message expectation |
| btp/provider/datasource_subaccounts_test.go | Updated expected subaccount count from 9 to 10 to match re-recorded fixtures |
| btp/provider/fixtures/*.yaml | Re-recorded test fixtures with new API response fields (contractStatus, lastModifiedBy) |
88e0775 to
ec9f3c4
Compare
This file contains hidden or 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
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.
Purpose
This PR contains the prework for the pending deletion of subaccounts by:
contract_statusas a new computed field in thebtp_subaccountresource. This field will be needed to discover a pending deletion.UPDATINGstatus, which is also fixed in this PR.Test fixtures have been re-recorded, so a drift is expected here.
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?
How to Test
make testWhat to Check
Verify that the following are valid:
Other Information
See #1331
Checklist for reviewer
The following organizational tasks must be completed before merging this PR: