From 40caa1042c873ea1ed0cf23dd528176e56529d77 Mon Sep 17 00:00:00 2001
From: Naoki Takezoe <takezoe@gmail.com>
Date: Sun, 17 Apr 2022 16:32:09 +0900
Subject: [PATCH] Enhance custom field validation

---
 .../core/controller/IssuesController.scala    |  27 ++---
 .../gitbucket/core/model/CustomField.scala    | 108 +++++++++++-------
 .../gitbucket/core/issues/create.scala.html   |  13 ++-
 .../core/issues/issueinfo.scala.html          |   4 +-
 4 files changed, 93 insertions(+), 59 deletions(-)

diff --git a/src/main/scala/gitbucket/core/controller/IssuesController.scala b/src/main/scala/gitbucket/core/controller/IssuesController.scala
index 41308764ef..3ee85c8b2f 100644
--- a/src/main/scala/gitbucket/core/controller/IssuesController.scala
+++ b/src/main/scala/gitbucket/core/controller/IssuesController.scala
@@ -156,15 +156,12 @@ trait IssuesControllerBase extends ControllerBase {
           params.toMap.foreach {
             case (key, value) =>
               if (key.startsWith("custom-field-")) {
-                for {
-                  field <- getCustomField(
-                    repository.owner,
-                    repository.name,
-                    key.replaceFirst("^custom-field-", "").toInt
-                  )
-                  behavior <- CustomFieldBehavior(field.fieldType)
-                } {
-                  behavior.validate(field.fieldName, value, messages) match {
+                getCustomField(
+                  repository.owner,
+                  repository.name,
+                  key.replaceFirst("^custom-field-", "").toInt
+                ).foreach { field =>
+                  CustomFieldBehavior.validate(field, value, messages) match {
                     case None =>
                       insertOrUpdateCustomFieldValue(field, repository.owner, repository.name, issue.issueId, value)
                     case Some(_) => halt(400)
@@ -388,16 +385,13 @@ trait IssuesControllerBase extends ControllerBase {
     Ok("updated")
   })
 
-  ajaxPost("/:owner/:repository/issues/:id/customfield_validation/:fieldId")(writableUsersOnly { repository =>
+  ajaxPost("/:owner/:repository/issues/customfield_validation/:fieldId")(writableUsersOnly { repository =>
     val fieldId = params("fieldId").toInt
     val value = params("value")
     getCustomField(repository.owner, repository.name, fieldId)
       .flatMap { field =>
-        CustomFieldBehavior(field.fieldType).map { behavior =>
-          behavior.validate(field.fieldName, value, messages) match {
-            case None        => Ok()
-            case Some(error) => Ok(error)
-          }
+        CustomFieldBehavior.validate(field, value, messages).map { error =>
+          Ok(error)
         }
       }
       .getOrElse(Ok())
@@ -411,9 +405,8 @@ trait IssuesControllerBase extends ControllerBase {
     for {
       _ <- getIssue(repository.owner, repository.name, issueId.toString)
       field <- getCustomField(repository.owner, repository.name, fieldId)
-      behavior <- CustomFieldBehavior(field.fieldType)
     } {
-      behavior.validate(field.fieldName, value, messages) match {
+      CustomFieldBehavior.validate(field, value, messages) match {
         case None    => insertOrUpdateCustomFieldValue(field, repository.owner, repository.name, issueId, value)
         case Some(_) => halt(400)
       }
diff --git a/src/main/scala/gitbucket/core/model/CustomField.scala b/src/main/scala/gitbucket/core/model/CustomField.scala
index 2f1e21b6d0..d25c853d7b 100644
--- a/src/main/scala/gitbucket/core/model/CustomField.scala
+++ b/src/main/scala/gitbucket/core/model/CustomField.scala
@@ -37,7 +37,7 @@ case class CustomField(
 )
 
 trait CustomFieldBehavior {
-  def createHtml(fieldId: Int): String
+  def createHtml(repository: RepositoryInfo, fieldId: Int)(implicit conext: Context): String
   def fieldHtml(repository: RepositoryInfo, issueId: Int, fieldId: Int, value: String, editable: Boolean)(
     implicit context: Context
   ): String
@@ -45,6 +45,15 @@ trait CustomFieldBehavior {
 }
 
 object CustomFieldBehavior {
+  def validate(field: CustomField, value: String, messages: Messages): Option[String] = {
+    if (value.isEmpty) None
+    else {
+      CustomFieldBehavior(field.fieldType).flatMap { behavior =>
+        behavior.validate(field.fieldName, value, messages)
+      }
+    }
+  }
+
   def apply(fieldType: String): Option[CustomFieldBehavior] = {
     fieldType match {
       case "long"   => Some(LongFieldBehavior)
@@ -83,8 +92,29 @@ object CustomFieldBehavior {
   trait TextFieldBehavior extends CustomFieldBehavior {
     protected val fieldType = "text"
 
-    def createHtml(fieldId: Int): String = {
-      s"""<input type="$fieldType" class="form-control input-sm" id="custom-field-$fieldId" name="custom-field-$fieldId" style="width: 120px;"/>"""
+    def createHtml(repository: RepositoryInfo, fieldId: Int)(implicit context: Context): String = {
+      val sb = new StringBuilder
+      sb.append(
+        s"""<input type="$fieldType" class="form-control input-sm" id="custom-field-$fieldId" name="custom-field-$fieldId" data-field-id="$fieldId" style="width: 120px;"/>"""
+      )
+      sb.append(s"""<script>
+                   |$$('#custom-field-$fieldId').focusout(function(){
+                   |  const $$this = $$(this);
+                   |  const fieldId = $$this.data('field-id');
+                   |  $$.post('${helpers.url(repository)}/issues/customfield_validation/' + fieldId,
+                   |    { value: $$this.val() },
+                   |    function(data){
+                   |      if (data != '') {
+                   |        $$('#custom-field-$fieldId-error').text(data);
+                   |      } else {
+                   |        $$('#custom-field-$fieldId-error').text('');
+                   |      }
+                   |    }
+                   |  );
+                   |});
+                   |</script>
+                   |""".stripMargin)
+      sb.toString()
     }
 
     def fieldHtml(repository: RepositoryInfo, issueId: Int, fieldId: Int, value: String, editable: Boolean)(
@@ -100,45 +130,45 @@ object CustomFieldBehavior {
           s"""<input type="$fieldType" id="custom-field-$fieldId-editor" class="form-control input-sm custom-field-editor" data-field-id="$fieldId" style="width: 120px; display: none;"/>"""
         )
         sb.append(s"""<script>
-            |    $$('#custom-field-$fieldId-label').click(function(){
-            |      const $$this = $$(this);
-            |      $$this.hide();
-            |      $$this.next().val($$this.text()).show().focus();
-            |    });
+            |$$('#custom-field-$fieldId-label').click(function(){
+            |  const $$this = $$(this);
+            |  $$this.hide();
+            |  $$this.next().val($$this.text()).show().focus();
+            |});
             |
-            |    $$('#custom-field-$fieldId-editor').focusout(function(){
-            |      const $$this = $$(this);
-            |      const fieldId = $$this.data('field-id');
-            |      $$.post('${helpers.url(repository)}/issues/$issueId/customfield_validation/' + fieldId,
-            |        { value: $$this.val() },
-            |        function(data){
-            |          if (data != '') {
-            |            $$('#custom-field-$fieldId-error').text(data);
-            |          } else {
-            |            $$('#custom-field-$fieldId-error').text('');
-            |            $$.post('${helpers.url(repository)}/issues/$issueId/customfield/' + fieldId,
-            |              { value: $$this.val() },
-            |              function(data){
-            |                $$this.hide();
-            |                $$this.prev().text(data).show();
-            |              }
-            |            );
+            |$$('#custom-field-$fieldId-editor').focusout(function(){
+            |  const $$this = $$(this);
+            |  const fieldId = $$this.data('field-id');
+            |  $$.post('${helpers.url(repository)}/issues/customfield_validation/' + fieldId,
+            |    { value: $$this.val() },
+            |    function(data){
+            |      if (data != '') {
+            |        $$('#custom-field-$fieldId-error').text(data);
+            |      } else {
+            |        $$('#custom-field-$fieldId-error').text('');
+            |        $$.post('${helpers.url(repository)}/issues/$issueId/customfield/' + fieldId,
+            |          { value: $$this.val() },
+            |          function(data){
+            |            $$this.hide();
+            |            $$this.prev().text(data).show();
             |          }
-            |        }
-            |      );
-            |    });
-            |
-            |    // ESC key handling in text field
-            |    $$('#custom-field-$fieldId-editor').keyup(function(e){
-            |      if (e.keyCode == 27) {
-            |        const $$this = $$(this);
-            |        $$this.hide();
-            |        $$this.prev().show();
+            |        );
             |      }
-            |      if (e.keyCode == 13) {
-            |        $$('#custom-field-$fieldId-editor').blur();
-            |      }
-            |    });
+            |    }
+            |  );
+            |});
+            |
+            |// ESC key handling in text field
+            |$$('#custom-field-$fieldId-editor').keyup(function(e){
+            |  if (e.keyCode == 27) {
+            |    const $$this = $$(this);
+            |    $$this.hide();
+            |    $$this.prev().show();
+            |  }
+            |  if (e.keyCode == 13) {
+            |    $$('#custom-field-$fieldId-editor').blur();
+            |  }
+            |});
             |</script>
             |""".stripMargin)
       }
diff --git a/src/main/twirl/gitbucket/core/issues/create.scala.html b/src/main/twirl/gitbucket/core/issues/create.scala.html
index 35a73fb08e..045acbefad 100644
--- a/src/main/twirl/gitbucket/core/issues/create.scala.html
+++ b/src/main/twirl/gitbucket/core/issues/create.scala.html
@@ -28,7 +28,7 @@
             elastic            = true
           )
           <div class="align-right">
-            <input type="submit" class="btn btn-success" value="Submit new issue"/>
+            <input type="submit" class="btn btn-success" value="Submit new issue" onclick="javascript:return checkCustomFieldErrors();"/>
           </div>
         </div>
         <div class="col-md-3">
@@ -48,5 +48,16 @@
         </div>
       </div>
     </form>
+    <script>
+      function checkCustomFieldErrors() {
+        let error = false;
+        $('.custom-field-error').each(function(i, e) {
+          if ($(e).text() != '') {
+            error = true;
+          }
+        });
+        return !error;
+      }
+    </script>
   }
 }
diff --git a/src/main/twirl/gitbucket/core/issues/issueinfo.scala.html b/src/main/twirl/gitbucket/core/issues/issueinfo.scala.html
index e7350da646..d1de4c3a85 100644
--- a/src/main/twirl/gitbucket/core/issues/issueinfo.scala.html
+++ b/src/main/twirl/gitbucket/core/issues/issueinfo.scala.html
@@ -160,12 +160,12 @@
           @Html(behavior.fieldHtml(repository, issue.get.issueId, field.fieldId, value.map(_.value).getOrElse(""), isManageable))
         }
         @if(issue.isEmpty) {
-          @Html(behavior.createHtml(field.fieldId))
+          @Html(behavior.createHtml(repository, field.fieldId))
         }
       }
     </div>
   </div>
-  <span id="custom-field-@field.fieldId-error" class="error"></span>
+  <span id="custom-field-@field.fieldId-error" class="error custom-field-error"></span>
 }
 
 @issue.map { issue =>