Naming triggers #43

CHANGELOG.md

@@ -3,6 +3,7 @@
 Changelog of Pull Request Notifier for Stash.
 
 ## 1.18
+* Triggers can be named. To make it easier to keep track of them in large installations.
 * Building against latest Stash version (3.11.1) using latest Atlassian Maven Plugin Suite version (6.0.3)
 
 ## 1.17

src/main/java/se/bjurr/prnfs/admin/AdminFormValues.java

@@ -14,7 +14,9 @@ public final class AdminFormValues extends ArrayList<Map<String, String>> {
  private static final long serialVersionUID = 9084184120202816120L;
  public static final String VALUE = "value";
 
+ public static final String DEFAULT_NAME = "Unnamed trigger";
+
  public enum FIELDS {
-  user, password, events, FORM_IDENTIFIER, url, filter_string, filter_regexp, method, post_content, proxy_user, proxy_password, proxy_server, proxy_port, header_name, header_value
+  user, password, events, FORM_IDENTIFIER, url, filter_string, filter_regexp, method, post_content, proxy_user, proxy_password, proxy_server, proxy_port, header_name, header_value, name
  }
 }

src/main/java/se/bjurr/prnfs/listener/PrnfsPullRequestEventListener.java

@@ -135,7 +135,7 @@ public void handleEvent(PullRequestEvent pullRequestEvent) {
       postContent = Optional.of(renderer.render(notification.getPostContent().get()));
      }
      String renderedUrl = renderer.render(notification.getUrl());
