Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-22936

Move rename infrastructure from Job to AbstractItem

    • Icon: Task Task
    • Resolution: Fixed
    • Icon: Major Major
    • core

      Job has some fairly complex infrastructure for handling renames:

      • the special textbox in configure.jelly
      • the handling in doConfigSubmit, including ProjectNamingStrategy integration, Apply vs. Save logic (including the tricky JENKINS-17401 fix), and the redirect
      • rename.jelly (and rename.properties)
      • doDoRename, with more checks and redirects

      Reimplementing all this in another item type (such as Folder) is rather painful, involving a lot of duplicated code and thus an inability to fix a bug once in one placeā€”or pick a different implementation, such as using a JavaScript dialog like we now use for deletion and other actions requiring confirmation.

      This code should be pulled up into AbstractItem for easy reuse, with some hooks for domain-specific validation (such as Job.isBuilding).

          [JENKINS-22936] Move rename infrastructure from Job to AbstractItem

          Jesse Glick created issue -
          Jesse Glick made changes -
          Labels Original: api dry New: api dry folders
          Jesse Glick made changes -
          Link New: This issue is related to JENKINS-17401 [ JENKINS-17401 ]

          Jesse Glick added a comment -

          Also handling of possibly distinct display names (displayNameOrNull, AbstractProject/configure-common.jelly) would logically belong in the same place.

          Jesse Glick added a comment - Also handling of possibly distinct display names ( displayNameOrNull , AbstractProject/configure-common.jelly ) would logically belong in the same place.
          Jesse Glick made changes -
          Link New: This issue is related to SECURITY-128 [ SECURITY-128 ]
          Jesse Glick made changes -
          Link New: This issue is related to JENKINS-18649 [ JENKINS-18649 ]

          Daniel Beck added a comment -

          Also, rename is not the same as Delete + Create. That needs to go (Job.doDoRename).

          Daniel Beck added a comment - Also, rename is not the same as Delete + Create. That needs to go ( Job.doDoRename ).

          Jesse Glick added a comment -

          ndeloof also found a problem with Trigger.start. I am thinking that the current system should be abandoned and replaced with a JavaScript prompt before the form is submitted. Once you POST the submission the change is final.

          Jesse Glick added a comment - ndeloof also found a problem with Trigger.start . I am thinking that the current system should be abandoned and replaced with a JavaScript prompt before the form is submitted. Once you POST the submission the change is final.

          Jesse Glick added a comment -

          rename is not the same as Delete + Create

          Even if it were, it is implemented incorrectly; need

          @@ -1436,7 +1438,10 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
           
                   if (!hasPermission(CONFIGURE)) {
                       // rename is essentially delete followed by a create
          -            checkPermission(CREATE);
          +            ItemGroup<?> parent = getParent();
          +            if (parent instanceof AccessControlled) {
          +                ((AccessControlled) parent).checkPermission(CREATE);
          +            }
                       checkPermission(DELETE);
                   }
           

          Jesse Glick added a comment - rename is not the same as Delete + Create Even if it were, it is implemented incorrectly; need @@ -1436,7 +1438,10 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R if (!hasPermission(CONFIGURE)) { // rename is essentially delete followed by a create - checkPermission(CREATE); + ItemGroup<?> parent = getParent(); + if (parent instanceof AccessControlled) { + ((AccessControlled) parent).checkPermission(CREATE); + } checkPermission(DELETE); }
          R. Tyler Croy made changes -
          Workflow Original: JNJira [ 155082 ] New: JNJira + In-Review [ 179008 ]

            dnusbaum Devin Nusbaum
            jglick Jesse Glick
            Votes:
            1 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: