From 0656f70b7d8c3ff1ab183b0674e31808b95b6317 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Tue, 26 May 2026 02:22:01 -0400 Subject: [PATCH 1/3] Always use topological sort for published pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #10476 introduced `MAX_TOPOLOGICAL_PAGE_COUNT = 100` to cap the topological (hierarchical) sort and fall back to sorting by publish date for sites with 100+ published pages. This was done to match Calypso's behavior at the time. The date-based fallback produced a completely different ordering than what users see on the web — pages lost their parent/child hierarchy and indentation, and edited pages could jump to the top of the list due to transient local state during save. This commit: - Removes `MAX_TOPOLOGICAL_PAGE_COUNT` and the date-based fallback so all published pages are sorted hierarchically regardless of count. - Rewrites `topologicalSort` to build a `parentId → children` lookup map in a single `groupBy` pass then walk the tree once, instead of re-scanning the full page list on every recursive call. --- .../viewmodel/pages/PageListViewModel.kt | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageListViewModel.kt index 97b9650d4f6b..876960776fa2 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageListViewModel.kt @@ -57,7 +57,6 @@ import org.wordpress.android.viewmodel.uistate.ProgressBarUiState import javax.inject.Inject import javax.inject.Named -private const val MAX_TOPOLOGICAL_PAGE_COUNT = 100 private const val DEFAULT_INDENT = 0 class PageListViewModel @Inject constructor( @@ -317,23 +316,16 @@ class PageListViewModel @Inject constructor( } private fun preparePublishedPages(pages: List, actionsEnabled: Boolean): List { - val shouldSortTopologically = pages.size < MAX_TOPOLOGICAL_PAGE_COUNT - val sortedPages = (if (shouldSortTopologically) { - topologicalSort(pages.sortedBy { !(it.isHomepage && it.parent == null) }, listType = PUBLISHED) - } else { - pages.sortedByDescending { it.date }.sortedBy { !it.isHomepage } - }) + val sortedPages = topologicalSort( + pages.sortedBy { !(it.isHomepage && it.parent == null) }, + listType = PUBLISHED + ) val showVirtualHomepage = siteEditorMVPFeatureConfig.isEnabled() && isBlockBasedTheme.value return sortedPages .let { if (showVirtualHomepage) it.filterNot { page -> page.isHomepage } else it } .map { - val pageItemIndent = if (shouldSortTopologically) { - getPageItemIndent(it) - } else { - DEFAULT_INDENT - } val itemUiStateData = createItemUiStateData(it) val author = getAuthorName(it.post) PublishedPage( @@ -345,7 +337,7 @@ class PageListViewModel @Inject constructor( date = it.date, labels = itemUiStateData.labels, labelsColor = itemUiStateData.labelsColor, - indent = pageItemIndent, + indent = getPageItemIndent(it), imageUrl = getFeaturedImageUrl(it.featuredImageId), actions = itemUiStateData.actions, actionsEnabled = actionsEnabled, @@ -458,18 +450,25 @@ class PageListViewModel @Inject constructor( private fun topologicalSort( pages: List, - listType: PageListType, - parent: PageModel? = null + listType: PageListType ): List { - val sortedList = mutableListOf() - pages.filter { - it.parent?.remoteId == parent?.remoteId || - (parent == null && !listType.pageStatuses.contains(it.parent?.status)) - }.forEach { - sortedList += it - sortedList += topologicalSort(pages, listType, it) + val isRoot = { page: PageModel -> + page.parent?.remoteId == null || + !listType.pageStatuses.contains(page.parent?.status) } - return sortedList + val childrenByParentId = pages + .filterNot { isRoot(it) } + .groupBy { it.parent?.remoteId } + val roots = pages.filter { isRoot(it) } + + fun collect(page: PageModel, result: MutableList) { + result += page + childrenByParentId[page.remoteId]?.forEach { + collect(it, result) + } + } + + return buildList { roots.forEach { collect(it, this) } } } private fun getPageItemIndent(page: PageModel?): Int { From 50a70a9ee20f64ba607c8cf9a6250fed8e1ef0ff Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Tue, 26 May 2026 02:43:38 -0400 Subject: [PATCH 2/3] Remove unused DEFAULT_INDENT constant --- .../org/wordpress/android/viewmodel/pages/PageListViewModel.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageListViewModel.kt index 876960776fa2..0dc4d2f3fcf9 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageListViewModel.kt @@ -57,8 +57,6 @@ import org.wordpress.android.viewmodel.uistate.ProgressBarUiState import javax.inject.Inject import javax.inject.Named -private const val DEFAULT_INDENT = 0 - class PageListViewModel @Inject constructor( private val createPageListItemLabelsUseCase: CreatePageListItemLabelsUseCase, private val postModelUploadUiStateUseCase: PostModelUploadUiStateUseCase, From 6580efc8946da499963464fd1bd147adfc1822a7 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Tue, 26 May 2026 03:50:27 -0400 Subject: [PATCH 3/3] Remove date-based sort test and rename topological sort test The `sorts 100 or more pages by date DESC` test validated the date-based fallback that no longer exists. The remaining topological sort test is renamed since it now applies to all page counts. --- .../viewmodel/pages/PageListViewModelTest.kt | 47 +------------------ 1 file changed, 1 insertion(+), 46 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/PageListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/PageListViewModelTest.kt index 13d3fa2d60e6..2e4547fd44dd 100644 --- a/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/PageListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/PageListViewModelTest.kt @@ -238,52 +238,7 @@ class PageListViewModelTest : BaseUnitTest() { } @Test - fun `sorts 100 or more pages by date DESC`() { - val pages = MutableLiveData>() - whenever(pagesViewModel.pages).thenReturn(pages) - - viewModel.start(PUBLISHED, pagesViewModel) - - val result = mutableListOf, Boolean, Boolean>>() - - viewModel.pages.observeForever { result.add(it) } - - val earlyPages = (0..30).map { buildPageModel(it, Date(HOUR_IN_MILLISECONDS * it)) } - val latePages = (31..60).map { buildPageModel(it, Date(100 * HOUR_IN_MILLISECONDS * it)) } - val middlePages = (61..96).map { buildPageModel(it, Date(10 * HOUR_IN_MILLISECONDS * it)) } - val earlyChild = buildPageModel(97, Date(40 * HOUR_IN_MILLISECONDS), earlyPages[0]) - val middleChild = buildPageModel(98, Date(1000 * HOUR_IN_MILLISECONDS), middlePages[0]) - val lateChild = buildPageModel(99, Date(7000 * HOUR_IN_MILLISECONDS), latePages[0]) - val children = listOf(middleChild, earlyChild, lateChild) - - val pageModels = mutableListOf() - pageModels.addAll(middlePages) - pageModels.addAll(latePages) - pageModels.addAll(children) - pageModels.addAll(earlyPages) - pages.value = pageModels - - assertThat(result).hasSize(1) - val pageItems = result[0].first - assertThat(pageItems).hasSize(102) - assertPublishedPage(pageItems[0], lateChild) - for (index in 1..latePages.size) { - assertPublishedPage(pageItems[index], latePages[latePages.size - index]) - } - assertPublishedPage(pageItems[31], middleChild) - for (index in 1..middlePages.size) { - assertPublishedPage(pageItems[31 + index], middlePages[middlePages.size - index]) - } - assertPublishedPage(pageItems[68], earlyChild) - for (index in 1..earlyPages.size) { - assertPublishedPage(pageItems[68 + index], earlyPages[earlyPages.size - index]) - } - assertDivider(pageItems[100]) - assertDivider(pageItems[101]) - } - - @Test - fun `sorts up to 99 pages topologically`() { + fun `sorts pages topologically`() { val pages = MutableLiveData>() whenever(pagesViewModel.pages).thenReturn(pages)