[JENKINS-69916] Un-inline WorkflowJob/configure-entries.jelly

[krisstern]

Description

Testing done. Tried the functionality which appears intact after the code modifications, except for the original testing steps outlined in https://issues.jenkins.io/browse/JENKINS-69916. The checkURL validation is now completely gone. This work follows the upstream changes in jenkinsci/jenkins#8866, especially the ones in core/src/main/java/hudson/model/AbstractProject.java. The only requirement for this patch to work is that we will need the minimum Jenkins version requirement to set at 2.443.

JIRA Issue

https://issues.jenkins.io/browse/JENKINS-69916

Relevant Upstream Change(s)

jenkinsci/jenkins#8866

Testing Steps

Similar to the ones described in JIRA ticket, except the checkDisplayName validation is gone.

c.c. @Kevin-CB

Submitter checklist

Beta Give feedback

1. Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
2. Ensure that the pull request title represents the desired changelog entry
3. Please describe what you did
4. Link to relevant issues in GitHub or Jira
5. Link to relevant pull requests, esp. upstream and downstream changes
6. Ensure you have provided tests - that demonstrates feature works or fixes the issue

[mawinter69]

You can't depend on jenkinsci/jenkins#8866 because this changes AbstractProject and WorkflowJob is not inheriting from it. So no need to change the jenkins version. You would need to also implement the doCheckDisplayNameOrNull method.

Given that we have this now in MatrixProject (open PR jenkinsci/matrix-project-plugin#130), core (the mentioned change), folder plugin (https://github.com/jenkinsci/cloudbees-folder-plugin/blob/233481b4bb4accfe8582ad2f5c922168022af31d/src/main/java/com/cloudbees/hudson/plugins/folder/AbstractFolderDescriptor.java#L138) and here maybe we can have an AbstractItemDescriptor interface with a default implementation for that doCheckDisplayNameOrNull and have all 4 places implement that interface

[krisstern]

Hi @mawinter69 I have tried my best to apply the suggesions from your review. But I might have missed some important details though. If that if the case please let me know. Thanks!

[mawinter69]

Hi @mawinter69 I have tried my best to apply the suggesions from your review. But I might have missed some important details though. If that if the case please let me know. Thanks!

Sorry seems my wording was not precise enough. The thing with the AbstractItemDescriptor is something that would need to be implemented in Jenkins core, not in a plugin. This would allow the 4 places that now all have duplicated code for the doCheckDisplayNameOrNull method now to just add the dependency on that interface.

So for now you will need to add this method to the Descriptor of WorkflowJob, not WorkflowJob directly

[jglick]

something that would need to be implemented in Jenkins core, not in a plugin

So do so in Jenkins core, and set the jenkins.version here appropriately to pick up the new API.

[krisstern]

Let me follow up on this shortly

[krisstern]

Hi @mawinter69 @jglick,
I have tried my best in jenkinsci/jenkins#9150.

But if I have made any mistakes, please point these out to me so I can accommodate. Thanks!

[krisstern]

Working on the upstream PR pending review(s): jenkinsci/jenkins#9150

[daniel-beck]

With the core change, should be good enough just deleting the checkUrl?

Should increase the core dependency to the snapshot (and then the release once out).

[krisstern]

Hi @daniel-beck I have just made the changes as suggested

[daniel-beck]

Is this significant enough to need a core dependency with jenkinsci/jenkins#9150, or can we tolerate this not working with older cores?

[basil]

PCT: jenkinsci/bom#3657
ATH: jenkinsci/acceptance-test-harness#1751

[basil]

Retested manually against 2.477.x

[jglick]

Is a bom-2.477.x in preparation?

[jglick]

pending jenkinsci/plugin-pom#1004 (comment) IIUC