[JENKINS-59697] Better manage the form validation

And make it more beautiful with Bootstrap

src/main/java/com/cloudbees/jenkins/plugins/advisor/AdvisorGlobalConfiguration.java

@@ -320,6 +320,13 @@ public String getDisplayName() {
       return Messages.Insights_DisplayName();
     }
 
+    public FormValidation doCheckAcceptToS(@QueryParameter boolean value) {
+      if (!value) {
+        return FormValidation.error("Accepting our Terms and Conditions is mandatory to use this service.");
+      }
+      return FormValidation.ok();
+    }
+
     public FormValidation doCheckEmail(@QueryParameter String value) {
       String emailAddress = EmailUtil.fixEmptyAndTrimAllSpaces(value);
 
@@ -382,14 +389,13 @@ public FormValidation doTestConnection(@QueryParameter("email") final String ema
     }
 
     // Used from validateOnLoad.jelly
-    public String connectionTest(String email) {
+    public String validateServerConnection() {
       AdvisorGlobalConfiguration config = AdvisorGlobalConfiguration.getInstance();
-      if (!config.isAcceptToS()) {
-        return "tos-not-accepted";
+      if(!config.isValid()){
+        return "invalid-configuration";
       }
-
       try {
-        AdvisorClient advisorClient = new AdvisorClient(new Recipient(email));
+        AdvisorClient advisorClient = new AdvisorClient(new Recipient(config.email));
         advisorClient.doCheckHealth();
         return "service-operational";
       } catch (Exception e) {
@@ -411,23 +417,18 @@ public FormValidation doTestSendEmail(@QueryParameter("email") final String emai
         AdvisorClient advisorClient = new AdvisorClient(new Recipient(email.trim()));
 
         advisorClient.doTestEmail();
-        return FormValidation.ok("Sending email.  Please check your inbox and filters.");
+        return FormValidation.ok("A request to send a test email from the server was done. Please check your inbox and filters.");
       } catch (Exception e) {
         return FormValidation.error("Client error : " + e.getMessage());
       }
     }
 
     @Override
     public boolean configure(StaplerRequest req, JSONObject json) {
+      boolean acceptToS = json.getBoolean("acceptToS");
       String email = json.getString("email");
       String cc = json.getString("cc");
       JSONObject advanced = json.getJSONObject("advanced");
-      boolean acceptToS = json.getBoolean("acceptToS");
-
-      // Have to accept the Terms of Service to have a valid configuration
-      if (!acceptToS) {
-        return false;
-      }
 
       Set<String> remove = new HashSet<>();
       for (SupportAction.Selection s : req.bindJSONToList(SupportAction.Selection.class, advanced.get("components"))) {
@@ -447,15 +448,17 @@ public boolean configure(StaplerRequest req, JSONObject json) {
       }
 
       try {
-        if (doCheckEmail(email).kind.equals(FormValidation.Kind.ERROR) ||
-          doCheckCc(cc).kind.equals(FormValidation.Kind.ERROR)) {
-          return false;
-        }
+        return validate(acceptToS, email, cc);
       } catch (Exception e) {
         LOG.severe("Unexpected error while validating form: " + Functions.printThrowable(e));
         return false;
       }
-      return true;
+    }
+
+    public boolean validate(boolean acceptToS, String email, String cc) {
+      return !doCheckAcceptToS(acceptToS).kind.equals(FormValidation.Kind.ERROR)
+        && !doCheckEmail(email).kind.equals(FormValidation.Kind.ERROR)
+        && !doCheckCc(cc).kind.equals(FormValidation.Kind.ERROR);
     }
   }
 }

src/main/java/com/cloudbees/jenkins/plugins/advisor/BundleUpload.java

@@ -17,7 +17,6 @@
 
 import java.io.File;
 import java.io.FileOutputStream;
-import java.text.SimpleDateFormat;
 import java.util.Calendar;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
@@ -38,6 +37,7 @@ public class BundleUpload extends AsyncPeriodicWork {
   private static final String COULD_NOT_SAVE_SUPPORT_BUNDLE = "ERROR: Could not save support bundle";
   private static final String BUNDLE_DIR_DOES_NOT_EXIST =
     "Bundle root directory does not exist and could not be created";
+  protected static final String BUNDLE_SUCCESSFULLY_UPLOADED = "Bundle uploaded";
   private TaskListener task;
 
   public BundleUpload() {
@@ -55,14 +55,16 @@ protected void execute(TaskListener listener) {
 
     if (!config.isPluginEnabled()) {
       log(Level.INFO, "Jenkins Health Advisor by CloudBees plugin disabled. Skipping bundle upload.");
+      updateLastBundleResult(
+        config,
+        createTimestampedWarnMessage("Jenkins Health Advisor by CloudBees plugin disabled. Skipping bundle upload."));
       return;
     }
     if (!config.isValid()) {
       log(Level.INFO, "Invalid configuration. Skipping bundle upload.");
-      return;
-    }
-    if (!config.isAcceptToS()) {
-      log(Level.INFO, "Terms of conditions not accepted. Skipping bundle upload.");
+      updateLastBundleResult(
+        config,
+        createTimestampedWarnMessage("Invalid configuration. Skipping bundle upload."));
       return;
     }
 
@@ -72,7 +74,7 @@ protected void execute(TaskListener listener) {
       executeInternal(config.getEmail(), bundle, pluginVersion);
     } else {
       log(Level.SEVERE, UNABLE_TO_GENERATE_SUPPORT_BUNDLE);
-      config.setLastBundleResult(UNABLE_TO_GENERATE_SUPPORT_BUNDLE);
+      updateLastBundleResult(config, createTimestampedErrorMessage(UNABLE_TO_GENERATE_SUPPORT_BUNDLE));
     }
   }
 
@@ -82,7 +84,8 @@ private File generateBundle() {
       File bundleDir = SupportPlugin.getRootDirectory();
       if (!bundleDir.exists() && !bundleDir.mkdirs()) {
         log(Level.SEVERE, String.format("%s %s", COULD_NOT_SAVE_SUPPORT_BUNDLE, BUNDLE_DIR_DOES_NOT_EXIST));
-        config.setLastBundleResult(String.format("%s%n%s", COULD_NOT_SAVE_SUPPORT_BUNDLE, BUNDLE_DIR_DOES_NOT_EXIST));
+        updateLastBundleResult(config, createTimestampedErrorMessage(
+          String.format("%s%n%s", COULD_NOT_SAVE_SUPPORT_BUNDLE, BUNDLE_DIR_DOES_NOT_EXIST)));
         return null;
       }
 
@@ -93,7 +96,8 @@ private File generateBundle() {
       }
     } catch (Exception e) {
       logError(COULD_NOT_SAVE_SUPPORT_BUNDLE, e);
-      config.setLastBundleResult(String.format("%s%n%s", COULD_NOT_SAVE_SUPPORT_BUNDLE, e));
+      updateLastBundleResult(config,
+        createTimestampedErrorMessage(String.format("%s%n%s", COULD_NOT_SAVE_SUPPORT_BUNDLE, e)));
     }
     return null;
   }
@@ -106,17 +110,17 @@ private void executeInternal(String email, File file, String pluginVersion) {
       ClientResponse response = advisorClient
         .uploadFile(new ClientUploadRequest(Jenkins.get().getLegacyInstanceId(), file, config.getCc(), pluginVersion));
       if (response.getCode() == 200) {
-        config.setLastBundleResult("Successfully uploaded a bundle at " +
-          new SimpleDateFormat("yyyy MM dd HH:mm:ss").format(Calendar.getInstance().getTime()));
+        updateLastBundleResult(config, createTimestampedMessage(BUNDLE_SUCCESSFULLY_UPLOADED));
       } else {
-        config.setLastBundleResult("Bundle upload failed. Response code was: " + response.getCode() + ". " +
-          "Response message: " + response.getMessage());
+        updateLastBundleResult(config,
+          createTimestampedErrorMessage("Bundle upload failed. Response code was: " + response.getCode() + ". " +
+            "Response message: " + response.getMessage()));
       }
     } catch (Exception e) {
       log(Level.SEVERE, "Issue while uploading file to bundle upload service: " + e.getMessage());
       log(Level.FINEST,
         "Exception while uploading file to bundle upload service. Cause: " + ExceptionUtils.getStackTrace(e));
-      config.setLastBundleResult("ERROR: Issue while uploading file to bundle upload service: " + e.getMessage());
+      updateLastBundleResult(config, createTimestampedErrorMessage("Bundle upload failed: " + e.getMessage()));
     }
   }
 
@@ -147,6 +151,25 @@ private void logError(String message, Throwable t) {
     LOG.log(Level.SEVERE, message, t);
   }
 
+  private String createTimestampedWarnMessage(String message) {
+    return "[WARN] " + createTimestampedMessage(message);
+  }
+
+  private String createTimestampedErrorMessage(String message) {
+    return "[ERROR] " + createTimestampedMessage(message);
+  }
+
+  private String createTimestampedMessage(String message) {
+    return String.format("%1$tF %1$tT - %2$s",
+      Calendar.getInstance().getTime(),
+      message);
+  }
+
+  private void updateLastBundleResult(AdvisorGlobalConfiguration config, String message) {
+    config.setLastBundleResult(message);
+    config.save();
+  }
+
   /**
    * By default we wait a few minutes to allow support-core plugin time to generate a bundle first.
    *

src/main/java/com/cloudbees/jenkins/plugins/advisor/casc/AdvisorRootConfigurator.java

@@ -87,7 +87,7 @@ protected AdvisorGlobalConfiguration instance(Mapping mapping, ConfigurationCont
     if (!acceptToS) {
       // In UI, if you don't accept the ToS, the configuration is not applied, so here we throw an exception
       throw new ConfiguratorException(this,
-        "Terms of Service for CloudBees Jenkins Advisor have to be accepted. Please, review the acceptToS field in the yaml file.");
+        "Terms of Service for CloudBees Jenkins Advisor have to be accepted. Please review the acceptToS field in the yaml file.");
     }
 
     AdvisorGlobalConfiguration insights = getTargetComponent(configurationContext);
@@ -97,7 +97,7 @@ protected AdvisorGlobalConfiguration instance(Mapping mapping, ConfigurationCont
       descriptor.doCheckCc(cc).kind.equals(FormValidation.Kind.ERROR)) {
       // In UI, if the fields are invalid, the configuration is not applied, so here we throw an exception
       throw new ConfiguratorException(this,
-        "Invalid configuration for CloudBees Jenkins Advisor. Please, review the content of email and cc fields in the yaml file.");
+        "Invalid configuration for CloudBees Jenkins Advisor. Please review the content of email and cc fields in the yaml file.");
     }
     updateConfiguration(insights, email, cc, true, nagDisabled, excludedComponents);
 

src/main/resources/com/cloudbees/jenkins/plugins/advisor/AdvisorGlobalConfiguration/index.jelly

@@ -2,10 +2,23 @@
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:l="/lib/layout"
          xmlns:f="/lib/form" xmlns:a="/lib/advisor">
   <l:layout title="${it.actionTitleText}" norefresh="true" permission="${it.ADMINISTRATOR}">
+
+    <l:header>
+      <link rel="stylesheet" href="${resURL}/bootstrap/css/bootstrap.min.css" type="text/css"/>
+      <link rel="stylesheet" href="${resURL}/css/font-awesome/css/font-awesome.min.css"/>
+      <style type="text/css">
+        table {
+        border-collapse: separate;
+        border-spacing: 5px;
+        }
+      </style>
+    </l:header>
+
     <st:include page="sidepanel.jelly" it="${app}"/>
+
     <l:main-panel>
       <h1>
-        <img width="48" height="48" style="margin: 2px;" alt="Jenkins Health Advisor by CloudBees Icon"
+        <img width="48" height="48" alt="Jenkins Health Advisor by CloudBees Icon"
              src="${rootURL}${h.resourcePath}/plugin/cloudbees-jenkins-advisor/icons/advisor.svg"/>
         ${it.actionTitle}
       </h1>
@@ -19,13 +32,9 @@
       <p>As issues are detected in your master, Jenkins Health Advisor by CloudBees will send new reports to your
         email address.
       </p>
-      <a:validateOnLoad email="${it.email}"/>
-      <j:if test="${it.lastBundleResult!=null}">
-        <p>
-          <strong>Last bundle upload:</strong>
-          ${it.lastBundleResult}
-        </p>
-      </j:if>
+
+      <a:validateOnLoad/>
+
       <f:form name="config" method="POST" action="configure">
 
         <j:set var="instance" value="${it}"/>
@@ -38,9 +47,10 @@
             our Terms and Conditions</a>.
           </p>
         </f:block>
-        <f:entry field="acceptToS">
-          <f:checkbox title="${%I agree with these Terms and Conditions}"
-                      tooltip="No data will be sent to CloudBees if you don't accept these Terms and Conditions."/>
+
+        <f:entry field="acceptToS"
+                 description="No data will be sent to CloudBees if you don't accept these Terms and Conditions.">
+          <f:checkbox title="${%I agree with these Terms and Conditions}"/>
         </f:entry>
 
         <f:block>
@@ -50,10 +60,12 @@
         <f:entry title="${%Email}" field="email">
           <f:textbox/>
         </f:entry>
+
         <f:entry title="${%CC}" field="cc"
-                 description="You can optionally include a list of comma-separated emails to receive Advisor report.">
+                 description="You can optionally include a list of comma-separated emails to receive the report.">
           <f:textbox/>
         </f:entry>
+
         <f:validateButton
           title="${%Send a test email}" progress="${%Testing...}"
           method="testSendEmail" with="email,cc"/>
@@ -96,9 +108,8 @@
           <strong>${%Reminder}</strong>
         </f:block>
 
-        <f:entry title="${%Suppress the reminder to configure Jenkins Health Advisor by CloudBees}"
-                 field="nagDisabled">
-          <f:checkbox/>
+        <f:entry field="nagDisabled">
+          <f:checkbox title="${%Suppress the reminder to configure Jenkins Health Advisor by CloudBees}"/>
         </f:entry>
 
         <f:bottomButtonBar>
@@ -107,6 +118,48 @@
         </f:bottomButtonBar>
 
       </f:form>
+
+      <j:if test="${it.lastBundleResult!=null &amp;&amp; it.lastBundleResult.contains('ERROR')}">
+        <div class="alert alert-danger" role="alert">
+          <strong>
+            <i class="fa fa-minus-circle"/>
+            Last upload failed!
+          </strong>
+          <p>
+            <pre>
+              <code>${it.lastBundleResult}</code>
+            </pre>
+          </p>
+        </div>
+      </j:if>
+      <j:if test="${it.lastBundleResult!=null &amp;&amp; it.lastBundleResult.contains('WARN')}">
+        <div class="alert alert-warning" role="alert">
+          <strong>
+            <i class="fa fa-exclamation-triangle"/>
+            Last upload skipped!
+          </strong>
+          <p>
+            <pre>
+              <code>${it.lastBundleResult}</code>
+            </pre>
+          </p>
+        </div>
+      </j:if>
+      <j:if
+        test="${it.lastBundleResult!=null &amp;&amp; !it.lastBundleResult.contains('ERROR') &amp;&amp; !it.lastBundleResult.contains('WARN')}">
+        <div class="alert alert-info" role="alert">
+          <strong>
+            <i class="fa fa-check-circle"/>
+            Last upload succeeded!
+          </strong>
+          <p>
+            <pre>
+              <code>${it.lastBundleResult}</code>
+            </pre>
+          </p>
+        </div>
+      </j:if>
+
     </l:main-panel>
   </l:layout>
 </j:jelly>