Remove ControllerBase dependency from HandleCommentService

src/main/scala/gitbucket/core/controller/ApiController.scala

@@ -163,8 +163,10 @@ trait ApiControllerBase extends ControllerBase {
   post("/api/v3/repos/:owner/:repository/issues/:id/comments")(readableUsersOnly { repository =>
     (for{
       issueId      <- params("id").toIntOpt
+      issue        <- getIssue(repository.owner, repository.name, issueId.toString)
       body         <- extractFromJsonBody[CreateAComment].map(_.body) if ! body.isEmpty
-      (issue, id)  <- handleComment(issueId, Some(body), repository)()
+      action       =  params.get("action").filter(_ => isEditable(issue.userName, issue.repositoryName, issue.openedUserName))
+      (issue, id)  <- handleComment(issue, Some(body), repository, action)
       issueComment <- getComment(repository.owner, repository.name, id.toString())
     } yield {
       JsonFormat(ApiComment(issueComment, RepositoryName(repository), issueId, ApiUser(context.loginAccount.get), issue.isPullRequest))
@@ -380,5 +382,8 @@ trait ApiControllerBase extends ControllerBase {
     }) getOrElse NotFound
   })
 
+  private def isEditable(owner: String, repository: String, author: String)(implicit context: Context): Boolean =
+    hasWritePermission(owner, repository, context.loginAccount) || author == context.loginAccount.get.userName
+
 }
 

src/main/scala/gitbucket/core/controller/IssuesController.scala

@@ -160,16 +160,22 @@ trait IssuesControllerBase extends ControllerBase {
   })
 
   post("/:owner/:repository/issue_comments/new", commentForm)(readableUsersOnly { (form, repository) =>
-    handleComment(form.issueId, Some(form.content), repository)() map { case (issue, id) =>
+    getIssue(repository.owner, repository.name, form.issueId.toString).flatMap { issue =>
-      redirect(s"/${repository.owner}/${repository.name}/${
+      val actionOpt = params.get("action").filter(_ => isEditable(issue.userName, issue.repositoryName, issue.openedUserName))
-        if(issue.isPullRequest) "pull" else "issues"}/${form.issueId}#comment-${id}")
+      handleComment(issue, Some(form.content), repository, actionOpt) map { case (issue, id) =>
+        redirect(s"/${repository.owner}/${repository.name}/${
+          if(issue.isPullRequest) "pull" else "issues"}/${form.issueId}#comment-${id}")
+      }
     } getOrElse NotFound
   })
 
   post("/:owner/:repository/issue_comments/state", issueStateForm)(readableUsersOnly { (form, repository) =>
-    handleComment(form.issueId, form.content, repository)() map { case (issue, id) =>
+    getIssue(repository.owner, repository.name, form.issueId.toString).flatMap { issue =>
-      redirect(s"/${repository.owner}/${repository.name}/${
+      val actionOpt = params.get("action").filter(_ => isEditable(issue.userName, issue.repositoryName, issue.openedUserName))
-        if(issue.isPullRequest) "pull" else "issues"}/${form.issueId}#comment-${id}")
+      handleComment(issue, form.content, repository, actionOpt) map { case (issue, id) =>
+        redirect(s"/${repository.owner}/${repository.name}/${
+          if(issue.isPullRequest) "pull" else "issues"}/${form.issueId}#comment-${id}")
+      }
     } getOrElse NotFound
   })
 
@@ -287,8 +293,16 @@ trait IssuesControllerBase extends ControllerBase {
   post("/:owner/:repository/issues/batchedit/state")(collaboratorsOnly { repository =>
     defining(params.get("value")){ action =>
       action match {
-        case Some("open")  => executeBatch(repository) { handleComment(_, None, repository)( _ => Some("reopen")) }
+        case Some("open")  => executeBatch(repository) { issueId =>
-        case Some("close") => executeBatch(repository) { handleComment(_, None, repository)( _ => Some("close"))  }
+          getIssue(repository.owner, repository.name, issueId.toString).foreach { issue =>
+            handleComment(issue, None, repository, Some("reopen"))
+          }
+        }
+        case Some("close") => executeBatch(repository) { issueId =>
+          getIssue(repository.owner, repository.name, issueId.toString).foreach { issue =>
+            handleComment(issue, None, repository, Some("close"))
+          }
+        }
         case _ => // TODO BadRequest
       }
     }

src/main/scala/gitbucket/core/service/HandleCommentService.scala

@@ -1,97 +1,91 @@
 package gitbucket.core.service
 
-import gitbucket.core.controller.{ControllerBase, Context}
+import gitbucket.core.controller.Context
 import gitbucket.core.model.Issue
+import gitbucket.core.model.Profile._
 import gitbucket.core.util.ControlUtil._
 import gitbucket.core.util.Implicits._
 import gitbucket.core.util.Notifier
+import profile.simple._
 
-// TODO Remove dependency to ControllerBase and merge to IssueService
 trait HandleCommentService {
-  self: ControllerBase with RepositoryService with IssuesService with ActivityService
+  self: RepositoryService with IssuesService with ActivityService
     with WebHookService with WebHookIssueCommentService with WebHookPullRequestService =>
 
   /**
    * @see [[https://github.com/takezoe/gitbucket/wiki/CommentAction]]
    */
-  def handleComment(issueId: Int, content: Option[String], repository: RepositoryService.RepositoryInfo)
+  def handleComment(issue: Issue, content: Option[String], repository: RepositoryService.RepositoryInfo, actionOpt: Option[String])
-                           (getAction: Issue => Option[String] = p1 => params.get("action").filter(_ => isEditable(p1.userName, p1.repositoryName, p1.openedUserName))) = {
+                   (implicit context: Context, s: Session) = {
 
     defining(repository.owner, repository.name){ case (owner, name) =>
       val userName = context.loginAccount.get.userName
 
-      getIssue(owner, name, issueId.toString) flatMap { issue =>
+      val (action, recordActivity) = actionOpt
-        val (action, recordActivity) =
+        .collect {
-          getAction(issue)
+          case "close" if(!issue.closed) => true  ->
-            .collect {
+            (Some("close")  -> Some(if(issue.isPullRequest) recordClosePullRequestActivity _ else recordCloseIssueActivity _))
-              case "close" if(!issue.closed) => true  ->
+          case "reopen" if(issue.closed) => false ->
-                (Some("close")  -> Some(if(issue.isPullRequest) recordClosePullRequestActivity _ else recordCloseIssueActivity _))
+            (Some("reopen") -> Some(recordReopenIssueActivity _))
-              case "reopen" if(issue.closed) => false ->
-                (Some("reopen") -> Some(recordReopenIssueActivity _))
-            }
-            .map { case (closed, t) =>
-              updateClosed(owner, name, issueId, closed)
-              t
-            }
-            .getOrElse(None -> None)
-
-        val commentId = (content, action) match {
-          case (None, None) => None
-          case (None, Some(action)) => Some(createComment(owner, name, userName, issueId, action.capitalize, action))
-          case (Some(content), _) => Some(createComment(owner, name, userName, issueId, content, action.map(_+ "_comment").getOrElse("comment")))
         }
-
+        .map { case (closed, t) =>
-        // record comment activity if comment is entered
+          updateClosed(owner, name, issue.issueId, closed)
-        content foreach {
+          t
-          (if(issue.isPullRequest) recordCommentPullRequestActivity _ else recordCommentIssueActivity _)
-          (owner, name, userName, issueId, _)
         }
-        recordActivity foreach ( _ (owner, name, userName, issueId, issue.title) )
+        .getOrElse(None -> None)
 
-        // extract references and create refer comment
+      val commentId = (content, action) match {
-        content.map { content =>
+        case (None, None) => None
-          createReferComment(owner, name, issue, content, context.loginAccount.get)
+        case (None, Some(action)) => Some(createComment(owner, name, userName, issue.issueId, action.capitalize, action))
-        }
+        case (Some(content), _) => Some(createComment(owner, name, userName, issue.issueId, content, action.map(_+ "_comment").getOrElse("comment")))
-
-        // call web hooks
-        action match {
-          case None => commentId.map{ commentIdSome => callIssueCommentWebHook(repository, issue, commentIdSome, context.loginAccount.get) }
-          case Some(act) => val webHookAction = act match {
-            case "open"   => "opened"
-            case "reopen" => "reopened"
-            case "close"  => "closed"
-            case _ => act
-          }
-            if(issue.isPullRequest){
-              callPullRequestWebHook(webHookAction, repository, issue.issueId, context.baseUrl, context.loginAccount.get)
-            } else {
-              callIssuesWebHook(webHookAction, repository, issue, context.baseUrl, context.loginAccount.get)
-            }
-        }
-
-        // notifications
-        Notifier() match {
-          case f =>
-            content foreach {
-              f.toNotify(repository, issue, _){
-                Notifier.msgComment(s"${context.baseUrl}/${owner}/${name}/${
-                  if(issue.isPullRequest) "pull" else "issues"}/${issueId}#comment-${commentId.get}")
-              }
-            }
-            action foreach {
-              f.toNotify(repository, issue, _){
-                Notifier.msgStatus(s"${context.baseUrl}/${owner}/${name}/issues/${issueId}")
-              }
-            }
-        }
-
-        commentId.map( issue -> _ )
       }
+
+      // record comment activity if comment is entered
+      content foreach {
+        (if(issue.isPullRequest) recordCommentPullRequestActivity _ else recordCommentIssueActivity _)
+        (owner, name, userName, issue.issueId, _)
+      }
+      recordActivity foreach ( _ (owner, name, userName, issue.issueId, issue.title) )
+
+      // extract references and create refer comment
+      content.map { content =>
+        createReferComment(owner, name, issue, content, context.loginAccount.get)
+      }
+
+      // call web hooks
+      action match {
+        case None => commentId.map{ commentIdSome => callIssueCommentWebHook(repository, issue, commentIdSome, context.loginAccount.get) }
+        case Some(act) => val webHookAction = act match {
+          case "open"   => "opened"
+          case "reopen" => "reopened"
+          case "close"  => "closed"
+          case _ => act
+        }
+          if(issue.isPullRequest){
+            callPullRequestWebHook(webHookAction, repository, issue.issueId, context.baseUrl, context.loginAccount.get)
+          } else {
+            callIssuesWebHook(webHookAction, repository, issue, context.baseUrl, context.loginAccount.get)
+          }
+      }
+
+      // notifications
+      Notifier() match {
+        case f =>
+          content foreach {
+            f.toNotify(repository, issue, _){
+              Notifier.msgComment(s"${context.baseUrl}/${owner}/${name}/${
+                if(issue.isPullRequest) "pull" else "issues"}/${issue.issueId}#comment-${commentId.get}")
+            }
+          }
+          action foreach {
+            f.toNotify(repository, issue, _){
+              Notifier.msgStatus(s"${context.baseUrl}/${owner}/${name}/issues/${issue.issueId}")
+            }
+          }
+      }
+
+      commentId.map( issue -> _ )
     }
   }
 
-  private def isEditable(owner: String, repository: String, author: String)(implicit context: Context): Boolean =
-    hasWritePermission(owner, repository, context.loginAccount) || author == context.loginAccount.get.userName
-
-
 }