hive-forge: hoist a PR row's merge state in list --json
Forgejo's issue-list endpoint answers 'was this merged?' only inside the nested pull_request object, while state says closed for a merged PR and for one closed without merging alike. So the obvious top-level query is null or ambiguous for every row, and with a // default it renders as a confident 'nothing merged' that cannot ever be right -- a wrong answer shaped exactly like a clean one. Copy merged and merged_at up to the top level of each PR row so the obvious query is the correct one. Additive: the nested object is left untouched so an existing consumer keeps working, and issue rows have no pull_request and pass through unchanged. The head branch is deliberately not hoisted: this endpoint does not carry it at all -- the row's ref is an empty string, not the branch -- so there is nothing to lift. pr show has head_branch.
This commit is contained in:
parent
8363a459bf
commit
7630b0993c
1 changed files with 92 additions and 1 deletions
|
|
@ -173,8 +173,9 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
|||
.page_size(u32::try_from(args.limit).unwrap_or(u32::MAX))
|
||||
.send()?;
|
||||
let count = issues.len() as u64;
|
||||
let items = serde_json::to_value(issues)?;
|
||||
let mut items = serde_json::to_value(issues)?;
|
||||
if client.json_mode() {
|
||||
hoist_merge_state(&mut items);
|
||||
return print_json(&items);
|
||||
}
|
||||
for item in items.as_array().into_iter().flatten() {
|
||||
|
|
@ -256,6 +257,46 @@ fn trailer(total: Option<u64>, count: u64, page: u64, limit: u64) -> Option<Stri
|
|||
/// rather than a per-issue `show`. Defensive: missing fields drop to
|
||||
/// placeholders so a partial response from a future API change still
|
||||
/// produces readable output instead of panicking on `unwrap`.
|
||||
/// Copy a PR row's `merged` / `merged_at` from the nested `pull_request`
|
||||
/// object up to the top level of the row.
|
||||
///
|
||||
/// Forgejo's issue-list endpoint answers "was this merged?" only inside
|
||||
/// `pull_request`, while `state` says `closed` for a merged PR *and* one
|
||||
/// closed without merging. So the obvious top-level query — `.merged_at`,
|
||||
/// or `.state` — is null/ambiguous for **every** row, and with a `//`
|
||||
/// default it renders as a confident "nothing merged" that cannot ever be
|
||||
/// right. Hoisting makes the obvious query the correct one instead of
|
||||
/// leaving a trap only a nested path avoids.
|
||||
///
|
||||
/// Additive: the original `pull_request` object is left untouched, so a
|
||||
/// consumer already reading the nested path keeps working. Issue rows have
|
||||
/// no `pull_request` and pass through unchanged.
|
||||
///
|
||||
/// ⚠️ The head branch is deliberately NOT hoisted — this endpoint does not
|
||||
/// carry it at all. The row's `ref` is an **empty string**, not the branch,
|
||||
/// so there is nothing to lift; `pr show <n>` has `head_branch`.
|
||||
fn hoist_merge_state(items: &mut Value) {
|
||||
let Some(rows) = items.as_array_mut() else {
|
||||
return;
|
||||
};
|
||||
for row in rows {
|
||||
let Some(pr) = row.get("pull_request") else {
|
||||
continue;
|
||||
};
|
||||
let merged = pr.get("merged").cloned();
|
||||
let merged_at = pr.get("merged_at").cloned();
|
||||
let Some(obj) = row.as_object_mut() else {
|
||||
continue;
|
||||
};
|
||||
if let Some(v) = merged {
|
||||
obj.insert("merged".to_owned(), v);
|
||||
}
|
||||
if let Some(v) = merged_at {
|
||||
obj.insert("merged_at".to_owned(), v);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn print_row(item: &Value, progress: Option<(usize, usize)>, blocking_open: usize) {
|
||||
let number = item.get("number").and_then(Value::as_u64).unwrap_or(0);
|
||||
let title = item.get("title").and_then(Value::as_str).unwrap_or("");
|
||||
|
|
@ -315,6 +356,56 @@ mod tests {
|
|||
serde_json::json!({ "number": 1, "title": "x", "state": state })
|
||||
}
|
||||
|
||||
/// A merged PR, a closed-but-unmerged PR and a plain issue in one
|
||||
/// array — because telling the first two apart is the entire point,
|
||||
/// and `state` cannot: it reads `closed` for both.
|
||||
#[test]
|
||||
fn hoisting_separates_merged_from_closed_unmerged_and_leaves_issues_alone() {
|
||||
let mut items = serde_json::json!([
|
||||
{ "number": 1, "state": "closed",
|
||||
"pull_request": { "merged": true, "merged_at": "2026-08-26T22:47:32+02:00" } },
|
||||
{ "number": 2, "state": "closed",
|
||||
"pull_request": { "merged": false, "merged_at": null } },
|
||||
{ "number": 3, "state": "closed" },
|
||||
]);
|
||||
|
||||
// Control: before hoisting, a top-level query cannot tell 1 from 2.
|
||||
assert!(items[0].get("merged").is_none(), "control: not hoisted yet");
|
||||
assert_eq!(
|
||||
items[0]["state"], items[1]["state"],
|
||||
"control: `state` is identical"
|
||||
);
|
||||
|
||||
hoist_merge_state(&mut items);
|
||||
|
||||
assert_eq!(items[0]["merged"], serde_json::json!(true), "merged PR");
|
||||
assert_eq!(
|
||||
items[1]["merged"],
|
||||
serde_json::json!(false),
|
||||
"closed unmerged"
|
||||
);
|
||||
assert_eq!(
|
||||
items[0]["merged_at"],
|
||||
serde_json::json!("2026-08-26T22:47:32+02:00")
|
||||
);
|
||||
assert!(items[1]["merged_at"].is_null());
|
||||
assert!(
|
||||
items[2].get("merged").is_none(),
|
||||
"an issue must not gain a `merged` field"
|
||||
);
|
||||
// Additive: the nested path an existing consumer reads still works.
|
||||
assert_eq!(items[0]["pull_request"]["merged"], serde_json::json!(true));
|
||||
}
|
||||
|
||||
/// Non-array input (an error object, say) must not panic or mangle.
|
||||
#[test]
|
||||
fn hoisting_a_non_array_is_a_no_op() {
|
||||
let mut v = serde_json::json!({ "message": "not found" });
|
||||
let before = v.clone();
|
||||
hoist_merge_state(&mut v);
|
||||
assert_eq!(v, before);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dep_progress_none_when_no_dependencies() {
|
||||
assert_eq!(dep_progress(&[]), None);
|
||||
|
|
|
|||
Loading…
Reference in a new issue