forge
ExploreLog in

Tahinli/forge

public
0 watching
★ 0
42.5 KiB
1//! Pull request review routes: line comments on the Files tab, replies, edits, deletes, thread
2//! resolution, review submission, discarding a pending review and dismissing a review. Writes
3//! are POST with CSRF. The server recomputes the diff and decides what a line is, so a form can
4//! only comment on a line the current diff shows.
5//!
6//! A refused write redirects back to its tab with `?review_error=<code>`; the codes are the
7//! `ERR_*` constants below. Permissions: any signed-in reader may comment and review; only the
8//! author edits a comment; the author or Triage deletes; the PR author or Write resolves; Write
9//! dismisses. Archived repos refuse every write. Closed or merged pull requests accept comments
10//! and replies but no new reviews.
11
12use forge_core::reviews::{ReviewComment, ReviewState, Side};
13use forge_core::{Action, IssueState, PullRequest, User, authorize};
14use forge_git::history::{CommitDiff, DiffStatus, FileDiff, LineKind};
15use forge_git::pulls::range_diff_files;
16use serde::Deserialize;
17use topcoat::Result;
18use topcoat::context::{Cx, app_context};
19use topcoat::router::content::Form;
20use topcoat::router::error::{SeeOther, forbidden, not_found, see_other};
21use topcoat::router::request::uri;
22use topcoat::router::{page, path_param, route};
23use topcoat::view::{View, view};
24
25use crate::auth::require_user;
26use crate::i18n::pulls_review::{self as g, K};
27use crate::i18n::{self, Lang};
28use crate::notify::{self, Activity};
29use crate::pages::issue_parts::{check_write, csrf_of, may_change, may_converse, parse_number};
30use crate::pages::repo::repo_head;
31use crate::pages::repo_ctx::{RepoCtx, load_repo};
32use crate::state::AppState;
33use crate::views::diff::LIMITS;
34use crate::views::{alert, repo_href, shell};
35use crate::webhooks::emit::{Sender, emit_review};
36
37path_param!(owner);
38path_param!(repo);
39path_param!(number);
40path_param!(comment_id);
41path_param!(review_id);
42
43/// The `review_error` codes a refused write redirects with.
44pub(crate) const ERR_STALE: &str = "stale";
45pub(crate) const ERR_OWN_PR: &str = "own_pr";
46pub(crate) const ERR_EMPTY: &str = "empty";
47pub(crate) const ERR_INVALID: &str = "invalid";
48pub(crate) const ERR_BAD_LINE: &str = "bad_line";
49pub(crate) const ERR_CLOSED: &str = "closed";
50
51/// The message for a `review_error` code in `lang`, or `None` for an unknown code.
52pub(crate) fn error_text_in(lang: Lang, code: &str) -> Option<&'static str> {
53 match code {
54 ERR_STALE => Some(g::t(lang, K::ReviewErrStale)),
55 ERR_OWN_PR => Some(g::t(lang, K::ReviewErrOwnPr)),
56 ERR_EMPTY => Some(g::t(lang, K::ReviewErrEmpty)),
57 ERR_INVALID => Some(g::t(lang, K::ReviewErrInvalid)),
58 ERR_BAD_LINE => Some(g::t(lang, K::ReviewErrBadLine)),
59 ERR_CLOSED => Some(g::t(lang, K::ReviewErrClosed)),
60 _ => None,
61 }
62}
63
64/// The message for the `review_error` query value of the request, when it is a known code.
65fn error_from(lang: Lang, cx: &Cx) -> Option<&'static str> {
66 let request = uri(cx);
67 let query = request.query()?;
68 let (_, code) =
69 form_urlencoded::parse(query.as_bytes()).find(|(key, _)| *key == "review_error")?;
70 error_text_in(lang, &code)
71}
72
73/// The form fields of every review POST. Unused fields are empty.
74#[derive(Deserialize, Default)]
75#[serde(default)]
76struct ReviewForm {
77 csrf: String,
78 body: String,
79 /// `single` or `review` for a line comment.
80 action: String,
81 head_oid: String,
82 file_index: String,
83 side: String,
84 line: String,
85 /// `comment`, `approve` or `request_changes` for a review submit.
86 #[serde(rename = "state")]
87 verdict: String,
88 reason: String,
89 /// `1` to resolve a thread, `0` to unresolve it.
90 resolved: String,
91 /// The tab the form was posted from: `files` or anything else for the conversation.
92 #[serde(rename = "return")]
93 ret: String,
94}
95
96// Diff and lookups
97
98/// The diff of the pull request: the merge base of `base_oid` and `head_oid` to head, with the
99/// Files tab's limits. The Files tab and the comment routes both read it, so a line means the
100/// same thing on both.
101pub(crate) async fn pull_diff(
102 state: &AppState,
103 ctx: &RepoCtx,
104 pr: &PullRequest,
105) -> Result<CommitDiff> {
106 Ok(range_diff_files(&state.git, &ctx.path, &pr.base_oid, &pr.head_oid, LIMITS).await?)
107}
108
109/// The pull request named by `{number}` in this repo, or 404.
110async fn pull_for(state: &AppState, ctx: &RepoCtx, number: i64) -> Result<PullRequest> {
111 match state.store.pull_by_number(&ctx.repo.id, number).await? {
112 Some(pr) => Ok(pr),
113 None => Err(not_found().into()),
114 }
115}
116
117/// Whether the pull request is closed or merged. Its comments stay open; its reviews do not.
118fn is_closed(pr: &PullRequest) -> bool {
119 pr.issue.state != IssueState::Open || pr.merged_at.is_some()
120}
121
122/// The path a diff file's comments are stored under: the new path, or the old path for a
123/// deletion.
124fn file_path(file: &FileDiff) -> Option<&str> {
125 if file.status == DiffStatus::Deleted {
126 file.old_path.as_deref()
127 } else {
128 file.new_path.as_deref().or(file.old_path.as_deref())
129 }
130}
131
132/// Whether `line` is the line `number` on `side`: an added or context line on the new side, a
133/// deleted line on the old side.
134fn on_line(line: &forge_git::history::DiffLine, side: Side, number: u32) -> bool {
135 match side {
136 Side::New => {
137 matches!(line.kind, LineKind::Add | LineKind::Context) && line.new_no == Some(number)
138 }
139 Side::Old => line.kind == LineKind::Del && line.old_no == Some(number),
140 }
141}
142
143/// The `diff_hunk` for a comment on `number` of `side`: up to four lines of the same hunk ending
144/// at that line, each with a `+`, `-` or space prefix. `None` when the diff has no such line.
145fn diff_hunk_at(file: &FileDiff, side: Side, number: u32) -> Option<String> {
146 for hunk in &file.hunks {
147 let Some(at) = hunk
148 .lines
149 .iter()
150 .position(|line| on_line(line, side, number))
151 else {
152 continue;
153 };
154 let mut lines: Vec<String> = hunk.lines[..=at]
155 .iter()
156 .rev()
157 .filter_map(|line| {
158 let prefix = match line.kind {
159 LineKind::Add => '+',
160 LineKind::Del => '-',
161 LineKind::Context => ' ',
162 LineKind::NoNewline => return None,
163 };
164 Some(format!("{prefix}{}", line.text))
165 })
166 .take(4)
167 .collect();
168 lines.reverse();
169 return Some(lines.join("\n"));
170 }
171 None
172}
173
174/// A comment of this pull request that the viewer can see (a submitted one, or their own
175/// pending one), root or reply. Anything else is 404.
176async fn comment_on_pull(
177 state: &AppState,
178 pr: &PullRequest,
179 viewer: &User,
180 comment_id: &str,
181) -> Result<ReviewComment> {
182 for thread in state
183 .store
184 .review_threads(&pr.issue.id, Some(&viewer.id))
185 .await?
186 {
187 if thread.root.id == comment_id {
188 return Ok(thread.root);
189 }
190 if let Some(reply) = thread.replies.into_iter().find(|r| r.id == comment_id) {
191 return Ok(reply);
192 }
193 }
194 Err(not_found().into())
195}
196
197/// The id of the root comment a comment belongs to (its own id for a root).
198fn root_of(comment: &ReviewComment) -> String {
199 comment
200 .reply_to
201 .clone()
202 .unwrap_or_else(|| comment.id.clone())
203}
204
205/// The conversation, or the Files tab when `ret` is `files`, for this pull request.
206fn tab_url(ctx: &RepoCtx, number: i64, ret: &str) -> String {
207 let base = repo_href(
208 &ctx.repo.owner_name,
209 &ctx.repo.name,
210 "pulls",
211 &number.to_string(),
212 );
213 if ret == "files" {
214 format!("{base}/files")
215 } else {
216 base
217 }
218}
219
220/// Redirects back to `tab` with a refusal code; `anchor` is `#rc-<root>` or empty.
221fn refusal(tab: &str, code: &str, anchor: &str) -> SeeOther {
222 see_other(format!("{tab}?review_error={code}{anchor}"))
223}
224
225// Line comments
226
227/// Adds a line comment (single, or into the author's pending review) on the Files tab. The
228/// request must name the head the form was rendered for, and the line must be on that diff.
229#[route(POST "/{owner}/{repo}/pulls/{number}/review_comments")]
230async fn review_comment_post(cx: &Cx, Form(form): Form<ReviewForm>) -> Result<SeeOther> {
231 let number = parse_number(path_param::<Number>(cx))?;
232 let ctx = load_repo(cx, path_param::<Owner>(cx), path_param::<Repo>(cx)).await?;
233 let (user, session) = require_user(cx).await?;
234 check_write(&ctx, &session, &form.csrf)?;
235 let state = app_context::<AppState>(cx);
236 let pr = pull_for(state, &ctx, number).await?;
237 if !may_converse(&ctx, &pr.issue) {
238 return Err(forbidden().into());
239 }
240 let files = tab_url(&ctx, number, "files");
241
242 if form.head_oid != pr.head_oid {
243 return Ok(refusal(&files, ERR_STALE, ""));
244 }
245 let pending = match form.action.as_str() {
246 "single" => false,
247 "review" => true,
248 _ => return Ok(refusal(&files, ERR_INVALID, "")),
249 };
250 if form.body.trim().is_empty() {
251 return Ok(refusal(&files, ERR_EMPTY, ""));
252 }
253 if pending && is_closed(&pr) {
254 return Ok(refusal(&files, ERR_CLOSED, ""));
255 }
256 let (Ok(file_index), Ok(line), Ok(side)) = (
257 form.file_index.parse::<usize>(),
258 form.line.parse::<u32>(),
259 form.side.parse::<Side>(),
260 ) else {
261 return Ok(refusal(&files, ERR_BAD_LINE, ""));
262 };
263 let back = format!("{files}?comment={file_index}:{}:{line}", side.as_str());
264
265 let diff = pull_diff(state, &ctx, &pr).await?;
266 let Some(file) = diff.files.get(file_index) else {
267 return Ok(refusal(&files, ERR_BAD_LINE, ""));
268 };
269 let (Some(path), Some(hunk)) = (file_path(file), diff_hunk_at(file, side, line)) else {
270 return Ok(refusal(&files, ERR_BAD_LINE, ""));
271 };
272 match state
273 .store
274 .add_review_comment(
275 &pr.issue.id,
276 &user.id,
277 pending,
278 path,
279 side,
280 line,
281 &pr.head_oid,
282 &hunk,
283 &form.body,
284 )
285 .await
286 {
287 Ok(_) => Ok(see_other(format!(
288 "{files}#L-{file_index}-{}-{line}",
289 side.as_str()
290 ))),
291 Err(forge_core::Error::Invalid(_)) => {
292 Ok(see_other(format!("{back}&review_error={ERR_INVALID}")))
293 }
294 Err(e) => Err(e.into()),
295 }
296}
297
298/// Replies in the thread of a root comment. Replies post at once, even on a closed pull request.
299#[route(POST "/{owner}/{repo}/pulls/{number}/review_comments/{comment_id}/reply")]
300async fn review_comment_reply(cx: &Cx, Form(form): Form<ReviewForm>) -> Result<SeeOther> {
301 let number = parse_number(path_param::<Number>(cx))?;
302 let ctx = load_repo(cx, path_param::<Owner>(cx), path_param::<Repo>(cx)).await?;
303 let (user, session) = require_user(cx).await?;
304 check_write(&ctx, &session, &form.csrf)?;
305 let state = app_context::<AppState>(cx);
306 let pr = pull_for(state, &ctx, number).await?;
307 if !may_converse(&ctx, &pr.issue) {
308 return Err(forbidden().into());
309 }
310 let parent = comment_on_pull(state, &pr, &user, path_param::<CommentId>(cx)).await?;
311 let root = root_of(&parent);
312 let tab = tab_url(&ctx, number, &form.ret);
313 let anchor = format!("#rc-{root}");
314 if form.body.trim().is_empty() {
315 return Ok(refusal(&tab, ERR_EMPTY, &anchor));
316 }
317 match state
318 .store
319 .reply_review_comment(&pr.issue.id, &user.id, &root, &form.body)
320 .await
321 {
322 Ok(_) => Ok(see_other(format!("{tab}{anchor}"))),
323 Err(forge_core::Error::Invalid(_)) => Ok(refusal(&tab, ERR_INVALID, &anchor)),
324 Err(e) => Err(e.into()),
325 }
326}
327
328/// The edit form for a comment. Only its author may open it.
329#[page("/{owner}/{repo}/pulls/{number}/review_comments/{comment_id}/edit")]
330async fn review_comment_edit_form(cx: &Cx) -> Result<impl View> {
331 let number = parse_number(path_param::<Number>(cx))?;
332 let ctx = load_repo(cx, path_param::<Owner>(cx), path_param::<Repo>(cx)).await?;
333 let (user, _) = require_user(cx).await?;
334 let lang = i18n::lang(cx, Some(&user));
335 let state = app_context::<AppState>(cx);
336 let pr = pull_for(state, &ctx, number).await?;
337 let comment = comment_on_pull(state, &pr, &user, path_param::<CommentId>(cx)).await?;
338 if comment.author_id != user.id {
339 return Err(forbidden().into());
340 }
341 let error = error_from(lang, cx);
342 edit_page(cx, ctx, number, comment, error, lang).await
343}
344
345/// The edit page: a textarea with the comment's body, and the refusal message when there is one.
346async fn edit_page(
347 cx: &Cx,
348 ctx: RepoCtx,
349 number: i64,
350 comment: ReviewComment,
351 error: Option<&'static str>,
352 lang: Lang,
353) -> Result<impl View> {
354 let state = app_context::<AppState>(cx);
355 let owner = ctx.repo.owner_name.clone();
356 let name = ctx.repo.name.clone();
357 let csrf = csrf_of(&ctx);
358 let action = repo_href(
359 &owner,
360 &name,
361 "pulls",
362 &format!("{number}/review_comments/{}/edit", comment.id),
363 );
364 let back = format!("{}#rc-{}", tab_url(&ctx, number, ""), root_of(&comment));
365 let page_title = g::edit_title(lang, number, &format!("{owner}/{name}"));
366 let heading = g::t(lang, K::EditReviewComment);
367 let comment_label = g::t(lang, K::CommentLabel);
368 let save_label = g::t(lang, K::SaveComment);
369 let cancel_label = g::t(lang, K::CancelButton);
370 let header = ctx.head(state, "pulls").await?;
371 Ok(view! { cx =>
372 shell(title: page_title.as_str(), viewer: ctx.viewer.as_ref(),
373 repo_head(head: header)
374 <section class="card issue-edit">
375 <h2>(heading)</h2>
376 if let Some(message) = error {
377 alert(message: message)
378 }
379 <form method="post" action=(action.as_str()) class="stack">
380 <input type="hidden" name="csrf" value=(csrf.as_str())>
381 <label>
382 <span>(comment_label)</span>
383 <textarea name="body" rows="10">(comment.body.as_str())</textarea>
384 </label>
385 <div class="issue-actions">
386 <button type="submit">(save_label)</button>
387 <a href=(back.as_str())>(cancel_label)</a>
388 </div>
389 </form>
390 </section>
391 )
392 })
393}
394
395/// Saves an edited comment. Only its author; the store records the previous body.
396#[route(POST "/{owner}/{repo}/pulls/{number}/review_comments/{comment_id}/edit")]
397async fn review_comment_edit_save(cx: &Cx, Form(form): Form<ReviewForm>) -> Result<SeeOther> {
398 let number = parse_number(path_param::<Number>(cx))?;
399 let ctx = load_repo(cx, path_param::<Owner>(cx), path_param::<Repo>(cx)).await?;
400 let (user, session) = require_user(cx).await?;
401 check_write(&ctx, &session, &form.csrf)?;
402 let state = app_context::<AppState>(cx);
403 let pr = pull_for(state, &ctx, number).await?;
404 let comment = comment_on_pull(state, &pr, &user, path_param::<CommentId>(cx)).await?;
405 if comment.author_id != user.id {
406 return Err(forbidden().into());
407 }
408 let edit_url = format!(
409 "{}/review_comments/{}/edit",
410 tab_url(&ctx, number, ""),
411 comment.id
412 );
413 if form.body.trim().is_empty() {
414 return Ok(refusal(&edit_url, ERR_EMPTY, ""));
415 }
416 match state
417 .store
418 .edit_review_comment(&comment.id, &form.body)
419 .await
420 {
421 Ok(()) => Ok(see_other(format!(
422 "{}#rc-{}",
423 tab_url(&ctx, number, &form.ret),
424 root_of(&comment)
425 ))),
426 Err(forge_core::Error::Invalid(_)) => Ok(refusal(&edit_url, ERR_INVALID, "")),
427 Err(e) => Err(e.into()),
428 }
429}
430
431/// Deletes a comment. Its author or Triage. Deleting a root deletes its replies.
432#[route(POST "/{owner}/{repo}/pulls/{number}/review_comments/{comment_id}/delete")]
433async fn review_comment_delete(cx: &Cx, Form(form): Form<ReviewForm>) -> Result<SeeOther> {
434 let number = parse_number(path_param::<Number>(cx))?;
435 let ctx = load_repo(cx, path_param::<Owner>(cx), path_param::<Repo>(cx)).await?;
436 let (user, session) = require_user(cx).await?;
437 check_write(&ctx, &session, &form.csrf)?;
438 let state = app_context::<AppState>(cx);
439 let pr = pull_for(state, &ctx, number).await?;
440 let comment = comment_on_pull(state, &pr, &user, path_param::<CommentId>(cx)).await?;
441 if !may_change(&ctx, &comment.author_id) {
442 return Err(forbidden().into());
443 }
444 state.store.delete_review_comment(&comment.id).await?;
445 let tab = tab_url(&ctx, number, &form.ret);
446 if comment.reply_to.is_some() {
447 Ok(see_other(format!("{tab}#rc-{}", root_of(&comment))))
448 } else {
449 Ok(see_other(tab))
450 }
451}
452
453/// Resolves or unresolves a thread (`resolved=1|0`). The PR author or Write.
454#[route(POST "/{owner}/{repo}/pulls/{number}/review_comments/{comment_id}/resolve")]
455async fn review_comment_resolve(cx: &Cx, Form(form): Form<ReviewForm>) -> Result<SeeOther> {
456 let number = parse_number(path_param::<Number>(cx))?;
457 let ctx = load_repo(cx, path_param::<Owner>(cx), path_param::<Repo>(cx)).await?;
458 let (user, session) = require_user(cx).await?;
459 check_write(&ctx, &session, &form.csrf)?;
460 let state = app_context::<AppState>(cx);
461 let pr = pull_for(state, &ctx, number).await?;
462 let comment = comment_on_pull(state, &pr, &user, path_param::<CommentId>(cx)).await?;
463 let is_pr_author = pr.issue.author_id == user.id;
464 if !is_pr_author
465 && authorize(
466 Some(&user),
467 &ctx.repo,
468 ctx.role,
469 ctx.org_role,
470 Action::Write,
471 )
472 .is_err()
473 {
474 return Err(forbidden().into());
475 }
476 let root = root_of(&comment);
477 let tab = tab_url(&ctx, number, &form.ret);
478 let anchor = format!("#rc-{root}");
479 let resolved = match form.resolved.as_str() {
480 "1" => true,
481 "0" => false,
482 _ => return Ok(refusal(&tab, ERR_INVALID, &anchor)),
483 };
484 state
485 .store
486 .set_thread_resolved(&root, &user.id, resolved)
487 .await?;
488 Ok(see_other(format!("{tab}{anchor}")))
489}
490
491// Reviews
492
493/// Submits the author's pending review as a comment, approval or change request. The PR author
494/// cannot approve or request changes on their own pull request. Closed pull requests refuse it.
495#[route(POST "/{owner}/{repo}/pulls/{number}/reviews")]
496async fn review_submit(cx: &Cx, Form(form): Form<ReviewForm>) -> Result<SeeOther> {
497 let number = parse_number(path_param::<Number>(cx))?;
498 let ctx = load_repo(cx, path_param::<Owner>(cx), path_param::<Repo>(cx)).await?;
499 let (user, session) = require_user(cx).await?;
500 check_write(&ctx, &session, &form.csrf)?;
501 let state = app_context::<AppState>(cx);
502 let pr = pull_for(state, &ctx, number).await?;
503 if !may_converse(&ctx, &pr.issue) {
504 return Err(forbidden().into());
505 }
506 let tab = tab_url(&ctx, number, &form.ret);
507 let verdict = match form.verdict.as_str() {
508 "comment" => ReviewState::Commented,
509 "approve" => ReviewState::Approved,
510 "request_changes" => ReviewState::ChangesRequested,
511 _ => return Ok(refusal(&tab, ERR_INVALID, "")),
512 };
513 if is_closed(&pr) {
514 return Ok(refusal(&tab, ERR_CLOSED, ""));
515 }
516 match state
517 .store
518 .submit_review(&pr.issue.id, &user.id, verdict, &form.body, &pr.head_oid)
519 .await
520 {
521 Ok(review) => {
522 emit_review(state, &ctx.repo, &Sender::of(&user), &pr, &review).await;
523 let text = review_text(state, &pr.issue, &user.id, &review).await;
524 notify::dispatch(
525 state,
526 &ctx.repo,
527 &pr.issue,
528 &user.id,
529 Activity::Review,
530 Some(&text),
531 )
532 .await;
533 Ok(see_other(format!("{tab}#review-{}", review.id)))
534 }
535 Err(forge_core::Error::Forbidden(_)) => Ok(refusal(&tab, ERR_OWN_PR, "")),
536 Err(forge_core::Error::Invalid(_)) => Ok(refusal(&tab, ERR_EMPTY, "")),
537 Err(e) => Err(e.into()),
538 }
539}
540
541/// The review body and the bodies of the review's comments, joined for mention extraction.
542async fn review_text(
543 state: &AppState,
544 issue: &forge_core::Issue,
545 viewer_id: &str,
546 review: &forge_core::Review,
547) -> String {
548 let mut parts = vec![review.body.clone()];
549 match state.store.review_threads(&issue.id, Some(viewer_id)).await {
550 Ok(threads) => {
551 for thread in threads {
552 for comment in std::iter::once(thread.root).chain(thread.replies) {
553 if comment.review_id == review.id {
554 parts.push(comment.body);
555 }
556 }
557 }
558 }
559 Err(e) => {
560 tracing::error!(error = %e, issue_id = %issue.id, "notify: review comments lookup failed");
561 }
562 }
563 parts.join("\n\n")
564}
565
566/// Discards the viewer's pending review and its draft comments.
567#[route(POST "/{owner}/{repo}/pulls/{number}/reviews/discard")]
568async fn review_discard(cx: &Cx, Form(form): Form<ReviewForm>) -> Result<SeeOther> {
569 let number = parse_number(path_param::<Number>(cx))?;
570 let ctx = load_repo(cx, path_param::<Owner>(cx), path_param::<Repo>(cx)).await?;
571 let (user, session) = require_user(cx).await?;
572 check_write(&ctx, &session, &form.csrf)?;
573 let state = app_context::<AppState>(cx);
574 let pr = pull_for(state, &ctx, number).await?;
575 state
576 .store
577 .discard_pending_review(&pr.issue.id, &user.id)
578 .await?;
579 Ok(see_other(tab_url(&ctx, number, &form.ret)))
580}
581
582/// Dismisses an approval or a change request with a reason. Write only.
583#[route(POST "/{owner}/{repo}/pulls/{number}/reviews/{review_id}/dismiss")]
584async fn review_dismiss(cx: &Cx, Form(form): Form<ReviewForm>) -> Result<SeeOther> {
585 let number = parse_number(path_param::<Number>(cx))?;
586 let ctx = load_repo(cx, path_param::<Owner>(cx), path_param::<Repo>(cx)).await?;
587 let (user, session) = require_user(cx).await?;
588 check_write(&ctx, &session, &form.csrf)?;
589 if authorize(
590 Some(&user),
591 &ctx.repo,
592 ctx.role,
593 ctx.org_role,
594 Action::Write,
595 )
596 .is_err()
597 {
598 return Err(forbidden().into());
599 }
600 let state = app_context::<AppState>(cx);
601 let pr = pull_for(state, &ctx, number).await?;
602 let wanted = path_param::<ReviewId>(cx);
603 let Some(review) = state
604 .store
605 .list_reviews(&pr.issue.id)
606 .await?
607 .into_iter()
608 .find(|r| r.id == wanted)
609 else {
610 return Err(not_found().into());
611 };
612 let tab = tab_url(&ctx, number, &form.ret);
613 if form.reason.trim().is_empty() {
614 return Ok(refusal(&tab, ERR_EMPTY, ""));
615 }
616 match state
617 .store
618 .dismiss_review(&review.id, &user.id, &form.reason)
619 .await
620 {
621 Ok(()) => Ok(see_other(format!("{tab}#review-{}", review.id))),
622 Err(forge_core::Error::Invalid(_)) => Ok(refusal(&tab, ERR_INVALID, "")),
623 Err(e) => Err(e.into()),
624 }
625}
626
627#[cfg(test)]
628mod tests {
629 use forge_core::reviews::{ReviewState, Side};
630 use forge_core::{IssueState, PullRequest, User, Visibility};
631 use forge_git::GitEnv;
632 use topcoat::router::request::Request;
633 use topcoat::router::{Body, Router, to_bytes};
634
635 use crate::auth::{SESSION_COOKIE, percent_encode};
636 use crate::pages::commits::test_support::{Fixture, OWNER, REPO, git};
637
638 fn cookie(token: &str) -> String {
639 format!("__Host-{SESSION_COOKIE}={token}")
640 }
641
642 /// Sends one request through the router and returns `(status, location, body)`.
643 async fn send(
644 router: &Router,
645 method: &str,
646 uri: &str,
647 token: Option<&str>,
648 form: Option<String>,
649 ) -> (u16, Option<String>, String) {
650 let mut req = Request::builder().method(method).uri(uri);
651 if let Some(token) = token {
652 req = req.header("cookie", cookie(token));
653 }
654 let body = match form {
655 Some(form) => {
656 req = req.header("content-type", "application/x-www-form-urlencoded");
657 Body::from(form)
658 }
659 None => Body::empty(),
660 };
661 let res = router.handle(req.body(body).unwrap()).await;
662 let status = res.status().as_u16();
663 let location = res
664 .headers()
665 .get("location")
666 .map(|v| v.to_str().unwrap().to_owned());
667 let bytes = to_bytes(res.into_body(), usize::MAX).await.unwrap();
668 (
669 status,
670 location,
671 String::from_utf8_lossy(&bytes).into_owned(),
672 )
673 }
674
675 /// A user with a session whose CSRF token is `{token}-csrf`.
676 async fn sign_in(fx: &Fixture, name: &str, token: &str) -> User {
677 let user = fx
678 .store
679 .create_user(name, None, "not-a-real-hash", false)
680 .await
681 .unwrap();
682 crate::test_support::create_session(&fx.store, &user.id, token, &format!("{token}-csrf"))
683 .await;
684 user
685 }
686
687 /// The repo owner, signed in with token `alice` (CSRF `alice-csrf`).
688 async fn owner_session(fx: &Fixture) {
689 let owner = fx.store.user_by_username(OWNER).await.unwrap().unwrap();
690 crate::test_support::create_session(&fx.store, &owner.id, "alice", "alice-csrf").await;
691 }
692
693 fn csrf_form(token: &str, pairs: &[(&str, &str)]) -> String {
694 let mut form = format!("csrf={}", percent_encode(&format!("{token}-csrf")));
695 for (key, value) in pairs {
696 form.push_str(&format!("&{key}={}", percent_encode(value)));
697 }
698 form
699 }
700
701 async fn pull(fx: &Fixture, number: i64) -> PullRequest {
702 let repo = fx
703 .store
704 .repo_by_owner_and_name(OWNER, REPO)
705 .await
706 .unwrap()
707 .unwrap();
708 fx.store
709 .pull_by_number(&repo.id, number)
710 .await
711 .unwrap()
712 .unwrap()
713 }
714
715 /// Pushes a `feature` branch from the root that changes `a.txt` (`two` becomes `changed`) and
716 /// opens a pull request for it against `main`. Returns the pull request number.
717 async fn open_pull(fx: &Fixture) -> i64 {
718 let base_oid = git(&fx.work, &["rev-parse", "HEAD"]);
719 git(&fx.work, &["checkout", "-q", "-b", "feature", &fx.root]);
720 std::fs::write(fx.work.join("a.txt"), "one\nchanged\nthree\n").unwrap();
721 git(&fx.work, &["add", "-A"]);
722 git(&fx.work, &["commit", "-q", "-m", "Change a.txt"]);
723 let head_oid = git(&fx.work, &["rev-parse", "HEAD"]);
724 git(
725 &fx.work,
726 &[
727 "push",
728 "-q",
729 &fx.bare.to_string_lossy(),
730 "HEAD:refs/heads/feature",
731 ],
732 );
733
734 let repo = fx
735 .store
736 .repo_by_owner_and_name(OWNER, REPO)
737 .await
738 .unwrap()
739 .unwrap();
740 let owner = fx.store.user_by_username(OWNER).await.unwrap().unwrap();
741 let pr = fx
742 .store
743 .create_pull(
744 &repo.id, &owner.id, "Change a", "", "main", &repo.id, "feature", &head_oid,
745 &base_oid,
746 )
747 .await
748 .unwrap();
749 let env = GitEnv {
750 git_bin: "git".into(),
751 data_dir: fx.work.clone(),
752 };
753 forge_git::pulls::fetch_into_pull_ref(&env, &fx.bare, &fx.bare, &head_oid, pr.issue.number)
754 .await
755 .unwrap();
756 pr.issue.number
757 }
758
759 /// The pull request's current head, for the `head_oid` field of a comment form.
760 async fn head_of(fx: &Fixture, number: i64) -> String {
761 pull(fx, number).await.head_oid
762 }
763
764 /// Posts a single line comment on line `line` of the new side of `a.txt` (file index 0).
765 async fn comment_on(
766 fx: &Fixture,
767 number: i64,
768 token: &str,
769 line: &str,
770 body: &str,
771 ) -> (u16, Option<String>) {
772 let head = head_of(fx, number).await;
773 let form = csrf_form(
774 token,
775 &[
776 ("file_index", "0"),
777 ("side", "new"),
778 ("line", line),
779 ("head_oid", head.as_str()),
780 ("action", "single"),
781 ("body", body),
782 ],
783 );
784 let (status, location, _) = send(
785 &fx.router,
786 "POST",
787 &format!("/alice/demo/pulls/{number}/review_comments"),
788 Some(token),
789 Some(form),
790 )
791 .await;
792 (status, location)
793 }
794
795 /// The id of the first root comment on the pull request.
796 async fn root_id(fx: &Fixture, number: i64) -> String {
797 let pr = pull(fx, number).await;
798 fx.store.review_threads(&pr.issue.id, None).await.unwrap()[0]
799 .root
800 .id
801 .clone()
802 }
803
804 #[tokio::test]
805 async fn single_comment_on_an_added_line_creates_a_thread_with_its_hunk() {
806 let fx = Fixture::new(Visibility::Public).await;
807 owner_session(&fx).await;
808 let number = open_pull(&fx).await;
809 let pr = pull(&fx, number).await;
810
811 let (status, location) = comment_on(&fx, number, "alice", "2", "Looks good").await;
812 assert_eq!(status, 303);
813 assert_eq!(
814 location.as_deref(),
815 Some(format!("/alice/demo/pulls/{number}/files#L-0-new-2").as_str())
816 );
817 let threads = fx.store.review_threads(&pr.issue.id, None).await.unwrap();
818 assert_eq!(threads.len(), 1);
819 let root = &threads[0].root;
820 assert_eq!(
821 (root.path.as_str(), root.side, root.line),
822 ("a.txt", Side::New, 2)
823 );
824 assert_eq!(root.diff_hunk, " one\n-two\n+changed");
825 assert!(!root.pending);
826 }
827
828 #[tokio::test]
829 async fn comment_on_a_line_outside_the_diff_is_rejected() {
830 let fx = Fixture::new(Visibility::Public).await;
831 owner_session(&fx).await;
832 let number = open_pull(&fx).await;
833 let pr = pull(&fx, number).await;
834
835 let (status, location) = comment_on(&fx, number, "alice", "99", "Nope").await;
836 assert_eq!(status, 303);
837 assert!(location.unwrap().contains("review_error=bad_line"));
838 assert!(
839 fx.store
840 .review_threads(&pr.issue.id, None)
841 .await
842 .unwrap()
843 .is_empty()
844 );
845 }
846
847 #[tokio::test]
848 async fn stale_head_is_rejected() {
849 let fx = Fixture::new(Visibility::Public).await;
850 owner_session(&fx).await;
851 let number = open_pull(&fx).await;
852 let pr = pull(&fx, number).await;
853 let zero = "0".repeat(40);
854 let form = csrf_form(
855 "alice",
856 &[
857 ("file_index", "0"),
858 ("side", "new"),
859 ("line", "2"),
860 ("head_oid", zero.as_str()),
861 ("action", "single"),
862 ("body", "Stale"),
863 ],
864 );
865 let (status, location, _) = send(
866 &fx.router,
867 "POST",
868 &format!("/alice/demo/pulls/{number}/review_comments"),
869 Some("alice"),
870 Some(form),
871 )
872 .await;
873 assert_eq!(status, 303);
874 assert_eq!(
875 location.as_deref(),
876 Some(format!("/alice/demo/pulls/{number}/files?review_error=stale").as_str())
877 );
878 assert!(
879 fx.store
880 .review_threads(&pr.issue.id, None)
881 .await
882 .unwrap()
883 .is_empty()
884 );
885 }
886
887 #[tokio::test]
888 async fn start_review_keeps_the_comment_pending_until_submitted() {
889 let fx = Fixture::new(Visibility::Public).await;
890 owner_session(&fx).await;
891 let number = open_pull(&fx).await;
892 let pr = pull(&fx, number).await;
893 let alice = fx.store.user_by_username(OWNER).await.unwrap().unwrap();
894 let head = pr.head_oid.clone();
895 let form = csrf_form(
896 "alice",
897 &[
898 ("file_index", "0"),
899 ("side", "new"),
900 ("line", "2"),
901 ("head_oid", head.as_str()),
902 ("action", "review"),
903 ("body", "Draft"),
904 ],
905 );
906 let (status, _, _) = send(
907 &fx.router,
908 "POST",
909 &format!("/alice/demo/pulls/{number}/review_comments"),
910 Some("alice"),
911 Some(form),
912 )
913 .await;
914 assert_eq!(status, 303);
915 // Hidden from everyone but the author while pending.
916 assert!(
917 fx.store
918 .review_threads(&pr.issue.id, None)
919 .await
920 .unwrap()
921 .is_empty()
922 );
923 let mine = fx
924 .store
925 .review_threads(&pr.issue.id, Some(&alice.id))
926 .await
927 .unwrap();
928 assert!(mine[0].root.pending);
929
930 let form = csrf_form("alice", &[("state", "comment"), ("body", "Done")]);
931 let (status, _, _) = send(
932 &fx.router,
933 "POST",
934 &format!("/alice/demo/pulls/{number}/reviews"),
935 Some("alice"),
936 Some(form),
937 )
938 .await;
939 assert_eq!(status, 303);
940 let threads = fx.store.review_threads(&pr.issue.id, None).await.unwrap();
941 assert_eq!(threads.len(), 1);
942 assert!(!threads[0].root.pending);
943 }
944
945 #[tokio::test]
946 async fn reader_approval_is_recorded_and_author_approval_is_refused() {
947 let fx = Fixture::new(Visibility::Public).await;
948 owner_session(&fx).await;
949 sign_in(&fx, "bob", "bob").await;
950 let number = open_pull(&fx).await;
951 let pr = pull(&fx, number).await;
952 let uri = format!("/alice/demo/pulls/{number}/reviews");
953
954 let form = csrf_form("bob", &[("state", "approve")]);
955 let (status, location, _) = send(&fx.router, "POST", &uri, Some("bob"), Some(form)).await;
956 assert_eq!(status, 303);
957 assert!(location.unwrap().contains("#review-"));
958 let reviews = fx.store.list_reviews(&pr.issue.id).await.unwrap();
959 assert_eq!(reviews.len(), 1);
960 assert_eq!(reviews[0].state, ReviewState::Approved);
961
962 let form = csrf_form("alice", &[("state", "approve")]);
963 let (status, location, _) = send(&fx.router, "POST", &uri, Some("alice"), Some(form)).await;
964 assert_eq!(status, 303);
965 assert_eq!(
966 location.as_deref(),
967 Some(format!("/alice/demo/pulls/{number}?review_error=own_pr").as_str())
968 );
969 assert_eq!(fx.store.list_reviews(&pr.issue.id).await.unwrap().len(), 1);
970 }
971
972 #[tokio::test]
973 async fn reply_joins_the_thread_and_redirects_to_the_root() {
974 let fx = Fixture::new(Visibility::Public).await;
975 owner_session(&fx).await;
976 sign_in(&fx, "bob", "bob").await;
977 let number = open_pull(&fx).await;
978 comment_on(&fx, number, "alice", "2", "Root").await;
979 let root = root_id(&fx, number).await;
980 let pr = pull(&fx, number).await;
981
982 let form = csrf_form("bob", &[("body", "Reply")]);
983 let (status, location, _) = send(
984 &fx.router,
985 "POST",
986 &format!("/alice/demo/pulls/{number}/review_comments/{root}/reply"),
987 Some("bob"),
988 Some(form),
989 )
990 .await;
991 assert_eq!(status, 303);
992 assert_eq!(
993 location.as_deref(),
994 Some(format!("/alice/demo/pulls/{number}#rc-{root}").as_str())
995 );
996 let threads = fx.store.review_threads(&pr.issue.id, None).await.unwrap();
997 assert_eq!(threads[0].replies.len(), 1);
998 assert_eq!(threads[0].replies[0].author_name, "bob");
999 }
1000
1001 #[tokio::test]
1002 async fn edit_is_for_the_author_only() {
1003 let fx = Fixture::new(Visibility::Public).await;
1004 owner_session(&fx).await;
1005 sign_in(&fx, "bob", "bob").await;
1006 let number = open_pull(&fx).await;
1007 comment_on(&fx, number, "alice", "2", "First").await;
1008 let root = root_id(&fx, number).await;
1009 let edit = format!("/alice/demo/pulls/{number}/review_comments/{root}/edit");
1010
1011 let (status, _, body) = send(&fx.router, "GET", &edit, Some("alice"), None).await;
1012 assert_eq!(status, 200);
1013 assert!(body.contains("Save comment"));
1014 let (status, _, _) = send(&fx.router, "GET", &edit, Some("bob"), None).await;
1015 assert_eq!(status, 403);
1016
1017 let form = csrf_form("bob", &[("body", "Hijacked")]);
1018 let (status, _, _) = send(&fx.router, "POST", &edit, Some("bob"), Some(form)).await;
1019 assert_eq!(status, 403);
1020
1021 let form = csrf_form("alice", &[("body", "Edited")]);
1022 let (status, location, _) =
1023 send(&fx.router, "POST", &edit, Some("alice"), Some(form)).await;
1024 assert_eq!(status, 303);
1025 assert_eq!(
1026 location.as_deref(),
1027 Some(format!("/alice/demo/pulls/{number}#rc-{root}").as_str())
1028 );
1029 let pr = pull(&fx, number).await;
1030 let threads = fx.store.review_threads(&pr.issue.id, None).await.unwrap();
1031 assert_eq!(threads[0].root.body, "Edited");
1032 }
1033
1034 #[tokio::test]
1035 async fn delete_removes_the_thread_for_its_author_and_refuses_others() {
1036 let fx = Fixture::new(Visibility::Public).await;
1037 owner_session(&fx).await;
1038 sign_in(&fx, "bob", "bob").await;
1039 let number = open_pull(&fx).await;
1040 comment_on(&fx, number, "alice", "2", "Doomed").await;
1041 let root = root_id(&fx, number).await;
1042 let pr = pull(&fx, number).await;
1043 let uri = format!("/alice/demo/pulls/{number}/review_comments/{root}/delete");
1044
1045 let form = csrf_form("bob", &[]);
1046 let (status, _, _) = send(&fx.router, "POST", &uri, Some("bob"), Some(form)).await;
1047 assert_eq!(status, 403);
1048
1049 let form = csrf_form("alice", &[]);
1050 let (status, _, _) = send(&fx.router, "POST", &uri, Some("alice"), Some(form)).await;
1051 assert_eq!(status, 303);
1052 assert!(
1053 fx.store
1054 .review_threads(&pr.issue.id, None)
1055 .await
1056 .unwrap()
1057 .is_empty()
1058 );
1059 }
1060
1061 #[tokio::test]
1062 async fn resolve_is_for_the_pr_author_or_write() {
1063 let fx = Fixture::new(Visibility::Public).await;
1064 owner_session(&fx).await;
1065 sign_in(&fx, "bob", "bob").await;
1066 let number = open_pull(&fx).await;
1067 comment_on(&fx, number, "alice", "2", "Thread").await;
1068 let root = root_id(&fx, number).await;
1069 let pr = pull(&fx, number).await;
1070 let uri = format!("/alice/demo/pulls/{number}/review_comments/{root}/resolve");
1071
1072 let form = csrf_form("bob", &[("resolved", "1")]);
1073 let (status, _, _) = send(&fx.router, "POST", &uri, Some("bob"), Some(form)).await;
1074 assert_eq!(status, 403);
1075
1076 let form = csrf_form("alice", &[("resolved", "1")]);
1077 let (status, _, _) = send(&fx.router, "POST", &uri, Some("alice"), Some(form)).await;
1078 assert_eq!(status, 303);
1079 let threads = fx.store.review_threads(&pr.issue.id, None).await.unwrap();
1080 assert!(threads[0].root.resolved);
1081 }
1082
1083 #[tokio::test]
1084 async fn dismiss_needs_write_and_a_reason() {
1085 let fx = Fixture::new(Visibility::Public).await;
1086 owner_session(&fx).await;
1087 sign_in(&fx, "bob", "bob").await;
1088 let number = open_pull(&fx).await;
1089 let pr = pull(&fx, number).await;
1090 let form = csrf_form("bob", &[("state", "request_changes")]);
1091 send(
1092 &fx.router,
1093 "POST",
1094 &format!("/alice/demo/pulls/{number}/reviews"),
1095 Some("bob"),
1096 Some(form),
1097 )
1098 .await;
1099 let review = fx.store.list_reviews(&pr.issue.id).await.unwrap().remove(0);
1100 let uri = format!("/alice/demo/pulls/{number}/reviews/{}/dismiss", review.id);
1101
1102 let form = csrf_form("bob", &[("reason", "mine")]);
1103 let (status, _, _) = send(&fx.router, "POST", &uri, Some("bob"), Some(form)).await;
1104 assert_eq!(status, 403);
1105
1106 let form = csrf_form("alice", &[("reason", " ")]);
1107 let (status, location, _) = send(&fx.router, "POST", &uri, Some("alice"), Some(form)).await;
1108 assert_eq!(status, 303);
1109 assert!(location.unwrap().ends_with("?review_error=empty"));
1110
1111 let form = csrf_form("alice", &[("reason", "Addressed")]);
1112 let (status, _, _) = send(&fx.router, "POST", &uri, Some("alice"), Some(form)).await;
1113 assert_eq!(status, 303);
1114 let review = fx.store.list_reviews(&pr.issue.id).await.unwrap().remove(0);
1115 assert_eq!(review.state, ReviewState::Dismissed);
1116 }
1117
1118 #[tokio::test]
1119 async fn closed_pull_takes_comments_but_not_reviews() {
1120 let fx = Fixture::new(Visibility::Public).await;
1121 owner_session(&fx).await;
1122 let number = open_pull(&fx).await;
1123 let pr = pull(&fx, number).await;
1124 let alice = fx.store.user_by_username(OWNER).await.unwrap().unwrap();
1125 fx.store
1126 .set_issue_state(
1127 &pr.issue.id,
1128 Some(&alice.id),
1129 IssueState::Closed,
1130 None,
1131 serde_json::Value::Null,
1132 )
1133 .await
1134 .unwrap();
1135
1136 let (status, location) = comment_on(&fx, number, "alice", "2", "After close").await;
1137 assert_eq!(status, 303);
1138 assert!(location.unwrap().contains("#L-0-new-2"));
1139
1140 let form = csrf_form("alice", &[("state", "approve")]);
1141 let (status, location, _) = send(
1142 &fx.router,
1143 "POST",
1144 &format!("/alice/demo/pulls/{number}/reviews"),
1145 Some("alice"),
1146 Some(form),
1147 )
1148 .await;
1149 assert_eq!(status, 303);
1150 assert!(location.unwrap().ends_with("?review_error=closed"));
1151 }
1152
1153 #[tokio::test]
1154 async fn anonymous_writes_redirect_to_login_and_missing_csrf_is_forbidden() {
1155 let fx = Fixture::new(Visibility::Public).await;
1156 owner_session(&fx).await;
1157 let number = open_pull(&fx).await;
1158 let head = head_of(&fx, number).await;
1159 let uri = format!("/alice/demo/pulls/{number}/review_comments");
1160 let form = csrf_form(
1161 "alice",
1162 &[
1163 ("file_index", "0"),
1164 ("side", "new"),
1165 ("line", "2"),
1166 ("head_oid", head.as_str()),
1167 ("action", "single"),
1168 ("body", "Anon"),
1169 ],
1170 );
1171 let (status, location, _) = send(&fx.router, "POST", &uri, None, Some(form)).await;
1172 assert_eq!(status, 303);
1173 assert!(location.unwrap().starts_with("/login"));
1174
1175 let form =
1176 format!("file_index=0&side=new&line=2&head_oid={head}&action=single&body=NoCsrf");
1177 let (status, _, _) = send(&fx.router, "POST", &uri, Some("alice"), Some(form)).await;
1178 assert_eq!(status, 403);
1179 }
1180}