refactor: Reduce cognitive complexity of WebhookNotification handler. 2/2

Closes kemitix/git-next#49

crates/repo-actor/src/handlers/webhook_notification.rs

@@ -2,9 +2,12 @@
 use actix::prelude::*;
 
 use crate::{self as actor, messages::WebhookNotification, RepoActorLog};
-use git_next_config::webhook::push::Branch;
 use git_next_config::WebhookAuth;
-use git_next_git::{self as git, ForgeLike};
+use git_next_config::{
+    webhook::{push::Branch, Push},
+    BranchName,
+};
+use git_next_git::{self as git, Commit, ForgeLike};
 
 use tracing::{info, warn};
 
@@ -38,46 +41,40 @@ impl Handler<WebhookNotification> for actor::RepoActor {
                     return;
                 }
                 Some(Branch::Main) => {
-                    actor::logger(&self.log, "message is for main branch");
+                    if handle_push(
-                    let commit = git::Commit::from(push);
+                        push,
-                    if self.last_main_commit.as_ref() == Some(&commit) {
+                        config.branches().main(),
-                        actor::logger(&self.log, "not a new commit on main");
+                        &mut self.last_main_commit,
-                        info!(
+                        &self.log,
-                            branch = %config.branches().main(),
+                    )
-                            %commit,
+                    .is_err()
-                            "Ignoring - already aware of branch at commit",
+                    {
-                        );
                         return;
-                    }
+                    };
-                    self.last_main_commit.replace(commit);
                 }
                 Some(Branch::Next) => {
-                    actor::logger(&self.log, "message is for next branch");
+                    if handle_push(
-                    let commit = git::Commit::from(push);
+                        push,
-                    if self.last_next_commit.as_ref() == Some(&commit) {
+                        config.branches().next(),
-                        actor::logger(&self.log, "not a new commit on next");
+                        &mut self.last_next_commit,
-                        info!(
+                        &self.log,
-                            branch = %config.branches().next(),
+                    )
-                            %commit,
+                    .is_err()
-                            "Ignoring - already aware of branch at commit",
+                    {
-                        );
                         return;
-                    }
+                    };
-                    self.last_next_commit.replace(commit);
                 }
                 Some(Branch::Dev) => {
-                    actor::logger(&self.log, "message is for dev branch");
+                    if handle_push(
-                    let commit = git::Commit::from(push);
+                        push,
-                    if self.last_dev_commit.as_ref() == Some(&commit) {
+                        config.branches().dev(),
-                        actor::logger(&self.log, "not a new commit on dev");
+                        &mut self.last_dev_commit,
-                        info!(
+                        &self.log,
-                            branch = %config.branches().dev(),
+                    )
-                            %commit,
+                    .is_err()
-                            "Ignoring - already aware of branch at commit",
+                    {
-                        );
                         return;
-                    }
+                    };
-                    self.last_dev_commit.replace(commit);
                 }
             },
         }
@@ -120,3 +117,24 @@ fn validate_notification(
     }
     Ok(())
 }
+
+fn handle_push(
+    push: Push,
+    branch: BranchName,
+    last_commit: &mut Option<Commit>,
+    log: &Option<RepoActorLog>,
+) -> Result<(), ()> {
+    actor::logger(log, "message is for dev branch");
+    let commit = git::Commit::from(push);
+    if last_commit.as_ref() == Some(&commit) {
+        actor::logger(log, format!("not a new commit on {branch}"));
+        info!(
+            %branch ,
+            %commit,
+            "Ignoring - already aware of branch at commit",
+        );
+        return Err(());
+    }
+    last_commit.replace(commit);
+    Ok(())
+}

crates/repo-actor/src/tests/handlers/webhook_notification.rs

@@ -252,8 +252,9 @@ async fn when_message_is_push_already_seen_commit_to_main() -> TestResult {
     let forge_notification = ForgeNotification::new(forge_alias, repo_alias, headers, body);
     let repository_factory = MockRepositoryFactory::new();
     let commit = given::a_commit();
+    let main = repo_config.branches().main();
     let push = given::a_push()
-        .with_branch(repo_config.branches().main())
+        .with_branch(main.clone())
         .with_sha(commit.sha().to_string())
         .with_message(commit.message().to_string());
     let mut forge = given::a_forge();
@@ -283,7 +284,7 @@ async fn when_message_is_push_already_seen_commit_to_main() -> TestResult {
 
     //then
     log.no_message_contains("send")?;
-    log.require_message_containing("not a new commit on main")?;
+    log.require_message_containing(format!("not a new commit on {main}"))?;
     Ok(())
 }
 
@@ -300,8 +301,9 @@ async fn when_message_is_push_already_seen_commit_to_next() -> TestResult {
     let forge_notification = ForgeNotification::new(forge_alias, repo_alias, headers, body);
     let repository_factory = MockRepositoryFactory::new();
     let commit = given::a_commit();
+    let next = repo_config.branches().next();
     let push = given::a_push()
-        .with_branch(repo_config.branches().next())
+        .with_branch(next.clone())
         .with_sha(commit.sha().to_string())
         .with_message(commit.message().to_string());
     let mut forge = given::a_forge();
@@ -331,7 +333,7 @@ async fn when_message_is_push_already_seen_commit_to_next() -> TestResult {
 
     //then
     log.no_message_contains("send")?;
-    log.require_message_containing("not a new commit on next")?;
+    log.require_message_containing(format!("not a new commit on {next}"))?;
     Ok(())
 }
 
@@ -348,8 +350,9 @@ async fn when_message_is_push_already_seen_commit_to_dev() -> TestResult {
     let forge_notification = ForgeNotification::new(forge_alias, repo_alias, headers, body);
     let repository_factory = MockRepositoryFactory::new();
     let commit = given::a_commit();
+    let dev = repo_config.branches().dev();
     let push = given::a_push()
-        .with_branch(repo_config.branches().dev())
+        .with_branch(dev.clone())
         .with_sha(commit.sha().to_string())
         .with_message(commit.message().to_string());
     let mut forge = given::a_forge();
@@ -379,7 +382,7 @@ async fn when_message_is_push_already_seen_commit_to_dev() -> TestResult {
 
     //then
     log.no_message_contains("send")?;
-    log.require_message_containing("not a new commit on dev")?;
+    log.require_message_containing(format!("not a new commit on {dev}"))?;
     Ok(())
 }