fix: skip duplicate Analysis stage in approve & upgrade flow

[jrangelramos]

Summary

• When an ApprovalPolicy auto-approves the Analysis stage, clicking "Approve & upgrade" failed with Duplicate value: {"type":"Analysis"} because approveStage('Analysis') was called unconditionally
• Now checks if the Analysis stage already exists in the ProposalApproval before calling approveStage, preventing the duplicate key API error
• Added approval?.spec?.stages to the useCallback dependency array to satisfy the exhaustive-deps lint rule

Test plan

• With an ApprovalPolicy that auto-approves Analysis, click "Approve & upgrade" — should succeed without duplicate error
• Without an ApprovalPolicy (manual approval), click "Approve & upgrade" — should still call approveStage('Analysis') normally
• yarn test passes (56/56)

🤖 Generated with Claude Code

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jrangelramos
Once this PR has been reviewed and has the lgtm label, please assign davidhurta for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[jhadvig]

Thanks for catching this @jrangelramos 🏆 the bug is real and the fix is correct for the DecisionActions path.

However, there's a second unguarded caller of approveStage('Analysis') in UpdatePlanTab.tsx:177 (handleAnalyse) that has the same problem.... if an ApprovalPolicy auto-approves Analysis, the "Analyse" button in the Active Update Plans tab will also hit the duplicate error.

Rather than adding the same guard at every call site, could we move the dedup check into useApprovalActions.approveStage() itself? A one-liner at the top of the function:

const existingStages = currentApproval.spec?.stages ?? [];
if (existingStages.some((s) => s.type === stageType)) return true;  // already approved

This makes approveStage idempotent... calling it when the stage already exists just returns true , which is the right semantic for all callers, current and future.

[jhadvig]

LGTM 👍 the idempotent guard in the hook looks great, thanks for moving it there @jrangelramos

One small nit.... approval?.spec?.stages in the handleApproveClick dependency array (DecisionActions.tsx:114) is no longer needed since the guard moved into the hook. It won't break anything, just unnecessary callback re-creation when stages change. Feel free to drop it if you're doing another push, otherwise no big deal.

[jrangelramos]

just dropped that! thanks

[openshift-ci]

@jrangelramos: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.