XWIKI-22627: Pinned Pages are lost on space move

(cherry picked from commit 6c9b77e)

xwiki-platform-core/xwiki-platform-index/xwiki-platform-index-test/xwiki-platform-index-test-docker/pom.xml

@@ -54,6 +54,14 @@
       <version>${project.version}</version>
       <type>xar</type>
     </dependency>
+    <!-- Needed to test the Pinned Child Pages feature (provides the RequireJS configuration for some of the JavaScript
+      modules used by Pinned Child Pages, such as jQueryUI) -->
+    <dependency>
+      <groupId>${project.groupId}</groupId>
+      <artifactId>xwiki-platform-panels-ui</artifactId>
+      <version>${project.version}</version>
+      <type>xar</type>
+    </dependency>
     <!-- ================================
          Test only dependencies
          ================================ -->

xwiki-platform-core/xwiki-platform-index/xwiki-platform-index-test/xwiki-platform-index-test-docker/src/test/it/org/xwiki/index/test/ui/docker/AllIT.java

@@ -55,4 +55,10 @@ class NestedOrphanedPagesIT extends OrphanedPagesIT
     class NestedDocumentsMacroIT extends DocumentsMacroIT
     {
     }
+
+    @Nested
+    @DisplayName("Pinned Pages UI")
+    class NestedPinnedPagesIT extends PinnedPagesIT
+    {
+    }
 }

xwiki-platform-core/xwiki-platform-index/xwiki-platform-index-test/xwiki-platform-index-test-docker/src/test/it/org/xwiki/index/test/ui/docker/PinnedPagesIT.java

