feat: configure personal pull request review indicators

This commit is contained in:
Valdemar Lorenzen
2026-10-21 11:14:50 +02:00
parent bd71d463a4
commit 0202644efa
18 changed files with 901 additions and 88 deletions
+309 -1
View File
@@ -174,7 +174,9 @@ fn unavailable_viewer_or_metadata_never_fabricates_personal_approval() {
let forge = parse(url, None).unwrap();
for unavailable_viewer in [true, false] {
let metadata = forge.metadata_with(
&[json!({"number":7,"iid":7,"additions":0,"deletions":0})],
&[json!({"number":7,"iid":7,"additions":0,"deletions":0,
"requested_reviewers":[{"login":"viewer","id":42}],
"reviewers":[{"username":"viewer"}]})],
&mut |url, body| {
if url.ends_with("/user") {
return if unavailable_viewer {
@@ -194,6 +196,8 @@ fn unavailable_viewer_or_metadata_never_fabricates_personal_approval() {
},
);
assert!(metadata.values().all(|details| !details.approved));
assert!(metadata.values().all(|details| details.review_requested
== (provider == Provider::Github && !unavailable_viewer)));
if provider == Provider::Gitea {
assert_eq!(metadata[&7].size, Some((0, 0)));
}
@@ -201,6 +205,310 @@ fn unavailable_viewer_or_metadata_never_fabricates_personal_approval() {
}
}
#[test]
fn github_outstanding_requests_use_list_users_independently_of_review_metadata() {
let forge = parse("https://github.com/owner/repo", None).unwrap();
for (reviewers, expected) in [
(json!([{"login":"VIEWER"}]), true), // Direct or re-requested after approval.
(json!([]), false), // Completed or removed requests are absent.
(json!([{"login":"another-user"}]), false),
(Value::Null, false),
] {
for denied in [false, true] {
let metadata = forge.metadata_with(
&[json!({"number":7,"requested_reviewers":reviewers,
"requested_teams":[{"slug":"viewer"}],"additions":3,"deletions":4})],
&mut |url, body| {
if url.ends_with("/user") {
return Ok(json!({"login":"viewer"}));
}
assert!(body.is_some());
if denied {
return Err("GraphQL denied".into());
}
Ok(
json!({"data":{"repository":{"p7":{"additions":3,"deletions":4,
"reviews":{"nodes":[{"state":"APPROVED","author":{"login":"viewer"}}]}}}}}),
)
},
);
assert_eq!(metadata[&7].review_requested, expected);
assert_eq!(metadata[&7].approved, !denied);
assert_eq!(metadata[&7].size, Some((3, 4)));
}
}
}
#[test]
fn gitlab_outstanding_requests_require_personal_unfinished_review_and_follow_pages() {
let forge = parse("https://gitlab.com/group/repo", None).unwrap();
let mut values: Vec<_> = (1..=10)
.map(|id| json!({"iid":id,"reviewers":[{"username":"viewer"}]}))
.collect();
values[8]["reviewers"] = json!([]); // Removed reviewer.
values[9]["reviewers"] = json!([{"username":"another-user"}]);
let mut request_reads = 0;
let metadata = forge.metadata_with(&values, &mut |url, body| {
if url.ends_with("/user") {
return Ok(json!({"username":"viewer"}));
}
let body: Value = serde_json::from_str(body.unwrap()).unwrap();
let query = body["query"].as_str().unwrap();
assert!(!query.contains("mutation"));
if !query.contains("reviewState") {
return Ok(json!({"data":{"project":{
"p1":{"diffStatsSummary":{"additions":5,"deletions":6}},
"approved":{"nodes":[{"iid":"1"},{"iid":"4"}],"pageInfo":{"hasNextPage":false}}
}}}));
}
request_reads += 1;
assert!(query.contains("reviewers(first:20"));
assert!(!query.contains("p9:") && !query.contains("p10:"));
if query.contains("after:\"next\"") {
return Ok(json!({"data":{"project":{"p2":{"reviewers":{
"nodes":[{"username":"viewer","mergeRequestInteraction":{"reviewState":"REVIEW_STARTED"}}],
"pageInfo":{"hasNextPage":false}
}}}}}));
}
let mut entries = serde_json::Map::new();
for (id, state) in [(1, "UNREVIEWED"), (3, "REVIEWED"), (4, "APPROVED"),
(5, "REQUESTED_CHANGES"), (6, "UNAPPROVED"), (7, ""), (8, "UNKNOWN")] {
entries.insert(format!("p{id}"), json!({"reviewers":{
"nodes":[{"username":"viewer","mergeRequestInteraction":{"reviewState":state}}],
"pageInfo":{"hasNextPage":false}
}}));
}
entries.insert("p2".into(), json!({"reviewers":{
"nodes":[{"username":"another-user","mergeRequestInteraction":{"reviewState":"UNREVIEWED"}}],
"pageInfo":{"hasNextPage":true,"endCursor":"next"}
}}));
Ok(json!({"data":{"project":entries}}))
});
assert_eq!(request_reads, 2);
assert_eq!(
metadata
.iter()
.filter_map(|(id, details)| details.review_requested.then_some(*id))
.collect::<Vec<_>>(),
[1, 2]
);
assert!(metadata[&1].approved && metadata[&4].approved);
assert_eq!(metadata[&1].size, Some((5, 6)));
}
#[test]
fn gitlab_request_failures_preserve_approval_and_statistics() {
let forge = parse("https://gitlab.com/group/repo", None).unwrap();
for denied in [
Err("review states denied".into()),
Ok(json!({"data":{"project":{
"p7":{"reviewers":null}
}}})),
Ok(json!({"data":{"project":{"p7":{"reviewers":{"nodes":[
{"username":"viewer","mergeRequestInteraction":{"reviewState":null}}
]}}}}})),
] {
let metadata = forge.metadata_with(
&[json!({"iid":7,"reviewers":[{"username":"viewer"}]})],
&mut |url, body| {
if url.ends_with("/user") {
return Ok(json!({"username":"viewer"}));
}
let body: Value = serde_json::from_str(body.unwrap()).unwrap();
if body["query"].as_str().unwrap().contains("reviewState") {
return denied.clone();
}
Ok(json!({"data":{"project":{
"p7":{"diffStatsSummary":{"additions":0,"deletions":1}},
"approved":{"nodes":[{"iid":"7"}],"pageInfo":{"hasNextPage":false}}
}}}))
},
);
assert!(!metadata[&7].review_requested);
assert!(metadata[&7].approved);
assert_eq!(metadata[&7].size, Some((0, 1)));
}
}
#[test]
fn gitlab_request_batches_cover_fifty_rows_without_exceeding_complexity_limit() {
let forge = parse("https://gitlab.com/group/repo", None).unwrap();
let values: Vec<_> = (1..=50)
.map(|id| {
json!({
"iid":id,"reviewers":[{"username":"viewer"}]
})
})
.collect();
let mut batches = 0;
let metadata = forge.metadata_with(&values, &mut |url, body| {
if url.ends_with("/user") {
return Ok(json!({"username":"viewer"}));
}
let body: Value = serde_json::from_str(body.unwrap()).unwrap();
let query = body["query"].as_str().unwrap();
if !query.contains("reviewState") {
return Err("independent metadata unavailable".into());
}
batches += 1;
assert_eq!(query.matches("reviewers(first:20)").count(), 10);
let entries: serde_json::Map<_, _> = (1..=50)
.filter(|id| query.contains(&format!("p{id}:mergeRequest")))
.map(|id| (format!("p{id}"), json!({"reviewers":{
"nodes":[{"username":"viewer","mergeRequestInteraction":{"reviewState":"UNREVIEWED"}}],
"pageInfo":{"hasNextPage":false}
}}))).collect();
Ok(json!({"data":{"project":entries}}))
});
assert_eq!(batches, 5);
assert_eq!(metadata.len(), 50);
assert!(metadata.values().all(|details| details.review_requested));
}
#[test]
fn gitea_requests_ignore_list_assignments_and_use_current_direct_review_rows() {
let forge = parse("https://codeberg.org/owner/repo", None).unwrap();
let own = |id, state: &str| {
json!({
"id":id,"user":{"id":42},"team":null,"state":state,"dismissed":false
})
};
for (reviews, requested, approved) in [
(json!([own(10, "REQUEST_REVIEW")]), true, false),
(json!([own(10, "APPROVED")]), false, true), // Completed or removed request row.
(json!([own(10, "COMMENT")]), false, false),
(json!([own(10, "REQUEST_CHANGES")]), false, false),
(
json!([own(10, "APPROVED"), own(20, "REQUEST_REVIEW")]),
true,
true,
),
(
json!([
own(20, "REQUEST_REVIEW"),
own(10, "APPROVED"),
own(30, "PENDING")
]),
true,
true,
),
(
json!([own(10, "REQUEST_REVIEW"), own(20, "COMMENT")]),
false,
false,
),
(
json!([own(10, "REQUEST_REVIEW"), own(20, "APPROVED")]),
false,
true,
),
(
json!([{"id":10,"user":{"id":7},"state":"REQUEST_REVIEW","dismissed":false}]),
false,
false,
),
(
json!([{"id":10,"user":{"id":42},"team":{"id":1},"state":"REQUEST_REVIEW","dismissed":false}]),
false,
false,
),
(
json!([{"id":20,"user":{"id":42},"state":"REQUEST_REVIEW"},
own(10, "APPROVED")]),
false,
true,
),
(json!([]), false, false),
] {
let mut pages = vec![reviews, json!([])].into_iter();
let metadata = forge.metadata_with(
&[json!({"number":7,"additions":1,"deletions":2,"requested_reviewers":[{"id":42}]})],
&mut |url, body| {
assert!(body.is_none());
if url.ends_with("/user") {
return Ok(json!({"id":42}));
}
pages.next().ok_or_else(|| "unexpected page".into())
},
);
assert_eq!(metadata[&7].review_requested, requested);
assert_eq!(metadata[&7].approved, approved);
assert_eq!(metadata[&7].size, Some((1, 2)));
}
}
#[test]
fn gitea_incomplete_request_pages_do_not_fabricate_requests_or_lose_list_statistics() {
let forge = parse("https://codeberg.org/owner/repo", None).unwrap();
let metadata = forge.metadata_with(
&[json!({"number":7,"additions":0,"deletions":2})],
&mut |url, _| {
if url.ends_with("/user") {
return Ok(json!({"id":42}));
}
if url.ends_with("page=1") {
return Ok(json!([{"id":10,"user":{"id":42},"team":null,
"state":"REQUEST_REVIEW","dismissed":false}]));
}
Err("remaining reviews denied".into())
},
);
assert!(!metadata[&7].review_requested);
assert_eq!(metadata[&7].size, Some((0, 2)));
}
#[test]
fn summaries_keep_outstanding_requests_and_approvals_independent() {
for (url, provider) in [
("https://github.com/owner/repo", Provider::Github),
("https://gitlab.com/owner/repo", Provider::Gitlab),
("https://codeberg.org/owner/repo", Provider::Gitea),
] {
let forge = parse(url, None).unwrap();
let page = forge.page_with(false, 1, |url, body| {
if url.ends_with("/user") {
return Ok(json!({"id":42,"login":"viewer","username":"viewer"}));
}
if let Some(body) = body {
let body: Value = serde_json::from_str(body).unwrap();
if body["query"].as_str().unwrap().contains("reviewState") {
return Ok(json!({"data":{"project":{"p7":{"reviewers":{
"nodes":[{"username":"viewer","mergeRequestInteraction":{"reviewState":"UNREVIEWED"}}],
"pageInfo":{"hasNextPage":false}
}}}}}));
}
return if provider == Provider::Github {
Ok(json!({"data":{"repository":{"p7":{"additions":1,"deletions":2,
"reviews":{"nodes":[{"state":"APPROVED","author":{"login":"viewer"}}]}}}}}))
} else {
Ok(json!({"data":{"project":{
"p7":{"diffStatsSummary":{"additions":1,"deletions":2}},
"approved":{"nodes":[{"iid":"7"}],"pageInfo":{"hasNextPage":false}}
}}}))
};
}
if url.contains("/7/reviews?") {
return Ok(if url.ends_with("page=1") {
json!([
{"id":10,"user":{"id":42},"state":"APPROVED","dismissed":false},
{"id":20,"user":{"id":42},"state":"REQUEST_REVIEW","dismissed":false}
])
} else {
json!([])
});
}
Ok(json!([{"number":7,"iid":7,"title":"Request again","created_at":"2024-01-01T00:00:00Z",
"state":if provider == Provider::Gitlab { "opened" } else { "open" },
"merged":false,"merged_at":null,"user":{"login":"author"},"author":{"username":"author"},
"requested_reviewers":[{"login":"viewer","id":42}],"reviewers":[{"username":"viewer"}],
"additions":1,"deletions":2}]))
}).unwrap();
assert_eq!(page.entries.len(), 1);
assert!(page.entries[0].approved && page.entries[0].review_requested);
assert!(page.entries[0].description.contains("+1 -2"));
}
}
#[test]
fn draft_summaries_keep_open_closed_and_merged_states_distinct() {
for provider in [Provider::Github, Provider::Gitlab, Provider::Gitea] {