CRAPP-435: Allow to refactor merge change request

application-changerequest-api/src/main/java/org/xwiki/contrib/changerequest/ChangeRequestConfiguration.java

@@ -223,4 +223,19 @@ default boolean acceptOnlyAllowedApprovers()
     {
         return false;
     }
+
+    /**
+     * Define if the reference of the pages impacted by a change request should also be refactored when that change
+     * request already has the status {@link ChangeRequestStatus#MERGED}, in case those pages get renamed or moved.
+     * Note that this only updates the reference of the impacted pages stored in the change request: the content of
+     * the change request itself is never modified.
+     *
+     * @return {@code true} if merged change requests should also be refactored when one of their pages is renamed
+     *          or moved.
+     * @since 1.24
+     */
+    default boolean isMergedChangeRequestRefactoringEnabled()
+    {
+        return false;
+    }
 }

application-changerequest-default/src/main/java/org/xwiki/contrib/changerequest/internal/DefaultChangeRequestConfiguration.java

@@ -232,4 +232,10 @@ public boolean acceptOnlyAllowedApprovers()
     {
         return this.configurationSource.getProperty("acceptOnlyAllowedApprovers", false);
     }
+
+    @Override
+    public boolean isMergedChangeRequestRefactoringEnabled()
+    {
+        return this.configurationSource.getProperty("refactorMergedChangeRequests", false);
+    }
 }

application-changerequest-default/src/main/java/org/xwiki/contrib/changerequest/internal/listeners/DocumentRenamedListener.java

@@ -29,6 +29,7 @@
 import org.slf4j.Logger;
 import org.xwiki.component.annotation.Component;
 import org.xwiki.contrib.changerequest.ChangeRequest;
+import org.xwiki.contrib.changerequest.ChangeRequestConfiguration;
 import org.xwiki.contrib.changerequest.ChangeRequestException;
 import org.xwiki.contrib.changerequest.ChangeRequestStatus;
 import org.xwiki.contrib.changerequest.discussions.ChangeRequestDiscussionService;
@@ -68,6 +69,9 @@ public class DocumentRenamedListener extends AbstractLocalEventListener
     @Inject
     private ChangeRequestDiscussionService changeRequestDiscussionService;
 
+    @Inject
+    private ChangeRequestConfiguration configuration;
+
     @Inject
     private Logger logger;
 
@@ -99,10 +103,12 @@ private void updateChangeRequests(DocumentReference source, DocumentReference ta
 
         try {
             changeRequests = this.storageManager.findChangeRequestTargeting(source);
-            changeRequests =
-                changeRequests.stream()
-                    .filter(changeRequest -> changeRequest.getStatus() != ChangeRequestStatus.MERGED)
-                    .collect(Collectors.toList());
+            if (!this.configuration.isMergedChangeRequestRefactoringEnabled()) {
+                changeRequests =
+                    changeRequests.stream()
+                        .filter(changeRequest -> changeRequest.getStatus() != ChangeRequestStatus.MERGED)
+                        .collect(Collectors.toList());
+            }
         } catch (ChangeRequestException e) {
             this.logger.error("Failed to find change requests using document [{}].", source, e);
         }

application-changerequest-default/src/test/java/org/xwiki/contrib/changerequest/internal/listeners/DocumentRenamedListenerTest.java

@@ -25,6 +25,7 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.xwiki.contrib.changerequest.ChangeRequest;
+import org.xwiki.contrib.changerequest.ChangeRequestConfiguration;
 import org.xwiki.contrib.changerequest.ChangeRequestException;
 import org.xwiki.contrib.changerequest.ChangeRequestStatus;
 import org.xwiki.contrib.changerequest.discussions.ChangeRequestDiscussionService;
@@ -76,6 +77,9 @@ class DocumentRenamedListenerTest
     @MockComponent
     private ChangeRequestDiscussionService changeRequestDiscussionService;
 
+    @MockComponent
+    private ChangeRequestConfiguration configuration;
+
     @RegisterExtension
     private LogCaptureExtension logCapture = new LogCaptureExtension(LogLevel.INFO);
 
@@ -139,6 +143,41 @@ void processLocalEventRefactorsMatchingChangeRequests() throws ChangeRequestExce
         assertEquals("Updating change request [openCR].", this.logCapture.getMessage(1));
     }
 
