[JENKINS-61068] set correct value on parambuild page

[agabrys]

getDefaultParameterValue is used to get value on parambuild and buildWithParameters urls (trigger build). Those pages use text boxes to represent Active Choices parameters.

The method should respect users configuration and return values with ":selected" suffix. If no value contains the suffix, it should return the first element (backward compatibility).

[kinow]

Code looks good. Thanks for adding a unit test.

Just need to find time now to review it with calm, testing and understanding more about this change 👍

Thanks @agabrys !

[agabrys]

If you need more details - let me know. I described how the feature works in the JENKINS-61068 ticket.

[kinow]

Finally had some minutes to review. Code is excellent, the issue has excellent description.

I initially couldn't replicate it. But that's because I was using a multi select element, not a single element.

With a single element, I can reproduce the issue. But my only question about fixing this, is that this could actually introduce another issue.

As far as I know, an HTML input with no multiple selection enabled, shouldn't return two values. The change in this PR appears to change that.

In the image above. States & Cities is the example I always test. Cities has an input for multiple elements, and the last two items have :selected. The third element is an input for a single element, with just one item with :selected. Then the test2 has two entries with :selected, and the HTML input is a single element, but it still has 2 values selected.

So I am a bit reluctant to merge & release it right now. /cc @imoutsatsos any thoughts?

[agabrys]

I'm aware of it and I ignored it. Why? The method which I introduced is used only on parambuild and buildWithParameters. These both pages display the parameter as a simple text box. It means that even if a job administrator configured something as a radio button, any user is still able to type value1,value2 and get two values instead of one. Second thing is that the change returns a default value(s). If the job administrator configured an input parameter as a radio button and marked two values as :selected, then I think we shouldn't care about it. On the job page the second value will be marked as default and on the parambuild \ buildWithParameters both. But the problem is easy to fix by the job owner (don't configure two default values for the radio button).

With this fix the job administrator has control which value(s) will be displayed by default on the parambuild \ buildWithParameters pages. Without it, always the first value is taken without trimming the :selected. And this cannot be fixed by the job owner, because the logic is in the plugin, not in the job. Install the plugin without the change, and check the job which you used before. It will return:

+-----------+------------+----------------------------------------+
| Parameter |    Value   |                 Comment                |
+-----------+------------+----------------------------------------+
| test      |      1     | ['1', '2', '3:selected', '4', '5']     |
+-----------+------------+----------------------------------------+
| test2     | 1:selected | ['1:selected', '2', '3:selected', '4'] |
+-----------+------------+----------------------------------------+

So the PR does not completely fix the problem (you are still able to set multiple values to radio button), but at least the job administrator has control which values will popup as default. No the job administrator does not have any control on parambuild \ buildWithParameters pages.

[imoutsatsos]

I think we can merge this. I think @agabrys has given a rather good argument. If we can have an experimental build before the final release, I would love to give it a try. Thanks @agabrys and @kinow for this!!

[kinow]

Roger that @imoutsatsos . Merging tonight, then prepare an hpi and attach to the issue and CC you there. We can postpone the release for a few days/weeks to give you and others time to test it too 👍 Thanks and thanks @agabrys too!

[agabrys]

Hello 🙂 Is any chance for a release in the near future?

[kinow]

Hi @agabrys just waiting for https://issues.jenkins-ci.org/browse/JENKINS-61751 to be tested/merged too, then planning a release by the end of this month 👍 (or sooner if that issue is solved faster)