Use WriterT.tell

src/main/scala/com/gu/tip/GitHubApi.scala

@@ -2,12 +2,13 @@ package com.gu.tip
 
 import cats.data.WriterT
 import cats.effect.IO
+import cats.implicits._
 import com.typesafe.scalalogging.LazyLogging
 import net.liftweb.json._
 import net.liftweb.json.DefaultFormats
 
 trait GitHubApiIf { this: HttpClientIf =>
-  def getLastMergeCommitMessage(): WriterT[IO, List[Log], String]
+  def getLastMergeCommitMessage: WriterT[IO, List[Log], String]
   def setLabel(prNumber: String): WriterT[IO, List[Log], String]
 
   val githubApiRoot = "https://api.github.com"
@@ -24,27 +25,14 @@ trait GitHubApi extends GitHubApiIf with LazyLogging { this: HttpClientIf =>
 
     So we try to pick out pull request number #118
   */
-  override def getLastMergeCommitMessage(): WriterT[IO, List[Log], String] =
-  WriterT {
-    get(s"$githubApiRoot/repos/$owner/$repo/commits/master", authHeader).map { responseBody =>
-      val commitMessage = (parse(responseBody) \ "commit" \ "message").extract[String]
-      (
-        List(Log("INFO", s"Successfully retrieved commit message of last merged PR: $commitMessage")),
-        commitMessage
-      )
-    }
-  }
+  override def getLastMergeCommitMessage: WriterT[IO, List[Log], String] =
+    get(s"$githubApiRoot/repos/$owner/$repo/commits/master", authHeader).map(
+      response => (parse(response) \ "commit" \ "message").extract[String]
+    ).tell(List(Log("INFO", s"Successfully retrieved commit message of last merged PR")))
 
   override def setLabel(prNumber: String): WriterT[IO, List[Log], String] =
-  WriterT {
-    post(s"$githubApiRoot/repos/$owner/$repo/issues/$prNumber/labels", authHeader, s"""["$label"]""").map {
-      responseBody =>
-        (
-          List(Log("INFO", s"Successfully set label '$label' on PR $prNumber")),
-          responseBody
-        )
-    }
-  }
+    post(s"$githubApiRoot/repos/$owner/$repo/issues/$prNumber/labels", authHeader, s"""["$label"]""")
+      .tell(List(Log("INFO", s"Successfully set label '$label' on PR $prNumber")))
 
   private lazy val authHeader = "Authorization" -> s"token $personalAccessToken"
 }

src/main/scala/com/gu/tip/Notifier.scala

@@ -25,14 +25,11 @@ trait Notifier extends NotifierIf with LazyLogging { this: GitHubApiIf =>
     So we try to pick out pull request number #118
   */
   private def getLastMergedPullRequestNumber(): WriterT[IO, List[Log], String] =
-    getLastMergeCommitMessage().mapBoth { (logs, commitMessage) =>
+    getLastMergeCommitMessage.map { commitMessage =>
       val prNumberPattern = """#\d+""".r
       val prNumber = prNumberPattern.findFirstIn(commitMessage).get.tail // Using get() because currently we just swallow any exception in Tip.verify()
-      (
-        List(Log("INFO", s"Successfully extracted PR number $prNumber from the commit message of the last merged PR: $commitMessage")),
-        prNumber
-      )
-    }
+      prNumber
+    }.tell(List(Log("INFO", s"Successfully extracted PR number from the commit message of the last merged PR")))
 
   private def setGitHubLabel(prNumber: String): WriterT[IO, List[Log], String] =
     setLabel(prNumber).tell(List(Log("INFO", s"Successfully set verification label on PR $prNumber")))

src/test/scala/com/gu/tip/HttpClientTest.scala

@@ -10,15 +10,15 @@ class HttpClientTest extends FlatSpec with MustMatchers {
 
   it should "be able to make a GET request" in {
     HttpClient.get("http://example.com", ("Authorization","test"))
-      .attempt.map(_.fold(error => fail, _ => succeed)).unsafeRunSync()
+      .run.attempt.map(_.fold(error => fail, _ => succeed)).unsafeRunSync()
   }
 
   it should "be able to make a POST request" in {
     HttpClient.post(
       "https://duckduckgo.com",
       ("",""),
       "test body"
-    ).attempt.map(_.fold(error => fail, _ => succeed)).unsafeRunSync()
+    ).run.attempt.map(_.fold(error => fail, _ => succeed)).unsafeRunSync()
   }
 
 }

src/test/scala/com/gu/tip/NotifierTest.scala

@@ -1,5 +1,6 @@
 package com.gu.tip
 
+import cats.data.WriterT
 import cats.effect.IO
 
 import scala.concurrent.ExecutionContext.Implicits.global
@@ -25,10 +26,10 @@ class NotifierTest extends FlatSpec with MustMatchers {
   it should "return non-200 if cannot set the label" in {
     trait MockHttpClient extends HttpClient {
       override def get(endpoint: String = "", authHeader: (String, String) = ("", "")) =
-        IO(mockCommitMessageResponse)
+        WriterT.putT(IO(mockCommitMessageResponse))(List(Log("", "")))
 
       override def post(endpoint: String = "", authHeader: (String, String) = ("", ""), jsonBody: String = "") =
-        IO.raiseError(UnexpectedStatus(InternalServerError))
+        WriterT(IO.raiseError(UnexpectedStatus(InternalServerError)).map(_ => (List(Log("", "")), "")))
     }
 
     object Notifier extends Notifier with GitHubApi with MockHttpClient
@@ -41,10 +42,10 @@ class NotifierTest extends FlatSpec with MustMatchers {
   it should "return 200 if successfully set the label on the latest merged pull request" in {
     trait MockHttpClient extends HttpClient {
       override def get(endpoint: String = "", authHeader: (String, String) = ("", "")) =
-        IO(mockCommitMessageResponse)
+        WriterT.putT(IO(mockCommitMessageResponse))(List(Log("", "")))
 
       override def post(endpoint: String = "", authHeader: (String, String) = ("", ""), jsonBody: String = "") =
-        IO("")
+        WriterT.putT(IO(""))(List(Log("", "")))
     }
 
     object Notifier extends Notifier with GitHubApi with MockHttpClient

src/test/scala/com/gu/tip/TipTest.scala

@@ -21,8 +21,8 @@ class TipTest extends AsyncFlatSpec with MustMatchers {
     )
 
   trait MockHttpClient extends HttpClient {
-    override def get(endpoint: String = "", authHeader: (String, String) = ("", "")) = IO("")
-    override def post(endpoint: String = "", authHeader: (String, String) = ("", ""), jsonBody: String = "") = IO("")
+    override def get(endpoint: String = "", authHeader: (String, String) = ("", "")) = mockOkResponse
+    override def post(endpoint: String = "", authHeader: (String, String) = ("", ""), jsonBody: String = "") = mockOkResponse
   }
 
   object Tip extends Tip with Notifier with GitHubApi with MockHttpClient
@@ -53,9 +53,7 @@ class TipTest extends AsyncFlatSpec with MustMatchers {
   it should "handle exceptions thrown from Notifier" in {
     trait MockNotifier extends NotifierIf { this: GitHubApiIf =>
       override def setLabelOnLatestMergedPr: WriterT[IO, List[Log], String] =
-        WriterT(
-          IO.raiseError(throw new RuntimeException).map(_ => (List(Log("", "")), ""))
-        )
+        WriterT(IO.raiseError(throw new RuntimeException).map(_ => (List(Log("", "")), "")))
     }
 
     object Tip extends Tip with MockNotifier with GitHubApi with MockHttpClient
@@ -69,9 +67,7 @@ class TipTest extends AsyncFlatSpec with MustMatchers {
   it should "handle failure to set the label" in {
     trait MockNotifier extends NotifierIf { this: GitHubApiIf =>
       override def setLabelOnLatestMergedPr: WriterT[IO, List[Log], String] =
-        WriterT(
-          IO.raiseError(UnexpectedStatus(InternalServerError)).map(_ => (List(Log("", "")), ""))
-        )
+        WriterT(IO.raiseError(UnexpectedStatus(InternalServerError)).map(_ => (List(Log("", "")), "")))
     }
 
     object Tip extends Tip with MockNotifier with GitHubApi with MockHttpClient