cob: Guarantee there is always a latest revision

We change `Patch::latest` to always return something, because a patch is
malformed if it doesn't have at least one revision, and thus would not
load in the first place.

This also implies that there is a latest revision in the face of
concurrent updates.

Note that currently, traversal order for concurrent updates is still random.
This will be fixed in another patch.

radicle-cli/src/commands/patch/checkout.rs

@@ -53,9 +53,7 @@ fn find_patch_commit<'a>(
         Ok(commit) => Ok(commit),
         Err(e) if git::ext::is_not_found_err(&e) => {
             // TODO: Handle case of concurrent revisions.
-            let (_, rev) = patch
+            let (_, rev) = patch.latest();
-                .latest()
-                .ok_or(anyhow!("patch does not have any revisions"))?;
             let author = *rev.author().id();
             let remote = stored.remote(&author)?;
 

radicle-cli/src/commands/patch/list.rs

@@ -1,5 +1,3 @@
-use anyhow::anyhow;
-
 use radicle::cob::patch;
 use radicle::cob::patch::{Patch, PatchId, Patches, Verdict};
 use radicle::prelude::*;
@@ -58,10 +56,7 @@ pub fn run(
     all.sort_by(|(id1, p1), (id2, p2)| {
         let is_me = (p2.author().id().as_key() == &me).cmp(&(p1.author().id().as_key() == &me));
         let by_id = id1.cmp(id2);
-        let by_rev_time = p2
+        let by_rev_time = p1.updated_at().cmp(&p2.updated_at());
-            .latest()
-            .map(|(_, r)| r.timestamp())
-            .cmp(&p1.latest().map(|(_, r)| r.timestamp()));
 
         is_me.then(by_rev_time).then(by_id)
     });
@@ -100,9 +95,7 @@ pub fn row(
     repository: &Repository,
 ) -> anyhow::Result<[term::Line; 9]> {
     let state = patch.state();
-    let (_, revision) = patch
+    let (_, revision) = patch.latest();
-        .latest()
-        .ok_or_else(|| anyhow!("patch is malformed: no revisions found"))?;
     let stats = common::diff_stats(repository.raw(), revision.base(), &revision.head())?;
     let author = patch.author().id;
 
@@ -126,15 +119,10 @@ pub fn row(
         term::format::secondary(term::format::oid(revision.head())).into(),
         term::format::positive(format!("+{}", stats.insertions())).into(),
         term::format::negative(format!("-{}", stats.deletions())).into(),
-        term::format::timestamp(
+        term::format::timestamp(&patch.updated_at())
-            &patch
+            .dim()
-                .latest()
+            .italic()
-                .map(|(_, r)| r.timestamp())
+            .into(),
-                .unwrap_or_default(),
-        )
-        .dim()
-        .italic()
-        .into(),
     ])
 }
 

radicle-cli/src/commands/patch/show.rs

@@ -64,9 +64,7 @@ pub fn run(
     let Some(patch) = patches.get(patch_id)? else {
         anyhow::bail!("Patch `{patch_id}` not found");
     };
-    let (_, revision) = patch
+    let (_, revision) = patch.latest();
-        .latest()
-        .ok_or_else(|| anyhow!("patch is malformed: no revisions found"))?;
     let state = patch.state();
     let branches = common::branches(&revision.head(), workdir)?;
     let target_head = common::patch_merge_target_oid(patch.target(), stored)?;

radicle-cli/src/commands/patch/update.rs

@@ -78,8 +78,7 @@ pub fn run(
         anyhow::bail!("Patch `{patch_id}` not found");
     };
 
-    // TODO(cloudhead): Handle error.
+    let (_, current_revision) = patch.latest();
-    let (_, current_revision) = patch.latest().unwrap();
     if current_revision.head() == branch_oid(&head_branch)? {
         if !quiet {
             term::info!("Nothing to do, patch is already up to date.");

radicle-remote-helper/src/push.rs

@@ -416,9 +416,7 @@ fn patch_merge<G: Signer>(
     let mut patches = patch::Patches::open(stored)?;
     for patch in patches.all()? {
         let (id, patch, clock) = patch?;
-        let Some((revision_id, revision)) = patch.latest() else {
+        let (revision_id, revision) = patch.latest();
-            continue;
-        };
 
         if patch.is_open() && commits.contains(&revision.head()) {
             let revision_id = *revision_id;

radicle-tui/src/cob/patch.rs

@@ -26,7 +26,7 @@ pub fn load_proposed(repository: &Repository) -> Result<Vec<(PatchId, Patch)>> {
 }
 
 pub fn sync_status(repository: &Repository, patch: &Patch) -> Result<(usize, usize)> {
-    let (_, revision) = patch.latest().unwrap();
+    let (_, revision) = patch.latest();
     let target_oid = merge_target_oid(patch.target(), repository)?;
     let (ahead, behind) = repository
         .raw()

radicle-tui/src/ui/cob/patch.rs

@@ -49,10 +49,8 @@ pub fn format_timestamp(patch: &Patch) -> String {
 }
 
 pub fn format_comments(patch: &Patch) -> String {
-    let count = match patch.latest() {
+    let count: usize = patch.revisions().map(|(_, r)| r.discussion().len()).sum();
-        Some((_, rev)) => rev.discussion().len(),
+
-        None => 0,
-    };
     format!("{count}")
 }
 

radicle/src/cob/patch.rs

@@ -281,21 +281,13 @@ impl Patch {
 
     /// Reference to the Git object containing the code on the latest revision.
     pub fn head(&self) -> &git::Oid {
-        &self
+        &self.latest().1.oid
-            .latest()
-            .map(|(_, r)| r)
-            .expect("Patch::head: at least one revision is present")
-            .oid
     }
 
     /// Get the commit of the target branch on which this patch is based.
     /// This can change via a patch update.
     pub fn base(&self) -> &git::Oid {
-        &self
+        &self.latest().1.base
-            .latest()
-            .map(|(_, r)| r)
-            .expect("Patch::base: at least one revision is present")
-            .base
     }
 
     /// Index of latest revision in the revisions list.
@@ -317,8 +309,15 @@ impl Patch {
     }
 
     /// Latest revision.
-    pub fn latest(&self) -> Option<(&RevisionId, &Revision)> {
+    pub fn latest(&self) -> (&RevisionId, &Revision) {
-        self.revisions().next_back()
+        self.revisions()
+            .next_back()
+            .expect("Patch::latest: there is always at least one revision")
+    }
+
+    /// Time of last update.
+    pub fn updated_at(&self) -> Timestamp {
+        self.latest().1.timestamp()
     }
 
     /// Check if the patch is open.
@@ -1516,7 +1515,7 @@ mod test {
         assert_eq!(patch.target(), target);
         assert_eq!(patch.version(), 0);
 
-        let (rev_id, revision) = patch.latest().unwrap();
+        let (rev_id, revision) = patch.latest();
 
         assert_eq!(revision.author.id(), &author);
         assert_eq!(revision.description(), "Blah blah blah.");
@@ -1665,7 +1664,7 @@ mod test {
             )
             .unwrap();
 
-        let (rid, _) = patch.latest().unwrap();
+        let (rid, _) = patch.latest();
         patch
             .review(
                 *rid,
@@ -1678,7 +1677,7 @@ mod test {
 
         let id = patch.id;
         let patch = patches.get(&id).unwrap().unwrap();
-        let (_, revision) = patch.latest().unwrap();
+        let (_, revision) = patch.latest();
         assert_eq!(revision.reviews.len(), 1);
 
         let review = revision.reviews.get(signer.public_key()).unwrap();
@@ -1794,7 +1793,7 @@ mod test {
             )
             .unwrap();
 
-        let (rid, _) = patch.latest().unwrap();
+        let (rid, _) = patch.latest();
         let rid = *rid;
 
         let inline = vec![CodeComment {
@@ -1828,7 +1827,7 @@ mod test {
 
         let id = patch.id;
         let mut patch = patches.get_mut(&id).unwrap();
-        let (_, revision) = patch.latest().unwrap();
+        let (_, revision) = patch.latest();
         assert_eq!(revision.reviews.len(), 1, "the reviews were merged");
 
         let review = revision.reviews.get(signer.public_key()).unwrap();
@@ -1845,7 +1844,7 @@ mod test {
                 signer,
             )
             .unwrap(); // Overwrite the comment.
-        let (_, revision) = patch.latest().unwrap();
+        let (_, revision) = patch.latest();
         let review = revision.reviews.get(signer.public_key()).unwrap();
         assert_eq!(review.verdict(), Some(Verdict::Reject));
         assert_eq!(review.comment(), Some("Whoops!"));
@@ -1871,7 +1870,7 @@ mod test {
             )
             .unwrap();
 
-        let (rid, _) = patch.latest().unwrap();
+        let (rid, _) = patch.latest();
         let rid = *rid;
 
         patch
@@ -1895,7 +1894,7 @@ mod test {
 
         let id = patch.id;
         let patch = patches.get_mut(&id).unwrap();
-        let (_, revision) = patch.latest().unwrap();
+        let (_, revision) = patch.latest();
         assert_eq!(revision.reviews.len(), 1, "the reviews were merged");
 
         let review = revision.reviews.get(signer.public_key()).unwrap();
@@ -1922,7 +1921,7 @@ mod test {
             )
             .unwrap();
 
-        let (rid, _) = patch.latest().unwrap();
+        let (rid, _) = patch.latest();
         let rid = *rid;
 
         patch
@@ -1938,7 +1937,7 @@ mod test {
 
         let id = patch.id;
         let patch = patches.get_mut(&id).unwrap();
-        let (_, revision) = patch.latest().unwrap();
+        let (_, revision) = patch.latest();
 
         let review = revision.reviews.get(signer.public_key()).unwrap();
         assert_eq!(review.verdict(), None);
@@ -1988,7 +1987,7 @@ mod test {
             "I've made changes."
         );
 
-        let (_, revision) = patch.latest().unwrap();
+        let (_, revision) = patch.latest();
 
         assert_eq!(patch.version(), 1);
         assert_eq!(revision.oid, update.oid);