Fix #22283: child pages are not moved under parent pages

RELEASE-NOTES.txt

@@ -17,6 +17,7 @@
 * [*] Fix an issue with BlogDashboardPersonalizationService being used on the background thread [#22335]
 * [***] Block Editor: Avoid keyboard dismiss when interacting with text blocks [https://github.com/WordPress/gutenberg/pull/57070]
 * [**] Block Editor: Auto-scroll upon block insertion [https://github.com/WordPress/gutenberg/pull/57273]
+* [**] Fix an issue in Pages List where the pages are not displayed in a hierarchical order [#22338]
 
 23.9
 -----

WordPress/Classes/Utility/PageTree.swift

@@ -110,7 +110,6 @@ final class PageTree {
     /// This function assumes none of array elements already exists in the current page tree.
     func add(_ newPages: [Page]) {
         let newNodes = newPages.map { TreeNode(page: $0) }
-        relocateOrphans(to: newNodes)
 
         // First try to constrcuture a smaller subtree from the given pages, then move the new subtree to the existing
         // page tree (`self`).
@@ -151,6 +150,8 @@ final class PageTree {
     }
 
     private func add(_ newNodes: [TreeNode]) {
+        relocateOrphans(to: newNodes)
+
         newNodes.forEach { newNode in
             let parentID = newNode.pageData.parentID ?? 0
 
@@ -177,13 +178,17 @@ final class PageTree {
 
     /// Move all the nodes in the given argument to the current page tree.
     private func merge(subtree: PageTree) {
-        var parentIDs = subtree.nodes.reduce(into: Set()) { $0.insert($1.pageData.parentID ?? 0) }
+        let subtreeNodes = subtree.nodes
+
+        relocateOrphans(to: subtreeNodes)
+
+        var parentIDs = subtreeNodes.reduce(into: Set()) { $0.insert($1.pageData.parentID ?? 0) }
         // No need to look for root level
         parentIDs.remove(0)
         // Look up parent nodes upfront, to avoid repeated iteration for each node in `subtree`.
         let parentNodes = findNodes(postIDs: parentIDs)
 
-        subtree.nodes.forEach { newNode in
+        subtreeNodes.forEach { newNode in
             let parentID = newNode.pageData.parentID ?? 0
 
             // If the new node is at the root level, then simply add it as a child

WordPress/WordPressTest/PagesListTests.swift

@@ -22,6 +22,11 @@ class PagesListTests: CoreDataTestCase {
         try makeAssertions(pages: pages)
     }
 
+    func testOneNestedListInReversedOrder() throws {
+        let pages = parentPage(childrenCount: 17, additionalLevels: 7).reversed()
+        try makeAssertions(pages: Array(pages))
+    }
+
     func testManyNestedLists() throws {
         let groups = [
             parentPage(childrenCount: 5),
@@ -108,7 +113,33 @@ class PagesListTests: CoreDataTestCase {
         try makeAssertions(pages: pages)
     }
 
-    func testHierachyListRepresentationRoundtrip() throws {
+    func testDistantChildAndParentPages() throws {
+        let child = PageBuilder(mainContext).build()
+        child.postID = NSNumber(value: randomID.next())
+        child.parentID = NSNumber(value: randomID.next())
+
+        let parent = PageBuilder(mainContext).build()
+        parent.postID = child.parentID
+        parent.parentID = 0
+
+        let manyPages = parentPage(childrenCount: 17, additionalLevels: 7)
+
+        // Test 1: place the child page at the begining and the parent page at the end.
+        var sorted = try PageTree.hierarchyList(of: [child] + manyPages + [parent])
+        XCTAssertEqual(parent.hierarchyIndex, 0)
+        XCTAssertEqual(child.hierarchyIndex, 1)
+        // The child page should follow the parent page in the sorted list
+        try XCTAssertEqual(XCTUnwrap(sorted.firstIndex(of: parent)) + 1, XCTUnwrap(sorted.firstIndex(of: child)))
+
+        // Test 2: place the child page at the end and the parent page at the begining.
+        sorted = try PageTree.hierarchyList(of: [parent] + manyPages + [child])
+        XCTAssertEqual(parent.hierarchyIndex, 0)
+        XCTAssertEqual(child.hierarchyIndex, 1)
+        // The child page should follow the parent page in the sorted list
+        try XCTAssertEqual(XCTUnwrap(sorted.firstIndex(of: parent)) + 1, XCTUnwrap(sorted.firstIndex(of: child)))
+    }
+
+    func testHierarchyListRepresentationRoundtrip() throws {
         let roundtrip: (String) throws -> Void = { string in
             let pages = try Array<Page>(hierarchyListRepresentation: string, context: self.mainContext)
             try XCTAssertEqual(PageTree.hierarchyList(of: pages).hierarchyListRepresentation(), string)
@@ -175,10 +206,27 @@ class PagesListTests: CoreDataTestCase {
         _ = pages.sorted { ($0.postID?.int64Value ?? 0) < ($1.postID?.int64Value ?? 0) }
         NSLog("Array.sort took \(String(format: "%.3f", (CFAbsoluteTimeGetCurrent() - start) * 1000)) millisecond to process \(pages.count) pages")
 
-        let originalIDs = original.map { $0.postID! }
-        let newIDs = new.map { $0.postID! }
-        let diff = originalIDs.difference(from: newIDs).inferringMoves()
-        XCTAssertTrue(diff.count == 0, "Unexpected diff: \(diff)", file: file, line: line)
+        // Compare the two implementions to make sure their results are similar. The pages don'n't need to be in the exact same order,
+        // but each hierachy level should contain the same child pages in it.
+
+        let originalList = HierarchyList(pages: original)
+        let newList = HierarchyList(pages: new)
+
+        // They have the same hierachy level.
+        XCTAssertEqual(originalList.numberOfLevels, newList.numberOfLevels)
+
+        // For each hierachy level, the same child pages are present in both results, without the need of being in the same order.
+        for level in 1...(originalList.numberOfLevels) {
+            let pagesAtLevelOriginal = originalList.pages(atLevel: level)
+            let pagesAtLevelNew = newList.pages(atLevel: level)
+            XCTAssertEqual(Set(pagesAtLevelOriginal.keys), Set(pagesAtLevelNew.keys), "The parent page ids in each level should be the same")
+
+            for parentPageID in pagesAtLevelOriginal.keys {
+                let childrenPageIDsOriginal = try XCTUnwrap(pagesAtLevelOriginal[parentPageID]).map { $0.postID }
+                let childrenPageIDsNew = try XCTUnwrap(pagesAtLevelNew[parentPageID]).map { $0.postID }
+                XCTAssertEqual(Set(childrenPageIDsOriginal), Set(childrenPageIDsNew), "The children page ids in each level should be the same")
+            }
+        }
     }
 }
 
@@ -252,3 +300,31 @@ private extension Array where Element == Page {
         self = pages
     }
 }
+
+private struct HierarchyList {
+    let pages: [Page]
+
+    var numberOfLevels: Int {
+        pages.map { $0.hierarchyIndex }.max()! + 1
+    }
+
+    func pages(atLevel level: Int) -> [NSNumber: [Page]] {
+        var result = [NSNumber: [Page]]()
+        for page in pages {
+            guard page.hierarchyIndex + 1 == level else {
+                continue
+            }
+
+            let parentID = page.parentID ?? 0
+            result[parentID, default: []].append(page)
+        }
+        return result
+    }
+
+    func print() {
+        for page in pages {
+            Swift.print(String(repeating: " ", count: page.hierarchyIndex * 2), terminator: "|- ")
+            Swift.print("post id: \(page.postID!), parent id: \(page.parentID ?? 0)")
+        }
+    }
+}