From 548e6e9e0c3b852e5ac51640db6c09dfebbeb5de Mon Sep 17 00:00:00 2001 From: fluffy88 Date: Thu, 13 Nov 2014 18:19:51 +0000 Subject: [PATCH] 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. --- .../jenkins/pipeline/PipelineProperty.java | 42 +++++++++++-------- .../pipeline/PipelineProperty/config.jelly | 6 +-- .../pipeline/PipelinePropertyTest.java | 14 ++++++- 3 files changed, 39 insertions(+), 23 deletions(-) diff --git a/src/main/java/se/diabol/jenkins/pipeline/PipelineProperty.java b/src/main/java/se/diabol/jenkins/pipeline/PipelineProperty.java index 0b88ad98a..2a8e510cb 100644 --- a/src/main/java/se/diabol/jenkins/pipeline/PipelineProperty.java +++ b/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> { public PipelineProperty() { } + @DataBoundConstructor public PipelineProperty(String taskName, String stageName) { setStageName(stageName); setTaskName(taskName); @@ -77,6 +80,8 @@ public static Set 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); + } } } diff --git a/src/main/resources/se/diabol/jenkins/pipeline/PipelineProperty/config.jelly b/src/main/resources/se/diabol/jenkins/pipeline/PipelineProperty/config.jelly index 5c89b96fa..92ce873ac 100644 --- a/src/main/resources/se/diabol/jenkins/pipeline/PipelineProperty/config.jelly +++ b/src/main/resources/se/diabol/jenkins/pipeline/PipelineProperty/config.jelly @@ -1,7 +1,7 @@ - + @@ -9,5 +9,5 @@ - - \ No newline at end of file + + diff --git a/src/test/java/se/diabol/jenkins/pipeline/PipelinePropertyTest.java b/src/test/java/se/diabol/jenkins/pipeline/PipelinePropertyTest.java index aa2dbdc4d..567639562 100644 --- a/src/test/java/se/diabol/jenkins/pipeline/PipelinePropertyTest.java +++ b/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