-     logger.info(action.getName() + " "//
+     logger.info(notification.getName() + " > " + action.getName() + " "//
        + pr.getFromRef().getId() + "(" + pr.getFromRef().getLatestChangeset() + ") -> " //
        + pr.getToRef().getId() + "(" + pr.getToRef().getLatestChangeset() + ")" + " " //
        + renderedUrl);

src/main/java/se/bjurr/prnfs/settings/PrnfsNotification.java

@@ -6,6 +6,7 @@
 import static com.google.common.base.Strings.emptyToNull;
 import static com.google.common.base.Strings.nullToEmpty;
 import static java.util.regex.Pattern.compile;
+import static se.bjurr.prnfs.admin.AdminFormValues.DEFAULT_NAME;
 
 import java.net.URL;
 import java.util.List;
@@ -29,10 +30,11 @@ public class PrnfsNotification {
  private final String proxyPassword;
  private final String proxyServer;
  private final Integer proxyPort;
+ private final String name;
 
  public PrnfsNotification(List<PrnfsPullRequestAction> triggers, String url, String user, String password,
    String filterString, String filterRegexp, String method, String postContent, List<Header> headers, String proxyUser,
-   String proxyPassword, String proxyServer, String proxyPort) throws ValidationException {
+   String proxyPassword, String proxyServer, String proxyPort, String name) throws ValidationException {
   this.proxyUser = emptyToNull(nullToEmpty(proxyUser).trim());
   this.proxyPassword = emptyToNull(nullToEmpty(proxyPassword).trim());
   this.proxyServer = emptyToNull(nullToEmpty(proxyServer).trim());
@@ -66,6 +68,7 @@ public PrnfsNotification(List<PrnfsPullRequestAction> triggers, String url, Stri
   this.triggers = checkNotNull(triggers);
   this.filterString = filterString;
   this.filterRegexp = filterRegexp;
+  this.name = firstNonNull(emptyToNull(nullToEmpty(name).trim()), DEFAULT_NAME);
  }
 
  public Optional<String> getFilterRegexp() {
@@ -96,6 +99,10 @@ public Optional<String> getProxyUser() {
   return fromNullable(proxyUser);
  }
 
+ public String getName() {
+  return name;
+ }
+
  public List<PrnfsPullRequestAction> getTriggers() {
   return triggers;
  }

src/main/java/se/bjurr/prnfs/settings/PrnfsNotificationBuilder.java

@@ -28,13 +28,14 @@ public static PrnfsNotificationBuilder prnfsNotificationBuilder() {
  private String proxyPassword;
  private String proxyServer;
  private String proxyPort;
+ private String name;
 
  private PrnfsNotificationBuilder() {
  }
 
  public PrnfsNotification build() throws ValidationException {
   return new PrnfsNotification(triggers, url, user, password, filterString, filterRegexp, method, postContent, headers,
-    proxyUser, proxyPassword, proxyServer, proxyPort);
+    proxyUser, proxyPassword, proxyServer, proxyPort, name);
  }
 
  public PrnfsNotificationBuilder withPassword(String password) {
@@ -101,4 +102,9 @@ public PrnfsNotificationBuilder withProxyPassword(String s) {
   this.proxyPassword = checkNotNull(s);
   return this;
  }
+
+ public PrnfsNotificationBuilder withName(String name) {
+  this.name = name;
+  return this;
+ }
 }

src/main/java/se/bjurr/prnfs/settings/SettingsStorage.java

@@ -9,6 +9,7 @@
 import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Maps.newTreeMap;
 import static java.lang.System.currentTimeMillis;
+import static se.bjurr.prnfs.admin.AdminFormValues.DEFAULT_NAME;
 import static se.bjurr.prnfs.admin.AdminFormValues.NAME;
 import static se.bjurr.prnfs.admin.AdminFormValues.VALUE;
 import static se.bjurr.prnfs.settings.PrnfsNotificationBuilder.prnfsNotificationBuilder;
@@ -34,10 +35,6 @@
 import com.google.gson.Gson;
 
 public class SettingsStorage {
- /**
-  * Every form should have a field with this name, with a unique value.
-  */
- public static final String FORM_IDENTIFIER_NAME = "FORM_IDENTIFIER";
 
  private static final Gson gson = new Gson();
  private static Logger logger = LoggerFactory.getLogger(SettingsStorage.class);
@@ -73,8 +70,8 @@ public static Logger getLogger() {
  private static Map<String, AdminFormValues> getNotificationsMap(PluginSettings pluginSettings) {
   final Map<String, AdminFormValues> allNotificationsMap = newTreeMap();
   for (final AdminFormValues a : getSettingsAsFormValues(pluginSettings)) {
-   if (tryFind(a, predicate(FORM_IDENTIFIER_NAME)).isPresent()) {
-    allNotificationsMap.put(find(a, predicate(FORM_IDENTIFIER_NAME)).get(VALUE), a);
+   if (tryFind(a, predicate(AdminFormValues.FIELDS.FORM_IDENTIFIER.name())).isPresent()) {
+    allNotificationsMap.put(find(a, predicate(AdminFormValues.FIELDS.FORM_IDENTIFIER.name())).get(VALUE), a);
    }
   }
   return allNotificationsMap;
@@ -152,6 +149,9 @@ public static PrnfsNotification getPrnfsNotification(AdminFormValues adminFormVa
    prnfsNotificationBuilder
      .withPostContent(find(adminFormValues, predicate(AdminFormValues.FIELDS.post_content.name())).get(VALUE));
   }
+  if (tryFind(adminFormValues, predicate(AdminFormValues.FIELDS.name.name())).isPresent()) {
+   prnfsNotificationBuilder.withName(find(adminFormValues, predicate(AdminFormValues.FIELDS.name.name())).get(VALUE));
+  }
   return prnfsNotificationBuilder.build();
  }
 
@@ -172,14 +172,29 @@ public static List<AdminFormValues> getSettingsAsFormValues(PluginSettings setti
    @SuppressWarnings("unchecked")
    final List<String> settingsList = newArrayList((List<String>) settings.get(STORAGE_KEY));
    for (final String storedJson : settingsList) {
-    toReturn.add(gson.fromJson(storedJson, AdminFormValues.class));
+    toReturn.add(injectConfigurationName(gson.fromJson(storedJson, AdminFormValues.class)));
    }
   } catch (final Exception e) {
    logger.error("Unable to deserialize settings", e);
   }
   return toReturn;
  }
 
+ /**
+  * Inject a default name for the trigger. To make the plugin backwards
+  * compatible.
+  */
+ private static AdminFormValues injectConfigurationName(AdminFormValues adminFormValues) {
+  final Optional<Map<String, String>> nameMapOpt = tryFind(adminFormValues,
+    predicate(AdminFormValues.FIELDS.name.name()));
+  if (nameMapOpt.isPresent()) {
+   return adminFormValues;
+  }
+  adminFormValues.add(ImmutableMap.<String, String> builder().put(NAME, AdminFormValues.FIELDS.name.name())
+    .put(VALUE, DEFAULT_NAME).build());
+  return adminFormValues;
+ }
+
  @VisibleForTesting
  public static void setLogger(Logger loggerParam) {
   logger = loggerParam;
@@ -189,9 +204,10 @@ private static void storeNotificationsMap(PluginSettings pluginSettings,
    Map<String, AdminFormValues> allNotificationsMap) throws ValidationException {
   final List<String> toStore = newArrayList();
   for (final AdminFormValues adminFormValues : allNotificationsMap.values()) {
-   final Optional<Map<String, String>> formIdOpt = tryFind(adminFormValues, predicate(FORM_IDENTIFIER_NAME));
+   final Optional<Map<String, String>> formIdOpt = tryFind(adminFormValues,
+     predicate(AdminFormValues.FIELDS.FORM_IDENTIFIER.name()));
    if (!formIdOpt.isPresent() || formIdOpt.get().get(VALUE).trim().isEmpty()) {
-    throw new ValidationException(FORM_IDENTIFIER_NAME, "Not set!");
+    throw new ValidationException(AdminFormValues.FIELDS.FORM_IDENTIFIER.name(), "Not set!");
    }
    toStore.add(new Gson().toJson(adminFormValues));
   }
@@ -202,15 +218,16 @@ public static void storeSettings(PluginSettings pluginSettings, final AdminFormV
    throws ValidationException {
   final Map<String, AdminFormValues> allNotificationsMap = getNotificationsMap(pluginSettings);
 
-  final Optional<Map<String, String>> formIdOpt = tryFind(config, predicate(FORM_IDENTIFIER_NAME));
+  final Optional<Map<String, String>> formIdOpt = tryFind(config,
+    predicate(AdminFormValues.FIELDS.FORM_IDENTIFIER.name()));
   if (!formIdOpt.isPresent() || formIdOpt.get().get(VALUE).trim().isEmpty()) {
    final String generatedIdentifier = formIdentifierGnerator();
-   removeIf(config, predicate(FORM_IDENTIFIER_NAME));
-   config.add(new ImmutableMap.Builder<String, String>().put(NAME, FORM_IDENTIFIER_NAME)
+   removeIf(config, predicate(AdminFormValues.FIELDS.FORM_IDENTIFIER.name()));
+   config.add(new ImmutableMap.Builder<String, String>().put(NAME, AdminFormValues.FIELDS.FORM_IDENTIFIER.name())
      .put(VALUE, generatedIdentifier).build());
   }
 
-  allNotificationsMap.put(find(config, predicate(FORM_IDENTIFIER_NAME)).get(VALUE), config);
+  allNotificationsMap.put(find(config, predicate(AdminFormValues.FIELDS.FORM_IDENTIFIER.name())).get(VALUE), config);
 
   storeNotificationsMap(pluginSettings, allNotificationsMap);
  }

src/main/resources/admin.css

@@ -57,3 +57,11 @@ td > input[type="text"] {
  margin: 0 10% 0 0;
  float:left
 }
+
+.expandable .content {
+ display: none;
+}
+
+.expandable.expanded .content {
+ display: inherit;
+}

src/main/resources/admin.js

@@ -45,6 +45,17 @@
      });
    });
 
+   $('.expandable').each(function(index, el) {
+    var $element = $(el);
+    $element.find('.toggle').click(function() {
+     $element.toggleClass('expanded');
+    });
+   });
+   //If there are only a few triggers configured, they can be expanded by default without confusion.
+   if ($('.expandable').length < 4) {
+    $('.expandable').addClass('expanded');
+   }
+
    $('.headers').keyup(function(e) {
      var $headers = $(this);
      adjustHeaders($headers);
@@ -79,6 +90,7 @@
    var $template = $(".prnfs-template").clone();
    $('input[name="delete"]',$template).remove();
    $('input[name=method][value=GET]', $template).attr('checked','checked');
+   $('.expandable',$template).addClass('expanded');
    $(".prnfs").append($template.html());
   }
 
@@ -91,6 +103,7 @@
     $.each(configs, function(index, config) {
      var $template = $(".prnfs-template").clone();
      $.each(config, function(fieldIndex,field_map) {
+      $('.variable[data-variable="'+field_map.name+'"]', $template).html(field_map.value);
       $('input[type="text"][name="'+field_map.name+'"]', $template).attr('value', field_map.value);
       $('input[type="password"][name="'+field_map.name+'"]', $template).attr('value', field_map.value);
       $('textarea[name="'+field_map.name+'"]', $template).text(field_map.value);

src/main/resources/admin.vm

@@ -51,8 +51,20 @@
     </div>
 
     <div class="prnfs-template">
-      <form class="trigger">
+     <form class="trigger">
+      <div class="expandable">
+       <div class="title">
+        <a href="javascript:void(0);" class="variable toggle" data-variable="name"></a>
+       </div>
+       <div class="content">
         <input type="hidden" name="FORM_IDENTIFIER">
+        <div>
+          <fieldset>
+            <legend>Name</legend>
+            <input type="text" name="name" value="Unnamed trigger">
+            <label for="name">Name of this trigger. The name will show up in the log when trigger is executed. Other then that, this is just a feature to help you keep track of your triggers.</label><br/>
+          </fieldset>
+        </div>
         <div>
           <fieldset>
             <legend>Filter (Optional). Variables above are available.</legend>
@@ -180,7 +192,9 @@
             <span class="post"></span>
           </fieldset>
         </div>
-      </form>
+       </div>
+      </div>
+     </form>
     </div>
   </body>
 </html>

src/test/java/se/bjurr/prnfs/admin/NotificationsStorageTest.java

@@ -1,6 +1,7 @@
 package se.bjurr.prnfs.admin;
 
 import static com.atlassian.stash.pull.PullRequestAction.OPENED;
+import static se.bjurr.prnfs.admin.AdminFormValues.DEFAULT_NAME;
 import static se.bjurr.prnfs.admin.utils.NotificationBuilder.notificationBuilder;
 import static se.bjurr.prnfs.admin.utils.PrnfsTestBuilder.prnfsTestBuilder;
 
@@ -15,6 +16,7 @@ public void testThatANewNotificationCanBeStored() {
       notificationBuilder().withFieldValue(AdminFormValues.FIELDS.url, "http://bjurr.se/")
         .withFieldValue(AdminFormValues.FIELDS.events, OPENED.name()).build()).store().hasNotifications(1)
     .hasFieldValueAt(AdminFormValues.FIELDS.url, "http://bjurr.se/", "0")
+    .hasFieldValueAt(AdminFormValues.FIELDS.name, DEFAULT_NAME, "0")
     .hasNoneEmptyFieldAt(AdminFormValues.FIELDS.FORM_IDENTIFIER, "0");
  }
 

src/test/java/se/bjurr/prnfs/admin/utils/PrnfsTestBuilder.java

@@ -19,7 +19,6 @@
 import static se.bjurr.prnfs.listener.PrnfsPullRequestEventListener.setInvoker;
 import static se.bjurr.prnfs.listener.UrlInvoker.getHeaderValue;
 import static se.bjurr.prnfs.settings.PrnfsPredicates.predicate;
-import static se.bjurr.prnfs.settings.SettingsStorage.FORM_IDENTIFIER_NAME;
 import static se.bjurr.prnfs.settings.SettingsStorage.fakeRandom;
 
 import java.util.List;
@@ -162,7 +161,7 @@ private Map<String, AdminFormValues> getAdminFormFields() {
     new Function<AdminFormValues, String>() {
      @Override
      public String apply(AdminFormValues input) {
-      return find(input, predicate(FORM_IDENTIFIER_NAME)).get(VALUE);
+      return find(input, predicate(AdminFormValues.FIELDS.FORM_IDENTIFIER.name())).get(VALUE);
      }
     });
  }
@@ -273,7 +272,8 @@ public PullRequestEventBuilder triggerPullRequestEventBuilder() {
  }
 
  public PrnfsTestBuilder withNotification(AdminFormValues adminFormValues) {
-  final Optional<Map<String, String>> existing = tryFind(adminFormValues, predicate(FORM_IDENTIFIER_NAME));
+  final Optional<Map<String, String>> existing = tryFind(adminFormValues,
+    predicate(AdminFormValues.FIELDS.FORM_IDENTIFIER.name()));
   if (existing.isPresent()) {
    this.adminFormValuesMap.put(existing.get().get(VALUE), adminFormValues);
   } else {