+    @Test
+    void processLocalEventRefactorsMergedChangeRequestsWhenEnabled() throws ChangeRequestException
+    {
+        when(this.configuration.isMergedChangeRequestRefactoringEnabled()).thenReturn(true);
+
+        MoveRequest moveRequest = mock(MoveRequest.class);
+        when(moveRequest.isUpdateLinks()).thenReturn(true);
+        when(moveRequest.isDeep()).thenReturn(true);
+
+        DocumentReference source = new DocumentReference("wiki", "Space", "Source");
+        DocumentReference target = new DocumentReference("wiki", "Space", "Target");
+        DocumentRenamedEvent event = new DocumentRenamedEvent(source, target);
+
+        ChangeRequest mergedChangeRequest = mock(ChangeRequest.class);
+        when(mergedChangeRequest.getStatus()).thenReturn(ChangeRequestStatus.MERGED);
+        when(mergedChangeRequest.getId()).thenReturn("mergedCR");
+
+        when(this.storageManager.findChangeRequestTargeting(source)).thenReturn(Collections.singletonList(
+            mergedChangeRequest));
+
+        this.listener.processLocalEvent(event, null, moveRequest);
+
+        verify(this.storageManager).refactorTargetEntity(mergedChangeRequest, source, target, true);
+        verify(this.changeRequestDiscussionService)
+            .refactorDiscussionFileReference("mergedCR", source, target, true);
+        verify(this.observationManager).notify(any(ChangeRequestRefactoringEvent.class), eq("mergedCR"));
+        verify(this.observationManager).notify(any(ChangeRequestRefactoredEvent.class), eq("mergedCR"));
+
+        verify(this.progressManager).pushLevelProgress(1, this.listener);
+
+        assertEquals("Updating the change requests to refactor document [wiki:Space.Source] to "
+            + "[wiki:Space.Target].", this.logCapture.getMessage(0));
+        assertEquals("Updating change request [mergedCR].", this.logCapture.getMessage(1));
+    }
+
     @Test
     void processLocalEventWhenStorageManagerFails() throws ChangeRequestException
     {

application-changerequest-default/src/test/java/org/xwiki/contrib/changerequest/internal/storage/DefaultChangeRequestStorageManagerTest.java

@@ -27,6 +27,7 @@
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
@@ -85,6 +86,7 @@
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -235,6 +237,57 @@ void save() throws Exception
         verify(document).clone();
     }
 