@@ -0,0 +1,146 @@
+/*
+ * See the NOTICE file distributed with this work for additional
+ * information regarding copyright ownership.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
+package org.xwiki.index.test.ui.docker;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.junit.jupiter.api.Order;
+import org.junit.jupiter.api.Test;
+import org.openqa.selenium.Keys;
+import org.xwiki.index.tree.test.po.BreadcrumbTree;
+import org.xwiki.index.tree.test.po.DocumentTreeElement;
+import org.xwiki.index.tree.test.po.PinnedPagesAdministrationSectionPage;
+import org.xwiki.model.reference.DocumentReference;
+import org.xwiki.model.reference.EntityReference;
+import org.xwiki.model.reference.SpaceReference;
+import org.xwiki.test.docker.junit5.TestReference;
+import org.xwiki.test.docker.junit5.UITest;
+import org.xwiki.test.ui.TestUtils;
+import org.xwiki.test.ui.po.RenamePage;
+import org.xwiki.test.ui.po.SuggestInputElement;
+import org.xwiki.tree.test.po.TreeNodeElement;
+
+/**
+ * Functional tests for the pinned pages feature.
+ *
+ * @version $Id$
+ */
+@UITest
+class PinnedPagesIT
+{
+    @Test
+    @Order(1)
+    void refactorPinnedPages(TestUtils setup, TestReference testReference)
+    {
+        setup.loginAsSuperAdmin();
+
+        // Cleanup.
+        setup.deletePage(testReference, true);
+
+        // Create the pages to be pinned.
+        DocumentReference levelOne =
+            new DocumentReference("WebHome", new SpaceReference("LevelOne", testReference.getLastSpaceReference()));
+        DocumentReference levelTwo =
+            new DocumentReference("WebHome", new SpaceReference("LevelTwo", levelOne.getLastSpaceReference()));
+        DocumentReference zero = new DocumentReference("000", levelTwo.getLastSpaceReference());
+        DocumentReference alice = new DocumentReference("Alice", levelTwo.getLastSpaceReference());
+        DocumentReference bob =
+            new DocumentReference("WebHome", new SpaceReference("Bob", levelTwo.getLastSpaceReference()));
+        DocumentReference carol = new DocumentReference("Carol", levelTwo.getLastSpaceReference());
+        DocumentReference denis =
+            new DocumentReference("WebHome", new SpaceReference("Denis", levelTwo.getLastSpaceReference()));
+
+        setup.createPage(testReference, "", "");
+        setup.createPage(levelOne, "", "");
+        setup.createPage(levelTwo, "", "");
+        setup.createPage(zero, "", "");
+        setup.createPage(alice, "", "");
+        setup.createPage(bob, "", "");
+        setup.createPage(carol, "", "");
+        setup.createPage(denis, "", "");
+
+        // Pin the pages.
+        PinnedPagesAdministrationSectionPage pinnedPagesAdminSection =
+            PinnedPagesAdministrationSectionPage.gotoPage(levelTwo.getLastSpaceReference());
+        SuggestInputElement pinnedPagesPicker = pinnedPagesAdminSection.getPinnedPagesPicker();
+        pinnedPagesPicker.sendKeys("Carol").waitForSuggestions().selectByIndex(0);
+        pinnedPagesPicker.sendKeys("Bob").waitForSuggestions().selectByIndex(0);
+        pinnedPagesPicker.sendKeys("Denis").waitForSuggestions().selectByIndex(0);
+        pinnedPagesPicker.sendKeys("Alice").waitForSuggestions().selectByIndex(0);
+        // Close the suggestions dropdown because it may hide the save button.
+        pinnedPagesPicker.sendKeys(Keys.ESCAPE);
+        pinnedPagesAdminSection.clickSave();
+
+        // Verify the result.
+        setup.gotoPage(levelTwo);
+        DocumentTreeElement tree = BreadcrumbTree.open("LevelTwo");
+        List<String> children =
+            tree.getNode(levelTwo).getChildren().stream().map(TreeNodeElement::getLabel).collect(Collectors.toList());
+        assertEquals(List.of("Carol", "Bob", "Denis", "Alice", "000", "Page Administration"), children);
+
+        // Refactor the pinned pages.
+        setup.deletePage(bob);
+        renamePage(setup, carol, new DocumentReference("Charlie", carol.getLastSpaceReference()));
+        movePage(setup, alice, levelOne);
+        DocumentReference levelOneRenamed = new DocumentReference("WebHome",
+            new SpaceReference("LevelOneRenamed", testReference.getLastSpaceReference()));
+        renamePage(setup, levelOne, levelOneRenamed);
+
+        // Verify the result.
+        DocumentReference levelTwoRenamed =
+            new DocumentReference("WebHome", new SpaceReference("LevelTwo", levelOneRenamed.getLastSpaceReference()));
+        pinnedPagesAdminSection =
+            PinnedPagesAdministrationSectionPage.gotoPage(levelTwoRenamed.getLastSpaceReference());
+        pinnedPagesPicker = pinnedPagesAdminSection.getPinnedPagesPicker();
+        assertEquals(List.of("Charlie", "Denis/"), pinnedPagesPicker.getValues());
+
+        setup.gotoPage(levelTwoRenamed);
+        tree = BreadcrumbTree.open("LevelTwo");
+        children = tree.getNode(levelTwoRenamed).getChildren().stream().map(TreeNodeElement::getLabel)
+            .collect(Collectors.toList());
+        assertEquals(List.of("Charlie", "Denis", "000", "Page Administration"), children);
+    }
+
+    private void movePage(TestUtils setup, DocumentReference source, DocumentReference target)
+    {
+        if ("WebHome".equals(source.getName())) {
+            renamePage(setup, source, new DocumentReference("WebHome",
+                new SpaceReference(source.getLastSpaceReference().getName(), target.getLastSpaceReference())));
+        } else {
+            renamePage(setup, source, new DocumentReference(source.getName(), target.getLastSpaceReference()));
+        }
+    }
+
+    private void renamePage(TestUtils setup, DocumentReference source, DocumentReference target)
+    {
+        EntityReference actualTarget = target;
+        if ("WebHome".equals(actualTarget.getName())) {
+            actualTarget = actualTarget.getParent();
+        }
+
+        RenamePage renamePage = setup.gotoPage(source).rename();
+        renamePage.getDocumentPicker().setTitle(actualTarget.getName())
+            .setParent(setup.serializeLocalReference(actualTarget.getParent()));
+        renamePage.clickRenameButton().waitUntilFinished();
+    }
+}

xwiki-platform-core/xwiki-platform-index/xwiki-platform-index-tree/xwiki-platform-index-tree-api/pom.xml

