Refactoring button forms #173

 * Triggering without showing dialog, if no form supplied.
 * Adding DTO:s to properly validate JSON in form.
 * Storing the button form in objects, not a String.
 * Adjusting textarea size in GUI.
 * Supplying form as JSON to GUI, instead of escaped JSON string.

CHANGELOG.md

@@ -4,13 +4,23 @@ Changelog of Pull Request Notifier for Bitbucket.
 
 ## Unreleased
 ### GitHub [#173](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/pull/173) Add the ability to specify forms for buttons
+  Refactoring button forms 
+
+ * Triggering without showing dialog, if no form supplied.
+ * Adding DTO:s to properly validate JSON in form.
+ * Storing the button form in objects, not a String.
+ * Adjusting textarea size in GUI.
+ * Supplying form as JSON to GUI, instead of escaped JSON string.
+
+  [3ea857a88f950fb](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/3ea857a88f950fb) Tomas Bjerre *2016-12-26 19:08:09*
+
   Adjustments after merge of 
 
  * Adding fmt-maven-plugin to format code on Google Java Format.
  * Correcting warnings from Grunt / JSHint.
  * Adding, and using, enum, RENDER_FOR, in PrnfbRenderer.
 
-  [95fcfc4f334fb1e](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/95fcfc4f334fb1e) Tomas Bjerre *2016-12-25 21:29:22*
+  [649638d30d2d32c](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/649638d30d2d32c) Tomas Bjerre *2016-12-25 21:46:25*
 
 ### No issue
   Add interactive forms to buttons

README.md

