Merge pull request #2364 from Urgau/gh-changes-since-review-comment

Handle view changes link for body-less reviews with review comments

Author: Urgau

src/github/issue.rs

@@ -237,6 +237,28 @@ impl Issue {
         Ok(())
     }
 
+    pub async fn edit_review_comment(
+        &self,
+        client: &GithubClient,
+        id: u64,
+        new_body: &str,
+    ) -> anyhow::Result<Comment> {
+        let comment_url = format!("{}/pulls/comments/{}", self.repository().url(client), id);
+        #[derive(serde::Serialize)]
+        struct EditComment<'a> {
+            body: &'a str,
+        }
+        let comment = client
+            .json(
+                client
+                    .patch(&comment_url)
+                    .json(&EditComment { body: new_body }),
+            )
+            .await
+            .context("failed to edit review comment")?;
+        Ok(comment)
+    }
+
     pub async fn remove_labels(
         &self,
         client: &GithubClient,
@@ -596,6 +618,23 @@ impl Issue {
             .await?;
         Ok(())
     }
+
+    pub async fn get_review(
+        &self,
+        client: &GithubClient,
+        review_id: u64,
+    ) -> anyhow::Result<Comment> {
+        let review_url = format!(
+            "{}/pulls/{}/reviews/{review_id}",
+            self.repository().url(client),
+            self.number,
+        );
+        let review = client
+            .json(client.get(&review_url))
+            .await
+            .context("unable to fetch review")?;
+        Ok(review)
+    }
 }
 
 // Comments
@@ -604,6 +643,10 @@ impl Issue {
 pub struct Comment {
     pub id: u64,
     pub node_id: String,
+    #[serde(default)]
+    pub in_reply_to_id: Option<u64>,
+    #[serde(default)]
+    pub pull_request_review_id: Option<u64>,
     #[serde(deserialize_with = "opt_string")]
     pub body: String,
     pub html_url: String,

src/handlers/review_changes_since.rs

@@ -1,11 +1,30 @@
+use std::sync::{Arc, LazyLock};
+
 use anyhow::Context as _;
 
 use crate::{
+    cache,
     config::ReviewChangesSinceConfig,
     github::{Comment, Event, Issue, IssueCommentAction, IssueCommentEvent},
     handlers::Context,
 };
 
+static REVIEW_BODY_CACHE: LazyLock<
+    tokio::sync::Mutex<cache::LeastRecentlyUsedCache<String, ReviewBodyState>>,
+> = LazyLock::new(|| tokio::sync::Mutex::new(cache::LeastRecentlyUsedCache::new(1000)));
+
+#[derive(Copy, Clone, Debug)]
+enum ReviewBodyState {
+    Present,
+    Absent,
+}
+
+impl cache::EstimatedSize for ReviewBodyState {
+    fn estimated_size(&self) -> usize {
+        std::mem::size_of::<Self>()
+    }
+}
+
 /// Checks if this event is a PR review creation and adds in the body (if there is one)
 /// a link our `gh-changes-since` endpoint to view changes since this review.
 pub(crate) async fn handle(
@@ -14,6 +33,7 @@ pub(crate) async fn handle(
     event: &Event,
     _config: &ReviewChangesSinceConfig,
 ) -> anyhow::Result<()> {
+    // Match on each review and top-level review comment
     if let Event::IssueComment(
         event @ IssueCommentEvent {
             action: IssueCommentAction::Created,
@@ -23,33 +43,105 @@ pub(crate) async fn handle(
             },
             comment:
                 Comment {
-                    pr_review_state: Some(_),
+                    in_reply_to_id: None,
                     ..
                 },
             ..
         },
     ) = event
-        && !event.comment.body.is_empty()
     {
-        // Add link our gh-changes-since endpoint to view changes since this review
-
         let issue_repo = event.issue.repository();
         let pr_num = event.issue.number;
 
         let base = &event.issue.base.as_ref().context("no base")?.sha;
         let head = &event.issue.head.as_ref().context("no head")?.sha;
 
         let link = format!("https://{host}/gh-changes-since/{issue_repo}/{pr_num}/{base}..{head}");
-        let new_body = format!(
-            "{}\n\n*[View changes since this review]({link})*",
-            event.comment.body
-        );
-
-        event
-            .issue
-            .edit_review(&ctx.github, event.comment.id, &new_body)
-            .await
-            .context("failed to update the review body")?;
+
+        if event.comment.pull_request_review_id.is_none() && event.comment.pr_review_state.is_some()
+        {
+            // this is a review (not a review comment)
+
+            {
+                // first let's store it's review body state in the cache to avoid future api calls
+                // when the review comments webhook arrives (a few milliseconds after)
+                let cache_key = format!(
+                    "{}/{}/{}",
+                    &event.repository.full_name, event.issue.number, event.comment.id
+                );
+                REVIEW_BODY_CACHE.lock().await.put(
+                    cache_key,
+                    Arc::new(if event.comment.body.is_empty() {
+                        ReviewBodyState::Absent
+                    } else {
+                        ReviewBodyState::Present
+                    }),
+                );
+            }
+
+            if !event.comment.body.is_empty() {
+                // the review body is not empty, we can add to it the link to
+                // our gh-changes-since endpoint
+                let new_body = format!(
+                    "{}\n\n*[View changes since this review]({link})*",
+                    event.comment.body,
+                );
+
+                event
+                    .issue
+                    .edit_review(&ctx.github, event.comment.id, &new_body)
+                    .await
+                    .context("failed to update the review body")?;
+            }
+        } else if !event.comment.body.is_empty()
+            && let Some(review_id) = event.comment.pull_request_review_id
+        {
+            // this is a review comment (not a review), we need to check if the parent
+            // review already has a body (and as such a link)
+
+            // fetch the parent review body state, first look into the cache
+            let review_body_state = {
+                let cache_key = format!(
+                    "{}/{}/{}",
+                    &event.repository.full_name, event.issue.number, review_id
+                );
+                match { REVIEW_BODY_CACHE.lock().await.get(&cache_key) } {
+                    Some(state) => *state,
+                    None => {
+                        let review = event
+                            .issue
+                            .get_review(&ctx.github, review_id)
+                            .await
+                            .context("unable to fetch the parent review")?;
+                        let state = if review.body.is_empty() {
+                            ReviewBodyState::Absent
+                        } else {
+                            ReviewBodyState::Present
+                        };
+                        REVIEW_BODY_CACHE
+                            .lock()
+                            .await
+                            .put(cache_key, Arc::new(state));
+                        state
+                    }
+                }
+            };
+
+            if let ReviewBodyState::Absent = review_body_state {
+                // parent review is empty, let's add the link to the review comment instead
+
+                let new_body = format!(
+                    "*[View changes since the review]({link})*\n\n{}",
+                    event.comment.body
+                );
+
+                event
+                    .issue
+                    .edit_review_comment(&ctx.github, event.comment.id, &new_body)
+                    .await
+                    .context("failed to update the review comment body")?;
+            }
+        }
     }
 
     Ok(())