OU-1465: feat: add deny button on run detail view#117
Conversation
|
@jgbernalp: This pull request references OU-1465 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
|
Warning Review limit reached
Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAgentic run approval and denial now use localized messages, shared permission-gated buttons, and confirmation modals. Timeline labels and mutation errors are translated, while denial moves from remediation cards to the run detail page. ChangesAgentic run approval and denial
Sequence Diagram(s)sequenceDiagram
participant Operator
participant RunDetailPage
participant ConfirmationModal
participant useAgenticRun
participant KubernetesAPI
Operator->>RunDetailPage: Select Deny
RunDetailPage->>ConfirmationModal: Open confirmation
Operator->>ConfirmationModal: Confirm denial
ConfirmationModal->>RunDetailPage: Invoke denial action
RunDetailPage->>useAgenticRun: denyExecution()
useAgenticRun->>KubernetesAPI: Patch execution state
KubernetesAPI-->>useAgenticRun: Return mutation result
useAgenticRun-->>RunDetailPage: Update mutation state or error
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
jgbernalp
force-pushed
the
add-deny-button
branch
2 times, most recently
from
18fb24e to
b2d66a7
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/hooks/useAgenticRun.ts (1)
430-465: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated precondition/mutation-lifecycle logic between
approveExecutionanddenyExecution.Both callbacks repeat the same
!canApprove→ error,!run || !approval→ error,setMutationInProgress/setMutationErrorbracketing, and catch/fallback pattern. Extracting a shared helper (e.g.,runApprovalMutation(patchData, fallbackErrorKey)) would remove ~25 lines of duplication and prevent the two paths from drifting apart on future changes.♻️ Sketch of a shared helper
+ const runApprovalMutation = useCallback( + async (patchData: unknown, fallbackErrorKey: string): Promise<boolean> => { + if (!canApprove) { + setMutationError(t('You do not have permission to approve or deny runs.')); + return false; + } + if (!run || !approval) { + setMutationError(t('Approval resource is not available yet.')); + return false; + } + setMutationInProgress(true); + setMutationError(undefined); + try { + await k8sPatch({ + model: LightspeedAgenticRunApprovalModel, + resource: { metadata: { name: run.metadata?.name, namespace } }, + data: patchData, + }); + return true; + } catch (err) { + setMutationError((err as Error)?.message || t(fallbackErrorKey)); + return false; + } finally { + setMutationInProgress(false); + } + }, + [run, approval, namespace, canApprove, t], + );Also applies to: 467-496
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useAgenticRun.ts` around lines 430 - 465, Extract the shared precondition checks, mutation state lifecycle, and error handling from approveExecution and denyExecution into a helper such as runApprovalMutation. Have both callbacks build only their operation-specific patch data and delegate to that helper, preserving the existing permission/resource validation and operation-specific fallback error messages.locales/en/plugin__lightspeed-agentic-console-plugin.json (1)
124-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNear-duplicate permission strings.
"You do not have permission to approve or deny runs."(line 124, used inuseAgenticRun.tsmutation errors) and"You don't have permission to approve or deny runs."(line 125, used inApprovalGatedButton.tsxtooltip) differ only by contraction. Consumers see one wording in the tooltip and a different one in the error banner for the same underlying condition. Consolidate to a single string/key reused in both places to avoid translator duplication and inconsistent voice.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@locales/en/plugin__lightspeed-agentic-console-plugin.json` around lines 124 - 125, Consolidate the two approval-permission translation entries into one canonical string/key, then update the references in useAgenticRun and ApprovalGatedButton to reuse it. Preserve the existing permission-denied behavior while ensuring the tooltip and mutation error display identical wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@locales/en/plugin__lightspeed-agentic-console-plugin.json`:
- Around line 124-125: Consolidate the two approval-permission translation
entries into one canonical string/key, then update the references in
useAgenticRun and ApprovalGatedButton to reuse it. Preserve the existing
permission-denied behavior while ensuring the tooltip and mutation error display
identical wording.
In `@src/hooks/useAgenticRun.ts`:
- Around line 430-465: Extract the shared precondition checks, mutation state
lifecycle, and error handling from approveExecution and denyExecution into a
helper such as runApprovalMutation. Have both callbacks build only their
operation-specific patch data and delegate to that helper, preserving the
existing permission/resource validation and operation-specific fallback error
messages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5d40b411-c684-4f0c-a437-09b6cfa918eb
📒 Files selected for processing (5)
locales/en/plugin__lightspeed-agentic-console-plugin.jsonsrc/components/ApprovalGatedButton.tsxsrc/components/runs/RunDetailPage.tsxsrc/components/runs/detail/RemediationOptionCard.tsxsrc/hooks/useAgenticRun.ts
Signed-off-by: Gabriel Bernal <gbernal@redhat.com>
jgbernalp
force-pushed
the
add-deny-button
branch
from
b2d66a7 to
7002ced
Compare
2 participants
This PR is a follow up from refactoring the detail view to match the UX mockups.
It adds the "Deny" button.