also looking at this
fix(reviews): the sidebar computes its own data again
#2
1 file
+147
-0
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -151,6 +151,153 @@ for (const file of files) { | ||
| 151 | 151 | }) |
| 152 | 152 | } |
| 153 | 153 | |
| 154 | const descriptionHtml = pullRequest?.body | |
| 155 | ? await renderMarkdownHighlighted(String(pullRequest.body), markdownContext) | |
| 156 | : '' | |
| 157 | ||
| 158 | const protection = repoRow && pullRequest | |
| 159 | ? await db | |
| 160 | .selectFrom('protected_branches') | |
| 161 | .selectAll() | |
| 162 | .where('repository_id', '=', Number(repoRow.id)) | |
| 163 | .where('pattern', '=', pullRequest.base_branch) | |
| 164 | .executeTakeFirst() | |
| 165 | : undefined | |
| 166 | ||
| 167 | let requiredChecks: string[] = [] | |
| 168 | try { | |
| 169 | const parsed = JSON.parse(String(protection?.required_checks ?? '[]')) | |
| 170 | if (Array.isArray(parsed)) | |
| 171 | requiredChecks = parsed.map(String) | |
| 172 | } | |
| 173 | catch { | |
| 174 | requiredChecks = [] | |
| 175 | } | |
| 176 | ||
| 177 | const checkRows = pullRequest && requiredChecks.length > 0 | |
| 178 | ? await db | |
| 179 | .selectFrom('check_runs') | |
| 180 | .select(['name', 'status', 'conclusion', 'started_at']) | |
| 181 | .where('repository_id', '=', Number(repoRow!.id)) | |
| 182 | .where('head_sha', '=', pullRequest.head_sha) | |
| 183 | .execute() | |
| 184 | : [] | |
| 185 | ||
| 186 | const checkResult = requirementsSatisfied( | |
| 187 | checkRows.map((row: any) => ({ | |
| 188 | name: String(row.name), | |
| 189 | status: row.status, | |
| 190 | conclusion: row.conclusion, | |
| 191 | startedAt: Date.parse(String(row.started_at ?? '')) || 0, | |
| 192 | })), | |
| 193 | requiredChecks, | |
| 194 | ) | |
| 195 | ||
| 196 | const approval = approvalsSatisfied({ | |
| 197 | reviews: reviews.map((review: any) => ({ | |
| 198 | reviewerId: Number(review.reviewer_id), | |
| 199 | state: String(review.state), | |
| 200 | commitSha: review.commit_sha as string | null, | |
| 201 | })), | |
| 202 | headSha: pullRequest ? (pullRequest.head_sha as string | null) : null, | |
| 203 | requiredApprovals: Number(protection?.required_approvals ?? 0), | |
| 204 | dismissStaleReviews: Boolean(protection?.dismiss_stale_reviews), | |
| 205 | }) | |
| 206 | ||
| 207 | const unresolvedThreads = threads.filter((thread: any) => !thread.resolved).length | |
| 208 | ||
| 209 | /* | |
| 210 | * Mergeability, cached against the two commits it was computed from. | |
| 211 | * | |
| 212 | * The roadmap wants this computed in the background; it is computed here for | |
| 213 | * now, and the cache is what makes that acceptable: the answer is recomputed | |
| 214 | * only when one of the branches has moved, so a hundred people opening this | |
| 215 | * page cost one merge between them rather than a hundred. Nothing here can move | |
| 216 | * a ref, because `git merge-tree` merges in memory. | |
| 217 | */ | |
| 218 | const mergeability = pullRequest | |
| 219 | ? await refreshMergeability(owner, repositoryName, { | |
| 220 | id: Number(pullRequest.id), | |
| 221 | base_sha: String(pullRequest.base_sha), | |
| 222 | head_sha: String(pullRequest.head_sha), | |
| 223 | mergeable_state: pullRequest.mergeable_state as string | null, | |
| 224 | mergeable_base_sha: pullRequest.mergeable_base_sha as string | null, | |
| 225 | mergeable_head_sha: pullRequest.mergeable_head_sha as string | null, | |
| 226 | mergeable_conflicts: pullRequest.mergeable_conflicts as string | null, | |
| 227 | }) | |
| 228 | : { state: 'unknown' as const, treeSha: null, conflictingPaths: [], recomputed: false } | |
| 229 | ||
| 230 | const blockers = pullRequest | |
| 231 | ? mergeBlockers( | |
| 232 | { | |
| 233 | state: pullRequest.state as 'open' | 'closed' | 'merged', | |
| 234 | draft: Boolean(pullRequest.draft), | |
| 235 | mergeable: mergeability.state === 'clean' | |
| 236 | ? true | |
| 237 | : (mergeability.state === 'unknown' ? null : false), | |
| 238 | stackParent: null, | |
| 239 | }, | |
| 240 | { | |
| 241 | requiredApprovals: Number(protection?.required_approvals ?? 0), | |
| 242 | requireThreadsResolved: Boolean(protection?.require_conversation_resolution), | |
| 243 | requireLinearHistory: Boolean(protection?.require_linear_history), | |
| 244 | allowedStrategies: ['merge', 'squash', 'rebase'], | |
| 245 | requiredChecks, | |
| 246 | }, | |
| 247 | { | |
| 248 | approvals: approval.approvals, | |
| 249 | blockingReviews: approval.blocking, | |
| 250 | unresolvedThreads, | |
| 251 | checks: checkResult, | |
| 252 | }, | |
| 253 | 'merge', | |
| 254 | ) | |
| 255 | : [] | |
| 256 | ||
| 257 | /* | |
| 258 | * The stack this pull request belongs to. | |
| 259 | * | |
| 260 | * Loaded for the whole repository in one query rather than by walking parent | |
| 261 | * links a level at a time: a stack is small, and a round trip per level is the | |
| 262 | * shape that makes a deep stack slow to open. | |
| 263 | */ | |
| 264 | const stackRows = pullRequest | |
| 265 | ? await db | |
| 266 | .selectFrom('pull_requests') | |
| 267 | .select(['id', 'number', 'title', 'state', 'head_branch', 'base_branch', 'stack_parent_id', 'draft']) | |
| 268 | .where('repository_id', '=', Number(repoRow.id)) | |
| 269 | .execute() | |
| 270 | : [] | |
| 271 | ||
| 272 | const stackMembers = stackRows.map((row: any) => ({ | |
| 273 | id: Number(row.id), | |
| 274 | number: Number(row.number), | |
| 275 | title: String(row.title), | |
| 276 | state: String(row.state), | |
| 277 | headBranch: String(row.head_branch), | |
| 278 | baseBranch: String(row.base_branch), | |
| 279 | stackParentId: row.stack_parent_id ? Number(row.stack_parent_id) : null, | |
| 280 | draft: Boolean(row.draft), | |
| 281 | })) | |
| 282 | ||
| 283 | const currentMember = pullRequest ? stackMembers.find((entry: any) => entry.id === Number(pullRequest.id)) : undefined | |
| 284 | const stack = currentMember ? buildStack(stackMembers, currentMember.id) : [] | |
| 285 | const stackText = stackSummary(stack) | |
| 286 | const stackOrphan = currentMember ? orphanMessage(orphanReason(currentMember, stackMembers)) : null | |
| 287 | ||
| 288 | /* | |
| 289 | * Only this pull request's own blockers are known here, so the rest of the | |
| 290 | * stack is judged on what can be checked cheaply. A member whose readiness is | |
| 291 | * unknown counts as not ready, which errs toward saying "waiting on" rather | |
| 292 | * than promising a merge that would then be refused. | |
| 293 | */ | |
| 294 | const stackReadiness = stack.map((entry: any) => ({ | |
| 295 | id: entry.id, | |
| 296 | blockers: currentMember && entry.id === currentMember.id ? blockers : (entry.state === 'open' && !entry.draft ? [] : ['not ready']), | |
| 297 | })) | |
| 298 | ||
| 299 | const stackBlocker = currentMember ? blockedBy(stack, currentMember.id, stackReadiness) : null | |
| 300 | ||
| 154 | 301 | const title = pullRequest |
| 155 | 302 | ? `${pullRequest.title} by ${authorName ?? 'someone'} · ${owner}/${repositoryName} #${number}` |
| 156 | 303 | : 'Pull request not found' |