forge
ExploreLog in

Tahinli/forge

public
0 watching
★ 0
22.5 KiB
1//! The Files changed tab of a pull request, `/{owner}/{repo}/pulls/{n}/files`. Each diff line has
2//! an anchor and a "+" link (`?comment=<file>:<old|new>:<line>`), which opens an inline comment
3//! form under that line. Existing review threads render under their lines. Signed-in viewers see
4//! the review bar at the top. The header and tabs come from `pull_view`. The review POST routes
5//! are in `pull_reviews`.
6
7use std::collections::HashMap;
8
9use forge_core::{Action, IssueState, Side, Thread, authorize};
10use forge_git::history::CommitDiff;
11use topcoat::Result;
12use topcoat::context::{Cx, app_context};
13use topcoat::router::error::not_found;
14use topcoat::router::request::uri;
15use topcoat::router::{page, path_param};
16use topcoat::view::{View, component, view};
17
18use crate::i18n::pulls::{self as g, K};
19use crate::i18n::{self, Lang};
20use crate::pages::edit_history::thread_edit_counts;
21use crate::pages::issue_parts::{csrf_of, may_converse, parse_number, triage, viewer};
22use crate::pages::pull_reviews::{error_text_in, pull_diff};
23use crate::pages::pull_view::{commits_of, frame_in, is_merged, pull_header, pull_subnav_in};
24use crate::pages::repo::repo_head;
25use crate::pages::repo_ctx::load_repo;
26use crate::pages::tree::link_context;
27use crate::state::AppState;
28use crate::views::diff::{DiffLang, LineComments, diff_section};
29use crate::views::review_thread::ThreadCtx;
30use crate::views::{alert, shell};
31
32path_param!(owner);
33path_param!(repo);
34path_param!(number);
35
36/// Parses a non-empty run of ASCII digits. `str::parse` alone would also accept a leading `+`.
37fn digits<T: std::str::FromStr>(raw: &str) -> Option<T> {
38 if raw.is_empty() || !raw.bytes().all(|b| b.is_ascii_digit()) {
39 return None;
40 }
41 raw.parse().ok()
42}
43
44/// The `comment=<file>:<old|new>:<line>` target of an inline form. Anything else is ignored.
45fn comment_target(cx: &Cx) -> Option<(usize, Side, u32)> {
46 let request = uri(cx);
47 let query = request.query()?;
48 let (_, raw) = form_urlencoded::parse(query.as_bytes()).find(|(key, _)| *key == "comment")?;
49 let mut parts = raw.split(':');
50 let index = digits::<usize>(parts.next()?)?;
51 let side = parts.next()?.parse::<Side>().ok()?;
52 let line = digits::<u32>(parts.next()?)?;
53 parts.next().is_none().then_some((index, side, line))
54}
55
56/// The message for the `review_error` query value of the request, when it is a known code. The
57/// codes and their messages are shared with the review routes (`pull_reviews::error_text`).
58fn review_error_from(lang: Lang, cx: &Cx) -> Option<String> {
59 let request = uri(cx);
60 let query = request.query()?;
61 let (_, code) =
62 form_urlencoded::parse(query.as_bytes()).find(|(key, _)| *key == "review_error")?;
63 error_text_in(lang, &code).map(str::to_owned)
64}
65
66/// Index of the diff file a thread's path names, matched on either side of a rename.
67fn file_index(diff: &CommitDiff, path: &str) -> Option<usize> {
68 diff.files
69 .iter()
70 .position(|f| f.new_path.as_deref() == Some(path) || f.old_path.as_deref() == Some(path))
71}
72
73/// The review bar: the viewer's pending comment count, a summary, the review type and Submit,
74/// plus Discard. The PR author can only comment, so approve and request changes are left out.
75#[component]
76async fn review_bar(
77 lang: Lang,
78 pr_href: String,
79 csrf: String,
80 pending: usize,
81 is_author: bool,
82) -> Result<impl View> {
83 let reviews_action = format!("{pr_href}/reviews");
84 let discard_action = format!("{pr_href}/reviews/discard");
85 let pending_text = g::pending_comments(lang, pending);
86 Ok(view! {
87 <section class="card review-bar">
88 <h2>(g::t(lang, K::FinishReview))</h2>
89 <p class="muted">(pending_text.as_str())</p>
90 <form id="review-form" method="post" action=(reviews_action.as_str()) class="stack">
91 <input type="hidden" name="csrf" value=(csrf.as_str())>
92 <label>
93 <span>(g::t(lang, K::ReviewSummary))</span>
94 <textarea name="body" rows="4"></textarea>
95 </label>
96 <fieldset class="review-choices">
97 <legend>(g::t(lang, K::ReviewType))</legend>
98 <label><input type="radio" name="state" value="comment" checked=""> (g::t(lang, K::CommentChoice))</label>
99 if !is_author {
100 <label><input type="radio" name="state" value="approve"> (g::t(lang, K::ApproveChoice))</label>
101 <label><input type="radio" name="state" value="request_changes"> (g::t(lang, K::RequestChangesChoice))</label>
102 }
103 </fieldset>
104 </form>
105 <div class="review-actions">
106 <button type="submit" class="primary" form="review-form">(g::t(lang, K::SubmitReview))</button>
107 <form method="post" action=(discard_action.as_str()) class="review-inline">
108 <input type="hidden" name="csrf" value=(csrf.as_str())>
109 <button type="submit" class="link-button">(g::t(lang, K::DiscardPending))</button>
110 </form>
111 </div>
112 </section>
113 })
114}
115
116#[page("/{owner}/{repo}/pulls/{number}/files")]
117async fn pull_files(cx: &Cx) -> Result<impl View> {
118 let number = parse_number(path_param::<Number>(cx))?;
119 let ctx = load_repo(cx, path_param::<Owner>(cx), path_param::<Repo>(cx)).await?;
120 let state = app_context::<AppState>(cx);
121 let pr = match state.store.pull_by_number(&ctx.repo.id, number).await? {
122 Some(pr) => pr,
123 None => return Err(not_found().into()),
124 };
125
126 let lang = i18n::lang(cx, viewer(&ctx));
127 let commit_count = commits_of(state, &ctx, &pr).await?.len();
128 let diff = pull_diff(state, &ctx, &pr).await?;
129 let frame = frame_in(lang, state, &ctx, &pr, commit_count, diff.files.len()).await?;
130
131 let viewer_id = viewer(&ctx).map(|user| user.id.clone());
132 let signed_in = viewer_id.is_some();
133 let archived = ctx.repo.archived;
134 let open = matches!(pr.issue.state, IssueState::Open) && !is_merged(&pr);
135 let is_author = viewer_id.as_deref() == Some(pr.issue.author_id.as_str());
136 let can_comment = signed_in && !archived && may_converse(&ctx, &pr.issue);
137 let can_write = !archived
138 && authorize(
139 viewer(&ctx),
140 &ctx.repo,
141 ctx.role,
142 ctx.org_role,
143 Action::Write,
144 )
145 .is_ok();
146 // Comments and replies stay open on a closed pull request; new reviews do not.
147 let can_review = can_comment && open;
148 let href = frame.href.clone();
149 let csrf = csrf_of(&ctx);
150
151 // The viewer's own pending comments are included by `review_threads`. Outdated threads are
152 // left out of the Files tab; they still show in the Conversation.
153 let threads = state
154 .store
155 .review_threads(&pr.issue.id, viewer_id.as_deref())
156 .await?;
157 let pending = threads.iter().filter(|t| t.root.pending).count();
158 let edits = thread_edit_counts(state, &threads).await?;
159 let mut by_line: HashMap<(usize, Side, u32), Vec<Thread>> = HashMap::new();
160 for thread in threads.into_iter().filter(|t| !t.root.outdated) {
161 if let Some(index) = file_index(&diff, &thread.root.path) {
162 by_line
163 .entry((index, thread.root.side, thread.root.line))
164 .or_default()
165 .push(thread);
166 }
167 }
168
169 let link = link_context(state, &ctx.repo, &ctx.repo.default_branch, "");
170 let thread_ctx = ThreadCtx {
171 pr_href: href.clone(),
172 csrf: csrf.clone(),
173 viewer_id: viewer_id.clone(),
174 can_comment,
175 can_resolve: can_comment && (is_author || can_write),
176 can_moderate: can_comment && triage(&ctx),
177 can_review,
178 tz: ctx.viewer_tz().to_owned(),
179 link: Some(link),
180 edits,
181 };
182 let comments = LineComments {
183 anchors: can_comment,
184 threads: by_line,
185 open_form: comment_target(cx).filter(|_| can_comment),
186 files_href: format!("{href}/files"),
187 has_pending: pending > 0,
188 ctx: thread_ctx,
189 };
190
191 let error = review_error_from(lang, cx);
192 let owner = ctx.repo.owner_name.clone();
193 let name = ctx.repo.name.clone();
194 let parent = pr.merge_base.clone();
195 let page_title = frame.page_title.clone();
196 let header = ctx.head(state, "pulls").await?;
197 Ok(view! {
198 shell(title: page_title.as_str(), viewer: ctx.viewer.as_ref(),
199 repo_head(head: header)
200 pull_header(frame: frame.clone())
201 pull_subnav_in(lang: lang, href: href.clone(), active: "files".to_owned(),
202 commits: frame.commit_count, files: frame.file_count)
203 if let Some(message) = error {
204 alert(message: message.as_str())
205 }
206 if can_review {
207 review_bar(lang: lang, pr_href: href.clone(), csrf: csrf.clone(), pending: pending, is_author: is_author)
208 }
209 diff_section(diff: &diff, owner: owner.as_str(), repo: name.as_str(),
210 oid: pr.head_oid.as_str(), parent: parent.as_deref(),
211 subject: g::t(lang, K::ThisPullRequest), lang: DiffLang(lang), comments: comments)
212 )
213 })
214}
215
216#[cfg(test)]
217mod tests {
218 use forge_core::{Side, User, Visibility};
219 use forge_git::GitEnv;
220 use topcoat::router::request::Request;
221 use topcoat::router::{Body, Router, to_bytes};
222
223 use crate::auth::SESSION_COOKIE;
224 use crate::pages::commits::test_support::{Fixture, OWNER, REPO, git};
225
226 fn cookie(token: &str) -> String {
227 format!("__Host-{SESSION_COOKIE}={token}")
228 }
229
230 /// The page's own `<main>` element. The shell's header echoes the request URL (theme
231 /// form `back` field), so the body checks are scoped to `<main>`.
232 fn page_main(body: &str) -> &str {
233 body.split_once("<main tabindex=\"-1\">")
234 .and_then(|(_, rest)| rest.split_once("</main>"))
235 .map(|(main, _)| main)
236 .expect("page has a <main> element")
237 }
238
239 /// Sends one GET through the router and returns `(status, body)`.
240 async fn get(router: &Router, uri: &str, token: Option<&str>) -> (u16, String) {
241 let mut req = Request::builder().method("GET").uri(uri);
242 if let Some(token) = token {
243 req = req.header("cookie", cookie(token));
244 }
245 let res = router.handle(req.body(Body::empty()).unwrap()).await;
246 let status = res.status().as_u16();
247 let bytes = to_bytes(res.into_body(), usize::MAX).await.unwrap();
248 (status, String::from_utf8_lossy(&bytes).into_owned())
249 }
250
251 /// A user with a session whose CSRF token is `{token}-csrf`.
252 async fn sign_in(fx: &Fixture, name: &str, token: &str) -> User {
253 let user = fx
254 .store
255 .create_user(name, None, "not-a-real-hash", false)
256 .await
257 .unwrap();
258 crate::test_support::create_session(&fx.store, &user.id, token, &format!("{token}-csrf"))
259 .await;
260 user
261 }
262
263 /// The repo owner, signed in with token `alice`.
264 async fn owner_session(fx: &Fixture) {
265 let owner = fx.store.user_by_username(OWNER).await.unwrap().unwrap();
266 crate::test_support::create_session(&fx.store, &owner.id, "alice", "alice-csrf").await;
267 }
268
269 /// Pushes a `feature` branch (one commit adding `feat.txt` with the line `feature`) and opens
270 /// a pull request for it against `main`. Returns the pull request number.
271 async fn open_pull(fx: &Fixture) -> i64 {
272 let base_oid = git(&fx.work, &["rev-parse", "HEAD"]);
273 git(&fx.work, &["checkout", "-q", "-b", "feature", &fx.root]);
274 std::fs::write(fx.work.join("feat.txt"), "feature\n").unwrap();
275 git(&fx.work, &["add", "-A"]);
276 git(&fx.work, &["commit", "-q", "-m", "Add feature file"]);
277 let head_oid = git(&fx.work, &["rev-parse", "HEAD"]);
278 git(
279 &fx.work,
280 &[
281 "push",
282 "-q",
283 &fx.bare.to_string_lossy(),
284 "HEAD:refs/heads/feature",
285 ],
286 );
287
288 let repo = fx
289 .store
290 .repo_by_owner_and_name(OWNER, REPO)
291 .await
292 .unwrap()
293 .unwrap();
294 let owner = fx.store.user_by_username(OWNER).await.unwrap().unwrap();
295 let pr = fx
296 .store
297 .create_pull(
298 &repo.id,
299 &owner.id,
300 "Add feature",
301 "Body text",
302 "main",
303 &repo.id,
304 "feature",
305 &head_oid,
306 &base_oid,
307 )
308 .await
309 .unwrap();
310 let env = GitEnv {
311 git_bin: "git".into(),
312 data_dir: fx.work.clone(),
313 };
314 forge_git::pulls::fetch_into_pull_ref(&env, &fx.bare, &fx.bare, &head_oid, pr.issue.number)
315 .await
316 .unwrap();
317 pr.issue.number
318 }
319
320 /// The pull request's issue id and head commit.
321 async fn pull_ids(fx: &Fixture, number: i64) -> (String, String) {
322 let repo = fx
323 .store
324 .repo_by_owner_and_name(OWNER, REPO)
325 .await
326 .unwrap()
327 .unwrap();
328 let pr = fx
329 .store
330 .pull_by_number(&repo.id, number)
331 .await
332 .unwrap()
333 .unwrap();
334 (pr.issue.id.clone(), pr.head_oid.clone())
335 }
336
337 fn files_uri(number: i64, query: &str) -> String {
338 format!("/{OWNER}/{REPO}/pulls/{number}/files{query}")
339 }
340
341 #[tokio::test]
342 async fn turkish_viewer_gets_turkish_review_bar() {
343 let fx = Fixture::new(Visibility::Public).await;
344 owner_session(&fx).await;
345 let number = open_pull(&fx).await;
346
347 let req = Request::builder()
348 .method("GET")
349 .uri(files_uri(number, ""))
350 .header("cookie", cookie("alice"))
351 .header("accept-language", "tr")
352 .body(Body::empty())
353 .unwrap();
354 let res = fx.router.handle(req).await;
355 let bytes = to_bytes(res.into_body(), usize::MAX).await.unwrap();
356 let body = String::from_utf8_lossy(&bytes).into_owned();
357 assert!(body.contains("İncelemeyi tamamla"), "{body}");
358 assert!(body.contains("Bekleyen yorumları sil"), "{body}");
359 }
360
361 #[tokio::test]
362 async fn files_tab_has_line_anchors_and_plus_links_for_signed_in_viewers() {
363 let fx = Fixture::new(Visibility::Public).await;
364 owner_session(&fx).await;
365 let number = open_pull(&fx).await;
366
367 let (status, body) = get(&fx.router, &files_uri(number, ""), Some("alice")).await;
368 assert_eq!(status, 200);
369 assert!(body.contains("id=\"L-0-new-1\""), "{body}");
370 assert!(body.contains("?comment=0:new:1#L-0-new-1"), "{body}");
371 assert!(body.contains("review-bar"), "{body}");
372 }
373
374 #[tokio::test]
375 async fn comment_param_opens_the_form_under_its_line() {
376 let fx = Fixture::new(Visibility::Public).await;
377 owner_session(&fx).await;
378 let number = open_pull(&fx).await;
379 let (_, head) = pull_ids(&fx, number).await;
380
381 let (status, body) = get(
382 &fx.router,
383 &files_uri(number, "?comment=0:new:1"),
384 Some("alice"),
385 )
386 .await;
387 assert_eq!(status, 200);
388 assert!(body.contains("name=\"file_index\" value=\"0\""), "{body}");
389 assert!(body.contains("name=\"side\" value=\"new\""), "{body}");
390 assert!(body.contains("name=\"line\" value=\"1\""), "{body}");
391 assert!(
392 body.contains(&format!("name=\"head_oid\" value=\"{head}\"")),
393 "{body}"
394 );
395 assert!(body.contains("value=\"single\""), "{body}");
396 assert!(body.contains("value=\"review\""), "{body}");
397 let line = body.find("id=\"L-0-new-1\"").unwrap();
398 let form = body.find("name=\"file_index\"").unwrap();
399 assert!(form > line, "the form sits after its line");
400 }
401
402 #[tokio::test]
403 async fn garbage_comment_param_is_ignored() {
404 let fx = Fixture::new(Visibility::Public).await;
405 owner_session(&fx).await;
406 let number = open_pull(&fx).await;
407
408 for query in [
409 "?comment=abc",
410 "?comment=0:bogus:1",
411 "?comment=%2B0:new:1",
412 "?comment=0:new:1:2",
413 "?comment=99:new:1",
414 "?comment=0:new:",
415 "?comment=0:new:-1",
416 ] {
417 let (status, body) = get(&fx.router, &files_uri(number, query), Some("alice")).await;
418 assert_eq!(status, 200, "{query}");
419 assert!(!body.contains("name=\"file_index\""), "{query}");
420 }
421 }
422
423 #[tokio::test]
424 async fn existing_thread_renders_under_its_line() {
425 let fx = Fixture::new(Visibility::Public).await;
426 owner_session(&fx).await;
427 let number = open_pull(&fx).await;
428 let (issue_id, head) = pull_ids(&fx, number).await;
429 let owner = fx.store.user_by_username(OWNER).await.unwrap().unwrap();
430 fx.store
431 .add_review_comment(
432 &issue_id,
433 &owner.id,
434 false,
435 "feat.txt",
436 Side::New,
437 1,
438 &head,
439 "+feature",
440 "Looks good",
441 )
442 .await
443 .unwrap();
444
445 let (status, body) = get(&fx.router, &files_uri(number, ""), Some("alice")).await;
446 assert_eq!(status, 200);
447 assert!(body.contains("Looks good"), "{body}");
448 let line = body.find("id=\"L-0-new-1\"").unwrap();
449 let thread = body.find("class=\"review-thread\"").unwrap();
450 assert!(thread > line, "the thread sits after its line");
451 }
452
453 #[tokio::test]
454 async fn outdated_thread_is_not_shown_on_the_files_tab() {
455 let fx = Fixture::new(Visibility::Public).await;
456 owner_session(&fx).await;
457 let number = open_pull(&fx).await;
458 let (issue_id, head) = pull_ids(&fx, number).await;
459 let owner = fx.store.user_by_username(OWNER).await.unwrap().unwrap();
460 let comment = fx
461 .store
462 .add_review_comment(
463 &issue_id,
464 &owner.id,
465 false,
466 "feat.txt",
467 Side::New,
468 1,
469 &head,
470 "+feature",
471 "Stale remark",
472 )
473 .await
474 .unwrap();
475 fx.store
476 .mark_outdated(&issue_id, std::slice::from_ref(&comment.id))
477 .await
478 .unwrap();
479
480 let (status, body) = get(&fx.router, &files_uri(number, ""), Some("alice")).await;
481 assert_eq!(status, 200);
482 assert!(!body.contains("Stale remark"), "{body}");
483 assert!(!body.contains("class=\"review-thread\""), "{body}");
484 }
485
486 #[tokio::test]
487 async fn pending_comment_is_visible_only_to_its_author() {
488 let fx = Fixture::new(Visibility::Public).await;
489 owner_session(&fx).await;
490 sign_in(&fx, "bob", "bob").await;
491 let number = open_pull(&fx).await;
492 let (issue_id, head) = pull_ids(&fx, number).await;
493 let owner = fx.store.user_by_username(OWNER).await.unwrap().unwrap();
494 fx.store
495 .add_review_comment(
496 &issue_id,
497 &owner.id,
498 true,
499 "feat.txt",
500 Side::New,
501 1,
502 &head,
503 "+feature",
504 "Draft secret",
505 )
506 .await
507 .unwrap();
508
509 let (_, anonymous) = get(&fx.router, &files_uri(number, ""), None).await;
510 assert!(!anonymous.contains("Draft secret"));
511 let (_, bob) = get(&fx.router, &files_uri(number, ""), Some("bob")).await;
512 assert!(!bob.contains("Draft secret"));
513 let (_, author) = get(&fx.router, &files_uri(number, ""), Some("alice")).await;
514 assert!(author.contains("Draft secret"), "{author}");
515 assert!(author.contains("Pending"), "{author}");
516 assert!(author.contains("1 pending comment."), "{author}");
517 }
518
519 #[tokio::test]
520 async fn review_bar_hides_approve_for_the_pr_author() {
521 let fx = Fixture::new(Visibility::Public).await;
522 owner_session(&fx).await;
523 sign_in(&fx, "bob", "bob").await;
524 let number = open_pull(&fx).await;
525
526 let (_, author) = get(&fx.router, &files_uri(number, ""), Some("alice")).await;
527 assert!(author.contains("value=\"comment\""), "{author}");
528 assert!(!author.contains("value=\"approve\""), "{author}");
529 assert!(!author.contains("value=\"request_changes\""), "{author}");
530
531 let (_, other) = get(&fx.router, &files_uri(number, ""), Some("bob")).await;
532 assert!(other.contains("value=\"approve\""), "{other}");
533 assert!(other.contains("value=\"request_changes\""), "{other}");
534 }
535
536 #[tokio::test]
537 async fn anonymous_viewer_sees_no_comment_controls() {
538 let fx = Fixture::new(Visibility::Public).await;
539 let number = open_pull(&fx).await;
540
541 let (status, body) = get(&fx.router, &files_uri(number, "?comment=0:new:1"), None).await;
542 assert_eq!(status, 200);
543 assert!(!page_main(&body).contains("?comment="), "{body}");
544 assert!(!body.contains("id=\"L-"), "{body}");
545 assert!(!body.contains("name=\"file_index\""), "{body}");
546 assert!(!body.contains("review-bar"), "{body}");
547 assert!(!body.contains("class=\"diff-plus\""), "{body}");
548 }
549
550 #[tokio::test]
551 async fn known_review_error_shows_its_message_and_unknown_code_shows_nothing() {
552 let fx = Fixture::new(Visibility::Public).await;
553 owner_session(&fx).await;
554 let number = open_pull(&fx).await;
555
556 let (_, known) = get(
557 &fx.router,
558 &files_uri(number, "?review_error=own_pr"),
559 Some("alice"),
560 )
561 .await;
562 assert!(known.contains("cannot approve"), "{known}");
563
564 let (status, unknown) = get(
565 &fx.router,
566 &files_uri(number, "?review_error=%3Cb%3Ehi%3C%2Fb%3E"),
567 Some("alice"),
568 )
569 .await;
570 assert_eq!(status, 200);
571 assert!(!unknown.contains("role=\"alert\""), "{unknown}");
572 assert!(!unknown.contains("<b>hi</b>"), "{unknown}");
573 }
574
575 #[tokio::test]
576 async fn commit_page_still_renders_the_plain_diff() {
577 let fx = Fixture::new(Visibility::Public).await;
578 let (status, body) = get(
579 &fx.router,
580 &format!("/{OWNER}/{REPO}/commit/{}", fx.modified),
581 None,
582 )
583 .await;
584 assert_eq!(status, 200);
585 assert!(body.contains("class=\"diff-table\""), "{body}");
586 assert!(!body.contains("id=\"L-"), "{body}");
587 assert!(!body.contains("diff-plus"), "{body}");
588 }
589}