Fix issue closing vote (#399)

Signed-off-by: Sergio Castaño Arteaga <[email protected]>

Author: tegioz

src/db.rs

@@ -1,7 +1,7 @@
 use crate::{
     cfg::CfgProfile,
     cmd::{CheckVoteInput, CreateVoteInput},
-    github::{split_full_name, DynGH},
+    github::{self, split_full_name, DynGH},
     results::{self, Vote, VoteResults},
 };
 use anyhow::Result;
@@ -28,7 +28,7 @@ pub(crate) trait DB {
     ) -> Result<Option<Uuid>>;
 
     /// Close any pending finished vote.
-    async fn close_finished_vote(&self, gh: DynGH) -> Result<Option<(Vote, VoteResults)>>;
+    async fn close_finished_vote(&self, gh: DynGH) -> Result<Option<(Vote, Option<VoteResults>)>>;
 
     /// Get open vote (if available) in the issue/pr provided.
     async fn get_open_vote(
@@ -84,6 +84,7 @@ impl PgDB {
                 from vote
                 where current_timestamp > ends_at
                 and closed = false
+                order by random()
                 for update of vote skip locked
                 limit 1
                 ",
@@ -98,7 +99,7 @@ impl PgDB {
     async fn store_vote_results(
         tx: &Transaction<'_>,
         vote_id: Uuid,
-        results: &VoteResults,
+        results: &Option<VoteResults>,
     ) -> Result<()> {
         tx.execute(
             "
@@ -141,7 +142,7 @@ impl DB for PgDB {
     }
 
     /// [DB::close_finished_vote]
-    async fn close_finished_vote(&self, gh: DynGH) -> Result<Option<(Vote, VoteResults)>> {
+    async fn close_finished_vote(&self, gh: DynGH) -> Result<Option<(Vote, Option<VoteResults>)>> {
         // Get pending finished vote (if any) from database
         let mut db = self.pool.get().await?;
         let tx = db.transaction().await?;
@@ -151,7 +152,18 @@ impl DB for PgDB {
 
         // Calculate results
         let (owner, repo) = split_full_name(&vote.repository_full_name);
-        let results = results::calculate(gh, owner, repo, &vote).await?;
+        let results = match results::calculate(gh, owner, repo, &vote).await {
+            Ok(results) => Some(results),
+            Err(err) => {
+                if github::is_not_found_error(&err) {
+                    // Vote comment was deleted. We still want to proceed and
+                    // close the vote so that we don't try again to close it.
+                    None
+                } else {
+                    return Err(err);
+                }
+            }
+        };
 
         // Store results in database
         PgDB::store_vote_results(&tx, vote.vote_id, &results).await?;

src/github.rs

@@ -1,5 +1,5 @@
 use crate::cfg::CfgProfile;
-use anyhow::Result;
+use anyhow::{Error, Result};
 use async_trait::async_trait;
 use axum::http::HeaderValue;
 #[cfg(test)]
@@ -595,3 +595,17 @@ pub(crate) fn split_full_name(full_name: &str) -> (&str, &str) {
     let mut parts = full_name.split('/');
     (parts.next().unwrap(), parts.next().unwrap())
 }
+
+/// Check if the provided error is a "Not Found" error from GitHub.
+pub(crate) fn is_not_found_error(err: &Error) -> bool {
+    if let Some(octocrab::Error::GitHub {
+        source,
+        backtrace: _,
+    }) = err.downcast_ref::<octocrab::Error>()
+    {
+        if source.message == "Not Found" {
+            return true;
+        }
+    }
+    false
+}

src/processor.rs

@@ -424,32 +424,40 @@ impl Processor {
         // Record vote_id as part of the current span
         tracing::Span::current().record("vote_id", &vote.vote_id.to_string());
 
-        // Post vote closed comment on the issue/pr
+        // Post vote closed comment on the issue/pr if results were returned
+        // (i.e. if the vote comment was removed, the vote will be closed but
+        // no results will be received)
         let inst_id = vote.installation_id as u64;
         let (owner, repo) = split_full_name(&vote.repository_full_name);
-        let body = tmpl::VoteClosed::new(&results).render()?;
-        self.gh
-            .post_comment(inst_id, owner, repo, vote.issue_number, &body)
-            .await?;
+        if let Some(results) = &results {
+            let body = tmpl::VoteClosed::new(results).render()?;
+            self.gh
+                .post_comment(inst_id, owner, repo, vote.issue_number, &body)
+                .await?;
+        }
 
         // Create check run if the vote is on a pull request
         if vote.is_pull_request {
-            let (conclusion, summary) = if results.passed {
-                (
-                    "success",
-                    format!(
-                        "The vote passed! {} out of {} voted in favor.",
-                        results.in_favor, results.allowed_voters
-                    ),
-                )
+            let (conclusion, summary) = if let Some(results) = &results {
+                if results.passed {
+                    (
+                        "success",
+                        format!(
+                            "The vote passed! {} out of {} voted in favor.",
+                            results.in_favor, results.allowed_voters
+                        ),
+                    )
+                } else {
+                    (
+                        "failure",
+                        format!(
+                            "The vote did not pass. {} out of {} voted in favor.",
+                            results.in_favor, results.allowed_voters
+                        ),
+                    )
+                }
             } else {
-                (
-                    "failure",
-                    format!(
-                        "The vote did not pass. {} out of {} voted in favor.",
-                        results.in_favor, results.allowed_voters
-                    ),
-                )
+                ("success", "The vote was cancelled".to_string())
             };
             let check_details = CheckDetails {
                 status: "completed".to_string(),
@@ -1194,7 +1202,7 @@ mod tests {
         db.expect_close_finished_vote().returning(move |_| {
             Box::pin(future::ready(Ok(Some((
                 setup_test_vote(),
-                results_copy.clone(),
+                Some(results_copy.clone()),
             )))))
         });
         let mut gh = MockGH::new();
@@ -1222,7 +1230,7 @@ mod tests {
         db.expect_close_finished_vote().returning(move |_| {
             let mut vote = setup_test_vote();
             vote.is_pull_request = true;
-            Box::pin(future::ready(Ok(Some((vote, results_copy.clone())))))
+            Box::pin(future::ready(Ok(Some((vote, Some(results_copy.clone()))))))
         });
         let mut gh = MockGH::new();
         gh.expect_post_comment()
@@ -1266,7 +1274,7 @@ mod tests {
         db.expect_close_finished_vote().returning(move |_| {
             let mut vote = setup_test_vote();
             vote.is_pull_request = true;
-            Box::pin(future::ready(Ok(Some((vote, results_copy.clone())))))
+            Box::pin(future::ready(Ok(Some((vote, Some(results_copy.clone()))))))
         });
         let mut gh = MockGH::new();
         gh.expect_post_comment()