@@ -32,7 +32,7 @@
   <description>Defines multiple document hierarchies (nested pages, nested spaces, parent-child, etc.) that can be displayed using the document tree macro.</description>
   <packaging>jar</packaging>
   <properties>
-    <xwiki.jacoco.instructionRatio>0.44</xwiki.jacoco.instructionRatio>
+    <xwiki.jacoco.instructionRatio>0.45</xwiki.jacoco.instructionRatio>
     <!-- Name to display by the Extension Manager -->
     <xwiki.extension.name>Document Tree API</xwiki.extension.name>
   </properties>

xwiki-platform-core/xwiki-platform-index/xwiki-platform-index-tree/xwiki-platform-index-tree-api/src/main/java/org/xwiki/index/tree/internal/nestedpages/pinned/PinnedChildPagesListener.java

@@ -19,6 +19,7 @@
  */
 package org.xwiki.index.tree.internal.nestedpages.pinned;
 
+import java.util.Collection;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Objects;
@@ -29,18 +30,23 @@
 
 import org.xwiki.bridge.event.DocumentDeletedEvent;
 import org.xwiki.component.annotation.Component;
+import org.xwiki.job.Job;
 import org.xwiki.job.JobContext;
+import org.xwiki.job.Request;
+import org.xwiki.model.EntityType;
 import org.xwiki.model.reference.DocumentReference;
 import org.xwiki.model.reference.EntityReference;
+import org.xwiki.model.reference.EntityReferenceProvider;
 import org.xwiki.observation.event.AbstractLocalEventListener;
 import org.xwiki.observation.event.Event;
 import org.xwiki.refactoring.event.DocumentRenamedEvent;
-import org.xwiki.refactoring.internal.job.DeleteJob;
+import org.xwiki.refactoring.job.MoveRequest;
+import org.xwiki.refactoring.job.RefactoringJobs;
 
 import com.xpn.xwiki.doc.XWikiDocument;
 
 /**
- * Update the list of pinned child pages when a pinned page is delete, moved or renamed.
+ * Update the list of pinned child pages when a pinned page is deleted, moved or renamed.
  * 
  * @version $Id$
  * @since 16.4.0RC1
@@ -56,6 +62,9 @@ public class PinnedChildPagesListener extends AbstractLocalEventListener
     @Inject
     private JobContext jobContext;
 
+    @Inject
+    private EntityReferenceProvider defaultEntityReferenceProvider;
+
     /**
      * Default constructor.
      */
@@ -70,14 +79,20 @@ public void processLocalEvent(Event event, Object source, Object data)
         if (event instanceof DocumentRenamedEvent) {
             DocumentRenamedEvent documentRenamedEvent = (DocumentRenamedEvent) event;
             onDocumentRenamed(documentRenamedEvent.getSourceReference(), documentRenamedEvent.getTargetReference());
-        } else if (event instanceof DocumentDeletedEvent && this.jobContext.getCurrentJob() instanceof DeleteJob) {
+        } else if (event instanceof DocumentDeletedEvent && RefactoringJobs.DELETE.equals(getCurrentJobType())) {
             // A delete event is triggered before each document rename event, but we don't want to remove the pinned
             // child page in this case because we won't be able to replace the pinned child page later when the rename
             // event is triggered. For this reason we have to check if this is really a delete and not a rename.
             onDocumentDeleted(((XWikiDocument) source).getDocumentReference());
         }
     }
 
+    /**
+     * When a document is deleted (not moved or renamed) we need to remove it from the list of pinned child pages of its
+     * parent document.
+     *
+     * @param documentReference the reference of the document that was deleted
+     */
     private void onDocumentDeleted(DocumentReference documentReference)
     {
         EntityReference parentReference = this.pinnedChildPagesManager.getParent(documentReference);
@@ -87,24 +102,90 @@ private void onDocumentDeleted(DocumentReference documentReference)
         }
     }
 
