From ef24e69d3dddcdf07a1b5ce334ce789917dc960f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sandro=20J=C3=A4ckel?= <sandro.jaeckel@sap.com>
Date: Fri, 16 Aug 2024 18:25:46 +0200
Subject: [PATCH] Only consider approvals for the last commit in a PR

---
 Cargo.lock     |  3 +--
 Cargo.toml     |  3 ++-
 src/changes.rs | 33 +++++++++++++++++++++++++++++----
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 7293f24..db5ade5 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -786,8 +786,7 @@ dependencies = [
 [[package]]
 name = "octocrab"
 version = "0.39.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9305e4c99543ecd0f42bd659c9e9d6ca7115fe5e37d5c85a7277b1db0d4c4101"
+source = "git+https://github.com/SuperSandro2000/octocrab.git?branch=pr_commits#24575e78ec3a3449dd042cf4be46a7faad3ad430"
 dependencies = [
  "arc-swap",
  "async-trait",
diff --git a/Cargo.toml b/Cargo.toml
index 0e14692..bec8b1e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -9,7 +9,8 @@ edition = "2021"
 anyhow = "^1"
 clap = { version = "^4", features = ["derive", "env"] }
 git2 = "^0"
-octocrab = "^0"
+# change back to crates.io after https://github.com/XAMPPRocky/octocrab/pull/680 got released
+octocrab = { version = "^0", git = "https://github.com/SuperSandro2000/octocrab.git", branch = "pr_commits" }
 serde = "^1"
 serde_yml = "^0"
 tokio = { version = "^1", features = ["macros", "rt-multi-thread"] }
diff --git a/src/changes.rs b/src/changes.rs
index 104518a..e16aa13 100644
--- a/src/changes.rs
+++ b/src/changes.rs
@@ -85,9 +85,26 @@ impl RepoChangeset {
                     .to_string(),
             );
 
+            let mut pr_commits_page = octocrab
+              .pulls(&self.remote.owner, &self.remote.repository)
+              .pr_commits(associated_pr.number)
+              .await
+              .context("failed to get pr commits")?;
+            assert!(
+              pr_commits_page.next.is_none(),
+              "found more than one page for associated_prs"
+            );
+
+            let pr_commits = pr_commits_page.take_items();
+            assert!(
+              pr_commits.len() <= 250,
+              "found more than 250 commits which requires a different api endpoint per doc"
+            );
+            let head_sha = pr_commits.last().ok_or(anyhow!("PR contains no commits?"))?.sha.clone();
+
             if let Some(changeset) = self.changes.iter_mut().find(|cs| cs.pr_link == associated_pr_link) {
                 changeset.commits.push(change_commit.clone());
-                changeset.collect_approved_reviews(&pr_reviews);
+                changeset.collect_approved_reviews(&pr_reviews, &head_sha);
                 continue;
             }
 
@@ -97,7 +114,7 @@ impl RepoChangeset {
                 approvals: Vec::new(),
             };
 
-            changeset.collect_approved_reviews(&pr_reviews);
+            changeset.collect_approved_reviews(&pr_reviews, &head_sha);
             self.changes.push(changeset);
         }
 
@@ -114,7 +131,7 @@ pub struct Changeset {
 
 impl Changeset {
     // pr_reviews must be sorted by key submitted_at!
-    pub fn collect_approved_reviews(&mut self, pr_reviews: &[Review]) {
+    pub fn collect_approved_reviews(&mut self, pr_reviews: &[Review], head_sha: &String) {
         let mut last_review_by: Vec<&String> = vec![];
 
         // reverse the order of reviews to start with the oldest
@@ -123,12 +140,20 @@ impl Changeset {
                 continue;
             };
 
-            // only consider the last review of any user
+            // Only consider the last review of any user.
+            // For example a user might have requested changes early on in the PR and later approved it
+            // or requested additional changes after supplying an approval first.
             if last_review_by.contains(&&user.login) {
                 continue;
             }
             last_review_by.push(&user.login);
 
+            // Only account for reviews done on the last commit of the PR.
+            // We could count the PR as partly reviewed but that is to complicated to present at the moment.
+            if pr_review.commit_id != Some(head_sha.to_string()) {
+              continue;
+            }
+
             // in case it isn't approve, ignore it
             if pr_review.state != Some(Approved) {
                 continue;