a73x

44552499

Make relates_to a set instead of a single value

a73x   2026-08-09 16:50

Commit message
Make relates_to a set instead of a single value

Revision requested in review: a singular Option<String> relates_to
can't represent the motivating case (one issue depending on more than
one other), so relate/unrelate did not actually solve the problem
they were added for.

IssueState.relates_to is now Vec<String>. relate adds a member and is
idempotent; unrelate removes one member and is a no-op if absent, same
as labels. --relates-to at issue open still takes a single value and
now seeds a one-element set. Direction is unchanged: relate/unrelate
still write one event to the named issue's own ref only.

Action::IssueOpen/IssueRelate/IssueUnrelate keep carrying a single
String, so no signed-event format changes. Old cached IssueState JSON
(and any other on-disk JSON) with the previous singular-or-null shape
still deserializes via a custom deserializer, tested directly against
the legacy string/null/missing shapes as well as the new array shape.
Cache format version bumped since the state-fold logic changed.
src/cache.rs
Old New
@@ -18,7 +18,9 @@ fn sanitize_ref_name(ref_name: &str) -> String {
18 /// Cache format version. Bump whenever state-fold logic changes so that 18 /// Cache format version. Bump whenever state-fold logic changes so that
19 /// entries computed with older logic are discarded even if the ref tip 19 /// entries computed with older logic are discarded even if the ref tip
20 /// hasn't moved. v2: review vote supersession per (author, revision). 20 /// hasn't moved. v2: review vote supersession per (author, revision).
21 const CACHE_FORMAT_VERSION: u32 = 2; 21 /// v3: `IssueState::relates_to` became a multi-member set instead of a
22 /// single optional value.
23 const CACHE_FORMAT_VERSION: u32 = 3;
22 24
23 /// Cache entry stored on disk: the tip OID at cache time + serialized state. 25 /// Cache entry stored on disk: the tip OID at cache time + serialized state.
24 #[derive(serde::Serialize, serde::Deserialize)] 26 #[derive(serde::Serialize, serde::Deserialize)]
src/lib.rs
Old New
@@ -176,8 +176,10 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
176 if !i.assignees.is_empty() { 176 if !i.assignees.is_empty() {
177 println!("Assignees: {}", i.assignees.join(", ")); 177 println!("Assignees: {}", i.assignees.join(", "));
178 } 178 }
179 if let Some(ref relates_to) = i.relates_to { 179 if !i.relates_to.is_empty() {
180 println!("Relates-to: {:.8}", relates_to); 180 let short: Vec<String> =
181 i.relates_to.iter().map(|r| format!("{:.8}", r)).collect();
182 println!("Relates-to: {}", short.join(", "));
181 } 183 }
182 if let Some(ref reason) = i.close_reason { 184 if let Some(ref reason) = i.close_reason {
183 println!("Closed: {}", reason); 185 println!("Closed: {}", reason);
src/state.rs
Old New
@@ -52,6 +52,27 @@ fn deserialize_verdict<'de, D: serde::Deserializer<'de>>(d: D) -> Result<ReviewV
52 s.parse().map_err(serde::de::Error::custom) 52 s.parse().map_err(serde::de::Error::custom)
53 } 53 }
54 54
55 /// `relates_to` used to be a single `Option<String>`, written only at issue
56 /// creation. It is now a `Vec<String>` so an issue can relate to more than
57 /// one other issue. Cached state (and any other JSON on disk) written by an
58 /// older git-collab version still has the old shape, so this accepts a bare
59 /// string (-> one-element vec), `null`/absent (-> empty vec, via `#[serde(default)]`
60 /// on the field), or the current array shape.
61 #[derive(Deserialize)]
62 #[serde(untagged)]
63 enum RelatesToShape {
64 Many(Vec<String>),
65 One(String),
66 }
67
68 fn deserialize_relates_to<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Vec<String>, D::Error> {
69 match Option::<RelatesToShape>::deserialize(d)? {
70 None => Ok(Vec::new()),
71 Some(RelatesToShape::Many(v)) => Ok(v),
72 Some(RelatesToShape::One(s)) => Ok(vec![s]),
73 }
74 }
75
55 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] 76 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
56 #[serde(rename_all = "lowercase")] 77 #[serde(rename_all = "lowercase")]
57 pub enum IssueStatus { 78 pub enum IssueStatus {
@@ -115,8 +136,8 @@ pub struct IssueState {
115 #[serde(default)] 136 #[serde(default)]
116 pub last_updated: String, 137 pub last_updated: String,
117 pub author: Author, 138 pub author: Author,
118 #[serde(default, skip_serializing_if = "Option::is_none")] 139 #[serde(default, deserialize_with = "deserialize_relates_to")]
119 pub relates_to: Option<String>, 140 pub relates_to: Vec<String>,
120 } 141 }
121 142
122 #[derive(Debug, Clone, Serialize, Deserialize)] 143 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -317,7 +338,9 @@ impl IssueState {
317 created_at: event.timestamp.clone(), 338 created_at: event.timestamp.clone(),
318 last_updated: String::new(), 339 last_updated: String::new(),
319 author: event.author.clone(), 340 author: event.author.clone(),
320 relates_to, 341 // `--relates-to` at open time seeds the relation set
342 // with a single member.
343 relates_to: relates_to.into_iter().collect(),
321 }); 344 });
322 } 345 }
323 Action::IssueComment { body } => { 346 Action::IssueComment { body } => {
@@ -365,18 +388,14 @@ impl IssueState {
365 } 388 }
366 Action::IssueRelate { relates_to } => { 389 Action::IssueRelate { relates_to } => {
367 if let Some(ref mut s) = state { 390 if let Some(ref mut s) = state {
368 s.relates_to = Some(relates_to); 391 if !s.relates_to.contains(&relates_to) {
392 s.relates_to.push(relates_to);
393 }
369 } 394 }
370 } 395 }
371 Action::IssueUnrelate { relates_to } => { 396 Action::IssueUnrelate { relates_to } => {
372 if let Some(ref mut s) = state { 397 if let Some(ref mut s) = state {
373 // relates_to is a single field, not a set: only clear 398 s.relates_to.retain(|r| r != &relates_to);
374 // it if it still points at the target being removed,
375 // so an unrelate against a stale/mismatched target is
376 // a no-op rather than clobbering a newer relation.
377 if s.relates_to.as_deref() == Some(relates_to.as_str()) {
378 s.relates_to = None;
379 }
380 } 399 }
381 } 400 }
382 Action::IssueAssign { assignee } => { 401 Action::IssueAssign { assignee } => {
@@ -933,3 +952,80 @@ pub fn resolve_patch_ref(
933 ) -> Result<(String, String), crate::error::Error> { 952 ) -> Result<(String, String), crate::error::Error> {
934 resolve_ref(repo, "patches", "patch", prefix) 953 resolve_ref(repo, "patches", "patch", prefix)
935 } 954 }
955
956 #[cfg(test)]
957 mod tests {
958 use super::*;
959
960 // `relates_to` used to be a single `Option<String>`. Cached state and any
961 // other on-disk JSON written by older git-collab versions can still hold
962 // that shape, so `IssueState` must keep reading it even though it now
963 // materializes a `Vec<String>`.
964
965 fn issue_json_with_relates_to(relates_to_json: &str) -> String {
966 format!(
967 r#"{{
968 "id": "abc123",
969 "title": "t",
970 "body": "",
971 "status": "open",
972 "close_reason": null,
973 "closed_by": null,
974 "labels": [],
975 "assignees": [],
976 "comments": [],
977 "linked_commits": [],
978 "created_at": "2026-01-01T00:00:00Z",
979 "last_updated": "",
980 "author": {{"name": "A", "email": "a@example.com"}},
981 "relates_to": {relates_to_json}
982 }}"#
983 )
984 }
985
986 #[test]
987 fn deserializes_legacy_singular_string_relates_to_as_one_element_vec() {
988 let json = issue_json_with_relates_to(r#""def456""#);
989 let issue: IssueState = serde_json::from_str(&json).unwrap();
990 assert_eq!(issue.relates_to, vec!["def456".to_string()]);
991 }
992
993 #[test]
994 fn deserializes_null_relates_to_as_empty_vec() {
995 let json = issue_json_with_relates_to("null");
996 let issue: IssueState = serde_json::from_str(&json).unwrap();
997 assert_eq!(issue.relates_to, Vec::<String>::new());
998 }
999
1000 #[test]
1001 fn deserializes_new_array_relates_to() {
1002 let json = issue_json_with_relates_to(r#"["def456", "ghi789"]"#);
1003 let issue: IssueState = serde_json::from_str(&json).unwrap();
1004 assert_eq!(
1005 issue.relates_to,
1006 vec!["def456".to_string(), "ghi789".to_string()]
1007 );
1008 }
1009
1010 #[test]
1011 fn deserializes_missing_relates_to_field_as_empty_vec() {
1012 // Predates the relates_to field existing at all.
1013 let json = r#"{
1014 "id": "abc123",
1015 "title": "t",
1016 "body": "",
1017 "status": "open",
1018 "close_reason": null,
1019 "closed_by": null,
1020 "labels": [],
1021 "assignees": [],
1022 "comments": [],
1023 "linked_commits": [],
1024 "created_at": "2026-01-01T00:00:00Z",
1025 "last_updated": "",
1026 "author": {"name": "A", "email": "a@example.com"}
1027 }"#;
1028 let issue: IssueState = serde_json::from_str(json).unwrap();
1029 assert_eq!(issue.relates_to, Vec::<String>::new());
1030 }
1031 }
src/tui/mod.rs
Old New
@@ -67,7 +67,7 @@ mod tests {
67 created_at: String::new(), 67 created_at: String::new(),
68 last_updated: String::new(), 68 last_updated: String::new(),
69 author: make_author(), 69 author: make_author(),
70 relates_to: None, 70 relates_to: vec![],
71 } 71 }
72 } 72 }
73 73
@@ -258,7 +258,7 @@ mod tests {
258 created_at: "2026-01-01T00:00:00Z".to_string(), 258 created_at: "2026-01-01T00:00:00Z".to_string(),
259 last_updated: "2026-01-01T00:00:00Z".to_string(), 259 last_updated: "2026-01-01T00:00:00Z".to_string(),
260 author: test_author(), 260 author: test_author(),
261 relates_to: None, 261 relates_to: vec![],
262 }) 262 })
263 .collect() 263 .collect()
264 } 264 }
tests/archive_test.rs
Old New
@@ -250,7 +250,7 @@ fn test_issue_open_with_relates_to() {
250 250
251 let issue = IssueState::from_ref(&repo, &ref_name, &id).unwrap(); 251 let issue = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
252 assert_eq!(issue.title, "Child issue"); 252 assert_eq!(issue.title, "Child issue");
253 assert_eq!(issue.relates_to.as_deref(), Some(id1.as_str())); 253 assert_eq!(issue.relates_to, vec![id1]);
254 } 254 }
255 255
256 #[test] 256 #[test]
@@ -261,5 +261,5 @@ fn test_issue_open_without_relates_to() {
261 let (ref_name, id) = open_issue(&repo, &alice(), "Solo issue"); 261 let (ref_name, id) = open_issue(&repo, &alice(), "Solo issue");
262 262
263 let issue = IssueState::from_ref(&repo, &ref_name, &id).unwrap(); 263 let issue = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
264 assert_eq!(issue.relates_to, None); 264 assert_eq!(issue.relates_to, Vec::<String>::new());
265 } 265 }
tests/cli_test.rs
Old New
@@ -340,9 +340,9 @@ fn test_issue_relate_does_not_touch_target() {
340 } 340 }
341 341
342 #[test] 342 #[test]
343 fn test_issue_relate_replaces_existing_relation() { 343 fn test_issue_relate_to_multiple_issues() {
344 // relates_to is a single field, not a list: relating to a new target 344 // The motivating case: one issue can depend on more than one other.
345 // overwrites whatever was there before. 345 // c65ac0e2 relates to both 8db8d346 and 20c577fc.
346 let repo = TestRepo::new("Alice", "alice@example.com"); 346 let repo = TestRepo::new("Alice", "alice@example.com");
347 let id1 = repo.issue_open("First issue"); 347 let id1 = repo.issue_open("First issue");
348 let id2 = repo.issue_open("Second issue"); 348 let id2 = repo.issue_open("Second issue");
@@ -352,8 +352,25 @@ fn test_issue_relate_replaces_existing_relation() {
352 repo.run_ok(&["issue", "relate", &id3, &id2]); 352 repo.run_ok(&["issue", "relate", &id3, &id2]);
353 353
354 let out = repo.run_ok(&["issue", "show", &id3]); 354 let out = repo.run_ok(&["issue", "show", &id3]);
355 assert!(out.contains(&id1[..8]));
355 assert!(out.contains(&id2[..8])); 356 assert!(out.contains(&id2[..8]));
356 assert!(!out.contains(&id1[..8])); 357 }
358
359 #[test]
360 fn test_issue_relate_is_idempotent() {
361 // Relating to the same target twice is not an error and does not
362 // duplicate the entry.
363 let repo = TestRepo::new("Alice", "alice@example.com");
364 let id1 = repo.issue_open("First issue");
365 let id2 = repo.issue_open("Second issue");
366
367 repo.run_ok(&["issue", "relate", &id2, &id1]);
368 repo.run_ok(&["issue", "relate", &id2, &id1]);
369
370 let out = repo.run_ok(&["issue", "show", &id2, "--json"]);
371 let parsed: serde_json::Value = serde_json::from_str(&out).unwrap();
372 let relates_to = parsed["relates_to"].as_array().unwrap();
373 assert_eq!(relates_to.len(), 1);
357 } 374 }
358 375
359 #[test] 376 #[test]
@@ -371,9 +388,26 @@ fn test_issue_unrelate_removes_relation() {
371 } 388 }
372 389
373 #[test] 390 #[test]
374 fn test_issue_unrelate_mismatched_target_is_noop() { 391 fn test_issue_unrelate_removes_only_the_named_member() {
375 // Unrelating from an issue that isn't the current relation target 392 // Unrelating one member of the set leaves the others intact.
376 // leaves the existing relation untouched. 393 let repo = TestRepo::new("Alice", "alice@example.com");
394 let id1 = repo.issue_open("First issue");
395 let id2 = repo.issue_open("Second issue");
396 let id3 = repo.issue_open("Third issue");
397
398 repo.run_ok(&["issue", "relate", &id3, &id1]);
399 repo.run_ok(&["issue", "relate", &id3, &id2]);
400 repo.run_ok(&["issue", "unrelate", &id3, &id1]);
401
402 let out = repo.run_ok(&["issue", "show", &id3]);
403 assert!(!out.contains(&id1[..8]));
404 assert!(out.contains(&id2[..8]));
405 }
406
407 #[test]
408 fn test_issue_unrelate_absent_target_is_noop() {
409 // Unrelating a target that was never related leaves the existing
410 // relation(s) untouched.
377 let repo = TestRepo::new("Alice", "alice@example.com"); 411 let repo = TestRepo::new("Alice", "alice@example.com");
378 let id1 = repo.issue_open("First issue"); 412 let id1 = repo.issue_open("First issue");
379 let id2 = repo.issue_open("Second issue"); 413 let id2 = repo.issue_open("Second issue");
@@ -392,22 +426,28 @@ fn test_issue_relate_shown_in_json() {
392 let repo = TestRepo::new("Alice", "alice@example.com"); 426 let repo = TestRepo::new("Alice", "alice@example.com");
393 let id1 = repo.issue_open("First issue"); 427 let id1 = repo.issue_open("First issue");
394 let id2 = repo.issue_open("Second issue"); 428 let id2 = repo.issue_open("Second issue");
429 let id3 = repo.issue_open("Third issue");
395 430
396 repo.run_ok(&["issue", "relate", &id2, &id1]); 431 repo.run_ok(&["issue", "relate", &id3, &id1]);
432 repo.run_ok(&["issue", "relate", &id3, &id2]);
397 433
398 let out = repo.run_ok(&["issue", "show", &id2, "--json"]); 434 let out = repo.run_ok(&["issue", "show", &id3, "--json"]);
399 let parsed: serde_json::Value = serde_json::from_str(&out).unwrap(); 435 let parsed: serde_json::Value = serde_json::from_str(&out).unwrap();
400 assert!(parsed["relates_to"] 436 let relates_to: Vec<&str> = parsed["relates_to"]
401 .as_str() 437 .as_array()
402 .unwrap() 438 .unwrap()
403 .starts_with(&id1[..8])); 439 .iter()
440 .map(|v| v.as_str().unwrap())
441 .collect();
442 assert!(relates_to.contains(&id1.as_str()));
443 assert!(relates_to.contains(&id2.as_str()));
404 } 444 }
405 445
406 #[test] 446 #[test]
407 fn test_issue_relate_consistent_with_open_relates_to() { 447 fn test_issue_relate_consistent_with_open_relates_to() {
408 // `issue relate` after the fact should leave the same state shape as 448 // `issue relate` after the fact should leave the same state shape as
409 // `--relates-to` at creation time: a single relates_to field on the 449 // `--relates-to` at creation time: `--relates-to` seeds a one-element
410 // issue that names the other issue. 450 // relates_to set, just like a single `relate` call would.
411 let repo = TestRepo::new("Alice", "alice@example.com"); 451 let repo = TestRepo::new("Alice", "alice@example.com");
412 let id1 = repo.issue_open("First issue"); 452 let id1 = repo.issue_open("First issue");
413 453
@@ -425,10 +465,8 @@ fn test_issue_relate_consistent_with_open_relates_to() {
425 let out3 = repo.run_ok(&["issue", "show", &id3, "--json"]); 465 let out3 = repo.run_ok(&["issue", "show", &id3, "--json"]);
426 let parsed2: serde_json::Value = serde_json::from_str(&out2).unwrap(); 466 let parsed2: serde_json::Value = serde_json::from_str(&out2).unwrap();
427 let parsed3: serde_json::Value = serde_json::from_str(&out3).unwrap(); 467 let parsed3: serde_json::Value = serde_json::from_str(&out3).unwrap();
428 assert_eq!( 468 assert_eq!(parsed2["relates_to"], serde_json::json!([id1]));
429 parsed2["relates_to"].is_string(), 469 assert_eq!(parsed3["relates_to"], serde_json::json!([id1]));
430 parsed3["relates_to"].is_string()
431 );
432 } 470 }
433 471
434 // =========================================================================== 472 // ===========================================================================