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 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.

          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); }

          Devin Nusbaum added a comment -

          See PR 3289 for my proposal on how to fix this.

          Devin Nusbaum added a comment - See PR 3289 for my proposal on how to fix this.

          Jesse Glick added a comment -

          Is there something you are still doing, or is this supposed to be In Review?

          Jesse Glick added a comment - Is there something you are still doing, or is this supposed to be In Review ?

          Code changed in jenkins
          User: Devin Nusbaum
          Path:
          core/src/main/java/hudson/model/AbstractItem.java
          core/src/main/java/hudson/model/Job.java
          core/src/main/java/jenkins/model/RenameAction.java
          core/src/main/resources/hudson/model/AbstractItem/confirm-rename.jelly
          core/src/main/resources/hudson/model/AbstractItem/confirm-rename.properties
          core/src/main/resources/hudson/model/Job/configure.jelly
          core/src/main/resources/hudson/model/Job/rename.jelly
          core/src/main/resources/hudson/model/Job/rename.properties
          core/src/main/resources/hudson/model/Job/rename_bg.properties
          core/src/main/resources/hudson/model/Job/rename_da.properties
          core/src/main/resources/hudson/model/Job/rename_de.properties
          core/src/main/resources/hudson/model/Job/rename_es.properties
          core/src/main/resources/hudson/model/Job/rename_fr.properties
          core/src/main/resources/hudson/model/Job/rename_it.properties
          core/src/main/resources/hudson/model/Job/rename_ja.properties
          core/src/main/resources/hudson/model/Job/rename_pt_BR.properties
          core/src/main/resources/hudson/model/Job/rename_ru.properties
          core/src/main/resources/hudson/model/Job/rename_sr.properties
          core/src/main/resources/hudson/model/Job/rename_sv_SE.properties
          core/src/main/resources/hudson/model/Job/rename_zh_CN.properties
          core/src/main/resources/hudson/model/Job/rename_zh_TW.properties
          core/src/main/resources/hudson/model/Messages.properties
          core/src/main/resources/hudson/model/Messages_bg.properties
          core/src/main/resources/hudson/model/Messages_da.properties
          core/src/main/resources/hudson/model/Messages_de.properties
          core/src/main/resources/hudson/model/Messages_es.properties
          core/src/main/resources/hudson/model/Messages_it.properties
          core/src/main/resources/hudson/model/Messages_ja.properties
          core/src/main/resources/hudson/model/Messages_pt_BR.properties
          core/src/main/resources/hudson/model/Messages_ru.properties
          core/src/main/resources/hudson/model/Messages_sr.properties
          core/src/main/resources/hudson/model/Messages_zh_CN.properties
          core/src/main/resources/hudson/model/Messages_zh_TW.properties
          core/src/main/resources/jenkins/model/RenameAction/action.jelly
          test/src/test/java/hudson/model/AbstractItemTest.java
          test/src/test/java/hudson/model/JobTest.java
          http://jenkins-ci.org/commit/jenkins/c816f9655b5594333e65c8dc68d13ecf5c75901f
          Log:
          JENKINS-22936 Move rename logic to AbstractItem (#3289)

          • Move rename infrastructure to a dedicated page at the AbstractItem level
          • Preserve existing translations where applicable
          • Keep doDoRename method at Job level and remove unneeded compatibility page
          • Fix tests
          • Fix Javadoc
          • Update existing rename tests to use new page
          • Update test names for clarity
          • Add since tag for newly introduced APIs
          • Apply project naming strategy to all renamed items, not just jobs
          • Use shorthand accessor
          • Clean up Job#doDoRename and deprecate it
          • Use new-style web method for doDoRename2
          • Remove CheckForNull tag from void method and update Javadoc
          • Change names to doConfirmRename and confirm-rename.jelly
          • Rename index.jelly to action.jelly and use j:if tag correctly
          • Update permission check and remove unused import
          • Use new url and update method name in test
          • Fix Javadoc
          • Fix URL in JobTest
          • Update Javadoc for AbstractItem#isNameEditable

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Devin Nusbaum Path: core/src/main/java/hudson/model/AbstractItem.java core/src/main/java/hudson/model/Job.java core/src/main/java/jenkins/model/RenameAction.java core/src/main/resources/hudson/model/AbstractItem/confirm-rename.jelly core/src/main/resources/hudson/model/AbstractItem/confirm-rename.properties core/src/main/resources/hudson/model/Job/configure.jelly core/src/main/resources/hudson/model/Job/rename.jelly core/src/main/resources/hudson/model/Job/rename.properties core/src/main/resources/hudson/model/Job/rename_bg.properties core/src/main/resources/hudson/model/Job/rename_da.properties core/src/main/resources/hudson/model/Job/rename_de.properties core/src/main/resources/hudson/model/Job/rename_es.properties core/src/main/resources/hudson/model/Job/rename_fr.properties core/src/main/resources/hudson/model/Job/rename_it.properties core/src/main/resources/hudson/model/Job/rename_ja.properties core/src/main/resources/hudson/model/Job/rename_pt_BR.properties core/src/main/resources/hudson/model/Job/rename_ru.properties core/src/main/resources/hudson/model/Job/rename_sr.properties core/src/main/resources/hudson/model/Job/rename_sv_SE.properties core/src/main/resources/hudson/model/Job/rename_zh_CN.properties core/src/main/resources/hudson/model/Job/rename_zh_TW.properties core/src/main/resources/hudson/model/Messages.properties core/src/main/resources/hudson/model/Messages_bg.properties core/src/main/resources/hudson/model/Messages_da.properties core/src/main/resources/hudson/model/Messages_de.properties core/src/main/resources/hudson/model/Messages_es.properties core/src/main/resources/hudson/model/Messages_it.properties core/src/main/resources/hudson/model/Messages_ja.properties core/src/main/resources/hudson/model/Messages_pt_BR.properties core/src/main/resources/hudson/model/Messages_ru.properties core/src/main/resources/hudson/model/Messages_sr.properties core/src/main/resources/hudson/model/Messages_zh_CN.properties core/src/main/resources/hudson/model/Messages_zh_TW.properties core/src/main/resources/jenkins/model/RenameAction/action.jelly test/src/test/java/hudson/model/AbstractItemTest.java test/src/test/java/hudson/model/JobTest.java http://jenkins-ci.org/commit/jenkins/c816f9655b5594333e65c8dc68d13ecf5c75901f Log: JENKINS-22936 Move rename logic to AbstractItem (#3289) Move rename infrastructure to a dedicated page at the AbstractItem level Preserve existing translations where applicable Keep doDoRename method at Job level and remove unneeded compatibility page Fix tests Fix Javadoc Update existing rename tests to use new page Update test names for clarity Add since tag for newly introduced APIs Apply project naming strategy to all renamed items, not just jobs Use shorthand accessor Clean up Job#doDoRename and deprecate it Use new-style web method for doDoRename2 Remove CheckForNull tag from void method and update Javadoc Change names to doConfirmRename and confirm-rename.jelly Rename index.jelly to action.jelly and use j:if tag correctly Update permission check and remove unused import Use new url and update method name in test Fix Javadoc Fix URL in JobTest Update Javadoc for AbstractItem#isNameEditable

          Oleg Nenashev added a comment -

          The patch has been integrated towards 2.110

          Oleg Nenashev added a comment - The patch has been integrated towards 2.110

          Code changed in jenkins
          User: Raul Arabaolaza
          Path:
          src/main/java/org/jenkinsci/test/acceptance/po/TopLevelItem.java
          http://jenkins-ci.org/commit/acceptance-test-harness/f18d77a684925e40b80a1498fec9da576f7b79c2
          Log:
          JENKINS-50023 Fix `TopLevelItem#renameTo` after JENKINS-22936

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Raul Arabaolaza Path: src/main/java/org/jenkinsci/test/acceptance/po/TopLevelItem.java http://jenkins-ci.org/commit/acceptance-test-harness/f18d77a684925e40b80a1498fec9da576f7b79c2 Log: JENKINS-50023 Fix `TopLevelItem#renameTo` after JENKINS-22936

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

              Created:
              Updated:
              Resolved: