fix: keep the deployment delete request on the workflow mapping DTO by arpitjain099 · Pull Request #7063 · devtron-labs/devtron · GitHub
Skip to content

fix: keep the deployment delete request on the workflow mapping DTO - #7063

Open
arpitjain099 wants to merge 1 commit into
devtron-labs:mainfrom
arpitjain099:fix/workflow-mapping-delete-flag
Open

arpitjain099 wants to merge 1 commit into
devtron-labs:mainfrom
arpitjain099:fix/workflow-mapping-delete-flag

Conversation

@arpitjain099

Copy link
Copy Markdown

Description

FindAllAppWorkflowMapping looks up which CD pipelines have a pending deployment delete request, then tries to set the flag on the DTOs it is about to return:

for _, workflowMapping := range workflowMappingDtos {
    if workflowMapping.Type == "CD_PIPELINE" && cdPipelineIdMap[workflowMapping.ComponentId] {
        workflowMapping.DeploymentAppDeleteRequest = true
    }
}

workflowMappingDtos is a []bean4.AppWorkflowMappingDto, a value slice, so workflowMapping is a copy and the write is discarded. The FilterDeploymentDeleteRequestedPipelineIds query above it runs and its result can never reach the response, so DeploymentAppDeleteRequest is always false on every mapping this function returns.

The two sibling functions building the same DTO, FindAppWorkflowMapping and FindAppWorkflowMappingForEnv, both set the field on a local before appending and are correct. IsLast a few lines earlier in this same loop region is also set before the append, which is why that one arrives and this one does not.

Two live GET routes serve the result, /app/app-wf/{app-id} and /app/app-wf/view/{app-id}, and the field is serialized as deploymentAppDeleteRequest. No Go code reads it back off this DTO, so the consumer is the dashboard, which branches on it in CDNode.tsx to open the delete popup rather than the editor, refuse a trigger, and render the DELETE badge. I have not traced which dashboard fetch maps to which route, so I would put it as: both routes return false unconditionally while the database says otherwise, and the badge is the probable visible effect rather than a proven one.

The fix is the indexed loop, and it uses bean4.CD_PIPELINE_TYPE instead of the "CD_PIPELINE" literal since the same function already uses the constant ten lines above.

Fixes #

pkg/appWorkflow had no test file. The new one stubs only the two repository methods this function calls, embedding the interfaces so anything else would fail loudly. Three cases: no delete request, one of two CD pipelines, then both. The first passes either way; the other two fail on main with DeploymentAppDeleteRequest = false, want true. Each case also asserts IsLast, which is set on the local before the append, so a passing IsLast shows the DTOs really were populated and the failure is the dropped write rather than an empty fixture.

Plain correctness, no authz or tenancy surface involved; the flag is only ever written server-side.

AI assistance was used on this change and is disclosed in the commit trailer.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@bito-code-review

Copy link
Copy Markdown

@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant