diff --git a/src/clients/tangled/comments.rs b/src/clients/tangled/comments.rs index 81ebbad..60ddf3e 100644 --- a/src/clients/tangled/comments.rs +++ b/src/clients/tangled/comments.rs @@ -16,14 +16,20 @@ //! Tangled unified issue, pull and string comments into //! `sh.tangled.feed.comment`, and //! [`crate::lexicon::tangled::LEGACY_ISSUE_COMMENT_NSID`] is deprecated for -//! *writing*. Reading is the other direction: the index returns whatever was -//! written, and a thread opened before the unification is mostly legacy -//! records. Both spellings are read here, because a reader that understood -//! only the current one would show a two-year-old discussion as empty. +//! *writing*. Reading is the other direction: a thread opened before the +//! unification is mostly legacy records, and one is still returned under its +//! own `$type`. //! -//! The two also spell their body differently — the legacy record nests it -//! under `body.text` and the current one under `body` as a markdown object — -//! which is what [`Comment::body`] exists to hide. +//! What is *not* returned is the record. The index normalises a legacy +//! comment towards the current shape on the way out: the record's `body` is a +//! plain string and arrives here as an object with a `text`, and the record's +//! bare `issue` at-uri arrives as a `subject` strongRef whose `cid` is +//! `bafkqaaa`, the CID of no bytes at all. So both collections reach +//! [`Comment::read`] spelling their body the same way, and the plain-string +//! branch there is for the record as a PDS holds it — see +//! `tests/fixtures/issue_comment_records.json` beside +//! `feed_comments_on_issue.json`, which are the same three records off the +//! two services. use anyhow::Result; @@ -60,9 +66,11 @@ impl Comment { fn read(item: &serde_json::Value) -> Option { let uri = item["uri"].as_str()?.to_string(); let value = &item["value"]; - // `body.text` is the legacy `sh.tangled.repo.issue.comment` shape; - // a bare string is `sh.tangled.feed.comment`'s markdown object seen - // through serde, whose own `text` is the same field one level in. + // `body.text` is what the index returns for either collection: a + // `sh.tangled.markup.markdown` object for a current comment, and the + // same shape synthesised for a legacy one. A plain string is the + // legacy *record* as its own PDS holds it, which this path does not + // read today and which costs one `or_else` to keep readable. let body = value["body"]["text"] .as_str() .or_else(|| value["body"].as_str())? @@ -110,28 +118,79 @@ pub async fn of(subject_uri: &str, limit: u32) -> Result> { #[cfg(test)] mod tests { use super::Comment; + use crate::lexicon::tangled::LEGACY_ISSUE_COMMENT_NSID; use serde_json::json; - /// Both collections' body spellings, because a thread older than the - /// unification is mostly the legacy one and a reader that knew only the - /// current shape would show it as empty. + /// Both collections, on the bytes the index actually returns. + /// + /// A thread older than the unification is entirely legacy records, and a + /// reader that understood only the current collection would show that + /// discussion as empty. The captured pair is three comments on one issue: + /// what `sh.tangled.feed.listComments` hands back, and the same records + /// as their author's PDS holds them. #[test] fn both_comment_collections_are_read() { - let legacy = json!({ - "uri": "at://did:plc:them/sh.tangled.repo.issue.comment/1", - "value": { "body": { "text": "the old shape" }, "createdAt": "2026-05-02T00:31:23+03:00" }, - }); - let current = json!({ - "uri": "at://did:plc:them/sh.tangled.feed.comment/2", - "value": { "body": "the new shape", "createdAt": "2026-05-03T00:00:00Z" }, - }); - assert_eq!(Comment::read(&legacy).unwrap().body, "the old shape"); - assert_eq!(Comment::read(¤t).unwrap().body, "the new shape"); - assert_eq!( - Comment::read(&legacy).unwrap().author_did, - "did:plc:them", - "the commenter is the at-uri's authority, not the subject's" + let items = |fixture: &str, key: &str| -> Vec { + serde_json::from_str::(fixture).expect("fixture is JSON")[key] + .as_array() + .expect("fixture holds that array") + .clone() + }; + + // A legacy thread, as the index returns it: `$type` still names the + // deprecated collection, and the body has been lifted into an object. + let legacy = items( + include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/fixtures/feed_comments_on_issue.json" + )), + "items", + ); + assert_eq!(legacy.len(), 3, "the captured thread changed"); + for item in &legacy { + assert_eq!(item["value"]["$type"], LEGACY_ISSUE_COMMENT_NSID); + let comment = Comment::read(item).expect("a legacy comment is readable"); + assert!(!comment.body.is_empty()); + assert_eq!( + comment.author_did, + item["uri"] + .as_str() + .unwrap() + .trim_start_matches("at://") + .split('/') + .next() + .unwrap(), + "the commenter is the at-uri's authority, not the subject's" + ); + } + + // The same records off their author's PDS, where the body is a plain + // string. Not a shape this module reads today; read here anyway, + // which is the whole of what the `or_else` branch buys. + let records = items( + include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/fixtures/issue_comment_records.json" + )), + "records", ); + for record in &records { + assert!(record["value"]["body"].is_string()); + assert!(Comment::read(record).is_some()); + } + + // And a current thread, which is where the whole tree is heading. + let current = items( + include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/fixtures/feed_comments_on_pull.json" + )), + "items", + ); + for item in ¤t { + assert_eq!(item["value"]["$type"], "sh.tangled.feed.comment"); + assert!(Comment::read(item).is_some()); + } } /// A comment with nothing in it is not a comment. The appview refuses