+    @Test
+    void refactorTargetEntityPreservesStatusOfMergedChangeRequest() throws Exception
+    {
+        DocumentReference source = new DocumentReference("wiki", "Space", "Source", Locale.ROOT);
+        DocumentReference target = new DocumentReference("wiki", "Space", "Target", Locale.ROOT);
+
+        ChangeRequest changeRequest = new ChangeRequest()
+            .setId("mergedCR")
+            .setStatus(ChangeRequestStatus.MERGED)
+            .setCreationDate(new Date(12));
+
+        FileChange fileChange = mock(FileChange.class);
+        when(fileChange.getTargetEntity()).thenReturn(source);
+        when(fileChange.getType()).thenReturn(FileChange.FileChangeType.EDITION);
+        FileChange fileChangeClone = mock(FileChange.class);
+        when(fileChange.clone()).thenReturn(fileChangeClone);
+        when(fileChangeClone.getTargetEntity()).thenReturn(target);
+        changeRequest.addFileChange(fileChange);
+
+        FileChange newFileChangeVersion = mock(FileChange.class);
+        when(newFileChangeVersion.getTargetEntity()).thenReturn(target);
+        when(this.fileChangeStorageManager.refactorFileChangeEntity(fileChange, target))
+            .thenReturn(newFileChangeVersion);
+
+        DocumentReference crDocumentReference = mock(DocumentReference.class);
+        when(this.changeRequestDocumentReferenceResolver.resolve(any(ChangeRequest.class)))
+            .thenReturn(crDocumentReference);
+        XWikiDocument document = mock(XWikiDocument.class);
+        when(document.clone()).thenReturn(document);
+        when(this.wiki.getDocument(crDocumentReference, this.context)).thenReturn(document);
+        when(document.isNew()).thenReturn(false);
+        DocumentAuthors documentAuthors = mock(DocumentAuthors.class);
+        when(document.getAuthors()).thenReturn(documentAuthors);
+        DocumentReference userDocRef = mock(DocumentReference.class);
+        when(this.context.getUserReference()).thenReturn(userDocRef);
+        UserReference userReference = mock(UserReference.class);
+        when(this.userReferenceResolver.resolve(userDocRef)).thenReturn(userReference);
+        BaseObject xobject = mock(BaseObject.class);
+        when(document.getXObject(CHANGE_REQUEST_XCLASS, 0, true, this.context)).thenReturn(xobject);
+        when(document.isMetaDataDirty()).thenReturn(true);
+
+        this.storageManager.refactorTargetEntity(changeRequest, source, target, false);
+
+        // The status of a merged change request must remain "merged" even though its file changes are rewritten
+        // to point to the new document reference.
+        verify(xobject).set("status", "merged", this.context);
+        verify(xobject, never()).set(eq("status"), eq("draft"), any());
+        verify(xobject, never()).set(eq("status"), eq("ready_for_review"), any());
+        verify(this.wiki).saveDocument(eq(document), any(String.class), eq(this.context));
+    }
+
     @Test
     void load() throws Exception
     {

application-changerequest-test/application-changerequest-test-docker/src/test/it/org/xwiki/contrib/changerequest/test/ui/RefactoringChangeRequestIT.java

@@ -19,6 +19,7 @@
  */
 package org.xwiki.contrib.changerequest.test.ui;
 
+import java.util.Arrays;
 import java.util.List;
 
 import org.junit.jupiter.api.BeforeAll;
@@ -290,4 +291,150 @@ void refactoringSpace(TestUtils testUtils, TestReference testReference)
         assertFalse(
             fileChangesPane.isDiffOutdated(createdPageReferenceRefactored.getLocalDocumentReference().toString()));
     }
