Skip to content

fix(mobile): allow submitting a PR review with no comments - #4785

Merged
iscekic merged 2 commits into
mainfrom
fix/mobile-review-submit-no-comments
Jul 27, 2026
Merged

fix(mobile): allow submitting a PR review with no comments#4785
iscekic merged 2 commits into
mainfrom
fix/mobile-review-submit-no-comments

Conversation

@iscekic

@iscekic iscekic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Problem

On the mobile PR-review surface a user could not submit a review carrying no comments, so approving a PR outright was impossible. The gate was unreachability, not validation: the review-submit sheet's only entrance was the floating Finish review button over the Files-tab diff, rendered only when the pending-comment queue was non-empty. The sheet, the input mapper, the tRPC schema/procedure, and the GitHub params builder all already permitted an empty body and zero comments.

Fix

  • Overview Review entry point. The Overview tab now has a Review section (a11y Review pull request) above the merge section that opens the review-submit sheet unconditionally, mirroring the merge CTA idiom. The Files-tab Finish review shortcut is unchanged.
  • Client-side mirror of GitHub's body rule. APPROVE submits with an empty summary and zero comments. REQUEST_CHANGES/COMMENT with neither a summary nor a comment now disables the submit CTA and explains the requirement in place — in the existing hint slot, replacing the empty-queue hint so the two never contradict — instead of round-tripping to a GitHub 422. Typing a summary or queuing a comment re-enables it.

No backend change: all three backend layers already permit an empty body and zero comments.

Testing

  • Eight new unit cases for reviewSubmitBlockReason covering both blockable events on both allow paths.
  • pnpm format && pnpm typecheck && pnpm lint && pnpm check:unused and full pnpm test pass in apps/mobile.
  • E2E on iOS against a hermetic GitHub stub: approve with empty summary and zero comments succeeds end to end (wire-level 200 APPROVED); blocked state shows the hint and disabled CTA; the pre-existing queued-comment Finish review path is unchanged; retryable (stub down) and non-retryable (403) failure states behave per the existing classification.

@iscekic iscekic self-assigned this Jul 26, 2026
@kilo-code-bot

kilo-code-bot Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Reviewed the mobile PR-review-submit changes (block-reason gating, Overview entry point) and found no security, correctness, or breaking-change issues in the diff.

Files Reviewed (4 files)
  • apps/mobile/src/components/pr-review/pr-review-overview.tsx
  • apps/mobile/src/components/pr-review/pr-review-submit.tsx
  • apps/mobile/src/lib/pr-review/build-submit-review-input.ts
  • apps/mobile/src/lib/pr-review/build-submit-review-input.test.ts

Reviewed by claude-sonnet-5 · Input: 20 · Output: 4.2K · Cached: 420.3K

Review guidance: REVIEW.md from base branch main

@iscekic

iscekic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

(bot) Standin review (Kilobot is out of service — this is the review of record for head 3f0d2d4c1).

A fresh mobile-reviewer agent reviewed the full diff origin/main...3f0d2d4c1 (2 commits, 4 files, all under apps/mobile/): the new reviewSubmitBlockReason pure function and its eight unit cases, the submit-sheet wiring (hasSummary derived state, blockReason in submitDisabled, hint-slot mutual exclusion), and the new Overview Review entry point.

Verdict: No findings.

Verified against the review contract, among others: GitHub's rule mirrored exactly (APPROVE always allowed; REQUEST_CHANGES/COMMENT require a summary or ≥1 comment); block hint replaces PendingQueueHint rather than co-rendering; block hint visible regardless of keyboard visibility; submission still reads bodyRef.current and the summary TextInput stays uncontrolled; error-disable lifetimes unchanged; Overview CTA is visible Review with a11y Review pull request, Href-typed, no usePendingReview, loaded-data branch only.

Residual risks noted by the reviewer that are not defects: on-device E2E (AC1–AC4) is running separately and covers the sheet wiring; the keyboard-visible hint-slot layout at half-detent is validated by that E2E.

@iscekic
iscekic merged commit 4ea6abd into main Jul 27, 2026
19 checks passed
@iscekic
iscekic deleted the fix/mobile-review-submit-no-comments branch July 27, 2026 09:03
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.

2 participants