Merge pull request #1850 from Urgau/rendered_link_push+

Handle pushes and rename for rendered links
rust-lang · Oct 31, 2024 · 742b66b · 742b66b
2 parents a5f31cd + 18de61a
commit 742b66b
Showing 1 changed file with 69 additions and 49 deletions.
diff --git a/src/handlers/rendered_link.rs b/src/handlers/rendered_link.rs
@@ -1,3 +1,5 @@
+use std::borrow::Cow;
+
 use anyhow::bail;
 
 use crate::{
@@ -34,65 +36,83 @@ async fn add_rendered_link(ctx: &Context, e: &IssuesEvent, prefix: &str) -> anyh
     if e.action == IssuesAction::Opened
         || e.action == IssuesAction::Closed
         || e.action == IssuesAction::Reopened
+        || e.action == IssuesAction::Synchronize
     {
         let files = e.issue.files(&ctx.github).await?;
 
-        if let Some(file) = files.iter().find(|f| f.filename.starts_with(prefix)) {
-            let head = e.issue.head.as_ref().unwrap();
-            let base = e.issue.base.as_ref().unwrap();
+        let rendered_link = files
+            .iter()
+            .find(|f| f.filename.starts_with(prefix))
+            .map(|file| {
+                let head = e.issue.head.as_ref().unwrap();
+                let base = e.issue.base.as_ref().unwrap();
 
-            // This URL should be stable while the PR is open, even if the
-            // user pushes new commits.
-            //
-            // It will go away if the user deletes their branch, or if
-            // they reset it (such as if they created a PR from master).
-            // That should usually only happen after the PR is closed
-            // a which point we switch to a SHA-based url.
-            //
-            // If the PR is merged we use a URL that points to the actual
-            // repository, as to be resilient to branch deletion, as well
-            // be in sync with current "master" branch.
-            //
-            // For a PR "octocat:master" <- "Bob:patch-1", we generate,
-            //  - if merged: `https://github.com/octocat/REPO/blob/master/FILEPATH`
-            //  - if open: `https://github.com/Bob/REPO/blob/patch-1/FILEPATH`
-            //  - if closed: `https://github.com/octocat/REPO/blob/SHA/FILEPATH`
-            let rendered_link = format!(
-                "[Rendered](https://github.com/{}/blob/{}/{})",
-                if e.issue.merged || e.action == IssuesAction::Closed {
-                    &e.repository.full_name
-                } else {
-                    &head.repo.full_name
-                },
-                if e.issue.merged {
-                    &base.git_ref
-                } else if e.action == IssuesAction::Closed {
-                    &head.sha
-                } else {
-                    &head.git_ref
-                },
-                file.filename
-            );
+                // This URL should be stable while the PR is open, even if the
+                // user pushes new commits.
+                //
+                // It will go away if the user deletes their branch, or if
+                // they reset it (such as if they created a PR from master).
+                // That should usually only happen after the PR is closed
+                // a which point we switch to a SHA-based url.
+                //
+                // If the PR is merged we use a URL that points to the actual
+                // repository, as to be resilient to branch deletion, as well
+                // be in sync with current "master" branch.
+                //
+                // For a PR "octocat:master" <- "Bob:patch-1", we generate,
+                //  - if merged: `https://github.com/octocat/REPO/blob/master/FILEPATH`
+                //  - if open: `https://github.com/Bob/REPO/blob/patch-1/FILEPATH`
+                //  - if closed: `https://github.com/octocat/REPO/blob/SHA/FILEPATH`
+                format!(
+                    "[Rendered](https://github.com/{}/blob/{}/{})",
+                    if e.issue.merged || e.action == IssuesAction::Closed {
+                        &e.repository.full_name
+                    } else {
+                        &head.repo.full_name
+                    },
+                    if e.issue.merged {
+                        &base.git_ref
+                    } else if e.action == IssuesAction::Closed {
+                        &head.sha
+                    } else {
+                        &head.git_ref
+                    },
+                    file.filename
+                )
+            });
 
-            let new_body = if !e.issue.body.contains("[Rendered]") {
+        let new_body: Cow<'_, str> = if !e.issue.body.contains("[Rendered]") {
+            if let Some(rendered_link) = rendered_link {
                 // add rendered link to the end of the body
-                format!("{}\n\n{rendered_link}", e.issue.body)
-            } else if let Some(start_pos) = e.issue.body.find("[Rendered](") {
-                let Some(end_offset) = &e.issue.body[start_pos..].find(')') else {
-                    bail!("no `)` after `[Rendered]` found")
-                };
+                format!("{}\n\n{rendered_link}", e.issue.body).into()
+            } else {
+                // or return the original body since we don't have
+                // a rendered link to add
+                e.issue.body.as_str().into()
+            }
+        } else if let Some(start_pos) = e.issue.body.find("[Rendered](") {
+            let Some(end_offset) = &e.issue.body[start_pos..].find(')') else {
+                bail!("no `)` after `[Rendered]` found")
+            };
 
-                // replace the current rendered link with the new one
-                e.issue.body.replace(
+            // replace the current rendered link with the new one or replace
+            // it with an empty string if we don't have one
+            e.issue
+                .body
+                .replace(
                     &e.issue.body[start_pos..=(start_pos + end_offset)],
-                    &rendered_link,
+                    rendered_link.as_deref().unwrap_or(""),
                 )
-            } else {
-                bail!(
-                    "found `[Rendered]` but not it's associated link, can't replace it, bailing out"
-                )
-            };
+                .into()
+        } else {
+            bail!(
+                "found `[Rendered]` but not it's associated link, can't replace it or remove it, bailing out"
+            )
+        };
 
+        // avoid an expensive GitHub api call by first checking if we actually
+        // edited the pull request body
+        if e.issue.body != new_body {
             e.issue.edit_body(&ctx.github, &new_body).await?;
         }
     }