Added handling for cases where the user in a PR review is missing#998
Added handling for cases where the user in a PR review is missing#998avasconcelos114 merged 1 commit intomasterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe changes introduce optional user fields in the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 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 |
|
@ogi-m Since this is some defensive coding that prevents an issue that personally affects my account, I'll merge this and confirm that the RC build for 2.7.1 also resolves the issue as my test build did |
Summary
There is an edge case where if an account that previously reviewed the PR is deleted, the list of PRs in the sidebar will fail to load as it attempts to read the user attribute for the review.
This PR combats this by both filtering the PR reviews that don't have an user attached to them, as well as adding some defensive coding on the webapp to also prevent this from affecting the rendering of the sidebar contents
QA Notes
Reproducing this naturally is somewhat difficult given that you need to have given a review to a PR with a temporary account that is then deleted. The easiest way to confirm that the fix is working would be to use a browser extension to mock the response of
/plugins/github/api/v1/prsdetailsand remove theuserobject from one of the reviews.Once that is done just connect your account and open the "Your Open Pull Requests" sidebar from the LHS menu buttons
Change Impact: 🟡 Medium
Reasoning: These changes span both backend and frontend to handle the edge case of missing review authors, and include a type definition change to optionality. While the changes are isolated to review handling and are purely defensive, the scope across layers and the modification to the data model warrant medium-level scrutiny.
Regression Risk: Low. The changes add null-checking guards before accessing potentially undefined properties and filter invalid reviews server-side before transmission. No existing behavioral logic is modified; these are additive defensive measures that prevent crashes in an edge case (deleted/missing user accounts in PR reviews) without altering normal operation.
QA Recommendation: Light manual QA is recommended. Primary verification: confirm the "Your Open Pull Requests" sidebar loads successfully when a PR contains a review from a deleted/missing user account. The PR provides browser dev tools instructions to mock the edge case. Risk of skipping QA is medium—while changes are defensive and low-regression, the sidebar is user-facing and the edge case being fixed is real. A quick smoke test of sidebar loading with typical PRs is sufficient for low-risk release.