@@ -128,7 +128,7 @@ A form is defined as a JSON array. Here is an example that shows all possibiliti
         {   "name": "var3",
             "label": "var3 label",
             "defaultValue": "option2_name",
-            "options": [
+            "buttonFormElementOptionList": [
                 {"label": "option1 label", "name": "option1_name"},
                 {"label": "option2 label", "name": "option2_name"},
                 {"label": "option3 label", "name": "option3_name"}
@@ -141,7 +141,7 @@ A form is defined as a JSON array. Here is an example that shows all possibiliti
             "label": "var4 label",
             "type": "checkbox", 
             "required": true,
-            "options": [
+            "buttonFormElementOptionList": [
                 {"label": "option1 label", "name": "option1_name", "defaultValue": true}, 
                 {"label": "option2 label", "name": "option2_name", "defaultValue": true}
             ],

src/main/java/se/bjurr/prnfb/presentation/ButtonServlet.java

@@ -1,15 +1,18 @@
 package se.bjurr.prnfb.presentation;
 
+import static com.google.common.base.Strings.isNullOrEmpty;
 import static javax.ws.rs.core.MediaType.APPLICATION_JSON;
 import static javax.ws.rs.core.Response.ok;
 import static javax.ws.rs.core.Response.status;
 import static javax.ws.rs.core.Response.Status.OK;
 import static javax.ws.rs.core.Response.Status.UNAUTHORIZED;
+import static se.bjurr.prnfb.presentation.dto.ButtonFormType.radio;
 import static se.bjurr.prnfb.transformer.ButtonTransformer.toButtonDto;
 import static se.bjurr.prnfb.transformer.ButtonTransformer.toButtonDtoList;
 import static se.bjurr.prnfb.transformer.ButtonTransformer.toPrnfbButton;
 import static se.bjurr.prnfb.transformer.ButtonTransformer.toTriggerResultDto;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.UUID;
@@ -26,19 +29,22 @@
 import javax.ws.rs.core.Response;
 
 import com.atlassian.annotations.security.XsrfProtectionExcluded;
-import com.google.gson.Gson;
+import com.google.common.annotations.VisibleForTesting;
 
 import se.bjurr.prnfb.http.NotificationResponse;
 import se.bjurr.prnfb.presentation.dto.ButtonDTO;
+import se.bjurr.prnfb.presentation.dto.ButtonFormElementDTO;
 import se.bjurr.prnfb.presentation.dto.ButtonPressDTO;
 import se.bjurr.prnfb.service.ButtonsService;
+import se.bjurr.prnfb.service.PrnfbRenderer.ENCODE_FOR;
+import se.bjurr.prnfb.service.PrnfbRendererWrapper;
 import se.bjurr.prnfb.service.SettingsService;
 import se.bjurr.prnfb.service.UserCheckService;
 import se.bjurr.prnfb.settings.PrnfbButton;
 
 @Path("/settings/buttons")
 public class ButtonServlet {
-  private static final Gson gson = new Gson();
+
   private final ButtonsService buttonsService;
   private final SettingsService settingsService;
   private final UserCheckService userCheckService;
@@ -58,22 +64,12 @@ public ButtonServlet(
   @Produces(APPLICATION_JSON)
   public Response create(ButtonDTO buttonDto) {
     if (!userCheckService.isAdminAllowed( //
-        buttonDto.getProjectKey().orNull() //
-        ,
+        buttonDto.getProjectKey().orNull(), //
         buttonDto.getRepositorySlug().orNull())) {
       return status(UNAUTHORIZED) //
           .build();
     }
 
-    if (buttonDto.getButtonForm() != null && !buttonDto.getButtonForm().isEmpty()) {
-      try {
-        gson.fromJson(buttonDto.getButtonForm(), Object.class);
-        // TODO: Replace string with a DTO
-      } catch (com.google.gson.JsonSyntaxException ex) {
-        throw new Error("The form specification for the button must be a valid JSON string");
-      }
-    }
-
     PrnfbButton prnfbButton = toPrnfbButton(buttonDto);
     PrnfbButton created = settingsService.addOrUpdateButton(prnfbButton);
     ButtonDTO createdDto = toButtonDto(created);
@@ -124,14 +120,7 @@ public Response get(
     List<ButtonDTO> dtos = toButtonDtoList(allowedButtons);
     Collections.sort(dtos);
 
-    for (ButtonDTO dto : dtos) {
-      if (dto.getButtonForm() != null) {
-        // TODO: After replace string with a DTO, only render the defaultType string
-        dto.setButtonForm(
-            buttonsService.getRenderedButtonFormData(
-                repositoryId, pullRequestId, dto.getUuid(), dto.getButtonForm()));
-      }
-    }
+    populateButtonFormDtoList(repositoryId, pullRequestId, dtos);
 
     return ok(dtos, APPLICATION_JSON).build();
   }
@@ -178,6 +167,47 @@ public Response get(@PathParam("uuid") UUID uuid) {
     return ok(dto, APPLICATION_JSON).build();
   }
 
+  @VisibleForTesting
+  List<ButtonFormElementDTO> getButtonFormDTOList(List<ButtonFormElementDTO> buttonFormDtoList)
+      throws Error {
+    if (buttonFormDtoList == null || buttonFormDtoList.isEmpty()) {
+      return new ArrayList<>();
+    }
+    for (ButtonFormElementDTO buttonFormDto : buttonFormDtoList) {
+      if (isNullOrEmpty(buttonFormDto.getLabel())) {
+        throw new Error("The label must be set.");
+      }
+      if (isNullOrEmpty(buttonFormDto.getName())) {
+        throw new Error("The name must be set.");
+      }
+      if (buttonFormDto.getType() == radio
+          && (buttonFormDto.getButtonFormElementOptionList() == null
+              || buttonFormDto.getButtonFormElementOptionList().isEmpty())) {
+        throw new Error("When adding radio buttons, options must also be defined.");
+      }
+    }
+    return buttonFormDtoList;
+  }
+
+  private void populateButtonFormDtoList(
+      Integer repositoryId, Long pullRequestId, List<ButtonDTO> dtos) {
+    for (ButtonDTO dto : dtos) {
+      PrnfbRendererWrapper renderer =
+          buttonsService.getRenderer(repositoryId, pullRequestId, dto.getUuid());
+      List<ButtonFormElementDTO> buttonFormDtoList = dto.getButtonFormList();
+      if (buttonFormDtoList != null) {
+        for (ButtonFormElementDTO buttonFormElementDto : buttonFormDtoList) {
+          String defaultValue = buttonFormElementDto.getDefaultValue();
+          if (!isNullOrEmpty(defaultValue)) {
+            String defaultValueRendered = renderer.render(defaultValue, ENCODE_FOR.NONE);
+            buttonFormElementDto.setDefaultValue(defaultValueRendered);
+          }
+        }
+        dto.setButtonFormList(buttonFormDtoList);
+      }
+    }
+  }
+
   @POST
   @Path("{uuid}/press/repository/{repositoryId}/pullrequest/{pullRequestId}")
   @XsrfProtectionExcluded

src/main/java/se/bjurr/prnfb/presentation/dto/ButtonDTO.java

@@ -2,26 +2,39 @@
 
 import static javax.xml.bind.annotation.XmlAccessType.FIELD;
 
+import java.lang.reflect.Type;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.UUID;
 
 import javax.xml.bind.annotation.XmlAccessorType;
 import javax.xml.bind.annotation.XmlRootElement;
 
-import se.bjurr.prnfb.settings.USER_LEVEL;
-
 import com.google.common.base.Optional;
+import com.google.gson.reflect.TypeToken;
+
+import se.bjurr.prnfb.settings.USER_LEVEL;
 
 @XmlRootElement
 @XmlAccessorType(FIELD)
 public class ButtonDTO implements Comparable<ButtonDTO> {
+  public static Type BUTTON_FORM_LIST_DTO_TYPE =
+      new TypeToken<ArrayList<ButtonFormElementDTO>>() {}.getType();
+
+  private List<ButtonFormElementDTO> buttonFormList;
+  /**
+   * Makes it easier to implement GUI. JSON String representation of {@link #buttonFormList}. If
+   * {@link #buttonFormList} is not defined and {@link #buttonFormListString} is defined, then that
+   * is parsed to {@link #buttonFormList}.
+   */
+  private String buttonFormListString;
 
   private ON_OR_OFF confirmation;
   private String name;
   private String projectKey;
   private String repositorySlug;
   private USER_LEVEL userLevel;
   private UUID uuid;
-  private String buttonForm;
 
   @Override
   public int compareTo(ButtonDTO o) {
@@ -40,54 +53,54 @@ public boolean equals(Object obj) {
       return false;
     }
     ButtonDTO other = (ButtonDTO) obj;
-    if (this.projectKey == null) {
-      if (other.projectKey != null) {
+    if (buttonFormList == null) {
+      if (other.buttonFormList != null) {
         return false;
       }
-    } else if (!this.projectKey.equals(other.projectKey)) {
+    } else if (!buttonFormList.equals(other.buttonFormList)) {
       return false;
     }
-    if (this.repositorySlug == null) {
-      if (other.repositorySlug != null) {
-        return false;
-      }
-    } else if (!this.repositorySlug.equals(other.repositorySlug)) {
+    if (confirmation != other.confirmation) {
       return false;
     }
-    if (this.name == null) {
+    if (name == null) {
       if (other.name != null) {
         return false;
       }
-    } else if (!this.name.equals(other.name)) {
+    } else if (!name.equals(other.name)) {
       return false;
     }
-    if (this.userLevel != other.userLevel) {
-      return false;
-    }
-    if (this.confirmation == null) {
-      if (other.confirmation != null) {
+    if (projectKey == null) {
+      if (other.projectKey != null) {
         return false;
       }
-    } else if (!this.confirmation.equals(other.confirmation)) {
+    } else if (!projectKey.equals(other.projectKey)) {
       return false;
     }
-    if (this.buttonForm == null) {
-      if (other.buttonForm != null) {
+    if (repositorySlug == null) {
+      if (other.repositorySlug != null) {
         return false;
       }
-    } else if (!this.buttonForm.equals(other.buttonForm)) {
+    } else if (!repositorySlug.equals(other.repositorySlug)) {
       return false;
     }
-    if (this.uuid == null) {
+    if (userLevel != other.userLevel) {
+      return false;
+    }
+    if (uuid == null) {
       if (other.uuid != null) {
         return false;
       }
-    } else if (!this.uuid.equals(other.uuid)) {
+    } else if (!uuid.equals(other.uuid)) {
       return false;
     }
     return true;
   }
 
+  public List<ButtonFormElementDTO> getButtonFormList() {
+    return buttonFormList;
+  }
+
   public ON_OR_OFF getConfirmation() {
     return this.confirmation;
   }
@@ -96,10 +109,6 @@ public String getName() {
     return this.name;
   }
 
-  public String getButtonForm() {
-    return this.buttonForm;
-  }
-
   public Optional<String> getProjectKey() {
     return Optional.fromNullable(this.projectKey);
   }
@@ -124,16 +133,28 @@ public UUID getUUID() {
   public int hashCode() {
     final int prime = 31;
     int result = 1;
-    result = prime * result + ((this.projectKey == null) ? 0 : this.projectKey.hashCode());
-    result = prime * result + ((this.repositorySlug == null) ? 0 : this.repositorySlug.hashCode());
-    result = prime * result + ((this.name == null) ? 0 : this.name.hashCode());
-    result = prime * result + ((this.userLevel == null) ? 0 : this.userLevel.hashCode());
-    result = prime * result + ((this.uuid == null) ? 0 : this.uuid.hashCode());
-    result = prime * result + ((this.confirmation == null) ? 0 : this.confirmation.hashCode());
-    result = prime * result + ((this.buttonForm == null) ? 0 : this.buttonForm.hashCode());
+    result = prime * result + (buttonFormList == null ? 0 : buttonFormList.hashCode());
+    result = prime * result + (confirmation == null ? 0 : confirmation.hashCode());
+    result = prime * result + (name == null ? 0 : name.hashCode());
+    result = prime * result + (projectKey == null ? 0 : projectKey.hashCode());
+    result = prime * result + (repositorySlug == null ? 0 : repositorySlug.hashCode());
+    result = prime * result + (userLevel == null ? 0 : userLevel.hashCode());
+    result = prime * result + (uuid == null ? 0 : uuid.hashCode());
     return result;
   }
 
+  public void setButtonFormListString(String buttonFormDtoListString) {
+    this.buttonFormListString = buttonFormDtoListString;
+  }
+
+  public String getButtonFormListString() {
+    return buttonFormListString;
+  }
+
+  public void setButtonFormList(List<ButtonFormElementDTO> buttonFormList) {
+    this.buttonFormList = buttonFormList;
+  }
+
   public void setConfirmation(ON_OR_OFF confirmation) {
     this.confirmation = confirmation;
   }
@@ -142,10 +163,6 @@ public void setName(String name) {
     this.name = name;
   }
 
-  public void setButtonForm(String buttonForm) {
-    this.buttonForm = buttonForm;
-  }
-
   public void setProjectKey(String projectKey) {
     this.projectKey = projectKey;
   }
@@ -164,20 +181,20 @@ public void setUuid(UUID uuid) {
 
   @Override
   public String toString() {
-    return "ButtonDTO [name="
-        + this.name
+    return "ButtonDTO [buttonFormDtoList="
+        + buttonFormList
+        + ", confirmation="
+        + confirmation
+        + ", name="
+        + name
+        + ", projectKey="
+        + projectKey
+        + ", repositorySlug="
+        + repositorySlug
         + ", userLevel="
-        + this.userLevel
+        + userLevel
         + ", uuid="
-        + this.uuid
-        + ", repositorySlug="
-        + this.repositorySlug
-        + ", projectKey="
-        + this.projectKey
-        + ", buttonForm="
-        + this.buttonForm
-        + ", confirmation="
-        + this.confirmation
+        + uuid
         + "]";
   }
 }