+    /**
+     * When a document is renamed we have 3 cases:
+     * <ul>
+     * <li>the document is the explicit target of a rename or move job:
+     * <ul>
+     * <li>its parent doesn't change (i.e. the document is only renamed, not moved, which means the document is the
+     * target of a rename job): in this case we need to rename the corresponding entry in the list of pinned child
+     * pages</li>
+     * <li>its parent changes (i.e. the document is moved but its parent remains in place): in this case we need to
+     * remove the document from the list of pinned child pages of the old parent</li>
+     * </ul>
+     * </li>
+     * <li>the document is moved / renamed as a side effect of renaming / moving one of its ancestors (which is the
+     * actual target of the rename / move job): we don't have to do anything in this case because the pinned pages store
+     * (i.e. the WebPreferences page) is moved to the new location as well</li>
+     * </ul>
+     *
+     * @param oldReference the old reference of the document
+     * @param newReference the new reference of the document
+     */
     private void onDocumentRenamed(DocumentReference oldReference, DocumentReference newReference)
     {
-        EntityReference oldParentReference = this.pinnedChildPagesManager.getParent(oldReference);
-        EntityReference newParentReference = this.pinnedChildPagesManager.getParent(newReference);
-        if (Objects.equals(oldParentReference, newParentReference)) {
-            List<DocumentReference> pinnedChildPages = getMutablePinnedChildPages(oldParentReference);
-            int index = pinnedChildPages.indexOf(oldReference);
-            if (index >= 0) {
-                pinnedChildPages.set(index, newReference);
-                this.pinnedChildPagesManager.setPinnedChildPages(oldParentReference, pinnedChildPages);
+        if (isRenameOrMoveJobTarget(oldReference)) {
+            EntityReference oldParentReference = this.pinnedChildPagesManager.getParent(oldReference);
+            EntityReference newParentReference = this.pinnedChildPagesManager.getParent(newReference);
+            if (Objects.equals(oldParentReference, newParentReference)) {
+                // The document is only renamed, not moved.
+                List<DocumentReference> pinnedChildPages = getMutablePinnedChildPages(oldParentReference);
+                int index = pinnedChildPages.indexOf(oldReference);
+                if (index >= 0) {
+                    pinnedChildPages.set(index, newReference);
+                    this.pinnedChildPagesManager.setPinnedChildPages(oldParentReference, pinnedChildPages);
+                }
+            } else {
+                // The document is moved without its parent.
+                onDocumentDeleted(oldReference);
             }
-        } else {
-            onDocumentDeleted(oldReference);
         }
     }
 
     private List<DocumentReference> getMutablePinnedChildPages(EntityReference parentReference)
     {
         return new LinkedList<>(this.pinnedChildPagesManager.getPinnedChildPages(parentReference));
     }
+
+    /**
+     * Checks if the current refactoring job is a rename or move operation that targets explicitly the specified
+     * document.
+     *
+     * @param documentReference the reference of the document before the rename / move operation
+     * @return {@code true} if the specified document is the explicit target of the current rename or move job,
+     *         {@code false} otherwise
+     */
+    private boolean isRenameOrMoveJobTarget(DocumentReference documentReference)
+    {
+        String currentJobType = getCurrentJobType();
+        if (RefactoringJobs.RENAME.equals(currentJobType) || RefactoringJobs.MOVE.equals(currentJobType)) {
+            Request request = this.jobContext.getCurrentJob().getRequest();
+            if (request instanceof MoveRequest) {
+                MoveRequest moveRequest = (MoveRequest) request;
+                Collection<EntityReference> movedEntities = moveRequest.getEntityReferences();
+                return contains(movedEntities, documentReference);
+            }
+        }
+        return false;
+    }
+
+    private String getCurrentJobType()
+    {
+        Job job = this.jobContext.getCurrentJob();
+        return job != null ? job.getType() : null;
+    }
+
+    private boolean contains(Collection<EntityReference> entityReferences, EntityReference entityReference)
+    {
+        if (!entityReferences.contains(entityReference) && EntityType.DOCUMENT.equals(entityReference.getType())) {
+            // If the given entity reference is a reference of a space home page then look for the space reference as
+            // well because the refactoring API accepts a space reference when you want to rename / move an entire
+            // space.
+            String spaceHomePageName =
+                this.defaultEntityReferenceProvider.getDefaultReference(EntityType.DOCUMENT).getName();
+            return spaceHomePageName.equals(entityReference.getName())
+                && entityReferences.contains(entityReference.getParent());
+        }
+        return true;
+    }
 }