+
+    /**
+     * Same kind of scenario as {@link #refactoringSpace(TestUtils, TestReference)} but with the
+     * {@code refactorMergedChangeRequests} configuration enabled: contrary to the default behavior, a merged change
+     * request must also have its page reference refactored when that page is renamed, while its status and content
+     * remain untouched. The merged change request also contains a page creation and a page deletion, none of which
+     * should be impacted by the refactoring of the unrelated edited page. The published page is also further edited
+     * after the merge, to check that the file change diff of the merged and refactored change request correctly
+     * reports it as outdated, while still displaying the actual change performed by the change request itself.
+     */
+    @Test
+    void refactoringMergedChangeRequestWhenConfigurationEnabled(TestUtils testUtils, TestReference testReference)
+        throws Exception
+    {
+        testUtils.loginAsSuperAdmin();
+        testUtils.updateObject(Arrays.asList("ChangeRequest", "Code"), "Configuration",
+            "ChangeRequest.Code.ConfigurationClass", 0,
+            "refactorMergedChangeRequests", 1);
+
+        try {
+            DocumentReference originalPage = new DocumentReference("MergedRefactoringPage",
+                testReference.getLastSpaceReference());
+            testUtils.createPage(originalPage, "Content before refactoring.");
+            DocumentReference pageToDelete = new DocumentReference("MergedRefactoringPageToDelete",
+                testReference.getLastSpaceReference());
+            testUtils.createPage(pageToDelete, "Page to be deleted.");
+
+            // Create a change request editing the page, approve it and merge it.
+            testUtils.login(CR_USER, CR_USER);
+            testUtils.gotoPage(originalPage);
+            ExtendedViewPage extendedViewPage = new ExtendedViewPage();
+            ExtendedEditPage<CKEditor> extendedEditPage = extendedViewPage.clickStandardEdit(true);
+            extendedEditPage.getWrappedEditor().getRichTextArea()
+                .setContent("Content before refactoring.\nSome change.");
+            ChangeRequestSaveModal changeRequestSaveModal = extendedEditPage.clickSaveAsChangeRequest();
+            changeRequestSaveModal.setChangeRequestTitle("Refactoring_MergedCR");
+            ChangeRequestPage changeRequestPage = changeRequestSaveModal.clickSave();
+
+            // Add a page creation request to the same change request.
+            testUtils.gotoPage(originalPage);
+            extendedViewPage = new ExtendedViewPage();
+            ExtendedCreatePage extendedCreatePage = extendedViewPage.clickStandardCreate();
+            extendedCreatePage.getDocumentPicker().setTitle("MergedRefactoringPageCreated");
+            extendedEditPage = extendedCreatePage.clickChangeRequestCreateButton(true);
+            extendedEditPage.getWrappedEditor().getRichTextArea().setContent("Content of the created page.");
+            changeRequestSaveModal = extendedEditPage.clickSaveAsChangeRequest();
+            changeRequestSaveModal.openAddChangesToExistingChangeRequestCollapse();
+            changeRequestSaveModal.selectExistingChangeRequest("Refactoring_MergedCR").select();
+            changeRequestPage = changeRequestSaveModal.clickSave();
+            DocumentReference createdPage = new DocumentReference("WebHome",
+                new SpaceReference("MergedRefactoringPageCreated", testReference.getLastSpaceReference()));
+
+            // Add a page deletion request to the same change request.
+            testUtils.gotoPage(pageToDelete);
+            extendedViewPage = new ExtendedViewPage();
+            ExtendedDeleteConfirmationPage extendedDeleteConfirmationPage = extendedViewPage.clickRequestForDeletion();
+            changeRequestSaveModal = extendedDeleteConfirmationPage.clickChangeRequestDelete();
+            changeRequestSaveModal.openAddChangesToExistingChangeRequestCollapse();
+            changeRequestSaveModal.selectExistingChangeRequest("Refactoring_MergedCR").select();
+            changeRequestPage = changeRequestSaveModal.clickSave();
+
+            // Approve and merge the change request containing the edition, the creation and the deletion.
+            ReviewContainer reviewContainer = changeRequestPage.clickReviewButton();
+            reviewContainer.selectApprove();
+            changeRequestPage = reviewContainer.save();
+            changeRequestPage = changeRequestPage.clickMergeButton();
+            assertEquals("Published", changeRequestPage.getStatusLabel());
+
+            // Check that the creation and the deletion have actually been applied.
+            testUtils.gotoPage(createdPage);
+            assertFalse(new ExtendedViewPage().isNewDocument());
+            testUtils.gotoPage(pageToDelete);
+            assertTrue(new ExtendedViewPage().isNewDocument());
+
+            // Perform a direct edition of the published page, after the merge, so that the file change of the
+            // merged change request becomes outdated with regard to the published version of the page.
+            testUtils.gotoPage(originalPage);
+            ExtendedViewPage publishedViewPage = new ExtendedViewPage();
+            ExtendedEditPage<CKEditor> publishedEditPage = publishedViewPage.clickStandardEdit(true);
+            publishedEditPage.getWrappedEditor().getRichTextArea()
+                .setContent("Content before refactoring.\nSome change.\nDirect edit after merge.");
+            publishedEditPage.clickSaveAndView();
+
+            // Rename the page: with the configuration enabled, the merged change request should follow.
+            testUtils.loginAsSuperAdmin();
+            ViewPage viewPage = testUtils.gotoPage(originalPage);
+            RenamePage renamePage = viewPage.rename();
+            DocumentReference renamedPage = new DocumentReference("MergedRefactoringPageRenamed",
+                testReference.getLastSpaceReference());
+            renamePage.getDocumentPicker().setTitle("MergedRefactoringPageRenamed");
+            CopyOrRenameOrDeleteStatusPage renameStatusPage = renamePage.clickRenameButton();
+            renameStatusPage = renameStatusPage.waitUntilFinished();
+            assertEquals("Done.", renameStatusPage.getInfoMessage());
+
+            testUtils.gotoPage(renamedPage);
+            ExtendedViewPage renamedViewPage = new ExtendedViewPage();
+            assertFalse(renamedViewPage.isNewDocument());
+            ChangeRequestLiveDataElement changeRequestLiveDataElement = renamedViewPage.openChangeRequestTab();
+            // The merged change request is now attached to the renamed page.
+            assertEquals(1, changeRequestLiveDataElement.countRows());
+
+            ChangeRequestLiveDataElement.ChangeRequestRowElement rowElement =
+                changeRequestLiveDataElement.getChangeRequests().get(0);
+            assertEquals("Refactoring_MergedCR", rowElement.getTitle());
+            // The status of the change request is not affected by the refactoring.
+            assertEquals("Published", rowElement.getStatus());
+
+            changeRequestPage = rowElement.gotoChangeRequest();
+            assertEquals("Published", changeRequestPage.getStatusLabel());
+
+            FileChangesPane fileChangesPane = changeRequestPage.openFileChanges();
+            List<String> listOfChangedFiles = fileChangesPane.getListOfChangedFilesReferences();
+            // The edited page (now refactored), the created page and the deleted page.
+            assertEquals(3, listOfChangedFiles.size());
+            assertTrue(listOfChangedFiles.contains(renamedPage.getLocalDocumentReference().toString()),
+                String.format("List of changed files [%s] didn't contain [%s]", listOfChangedFiles,
+                    renamedPage.getLocalDocumentReference()));
+            assertFalse(listOfChangedFiles.contains(originalPage.getLocalDocumentReference().toString()),
+                String.format("List of changed files [%s] still contained the old reference [%s]",
+                    listOfChangedFiles, originalPage.getLocalDocumentReference()));
+            assertTrue(listOfChangedFiles.contains(createdPage.getLocalDocumentReference().toString()),
+                String.format("List of changed files [%s] didn't contain the created page [%s]", listOfChangedFiles,
+                    createdPage.getLocalDocumentReference()));
+            assertTrue(listOfChangedFiles.contains(pageToDelete.getLocalDocumentReference().toString()),
+                String.format("List of changed files [%s] didn't contain the deleted page [%s]", listOfChangedFiles,
+                    pageToDelete.getLocalDocumentReference()));
+
+            // The diff of the refactored file change still reflects the actual content change performed by the
+            // change request, and it wasn't altered by the refactoring.
+            ChangeRequestEntityDiff contentEntityDiff =
+                fileChangesPane.getEntityDiff(renamedPage.getLocalDocumentReference().toString(), "Page properties");
+            List<String> contentDiff = contentEntityDiff.getDiff("Content");
+            assertFalse(contentDiff.isEmpty());
+            assertTrue(contentDiff.stream().anyMatch(line -> line.contains("Some change")),
+                String.format("Diff [%s] doesn't contain the expected change.", contentDiff));
+
+            // The refactored file change is outdated since the published page was edited after the merge.
+            assertTrue(fileChangesPane.isDiffOutdated(renamedPage.getLocalDocumentReference().toString()));
+        } finally {
+            // Restore the default configuration so it doesn't leak onto other tests.
+            testUtils.loginAsSuperAdmin();
+            testUtils.updateObject(Arrays.asList("ChangeRequest", "Code"), "Configuration",
+                "ChangeRequest.Code.ConfigurationClass", 0,
+                "refactorMergedChangeRequests", 0);
+        }
+    }
 }