feat(dashboard): show "Ready for review" CTA on draft PRs#180
Merged
Conversation
… on draft PRs Draft PRs can't be merged on GitHub, so swap the merge footer for a "Ready for review" action that calls the markPullRequestReadyForReview GraphQL mutation. Reviews/checks/conflicts rows still render.
Contributor
📝 WalkthroughWalkthroughAdds a "Ready for review" CTA for draft PRs: UI threads Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as UI Component
participant Server as Server Mutation
participant GitHub as GitHub GraphQL API
participant Cache as React Query Cache
User->>UI: Click "Ready for review"
UI->>UI: Set loading state
UI->>Server: POST markPullReadyForReview(owner,repo,pullNumber,pullId)
Server->>Server: Validate input & check repo-scoped auth
Server->>GitHub: GraphQL markPullRequestReadyForReview(pullId)
GitHub->>Server: Return result
Server->>Cache: Invalidate ["github"] cache key
Server->>UI: Return MutationResult { ok: true } / error
UI->>User: Show success toast or error/permission warning
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested Labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
# Conflicts: # apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx`:
- Around line 1328-1349: The loading flag is never cleared on success in
handleMarkReady, causing the CTA to remain disabled; update handleMarkReady to
reset the loading state (call setIsMarking(false)) after a successful
markPullReadyForReview and queryClient.invalidateQueries (or move the
setIsMarking(false) into a finally block) so the spinner/button is re-enabled
even if the refetch doesn't immediately update the view.
In `@apps/dashboard/src/lib/github.functions.ts`:
- Line 6304: The bustPullDetailCaches call currently misses invalidating the
cached pull page-data resource that contains detail.isDraft; update
bustPullDetailCaches (or add an explicit call right after await
bustPullDetailCaches(context.session.user.id, data)) to also clear the
pulls.pageData.graphql.v2 cache entry for the affected pull (use the same
identifier available in data — e.g., data.id or data.pullNumber and
context.session.user.id). Specifically, ensure the function invokes the cache
invalidation API used elsewhere (e.g.,
invalidateResource/cacheClient.invalidate/removeResource) for the resource key
"pulls.pageData.graphql.v2" scoped to that pull so the draft state is not served
stale.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e92236b5-867c-4fee-85d2-93974a415874
📒 Files selected for processing (2)
apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsxapps/dashboard/src/lib/github.functions.ts
Comment on lines
+1328
to
+1349
| const handleMarkReady = async () => { | ||
| if (!pullId) { | ||
| toast.error("Missing pull request id"); | ||
| return; | ||
| } | ||
| setIsMarking(true); | ||
| try { | ||
| const result = await markPullReadyForReview({ | ||
| data: { owner, repo, pullNumber, pullId }, | ||
| }); | ||
| if (result.ok) { | ||
| await queryClient.invalidateQueries({ queryKey: ["github"] }); | ||
| } else { | ||
| toast.error(result.error); | ||
| checkPermissionWarning(result, `${owner}/${repo}`); | ||
| setIsMarking(false); | ||
| } | ||
| } catch { | ||
| toast.error("Failed to mark as ready for review"); | ||
| setIsMarking(false); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Reset loading state on success to prevent a permanently disabled CTA.
isMarking is never cleared on the success path. If the refetch/invalidation does not transition the view immediately, the button can stay disabled with a stuck spinner.
💡 Suggested fix
const handleMarkReady = async () => {
if (!pullId) {
toast.error("Missing pull request id");
return;
}
setIsMarking(true);
try {
const result = await markPullReadyForReview({
data: { owner, repo, pullNumber, pullId },
});
if (result.ok) {
await queryClient.invalidateQueries({ queryKey: ["github"] });
} else {
toast.error(result.error);
checkPermissionWarning(result, `${owner}/${repo}`);
- setIsMarking(false);
+ return;
}
} catch {
toast.error("Failed to mark as ready for review");
- setIsMarking(false);
+ } finally {
+ setIsMarking(false);
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleMarkReady = async () => { | |
| if (!pullId) { | |
| toast.error("Missing pull request id"); | |
| return; | |
| } | |
| setIsMarking(true); | |
| try { | |
| const result = await markPullReadyForReview({ | |
| data: { owner, repo, pullNumber, pullId }, | |
| }); | |
| if (result.ok) { | |
| await queryClient.invalidateQueries({ queryKey: ["github"] }); | |
| } else { | |
| toast.error(result.error); | |
| checkPermissionWarning(result, `${owner}/${repo}`); | |
| setIsMarking(false); | |
| } | |
| } catch { | |
| toast.error("Failed to mark as ready for review"); | |
| setIsMarking(false); | |
| } | |
| }; | |
| const handleMarkReady = async () => { | |
| if (!pullId) { | |
| toast.error("Missing pull request id"); | |
| return; | |
| } | |
| setIsMarking(true); | |
| try { | |
| const result = await markPullReadyForReview({ | |
| data: { owner, repo, pullNumber, pullId }, | |
| }); | |
| if (result.ok) { | |
| await queryClient.invalidateQueries({ queryKey: ["github"] }); | |
| } else { | |
| toast.error(result.error); | |
| checkPermissionWarning(result, `${owner}/${repo}`); | |
| return; | |
| } | |
| } catch { | |
| toast.error("Failed to mark as ready for review"); | |
| } finally { | |
| setIsMarking(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx` around
lines 1328 - 1349, The loading flag is never cleared on success in
handleMarkReady, causing the CTA to remain disabled; update handleMarkReady to
reset the loading state (call setIsMarking(false)) after a successful
markPullReadyForReview and queryClient.invalidateQueries (or move the
setIsMarking(false) into a finally block) so the spinner/button is re-enabled
even if the refetch doesn't immediately update the view.
| }`, | ||
| { pullRequestId: data.pullId }, | ||
| ); | ||
| await bustPullDetailCaches(context.session.user.id, data); |
Contributor
There was a problem hiding this comment.
Bust pulls.pageData.graphql.v2 cache after marking ready-for-review.
At Line 6304, bustPullDetailCaches does not clear the cached pull page-data resource that contains detail.isDraft. This can leave stale draft state after the mutation and keep showing the draft CTA until cache expiry.
💡 Proposed fix
async function bustPullDetailCaches(userId: string, params: PullCacheParams) {
await Promise.all([
+ bustGitHubCache(userId, "pulls.pageData.graphql.v2", params),
bustGitHubCache(userId, "pulls.detail.raw", params),
bustGitHubCache(userId, "pulls.status.raw", params),
bustGitHubCache(userId, "pulls.status.v1", params),
bustGitHubCache(userId, "pulls.status.v2", params),
bustGitHubCache(userId, "pulls.status.v3", params),
]);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/lib/github.functions.ts` at line 6304, The
bustPullDetailCaches call currently misses invalidating the cached pull
page-data resource that contains detail.isDraft; update bustPullDetailCaches (or
add an explicit call right after await
bustPullDetailCaches(context.session.user.id, data)) to also clear the
pulls.pageData.graphql.v2 cache entry for the affected pull (use the same
identifier available in data — e.g., data.id or data.pullNumber and
context.session.user.id). Specifically, ensure the function invokes the cache
invalidation API used elsewhere (e.g.,
invalidateResource/cacheClient.invalidate/removeResource) for the resource key
"pulls.pageData.graphql.v2" scoped to that pull so the draft state is not served
stale.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx (1)
1371-1392:⚠️ Potential issue | 🟠 MajorReset
isMarkingon success to avoid a stuck disabled CTA.On Line 1381, the success branch never clears
isMarking. If cache invalidation doesn’t immediately transition the view, the button can remain disabled with a spinner. Prefer resetting infinally.💡 Suggested fix
const handleMarkReady = async () => { if (!pullId) { toast.error("Missing pull request id"); return; } setIsMarking(true); try { const result = await markPullReadyForReview({ data: { owner, repo, pullNumber, pullId }, }); if (result.ok) { await queryClient.invalidateQueries({ queryKey: ["github"] }); } else { toast.error(result.error); checkPermissionWarning(result, `${owner}/${repo}`); - setIsMarking(false); } } catch { toast.error("Failed to mark as ready for review"); - setIsMarking(false); + } finally { + setIsMarking(false); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx` around lines 1371 - 1392, The handler handleMarkReady leaves the isMarking flag true on the success path so the CTA can stay disabled; update handleMarkReady to always reset loading by moving setIsMarking(false) into a finally block (or call it after await queryClient.invalidateQueries) so setIsMarking(false) runs on success, error, and exceptions; reference the handleMarkReady function and the setIsMarking variable, and ensure any early returns (e.g., missing pullId) still behave correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx`:
- Around line 1371-1392: The handler handleMarkReady leaves the isMarking flag
true on the success path so the CTA can stay disabled; update handleMarkReady to
always reset loading by moving setIsMarking(false) into a finally block (or call
it after await queryClient.invalidateQueries) so setIsMarking(false) runs on
success, error, and exceptions; reference the handleMarkReady function and the
setIsMarking variable, and ensure any early returns (e.g., missing pullId) still
behave correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 70917a12-8cf7-4eda-b38f-47bd3a9ea067
📒 Files selected for processing (2)
apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsxapps/dashboard/src/lib/github.functions.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/src/lib/github.functions.ts
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
1 participant
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.
Summary
Draft pull requests cannot be merged on GitHub, so the existing merge footer is misleading on a draft PR. This swaps the merge action footer for a "Ready for review" CTA when the PR is a draft, while keeping the reviews / checks / conflicts / branch-status rows in place.
Changes
markPullReadyForReviewserver function that calls GitHub'smarkPullRequestReadyForReviewGraphQL mutation and busts the PR detail caches.isDraftand the PR'sgraphqlIdintoMergeStatusSection/MergeStatusCard, and conditionally render a newReadyForReviewFooterinstead ofMergeFooterwhen the PR is a draft.["github"]queries on success so the merge controls appear automatically once the PR flips to ready.Test Plan
Summary by CodeRabbit