Remove custom stylesheet in jelly files. Use standard select for NodeJS installations (#181)

Author: nfalco79

src/main/java/jenkins/plugins/nodejs/NodeJSBuildWrapper.java

@@ -23,6 +23,9 @@
  */
 package jenkins.plugins.nodejs;
 
+import edu.umd.cs.findbugs.annotations.CheckForNull;
+import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.annotations.Nullable;
 import hudson.AbortException;
 import hudson.EnvVars;
 import hudson.Extension;
@@ -32,6 +35,7 @@
 import hudson.console.ConsoleLogFilter;
 import hudson.model.AbstractProject;
 import hudson.model.Computer;
+import hudson.model.Item;
 import hudson.model.ItemGroup;
 import hudson.model.Node;
 import hudson.model.Run;
@@ -51,9 +55,6 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Pattern;
-import edu.umd.cs.findbugs.annotations.CheckForNull;
-import edu.umd.cs.findbugs.annotations.NonNull;
-import edu.umd.cs.findbugs.annotations.Nullable;
 import jenkins.plugins.nodejs.cache.CacheLocationLocator;
 import jenkins.plugins.nodejs.cache.DefaultCacheLocationLocator;
 import jenkins.plugins.nodejs.tools.NodeJSInstallation;
@@ -68,6 +69,7 @@
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.DataBoundSetter;
 import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.verb.POST;
 
 /**
  * A simple build wrapper that contribute the NodeJS bin path to the PATH
@@ -254,8 +256,15 @@ public String getDisplayName() {
             return Messages.NodeJSBuildWrapper_displayName();
         }
 
-        public NodeJSInstallation[] getInstallations() {
-            return NodeJSUtils.getInstallations();
+        /**
+         * Returns all tools defined in the tool page.
+         *
+         * @param item context against check permission
+         * @return a collection of tools name.
+         */
+        @POST
+        public ListBoxModel doFillNodeJSInstallationNameItems(@Nullable @AncestorInPath Item item) {
+            return NodeJSDescriptorUtils.getNodeJSInstallations(item, false);
         }
 
         /**
@@ -264,6 +273,7 @@ public NodeJSInstallation[] getInstallations() {
          * @param context where lookup
          * @return a collection of user npmrc files.
          */
+        @POST
         public ListBoxModel doFillConfigIdItems(@AncestorInPath ItemGroup<?> context) {
             return NodeJSDescriptorUtils.getConfigs(context);
         }
@@ -275,7 +285,8 @@ public ListBoxModel doFillConfigIdItems(@AncestorInPath ItemGroup<?> context) {
          * @param configId the identifier of an npmrc file
          * @return an validation form for the given npmrc file identifier.
          */
-        public FormValidation doCheckConfigId(@Nullable @AncestorInPath ItemGroup<?> context, @CheckForNull @QueryParameter final String configId) {
+        public FormValidation doCheckConfigId(@Nullable @AncestorInPath ItemGroup<?> context,
+                                              @CheckForNull @QueryParameter final String configId) {
             return NodeJSDescriptorUtils.checkConfig(context, configId);
         }
 
@@ -294,7 +305,7 @@ private static final class SecretFilter extends ConsoleLogFilter implements Seri
         }
 
         @Override
-        public OutputStream decorateLogger(Run build, OutputStream logger) {
+        public OutputStream decorateLogger(@SuppressWarnings("rawtypes") Run build, OutputStream logger) {
             return new SecretPatterns.MaskingOutputStream(logger, () -> Pattern.compile(pattern.getPlainText()), charset);
         }
 

src/main/java/jenkins/plugins/nodejs/NodeJSCommandInterpreter.java

@@ -23,21 +23,8 @@
  */
 package jenkins.plugins.nodejs;
 
-import java.io.IOException;
-import java.util.ArrayList;
-
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.Nullable;
-
-import org.jenkinsci.Symbol;
-import org.jenkinsci.lib.configprovider.model.ConfigFile;
-import org.jenkinsci.lib.configprovider.model.ConfigFileManager;
-import org.jenkinsci.plugins.configfiles.common.CleanTempFilesAction;
-import org.kohsuke.stapler.AncestorInPath;
-import org.kohsuke.stapler.DataBoundConstructor;
-import org.kohsuke.stapler.DataBoundSetter;
-import org.kohsuke.stapler.QueryParameter;
-
 import hudson.AbortException;
 import hudson.EnvVars;
 import hudson.Extension;
@@ -47,6 +34,7 @@
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractProject;
 import hudson.model.Environment;
+import hudson.model.Item;
 import hudson.model.ItemGroup;
 import hudson.model.Node;
 import hudson.model.TaskListener;
@@ -56,10 +44,21 @@
 import hudson.util.ArgumentListBuilder;
 import hudson.util.FormValidation;
 import hudson.util.ListBoxModel;
+import java.io.IOException;
+import java.util.ArrayList;
 import jenkins.plugins.nodejs.cache.CacheLocationLocator;
 import jenkins.plugins.nodejs.cache.DefaultCacheLocationLocator;
 import jenkins.plugins.nodejs.tools.NodeJSInstallation;
 import jenkins.plugins.nodejs.tools.Platform;
+import org.jenkinsci.Symbol;
+import org.jenkinsci.lib.configprovider.model.ConfigFile;
+import org.jenkinsci.lib.configprovider.model.ConfigFileManager;
+import org.jenkinsci.plugins.configfiles.common.CleanTempFilesAction;
+import org.kohsuke.stapler.AncestorInPath;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.DataBoundSetter;
+import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.verb.POST;
 
 /**
  * This class executes a JavaScript file using node. The file should contain
@@ -276,25 +275,23 @@ public String getDisplayName() {
         }
 
         /**
-         * Return the help file.
+         * Returns all tools defined in the tool page.
          *
-         * @return the help file URL path
+         * @param item context against check permission
+         * @return a collection of tools name.
          */
-        @Override
-        public String getHelpFile() {
-            return "/plugin/nodejs/help.html";
-        }
-
-        public NodeJSInstallation[] getInstallations() {
-            return NodeJSUtils.getInstallations();
+        @POST
+        public ListBoxModel doFillNodeJSInstallationNameItems(@Nullable @AncestorInPath Item item) {
+            return NodeJSDescriptorUtils.getNodeJSInstallations(item, true);
         }
 
         /**
          * Gather all defined npmrc config files.
          *
-         * @param context where loopup
+         * @param context where lookup
          * @return a collection of user npmrc files.
          */
+        @POST
         public ListBoxModel doFillConfigIdItems(@AncestorInPath ItemGroup<?> context) {
             return NodeJSDescriptorUtils.getConfigs(context);
         }
@@ -306,6 +303,7 @@ public ListBoxModel doFillConfigIdItems(@AncestorInPath ItemGroup<?> context) {
          * @param configId the identifier of an npmrc file
          * @return an validation form for the given npmrc file identifier.
          */
+        @POST
         public FormValidation doCheckConfigId(@Nullable @AncestorInPath ItemGroup<?> context, @CheckForNull @QueryParameter final String configId) {
             return NodeJSDescriptorUtils.checkConfig(context, configId);
         }

src/main/java/jenkins/plugins/nodejs/NodeJSDescriptorUtils.java

@@ -29,12 +29,13 @@
 
 import org.jenkinsci.lib.configprovider.model.Config;
 import org.jenkinsci.plugins.configfiles.ConfigFiles;
-
+import hudson.model.Item;
 import hudson.model.ItemGroup;
 import hudson.util.FormValidation;
 import hudson.util.ListBoxModel;
 import jenkins.plugins.nodejs.configfiles.NPMConfig;
 import jenkins.plugins.nodejs.configfiles.NPMConfig.NPMConfigProvider;
+import jenkins.plugins.nodejs.tools.NodeJSInstallation;
 import jenkins.plugins.nodejs.configfiles.VerifyConfigProviderException;
 
 /*package*/ final class NodeJSDescriptorUtils {
@@ -61,7 +62,7 @@ public static ListBoxModel getConfigs(@Nullable ItemGroup<?> context) {
 
     /**
      * Verify that the given configId exists in the given context.
-     * 
+     *
      * @param context where lookup
      * @param configId the identifier of an npmrc file
      * @return an validation form for the given npmrc file identifier, otherwise
@@ -82,4 +83,18 @@ public static FormValidation checkConfig(@Nullable ItemGroup<?> context, @CheckF
         return FormValidation.ok();
     }
 
+    public static ListBoxModel getNodeJSInstallations(@Nullable Item item, boolean allowDefault) {
+        ListBoxModel items = new ListBoxModel();
+        if (item == null || !item.hasPermission(Item.CONFIGURE)) {
+            return items;
+        }
+        if (allowDefault) {
+            items.add(Messages.NodeJSInstallation_default(), "");
+        }
+        for (NodeJSInstallation tool : NodeJSUtils.getInstallations()) {
+            items.add(tool.getName(), tool.getName());
+        }
+        return items;
+    }
+
 }

src/main/resources/jenkins/plugins/nodejs/Messages.properties

@@ -26,6 +26,7 @@ NodeJSInstaller.FailedToInstallNodeJS=Failed to install NodeJS. Exit code={0}
 NodeJSInstaller.installFromCache=Installing NodeJS from {0} to {1} on {2}
 NodeJSInstaller.failedToUnpack=Failed to unpack {0} ({1} bytes read)
 NodeJSInstallation.displayName=NodeJS
+NodeJSInstallation.default=- use system default -
 NodeJSBuildWrapper.displayName=Provide Node & npm bin/ folder to PATH
 NodeJSCommandInterpreter.displayName=Execute NodeJS script
 NodeJSCommandInterpreter.commandFailed=command execution failed

src/main/resources/jenkins/plugins/nodejs/NodeJSBuildWrapper/config.jelly

@@ -24,11 +24,7 @@ THE SOFTWARE.
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
     <f:entry title="${%nodeJSInstallationName.title}" description="${%nodeJSInstallationName.description}">
-        <select class="setting-input" name="_.nodeJSInstallationName">
-            <j:forEach var="inst" items="${descriptor.installations}">
-                <f:option selected="${inst.name==instance.nodeJSInstallationName}">${inst.name}</f:option>
-            </j:forEach>
-        </select>
+        <f:select field="nodeJSInstallationName" />
     </f:entry>
 
     <f:entry title="${%configId.title}">

src/main/resources/jenkins/plugins/nodejs/NodeJSCommandInterpreter/config.jelly

@@ -24,12 +24,7 @@ THE SOFTWARE.
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
     <f:entry title="${%nodeJSInstallationName.title}" description="${%nodeJSInstallationName.description}">
-        <select class="setting-input" name="_.nodeJSInstallationName">
-            <f:option value="">${%nodeJSInstallationName.emptyValue}</f:option>
-            <j:forEach var="inst" items="${descriptor.installations}">
-                <f:option selected="${inst.name==instance.nodeJSInstallationName}">${inst.name}</f:option>
-            </j:forEach>
-        </select>
+        <f:select field="nodeJSInstallationName" />
     </f:entry>
 
     <f:entry title="${%command.title}" description="See &lt;a href='${rootURL}/env-vars.html' target=_new>the list of available environment variables&lt;/a> accessible by process.env.ENV_VARIABLE.">

src/main/webapp/help.html …odejs/NodeJSCommandInterpreter/help.htmlsrc/main/webapp/help.html renamed to src/main/resources/jenkins/plugins/nodejs/NodeJSCommandInterpreter/help.html src/main/webapp/help.html renamed to src/main/resources/jenkins/plugins/nodejs/NodeJSCommandInterpreter/help.html

@@ -90,11 +90,6 @@ void testDescriptorGetDisplayNameShouldGiveExpectedValue() {
         assertThat(descriptor.getDisplayName()).isEqualTo(Messages.NodeJSCommandInterpreter_displayName());
     }
 
-    @Test
-    void testDescriptorGetHelpFileShouldGiveExpectedValue() {
-        assertThat(descriptor.getHelpFile()).isEqualTo("/plugin/nodejs/help.html");
-    }
-
     private static File newFolder(File root, String... subDirs) throws IOException {
         String subFolder = String.join("/", subDirs);
         File result = new File(root, subFolder);