JENKINS-25744 Switch job property to use an optionalBlock

Normal convention for Jenkins plugins which contribute a JobProperty to a jobs
configuration page is to use an optionalBlock as the top level jelly tag.

In a jobs properties section of the configuration page, plugins 'hide' their
configuration entries by using the optionalBlock, this is presented to the user
as a checkbox. This keeps the jobs configuration page quite clean while allowing
the user full control over the properties they use.

src/main/java/se/diabol/jenkins/pipeline/PipelineProperty.java

@@ -22,6 +22,8 @@
 import hudson.util.FormValidation;
 import jenkins.model.Jenkins;
 import net.sf.json.JSONObject;
+
+import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.export.Exported;
@@ -38,6 +40,7 @@ public class PipelineProperty extends JobProperty<AbstractProject<?, ?>> {
     public PipelineProperty() {
     }
 
+    @DataBoundConstructor
     public PipelineProperty(String taskName, String stageName) {
         setStageName(stageName);
         setTaskName(taskName);
@@ -77,6 +80,8 @@ public static Set<String> getStageNames() {
 
     @Extension
     public static final class DescriptorImpl extends JobPropertyDescriptor {
+
+    	@Override
         public String getDisplayName() {
             return "Pipeline description";
         }
@@ -118,25 +123,26 @@ protected FormValidation checkValue(String value) {
                 return FormValidation.error("Value needs to be empty or include characters and/or numbers");
             }
             return FormValidation.ok();
-
         }
 
-
-        @Override
-        public PipelineProperty newInstance(StaplerRequest sr, JSONObject formData) throws FormException {
-            String task = sr.getParameter("taskName");
-            String stage = sr.getParameter("stageName");
-            if ("".equals(task)) {
-                task = null;
-            }
-            if ("".equals(stage)) {
-                stage = null;
-            }
-            if (task == null && stage == null) {
-                return null;
-            }
-            return new PipelineProperty(task,
-                    stage);
-        }
+		@Override
+		public PipelineProperty newInstance(StaplerRequest sr, JSONObject formData) throws FormException {
+			String task = sr.getParameter("taskName");
+			String stage = sr.getParameter("stageName");
+			boolean configEnabled = sr.getParameter("enabled") != null;
+			if (!configEnabled) {
+				return null;
+			}
+			if ("".equals(task)) {
+				task = null;
+			}
+			if ("".equals(stage)) {
+				stage = null;
+			}
+			if (task == null && stage == null) {
+				return null;
+			}
+			return new PipelineProperty(task, stage);
+		}
     }
 }

src/main/resources/se/diabol/jenkins/pipeline/PipelineProperty/config.jelly

@@ -1,13 +1,13 @@
 <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:section title="Delivery Pipeline configuration">
+    <f:optionalBlock name="enabled" title="Delivery Pipeline configuration" inline="true" checked="${instance != null}">
             <f:entry title="Stage Name" field="stageName">
                 <f:textbox name="stageName" value="${instance.getStageName()}"/>
             </f:entry>
 
             <f:entry title="Task Name" field="taskName">
                 <f:textbox name="taskName" value="${instance.getTaskName()}"/>
             </f:entry>
-    </f:section>
-</j:jelly>
+    </f:optionalBlock>
+</j:jelly>

src/test/java/se/diabol/jenkins/pipeline/PipelinePropertyTest.java

@@ -80,6 +80,7 @@ public void testNewInstanceEmpty() throws Exception {
         StaplerRequest request = Mockito.mock(StaplerRequest.class);
         when(request.getParameter("taskName")).thenReturn("");
         when(request.getParameter("stageName")).thenReturn("");
+        when(request.getParameter("enabled")).thenReturn("on");
         assertNull(d.newInstance(request, null));
     }
 
@@ -90,6 +91,7 @@ public void testNewInstanceNull() throws Exception {
         StaplerRequest request = Mockito.mock(StaplerRequest.class);
         when(request.getParameter("taskName")).thenReturn(null);
         when(request.getParameter("stageName")).thenReturn(null);
+        when(request.getParameter("enabled")).thenReturn("on");
         assertNull(d.newInstance(request, null));
     }
 
@@ -100,12 +102,21 @@ public void testNewInstanceTaskNull() throws Exception {
         StaplerRequest request = Mockito.mock(StaplerRequest.class);
         when(request.getParameter("taskName")).thenReturn(null);
         when(request.getParameter("stageName")).thenReturn("Stage");
+        when(request.getParameter("enabled")).thenReturn("on");
         PipelineProperty p = d.newInstance(request, null);
         assertNotNull(p);
         assertNull(p.getTaskName());
         assertEquals("Stage", p.getStageName());
     }
 
+    @Test
+    @WithoutJenkins
+    public void testNewInstanceTaskNullDisabled() throws Exception {
+        PipelineProperty.DescriptorImpl d = new PipelineProperty.DescriptorImpl();
+        StaplerRequest request = Mockito.mock(StaplerRequest.class);
+        when(request.getParameter("enabled")).thenReturn(null);
+        assertNull(d.newInstance(request, null));
+    }
 
     @Test
     @WithoutJenkins
@@ -114,6 +125,7 @@ public void testNewInstanceBothSet() throws Exception {
         StaplerRequest request = Mockito.mock(StaplerRequest.class);
         when(request.getParameter("taskName")).thenReturn("Task");
         when(request.getParameter("stageName")).thenReturn("Stage");
+        when(request.getParameter("enabled")).thenReturn("on");
         PipelineProperty p = d.newInstance(request, null);
         assertNotNull(p);
         assertEquals("Task", p.getTaskName());
@@ -138,8 +150,6 @@ public void testDoAutoCompleteStageName() throws Exception {
 
         AutoCompletionCandidates c3 = d.doAutoCompleteStageName(null);
         assertEquals(c3.getValues().size(), 0);
-
-
     }
 
     @Test