Skip to content

Always use topological sort for published pages #22896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
oguzkocer wants to merge 3 commits into
base: trunk
Choose a base branch
from
+23 −71
Open

Always use topological sort for published pages #22896

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0656f70
Always use topological sort for published pages
oguzkocer May 26, 2026
50a70a9
Remove unused DEFAULT_INDENT constant
oguzkocer May 26, 2026
6580efc
Remove date-based sort test and rename topological sort test
oguzkocer May 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 22 additions & 25 deletions WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageListViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +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(
private val createPageListItemLabelsUseCase: CreatePageListItemLabelsUseCase,
private val postModelUploadUiStateUseCase: PostModelUploadUiStateUseCase,
Expand Down Expand Up @@ -317,23 +314,16 @@ class PageListViewModel @Inject constructor(
}

private fun preparePublishedPages(pages: List<PageModel>, actionsEnabled: Boolean): List<PageItem> {
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(
Expand All @@ -345,7 +335,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,
Expand Down Expand Up @@ -458,18 +448,25 @@ class PageListViewModel @Inject constructor(

private fun topologicalSort(
pages: List<PageModel>,
listType: PageListType,
parent: PageModel? = null
listType: PageListType
): List<PageModel> {
val sortedList = mutableListOf<PageModel>()
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)
}
val childrenByParentId = pages
.filterNot { isRoot(it) }
.groupBy { it.parent?.remoteId }
val roots = pages.filter { isRoot(it) }

fun collect(page: PageModel, result: MutableList<PageModel>) {
result += page
childrenByParentId[page.remoteId]?.forEach {
collect(it, result)
}
}
return sortedList

return buildList { roots.forEach { collect(it, this) } }
}

private fun getPageItemIndent(page: PageModel?): Int {
Expand Down
47 changes: 1 addition & 46 deletions WordPress/src/test/java/org/wordpress/android/viewmodel/pages/PageListViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,52 +238,7 @@ class PageListViewModelTest : BaseUnitTest() {
}

@Test
fun `sorts 100 or more pages by date DESC`() {
val pages = MutableLiveData<List<PageModel>>()
whenever(pagesViewModel.pages).thenReturn(pages)

viewModel.start(PUBLISHED, pagesViewModel)

val result = mutableListOf<Triple<List<PageItem>, 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<PageModel>()
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<List<PageModel>>()
whenever(pagesViewModel.pages).thenReturn(pages)

Expand Down
Loading