ESD-1747: Surface a pending-approval cancel from DeleteProduct and DeleteVXC - #179
Open
Phil-Browne wants to merge 3 commits into
Open
ESD-1747: Surface a pending-approval cancel from DeleteProduct and DeleteVXC#179Phil-Browne wants to merge 3 commits into
Phil-Browne wants to merge 3 commits into
Conversation
…proval Megalith answers 202 "Request accepted and pending for approval" when a CANCEL_NOW lands on a live product in a company with cancellation order approval turned on. It files an order approval and cancels nothing. CheckResponse passes any 2xx, and DeleteProduct discarded the response, so the caller saw the same empty struct and nil error as a completed cancel. DeleteProduct now reads the status and returns the new sentinel on 202. DeleteVXC, DeletePort, DeleteMCR, DeleteMVE, DeleteIX, and DeleteNATGateway all forward it, so every product type gets the signal without a signature change.
The doc comments named "cancellation order approval". No such setting exists. Megalith gates the approval on a managed account with PARTNER_ORDERING_APPROVAL_ENABLED, then on LIVE state and a non-M2M term (OrderWorkflowApprovalCreationService.java:174-175). The comments also scoped the error to CANCEL_NOW while the status check is action-agnostic. Drop the trigger conditions from godoc instead of restating them. They belong to the API and they drift. DeleteVXC no longer carries a doc line of its own either: all six delete methods forward the error, and the other five say nothing.
The sentinel was documented only on DeleteProduct, which consumers do not call directly. Name it on the six wrappers instead, in both the interface and the implementation, matching how the cancel-later sentinels are already documented. Use the repo's existing term, order approval request, in place of the invented "files an order approval", and split the CHANGELOG run-on whose relative clause attached to the wrong antecedent.
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.
Megalith answers 202
Request accepted and pending for approvalwhen the API files an order approval instead of canceling.CheckResponsepasses any 2xx andDeleteProductthrew the response away, so that reached the caller as the same empty struct and nil error as a completed cancel.DeleteProductnow returns a new sentinel,ErrCancelPendingApproval, on 202.DeleteProduct's error, so every product type gets the signal for free. Each one names the sentinel in its doc comment, since consumers call the wrappers and notDeleteProduct.DeleteProductResponse: the cancel didn't happen, so it's a failed operation, not data. The field design would have meantDeleteVXCreturning(*DeleteVXCResponse, error)and touching 3 call sites in the provider and 7 in the CLI.CANCEL_NOW: the approval branch also needs LIVE state and a non-M2M term, neither of which the SDK can see.Callers that pin this start getting an error where they used to get a silent success. That's the point, but it is a change. In the provider it's the right direction: the old 202-as-success dropped the resource from state while the VXC stayed live and billable, and on a
RequiresReplacethat left two live VXCs with Terraform tracking one. Wiring the consumers is still ESD-1748 and a matching CLI ticket.Verified against megalith.
LifecycleActionController.java:206-217is the only 202 onPOST /v3/product/{uid}/action/{action}, gated onCANCEL_NOW && LIVE && orderRequiresApproval && verifyServiceTermIsNotM2M. Everything else returns 200, 400, 404, 409, or 500. Deleting the 202 block builds and vets clean, then fails exactly the two new tests; swapping it forStatusOKfails 14 subtests across seven suites.Pre-existing, wants separate tickets.
CANCELwas removed from megalith'sLifecycleActionenum in752145d81c(2026-07-06), so the SDK'sDeleteNow: falsepath sends an action the API rejects with a 400.ModifyProducthas this same bug:PUT /v2/product/{type}/{uid}returns the same 202 on a term increase, and it reportsIsUpdated: truefor any 2xx.Docalls discard the response and never close the body:product.go:281,340,367,385,431,mcr.go:573,589,677,712,service_keys.go:289.OrderApprovalTypeis missingEARLY_TERMINATION(what a cancel approval is typed as) andADD_ON.govulncheckis red on main too, not from this diff. I ran the pinned checker against 846a35c and got the same four advisories, exit 3. All four are standard library, fixed in go1.26.6, and the repo pins 1.26.5.What I could not verify
506f54afda(2026-08-19).-integrationteardown deletes now error, andNotNil(deleteErr, "delete should fail with safe delete")atvxc_integration_test.go:259passes for the wrong reason. CI never runs that suite.