vinzenz/hyperhive
vinzenz/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Activity

Compare commits

base: vinzenz:9986da4c7f68d2bad40963d8eca1181ec100c65a
Branches Tags
vinzenz:main
..
compare: vinzenz:6d10c1367eaab078fe7e198f66ed2a2ff49952dc
Branches Tags
vinzenz:main
2 changed files with 21 additions and 141 deletions
Download patch file Download diff file Expand all files Collapse all files

70
hive-forge/src/verbs/attach.rs
View file

@ -51,7 +51,7 @@ pub fn run_issue(client: &Client, args: IssueArgs) -> Result<()> {
.api()
.issue_create_issue_attachment(owner, name, index(args.number)?, &bytes, query)
.send()?;
println!("{}", require_url(&resp)?);
print_url(&resp);
Ok(())
}
@ -78,7 +78,7 @@ pub fn run_comment(client: &Client, args: CommentArgs) -> Result<()> {
.api()
.issue_create_issue_comment_attachment(owner, name, index(args.id)?, &bytes, query)
.send()?;
println!("{}", require_url(&resp)?);
print_url(&resp);
Ok(())
}
@ -93,68 +93,8 @@ fn file_name(path: &Path) -> Option<String> {
path.file_name().map(|n| n.to_string_lossy().into_owned())
}
/// The uploaded attachment's browser download URL, hard-erroring if the
/// forge answered 2xx with none. A caller piping the printed URL (e.g.
/// `url=$(hive-forge attach-issue …)`) used to get an empty string and no
/// signal when this was `None` — the upload had happened, but nothing said
/// so, and nothing said it hadn't.
fn require_url(v: &Attachment) -> Result<url::Url> {
v.browser_download_url.clone().ok_or_else(|| {
anyhow::anyhow!(
"hive-forge attach: upload succeeded but the forge returned no download URL \
(attachment {})",
attachment_ident(v)
)
})
}
/// Best-effort identifier for an attachment error message — id if present,
/// else name, else a placeholder so the message never comes out empty.
fn attachment_ident(v: &Attachment) -> String {
match (v.id, v.name.as_deref()) {
(Some(id), Some(n)) => format!("id={id} name={n}"),
(Some(id), None) => format!("id={id}"),
(None, Some(n)) => format!("name={n}"),
(None, None) => "<unknown>".to_owned(),
}
}
#[cfg(test)]
mod tests {
use super::require_url;
use forgejo_api::structs::Attachment;
fn attachment(url: Option<&str>) -> Attachment {
Attachment {
browser_download_url: url.map(|u| u.parse().unwrap()),
created_at: None,
download_count: None,
id: Some(7),
name: Some("out.log".to_owned()),
size: None,
r#type: None,
uuid: None,
}
}
#[test]
fn control_present_url_is_returned() {
let a = attachment(Some("https://forge.example/a/1"));
assert_eq!(
require_url(&a).unwrap().as_str(),
"https://forge.example/a/1"
);
}
#[test]
fn missing_url_errors_naming_the_attachment() {
// A 2xx response with no browser_download_url used to print nothing
// and exit 0 — a caller doing `url=$(hive-forge attach-issue …)`
// got an empty string with no signal anything went wrong.
let a = attachment(None);
let err = require_url(&a).unwrap_err();
let msg = err.to_string();
assert!(msg.contains("id=7"), "missing attachment id: {msg}");
assert!(msg.contains("out.log"), "missing attachment name: {msg}");
fn print_url(v: &Attachment) {
if let Some(url) = &v.browser_download_url {
println!("{url}");
}
}

92
hive-forge/src/verbs/labels.rs
View file

@ -69,36 +69,22 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
bail!("hive-forge labels remove: pass at least one label name");
}
let all = repo_labels(client)?;
// Same strict resolution as `add` — a typo used to be silently
// skipped here instead of refused.
let ids = resolve_ids(&all, &labels)?;
for id in ids {
client
.api()
.issue_remove_label(
owner,
name,
idx,
&id.to_string(),
DeleteLabelsOption { updated_at: None },
)
.send()?;
for label in &labels {
if let Some(id) = lookup_id(&all, label) {
let _ = client
.api()
.issue_remove_label(
owner,
name,
idx,
&id.to_string(),
DeleteLabelsOption { updated_at: None },
)
.send();
}
}
let current = client.api().issue_get_labels(owner, name, idx).send()?;
// Forgejo can 200 a remove it silently didn't apply (e.g.
// insufficient permission), same as the assignee case in
// `assign.rs` — diff the result against the intent instead of
// trusting the status code.
let still_present = still_present_names(&labels, &current);
if !still_present.is_empty() {
bail!(
"label(s) still present on #{} after remove — the forge \
rejected the change: {}",
args.number,
still_present.join(", ")
);
}
print_label_names(&current);
let labels = client.api().issue_get_labels(owner, name, idx).send()?;
print_label_names(&labels);
}
}
Ok(())
@ -176,21 +162,6 @@ fn lookup_id(all: &[Label], name: &str) -> Option<i64> {
.and_then(|l| l.id)
}
/// Which of `requested` names are still present in `current` — the
/// remove-verb's post-condition check. A non-empty result means the forge
/// didn't actually drop those labels even though the call returned 200.
fn still_present_names(requested: &[String], current: &[Label]) -> Vec<String> {
requested
.iter()
.filter(|r| {
current
.iter()
.any(|l| l.name.as_deref() == Some(r.as_str()))
})
.cloned()
.collect()
}
fn print_label_names(labels: &[Label]) {
let names: Vec<&str> = labels.iter().filter_map(|l| l.name.as_deref()).collect();
let _ = print_json(&json!(names));
@ -198,7 +169,7 @@ fn print_label_names(labels: &[Label]) {
#[cfg(test)]
mod tests {
use super::{resolve_ids, still_present_names};
use super::resolve_ids;
use forgejo_api::structs::Label;
fn label(id: i64, name: &str) -> Label {
@ -235,35 +206,4 @@ mod tests {
let err = resolve_ids(&[], &["anything".to_owned()]).unwrap_err();
assert!(err.to_string().contains("(none)"));
}
#[test]
fn unresolved_remove_name_is_refused_like_add() {
// `remove` resolves names through the same `resolve_ids` as `add`,
// so a typo is refused up front instead of silently doing nothing.
let all = vec![label(1, "area/ops")];
let err = resolve_ids(&all, &["area:ops".to_owned()]).unwrap_err();
assert!(err.to_string().contains("area:ops"));
}
#[test]
fn still_present_control_all_removed_reports_none() {
// Happy path: none of the requested names remain, so the caller
// should not bail.
let requested = vec!["area/ops".to_owned(), "type/bug".to_owned()];
let current = vec![label(3, "state/open")];
assert!(still_present_names(&requested, &current).is_empty());
}
#[test]
fn still_present_names_reports_labels_the_forge_kept() {
// A remove the forge silently rejected — the label is still on the
// issue after the "successful" call. This is the case the old code
// (`let _ = …send();` with no re-check) let through as success.
let requested = vec!["area/ops".to_owned(), "type/bug".to_owned()];
let current = vec![label(1, "area/ops")];
assert_eq!(
still_present_names(&requested, &current),
vec!["area/ops".to_owned()]
);
}
}