From 2fc6f16668224aa63243e5467f66a37b6d398aad Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 2 Jul 2026 12:22:09 -0400 Subject: [PATCH 01/21] Add wordpress-rs comments list (Phase 2a: core browsing) Add a Compose comments list backed by wordpress-rs, following the postsrs/pagesrs pattern. There's no rs mobile-cache support for comments yet, so each tab pages directly against /wp/v2/comments using the response's nextPageParams as the cursor. - 5 filter tabs (All/Pending/Approved/Spam/Trashed). The All and Approved tabs use CommentStatus.Custom("all"/"approve") because WP_Comment_Query doesn't recognise the "approved" value the rs enum serialises to - Rows show avatar, bold "author on post" title (post titles resolved via a batched sparse-field request), snippet, date, pending indicator - Pull-to-refresh, load-more, retry snackbars, shimmer/empty/error states - Tapping a row opens the rs comment detail; the list refreshes on return - Gated in ActivityLauncher.viewUnifiedComments by RS_UNIFIED_COMMENTS + site capability (WP.com REST or application password), falling back to the legacy list; the now-dead per-tap rs gate inside the legacy list fragment is removed Batch moderation and search follow in stacked PRs. Co-Authored-By: Claude Fable 5 --- WordPress/src/main/AndroidManifest.xml | 4 + .../android/ui/ActivityLauncher.java | 23 +- .../comments/unified/CommentsRsDataSource.kt | 126 ++++++++- .../unified/UnifiedCommentListFragment.kt | 33 +-- .../ui/commentsrs/CommentsRsListActivity.kt | 76 +++++ .../ui/commentsrs/CommentsRsListEvent.kt | 10 + .../ui/commentsrs/CommentsRsListTab.kt | 48 ++++ .../ui/commentsrs/CommentsRsListUiState.kt | 34 +++ .../ui/commentsrs/CommentsRsListViewModel.kt | 229 +++++++++++++++ .../commentsrs/screens/CommentsRsListItem.kt | 143 ++++++++++ .../screens/CommentsRsListScreen.kt | 122 ++++++++ .../screens/CommentsRsTabListScreen.kt | 224 +++++++++++++++ .../android/ui/main/BaseAppCompatActivity.kt | 2 + .../unified/CommentsRsListMappingTest.kt | 110 ++++++++ .../ui/commentsrs/CommentsRsListTabTest.kt | 26 ++ .../commentsrs/CommentsRsListViewModelTest.kt | 266 ++++++++++++++++++ 16 files changed, 1445 insertions(+), 31 deletions(-) create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListEvent.kt create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListTab.kt create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt create mode 100644 WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsListMappingTest.kt create mode 100644 WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListTabTest.kt create mode 100644 WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt diff --git a/WordPress/src/main/AndroidManifest.xml b/WordPress/src/main/AndroidManifest.xml index 72b6c62ba88c..712f239dc07c 100644 --- a/WordPress/src/main/AndroidManifest.xml +++ b/WordPress/src/main/AndroidManifest.xml @@ -313,6 +313,10 @@ android:theme="@style/WordPress.NoActionBar" android:launchMode="singleTop" android:label="@string/comments"/> + () + data class RsComment( val authorName: String, val authorAvatarUrl: String, @@ -38,12 +49,36 @@ class CommentsRsDataSource @Inject constructor( val status: CommentStatus ) + /** One comment list row, mapped from [CommentWithViewContext]. */ + data class RsCommentListItem( + val remoteCommentId: Long, + val authorName: String, + val authorAvatarUrl: String, + val dateGmt: Date, + val contentHtml: String, + val postId: Long, + val status: CommentStatus + ) + /** Result of a write request, carrying the server error message when one is available. */ sealed interface RsResult { object Success : RsResult data class Error(val message: String?) : RsResult } + /** Result of fetching one page of the comment list. */ + sealed interface RsCommentsPageResult { + data class Success( + val comments: List, + /** Ready-made params for the next page; null when this was the last page. */ + val nextPageParams: CommentListParams?, + /** Total matching comments from the X-WP-Total header, when the server provides it. */ + val total: Int? + ) : RsCommentsPageResult + + data class Error(val message: String?) : RsCommentsPageResult + } + suspend fun getComment(site: SiteModel, commentId: Long): RsComment? = safe(errorValue = null) { val client = wpApiClientProvider.getWpApiClient(site) when ( @@ -56,6 +91,71 @@ class CommentsRsDataSource @Inject constructor( } } + /** + * Fetches one page of the site's comments. Pass [firstPageParams] for the first page and the + * previous page's [RsCommentsPageResult.Success.nextPageParams] for subsequent pages. + */ + suspend fun fetchCommentsPage(site: SiteModel, params: CommentListParams): RsCommentsPageResult = + safe(errorValue = RsCommentsPageResult.Error(null)) { + val client = wpApiClientProvider.getWpApiClient(site) + when (val result = client.request { it.comments().listWithViewContext(params) }) { + is WpRequestResult.Success -> RsCommentsPageResult.Success( + comments = result.response.data.map { it.toRsCommentListItem() }, + nextPageParams = result.response.nextPageParams, + total = result.response.headerMap.wpTotal()?.toInt() + ) + is WpRequestResult.WpError -> RsCommentsPageResult.Error(result.errorMessage) + else -> RsCommentsPageResult.Error(null) + } + } + + fun firstPageParams(status: RsCommentStatus?, search: String? = null): CommentListParams = + CommentListParams( + perPage = COMMENTS_PAGE_SIZE, + search = search, + status = status, + orderby = WpApiParamCommentsOrderBy.DATE_GMT, + order = WpApiParamOrder.DESC + ) + + /** + * Fetches post titles for the given [postIds] in a single sparse-field network call, returning + * a map of post id to rendered title. Ids already cached skip the network round-trip. + * (List rows show "author commented on {post}", but the comment payload only has the post id.) + */ + suspend fun fetchPostTitles(site: SiteModel, postIds: List): Map { + val result = mutableMapOf() + val uncached = mutableListOf() + for (id in postIds.distinct()) { + val cached = postTitleCache[id] + if (cached != null) result[id] = cached else uncached.add(id) + } + if (uncached.isEmpty()) return result + + safe(errorValue = Unit) { + val client = wpApiClientProvider.getWpApiClient(site) + val response = client.request { + it.posts().filterListWithViewContext( + PostEndpointType.Posts, + PostListParams(include = uncached), + listOf(SparseAnyPostFieldWithViewContext.ID, SparseAnyPostFieldWithViewContext.TITLE) + ) + } + if (response is WpRequestResult.Success) { + for (post in response.response.data) { + val id = post.id ?: continue + val title = post.title?.rendered.orEmpty() + postTitleCache[id] = title + result[id] = title + } + } else { + val message = (response as? WpRequestResult.WpError<*>)?.errorMessage + AppLog.w(AppLog.T.COMMENTS, "fetchPostTitles failed: $message") + } + } + return result + } + suspend fun updateStatus(site: SiteModel, commentId: Long, status: CommentStatus): RsResult = write(site) { it.comments().update(commentId, CommentUpdateParams(status = status.toRsCommentStatus())) } @@ -93,8 +193,7 @@ class CommentsRsDataSource @Inject constructor( private fun CommentWithViewContext.toRsComment() = RsComment( authorName = authorName, - authorAvatarUrl = (authorAvatarUrls[UserAvatarSize.Size96] - ?: authorAvatarUrls.values.firstOrNull { !it.isNullOrEmpty() }).orEmpty(), + authorAvatarUrl = pickAvatarUrl(), // dateGmt is a UTC java.util.Date (an absolute instant), so relative-time formatting is // correct regardless of the site's timezone — unlike the offset-less local `date` field. dateGmt = dateGmt, @@ -103,8 +202,25 @@ class CommentsRsDataSource @Inject constructor( postId = post, status = status.toAppCommentStatus() ) + + companion object { + internal const val COMMENTS_PAGE_SIZE = 30u + } } +internal fun CommentWithViewContext.toRsCommentListItem() = CommentsRsDataSource.RsCommentListItem( + remoteCommentId = id, + authorName = authorName, + authorAvatarUrl = pickAvatarUrl(), + dateGmt = dateGmt, + contentHtml = content.rendered, + postId = post, + status = status.toAppCommentStatus() +) + +internal fun CommentWithViewContext.pickAvatarUrl(): String = + (authorAvatarUrls[UserAvatarSize.Size96] ?: authorAvatarUrls.values.firstOrNull { !it.isNullOrEmpty() }).orEmpty() + internal fun CommentStatus.toRsCommentStatus(): RsCommentStatus = when (this) { CommentStatus.APPROVED -> RsCommentStatus.Approved CommentStatus.UNAPPROVED -> RsCommentStatus.Hold diff --git a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/UnifiedCommentListFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/UnifiedCommentListFragment.kt index e433ac99dc3a..9893a0f21d35 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/UnifiedCommentListFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/UnifiedCommentListFragment.kt @@ -15,7 +15,6 @@ import org.wordpress.android.R import org.wordpress.android.WordPress import org.wordpress.android.databinding.UnifiedCommentListFragmentBinding import org.wordpress.android.fluxc.model.CommentStatus -import org.wordpress.android.ui.ActivityLauncher import org.wordpress.android.ui.comments.unified.CommentDetailsActivityContract.CommentDetailsActivityRequest import org.wordpress.android.ui.comments.unified.CommentDetailsActivityContract.CommentDetailsActivityResponse import org.wordpress.android.ui.comments.unified.CommentListUiModelHelper.ActionModeUiModel @@ -25,8 +24,6 @@ import org.wordpress.android.ui.comments.unified.CommentListUiModelHelper.Commen import org.wordpress.android.ui.comments.unified.CommentListUiModelHelper.ConfirmationDialogUiModel import org.wordpress.android.ui.comments.unified.CommentListUiModelHelper.ConfirmationDialogUiModel.Visible import org.wordpress.android.ui.mysite.SelectedSiteRepository -import org.wordpress.android.ui.prefs.experimentalfeatures.ExperimentalFeatures -import org.wordpress.android.ui.prefs.experimentalfeatures.ExperimentalFeatures.Feature import org.wordpress.android.ui.utils.UiHelpers import org.wordpress.android.util.NetworkUtilsWrapper import org.wordpress.android.util.SnackbarItem @@ -54,9 +51,6 @@ class UnifiedCommentListFragment : Fragment(R.layout.unified_comment_list_fragme @Inject lateinit var networkUtilsWrapper: NetworkUtilsWrapper - @Inject - lateinit var experimentalFeatures: ExperimentalFeatures - private lateinit var viewModel: UnifiedCommentListViewModel private lateinit var activityViewModel: UnifiedCommentActivityViewModel private lateinit var adapter: UnifiedCommentListAdapter @@ -166,25 +160,16 @@ class UnifiedCommentListFragment : Fragment(R.layout.unified_comment_list_fragme private fun showCommentDetails(commentId: Long, commentStatus: CommentStatus) { currentSnackbar?.dismiss() - val site = selectedSiteRepository.getSelectedSite() - // The rs detail can only authenticate WP.com-accessed or application-password sites, so - // fall back to the legacy detail elsewhere (same gating as the rs pages/posts screens). - val siteSupportsRs = site != null && (site.isUsingWpComRestApi || site.hasApplicationPassword()) - if (siteSupportsRs && experimentalFeatures.isEnabled(Feature.RS_UNIFIED_COMMENTS)) { - ActivityLauncher.viewUnifiedCommentsDetails( - context, - site, - commentId + // This legacy list is only reached when the RS_UNIFIED_COMMENTS flag is off or the site + // can't use wordpress-rs (see ActivityLauncher.viewUnifiedComments), so it always pairs + // with the legacy detail; the rs list launches the rs detail itself. + commentDetails.launch( + CommentDetailsActivityRequest( + commentId, + commentStatus, + selectedSiteRepository.getSelectedSite()!! ) - } else { - commentDetails.launch( - CommentDetailsActivityRequest( - commentId, - commentStatus, - selectedSiteRepository.getSelectedSite()!! - ) - ) - } + ) } val commentDetails = registerForActivityResult( diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt new file mode 100644 index 000000000000..f2a27e564543 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt @@ -0,0 +1,76 @@ +package org.wordpress.android.ui.commentsrs + +import android.content.Context +import android.content.Intent +import android.os.Bundle +import androidx.activity.result.contract.ActivityResultContracts +import androidx.activity.viewModels +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.lifecycleScope +import androidx.lifecycle.repeatOnLifecycle +import dagger.hilt.android.AndroidEntryPoint +import kotlinx.coroutines.launch +import org.wordpress.android.ui.comments.unified.UnifiedCommentsDetailsActivity +import org.wordpress.android.ui.compose.theme.AppThemeM3 +import org.wordpress.android.ui.main.BaseAppCompatActivity +import org.wordpress.android.ui.commentsrs.screens.CommentsRsListScreen +import org.wordpress.android.util.ToastUtils +import org.wordpress.android.util.extensions.setContent + +@AndroidEntryPoint +class CommentsRsListActivity : BaseAppCompatActivity() { + private val viewModel: CommentsRsListViewModel by viewModels() + + // The detail can moderate, reply, edit or delete, and doesn't report a result — refresh + // unconditionally on return so the list reflects whatever happened there. + private val detailLauncher = registerForActivityResult( + ActivityResultContracts.StartActivityForResult() + ) { + viewModel.refreshAllTabs() + } + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + + observeEvents() + + setContent { + val tabStates by viewModel.tabStates.collectAsState() + AppThemeM3 { + CommentsRsListScreen( + tabStates = tabStates, + snackbarMessages = viewModel.snackbarMessages, + onInitTab = viewModel::initTab, + onRefreshTab = { tab -> viewModel.refreshTab(tab, isUserRefresh = true) }, + onLoadMore = viewModel::loadMore, + onNavigateBack = { onBackPressedDispatcher.onBackPressed() }, + onCommentClick = { commentId, _ -> viewModel.onCommentClick(commentId) } + ) + } + } + } + + private fun observeEvents() { + lifecycleScope.launch { + repeatOnLifecycle(Lifecycle.State.STARTED) { + viewModel.events.collect { event -> handleEvent(event) } + } + } + } + + private fun handleEvent(event: CommentsRsListEvent) { + when (event) { + is CommentsRsListEvent.OpenCommentDetail -> detailLauncher.launch( + UnifiedCommentsDetailsActivity.createIntent(this, event.site, event.remoteCommentId) + ) + is CommentsRsListEvent.ShowToast -> ToastUtils.showToast(this, event.messageResId) + is CommentsRsListEvent.Finish -> finish() + } + } + + companion object { + fun createIntent(context: Context) = Intent(context, CommentsRsListActivity::class.java) + } +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListEvent.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListEvent.kt new file mode 100644 index 000000000000..9fa94130cac3 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListEvent.kt @@ -0,0 +1,10 @@ +package org.wordpress.android.ui.commentsrs + +import androidx.annotation.StringRes +import org.wordpress.android.fluxc.model.SiteModel + +sealed interface CommentsRsListEvent { + data class OpenCommentDetail(val site: SiteModel, val remoteCommentId: Long) : CommentsRsListEvent + data class ShowToast(@StringRes val messageResId: Int) : CommentsRsListEvent + object Finish : CommentsRsListEvent +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListTab.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListTab.kt new file mode 100644 index 000000000000..66978db5abef --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListTab.kt @@ -0,0 +1,48 @@ +package org.wordpress.android.ui.commentsrs + +import androidx.annotation.StringRes +import org.wordpress.android.R +import uniffi.wp_api.CommentStatus as RsCommentStatus + +/** + * Filter tabs for the rs comments list, matching the legacy unified list minus Unreplied + * (deferred; it needs client-side threading of ~100 comments). + * + * [queryStatus] is the `status` query param for `/wp/v2/comments`. Two tabs use + * [RsCommentStatus.Custom] because `WP_Comment_Query` only recognises the literal values + * `approve` and `all` — wordpress-rs serialises [RsCommentStatus.Approved] as `approved`, + * which WordPress core treats as an unknown status and returns nothing for (wordpress-rs + * `comments.rs` serialisation test asserts `status=approved`). `all` means approved+hold, + * matching the legacy ALL filter (APPROVED+UNAPPROVED). + */ +enum class CommentsRsListTab( + @StringRes val labelResId: Int, + @StringRes val emptyMessageResId: Int, + val queryStatus: RsCommentStatus +) { + ALL( + R.string.comment_status_all, + R.string.comments_empty_list, + RsCommentStatus.Custom("all") + ), + PENDING( + R.string.comment_status_unapproved, + R.string.comments_empty_list_filtered_pending, + RsCommentStatus.Hold + ), + APPROVED( + R.string.comment_status_approved, + R.string.comments_empty_list_filtered_approved, + RsCommentStatus.Custom("approve") + ), + SPAM( + R.string.comment_status_spam, + R.string.comments_empty_list_filtered_spam, + RsCommentStatus.Spam + ), + TRASHED( + R.string.comment_status_trash, + R.string.comments_empty_list_filtered_trashed, + RsCommentStatus.Trash + ) +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt new file mode 100644 index 000000000000..34ae3fb9bac9 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt @@ -0,0 +1,34 @@ +package org.wordpress.android.ui.commentsrs + +import org.wordpress.android.fluxc.model.CommentStatus + +data class CommentsRsSnackbarMessage( + val message: String, + val actionLabel: String? = null, + val onAction: (() -> Unit)? = null +) + +/** One comment row. */ +data class CommentRsUiModel( + val remoteCommentId: Long, + val authorName: String, + val avatarUrl: String, + val snippet: String, + val relativeDate: String, + val status: CommentStatus, + val postId: Long, + /** Resolved asynchronously in a batched request; null until then. */ + val postTitle: String? = null +) { + val isPending: Boolean get() = status == CommentStatus.UNAPPROVED +} + +data class CommentsTabUiState( + val comments: List = emptyList(), + val isLoading: Boolean = false, + val isRefreshing: Boolean = false, + val isLoadingMore: Boolean = false, + val canLoadMore: Boolean = false, + val error: String? = null, + val isAuthError: Boolean = false +) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt new file mode 100644 index 000000000000..4ccba38514c0 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt @@ -0,0 +1,229 @@ +package org.wordpress.android.ui.commentsrs + +import androidx.annotation.MainThread +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Job +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.receiveAsFlow +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import org.wordpress.android.R +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.modules.BG_THREAD +import org.wordpress.android.ui.comments.unified.CommentsRsDataSource +import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsCommentListItem +import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsCommentsPageResult +import org.wordpress.android.ui.mysite.SelectedSiteRepository +import org.wordpress.android.ui.postsrs.PostRsErrorUtils +import org.wordpress.android.util.DateTimeUtilsWrapper +import org.wordpress.android.util.HtmlUtils +import org.wordpress.android.util.NetworkUtilsWrapper +import org.wordpress.android.util.WPAvatarUtilsWrapper +import org.wordpress.android.viewmodel.ResourceProvider +import uniffi.wp_api.CommentListParams +import javax.inject.Inject +import javax.inject.Named + +/** + * ViewModel for the wordpress-rs comments list. Unlike the postsrs/pagesrs screens there is no + * rs cache support for comments yet, so each tab pages directly against `/wp/v2/comments`, + * keeping the response's `nextPageParams` as the pagination cursor. + */ +@HiltViewModel +class CommentsRsListViewModel @Inject constructor( + selectedSiteRepository: SelectedSiteRepository, + private val commentsRsDataSource: CommentsRsDataSource, + private val resourceProvider: ResourceProvider, + private val networkUtilsWrapper: NetworkUtilsWrapper, + private val dateTimeUtilsWrapper: DateTimeUtilsWrapper, + private val avatarUtilsWrapper: WPAvatarUtilsWrapper, + @Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher +) : ViewModel() { + private val _tabStates = MutableStateFlow>(emptyMap()) + val tabStates: StateFlow> = _tabStates.asStateFlow() + + private val _events = Channel(Channel.BUFFERED) + val events = _events.receiveAsFlow() + + private val _snackbarMessages = Channel(Channel.BUFFERED) + val snackbarMessages = _snackbarMessages.receiveAsFlow() + + // Pagination cursors: the next-page params returned with each fetched page, per tab. + private val nextPageParams = mutableMapOf() + private val initializingTabs = mutableSetOf() + private val resolveTitleJobs = mutableMapOf() + + private val _site: SiteModel? = selectedSiteRepository.getSelectedSite() + private val site: SiteModel + get() = requireNotNull(_site) { "No selected site — Activity should have finished" } + + init { + if (_site == null) { + _events.trySend(CommentsRsListEvent.ShowToast(R.string.blog_not_found)) + _events.trySend(CommentsRsListEvent.Finish) + } + } + + /** Loads the first page for [tab] unless it's already initialized or initializing. */ + @MainThread + fun initTab(tab: CommentsRsListTab) { + if (_tabStates.value.containsKey(tab) || initializingTabs.contains(tab)) return + initializingTabs.add(tab) + updateTabUiState(tab) { copy(isLoading = true) } + fetchFirstPage(tab) { initializingTabs.remove(tab) } + } + + /** + * Re-fetches the first page for [tab]. The pull-to-refresh indicator is only shown for a + * user-initiated refresh; silent refreshes (returning from the detail) keep the current + * list on screen until the new page arrives. + */ + @MainThread + fun refreshTab(tab: CommentsRsListTab, isUserRefresh: Boolean = false) { + if (!_tabStates.value.containsKey(tab)) return + updateTabUiState(tab) { copy(isRefreshing = isUserRefresh, error = null) } + fetchFirstPage(tab) + } + + /** Refreshes every initialized tab; comments move between tabs when moderated. */ + @MainThread + fun refreshAllTabs() { + _tabStates.value.keys.forEach { refreshTab(it) } + } + + /** Loads the next page for [tab] if there is one and nothing else is in flight. */ + @MainThread + fun loadMore(tab: CommentsRsListTab) { + val current = getTabUiState(tab) + val params = nextPageParams[tab] + val isBusy = current.isLoading || current.isRefreshing || current.isLoadingMore + if (params == null || isBusy) return + + updateTabUiState(tab) { copy(isLoadingMore = true) } + viewModelScope.launch { + val result = withContext(bgDispatcher) { commentsRsDataSource.fetchCommentsPage(site, params) } + when (result) { + is RsCommentsPageResult.Success -> { + nextPageParams[tab] = result.nextPageParams + updateTabUiState(tab) { + copy( + // A comment can shift pages if the list changed server-side between + // requests, so dedupe on append. + comments = (comments + result.comments.map { it.toUiModel() }) + .distinctBy { it.remoteCommentId }, + isLoadingMore = false, + canLoadMore = result.nextPageParams != null + ) + } + resolvePostTitles(tab) + } + is RsCommentsPageResult.Error -> { + updateTabUiState(tab) { copy(isLoadingMore = false) } + _snackbarMessages.trySend(CommentsRsSnackbarMessage(errorMessage(result.message))) + } + } + } + } + + /** Opens the rs comment detail for the tapped row. */ + @MainThread + fun onCommentClick(remoteCommentId: Long) { + _events.trySend(CommentsRsListEvent.OpenCommentDetail(site, remoteCommentId)) + } + + private fun fetchFirstPage(tab: CommentsRsListTab, onComplete: () -> Unit = {}) { + viewModelScope.launch { + val params = commentsRsDataSource.firstPageParams(status = tab.queryStatus) + val result = withContext(bgDispatcher) { commentsRsDataSource.fetchCommentsPage(site, params) } + when (result) { + is RsCommentsPageResult.Success -> { + nextPageParams[tab] = result.nextPageParams + updateTabUiState(tab) { + copy( + comments = result.comments.map { it.toUiModel() }, + isLoading = false, + isRefreshing = false, + canLoadMore = result.nextPageParams != null, + error = null, + isAuthError = false + ) + } + resolvePostTitles(tab) + } + is RsCommentsPageResult.Error -> onFirstPageError(tab, result.message) + } + onComplete() + } + } + + /** + * First-page failure: when the tab already shows comments keep them and offer a retry + * snackbar; when it's empty, show the full-screen error state. + */ + private fun onFirstPageError(tab: CommentsRsListTab, message: String?) { + val friendly = errorMessage(message) + if (getTabUiState(tab).comments.isNotEmpty()) { + updateTabUiState(tab) { copy(isLoading = false, isRefreshing = false, error = null) } + _snackbarMessages.trySend( + CommentsRsSnackbarMessage( + message = friendly, + actionLabel = resourceProvider.getString(R.string.retry), + onAction = { refreshTab(tab) } + ) + ) + } else { + updateTabUiState(tab) { + copy(comments = emptyList(), isLoading = false, isRefreshing = false, error = friendly) + } + } + } + + /** + * Resolves the "commented on {post}" titles for rows that don't have one yet, in a single + * batched request per tab. Rows keep an author-only title until (or if ever) resolved. + */ + private fun resolvePostTitles(tab: CommentsRsListTab) { + val unresolvedIds = getTabUiState(tab).comments + .filter { it.postTitle == null && it.postId > 0 } + .map { it.postId } + .distinct() + if (unresolvedIds.isEmpty()) return + + resolveTitleJobs[tab]?.cancel() + resolveTitleJobs[tab] = viewModelScope.launch { + val titles = withContext(bgDispatcher) { commentsRsDataSource.fetchPostTitles(site, unresolvedIds) } + if (titles.isEmpty()) return@launch + updateTabUiState(tab) { + copy(comments = comments.map { it.copy(postTitle = titles[it.postId] ?: it.postTitle) }) + } + } + } + + /** The server message when it sent one, otherwise a friendly offline/generic message. */ + private fun errorMessage(serverMessage: String?): String = + serverMessage?.takeIf { it.isNotBlank() } + ?: PostRsErrorUtils.friendlyErrorMessage(null, null, resourceProvider, networkUtilsWrapper) + + private fun RsCommentListItem.toUiModel() = CommentRsUiModel( + remoteCommentId = remoteCommentId, + authorName = authorName.ifBlank { resourceProvider.getString(R.string.anonymous) }, + avatarUrl = avatarUtilsWrapper.rewriteAvatarUrlWithResource(authorAvatarUrl, R.dimen.avatar_sz_medium), + snippet = HtmlUtils.fastStripHtml(contentHtml).trim(), + relativeDate = dateTimeUtilsWrapper.javaDateToTimeSpan(dateGmt), + status = status, + postId = postId + ) + + private fun getTabUiState(tab: CommentsRsListTab): CommentsTabUiState = + _tabStates.value[tab] ?: CommentsTabUiState(isLoading = true) + + private fun updateTabUiState(tab: CommentsRsListTab, update: CommentsTabUiState.() -> CommentsTabUiState) { + _tabStates.value += (tab to getTabUiState(tab).update()) + } +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt new file mode 100644 index 000000000000..068421b3d396 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt @@ -0,0 +1,143 @@ +package org.wordpress.android.ui.commentsrs.screens + +import androidx.compose.foundation.background +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.IntrinsicSize +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxHeight +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.width +import androidx.compose.foundation.shape.CircleShape +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.Person +import androidx.compose.material3.HorizontalDivider +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.graphics.vector.rememberVectorPainter +import androidx.compose.ui.layout.ContentScale +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.AnnotatedString +import androidx.compose.ui.text.SpanStyle +import androidx.compose.ui.text.buildAnnotatedString +import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import coil.compose.AsyncImage +import org.wordpress.android.R +import org.wordpress.android.ui.commentsrs.CommentRsUiModel + +private val AVATAR_SIZE = 40.dp +private val PENDING_INDICATOR_WIDTH = 4.dp + +@Composable +fun CommentsRsListItem( + comment: CommentRsUiModel, + onClick: () -> Unit, + modifier: Modifier = Modifier +) { + Column( + modifier = modifier + .fillMaxWidth() + .background(MaterialTheme.colorScheme.surface) + .clickable(onClick = onClick) + ) { + Row(modifier = Modifier.height(IntrinsicSize.Min)) { + // The pending indicator: a colored bar on the row's leading edge, like the legacy + // list's status indicator. + Box( + modifier = Modifier + .width(PENDING_INDICATOR_WIDTH) + .fillMaxHeight() + .background( + if (comment.isPending) { + MaterialTheme.colorScheme.tertiary + } else { + MaterialTheme.colorScheme.surface.copy(alpha = 0f) + } + ) + ) + Row(modifier = Modifier.padding(start = 12.dp, top = 16.dp, end = 16.dp, bottom = 16.dp)) { + CommentAvatar(comment = comment) + Column( + modifier = Modifier + .padding(start = 16.dp) + .weight(1f) + ) { + Text( + text = commentTitle(comment), + style = MaterialTheme.typography.bodyLarge, + maxLines = 1, + overflow = TextOverflow.Ellipsis + ) + if (comment.snippet.isNotBlank()) { + Text( + text = comment.snippet, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 2, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.padding(top = 2.dp) + ) + } + Text( + text = comment.relativeDate, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(top = 4.dp) + ) + } + } + } + HorizontalDivider(modifier = Modifier.padding(start = 72.dp)) + } +} + +@Composable +private fun CommentAvatar(comment: CommentRsUiModel) { + val fallback = rememberVectorPainter(Icons.Filled.Person) + AsyncImage( + model = comment.avatarUrl.ifBlank { null }, + contentDescription = null, + contentScale = ContentScale.Crop, + fallback = fallback, + error = fallback, + modifier = Modifier + .size(AVATAR_SIZE) + .clip(CircleShape) + ) +} + +/** + * The row title: "{author} on {post title}" via the shared [R.string.comment_title] template with + * both parts bold (like legacy `CommentListUiUtils.formatCommentTitle`), or just the bold author + * name while the post title is unresolved. + */ +@Composable +private fun commentTitle(comment: CommentRsUiModel): AnnotatedString { + val postTitle = comment.postTitle?.trim().orEmpty() + val formatted = if (postTitle.isEmpty()) { + comment.authorName + } else { + stringResource(R.string.comment_title, comment.authorName, postTitle) + } + return buildAnnotatedString { + append(formatted) + boldRange(formatted, comment.authorName) + if (postTitle.isNotEmpty()) boldRange(formatted, postTitle) + } +} + +private fun AnnotatedString.Builder.boldRange(formatted: String, part: String) { + val start = formatted.indexOf(part) + if (start >= 0) { + addStyle(SpanStyle(fontWeight = FontWeight.Bold), start, start + part.length) + } +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt new file mode 100644 index 000000000000..aeb7fe31e3f8 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -0,0 +1,122 @@ +package org.wordpress.android.ui.commentsrs.screens + +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.pager.HorizontalPager +import androidx.compose.foundation.pager.rememberPagerState +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.automirrored.filled.ArrowBack +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.PrimaryScrollableTabRow +import androidx.compose.material3.Scaffold +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState +import androidx.compose.material3.SnackbarResult +import androidx.compose.material3.Tab +import androidx.compose.material3.Text +import androidx.compose.material3.TopAppBar +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.snapshotFlow +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.unit.dp +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.launch +import org.wordpress.android.R +import org.wordpress.android.ui.commentsrs.CommentsRsListTab +import org.wordpress.android.ui.commentsrs.CommentsRsSnackbarMessage +import org.wordpress.android.ui.commentsrs.CommentsTabUiState + +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun CommentsRsListScreen( + tabStates: Map, + snackbarMessages: Flow = emptyFlow(), + onInitTab: (CommentsRsListTab) -> Unit, + onRefreshTab: (CommentsRsListTab) -> Unit, + onLoadMore: (CommentsRsListTab) -> Unit, + onNavigateBack: () -> Unit, + onCommentClick: (Long, CommentsRsListTab) -> Unit +) { + val tabs = CommentsRsListTab.entries + val pagerState = rememberPagerState(pageCount = { tabs.size }) + val coroutineScope = rememberCoroutineScope() + val snackbarHostState = remember { SnackbarHostState() } + + LaunchedEffect(snackbarMessages) { + snackbarMessages.collect { msg -> + val result = snackbarHostState.showSnackbar( + message = msg.message, + actionLabel = msg.actionLabel + ) + if (result == SnackbarResult.ActionPerformed) { + msg.onAction?.invoke() + } + } + } + + Scaffold( + snackbarHost = { SnackbarHost(snackbarHostState) }, + topBar = { + TopAppBar( + title = { Text(text = stringResource(R.string.comments)) }, + navigationIcon = { + IconButton(onClick = onNavigateBack) { + Icon( + Icons.AutoMirrored.Filled.ArrowBack, + contentDescription = stringResource(R.string.back) + ) + } + } + ) + } + ) { contentPadding -> + Column(modifier = Modifier.fillMaxSize().padding(contentPadding)) { + PrimaryScrollableTabRow( + selectedTabIndex = pagerState.settledPage, + edgePadding = 0.dp + ) { + tabs.forEachIndexed { index, tab -> + Tab( + selected = pagerState.settledPage == index, + onClick = { + coroutineScope.launch { pagerState.animateScrollToPage(index) } + }, + unselectedContentColor = MaterialTheme.colorScheme.onSurfaceVariant, + text = { Text(text = stringResource(tab.labelResId)) } + ) + } + } + + LaunchedEffect(pagerState) { + snapshotFlow { pagerState.settledPage }.collect { page -> + onInitTab(tabs[page]) + } + } + + HorizontalPager( + state = pagerState, + modifier = Modifier.weight(1f) + ) { page -> + val tab = tabs[page] + val tabState = tabStates[tab] ?: CommentsTabUiState(isLoading = true) + + CommentsRsTabListScreen( + state = tabState, + emptyMessageResId = tab.emptyMessageResId, + onRefresh = { onRefreshTab(tab) }, + onLoadMore = { onLoadMore(tab) }, + onCommentClick = { commentId -> onCommentClick(commentId, tab) } + ) + } + } + } +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt new file mode 100644 index 000000000000..66d8bdf2ab7b --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt @@ -0,0 +1,224 @@ +package org.wordpress.android.ui.commentsrs.screens + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.lazy.rememberLazyListState +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.shape.CircleShape +import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.foundation.verticalScroll +import androidx.compose.material3.Button +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.material3.pulltorefresh.PullToRefreshBox +import androidx.compose.material3.pulltorefresh.PullToRefreshDefaults +import androidx.compose.material3.pulltorefresh.rememberPullToRefreshState +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.snapshotFlow +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.style.TextAlign +import androidx.compose.ui.unit.dp +import kotlinx.coroutines.flow.distinctUntilChanged +import org.wordpress.android.R +import org.wordpress.android.ui.commentsrs.CommentRsUiModel +import org.wordpress.android.ui.commentsrs.CommentsTabUiState +import org.wordpress.android.ui.compose.components.ShimmerBox + +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun CommentsRsTabListScreen( + state: CommentsTabUiState, + emptyMessageResId: Int, + onRefresh: () -> Unit, + onLoadMore: () -> Unit, + onCommentClick: (Long) -> Unit, + modifier: Modifier = Modifier +) { + val pullToRefreshState = rememberPullToRefreshState() + + PullToRefreshBox( + modifier = modifier.fillMaxSize(), + isRefreshing = state.isRefreshing, + state = pullToRefreshState, + onRefresh = onRefresh, + indicator = { + PullToRefreshDefaults.Indicator( + state = pullToRefreshState, + isRefreshing = state.isRefreshing, + color = MaterialTheme.colorScheme.primary, + modifier = Modifier.align(Alignment.TopCenter) + ) + } + ) { + when { + state.isLoading -> ShimmerList() + state.error != null && state.comments.isEmpty() -> ErrorContent( + error = state.error, + onRetry = if (state.isAuthError) null else onRefresh + ) + state.comments.isEmpty() && !state.isRefreshing -> EmptyContent( + emptyMessageResId = emptyMessageResId + ) + else -> CommentListContent( + comments = state.comments, + isLoadingMore = state.isLoadingMore, + canLoadMore = state.canLoadMore, + onLoadMore = onLoadMore, + onCommentClick = onCommentClick + ) + } + } +} + +@Composable +private fun CommentListContent( + comments: List, + isLoadingMore: Boolean, + canLoadMore: Boolean, + onLoadMore: () -> Unit, + onCommentClick: (Long) -> Unit +) { + val listState = rememberLazyListState() + + LaunchedEffect(canLoadMore) { + if (!canLoadMore) return@LaunchedEffect + snapshotFlow { + val lastVisible = listState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 + val total = listState.layoutInfo.totalItemsCount + lastVisible >= total - LOAD_MORE_THRESHOLD + }.distinctUntilChanged().collect { shouldLoad -> + if (shouldLoad) onLoadMore() + } + } + + LazyColumn( + state = listState, + modifier = Modifier.fillMaxSize() + ) { + items( + items = comments, + key = { it.remoteCommentId } + ) { comment -> + CommentsRsListItem( + comment = comment, + onClick = { onCommentClick(comment.remoteCommentId) }, + modifier = Modifier.animateItem() + ) + } + + if (isLoadingMore) { + item(key = "loading_more") { + Box( + modifier = Modifier + .fillParentMaxWidth() + .padding(16.dp), + contentAlignment = Alignment.Center + ) { + CircularProgressIndicator( + modifier = Modifier.size(24.dp), + strokeWidth = 2.dp + ) + } + } + } + } +} + +@Composable +private fun ShimmerList() { + LazyColumn(modifier = Modifier.fillMaxSize()) { + items(SHIMMER_ITEM_COUNT) { + PlaceholderItem() + } + } +} + +@Composable +private fun PlaceholderItem() { + Row(modifier = Modifier.padding(16.dp)) { + ShimmerBox( + modifier = Modifier + .size(40.dp) + .clip(CircleShape) + ) + Column(modifier = Modifier.padding(start = 16.dp)) { + ShimmerBox( + modifier = Modifier + .size(width = 180.dp, height = 16.dp) + .clip(RoundedCornerShape(4.dp)) + ) + Spacer(modifier = Modifier.height(8.dp)) + ShimmerBox( + modifier = Modifier + .size(width = 260.dp, height = 14.dp) + .clip(RoundedCornerShape(4.dp)) + ) + } + } +} + +@Composable +private fun ErrorContent(error: String, onRetry: (() -> Unit)?) { + Column( + modifier = Modifier + .fillMaxSize() + .verticalScroll(rememberScrollState()) + .padding(32.dp), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center + ) { + Text( + text = stringResource(R.string.error_generic), + style = MaterialTheme.typography.titleMedium + ) + Spacer(modifier = Modifier.height(8.dp)) + Text( + text = error, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + textAlign = TextAlign.Center + ) + if (onRetry != null) { + Spacer(modifier = Modifier.height(16.dp)) + Button(onClick = onRetry) { + Text(text = stringResource(R.string.retry)) + } + } + } +} + +@Composable +private fun EmptyContent(emptyMessageResId: Int) { + Column( + modifier = Modifier + .fillMaxSize() + .verticalScroll(rememberScrollState()) + .padding(32.dp), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center + ) { + Text( + text = stringResource(emptyMessageResId), + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + } +} + +private const val LOAD_MORE_THRESHOLD = 5 +private const val SHIMMER_ITEM_COUNT = 8 diff --git a/WordPress/src/main/java/org/wordpress/android/ui/main/BaseAppCompatActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/main/BaseAppCompatActivity.kt index 3b2bd1551ce3..6225fc81a75d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/main/BaseAppCompatActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/main/BaseAppCompatActivity.kt @@ -17,6 +17,7 @@ import org.wordpress.android.ui.jetpackoverlay.JetpackStaticPosterActivity import org.wordpress.android.ui.main.feedbackform.FeedbackFormActivity import org.wordpress.android.ui.media.MediaPreviewActivity import org.wordpress.android.ui.media.MediaSettingsActivity +import org.wordpress.android.ui.commentsrs.CommentsRsListActivity import org.wordpress.android.ui.mysite.menu.MenuActivity import org.wordpress.android.ui.mysite.personalization.PersonalizationActivity import org.wordpress.android.ui.navmenus.NavMenusActivity @@ -83,6 +84,7 @@ private fun isExcludedActivity(activity: BaseAppCompatActivity) = private val excludedActivities = listOf( BlazeCampaignParentActivity::class.java.name, BloggingPromptsListActivity::class.java.name, + CommentsRsListActivity::class.java.name, DebugSharedPreferenceFlagsActivity::class.java.name, DomainManagementActivity::class.java.name, EditJetpackSocialShareMessageActivity::class.java.name, diff --git a/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsListMappingTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsListMappingTest.kt new file mode 100644 index 000000000000..a3ff1a094ffc --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsListMappingTest.kt @@ -0,0 +1,110 @@ +package org.wordpress.android.ui.comments.unified + +import org.assertj.core.api.Assertions.assertThat +import org.junit.Test +import org.mockito.kotlin.mock +import org.wordpress.android.fluxc.model.CommentStatus.APPROVED +import org.wordpress.android.fluxc.model.CommentStatus.UNAPPROVED +import uniffi.wp_api.CommentContentWithViewContext +import uniffi.wp_api.CommentType +import uniffi.wp_api.CommentWithViewContext +import uniffi.wp_api.UserAvatarSize +import uniffi.wp_api.WpAdditionalFields +import uniffi.wp_api.WpApiParamCommentsOrderBy +import uniffi.wp_api.WpApiParamOrder +import java.util.Date +import uniffi.wp_api.CommentStatus as RsCommentStatus + +class CommentsRsListMappingTest { + @Test + fun `toRsCommentListItem maps the fields the list rows need`() { + val item = rsComment().toRsCommentListItem() + + assertThat(item.remoteCommentId).isEqualTo(COMMENT_ID) + assertThat(item.authorName).isEqualTo("Jane") + assertThat(item.authorAvatarUrl).isEqualTo("https://example.com/avatar96.png") + assertThat(item.dateGmt).isEqualTo(DATE_GMT) + assertThat(item.contentHtml).isEqualTo("

hello

") + assertThat(item.postId).isEqualTo(POST_ID) + assertThat(item.status).isEqualTo(APPROVED) + } + + @Test + fun `toRsCommentListItem maps hold status to unapproved`() { + val item = rsComment(status = RsCommentStatus.Hold).toRsCommentListItem() + + assertThat(item.status).isEqualTo(UNAPPROVED) + } + + @Test + fun `pickAvatarUrl prefers size 96`() { + val comment = rsComment( + avatarUrls = mapOf( + UserAvatarSize.Size24 to "https://example.com/avatar24.png", + UserAvatarSize.Size96 to "https://example.com/avatar96.png" + ) + ) + + assertThat(comment.pickAvatarUrl()).isEqualTo("https://example.com/avatar96.png") + } + + @Test + fun `pickAvatarUrl falls back to the first non-empty url`() { + val comment = rsComment( + avatarUrls = mapOf( + UserAvatarSize.Size24 to "", + UserAvatarSize.Size48 to "https://example.com/avatar48.png" + ) + ) + + assertThat(comment.pickAvatarUrl()).isEqualTo("https://example.com/avatar48.png") + } + + @Test + fun `pickAvatarUrl returns empty when there are no avatar urls`() { + assertThat(rsComment(avatarUrls = emptyMap()).pickAvatarUrl()).isEmpty() + } + + @Test + fun `firstPageParams requests newest comments with the given status and search`() { + val dataSource = CommentsRsDataSource(mock()) + + val params = dataSource.firstPageParams(RsCommentStatus.Spam, search = "query") + + assertThat(params.perPage).isEqualTo(CommentsRsDataSource.COMMENTS_PAGE_SIZE) + assertThat(params.status).isEqualTo(RsCommentStatus.Spam) + assertThat(params.search).isEqualTo("query") + assertThat(params.orderby).isEqualTo(WpApiParamCommentsOrderBy.DATE_GMT) + assertThat(params.order).isEqualTo(WpApiParamOrder.DESC) + } + + private fun rsComment( + status: RsCommentStatus = RsCommentStatus.Approved, + avatarUrls: Map = mapOf( + UserAvatarSize.Size96 to "https://example.com/avatar96.png" + ) + ) = CommentWithViewContext( + id = COMMENT_ID, + author = 7L, + authorName = "Jane", + authorUrl = "https://example.com", + content = CommentContentWithViewContext(rendered = "

hello

"), + date = "2026-07-01T12:00:00", + dateGmt = DATE_GMT, + link = "https://example.com/post/#comment-42", + parent = 0L, + post = POST_ID, + status = status, + commentType = CommentType.Comment, + authorAvatarUrls = avatarUrls, + // Mocked: the real WpAdditionalFields constructor loads the uniffi native library, + // which isn't available in local unit tests. + additionalFields = mock() + ) + + companion object { + private const val COMMENT_ID = 42L + private const val POST_ID = 99L + private val DATE_GMT = Date(1_700_000_000_000) + } +} diff --git a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListTabTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListTabTest.kt new file mode 100644 index 000000000000..023bff617408 --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListTabTest.kt @@ -0,0 +1,26 @@ +package org.wordpress.android.ui.commentsrs + +import org.assertj.core.api.Assertions.assertThat +import org.junit.Test +import uniffi.wp_api.CommentStatus as RsCommentStatus + +class CommentsRsListTabTest { + @Test + fun `all tab queries status all which the server treats as approved plus hold`() { + assertThat(CommentsRsListTab.ALL.queryStatus).isEqualTo(RsCommentStatus.Custom("all")) + } + + @Test + fun `approved tab queries the literal approve status`() { + // WP_Comment_Query only recognises "approve"; the RsCommentStatus.Approved enum + // serialises to "approved", which the server treats as unknown and returns nothing for. + assertThat(CommentsRsListTab.APPROVED.queryStatus).isEqualTo(RsCommentStatus.Custom("approve")) + } + + @Test + fun `remaining tabs use the built-in statuses`() { + assertThat(CommentsRsListTab.PENDING.queryStatus).isEqualTo(RsCommentStatus.Hold) + assertThat(CommentsRsListTab.SPAM.queryStatus).isEqualTo(RsCommentStatus.Spam) + assertThat(CommentsRsListTab.TRASHED.queryStatus).isEqualTo(RsCommentStatus.Trash) + } +} diff --git a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt new file mode 100644 index 000000000000..48b627613ca0 --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt @@ -0,0 +1,266 @@ +package org.wordpress.android.ui.commentsrs + +import androidx.lifecycle.viewModelScope +import app.cash.turbine.test +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.cancel +import kotlinx.coroutines.test.StandardTestDispatcher +import org.assertj.core.api.Assertions.assertThat +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.mockito.Mock +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.eq +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.wordpress.android.BaseUnitTest +import org.wordpress.android.R +import org.wordpress.android.fluxc.model.CommentStatus.APPROVED +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.ui.comments.unified.CommentsRsDataSource +import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsCommentListItem +import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsCommentsPageResult +import org.wordpress.android.ui.mysite.SelectedSiteRepository +import org.wordpress.android.util.DateTimeUtilsWrapper +import org.wordpress.android.util.NetworkUtilsWrapper +import org.wordpress.android.util.WPAvatarUtilsWrapper +import org.wordpress.android.viewmodel.ResourceProvider +import uniffi.wp_api.CommentListParams +import java.util.Date + +@ExperimentalCoroutinesApi +class CommentsRsListViewModelTest : BaseUnitTest(StandardTestDispatcher()) { + @Mock lateinit var selectedSiteRepository: SelectedSiteRepository + @Mock lateinit var commentsRsDataSource: CommentsRsDataSource + @Mock lateinit var resourceProvider: ResourceProvider + @Mock lateinit var networkUtilsWrapper: NetworkUtilsWrapper + @Mock lateinit var dateTimeUtilsWrapper: DateTimeUtilsWrapper + @Mock lateinit var avatarUtilsWrapper: WPAvatarUtilsWrapper + + private lateinit var site: SiteModel + private var activeViewModel: CommentsRsListViewModel? = null + + @Before + fun setUp() = test { + site = SiteModel().apply { + id = 1 + siteId = 123L + } + whenever(selectedSiteRepository.getSelectedSite()).thenReturn(site) + whenever(resourceProvider.getString(any())).thenReturn("string") + whenever(dateTimeUtilsWrapper.javaDateToTimeSpan(any())).thenReturn("2 hours ago") + whenever(avatarUtilsWrapper.rewriteAvatarUrlWithResource(any(), any())).thenAnswer { it.arguments[0] } + whenever(commentsRsDataSource.firstPageParams(any(), anyOrNull())).thenReturn(FIRST_PAGE) + whenever(commentsRsDataSource.fetchPostTitles(any(), any())).thenReturn(emptyMap()) + } + + @After + fun tearDown() { + activeViewModel?.viewModelScope?.cancel() + activeViewModel = null + } + + private fun createViewModel() = CommentsRsListViewModel( + selectedSiteRepository = selectedSiteRepository, + commentsRsDataSource = commentsRsDataSource, + resourceProvider = resourceProvider, + networkUtilsWrapper = networkUtilsWrapper, + dateTimeUtilsWrapper = dateTimeUtilsWrapper, + avatarUtilsWrapper = avatarUtilsWrapper, + bgDispatcher = testDispatcher() + ).also { activeViewModel = it } + + private suspend fun givenPage( + comments: List, + nextPageParams: CommentListParams? = null + ) { + whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())) + .thenReturn(RsCommentsPageResult.Success(comments, nextPageParams, comments.size)) + } + + @Test + fun `when no site selected, emits ShowToast and Finish`() = test { + whenever(selectedSiteRepository.getSelectedSite()).thenReturn(null) + + val viewModel = createViewModel() + + viewModel.events.test { + val first = awaitItem() + assertThat(first).isInstanceOf(CommentsRsListEvent.ShowToast::class.java) + assertThat((first as CommentsRsListEvent.ShowToast).messageResId).isEqualTo(R.string.blog_not_found) + assertThat(awaitItem()).isEqualTo(CommentsRsListEvent.Finish) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `initTab loads the first page and maps rows`() = test { + givenPage(listOf(rsItem(id = 1), rsItem(id = 2)), nextPageParams = NEXT_PAGE) + val viewModel = createViewModel() + + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + + val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) + assertThat(state.isLoading).isFalse() + assertThat(state.comments).hasSize(2) + assertThat(state.comments.first().remoteCommentId).isEqualTo(1) + assertThat(state.comments.first().authorName).isEqualTo("Jane") + assertThat(state.comments.first().snippet).isEqualTo("hello") + assertThat(state.canLoadMore).isTrue() + } + + @Test + fun `initTab passes the tab's query status to the data source`() = test { + givenPage(emptyList()) + val viewModel = createViewModel() + + viewModel.initTab(CommentsRsListTab.APPROVED) + advanceUntilIdle() + + verify(commentsRsDataSource).firstPageParams(eq(CommentsRsListTab.APPROVED.queryStatus), anyOrNull()) + } + + @Test + fun `initTab is a no-op when the tab is already initialized`() = test { + givenPage(emptyList()) + val viewModel = createViewModel() + + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + + verify(commentsRsDataSource, times(1)).fetchCommentsPage(eq(site), any()) + } + + @Test + fun `initTab failure with no content shows the error state`() = test { + whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())) + .thenReturn(RsCommentsPageResult.Error("server said no")) + val viewModel = createViewModel() + + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + + val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) + assertThat(state.error).isEqualTo("server said no") + assertThat(state.isLoading).isFalse() + } + + @Test + fun `loadMore appends the next page and dedupes by comment id`() = test { + givenPage(listOf(rsItem(id = 1), rsItem(id = 2)), nextPageParams = NEXT_PAGE) + val viewModel = createViewModel() + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + givenPage(listOf(rsItem(id = 2), rsItem(id = 3)), nextPageParams = null) + + viewModel.loadMore(CommentsRsListTab.ALL) + advanceUntilIdle() + + val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) + assertThat(state.comments.map { it.remoteCommentId }).containsExactly(1L, 2L, 3L) + assertThat(state.canLoadMore).isFalse() + } + + @Test + fun `loadMore is a no-op when there is no next page`() = test { + givenPage(listOf(rsItem(id = 1)), nextPageParams = null) + val viewModel = createViewModel() + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + + viewModel.loadMore(CommentsRsListTab.ALL) + advanceUntilIdle() + + verify(commentsRsDataSource, times(1)).fetchCommentsPage(eq(site), any()) + } + + @Test + fun `refresh failure keeps the current comments and offers retry`() = test { + givenPage(listOf(rsItem(id = 1))) + val viewModel = createViewModel() + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())) + .thenReturn(RsCommentsPageResult.Error("boom")) + + viewModel.snackbarMessages.test { + viewModel.refreshTab(CommentsRsListTab.ALL, isUserRefresh = true) + advanceUntilIdle() + + val snackbar = awaitItem() + assertThat(snackbar.message).isEqualTo("boom") + assertThat(snackbar.onAction != null).isTrue() + cancelAndIgnoreRemainingEvents() + } + val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) + assertThat(state.comments).hasSize(1) + assertThat(state.error).isNull() + } + + @Test + fun `refreshAllTabs refreshes only initialized tabs`() = test { + givenPage(listOf(rsItem(id = 1))) + val viewModel = createViewModel() + viewModel.initTab(CommentsRsListTab.ALL) + viewModel.initTab(CommentsRsListTab.SPAM) + advanceUntilIdle() + + viewModel.refreshAllTabs() + advanceUntilIdle() + + // 2 init fetches + 2 refresh fetches, nothing for the 3 uninitialized tabs + verify(commentsRsDataSource, times(4)).fetchCommentsPage(eq(site), any()) + } + + @Test + fun `onCommentClick emits OpenCommentDetail`() = test { + givenPage(listOf(rsItem(id = 42))) + val viewModel = createViewModel() + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + + viewModel.events.test { + viewModel.onCommentClick(42L) + val event = awaitItem() + assertThat(event).isInstanceOf(CommentsRsListEvent.OpenCommentDetail::class.java) + assertThat((event as CommentsRsListEvent.OpenCommentDetail).remoteCommentId).isEqualTo(42L) + assertThat(event.site).isEqualTo(site) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `post titles are resolved in a batch and applied to rows`() = test { + givenPage(listOf(rsItem(id = 1, postId = 10), rsItem(id = 2, postId = 20))) + whenever(commentsRsDataSource.fetchPostTitles(site, listOf(10L, 20L))) + .thenReturn(mapOf(10L to "First post", 20L to "Second post")) + val viewModel = createViewModel() + + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + + val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) + assertThat(state.comments.map { it.postTitle }).containsExactly("First post", "Second post") + } + + private fun rsItem(id: Long, postId: Long = 99L) = RsCommentListItem( + remoteCommentId = id, + authorName = "Jane", + authorAvatarUrl = "https://example.com/avatar.png", + dateGmt = Date(0), + contentHtml = "

hello

", + postId = postId, + status = APPROVED + ) + + companion object { + private val FIRST_PAGE = CommentListParams() + private val NEXT_PAGE = CommentListParams(page = 2u) + } +} From aad42b4227e9047840d617aa7e4c171e627e9e5c Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 2 Jul 2026 13:27:42 -0400 Subject: [PATCH 02/21] Rename RS comments experimental flag to cover the whole comments experience Co-Authored-By: Claude Opus 4.8 (1M context) --- WordPress/src/main/res/values/strings.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index 2cb702d1ac13..1778bc344c8d 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -1026,8 +1026,8 @@ You can turn off New Stats from the overflow menu in the Stats screen. New Pages List Show pages list in a more modern UI for WordPress.com and self-hosted sites with application passwords - RS Unified Comment Detail - Show the comment detail in a more modern UI for WordPress.com and self-hosted sites with application passwords + RS Unified Comments + Show comments in a more modern UI for WordPress.com and self-hosted sites with application passwords Failed to load post Failed to load page Copy URL From a2cc6b342ba9f9f5709d4e284b218f9400fe6ce1 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 2 Jul 2026 13:54:58 -0400 Subject: [PATCH 03/21] Simplify rs comments list: drop dead/unused state, share page handling and SnackbarMessage Co-Authored-By: Claude Opus 4.8 (1M context) --- .../comments/unified/CommentsRsDataSource.kt | 7 +- .../ui/commentsrs/CommentsRsListUiState.kt | 9 +-- .../ui/commentsrs/CommentsRsListViewModel.kt | 67 ++++++++++--------- .../commentsrs/screens/CommentsRsListItem.kt | 3 +- .../screens/CommentsRsListScreen.kt | 4 +- .../screens/CommentsRsTabListScreen.kt | 12 ++-- .../commentsrs/CommentsRsListViewModelTest.kt | 2 +- 7 files changed, 49 insertions(+), 55 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt index 0abbc66bba71..a7714524a872 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt @@ -71,9 +71,7 @@ class CommentsRsDataSource @Inject constructor( data class Success( val comments: List, /** Ready-made params for the next page; null when this was the last page. */ - val nextPageParams: CommentListParams?, - /** Total matching comments from the X-WP-Total header, when the server provides it. */ - val total: Int? + val nextPageParams: CommentListParams? ) : RsCommentsPageResult data class Error(val message: String?) : RsCommentsPageResult @@ -101,8 +99,7 @@ class CommentsRsDataSource @Inject constructor( when (val result = client.request { it.comments().listWithViewContext(params) }) { is WpRequestResult.Success -> RsCommentsPageResult.Success( comments = result.response.data.map { it.toRsCommentListItem() }, - nextPageParams = result.response.nextPageParams, - total = result.response.headerMap.wpTotal()?.toInt() + nextPageParams = result.response.nextPageParams ) is WpRequestResult.WpError -> RsCommentsPageResult.Error(result.errorMessage) else -> RsCommentsPageResult.Error(null) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt index 34ae3fb9bac9..758c456cfd56 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt @@ -2,12 +2,6 @@ package org.wordpress.android.ui.commentsrs import org.wordpress.android.fluxc.model.CommentStatus -data class CommentsRsSnackbarMessage( - val message: String, - val actionLabel: String? = null, - val onAction: (() -> Unit)? = null -) - /** One comment row. */ data class CommentRsUiModel( val remoteCommentId: Long, @@ -29,6 +23,5 @@ data class CommentsTabUiState( val isRefreshing: Boolean = false, val isLoadingMore: Boolean = false, val canLoadMore: Boolean = false, - val error: String? = null, - val isAuthError: Boolean = false + val error: String? = null ) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt index 4ccba38514c0..32ad0d25dfad 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt @@ -21,6 +21,7 @@ import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsCommentL import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsCommentsPageResult import org.wordpress.android.ui.mysite.SelectedSiteRepository import org.wordpress.android.ui.postsrs.PostRsErrorUtils +import org.wordpress.android.ui.postsrs.SnackbarMessage import org.wordpress.android.util.DateTimeUtilsWrapper import org.wordpress.android.util.HtmlUtils import org.wordpress.android.util.NetworkUtilsWrapper @@ -51,7 +52,7 @@ class CommentsRsListViewModel @Inject constructor( private val _events = Channel(Channel.BUFFERED) val events = _events.receiveAsFlow() - private val _snackbarMessages = Channel(Channel.BUFFERED) + private val _snackbarMessages = Channel(Channel.BUFFERED) val snackbarMessages = _snackbarMessages.receiveAsFlow() // Pagination cursors: the next-page params returned with each fetched page, per tab. @@ -109,23 +110,10 @@ class CommentsRsListViewModel @Inject constructor( viewModelScope.launch { val result = withContext(bgDispatcher) { commentsRsDataSource.fetchCommentsPage(site, params) } when (result) { - is RsCommentsPageResult.Success -> { - nextPageParams[tab] = result.nextPageParams - updateTabUiState(tab) { - copy( - // A comment can shift pages if the list changed server-side between - // requests, so dedupe on append. - comments = (comments + result.comments.map { it.toUiModel() }) - .distinctBy { it.remoteCommentId }, - isLoadingMore = false, - canLoadMore = result.nextPageParams != null - ) - } - resolvePostTitles(tab) - } + is RsCommentsPageResult.Success -> applyPage(tab, result, append = true) is RsCommentsPageResult.Error -> { updateTabUiState(tab) { copy(isLoadingMore = false) } - _snackbarMessages.trySend(CommentsRsSnackbarMessage(errorMessage(result.message))) + _snackbarMessages.trySend(SnackbarMessage(errorMessage(result.message))) } } } @@ -142,26 +130,43 @@ class CommentsRsListViewModel @Inject constructor( val params = commentsRsDataSource.firstPageParams(status = tab.queryStatus) val result = withContext(bgDispatcher) { commentsRsDataSource.fetchCommentsPage(site, params) } when (result) { - is RsCommentsPageResult.Success -> { - nextPageParams[tab] = result.nextPageParams - updateTabUiState(tab) { - copy( - comments = result.comments.map { it.toUiModel() }, - isLoading = false, - isRefreshing = false, - canLoadMore = result.nextPageParams != null, - error = null, - isAuthError = false - ) - } - resolvePostTitles(tab) - } + is RsCommentsPageResult.Success -> applyPage(tab, result, append = false) is RsCommentsPageResult.Error -> onFirstPageError(tab, result.message) } onComplete() } } + /** + * A fetched page: stores the next-page cursor, applies the rows to the tab state, and kicks + * off post-title resolution. [append] is true when the page extends the list (load more) + * rather than replacing it (first page / refresh). + */ + private fun applyPage(tab: CommentsRsListTab, result: RsCommentsPageResult.Success, append: Boolean) { + nextPageParams[tab] = result.nextPageParams + val newRows = result.comments.map { it.toUiModel() } + updateTabUiState(tab) { + if (append) { + copy( + // A comment can shift pages if the list changed server-side between + // requests, so dedupe on append. + comments = (comments + newRows).distinctBy { it.remoteCommentId }, + isLoadingMore = false, + canLoadMore = result.nextPageParams != null + ) + } else { + copy( + comments = newRows, + isLoading = false, + isRefreshing = false, + canLoadMore = result.nextPageParams != null, + error = null + ) + } + } + resolvePostTitles(tab) + } + /** * First-page failure: when the tab already shows comments keep them and offer a retry * snackbar; when it's empty, show the full-screen error state. @@ -171,7 +176,7 @@ class CommentsRsListViewModel @Inject constructor( if (getTabUiState(tab).comments.isNotEmpty()) { updateTabUiState(tab) { copy(isLoading = false, isRefreshing = false, error = null) } _snackbarMessages.trySend( - CommentsRsSnackbarMessage( + SnackbarMessage( message = friendly, actionLabel = resourceProvider.getString(R.string.retry), onAction = { refreshTab(tab) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt index 068421b3d396..860c8ebaf760 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt @@ -21,6 +21,7 @@ import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip +import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.vector.rememberVectorPainter import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.res.stringResource @@ -60,7 +61,7 @@ fun CommentsRsListItem( if (comment.isPending) { MaterialTheme.colorScheme.tertiary } else { - MaterialTheme.colorScheme.surface.copy(alpha = 0f) + Color.Transparent } ) ) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt index aeb7fe31e3f8..ce6b05bc0c28 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -32,14 +32,14 @@ import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.launch import org.wordpress.android.R import org.wordpress.android.ui.commentsrs.CommentsRsListTab -import org.wordpress.android.ui.commentsrs.CommentsRsSnackbarMessage import org.wordpress.android.ui.commentsrs.CommentsTabUiState +import org.wordpress.android.ui.postsrs.SnackbarMessage @OptIn(ExperimentalMaterial3Api::class) @Composable fun CommentsRsListScreen( tabStates: Map, - snackbarMessages: Flow = emptyFlow(), + snackbarMessages: Flow = emptyFlow(), onInitTab: (CommentsRsListTab) -> Unit, onRefreshTab: (CommentsRsListTab) -> Unit, onLoadMore: (CommentsRsListTab) -> Unit, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt index 66d8bdf2ab7b..89f118963254 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt @@ -69,7 +69,7 @@ fun CommentsRsTabListScreen( state.isLoading -> ShimmerList() state.error != null && state.comments.isEmpty() -> ErrorContent( error = state.error, - onRetry = if (state.isAuthError) null else onRefresh + onRetry = onRefresh ) state.comments.isEmpty() && !state.isRefreshing -> EmptyContent( emptyMessageResId = emptyMessageResId @@ -173,7 +173,7 @@ private fun PlaceholderItem() { } @Composable -private fun ErrorContent(error: String, onRetry: (() -> Unit)?) { +private fun ErrorContent(error: String, onRetry: () -> Unit) { Column( modifier = Modifier .fillMaxSize() @@ -193,11 +193,9 @@ private fun ErrorContent(error: String, onRetry: (() -> Unit)?) { color = MaterialTheme.colorScheme.onSurfaceVariant, textAlign = TextAlign.Center ) - if (onRetry != null) { - Spacer(modifier = Modifier.height(16.dp)) - Button(onClick = onRetry) { - Text(text = stringResource(R.string.retry)) - } + Spacer(modifier = Modifier.height(16.dp)) + Button(onClick = onRetry) { + Text(text = stringResource(R.string.retry)) } } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt index 48b627613ca0..59ec8c3694f5 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt @@ -78,7 +78,7 @@ class CommentsRsListViewModelTest : BaseUnitTest(StandardTestDispatcher()) { nextPageParams: CommentListParams? = null ) { whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())) - .thenReturn(RsCommentsPageResult.Success(comments, nextPageParams, comments.size)) + .thenReturn(RsCommentsPageResult.Success(comments, nextPageParams)) } @Test From 3cd35f7c257686d983b52601999238b881b85c21 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 2 Jul 2026 14:37:40 -0400 Subject: [PATCH 04/21] Use new string keys for the renamed experimental flag copy Danger requires immutable string keys: changed copy gets a new key rather than editing the old one in place. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/prefs/experimentalfeatures/ExperimentalFeatures.kt | 4 ++-- WordPress/src/main/res/values/strings.xml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/prefs/experimentalfeatures/ExperimentalFeatures.kt b/WordPress/src/main/java/org/wordpress/android/ui/prefs/experimentalfeatures/ExperimentalFeatures.kt index c955267e611e..59abcf640bb2 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/prefs/experimentalfeatures/ExperimentalFeatures.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/prefs/experimentalfeatures/ExperimentalFeatures.kt @@ -47,8 +47,8 @@ class ExperimentalFeatures @Inject constructor( ), RS_UNIFIED_COMMENTS( "rs_unified_comments", - R.string.experimental_rs_unified_comments, - R.string.experimental_rs_unified_comments_description + R.string.experimental_rs_comments, + R.string.experimental_rs_comments_description ); } } diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index 1778bc344c8d..c200d82bd0a2 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -1026,8 +1026,8 @@ You can turn off New Stats from the overflow menu in the Stats screen. New Pages List Show pages list in a more modern UI for WordPress.com and self-hosted sites with application passwords - RS Unified Comments - Show comments in a more modern UI for WordPress.com and self-hosted sites with application passwords + RS Unified Comments + Show comments in a more modern UI for WordPress.com and self-hosted sites with application passwords Failed to load post Failed to load page Copy URL From b1bf05841d4eb61907ed49fd1a848f6eb064248a Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 2 Jul 2026 15:21:34 -0400 Subject: [PATCH 05/21] Harden rs comments list paging and post-title resolution - Key the post-title cache by (site, post id) so titles can't leak across sites, and negative-cache unresolvable ids so they aren't re-requested on every page load - Set perPage explicitly on title batches (server default of 10 silently truncated larger batches) and fall back to the pages endpoint for comments left unresolved by the posts endpoint - Discard load-more results whose fetch predates the latest applied first page, preventing stale pages/cursors after a silent refresh - Offer a retry action on load-more failures and auto-advance past pages that dedupe away entirely, so pagination can't stall at the list bottom Co-Authored-By: Claude Opus 4.8 (1M context) --- .../comments/unified/CommentsRsDataSource.kt | 74 ++++++++++++++----- .../ui/commentsrs/CommentsRsListViewModel.kt | 34 ++++++++- .../commentsrs/CommentsRsListViewModelTest.kt | 66 +++++++++++++++++ 3 files changed, 152 insertions(+), 22 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt index a7714524a872..44bebc60297c 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt @@ -37,7 +37,10 @@ import uniffi.wp_api.CommentStatus as RsCommentStatus class CommentsRsDataSource @Inject constructor( private val wpApiClientProvider: WpApiClientProvider ) { - private val postTitleCache = ConcurrentHashMap() + // Keyed by (local site id, post id): post ids are only unique per site, and this is a + // singleton shared across site switches. An empty value is a negative-cache entry for ids + // whose title can't be resolved (custom post types, non-published posts). + private val postTitleCache = ConcurrentHashMap, String>() data class RsComment( val authorName: String, @@ -116,43 +119,74 @@ class CommentsRsDataSource @Inject constructor( ) /** - * Fetches post titles for the given [postIds] in a single sparse-field network call, returning + * Fetches post titles for the given [postIds] in batched sparse-field network calls, returning * a map of post id to rendered title. Ids already cached skip the network round-trip. * (List rows show "author commented on {post}", but the comment payload only has the post id.) + * + * Comments can be attached to posts or pages, so unresolved ids fall back to the pages + * endpoint. Ids neither endpoint returns (custom post types, non-published posts) resolve to + * an empty title and are negative-cached so they aren't re-requested on every page load. */ suspend fun fetchPostTitles(site: SiteModel, postIds: List): Map { val result = mutableMapOf() val uncached = mutableListOf() for (id in postIds.distinct()) { - val cached = postTitleCache[id] + val cached = postTitleCache[site.id to id] if (cached != null) result[id] = cached else uncached.add(id) } if (uncached.isEmpty()) return result safe(errorValue = Unit) { - val client = wpApiClientProvider.getWpApiClient(site) - val response = client.request { - it.posts().filterListWithViewContext( - PostEndpointType.Posts, - PostListParams(include = uncached), - listOf(SparseAnyPostFieldWithViewContext.ID, SparseAnyPostFieldWithViewContext.TITLE) - ) - } - if (response is WpRequestResult.Success) { - for (post in response.response.data) { - val id = post.id ?: continue - val title = post.title?.rendered.orEmpty() - postTitleCache[id] = title - result[id] = title - } + val unresolved = fetchTitlesFromEndpoint(site, PostEndpointType.Posts, uncached, result) + ?: return@safe + val stillUnresolved = if (unresolved.isEmpty()) { + unresolved } else { - val message = (response as? WpRequestResult.WpError<*>)?.errorMessage - AppLog.w(AppLog.T.COMMENTS, "fetchPostTitles failed: $message") + fetchTitlesFromEndpoint(site, PostEndpointType.Pages, unresolved, result) ?: return@safe + } + // Only reached when both endpoints succeeded, so these ids genuinely have no + // resolvable title (as opposed to a transient request failure). + for (id in stillUnresolved) { + postTitleCache[site.id to id] = "" + result[id] = "" } } return result } + /** + * Fetches titles for [ids] from [endpointType] into [into] (and the cache), returning the ids + * the endpoint didn't include, or null when the request failed. [PostListParams.perPage] is + * set explicitly: the server default of 10 would silently truncate larger batches. + */ + private suspend fun fetchTitlesFromEndpoint( + site: SiteModel, + endpointType: PostEndpointType, + ids: List, + into: MutableMap + ): List? { + val client = wpApiClientProvider.getWpApiClient(site) + val response = client.request { + it.posts().filterListWithViewContext( + endpointType, + PostListParams(include = ids, perPage = ids.size.toUInt()), + listOf(SparseAnyPostFieldWithViewContext.ID, SparseAnyPostFieldWithViewContext.TITLE) + ) + } + if (response !is WpRequestResult.Success) { + val message = (response as? WpRequestResult.WpError<*>)?.errorMessage + AppLog.w(AppLog.T.COMMENTS, "fetchPostTitles ($endpointType) failed: $message") + return null + } + for (post in response.response.data) { + val id = post.id ?: continue + val title = post.title?.rendered.orEmpty() + postTitleCache[site.id to id] = title + into[id] = title + } + return ids.filter { it !in into } + } + suspend fun updateStatus(site: SiteModel, commentId: Long, status: CommentStatus): RsResult = write(site) { it.comments().update(commentId, CommentUpdateParams(status = status.toRsCommentStatus())) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt index 32ad0d25dfad..3f9711f2b716 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt @@ -57,6 +57,10 @@ class CommentsRsListViewModel @Inject constructor( // Pagination cursors: the next-page params returned with each fetched page, per tab. private val nextPageParams = mutableMapOf() + + // Bumped whenever a first page is applied. A load-more result whose fetch started before the + // bump paged from a cursor that predates the new page 1, so it's stale and gets discarded. + private val pageGenerations = mutableMapOf() private val initializingTabs = mutableSetOf() private val resolveTitleJobs = mutableMapOf() @@ -107,13 +111,38 @@ class CommentsRsListViewModel @Inject constructor( if (params == null || isBusy) return updateTabUiState(tab) { copy(isLoadingMore = true) } + val generation = pageGenerations[tab] viewModelScope.launch { val result = withContext(bgDispatcher) { commentsRsDataSource.fetchCommentsPage(site, params) } + if (pageGenerations[tab] != generation) { + // A refresh replaced the list while this page was in flight, so its cursor is + // stale; appending it would mix old and new pages and corrupt the cursor chain. + updateTabUiState(tab) { copy(isLoadingMore = false) } + return@launch + } when (result) { - is RsCommentsPageResult.Success -> applyPage(tab, result, append = true) + is RsCommentsPageResult.Success -> { + val sizeBefore = getTabUiState(tab).comments.size + applyPage(tab, result, append = true) + // When the whole page deduped away (the list shifted server-side), the + // scroll position hasn't changed so the load-more trigger won't re-fire; + // advance to the next page directly. + if (result.comments.isNotEmpty() && + getTabUiState(tab).comments.size == sizeBefore && + nextPageParams[tab] != null + ) { + loadMore(tab) + } + } is RsCommentsPageResult.Error -> { updateTabUiState(tab) { copy(isLoadingMore = false) } - _snackbarMessages.trySend(SnackbarMessage(errorMessage(result.message))) + _snackbarMessages.trySend( + SnackbarMessage( + message = errorMessage(result.message), + actionLabel = resourceProvider.getString(R.string.retry), + onAction = { loadMore(tab) } + ) + ) } } } @@ -143,6 +172,7 @@ class CommentsRsListViewModel @Inject constructor( * rather than replacing it (first page / refresh). */ private fun applyPage(tab: CommentsRsListTab, result: RsCommentsPageResult.Success, append: Boolean) { + if (!append) pageGenerations[tab] = (pageGenerations[tab] ?: 0) + 1 nextPageParams[tab] = result.nextPageParams val newRows = result.comments.map { it.toUiModel() } updateTabUiState(tab) { diff --git a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt index 59ec8c3694f5..825934f3ac64 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt @@ -167,6 +167,71 @@ class CommentsRsListViewModelTest : BaseUnitTest(StandardTestDispatcher()) { assertThat(state.canLoadMore).isFalse() } + @Test + fun `stale loadMore result arriving after a silent refresh is discarded`() = test { + givenPage(listOf(rsItem(id = 1), rsItem(id = 2)), nextPageParams = NEXT_PAGE) + val viewModel = createViewModel() + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + // Next two fetches: the silent refresh gets a fresh first page; the concurrently + // launched loadMore gets the (now stale) old second page. + whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())).thenReturn( + RsCommentsPageResult.Success(listOf(rsItem(id = 10), rsItem(id = 11)), NEXT_PAGE), + RsCommentsPageResult.Success(listOf(rsItem(id = 3), rsItem(id = 4)), null) + ) + + viewModel.refreshTab(CommentsRsListTab.ALL) // silent: sets no busy flag + viewModel.loadMore(CommentsRsListTab.ALL) + advanceUntilIdle() + + val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) + assertThat(state.comments.map { it.remoteCommentId }).containsExactly(10L, 11L) + assertThat(state.isLoadingMore).isFalse() + assertThat(state.canLoadMore).isTrue() + } + + @Test + fun `loadMore failure offers a retry snackbar`() = test { + givenPage(listOf(rsItem(id = 1)), nextPageParams = NEXT_PAGE) + val viewModel = createViewModel() + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())) + .thenReturn(RsCommentsPageResult.Error("boom")) + + viewModel.snackbarMessages.test { + viewModel.loadMore(CommentsRsListTab.ALL) + advanceUntilIdle() + + val snackbar = awaitItem() + assertThat(snackbar.message).isEqualTo("boom") + assertThat(snackbar.onAction != null).isTrue() + cancelAndIgnoreRemainingEvents() + } + assertThat(viewModel.tabStates.value.getValue(CommentsRsListTab.ALL).isLoadingMore).isFalse() + } + + @Test + fun `fully deduplicated page auto-advances to the next page`() = test { + givenPage(listOf(rsItem(id = 1), rsItem(id = 2)), nextPageParams = NEXT_PAGE) + val viewModel = createViewModel() + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + // The next page duplicates the current rows entirely (server-side shift), then the + // page after that has the genuinely new comment. + whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())).thenReturn( + RsCommentsPageResult.Success(listOf(rsItem(id = 1), rsItem(id = 2)), THIRD_PAGE), + RsCommentsPageResult.Success(listOf(rsItem(id = 3)), null) + ) + + viewModel.loadMore(CommentsRsListTab.ALL) + advanceUntilIdle() + + val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) + assertThat(state.comments.map { it.remoteCommentId }).containsExactly(1L, 2L, 3L) + assertThat(state.canLoadMore).isFalse() + } + @Test fun `loadMore is a no-op when there is no next page`() = test { givenPage(listOf(rsItem(id = 1)), nextPageParams = null) @@ -262,5 +327,6 @@ class CommentsRsListViewModelTest : BaseUnitTest(StandardTestDispatcher()) { companion object { private val FIRST_PAGE = CommentListParams() private val NEXT_PAGE = CommentListParams(page = 2u) + private val THIRD_PAGE = CommentListParams(page = 3u) } } From 82663174031a9aa483b061bdb670e32a581171a1 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 2 Jul 2026 15:43:17 -0400 Subject: [PATCH 06/21] Address review findings in rs comments paging hardening - Chunk post-title batches to the server's per_page maximum of 100; larger values are rejected outright with rest_invalid_param - Always attempt the pages endpoint for ids the posts endpoint didn't resolve, and only negative-cache when both endpoints succeeded - Clear negative-cached titles on user-initiated refresh so posts published after first sight can resolve - Cap unattended load-more auto-advance at 3 pages and let it handle empty pages, not just fully-deduplicated ones - Don't cancel/re-fire an identical in-flight title-resolve batch - Add CommentsRsDataSourceTest covering the cache, fallback, and chunking behavior, plus ViewModel tests for the new paging rules Co-Authored-By: Claude Opus 4.8 (1M context) --- .../comments/unified/CommentsRsDataSource.kt | 80 +++++---- .../ui/commentsrs/CommentsRsListViewModel.kt | 44 +++-- .../unified/CommentsRsDataSourceTest.kt | 157 ++++++++++++++++++ .../commentsrs/CommentsRsListViewModelTest.kt | 57 +++++++ 4 files changed, 296 insertions(+), 42 deletions(-) create mode 100644 WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt index 44bebc60297c..c3a1df5ff359 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt @@ -137,54 +137,69 @@ class CommentsRsDataSource @Inject constructor( if (uncached.isEmpty()) return result safe(errorValue = Unit) { - val unresolved = fetchTitlesFromEndpoint(site, PostEndpointType.Posts, uncached, result) - ?: return@safe - val stillUnresolved = if (unresolved.isEmpty()) { - unresolved + val postsSucceeded = fetchTitlesFromEndpoint(site, PostEndpointType.Posts, uncached, result) + val remaining = uncached.filter { it !in result } + val pagesSucceeded = if (remaining.isEmpty()) { + true } else { - fetchTitlesFromEndpoint(site, PostEndpointType.Pages, unresolved, result) ?: return@safe + fetchTitlesFromEndpoint(site, PostEndpointType.Pages, remaining, result) } - // Only reached when both endpoints succeeded, so these ids genuinely have no - // resolvable title (as opposed to a transient request failure). - for (id in stillUnresolved) { - postTitleCache[site.id to id] = "" - result[id] = "" + // Only when both endpoints succeeded are the leftover ids genuinely unresolvable + // (custom post types, non-published posts) rather than a transient failure. + if (postsSucceeded && pagesSucceeded) { + for (id in uncached.filter { it !in result }) { + postTitleCache[site.id to id] = "" + result[id] = "" + } } } return result } /** - * Fetches titles for [ids] from [endpointType] into [into] (and the cache), returning the ids - * the endpoint didn't include, or null when the request failed. [PostListParams.perPage] is - * set explicitly: the server default of 10 would silently truncate larger batches. + * Forgets negative-cached (unresolvable) titles for [site] so they're retried, e.g. on a + * user-initiated refresh — a post that wasn't published when first seen may be by now. + */ + fun clearUnresolvedPostTitles(site: SiteModel) { + postTitleCache.entries.removeIf { it.key.first == site.id && it.value.isEmpty() } + } + + /** + * Fetches titles for [ids] from [endpointType] into [into] (and the cache), returning whether + * every request succeeded. [PostListParams.perPage] is set explicitly (the server default of + * 10 would silently truncate larger batches) and ids are chunked to the server's per_page + * maximum of 100 (a larger value is rejected outright with `rest_invalid_param`). */ private suspend fun fetchTitlesFromEndpoint( site: SiteModel, endpointType: PostEndpointType, ids: List, into: MutableMap - ): List? { + ): Boolean { val client = wpApiClientProvider.getWpApiClient(site) - val response = client.request { - it.posts().filterListWithViewContext( - endpointType, - PostListParams(include = ids, perPage = ids.size.toUInt()), - listOf(SparseAnyPostFieldWithViewContext.ID, SparseAnyPostFieldWithViewContext.TITLE) - ) - } - if (response !is WpRequestResult.Success) { - val message = (response as? WpRequestResult.WpError<*>)?.errorMessage - AppLog.w(AppLog.T.COMMENTS, "fetchPostTitles ($endpointType) failed: $message") - return null - } - for (post in response.response.data) { - val id = post.id ?: continue - val title = post.title?.rendered.orEmpty() - postTitleCache[site.id to id] = title - into[id] = title + var allSucceeded = true + for (chunk in ids.chunked(MAX_TITLES_PER_REQUEST)) { + val response = client.request { + it.posts().filterListWithViewContext( + endpointType, + PostListParams(include = chunk, perPage = chunk.size.toUInt()), + listOf(SparseAnyPostFieldWithViewContext.ID, SparseAnyPostFieldWithViewContext.TITLE) + ) + } + if (response is WpRequestResult.Success) { + for (post in response.response.data) { + val id = post.id ?: continue + val title = post.title?.rendered.orEmpty() + postTitleCache[site.id to id] = title + into[id] = title + } + } else { + val message = (response as? WpRequestResult.WpError<*>)?.errorMessage + AppLog.w(AppLog.T.COMMENTS, "fetchPostTitles ($endpointType) failed: $message") + allSucceeded = false + } } - return ids.filter { it !in into } + return allSucceeded } suspend fun updateStatus(site: SiteModel, commentId: Long, status: CommentStatus): RsResult = @@ -236,6 +251,7 @@ class CommentsRsDataSource @Inject constructor( companion object { internal const val COMMENTS_PAGE_SIZE = 30u + private const val MAX_TITLES_PER_REQUEST = 100 } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt index 3f9711f2b716..7a4bfd7912d2 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt @@ -62,7 +62,10 @@ class CommentsRsListViewModel @Inject constructor( // bump paged from a cursor that predates the new page 1, so it's stale and gets discarded. private val pageGenerations = mutableMapOf() private val initializingTabs = mutableSetOf() - private val resolveTitleJobs = mutableMapOf() + + // The in-flight title-resolve job per tab, with the ids it's resolving so an identical + // request isn't cancelled and re-fired when an applied page adds nothing new. + private val resolveTitleJobs = mutableMapOf>>() private val _site: SiteModel? = selectedSiteRepository.getSelectedSite() private val site: SiteModel @@ -92,6 +95,9 @@ class CommentsRsListViewModel @Inject constructor( @MainThread fun refreshTab(tab: CommentsRsListTab, isUserRefresh: Boolean = false) { if (!_tabStates.value.containsKey(tab)) return + // An explicit refresh also retries post titles that previously resolved as + // unresolvable — e.g. a post that has been published since it was first seen. + if (isUserRefresh) commentsRsDataSource.clearUnresolvedPostTitles(site) updateTabUiState(tab) { copy(isRefreshing = isUserRefresh, error = null) } fetchFirstPage(tab) } @@ -105,6 +111,10 @@ class CommentsRsListViewModel @Inject constructor( /** Loads the next page for [tab] if there is one and nothing else is in flight. */ @MainThread fun loadMore(tab: CommentsRsListTab) { + loadMoreInternal(tab, autoAdvanceDepth = 0) + } + + private fun loadMoreInternal(tab: CommentsRsListTab, autoAdvanceDepth: Int) { val current = getTabUiState(tab) val params = nextPageParams[tab] val isBusy = current.isLoading || current.isRefreshing || current.isLoadingMore @@ -124,14 +134,16 @@ class CommentsRsListViewModel @Inject constructor( is RsCommentsPageResult.Success -> { val sizeBefore = getTabUiState(tab).comments.size applyPage(tab, result, append = true) - // When the whole page deduped away (the list shifted server-side), the - // scroll position hasn't changed so the load-more trigger won't re-fire; - // advance to the next page directly. - if (result.comments.isNotEmpty() && - getTabUiState(tab).comments.size == sizeBefore && - nextPageParams[tab] != null + // When a page adds no rows (it deduped away after a server-side shift, or + // came back empty mid-shrink), the scroll position hasn't changed so the + // load-more trigger won't re-fire; advance to the next page directly. The + // depth cap keeps a pathological cursor chain (e.g. a proxy ignoring the + // page param) from firing unbounded unattended requests. + if (getTabUiState(tab).comments.size == sizeBefore && + nextPageParams[tab] != null && + autoAdvanceDepth < MAX_AUTO_ADVANCE_PAGES ) { - loadMore(tab) + loadMoreInternal(tab, autoAdvanceDepth + 1) } } is RsCommentsPageResult.Error -> { @@ -230,14 +242,20 @@ class CommentsRsListViewModel @Inject constructor( .distinct() if (unresolvedIds.isEmpty()) return - resolveTitleJobs[tab]?.cancel() - resolveTitleJobs[tab] = viewModelScope.launch { + // An applied page that added no rows (deduped away) leaves the same ids unresolved; + // let the in-flight request finish rather than cancelling and re-firing it. + val inFlight = resolveTitleJobs[tab] + if (inFlight != null && inFlight.first.isActive && inFlight.second == unresolvedIds) return + + inFlight?.first?.cancel() + val job = viewModelScope.launch { val titles = withContext(bgDispatcher) { commentsRsDataSource.fetchPostTitles(site, unresolvedIds) } if (titles.isEmpty()) return@launch updateTabUiState(tab) { copy(comments = comments.map { it.copy(postTitle = titles[it.postId] ?: it.postTitle) }) } } + resolveTitleJobs[tab] = job to unresolvedIds } /** The server message when it sent one, otherwise a friendly offline/generic message. */ @@ -261,4 +279,10 @@ class CommentsRsListViewModel @Inject constructor( private fun updateTabUiState(tab: CommentsRsListTab, update: CommentsTabUiState.() -> CommentsTabUiState) { _tabStates.value += (tab to getTabUiState(tab).update()) } + + companion object { + // How many extra pages a single load-more may fetch unattended when applied pages + // add no rows, before waiting for the next user scroll. + private const val MAX_AUTO_ADVANCE_PAGES = 3 + } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt new file mode 100644 index 000000000000..475eddd0fd38 --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt @@ -0,0 +1,157 @@ +package org.wordpress.android.ui.comments.unified + +import kotlinx.coroutines.test.runTest +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.mock +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider +import rs.wordpress.api.kotlin.WpApiClient +import rs.wordpress.api.kotlin.WpRequestResult +import uniffi.wp_api.PostsRequestFilterListWithViewContextResponse +import uniffi.wp_api.RequestMethod +import uniffi.wp_api.SparseAnyPostWithViewContext +import uniffi.wp_api.SparsePostTitleWithViewContext +import uniffi.wp_api.WpErrorCode + +/** + * Tests for [CommentsRsDataSource.fetchPostTitles]: the per-site title cache, the posts→pages + * endpoint fallback, negative caching of unresolvable ids, and request chunking. + */ +class CommentsRsDataSourceTest { + private val wpApiClient: WpApiClient = mock() + private val wpApiClientProvider: WpApiClientProvider = mock() + private lateinit var dataSource: CommentsRsDataSource + + private val siteA = SiteModel().apply { id = 1 } + private val siteB = SiteModel().apply { id = 2 } + + @Before + fun setUp() { + dataSource = CommentsRsDataSource(wpApiClientProvider) + whenever(wpApiClientProvider.getWpApiClient(any(), anyOrNull())).thenReturn(wpApiClient) + } + + @Test + fun `resolved titles are cached and skip the network`() = runTest { + stubRequests(successResponse(sparsePost(5, "Hello"))) + + assertThat(dataSource.fetchPostTitles(siteA, listOf(5))).isEqualTo(mapOf(5L to "Hello")) + assertThat(dataSource.fetchPostTitles(siteA, listOf(5))).isEqualTo(mapOf(5L to "Hello")) + + verify(wpApiClient, times(1)).request(any()) + } + + @Test + fun `titles are cached per site, not just per post id`() = runTest { + stubRequests( + successResponse(sparsePost(5, "Site A post")), + successResponse(sparsePost(5, "Site B post")) + ) + + assertThat(dataSource.fetchPostTitles(siteA, listOf(5))).isEqualTo(mapOf(5L to "Site A post")) + assertThat(dataSource.fetchPostTitles(siteB, listOf(5))).isEqualTo(mapOf(5L to "Site B post")) + + verify(wpApiClient, times(2)).request(any()) + } + + @Test + fun `a posts endpoint failure still tries the pages endpoint`() = runTest { + stubRequests(wpError(), successResponse(sparsePost(7, "About"))) + + assertThat(dataSource.fetchPostTitles(siteA, listOf(7))).isEqualTo(mapOf(7L to "About")) + + verify(wpApiClient, times(2)).request(any()) + } + + @Test + fun `ids neither endpoint returns are negative-cached once both succeed`() = runTest { + stubRequests(successResponse(), successResponse()) + + assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "")) + assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "")) + + // Two requests (posts + pages) for the first call, none for the second. + verify(wpApiClient, times(2)).request(any()) + } + + @Test + fun `a transient failure is not negative-cached`() = runTest { + stubRequests( + successResponse(), // posts: id not found + wpError(), // pages: transient failure — must NOT negative-cache + successResponse(sparsePost(9, "Resolved later")) + ) + + assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEmpty() + assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "Resolved later")) + + verify(wpApiClient, times(3)).request(any()) + } + + @Test + fun `clearUnresolvedPostTitles retries negative-cached ids`() = runTest { + stubRequests( + successResponse(), // posts: not found + successResponse(), // pages: not found → negative-cached + successResponse(sparsePost(9, "Now published")) + ) + assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "")) + + dataSource.clearUnresolvedPostTitles(siteA) + + assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "Now published")) + verify(wpApiClient, times(3)).request(any()) + } + + @Test + fun `batches over 100 ids are chunked to the per_page maximum`() = runTest { + val ids = (1L..101L).toList() + stubRequests( + successResponse(*ids.take(100).map { sparsePost(it, "Post $it") }.toTypedArray()), + successResponse(sparsePost(101, "Post 101")) + ) + + val titles = dataSource.fetchPostTitles(siteA, ids) + + assertThat(titles).hasSize(101) + assertThat(titles[101L]).isEqualTo("Post 101") + verify(wpApiClient, times(2)).request(any()) + } + + private fun sparsePost(id: Long, title: String): SparseAnyPostWithViewContext { + val sparseTitle = mock() + whenever(sparseTitle.rendered).thenReturn(title) + val post = mock() + whenever(post.id).thenReturn(id) + whenever(post.title).thenReturn(sparseTitle) + return post + } + + private fun successResponse(vararg posts: SparseAnyPostWithViewContext) = WpRequestResult.Success( + response = PostsRequestFilterListWithViewContextResponse(posts.toList(), mock(), null, null) + ) + + private fun wpError() = WpRequestResult.WpError( + errorCode = WpErrorCode.Forbidden(), + errorMessage = "server said no", + statusCode = 403u, + response = "", + requestUrl = "https://example.com", + requestMethod = RequestMethod.GET + ) + + @Suppress("UNCHECKED_CAST") + private suspend fun stubRequests(vararg responses: WpRequestResult<*>) { + var stubbing = whenever(wpApiClient.request(any())) + for (response in responses) { + stubbing = stubbing.thenReturn(response as WpRequestResult) + } + } +} diff --git a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt index 825934f3ac64..e448ad830fd4 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt @@ -13,6 +13,7 @@ import org.mockito.Mock import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.eq +import org.mockito.kotlin.never import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -232,6 +233,62 @@ class CommentsRsListViewModelTest : BaseUnitTest(StandardTestDispatcher()) { assertThat(state.canLoadMore).isFalse() } + @Test + fun `an empty page with a remaining cursor auto-advances to the next page`() = test { + givenPage(listOf(rsItem(id = 1), rsItem(id = 2)), nextPageParams = NEXT_PAGE) + val viewModel = createViewModel() + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + // A middle page can come back empty (comments deleted server-side) while the + // headers still advertise more pages. + whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())).thenReturn( + RsCommentsPageResult.Success(emptyList(), THIRD_PAGE), + RsCommentsPageResult.Success(listOf(rsItem(id = 3)), null) + ) + + viewModel.loadMore(CommentsRsListTab.ALL) + advanceUntilIdle() + + val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) + assertThat(state.comments.map { it.remoteCommentId }).containsExactly(1L, 2L, 3L) + assertThat(state.canLoadMore).isFalse() + } + + @Test + fun `auto-advance is capped when pages keep adding nothing`() = test { + givenPage(listOf(rsItem(id = 1)), nextPageParams = NEXT_PAGE) + val viewModel = createViewModel() + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + // Pathological cursor chain: every page dedupes away with a cursor still present. + whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())) + .thenReturn(RsCommentsPageResult.Success(listOf(rsItem(id = 1)), NEXT_PAGE)) + + viewModel.loadMore(CommentsRsListTab.ALL) + advanceUntilIdle() + + // 1 init + 1 user loadMore + at most 3 auto-advances, then the chain stops. + verify(commentsRsDataSource, times(5)).fetchCommentsPage(eq(site), any()) + val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) + assertThat(state.isLoadingMore).isFalse() + } + + @Test + fun `a user refresh retries unresolvable post titles, a silent refresh does not`() = test { + givenPage(listOf(rsItem(id = 1))) + val viewModel = createViewModel() + viewModel.initTab(CommentsRsListTab.ALL) + advanceUntilIdle() + + viewModel.refreshTab(CommentsRsListTab.ALL) // silent + advanceUntilIdle() + verify(commentsRsDataSource, never()).clearUnresolvedPostTitles(any()) + + viewModel.refreshTab(CommentsRsListTab.ALL, isUserRefresh = true) + advanceUntilIdle() + verify(commentsRsDataSource).clearUnresolvedPostTitles(site) + } + @Test fun `loadMore is a no-op when there is no next page`() = test { givenPage(listOf(rsItem(id = 1)), nextPageParams = null) From ca271c875262da5fc33375720160a5535a8f3ee6 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 2 Jul 2026 16:04:41 -0400 Subject: [PATCH 07/21] Close title-resolve race windows and strengthen data source tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Cancel a tab's in-flight title-resolve job on user refresh so a job holding pre-clear 'not found' results can't re-apply them via the identical-ids guard - Generation-stamp the negative cache so a fetch already in flight when the user cleared can't re-poison entries with pre-clear results - Make the data source tests execute each request's builder lambda against a mocked uniffi client, asserting the endpoint, include count, and perPage actually sent — the chunking test now fails if chunking is reverted, and the posts-before-pages order is asserted, not assumed Co-Authored-By: Claude Opus 4.8 (1M context) --- .../comments/unified/CommentsRsDataSource.kt | 12 +- .../ui/commentsrs/CommentsRsListViewModel.kt | 12 +- .../unified/CommentsRsDataSourceTest.kt | 107 +++++++++++++++--- .../commentsrs/CommentsRsListViewModelTest.kt | 25 ++++ 4 files changed, 135 insertions(+), 21 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt index c3a1df5ff359..631f935c5f9a 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSource.kt @@ -42,6 +42,11 @@ class CommentsRsDataSource @Inject constructor( // whose title can't be resolved (custom post types, non-published posts). private val postTitleCache = ConcurrentHashMap, String>() + // Bumped by [clearUnresolvedPostTitles]: a fetch already in flight when the user cleared + // would otherwise re-poison the cache with its pre-clear "not found" results. + @Volatile + private var unresolvedCacheGeneration = 0 + data class RsComment( val authorName: String, val authorAvatarUrl: String, @@ -137,6 +142,7 @@ class CommentsRsDataSource @Inject constructor( if (uncached.isEmpty()) return result safe(errorValue = Unit) { + val generation = unresolvedCacheGeneration val postsSucceeded = fetchTitlesFromEndpoint(site, PostEndpointType.Posts, uncached, result) val remaining = uncached.filter { it !in result } val pagesSucceeded = if (remaining.isEmpty()) { @@ -145,8 +151,9 @@ class CommentsRsDataSource @Inject constructor( fetchTitlesFromEndpoint(site, PostEndpointType.Pages, remaining, result) } // Only when both endpoints succeeded are the leftover ids genuinely unresolvable - // (custom post types, non-published posts) rather than a transient failure. - if (postsSucceeded && pagesSucceeded) { + // (custom post types, non-published posts) rather than a transient failure — and + // only when no clear happened mid-flight, so pre-clear results can't re-poison it. + if (postsSucceeded && pagesSucceeded && generation == unresolvedCacheGeneration) { for (id in uncached.filter { it !in result }) { postTitleCache[site.id to id] = "" result[id] = "" @@ -161,6 +168,7 @@ class CommentsRsDataSource @Inject constructor( * user-initiated refresh — a post that wasn't published when first seen may be by now. */ fun clearUnresolvedPostTitles(site: SiteModel) { + unresolvedCacheGeneration++ postTitleCache.entries.removeIf { it.key.first == site.id && it.value.isEmpty() } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt index 7a4bfd7912d2..8b436a71238d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt @@ -95,9 +95,15 @@ class CommentsRsListViewModel @Inject constructor( @MainThread fun refreshTab(tab: CommentsRsListTab, isUserRefresh: Boolean = false) { if (!_tabStates.value.containsKey(tab)) return - // An explicit refresh also retries post titles that previously resolved as - // unresolvable — e.g. a post that has been published since it was first seen. - if (isUserRefresh) commentsRsDataSource.clearUnresolvedPostTitles(site) + if (isUserRefresh) { + // An explicit refresh also retries post titles that previously resolved as + // unresolvable — e.g. a post that has been published since it was first seen. + commentsRsDataSource.clearUnresolvedPostTitles(site) + // A resolve job that started before the clear may apply pre-clear "not found" + // titles; cancel it so the post-refresh pass fetches fresh results instead of + // deferring to it via the identical-ids guard. + resolveTitleJobs[tab]?.first?.cancel() + } updateTabUiState(tab) { copy(isRefreshing = isUserRefresh, error = null) } fetchFirstPage(tab) } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt index 475eddd0fd38..e704d2361259 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt @@ -6,29 +6,51 @@ import org.junit.Before import org.junit.Test import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.doSuspendableAnswer import org.mockito.kotlin.mock -import org.mockito.kotlin.times -import org.mockito.kotlin.verify +import org.mockito.kotlin.stub import org.mockito.kotlin.whenever import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider import rs.wordpress.api.kotlin.WpApiClient import rs.wordpress.api.kotlin.WpRequestResult +import uniffi.wp_api.PostEndpointType +import uniffi.wp_api.PostListParams +import uniffi.wp_api.PostsRequestExecutor import uniffi.wp_api.PostsRequestFilterListWithViewContextResponse import uniffi.wp_api.RequestMethod import uniffi.wp_api.SparseAnyPostWithViewContext import uniffi.wp_api.SparsePostTitleWithViewContext +import uniffi.wp_api.UniffiWpApiClient import uniffi.wp_api.WpErrorCode /** * Tests for [CommentsRsDataSource.fetchPostTitles]: the per-site title cache, the posts→pages * endpoint fallback, negative caching of unresolvable ids, and request chunking. + * + * The [wpApiClient] stub executes each request's builder lambda against a mocked + * [UniffiWpApiClient], recording the endpoint and paging params actually sent — so the tests + * pin the requests made, not just how many there were. */ class CommentsRsDataSourceTest { - private val wpApiClient: WpApiClient = mock() + private data class RecordedRequest( + val endpointType: PostEndpointType, + val includeCount: Int, + val perPage: UInt? + ) + private val wpApiClientProvider: WpApiClientProvider = mock() + private val wpApiClient: WpApiClient = mock() + private val uniffiClient: UniffiWpApiClient = mock() + private val postsExecutor: PostsRequestExecutor = mock() private lateinit var dataSource: CommentsRsDataSource + private val recordedRequests = mutableListOf() + private val cannedResults = ArrayDeque>() + + /** Invoked after each request completes; lets a test simulate concurrent work mid-fetch. */ + private var afterRequest: (() -> Unit)? = null + private val siteA = SiteModel().apply { id = 1 } private val siteB = SiteModel().apply { id = 2 } @@ -36,6 +58,29 @@ class CommentsRsDataSourceTest { fun setUp() { dataSource = CommentsRsDataSource(wpApiClientProvider) whenever(wpApiClientProvider.getWpApiClient(any(), anyOrNull())).thenReturn(wpApiClient) + whenever(uniffiClient.posts()).thenReturn(postsExecutor) + postsExecutor.stub { + on { filterListWithViewContext(any(), any(), any()) } doSuspendableAnswer { invocation -> + val params = invocation.getArgument(1) + recordedRequests += RecordedRequest( + endpointType = invocation.getArgument(0), + includeCount = params.include.size, + perPage = params.perPage + ) + // The payload is decided by the request-level stub below; this value is unused. + mock() + } + } + wpApiClient.stub { + on { request(any()) } doSuspendableAnswer { invocation -> + // Run the builder lambda so the executor stub above records what was requested. + val executor = invocation.getArgument Any>(0) + executor(uniffiClient) + val result = cannedResults.removeFirst() + afterRequest?.invoke() + result + } + } } @Test @@ -45,7 +90,7 @@ class CommentsRsDataSourceTest { assertThat(dataSource.fetchPostTitles(siteA, listOf(5))).isEqualTo(mapOf(5L to "Hello")) assertThat(dataSource.fetchPostTitles(siteA, listOf(5))).isEqualTo(mapOf(5L to "Hello")) - verify(wpApiClient, times(1)).request(any()) + assertThat(recordedRequests).hasSize(1) } @Test @@ -58,7 +103,7 @@ class CommentsRsDataSourceTest { assertThat(dataSource.fetchPostTitles(siteA, listOf(5))).isEqualTo(mapOf(5L to "Site A post")) assertThat(dataSource.fetchPostTitles(siteB, listOf(5))).isEqualTo(mapOf(5L to "Site B post")) - verify(wpApiClient, times(2)).request(any()) + assertThat(recordedRequests).hasSize(2) } @Test @@ -67,7 +112,8 @@ class CommentsRsDataSourceTest { assertThat(dataSource.fetchPostTitles(siteA, listOf(7))).isEqualTo(mapOf(7L to "About")) - verify(wpApiClient, times(2)).request(any()) + assertThat(recordedRequests.map { it.endpointType }) + .containsExactly(PostEndpointType.Posts, PostEndpointType.Pages) } @Test @@ -77,8 +123,9 @@ class CommentsRsDataSourceTest { assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "")) assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "")) - // Two requests (posts + pages) for the first call, none for the second. - verify(wpApiClient, times(2)).request(any()) + // Posts + pages for the first call, none for the second. + assertThat(recordedRequests.map { it.endpointType }) + .containsExactly(PostEndpointType.Posts, PostEndpointType.Pages) } @Test @@ -92,7 +139,8 @@ class CommentsRsDataSourceTest { assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEmpty() assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "Resolved later")) - verify(wpApiClient, times(3)).request(any()) + assertThat(recordedRequests.map { it.endpointType }) + .containsExactly(PostEndpointType.Posts, PostEndpointType.Pages, PostEndpointType.Posts) } @Test @@ -107,7 +155,29 @@ class CommentsRsDataSourceTest { dataSource.clearUnresolvedPostTitles(siteA) assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "Now published")) - verify(wpApiClient, times(3)).request(any()) + assertThat(recordedRequests).hasSize(3) + } + + @Test + fun `a clear during an in-flight fetch prevents negative caching`() = runTest { + stubRequests( + successResponse(), // posts: not found + successResponse(), // pages: not found — but a clear lands before the cache write + successResponse(), // retry: posts + successResponse() // retry: pages + ) + afterRequest = { + // Simulates another surface clearing while this fetch is between its network + // responses and its negative-cache write. + if (recordedRequests.size == 2) dataSource.clearUnresolvedPostTitles(siteA) + } + + assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEmpty() + + // Not negative-cached, so a later fetch goes back to the network. + afterRequest = null + assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "")) + assertThat(recordedRequests).hasSize(4) } @Test @@ -122,7 +192,13 @@ class CommentsRsDataSourceTest { assertThat(titles).hasSize(101) assertThat(titles[101L]).isEqualTo("Post 101") - verify(wpApiClient, times(2)).request(any()) + // Two posts-endpoint requests with the batch split at 100, each with a matching + // explicit perPage — NOT one oversized request plus a pages fallback. + assertThat(recordedRequests.map { Triple(it.endpointType, it.includeCount, it.perPage) }) + .containsExactly( + Triple(PostEndpointType.Posts, 100, 100u), + Triple(PostEndpointType.Posts, 1, 1u) + ) } private fun sparsePost(id: Long, title: String): SparseAnyPostWithViewContext { @@ -148,10 +224,9 @@ class CommentsRsDataSourceTest { ) @Suppress("UNCHECKED_CAST") - private suspend fun stubRequests(vararg responses: WpRequestResult<*>) { - var stubbing = whenever(wpApiClient.request(any())) - for (response in responses) { - stubbing = stubbing.thenReturn(response as WpRequestResult) - } + private fun stubRequests(vararg responses: WpRequestResult<*>) { + recordedRequests.clear() + cannedResults.clear() + responses.forEach { cannedResults.add(it as WpRequestResult) } } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt index e448ad830fd4..dbc2d9fe4fa2 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt @@ -4,7 +4,9 @@ import androidx.lifecycle.viewModelScope import app.cash.turbine.test import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.cancel +import kotlinx.coroutines.delay import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.runCurrent import org.assertj.core.api.Assertions.assertThat import org.junit.After import org.junit.Before @@ -12,6 +14,7 @@ import org.junit.Test import org.mockito.Mock import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.doSuspendableAnswer import org.mockito.kotlin.eq import org.mockito.kotlin.never import org.mockito.kotlin.times @@ -289,6 +292,28 @@ class CommentsRsListViewModelTest : BaseUnitTest(StandardTestDispatcher()) { verify(commentsRsDataSource).clearUnresolvedPostTitles(site) } + @Test + fun `a user refresh cancels an in-flight title resolve and fetches fresh titles`() = test { + givenPage(listOf(rsItem(id = 1, postId = 10))) + // The first resolve hangs mid-flight (holding pre-refresh results); the retry after + // the user refresh returns the fresh title. + whenever(commentsRsDataSource.fetchPostTitles(eq(site), any())) + .doSuspendableAnswer { delay(60_000); emptyMap() } + .thenReturn(mapOf(10L to "Fresh title")) + val viewModel = createViewModel() + viewModel.initTab(CommentsRsListTab.ALL) + runCurrent() // page applied; resolve job now suspended mid-flight + + viewModel.refreshTab(CommentsRsListTab.ALL, isUserRefresh = true) + advanceUntilIdle() + + // Without the cancel, the identical-ids guard defers to the hung pre-refresh job and + // the fresh title is never fetched. + val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) + assertThat(state.comments.first().postTitle).isEqualTo("Fresh title") + verify(commentsRsDataSource, times(2)).fetchPostTitles(eq(site), any()) + } + @Test fun `loadMore is a no-op when there is no next page`() = test { givenPage(listOf(rsItem(id = 1)), nextPageParams = null) From e666ee450945e52af068ecd08dc1abf274e43453 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 2 Jul 2026 16:29:33 -0400 Subject: [PATCH 08/21] Construct uniffi data classes in tests instead of mocking them Resolves the Android Lint code-scanning alerts on CommentsRsDataSourceTest: SparseAnyPostWithViewContext and PostsRequestFilterListWithViewContextResponse are pure data classes, so the tests now build real instances. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../unified/CommentsRsDataSourceTest.kt | 39 ++++++++++++++----- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt index e704d2361259..a313fa3bbe52 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt @@ -68,7 +68,7 @@ class CommentsRsDataSourceTest { perPage = params.perPage ) // The payload is decided by the request-level stub below; this value is unused. - mock() + PostsRequestFilterListWithViewContextResponse(emptyList(), mock(), null, null) } } wpApiClient.stub { @@ -201,14 +201,35 @@ class CommentsRsDataSourceTest { ) } - private fun sparsePost(id: Long, title: String): SparseAnyPostWithViewContext { - val sparseTitle = mock() - whenever(sparseTitle.rendered).thenReturn(title) - val post = mock() - whenever(post.id).thenReturn(id) - whenever(post.title).thenReturn(sparseTitle) - return post - } + // A sparse post as returned by the id+title sparse-field request: everything else null. + private fun sparsePost(id: Long, title: String) = SparseAnyPostWithViewContext( + id = id, + date = null, + dateGmt = null, + guid = null, + link = null, + modified = null, + modifiedGmt = null, + slug = null, + status = null, + postType = null, + title = SparsePostTitleWithViewContext(rendered = title), + content = null, + author = null, + excerpt = null, + featuredMedia = null, + commentStatus = null, + pingStatus = null, + format = null, + meta = null, + sticky = null, + template = null, + categories = null, + tags = null, + parent = null, + menuOrder = null, + additionalFields = null + ) private fun successResponse(vararg posts: SparseAnyPostWithViewContext) = WpRequestResult.Success( response = PostsRequestFilterListWithViewContextResponse(posts.toList(), mock(), null, null) From 89e2b61299baee0e02f0e463bef67713d235e00c Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Fri, 3 Jul 2026 08:13:25 -0400 Subject: [PATCH 09/21] Fix review findings and slim the list PR - Detail opened from the rs list backfills the FluxC cache (WP.com) and resolves the post title via rs, so title and like state are correct - Detail reports RESULT_OK only when the comment changed; the list only refreshes then, keeping paged lists and scroll position intact - Guard against duplicate first-page fetches; skip the retry snackbar when a silent refresh fails on a tab that already shows comments - Clear isLoadingMore when a refresh replaces the list so paging can't stall behind the busy guard - Stop the load-more trigger firing on the empty pre-layout list and re-arm it when the list size changes - Evict all cached post titles for the site on user refresh so renamed posts pick up their new title - Track COMMENT_FILTER_CHANGED on tab changes like the legacy list - Restore the singleTop launch mode the legacy comments activity has - Drop the dead initializingTabs bookkeeping - Merge RsCommentListItem into RsComment (one shared type and mapper) - Move the new unit tests to a follow-up PR to reduce this PR's size Co-Authored-By: Claude Fable 5 --- WordPress/src/main/AndroidManifest.xml | 1 + .../comments/unified/CommentsRsDataSource.kt | 48 +- .../unified/UnifiedCommentDetailsFragment.kt | 8 +- .../unified/UnifiedCommentDetailsViewModel.kt | 50 ++- .../ui/commentsrs/CommentsRsListActivity.kt | 12 +- .../ui/commentsrs/CommentsRsListTab.kt | 7 + .../ui/commentsrs/CommentsRsListViewModel.kt | 114 +++-- .../screens/CommentsRsListScreen.kt | 2 + .../screens/CommentsRsTabListScreen.kt | 9 +- .../unified/CommentsRsDataSourceTest.kt | 253 ----------- .../unified/CommentsRsListMappingTest.kt | 110 ----- .../UnifiedCommentDetailsViewModelTest.kt | 5 +- .../ui/commentsrs/CommentsRsListTabTest.kt | 26 -- .../commentsrs/CommentsRsListViewModelTest.kt | 414 ------------------ 14 files changed, 162 insertions(+), 897 deletions(-) delete mode 100644 WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt delete mode 100644 WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsListMappingTest.kt delete mode 100644 WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListTabTest.kt delete mode 100644 WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt diff --git a/WordPress/src/main/AndroidManifest.xml b/WordPress/src/main/AndroidManifest.xml index 712f239dc07c..6131afb24c14 100644 --- a/WordPress/src/main/AndroidManifest.xml +++ b/WordPress/src/main/AndroidManifest.xml @@ -316,6 +316,7 @@ , String>() - // Bumped by [clearUnresolvedPostTitles]: a fetch already in flight when the user cleared - // would otherwise re-poison the cache with its pre-clear "not found" results. + // Bumped by [clearPostTitles]: a fetch already in flight when the user cleared would + // otherwise re-poison the cache with its pre-clear "not found" results. @Volatile private var unresolvedCacheGeneration = 0 + /** A comment mapped from [CommentWithViewContext], shared by the detail and list screens. */ data class RsComment( - val authorName: String, - val authorAvatarUrl: String, - val dateGmt: Date, - val contentHtml: String, - val url: String, - val postId: Long, - val status: CommentStatus - ) - - /** One comment list row, mapped from [CommentWithViewContext]. */ - data class RsCommentListItem( val remoteCommentId: Long, val authorName: String, val authorAvatarUrl: String, val dateGmt: Date, val contentHtml: String, + val url: String, val postId: Long, val status: CommentStatus ) @@ -77,7 +68,7 @@ class CommentsRsDataSource @Inject constructor( /** Result of fetching one page of the comment list. */ sealed interface RsCommentsPageResult { data class Success( - val comments: List, + val comments: List, /** Ready-made params for the next page; null when this was the last page. */ val nextPageParams: CommentListParams? ) : RsCommentsPageResult @@ -106,7 +97,7 @@ class CommentsRsDataSource @Inject constructor( val client = wpApiClientProvider.getWpApiClient(site) when (val result = client.request { it.comments().listWithViewContext(params) }) { is WpRequestResult.Success -> RsCommentsPageResult.Success( - comments = result.response.data.map { it.toRsCommentListItem() }, + comments = result.response.data.map { it.toRsComment() }, nextPageParams = result.response.nextPageParams ) is WpRequestResult.WpError -> RsCommentsPageResult.Error(result.errorMessage) @@ -164,12 +155,14 @@ class CommentsRsDataSource @Inject constructor( } /** - * Forgets negative-cached (unresolvable) titles for [site] so they're retried, e.g. on a - * user-initiated refresh — a post that wasn't published when first seen may be by now. + * Forgets all cached titles for [site] so they're re-fetched, e.g. on a user-initiated + * refresh — a post that wasn't published when first seen may be by now, and a resolved + * title may have been renamed since it was cached. Fetches are batched, so re-resolving a + * page of titles costs a single request. */ - fun clearUnresolvedPostTitles(site: SiteModel) { + fun clearPostTitles(site: SiteModel) { unresolvedCacheGeneration++ - postTitleCache.entries.removeIf { it.key.first == site.id && it.value.isEmpty() } + postTitleCache.entries.removeIf { it.key.first == site.id } } /** @@ -245,30 +238,21 @@ class CommentsRsDataSource @Inject constructor( errorValue } - private fun CommentWithViewContext.toRsComment() = RsComment( - authorName = authorName, - authorAvatarUrl = pickAvatarUrl(), - // dateGmt is a UTC java.util.Date (an absolute instant), so relative-time formatting is - // correct regardless of the site's timezone — unlike the offset-less local `date` field. - dateGmt = dateGmt, - contentHtml = content.rendered, - url = link, - postId = post, - status = status.toAppCommentStatus() - ) - companion object { internal const val COMMENTS_PAGE_SIZE = 30u private const val MAX_TITLES_PER_REQUEST = 100 } } -internal fun CommentWithViewContext.toRsCommentListItem() = CommentsRsDataSource.RsCommentListItem( +internal fun CommentWithViewContext.toRsComment() = CommentsRsDataSource.RsComment( remoteCommentId = id, authorName = authorName, authorAvatarUrl = pickAvatarUrl(), + // dateGmt is a UTC java.util.Date (an absolute instant), so relative-time formatting is + // correct regardless of the site's timezone — unlike the offset-less local `date` field. dateGmt = dateGmt, contentHtml = content.rendered, + url = link, postId = post, status = status.toAppCommentStatus() ) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/UnifiedCommentDetailsFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/UnifiedCommentDetailsFragment.kt index d5870313a1ab..15e62225be12 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/UnifiedCommentDetailsFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/UnifiedCommentDetailsFragment.kt @@ -102,7 +102,7 @@ class UnifiedCommentDetailsFragment : private val editCommentLauncher: ActivityResultLauncher = registerForActivityResult(StartActivityForResult()) { result -> if (result.resultCode == AppCompatActivity.RESULT_OK) { - viewModel.refreshComment() + viewModel.onCommentEdited() } } @@ -234,6 +234,12 @@ class UnifiedCommentDetailsFragment : } viewModel.onSnackbarMessage.observeEvent(viewLifecycleOwner) { showSnackbar(it) } + + // Report the change to the launching screen (the rs comments list only refreshes when + // the result says something actually changed). + viewModel.commentChanged.observeEvent(viewLifecycleOwner) { + requireActivity().setResult(AppCompatActivity.RESULT_OK) + } } private fun clearReplyInput() { diff --git a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/UnifiedCommentDetailsViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/UnifiedCommentDetailsViewModel.kt index f1cff717dadc..22f8d361d34d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/UnifiedCommentDetailsViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/comments/unified/UnifiedCommentDetailsViewModel.kt @@ -60,11 +60,19 @@ class UnifiedCommentDetailsViewModel @Inject constructor( private val _uiState = MutableLiveData() private val _uiActionEvent = MutableLiveData>() private val _onSnackbarMessage = MutableLiveData>() + private val _commentChanged = MutableLiveData>() val uiState: LiveData = _uiState val uiActionEvent: LiveData> = _uiActionEvent val onSnackbarMessage: LiveData> = _onSnackbarMessage + /** + * Fires when the comment was changed on the server (moderated, replied to, edited or + * deleted) — NOT on like, which the list doesn't render. The host activity reports it as + * its result so the rs list only refreshes when there's something new to show. + */ + val commentChanged: LiveData> = _commentChanged + private var isStarted = false private lateinit var site: SiteModel private var remoteCommentId: Long = 0 @@ -83,10 +91,12 @@ class UnifiedCommentDetailsViewModel @Inject constructor( } /** - * Reloads the comment. Used after returning from the edit screen so any edits are reflected. + * The edit screen reported saved changes: mark the comment as changed and reload it so the + * edits are reflected. */ - fun refreshComment() { + fun onCommentEdited() { if (!isStarted) return + _commentChanged.value = Event(Unit) loadComment() } @@ -100,16 +110,32 @@ class UnifiedCommentDetailsViewModel @Inject constructor( if (!isRefresh) { _uiState.value = CommentDetailsUiState(showProgress = true) } - val (rsComment, cached) = withContext(bgDispatcher) { + val (rsComment, cached, fallbackPostTitle) = withContext(bgDispatcher) { val rs = commentsRsDataSource.getComment(site, remoteCommentId) - val local = commentsStore.getCommentByLocalSiteAndRemoteId(site.id, remoteCommentId).firstOrNull() - rs to local + var local = commentsStore.getCommentByLocalSiteAndRemoteId(site.id, remoteCommentId).firstOrNull() + // Opened from the rs list the FluxC cache may not have this comment at all (the + // legacy list guaranteed a row before the detail could open). Fetch it so the + // post title, like state and the edit screen's local id are available — WP.com + // only, since FluxC has no application-password transport. + if (local == null && site.isUsingWpComRestApi) { + commentsStore.fetchComment(site, remoteCommentId, null) + local = commentsStore.getCommentByLocalSiteAndRemoteId(site.id, remoteCommentId).firstOrNull() + } + // Still no title (self-hosted application-password site, or the fetch failed): + // resolve it the way the rs list does — usually a free hit on the shared title + // cache the list populated moments earlier. + val fallbackTitle = if (local?.postTitle.isNullOrBlank() && rs != null && rs.postId > 0) { + commentsRsDataSource.fetchPostTitles(site, listOf(rs.postId))[rs.postId].orEmpty() + } else { + "" + } + Triple(rs, local, fallbackTitle) } when { rsComment != null -> { loadedComment = rsComment localCommentId = cached?.id?.toInt() ?: 0 - _uiState.value = rsComment.toUiState(cached) + _uiState.value = rsComment.toUiState(cached, fallbackPostTitle) } isRefresh -> showSnackbar(R.string.error_load_comment) else -> _onSnackbarMessage.value = Event( @@ -205,6 +231,7 @@ class UnifiedCommentDetailsViewModel @Inject constructor( if (result is RsResult.Error) { showError(result.message, R.string.error_generic) } else { + _commentChanged.value = Event(Unit) // Replying to an unapproved comment implicitly approves it, matching legacy behaviour if (currentStatus() == UNAPPROVED) { approveAfterReply() @@ -236,8 +263,11 @@ class UnifiedCommentDetailsViewModel @Inject constructor( if (result is RsResult.Error) { _uiState.value = _uiState.value?.copy(status = previousStatus) showError(result.message, R.string.error_moderate_comment) - } else if (closeOnSuccess) { - _uiActionEvent.value = Event(Close) + } else { + _commentChanged.value = Event(Unit) + if (closeOnSuccess) { + _uiActionEvent.value = Event(Close) + } } } } @@ -284,14 +314,14 @@ class UnifiedCommentDetailsViewModel @Inject constructor( _onSnackbarMessage.value = Event(SnackbarMessageHolder(uiMessage)) } - private fun RsComment.toUiState(cached: CommentEntity?) = CommentDetailsUiState( + private fun RsComment.toUiState(cached: CommentEntity?, fallbackPostTitle: String) = CommentDetailsUiState( showProgress = false, contentVisible = true, authorName = authorName, authorAvatarUrl = authorAvatarUrl, datePublished = dateTimeUtilsWrapper.javaDateToTimeSpan(dateGmt), commentText = contentHtml, - postTitle = cached?.postTitle ?: "", + postTitle = cached?.postTitle?.takeIf { it.isNotBlank() } ?: fallbackPostTitle, commentUrl = url, status = status, isLiked = cached?.iLike ?: false diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt index f2a27e564543..971634de3e91 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt @@ -23,12 +23,15 @@ import org.wordpress.android.util.extensions.setContent class CommentsRsListActivity : BaseAppCompatActivity() { private val viewModel: CommentsRsListViewModel by viewModels() - // The detail can moderate, reply, edit or delete, and doesn't report a result — refresh - // unconditionally on return so the list reflects whatever happened there. + // The detail reports RESULT_OK when it changed the comment (moderation, reply, edit, + // delete); only then refresh, so a view-only visit keeps the paged lists and scroll + // position intact. private val detailLauncher = registerForActivityResult( ActivityResultContracts.StartActivityForResult() - ) { - viewModel.refreshAllTabs() + ) { result -> + if (result.resultCode == RESULT_OK) { + viewModel.refreshAllTabs() + } } override fun onCreate(savedInstanceState: Bundle?) { @@ -43,6 +46,7 @@ class CommentsRsListActivity : BaseAppCompatActivity() { tabStates = tabStates, snackbarMessages = viewModel.snackbarMessages, onInitTab = viewModel::initTab, + onTabChanged = viewModel::onTabChanged, onRefreshTab = { tab -> viewModel.refreshTab(tab, isUserRefresh = true) }, onLoadMore = viewModel::loadMore, onNavigateBack = { onBackPressedDispatcher.onBackPressed() }, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListTab.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListTab.kt index 66978db5abef..fa614ead0885 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListTab.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListTab.kt @@ -18,31 +18,38 @@ import uniffi.wp_api.CommentStatus as RsCommentStatus enum class CommentsRsListTab( @StringRes val labelResId: Int, @StringRes val emptyMessageResId: Int, + /** The `selected_filter` property value for COMMENT_FILTER_CHANGED, matching the legacy list. */ + @StringRes val trackingLabelResId: Int, val queryStatus: RsCommentStatus ) { ALL( R.string.comment_status_all, R.string.comments_empty_list, + R.string.comment_tracker_label_all, RsCommentStatus.Custom("all") ), PENDING( R.string.comment_status_unapproved, R.string.comments_empty_list_filtered_pending, + R.string.comment_tracker_label_pending, RsCommentStatus.Hold ), APPROVED( R.string.comment_status_approved, R.string.comments_empty_list_filtered_approved, + R.string.comment_tracker_label_approved, RsCommentStatus.Custom("approve") ), SPAM( R.string.comment_status_spam, R.string.comments_empty_list_filtered_spam, + R.string.comment_tracker_label_spam, RsCommentStatus.Spam ), TRASHED( R.string.comment_status_trash, R.string.comments_empty_list_filtered_trashed, + R.string.comment_tracker_label_trashed, RsCommentStatus.Trash ) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt index 8b436a71238d..387262fcd218 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt @@ -14,10 +14,11 @@ import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import org.wordpress.android.R +import org.wordpress.android.analytics.AnalyticsTracker.Stat import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.modules.BG_THREAD import org.wordpress.android.ui.comments.unified.CommentsRsDataSource -import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsCommentListItem +import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsComment import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsCommentsPageResult import org.wordpress.android.ui.mysite.SelectedSiteRepository import org.wordpress.android.ui.postsrs.PostRsErrorUtils @@ -26,6 +27,7 @@ import org.wordpress.android.util.DateTimeUtilsWrapper import org.wordpress.android.util.HtmlUtils import org.wordpress.android.util.NetworkUtilsWrapper import org.wordpress.android.util.WPAvatarUtilsWrapper +import org.wordpress.android.util.analytics.AnalyticsTrackerWrapper import org.wordpress.android.viewmodel.ResourceProvider import uniffi.wp_api.CommentListParams import javax.inject.Inject @@ -44,6 +46,7 @@ class CommentsRsListViewModel @Inject constructor( private val networkUtilsWrapper: NetworkUtilsWrapper, private val dateTimeUtilsWrapper: DateTimeUtilsWrapper, private val avatarUtilsWrapper: WPAvatarUtilsWrapper, + private val analyticsTracker: AnalyticsTrackerWrapper, @Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher ) : ViewModel() { private val _tabStates = MutableStateFlow>(emptyMap()) @@ -61,7 +64,11 @@ class CommentsRsListViewModel @Inject constructor( // Bumped whenever a first page is applied. A load-more result whose fetch started before the // bump paged from a cursor that predates the new page 1, so it's stale and gets discarded. private val pageGenerations = mutableMapOf() - private val initializingTabs = mutableSetOf() + + // The in-flight first-page fetch per tab, so overlapping refreshes don't duplicate it. + private val firstPageJobs = mutableMapOf() + + private var lastTrackedTab: CommentsRsListTab? = null // The in-flight title-resolve job per tab, with the ids it's resolving so an identical // request isn't cancelled and re-fired when an applied page adds nothing new. @@ -78,13 +85,14 @@ class CommentsRsListViewModel @Inject constructor( } } - /** Loads the first page for [tab] unless it's already initialized or initializing. */ + /** Loads the first page for [tab] unless it's already initialized. */ @MainThread fun initTab(tab: CommentsRsListTab) { - if (_tabStates.value.containsKey(tab) || initializingTabs.contains(tab)) return - initializingTabs.add(tab) + // updateTabUiState inserts the tab synchronously, so this also blocks re-entry while + // the first fetch is in flight. + if (_tabStates.value.containsKey(tab)) return updateTabUiState(tab) { copy(isLoading = true) } - fetchFirstPage(tab) { initializingTabs.remove(tab) } + fetchFirstPage(tab) } /** @@ -95,17 +103,21 @@ class CommentsRsListViewModel @Inject constructor( @MainThread fun refreshTab(tab: CommentsRsListTab, isUserRefresh: Boolean = false) { if (!_tabStates.value.containsKey(tab)) return + // A first page is already on its way (initial load or another refresh); a second fetch + // would only duplicate the request and double-bump the page generation. + if (firstPageJobs[tab]?.isActive == true) return if (isUserRefresh) { - // An explicit refresh also retries post titles that previously resolved as - // unresolvable — e.g. a post that has been published since it was first seen. - commentsRsDataSource.clearUnresolvedPostTitles(site) - // A resolve job that started before the clear may apply pre-clear "not found" - // titles; cancel it so the post-refresh pass fetches fresh results instead of - // deferring to it via the identical-ids guard. + // An explicit refresh retries every post title for the site: ones that previously + // resolved as unresolvable (e.g. a post published since it was first seen) and ones + // that may have gone stale (e.g. a post renamed since it was cached). + commentsRsDataSource.clearPostTitles(site) + // A resolve job that started before the clear may apply pre-clear titles; cancel it + // so the post-refresh pass fetches fresh results instead of deferring to it via the + // identical-ids guard. resolveTitleJobs[tab]?.first?.cancel() } updateTabUiState(tab) { copy(isRefreshing = isUserRefresh, error = null) } - fetchFirstPage(tab) + fetchFirstPage(tab, showErrorSnackbar = isUserRefresh) } /** Refreshes every initialized tab; comments move between tabs when moderated. */ @@ -172,15 +184,28 @@ class CommentsRsListViewModel @Inject constructor( _events.trySend(CommentsRsListEvent.OpenCommentDetail(site, remoteCommentId)) } - private fun fetchFirstPage(tab: CommentsRsListTab, onComplete: () -> Unit = {}) { - viewModelScope.launch { + /** + * Tracks the tab the pager settled on — the same COMMENT_FILTER_CHANGED event (including the + * initial selection) the legacy list emits, so filter metrics stay comparable. + */ + @MainThread + fun onTabChanged(tab: CommentsRsListTab) { + if (tab == lastTrackedTab) return + lastTrackedTab = tab + analyticsTracker.track( + Stat.COMMENT_FILTER_CHANGED, + mapOf(SELECTED_FILTER_PROPERTY to resourceProvider.getString(tab.trackingLabelResId)) + ) + } + + private fun fetchFirstPage(tab: CommentsRsListTab, showErrorSnackbar: Boolean = true) { + firstPageJobs[tab] = viewModelScope.launch { val params = commentsRsDataSource.firstPageParams(status = tab.queryStatus) val result = withContext(bgDispatcher) { commentsRsDataSource.fetchCommentsPage(site, params) } when (result) { is RsCommentsPageResult.Success -> applyPage(tab, result, append = false) - is RsCommentsPageResult.Error -> onFirstPageError(tab, result.message) + is RsCommentsPageResult.Error -> onFirstPageError(tab, result.message, showErrorSnackbar) } - onComplete() } } @@ -194,42 +219,42 @@ class CommentsRsListViewModel @Inject constructor( nextPageParams[tab] = result.nextPageParams val newRows = result.comments.map { it.toUiModel() } updateTabUiState(tab) { - if (append) { - copy( - // A comment can shift pages if the list changed server-side between - // requests, so dedupe on append. - comments = (comments + newRows).distinctBy { it.remoteCommentId }, - isLoadingMore = false, - canLoadMore = result.nextPageParams != null - ) - } else { - copy( - comments = newRows, - isLoading = false, - isRefreshing = false, - canLoadMore = result.nextPageParams != null, - error = null - ) - } + copy( + // A comment can shift pages if the list changed server-side between requests, + // so dedupe on append. + comments = if (append) (comments + newRows).distinctBy { it.remoteCommentId } else newRows, + isLoading = false, + isRefreshing = false, + // A replacing page must clear isLoadingMore too: a load-more in flight when the + // refresh landed is discarded by the generation check, so nothing else resets + // the flag promptly and paging would stay blocked behind the isBusy guard. + isLoadingMore = false, + canLoadMore = result.nextPageParams != null, + error = null + ) } resolvePostTitles(tab) } /** * First-page failure: when the tab already shows comments keep them and offer a retry - * snackbar; when it's empty, show the full-screen error state. + * snackbar; when it's empty, show the full-screen error state. Silent refreshes (returning + * from the detail) skip the snackbar — a failed background refresh shouldn't interrupt a + * user who's still looking at a perfectly good list. */ - private fun onFirstPageError(tab: CommentsRsListTab, message: String?) { + private fun onFirstPageError(tab: CommentsRsListTab, message: String?, showErrorSnackbar: Boolean) { val friendly = errorMessage(message) if (getTabUiState(tab).comments.isNotEmpty()) { updateTabUiState(tab) { copy(isLoading = false, isRefreshing = false, error = null) } - _snackbarMessages.trySend( - SnackbarMessage( - message = friendly, - actionLabel = resourceProvider.getString(R.string.retry), - onAction = { refreshTab(tab) } + if (showErrorSnackbar) { + _snackbarMessages.trySend( + SnackbarMessage( + message = friendly, + actionLabel = resourceProvider.getString(R.string.retry), + onAction = { refreshTab(tab, isUserRefresh = true) } + ) ) - ) + } } else { updateTabUiState(tab) { copy(comments = emptyList(), isLoading = false, isRefreshing = false, error = friendly) @@ -269,7 +294,7 @@ class CommentsRsListViewModel @Inject constructor( serverMessage?.takeIf { it.isNotBlank() } ?: PostRsErrorUtils.friendlyErrorMessage(null, null, resourceProvider, networkUtilsWrapper) - private fun RsCommentListItem.toUiModel() = CommentRsUiModel( + private fun RsComment.toUiModel() = CommentRsUiModel( remoteCommentId = remoteCommentId, authorName = authorName.ifBlank { resourceProvider.getString(R.string.anonymous) }, avatarUrl = avatarUtilsWrapper.rewriteAvatarUrlWithResource(authorAvatarUrl, R.dimen.avatar_sz_medium), @@ -290,5 +315,8 @@ class CommentsRsListViewModel @Inject constructor( // How many extra pages a single load-more may fetch unattended when applied pages // add no rows, before waiting for the next user scroll. private const val MAX_AUTO_ADVANCE_PAGES = 3 + + // Same property key as the legacy list's tracking (UnifiedCommentsActivity). + private const val SELECTED_FILTER_PROPERTY = "selected_filter" } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt index ce6b05bc0c28..037f0273b05f 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -41,6 +41,7 @@ fun CommentsRsListScreen( tabStates: Map, snackbarMessages: Flow = emptyFlow(), onInitTab: (CommentsRsListTab) -> Unit, + onTabChanged: (CommentsRsListTab) -> Unit, onRefreshTab: (CommentsRsListTab) -> Unit, onLoadMore: (CommentsRsListTab) -> Unit, onNavigateBack: () -> Unit, @@ -99,6 +100,7 @@ fun CommentsRsListScreen( LaunchedEffect(pagerState) { snapshotFlow { pagerState.settledPage }.collect { page -> onInitTab(tabs[page]) + onTabChanged(tabs[page]) } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt index 89f118963254..ea83ebaf4806 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt @@ -95,12 +95,17 @@ private fun CommentListContent( ) { val listState = rememberLazyListState() - LaunchedEffect(canLoadMore) { + // Also keyed on the list size: a refresh that truncates the list (or an appended page) restarts + // the flow, so a `true` latched by distinctUntilChanged before the change can't suppress the + // re-fire needed to resume paging. The ViewModel's busy/cursor guards make re-fires safe. + LaunchedEffect(canLoadMore, comments.size) { if (!canLoadMore) return@LaunchedEffect snapshotFlow { + // total > 0 keeps the pre-layout pass (empty layoutInfo, 0 >= -threshold) from + // triggering a load of the next page before the user has scrolled at all. val lastVisible = listState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 val total = listState.layoutInfo.totalItemsCount - lastVisible >= total - LOAD_MORE_THRESHOLD + total > 0 && lastVisible >= total - LOAD_MORE_THRESHOLD }.distinctUntilChanged().collect { shouldLoad -> if (shouldLoad) onLoadMore() } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt deleted file mode 100644 index a313fa3bbe52..000000000000 --- a/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsDataSourceTest.kt +++ /dev/null @@ -1,253 +0,0 @@ -package org.wordpress.android.ui.comments.unified - -import kotlinx.coroutines.test.runTest -import org.assertj.core.api.Assertions.assertThat -import org.junit.Before -import org.junit.Test -import org.mockito.kotlin.any -import org.mockito.kotlin.anyOrNull -import org.mockito.kotlin.doSuspendableAnswer -import org.mockito.kotlin.mock -import org.mockito.kotlin.stub -import org.mockito.kotlin.whenever -import org.wordpress.android.fluxc.model.SiteModel -import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider -import rs.wordpress.api.kotlin.WpApiClient -import rs.wordpress.api.kotlin.WpRequestResult -import uniffi.wp_api.PostEndpointType -import uniffi.wp_api.PostListParams -import uniffi.wp_api.PostsRequestExecutor -import uniffi.wp_api.PostsRequestFilterListWithViewContextResponse -import uniffi.wp_api.RequestMethod -import uniffi.wp_api.SparseAnyPostWithViewContext -import uniffi.wp_api.SparsePostTitleWithViewContext -import uniffi.wp_api.UniffiWpApiClient -import uniffi.wp_api.WpErrorCode - -/** - * Tests for [CommentsRsDataSource.fetchPostTitles]: the per-site title cache, the posts→pages - * endpoint fallback, negative caching of unresolvable ids, and request chunking. - * - * The [wpApiClient] stub executes each request's builder lambda against a mocked - * [UniffiWpApiClient], recording the endpoint and paging params actually sent — so the tests - * pin the requests made, not just how many there were. - */ -class CommentsRsDataSourceTest { - private data class RecordedRequest( - val endpointType: PostEndpointType, - val includeCount: Int, - val perPage: UInt? - ) - - private val wpApiClientProvider: WpApiClientProvider = mock() - private val wpApiClient: WpApiClient = mock() - private val uniffiClient: UniffiWpApiClient = mock() - private val postsExecutor: PostsRequestExecutor = mock() - private lateinit var dataSource: CommentsRsDataSource - - private val recordedRequests = mutableListOf() - private val cannedResults = ArrayDeque>() - - /** Invoked after each request completes; lets a test simulate concurrent work mid-fetch. */ - private var afterRequest: (() -> Unit)? = null - - private val siteA = SiteModel().apply { id = 1 } - private val siteB = SiteModel().apply { id = 2 } - - @Before - fun setUp() { - dataSource = CommentsRsDataSource(wpApiClientProvider) - whenever(wpApiClientProvider.getWpApiClient(any(), anyOrNull())).thenReturn(wpApiClient) - whenever(uniffiClient.posts()).thenReturn(postsExecutor) - postsExecutor.stub { - on { filterListWithViewContext(any(), any(), any()) } doSuspendableAnswer { invocation -> - val params = invocation.getArgument(1) - recordedRequests += RecordedRequest( - endpointType = invocation.getArgument(0), - includeCount = params.include.size, - perPage = params.perPage - ) - // The payload is decided by the request-level stub below; this value is unused. - PostsRequestFilterListWithViewContextResponse(emptyList(), mock(), null, null) - } - } - wpApiClient.stub { - on { request(any()) } doSuspendableAnswer { invocation -> - // Run the builder lambda so the executor stub above records what was requested. - val executor = invocation.getArgument Any>(0) - executor(uniffiClient) - val result = cannedResults.removeFirst() - afterRequest?.invoke() - result - } - } - } - - @Test - fun `resolved titles are cached and skip the network`() = runTest { - stubRequests(successResponse(sparsePost(5, "Hello"))) - - assertThat(dataSource.fetchPostTitles(siteA, listOf(5))).isEqualTo(mapOf(5L to "Hello")) - assertThat(dataSource.fetchPostTitles(siteA, listOf(5))).isEqualTo(mapOf(5L to "Hello")) - - assertThat(recordedRequests).hasSize(1) - } - - @Test - fun `titles are cached per site, not just per post id`() = runTest { - stubRequests( - successResponse(sparsePost(5, "Site A post")), - successResponse(sparsePost(5, "Site B post")) - ) - - assertThat(dataSource.fetchPostTitles(siteA, listOf(5))).isEqualTo(mapOf(5L to "Site A post")) - assertThat(dataSource.fetchPostTitles(siteB, listOf(5))).isEqualTo(mapOf(5L to "Site B post")) - - assertThat(recordedRequests).hasSize(2) - } - - @Test - fun `a posts endpoint failure still tries the pages endpoint`() = runTest { - stubRequests(wpError(), successResponse(sparsePost(7, "About"))) - - assertThat(dataSource.fetchPostTitles(siteA, listOf(7))).isEqualTo(mapOf(7L to "About")) - - assertThat(recordedRequests.map { it.endpointType }) - .containsExactly(PostEndpointType.Posts, PostEndpointType.Pages) - } - - @Test - fun `ids neither endpoint returns are negative-cached once both succeed`() = runTest { - stubRequests(successResponse(), successResponse()) - - assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "")) - assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "")) - - // Posts + pages for the first call, none for the second. - assertThat(recordedRequests.map { it.endpointType }) - .containsExactly(PostEndpointType.Posts, PostEndpointType.Pages) - } - - @Test - fun `a transient failure is not negative-cached`() = runTest { - stubRequests( - successResponse(), // posts: id not found - wpError(), // pages: transient failure — must NOT negative-cache - successResponse(sparsePost(9, "Resolved later")) - ) - - assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEmpty() - assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "Resolved later")) - - assertThat(recordedRequests.map { it.endpointType }) - .containsExactly(PostEndpointType.Posts, PostEndpointType.Pages, PostEndpointType.Posts) - } - - @Test - fun `clearUnresolvedPostTitles retries negative-cached ids`() = runTest { - stubRequests( - successResponse(), // posts: not found - successResponse(), // pages: not found → negative-cached - successResponse(sparsePost(9, "Now published")) - ) - assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "")) - - dataSource.clearUnresolvedPostTitles(siteA) - - assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "Now published")) - assertThat(recordedRequests).hasSize(3) - } - - @Test - fun `a clear during an in-flight fetch prevents negative caching`() = runTest { - stubRequests( - successResponse(), // posts: not found - successResponse(), // pages: not found — but a clear lands before the cache write - successResponse(), // retry: posts - successResponse() // retry: pages - ) - afterRequest = { - // Simulates another surface clearing while this fetch is between its network - // responses and its negative-cache write. - if (recordedRequests.size == 2) dataSource.clearUnresolvedPostTitles(siteA) - } - - assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEmpty() - - // Not negative-cached, so a later fetch goes back to the network. - afterRequest = null - assertThat(dataSource.fetchPostTitles(siteA, listOf(9))).isEqualTo(mapOf(9L to "")) - assertThat(recordedRequests).hasSize(4) - } - - @Test - fun `batches over 100 ids are chunked to the per_page maximum`() = runTest { - val ids = (1L..101L).toList() - stubRequests( - successResponse(*ids.take(100).map { sparsePost(it, "Post $it") }.toTypedArray()), - successResponse(sparsePost(101, "Post 101")) - ) - - val titles = dataSource.fetchPostTitles(siteA, ids) - - assertThat(titles).hasSize(101) - assertThat(titles[101L]).isEqualTo("Post 101") - // Two posts-endpoint requests with the batch split at 100, each with a matching - // explicit perPage — NOT one oversized request plus a pages fallback. - assertThat(recordedRequests.map { Triple(it.endpointType, it.includeCount, it.perPage) }) - .containsExactly( - Triple(PostEndpointType.Posts, 100, 100u), - Triple(PostEndpointType.Posts, 1, 1u) - ) - } - - // A sparse post as returned by the id+title sparse-field request: everything else null. - private fun sparsePost(id: Long, title: String) = SparseAnyPostWithViewContext( - id = id, - date = null, - dateGmt = null, - guid = null, - link = null, - modified = null, - modifiedGmt = null, - slug = null, - status = null, - postType = null, - title = SparsePostTitleWithViewContext(rendered = title), - content = null, - author = null, - excerpt = null, - featuredMedia = null, - commentStatus = null, - pingStatus = null, - format = null, - meta = null, - sticky = null, - template = null, - categories = null, - tags = null, - parent = null, - menuOrder = null, - additionalFields = null - ) - - private fun successResponse(vararg posts: SparseAnyPostWithViewContext) = WpRequestResult.Success( - response = PostsRequestFilterListWithViewContextResponse(posts.toList(), mock(), null, null) - ) - - private fun wpError() = WpRequestResult.WpError( - errorCode = WpErrorCode.Forbidden(), - errorMessage = "server said no", - statusCode = 403u, - response = "", - requestUrl = "https://example.com", - requestMethod = RequestMethod.GET - ) - - @Suppress("UNCHECKED_CAST") - private fun stubRequests(vararg responses: WpRequestResult<*>) { - recordedRequests.clear() - cannedResults.clear() - responses.forEach { cannedResults.add(it as WpRequestResult) } - } -} diff --git a/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsListMappingTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsListMappingTest.kt deleted file mode 100644 index a3ff1a094ffc..000000000000 --- a/WordPress/src/test/java/org/wordpress/android/ui/comments/unified/CommentsRsListMappingTest.kt +++ /dev/null @@ -1,110 +0,0 @@ -package org.wordpress.android.ui.comments.unified - -import org.assertj.core.api.Assertions.assertThat -import org.junit.Test -import org.mockito.kotlin.mock -import org.wordpress.android.fluxc.model.CommentStatus.APPROVED -import org.wordpress.android.fluxc.model.CommentStatus.UNAPPROVED -import uniffi.wp_api.CommentContentWithViewContext -import uniffi.wp_api.CommentType -import uniffi.wp_api.CommentWithViewContext -import uniffi.wp_api.UserAvatarSize -import uniffi.wp_api.WpAdditionalFields -import uniffi.wp_api.WpApiParamCommentsOrderBy -import uniffi.wp_api.WpApiParamOrder -import java.util.Date -import uniffi.wp_api.CommentStatus as RsCommentStatus - -class CommentsRsListMappingTest { - @Test - fun `toRsCommentListItem maps the fields the list rows need`() { - val item = rsComment().toRsCommentListItem() - - assertThat(item.remoteCommentId).isEqualTo(COMMENT_ID) - assertThat(item.authorName).isEqualTo("Jane") - assertThat(item.authorAvatarUrl).isEqualTo("https://example.com/avatar96.png") - assertThat(item.dateGmt).isEqualTo(DATE_GMT) - assertThat(item.contentHtml).isEqualTo("

hello

") - assertThat(item.postId).isEqualTo(POST_ID) - assertThat(item.status).isEqualTo(APPROVED) - } - - @Test - fun `toRsCommentListItem maps hold status to unapproved`() { - val item = rsComment(status = RsCommentStatus.Hold).toRsCommentListItem() - - assertThat(item.status).isEqualTo(UNAPPROVED) - } - - @Test - fun `pickAvatarUrl prefers size 96`() { - val comment = rsComment( - avatarUrls = mapOf( - UserAvatarSize.Size24 to "https://example.com/avatar24.png", - UserAvatarSize.Size96 to "https://example.com/avatar96.png" - ) - ) - - assertThat(comment.pickAvatarUrl()).isEqualTo("https://example.com/avatar96.png") - } - - @Test - fun `pickAvatarUrl falls back to the first non-empty url`() { - val comment = rsComment( - avatarUrls = mapOf( - UserAvatarSize.Size24 to "", - UserAvatarSize.Size48 to "https://example.com/avatar48.png" - ) - ) - - assertThat(comment.pickAvatarUrl()).isEqualTo("https://example.com/avatar48.png") - } - - @Test - fun `pickAvatarUrl returns empty when there are no avatar urls`() { - assertThat(rsComment(avatarUrls = emptyMap()).pickAvatarUrl()).isEmpty() - } - - @Test - fun `firstPageParams requests newest comments with the given status and search`() { - val dataSource = CommentsRsDataSource(mock()) - - val params = dataSource.firstPageParams(RsCommentStatus.Spam, search = "query") - - assertThat(params.perPage).isEqualTo(CommentsRsDataSource.COMMENTS_PAGE_SIZE) - assertThat(params.status).isEqualTo(RsCommentStatus.Spam) - assertThat(params.search).isEqualTo("query") - assertThat(params.orderby).isEqualTo(WpApiParamCommentsOrderBy.DATE_GMT) - assertThat(params.order).isEqualTo(WpApiParamOrder.DESC) - } - - private fun rsComment( - status: RsCommentStatus = RsCommentStatus.Approved, - avatarUrls: Map = mapOf( - UserAvatarSize.Size96 to "https://example.com/avatar96.png" - ) - ) = CommentWithViewContext( - id = COMMENT_ID, - author = 7L, - authorName = "Jane", - authorUrl = "https://example.com", - content = CommentContentWithViewContext(rendered = "

hello

"), - date = "2026-07-01T12:00:00", - dateGmt = DATE_GMT, - link = "https://example.com/post/#comment-42", - parent = 0L, - post = POST_ID, - status = status, - commentType = CommentType.Comment, - authorAvatarUrls = avatarUrls, - // Mocked: the real WpAdditionalFields constructor loads the uniffi native library, - // which isn't available in local unit tests. - additionalFields = mock() - ) - - companion object { - private const val COMMENT_ID = 42L - private const val POST_ID = 99L - private val DATE_GMT = Date(1_700_000_000_000) - } -} diff --git a/WordPress/src/test/java/org/wordpress/android/ui/comments/viewmodels/UnifiedCommentDetailsViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/comments/viewmodels/UnifiedCommentDetailsViewModelTest.kt index 30aa5d19eb25..9880c54ce1d1 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/comments/viewmodels/UnifiedCommentDetailsViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/comments/viewmodels/UnifiedCommentDetailsViewModelTest.kt @@ -120,7 +120,7 @@ class UnifiedCommentDetailsViewModelTest : BaseUnitTest() { viewModel.start(site, REMOTE_COMMENT_ID) whenever(commentsRsDataSource.getComment(site, REMOTE_COMMENT_ID)).thenReturn(null) - viewModel.refreshComment() + viewModel.onCommentEdited() assertThat(uiStates.last().contentVisible).isTrue assertThat(uiStates.last().status).isEqualTo(APPROVED) @@ -135,7 +135,7 @@ class UnifiedCommentDetailsViewModelTest : BaseUnitTest() { delay(LOAD_DELAY_MS) RS_COMMENT } - viewModel.refreshComment() + viewModel.onCommentEdited() viewModel.onApproveClicked() advanceUntilIdle() @@ -376,6 +376,7 @@ class UnifiedCommentDetailsViewModelTest : BaseUnitTest() { private const val LOAD_DELAY_MS = 1000L private val RS_COMMENT = RsComment( + remoteCommentId = REMOTE_COMMENT_ID, authorName = "authorName", authorAvatarUrl = "", dateGmt = Date(0), diff --git a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListTabTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListTabTest.kt deleted file mode 100644 index 023bff617408..000000000000 --- a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListTabTest.kt +++ /dev/null @@ -1,26 +0,0 @@ -package org.wordpress.android.ui.commentsrs - -import org.assertj.core.api.Assertions.assertThat -import org.junit.Test -import uniffi.wp_api.CommentStatus as RsCommentStatus - -class CommentsRsListTabTest { - @Test - fun `all tab queries status all which the server treats as approved plus hold`() { - assertThat(CommentsRsListTab.ALL.queryStatus).isEqualTo(RsCommentStatus.Custom("all")) - } - - @Test - fun `approved tab queries the literal approve status`() { - // WP_Comment_Query only recognises "approve"; the RsCommentStatus.Approved enum - // serialises to "approved", which the server treats as unknown and returns nothing for. - assertThat(CommentsRsListTab.APPROVED.queryStatus).isEqualTo(RsCommentStatus.Custom("approve")) - } - - @Test - fun `remaining tabs use the built-in statuses`() { - assertThat(CommentsRsListTab.PENDING.queryStatus).isEqualTo(RsCommentStatus.Hold) - assertThat(CommentsRsListTab.SPAM.queryStatus).isEqualTo(RsCommentStatus.Spam) - assertThat(CommentsRsListTab.TRASHED.queryStatus).isEqualTo(RsCommentStatus.Trash) - } -} diff --git a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt deleted file mode 100644 index dbc2d9fe4fa2..000000000000 --- a/WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModelTest.kt +++ /dev/null @@ -1,414 +0,0 @@ -package org.wordpress.android.ui.commentsrs - -import androidx.lifecycle.viewModelScope -import app.cash.turbine.test -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.cancel -import kotlinx.coroutines.delay -import kotlinx.coroutines.test.StandardTestDispatcher -import kotlinx.coroutines.test.runCurrent -import org.assertj.core.api.Assertions.assertThat -import org.junit.After -import org.junit.Before -import org.junit.Test -import org.mockito.Mock -import org.mockito.kotlin.any -import org.mockito.kotlin.anyOrNull -import org.mockito.kotlin.doSuspendableAnswer -import org.mockito.kotlin.eq -import org.mockito.kotlin.never -import org.mockito.kotlin.times -import org.mockito.kotlin.verify -import org.mockito.kotlin.whenever -import org.wordpress.android.BaseUnitTest -import org.wordpress.android.R -import org.wordpress.android.fluxc.model.CommentStatus.APPROVED -import org.wordpress.android.fluxc.model.SiteModel -import org.wordpress.android.ui.comments.unified.CommentsRsDataSource -import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsCommentListItem -import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsCommentsPageResult -import org.wordpress.android.ui.mysite.SelectedSiteRepository -import org.wordpress.android.util.DateTimeUtilsWrapper -import org.wordpress.android.util.NetworkUtilsWrapper -import org.wordpress.android.util.WPAvatarUtilsWrapper -import org.wordpress.android.viewmodel.ResourceProvider -import uniffi.wp_api.CommentListParams -import java.util.Date - -@ExperimentalCoroutinesApi -class CommentsRsListViewModelTest : BaseUnitTest(StandardTestDispatcher()) { - @Mock lateinit var selectedSiteRepository: SelectedSiteRepository - @Mock lateinit var commentsRsDataSource: CommentsRsDataSource - @Mock lateinit var resourceProvider: ResourceProvider - @Mock lateinit var networkUtilsWrapper: NetworkUtilsWrapper - @Mock lateinit var dateTimeUtilsWrapper: DateTimeUtilsWrapper - @Mock lateinit var avatarUtilsWrapper: WPAvatarUtilsWrapper - - private lateinit var site: SiteModel - private var activeViewModel: CommentsRsListViewModel? = null - - @Before - fun setUp() = test { - site = SiteModel().apply { - id = 1 - siteId = 123L - } - whenever(selectedSiteRepository.getSelectedSite()).thenReturn(site) - whenever(resourceProvider.getString(any())).thenReturn("string") - whenever(dateTimeUtilsWrapper.javaDateToTimeSpan(any())).thenReturn("2 hours ago") - whenever(avatarUtilsWrapper.rewriteAvatarUrlWithResource(any(), any())).thenAnswer { it.arguments[0] } - whenever(commentsRsDataSource.firstPageParams(any(), anyOrNull())).thenReturn(FIRST_PAGE) - whenever(commentsRsDataSource.fetchPostTitles(any(), any())).thenReturn(emptyMap()) - } - - @After - fun tearDown() { - activeViewModel?.viewModelScope?.cancel() - activeViewModel = null - } - - private fun createViewModel() = CommentsRsListViewModel( - selectedSiteRepository = selectedSiteRepository, - commentsRsDataSource = commentsRsDataSource, - resourceProvider = resourceProvider, - networkUtilsWrapper = networkUtilsWrapper, - dateTimeUtilsWrapper = dateTimeUtilsWrapper, - avatarUtilsWrapper = avatarUtilsWrapper, - bgDispatcher = testDispatcher() - ).also { activeViewModel = it } - - private suspend fun givenPage( - comments: List, - nextPageParams: CommentListParams? = null - ) { - whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())) - .thenReturn(RsCommentsPageResult.Success(comments, nextPageParams)) - } - - @Test - fun `when no site selected, emits ShowToast and Finish`() = test { - whenever(selectedSiteRepository.getSelectedSite()).thenReturn(null) - - val viewModel = createViewModel() - - viewModel.events.test { - val first = awaitItem() - assertThat(first).isInstanceOf(CommentsRsListEvent.ShowToast::class.java) - assertThat((first as CommentsRsListEvent.ShowToast).messageResId).isEqualTo(R.string.blog_not_found) - assertThat(awaitItem()).isEqualTo(CommentsRsListEvent.Finish) - cancelAndIgnoreRemainingEvents() - } - } - - @Test - fun `initTab loads the first page and maps rows`() = test { - givenPage(listOf(rsItem(id = 1), rsItem(id = 2)), nextPageParams = NEXT_PAGE) - val viewModel = createViewModel() - - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - - val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) - assertThat(state.isLoading).isFalse() - assertThat(state.comments).hasSize(2) - assertThat(state.comments.first().remoteCommentId).isEqualTo(1) - assertThat(state.comments.first().authorName).isEqualTo("Jane") - assertThat(state.comments.first().snippet).isEqualTo("hello") - assertThat(state.canLoadMore).isTrue() - } - - @Test - fun `initTab passes the tab's query status to the data source`() = test { - givenPage(emptyList()) - val viewModel = createViewModel() - - viewModel.initTab(CommentsRsListTab.APPROVED) - advanceUntilIdle() - - verify(commentsRsDataSource).firstPageParams(eq(CommentsRsListTab.APPROVED.queryStatus), anyOrNull()) - } - - @Test - fun `initTab is a no-op when the tab is already initialized`() = test { - givenPage(emptyList()) - val viewModel = createViewModel() - - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - - verify(commentsRsDataSource, times(1)).fetchCommentsPage(eq(site), any()) - } - - @Test - fun `initTab failure with no content shows the error state`() = test { - whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())) - .thenReturn(RsCommentsPageResult.Error("server said no")) - val viewModel = createViewModel() - - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - - val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) - assertThat(state.error).isEqualTo("server said no") - assertThat(state.isLoading).isFalse() - } - - @Test - fun `loadMore appends the next page and dedupes by comment id`() = test { - givenPage(listOf(rsItem(id = 1), rsItem(id = 2)), nextPageParams = NEXT_PAGE) - val viewModel = createViewModel() - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - givenPage(listOf(rsItem(id = 2), rsItem(id = 3)), nextPageParams = null) - - viewModel.loadMore(CommentsRsListTab.ALL) - advanceUntilIdle() - - val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) - assertThat(state.comments.map { it.remoteCommentId }).containsExactly(1L, 2L, 3L) - assertThat(state.canLoadMore).isFalse() - } - - @Test - fun `stale loadMore result arriving after a silent refresh is discarded`() = test { - givenPage(listOf(rsItem(id = 1), rsItem(id = 2)), nextPageParams = NEXT_PAGE) - val viewModel = createViewModel() - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - // Next two fetches: the silent refresh gets a fresh first page; the concurrently - // launched loadMore gets the (now stale) old second page. - whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())).thenReturn( - RsCommentsPageResult.Success(listOf(rsItem(id = 10), rsItem(id = 11)), NEXT_PAGE), - RsCommentsPageResult.Success(listOf(rsItem(id = 3), rsItem(id = 4)), null) - ) - - viewModel.refreshTab(CommentsRsListTab.ALL) // silent: sets no busy flag - viewModel.loadMore(CommentsRsListTab.ALL) - advanceUntilIdle() - - val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) - assertThat(state.comments.map { it.remoteCommentId }).containsExactly(10L, 11L) - assertThat(state.isLoadingMore).isFalse() - assertThat(state.canLoadMore).isTrue() - } - - @Test - fun `loadMore failure offers a retry snackbar`() = test { - givenPage(listOf(rsItem(id = 1)), nextPageParams = NEXT_PAGE) - val viewModel = createViewModel() - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())) - .thenReturn(RsCommentsPageResult.Error("boom")) - - viewModel.snackbarMessages.test { - viewModel.loadMore(CommentsRsListTab.ALL) - advanceUntilIdle() - - val snackbar = awaitItem() - assertThat(snackbar.message).isEqualTo("boom") - assertThat(snackbar.onAction != null).isTrue() - cancelAndIgnoreRemainingEvents() - } - assertThat(viewModel.tabStates.value.getValue(CommentsRsListTab.ALL).isLoadingMore).isFalse() - } - - @Test - fun `fully deduplicated page auto-advances to the next page`() = test { - givenPage(listOf(rsItem(id = 1), rsItem(id = 2)), nextPageParams = NEXT_PAGE) - val viewModel = createViewModel() - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - // The next page duplicates the current rows entirely (server-side shift), then the - // page after that has the genuinely new comment. - whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())).thenReturn( - RsCommentsPageResult.Success(listOf(rsItem(id = 1), rsItem(id = 2)), THIRD_PAGE), - RsCommentsPageResult.Success(listOf(rsItem(id = 3)), null) - ) - - viewModel.loadMore(CommentsRsListTab.ALL) - advanceUntilIdle() - - val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) - assertThat(state.comments.map { it.remoteCommentId }).containsExactly(1L, 2L, 3L) - assertThat(state.canLoadMore).isFalse() - } - - @Test - fun `an empty page with a remaining cursor auto-advances to the next page`() = test { - givenPage(listOf(rsItem(id = 1), rsItem(id = 2)), nextPageParams = NEXT_PAGE) - val viewModel = createViewModel() - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - // A middle page can come back empty (comments deleted server-side) while the - // headers still advertise more pages. - whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())).thenReturn( - RsCommentsPageResult.Success(emptyList(), THIRD_PAGE), - RsCommentsPageResult.Success(listOf(rsItem(id = 3)), null) - ) - - viewModel.loadMore(CommentsRsListTab.ALL) - advanceUntilIdle() - - val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) - assertThat(state.comments.map { it.remoteCommentId }).containsExactly(1L, 2L, 3L) - assertThat(state.canLoadMore).isFalse() - } - - @Test - fun `auto-advance is capped when pages keep adding nothing`() = test { - givenPage(listOf(rsItem(id = 1)), nextPageParams = NEXT_PAGE) - val viewModel = createViewModel() - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - // Pathological cursor chain: every page dedupes away with a cursor still present. - whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())) - .thenReturn(RsCommentsPageResult.Success(listOf(rsItem(id = 1)), NEXT_PAGE)) - - viewModel.loadMore(CommentsRsListTab.ALL) - advanceUntilIdle() - - // 1 init + 1 user loadMore + at most 3 auto-advances, then the chain stops. - verify(commentsRsDataSource, times(5)).fetchCommentsPage(eq(site), any()) - val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) - assertThat(state.isLoadingMore).isFalse() - } - - @Test - fun `a user refresh retries unresolvable post titles, a silent refresh does not`() = test { - givenPage(listOf(rsItem(id = 1))) - val viewModel = createViewModel() - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - - viewModel.refreshTab(CommentsRsListTab.ALL) // silent - advanceUntilIdle() - verify(commentsRsDataSource, never()).clearUnresolvedPostTitles(any()) - - viewModel.refreshTab(CommentsRsListTab.ALL, isUserRefresh = true) - advanceUntilIdle() - verify(commentsRsDataSource).clearUnresolvedPostTitles(site) - } - - @Test - fun `a user refresh cancels an in-flight title resolve and fetches fresh titles`() = test { - givenPage(listOf(rsItem(id = 1, postId = 10))) - // The first resolve hangs mid-flight (holding pre-refresh results); the retry after - // the user refresh returns the fresh title. - whenever(commentsRsDataSource.fetchPostTitles(eq(site), any())) - .doSuspendableAnswer { delay(60_000); emptyMap() } - .thenReturn(mapOf(10L to "Fresh title")) - val viewModel = createViewModel() - viewModel.initTab(CommentsRsListTab.ALL) - runCurrent() // page applied; resolve job now suspended mid-flight - - viewModel.refreshTab(CommentsRsListTab.ALL, isUserRefresh = true) - advanceUntilIdle() - - // Without the cancel, the identical-ids guard defers to the hung pre-refresh job and - // the fresh title is never fetched. - val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) - assertThat(state.comments.first().postTitle).isEqualTo("Fresh title") - verify(commentsRsDataSource, times(2)).fetchPostTitles(eq(site), any()) - } - - @Test - fun `loadMore is a no-op when there is no next page`() = test { - givenPage(listOf(rsItem(id = 1)), nextPageParams = null) - val viewModel = createViewModel() - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - - viewModel.loadMore(CommentsRsListTab.ALL) - advanceUntilIdle() - - verify(commentsRsDataSource, times(1)).fetchCommentsPage(eq(site), any()) - } - - @Test - fun `refresh failure keeps the current comments and offers retry`() = test { - givenPage(listOf(rsItem(id = 1))) - val viewModel = createViewModel() - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - whenever(commentsRsDataSource.fetchCommentsPage(eq(site), any())) - .thenReturn(RsCommentsPageResult.Error("boom")) - - viewModel.snackbarMessages.test { - viewModel.refreshTab(CommentsRsListTab.ALL, isUserRefresh = true) - advanceUntilIdle() - - val snackbar = awaitItem() - assertThat(snackbar.message).isEqualTo("boom") - assertThat(snackbar.onAction != null).isTrue() - cancelAndIgnoreRemainingEvents() - } - val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) - assertThat(state.comments).hasSize(1) - assertThat(state.error).isNull() - } - - @Test - fun `refreshAllTabs refreshes only initialized tabs`() = test { - givenPage(listOf(rsItem(id = 1))) - val viewModel = createViewModel() - viewModel.initTab(CommentsRsListTab.ALL) - viewModel.initTab(CommentsRsListTab.SPAM) - advanceUntilIdle() - - viewModel.refreshAllTabs() - advanceUntilIdle() - - // 2 init fetches + 2 refresh fetches, nothing for the 3 uninitialized tabs - verify(commentsRsDataSource, times(4)).fetchCommentsPage(eq(site), any()) - } - - @Test - fun `onCommentClick emits OpenCommentDetail`() = test { - givenPage(listOf(rsItem(id = 42))) - val viewModel = createViewModel() - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - - viewModel.events.test { - viewModel.onCommentClick(42L) - val event = awaitItem() - assertThat(event).isInstanceOf(CommentsRsListEvent.OpenCommentDetail::class.java) - assertThat((event as CommentsRsListEvent.OpenCommentDetail).remoteCommentId).isEqualTo(42L) - assertThat(event.site).isEqualTo(site) - cancelAndIgnoreRemainingEvents() - } - } - - @Test - fun `post titles are resolved in a batch and applied to rows`() = test { - givenPage(listOf(rsItem(id = 1, postId = 10), rsItem(id = 2, postId = 20))) - whenever(commentsRsDataSource.fetchPostTitles(site, listOf(10L, 20L))) - .thenReturn(mapOf(10L to "First post", 20L to "Second post")) - val viewModel = createViewModel() - - viewModel.initTab(CommentsRsListTab.ALL) - advanceUntilIdle() - - val state = viewModel.tabStates.value.getValue(CommentsRsListTab.ALL) - assertThat(state.comments.map { it.postTitle }).containsExactly("First post", "Second post") - } - - private fun rsItem(id: Long, postId: Long = 99L) = RsCommentListItem( - remoteCommentId = id, - authorName = "Jane", - authorAvatarUrl = "https://example.com/avatar.png", - dateGmt = Date(0), - contentHtml = "

hello

", - postId = postId, - status = APPROVED - ) - - companion object { - private val FIRST_PAGE = CommentListParams() - private val NEXT_PAGE = CommentListParams(page = 2u) - private val THIRD_PAGE = CommentListParams(page = 3u) - } -} From 0c828e9ff8572c7194c75964f4e474ebe16b79da Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Fri, 3 Jul 2026 09:35:35 -0400 Subject: [PATCH 10/21] Simplify review-pass leftovers - Delete the now-unused ActivityLauncher.viewUnifiedCommentsDetails - Track title-resolve jobs as a plain per-tab Job: cancel-and-relaunch is cheap because the data source caches resolved titles - Drop the dead tab parameter from onCommentClick and the unused snackbarMessages default argument Co-Authored-By: Claude Fable 5 --- .../android/ui/ActivityLauncher.java | 6 ----- .../ui/commentsrs/CommentsRsListActivity.kt | 2 +- .../ui/commentsrs/CommentsRsListViewModel.kt | 22 +++++++------------ .../screens/CommentsRsListScreen.kt | 7 +++--- 4 files changed, 12 insertions(+), 25 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java b/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java index 8cb33d9d805b..26f8801c91cf 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java @@ -46,7 +46,6 @@ import org.wordpress.android.ui.blaze.blazepromote.BlazePromoteParentActivity; import org.wordpress.android.ui.bloggingprompts.promptslist.BloggingPromptsListActivity; import org.wordpress.android.ui.comments.unified.UnifiedCommentsActivity; -import org.wordpress.android.ui.comments.unified.UnifiedCommentsDetailsActivity; import org.wordpress.android.ui.commentsrs.CommentsRsListActivity; import org.wordpress.android.ui.debug.cookies.DebugCookiesActivity; import org.wordpress.android.ui.debug.preferences.DebugSharedPreferenceFlagsActivity; @@ -769,11 +768,6 @@ private static boolean shouldUseRsCommentsList(@NonNull Context context, @NonNul return entryPoint.experimentalFeatures().isEnabled(Feature.RS_UNIFIED_COMMENTS); } - public static void viewUnifiedCommentsDetails(Context context, SiteModel site, long remoteCommentId) { - Intent intent = UnifiedCommentsDetailsActivity.createIntent(context, site, remoteCommentId); - context.startActivity(intent); - } - public static void viewCurrentBlogThemes(Context context, SiteModel site) { Intent intent = new Intent(context, ThemeBrowserActivity.class); intent.putExtra(WordPress.SITE, site); diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt index 971634de3e91..c4218f09e1ac 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt @@ -50,7 +50,7 @@ class CommentsRsListActivity : BaseAppCompatActivity() { onRefreshTab = { tab -> viewModel.refreshTab(tab, isUserRefresh = true) }, onLoadMore = viewModel::loadMore, onNavigateBack = { onBackPressedDispatcher.onBackPressed() }, - onCommentClick = { commentId, _ -> viewModel.onCommentClick(commentId) } + onCommentClick = viewModel::onCommentClick ) } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt index 387262fcd218..5a52702b0a06 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt @@ -70,9 +70,8 @@ class CommentsRsListViewModel @Inject constructor( private var lastTrackedTab: CommentsRsListTab? = null - // The in-flight title-resolve job per tab, with the ids it's resolving so an identical - // request isn't cancelled and re-fired when an applied page adds nothing new. - private val resolveTitleJobs = mutableMapOf>>() + // The in-flight title-resolve job per tab. + private val resolveTitleJobs = mutableMapOf() private val _site: SiteModel? = selectedSiteRepository.getSelectedSite() private val site: SiteModel @@ -112,9 +111,8 @@ class CommentsRsListViewModel @Inject constructor( // that may have gone stale (e.g. a post renamed since it was cached). commentsRsDataSource.clearPostTitles(site) // A resolve job that started before the clear may apply pre-clear titles; cancel it - // so the post-refresh pass fetches fresh results instead of deferring to it via the - // identical-ids guard. - resolveTitleJobs[tab]?.first?.cancel() + // so the post-refresh pass fetches fresh results. + resolveTitleJobs[tab]?.cancel() } updateTabUiState(tab) { copy(isRefreshing = isUserRefresh, error = null) } fetchFirstPage(tab, showErrorSnackbar = isUserRefresh) @@ -273,20 +271,16 @@ class CommentsRsListViewModel @Inject constructor( .distinct() if (unresolvedIds.isEmpty()) return - // An applied page that added no rows (deduped away) leaves the same ids unresolved; - // let the in-flight request finish rather than cancelling and re-firing it. - val inFlight = resolveTitleJobs[tab] - if (inFlight != null && inFlight.first.isActive && inFlight.second == unresolvedIds) return - - inFlight?.first?.cancel() - val job = viewModelScope.launch { + // Cancel-and-relaunch is cheap even when the ids haven't changed: the data source + // caches resolved titles, so a re-fired request skips them. + resolveTitleJobs[tab]?.cancel() + resolveTitleJobs[tab] = viewModelScope.launch { val titles = withContext(bgDispatcher) { commentsRsDataSource.fetchPostTitles(site, unresolvedIds) } if (titles.isEmpty()) return@launch updateTabUiState(tab) { copy(comments = comments.map { it.copy(postTitle = titles[it.postId] ?: it.postTitle) }) } } - resolveTitleJobs[tab] = job to unresolvedIds } /** The server message when it sent one, otherwise a friendly offline/generic message. */ diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt index 037f0273b05f..4e53ea7f3f96 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -28,7 +28,6 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.launch import org.wordpress.android.R import org.wordpress.android.ui.commentsrs.CommentsRsListTab @@ -39,13 +38,13 @@ import org.wordpress.android.ui.postsrs.SnackbarMessage @Composable fun CommentsRsListScreen( tabStates: Map, - snackbarMessages: Flow = emptyFlow(), + snackbarMessages: Flow, onInitTab: (CommentsRsListTab) -> Unit, onTabChanged: (CommentsRsListTab) -> Unit, onRefreshTab: (CommentsRsListTab) -> Unit, onLoadMore: (CommentsRsListTab) -> Unit, onNavigateBack: () -> Unit, - onCommentClick: (Long, CommentsRsListTab) -> Unit + onCommentClick: (Long) -> Unit ) { val tabs = CommentsRsListTab.entries val pagerState = rememberPagerState(pageCount = { tabs.size }) @@ -116,7 +115,7 @@ fun CommentsRsListScreen( emptyMessageResId = tab.emptyMessageResId, onRefresh = { onRefreshTab(tab) }, onLoadMore = { onLoadMore(tab) }, - onCommentClick = { commentId -> onCommentClick(commentId, tab) } + onCommentClick = onCommentClick ) } } From 8a944bc9011f68f874704ad61628d1cbf01720a7 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 2 Jul 2026 12:26:46 -0400 Subject: [PATCH 11/21] Add batch moderation to the rs comments list (Phase 2b) Long-press a comment to enter selection mode, then tap rows to build the selection. The top bar swaps to a contextual action bar offering the same per-tab actions as the legacy list's action mode (approve/unapprove/spam/ not-spam/trash/restore/delete), with confirmation dialogs for trash and delete-permanently and COMMENT_BATCH_* analytics. Moderation runs the rs writes in parallel, aggregates any failures into a single snackbar, then refreshes all initialized tabs, since moderated comments move between them. Selection clears on tab change and after each batch action. Co-Authored-By: Claude Fable 5 --- .../ui/commentsrs/CommentsRsListActivity.kt | 13 +- .../ui/commentsrs/CommentsRsListUiState.kt | 60 +++++++ .../ui/commentsrs/CommentsRsListViewModel.kt | 135 +++++++++++++++- .../commentsrs/screens/CommentsRsListItem.kt | 53 +++++-- .../screens/CommentsRsListScreen.kt | 147 ++++++++++++++++-- .../screens/CommentsRsTabListScreen.kt | 12 +- WordPress/src/main/res/values/strings.xml | 1 + 7 files changed, 384 insertions(+), 37 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt index c4218f09e1ac..c0383c77bd42 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt @@ -41,16 +41,27 @@ class CommentsRsListActivity : BaseAppCompatActivity() { setContent { val tabStates by viewModel.tabStates.collectAsState() + val selectedIds by viewModel.selectedIds.collectAsState() + val confirmation by viewModel.pendingConfirmation.collectAsState() AppThemeM3 { CommentsRsListScreen( tabStates = tabStates, + selectedIds = selectedIds, + confirmationDialog = ConfirmationDialogState( + pending = confirmation, + onDismiss = viewModel::onDismissPendingAction + ), snackbarMessages = viewModel.snackbarMessages, onInitTab = viewModel::initTab, onTabChanged = viewModel::onTabChanged, onRefreshTab = { tab -> viewModel.refreshTab(tab, isUserRefresh = true) }, onLoadMore = viewModel::loadMore, onNavigateBack = { onBackPressedDispatcher.onBackPressed() }, - onCommentClick = viewModel::onCommentClick + onCommentClick = viewModel::onCommentClick, + onCommentLongClick = viewModel::onCommentLongClick, + onClearSelection = viewModel::onClearSelection, + onBatchAction = viewModel::onBatchAction, + onConfirmPendingAction = viewModel::onConfirmPendingAction ) } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt index 758c456cfd56..4c9afe4ab86f 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt @@ -1,5 +1,7 @@ package org.wordpress.android.ui.commentsrs +import androidx.annotation.StringRes +import org.wordpress.android.R import org.wordpress.android.fluxc.model.CommentStatus /** One comment row. */ @@ -25,3 +27,61 @@ data class CommentsTabUiState( val canLoadMore: Boolean = false, val error: String? = null ) + +/** Destructive batch actions awaiting user confirmation. */ +sealed interface PendingConfirmation { + data class Trash(val commentIds: List) : PendingConfirmation + data class Delete(val commentIds: List) : PendingConfirmation +} + +data class ConfirmationDialogState( + val pending: PendingConfirmation? = null, + val onConfirm: () -> Unit = {}, + val onDismiss: () -> Unit = {} +) + +/** + * Batch moderation actions offered in selection mode. Each maps to a target [CommentStatus] + * applied to every selected comment (except [DELETE], which deletes permanently). + */ +enum class CommentsRsBatchAction( + @StringRes val labelResId: Int, + val targetStatus: CommentStatus, + val isDestructive: Boolean = false +) { + APPROVE(R.string.mnu_comment_approve, CommentStatus.APPROVED), + UNAPPROVE(R.string.mnu_comment_unapprove, CommentStatus.UNAPPROVED), + SPAM(R.string.mnu_comment_spam, CommentStatus.SPAM), + NOT_SPAM(R.string.mnu_comment_unspam, CommentStatus.APPROVED), + TRASH(R.string.mnu_comment_trash, CommentStatus.TRASH, isDestructive = true), + UNTRASH(R.string.mnu_comment_untrash, CommentStatus.APPROVED), + DELETE(R.string.mnu_comment_delete_permanently, CommentStatus.DELETED, isDestructive = true); +} + +/** The batch actions offered for a tab's selection, mirroring the legacy action mode. */ +internal fun CommentsRsListTab.batchActions(): List = when (this) { + CommentsRsListTab.ALL -> listOf( + CommentsRsBatchAction.APPROVE, + CommentsRsBatchAction.UNAPPROVE, + CommentsRsBatchAction.SPAM, + CommentsRsBatchAction.TRASH + ) + CommentsRsListTab.PENDING -> listOf( + CommentsRsBatchAction.APPROVE, + CommentsRsBatchAction.SPAM, + CommentsRsBatchAction.TRASH + ) + CommentsRsListTab.APPROVED -> listOf( + CommentsRsBatchAction.UNAPPROVE, + CommentsRsBatchAction.SPAM, + CommentsRsBatchAction.TRASH + ) + CommentsRsListTab.SPAM -> listOf( + CommentsRsBatchAction.NOT_SPAM, + CommentsRsBatchAction.TRASH + ) + CommentsRsListTab.TRASHED -> listOf( + CommentsRsBatchAction.UNTRASH, + CommentsRsBatchAction.DELETE + ) +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt index 5a52702b0a06..307d2da39e6e 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt @@ -6,6 +6,8 @@ import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Job +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -15,11 +17,13 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import org.wordpress.android.R import org.wordpress.android.analytics.AnalyticsTracker.Stat +import org.wordpress.android.fluxc.model.CommentStatus import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.modules.BG_THREAD import org.wordpress.android.ui.comments.unified.CommentsRsDataSource import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsComment import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsCommentsPageResult +import org.wordpress.android.ui.comments.unified.CommentsRsDataSource.RsResult import org.wordpress.android.ui.mysite.SelectedSiteRepository import org.wordpress.android.ui.postsrs.PostRsErrorUtils import org.wordpress.android.ui.postsrs.SnackbarMessage @@ -58,6 +62,13 @@ class CommentsRsListViewModel @Inject constructor( private val _snackbarMessages = Channel(Channel.BUFFERED) val snackbarMessages = _snackbarMessages.receiveAsFlow() + // Selection mode is active while this is non-empty. + private val _selectedIds = MutableStateFlow>(emptySet()) + val selectedIds: StateFlow> = _selectedIds.asStateFlow() + + private val _pendingConfirmation = MutableStateFlow(null) + val pendingConfirmation: StateFlow = _pendingConfirmation.asStateFlow() + // Pagination cursors: the next-page params returned with each fetched page, per tab. private val nextPageParams = mutableMapOf() @@ -96,8 +107,8 @@ class CommentsRsListViewModel @Inject constructor( /** * Re-fetches the first page for [tab]. The pull-to-refresh indicator is only shown for a - * user-initiated refresh; silent refreshes (returning from the detail) keep the current - * list on screen until the new page arrives. + * user-initiated refresh; silent refreshes (after moderation, returning from the detail) + * keep the current list on screen until the new page arrives. */ @MainThread fun refreshTab(tab: CommentsRsListTab, isUserRefresh: Boolean = false) { @@ -176,18 +187,130 @@ class CommentsRsListViewModel @Inject constructor( } } - /** Opens the rs comment detail for the tapped row. */ + /** Opens the rs comment detail for the tapped row, or toggles it while selecting. */ @MainThread fun onCommentClick(remoteCommentId: Long) { - _events.trySend(CommentsRsListEvent.OpenCommentDetail(site, remoteCommentId)) + if (_selectedIds.value.isNotEmpty()) { + toggleSelection(remoteCommentId) + } else { + _events.trySend(CommentsRsListEvent.OpenCommentDetail(site, remoteCommentId)) + } + } + + /** Enters selection mode with the long-pressed row selected. */ + @MainThread + fun onCommentLongClick(remoteCommentId: Long) { + toggleSelection(remoteCommentId) + } + + @MainThread + fun onClearSelection() { + _selectedIds.value = emptySet() + } + + /** + * Runs [action] on the selected comments, first asking for confirmation for the destructive + * ones (trash, delete) like the legacy list. + */ + @MainThread + fun onBatchAction(action: CommentsRsBatchAction, tab: CommentsRsListTab) { + val ids = _selectedIds.value.toList() + if (ids.isEmpty()) return + when (action) { + CommentsRsBatchAction.TRASH -> _pendingConfirmation.value = PendingConfirmation.Trash(ids) + CommentsRsBatchAction.DELETE -> _pendingConfirmation.value = PendingConfirmation.Delete(ids) + else -> performBatchModeration(ids, action.targetStatus, tab) + } + } + + @MainThread + fun onConfirmPendingAction(tab: CommentsRsListTab) { + when (val confirmation = _pendingConfirmation.value) { + is PendingConfirmation.Trash -> + performBatchModeration(confirmation.commentIds, CommentStatus.TRASH, tab) + is PendingConfirmation.Delete -> + performBatchModeration(confirmation.commentIds, CommentStatus.DELETED, tab) + null -> Unit + } + _pendingConfirmation.value = null + } + + @MainThread + fun onDismissPendingAction() { + _pendingConfirmation.value = null + } + + private fun toggleSelection(remoteCommentId: Long) { + _selectedIds.value = if (remoteCommentId in _selectedIds.value) { + _selectedIds.value - remoteCommentId + } else { + _selectedIds.value + remoteCommentId + } + } + + /** + * Applies [newStatus] to every comment in [ids] in parallel, then refreshes all initialized + * tabs (moderated comments move between tabs). Failures are aggregated into one snackbar. + */ + private fun performBatchModeration(ids: List, newStatus: CommentStatus, tab: CommentsRsListTab) { + if (!checkNetwork()) return + trackBatchModeration(newStatus) + onClearSelection() + updateTabUiState(tab) { copy(isRefreshing = true) } + viewModelScope.launch { + val results = withContext(bgDispatcher) { + ids.map { commentId -> + async { + if (newStatus == CommentStatus.DELETED) { + commentsRsDataSource.delete(site, commentId) + } else { + commentsRsDataSource.updateStatus(site, commentId, newStatus) + } + } + }.awaitAll() + } + val failures = results.count { it is RsResult.Error } + if (failures > 0) { + _snackbarMessages.trySend( + SnackbarMessage( + resourceProvider.getString(R.string.comments_rs_moderation_failed, failures, ids.size) + ) + ) + } + refreshAllTabs() + } + } + + private fun trackBatchModeration(newStatus: CommentStatus) { + val stat = when (newStatus) { + CommentStatus.APPROVED -> Stat.COMMENT_BATCH_APPROVED + CommentStatus.UNAPPROVED -> Stat.COMMENT_BATCH_UNAPPROVED + CommentStatus.SPAM -> Stat.COMMENT_BATCH_SPAMMED + CommentStatus.TRASH -> Stat.COMMENT_BATCH_TRASHED + CommentStatus.DELETED -> Stat.COMMENT_BATCH_DELETED + else -> return + } + analyticsTracker.track(stat) + } + + private fun checkNetwork(): Boolean { + if (!networkUtilsWrapper.isNetworkAvailable()) { + _snackbarMessages.trySend( + SnackbarMessage(resourceProvider.getString(R.string.no_network_message)) + ) + return false + } + return true } /** - * Tracks the tab the pager settled on — the same COMMENT_FILTER_CHANGED event (including the - * initial selection) the legacy list emits, so filter metrics stay comparable. + * The pager settled on a new tab: clears any active selection (like the legacy action mode) + * and tracks the same COMMENT_FILTER_CHANGED event (including the initial selection) the + * legacy list emits, so filter metrics stay comparable. */ @MainThread fun onTabChanged(tab: CommentsRsListTab) { + onClearSelection() if (tab == lastTrackedTab) return lastTrackedTab = tab analyticsTracker.track( diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt index 860c8ebaf760..5e7e79ee255f 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt @@ -1,7 +1,8 @@ package org.wordpress.android.ui.commentsrs.screens +import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.background -import androidx.compose.foundation.clickable +import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.IntrinsicSize @@ -14,8 +15,10 @@ import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.foundation.shape.CircleShape import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.CheckCircle import androidx.compose.material.icons.filled.Person import androidx.compose.material3.HorizontalDivider +import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -38,17 +41,26 @@ import org.wordpress.android.ui.commentsrs.CommentRsUiModel private val AVATAR_SIZE = 40.dp private val PENDING_INDICATOR_WIDTH = 4.dp +@OptIn(ExperimentalFoundationApi::class) @Composable fun CommentsRsListItem( comment: CommentRsUiModel, + isSelected: Boolean, onClick: () -> Unit, + onLongClick: () -> Unit, modifier: Modifier = Modifier ) { Column( modifier = modifier .fillMaxWidth() - .background(MaterialTheme.colorScheme.surface) - .clickable(onClick = onClick) + .background( + if (isSelected) { + MaterialTheme.colorScheme.surfaceVariant + } else { + MaterialTheme.colorScheme.surface + } + ) + .combinedClickable(onClick = onClick, onLongClick = onLongClick) ) { Row(modifier = Modifier.height(IntrinsicSize.Min)) { // The pending indicator: a colored bar on the row's leading edge, like the legacy @@ -66,7 +78,7 @@ fun CommentsRsListItem( ) ) Row(modifier = Modifier.padding(start = 12.dp, top = 16.dp, end = 16.dp, bottom = 16.dp)) { - CommentAvatar(comment = comment) + CommentAvatar(comment = comment, isSelected = isSelected) Column( modifier = Modifier .padding(start = 16.dp) @@ -102,18 +114,27 @@ fun CommentsRsListItem( } @Composable -private fun CommentAvatar(comment: CommentRsUiModel) { - val fallback = rememberVectorPainter(Icons.Filled.Person) - AsyncImage( - model = comment.avatarUrl.ifBlank { null }, - contentDescription = null, - contentScale = ContentScale.Crop, - fallback = fallback, - error = fallback, - modifier = Modifier - .size(AVATAR_SIZE) - .clip(CircleShape) - ) +private fun CommentAvatar(comment: CommentRsUiModel, isSelected: Boolean) { + if (isSelected) { + Icon( + imageVector = Icons.Filled.CheckCircle, + contentDescription = stringResource(R.string.comment_checkmark_desc), + tint = MaterialTheme.colorScheme.primary, + modifier = Modifier.size(AVATAR_SIZE) + ) + } else { + val fallback = rememberVectorPainter(Icons.Filled.Person) + AsyncImage( + model = comment.avatarUrl.ifBlank { null }, + contentDescription = null, + contentScale = ContentScale.Crop, + fallback = fallback, + error = fallback, + modifier = Modifier + .size(AVATAR_SIZE) + .clip(CircleShape) + ) + } } /** diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt index 4e53ea7f3f96..338e7ac6579a 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -1,5 +1,6 @@ package org.wordpress.android.ui.commentsrs.screens +import androidx.annotation.StringRes import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.padding @@ -7,6 +8,8 @@ import androidx.compose.foundation.pager.HorizontalPager import androidx.compose.foundation.pager.rememberPagerState import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack +import androidx.compose.material.icons.filled.Close +import androidx.compose.material3.AlertDialog import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -18,6 +21,7 @@ import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.SnackbarResult import androidx.compose.material3.Tab import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect @@ -25,31 +29,45 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.Color import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.launch import org.wordpress.android.R +import org.wordpress.android.ui.commentsrs.CommentsRsBatchAction import org.wordpress.android.ui.commentsrs.CommentsRsListTab import org.wordpress.android.ui.commentsrs.CommentsTabUiState +import org.wordpress.android.ui.commentsrs.ConfirmationDialogState +import org.wordpress.android.ui.commentsrs.PendingConfirmation +import org.wordpress.android.ui.commentsrs.batchActions import org.wordpress.android.ui.postsrs.SnackbarMessage +@Suppress("LongMethod") @OptIn(ExperimentalMaterial3Api::class) @Composable fun CommentsRsListScreen( tabStates: Map, + selectedIds: Set, + confirmationDialog: ConfirmationDialogState, snackbarMessages: Flow, onInitTab: (CommentsRsListTab) -> Unit, onTabChanged: (CommentsRsListTab) -> Unit, onRefreshTab: (CommentsRsListTab) -> Unit, onLoadMore: (CommentsRsListTab) -> Unit, onNavigateBack: () -> Unit, - onCommentClick: (Long) -> Unit + onCommentClick: (Long) -> Unit, + onCommentLongClick: (Long) -> Unit, + onClearSelection: () -> Unit, + onBatchAction: (CommentsRsBatchAction, CommentsRsListTab) -> Unit, + onConfirmPendingAction: (CommentsRsListTab) -> Unit ) { val tabs = CommentsRsListTab.entries val pagerState = rememberPagerState(pageCount = { tabs.size }) val coroutineScope = rememberCoroutineScope() + val activeTab = tabs[pagerState.settledPage] val snackbarHostState = remember { SnackbarHostState() } + val isSelectionActive = selectedIds.isNotEmpty() LaunchedEffect(snackbarMessages) { snackbarMessages.collect { msg -> @@ -66,17 +84,26 @@ fun CommentsRsListScreen( Scaffold( snackbarHost = { SnackbarHost(snackbarHostState) }, topBar = { - TopAppBar( - title = { Text(text = stringResource(R.string.comments)) }, - navigationIcon = { - IconButton(onClick = onNavigateBack) { - Icon( - Icons.AutoMirrored.Filled.ArrowBack, - contentDescription = stringResource(R.string.back) - ) + if (isSelectionActive) { + SelectionTopBar( + selectedCount = selectedIds.size, + actions = activeTab.batchActions(), + onClearSelection = onClearSelection, + onBatchAction = { action -> onBatchAction(action, activeTab) } + ) + } else { + TopAppBar( + title = { Text(text = stringResource(R.string.comments)) }, + navigationIcon = { + IconButton(onClick = onNavigateBack) { + Icon( + Icons.AutoMirrored.Filled.ArrowBack, + contentDescription = stringResource(R.string.back) + ) + } } - } - ) + ) + } } ) { contentPadding -> Column(modifier = Modifier.fillMaxSize().padding(contentPadding)) { @@ -113,11 +140,107 @@ fun CommentsRsListScreen( CommentsRsTabListScreen( state = tabState, emptyMessageResId = tab.emptyMessageResId, + selectedIds = selectedIds, onRefresh = { onRefreshTab(tab) }, onLoadMore = { onLoadMore(tab) }, - onCommentClick = onCommentClick + onCommentClick = onCommentClick, + onCommentLongClick = onCommentLongClick ) } } } + + when (val pending = confirmationDialog.pending) { + is PendingConfirmation.Trash -> ConfirmationDialog( + titleResId = R.string.trash, + message = stringResource(R.string.dlg_confirm_trash_comments), + confirmTextResId = R.string.dlg_confirm_action_trash, + isDestructive = true, + onConfirm = { onConfirmPendingAction(activeTab) }, + onDismiss = confirmationDialog.onDismiss + ) + is PendingConfirmation.Delete -> ConfirmationDialog( + titleResId = R.string.delete, + message = stringResource( + if (pending.commentIds.size > 1) { + R.string.dlg_sure_to_delete_comments + } else { + R.string.dlg_sure_to_delete_comment + } + ), + confirmTextResId = R.string.delete, + isDestructive = true, + onConfirm = { onConfirmPendingAction(activeTab) }, + onDismiss = confirmationDialog.onDismiss + ) + null -> {} + } +} + +@OptIn(ExperimentalMaterial3Api::class) +@Composable +private fun SelectionTopBar( + selectedCount: Int, + actions: List, + onClearSelection: () -> Unit, + onBatchAction: (CommentsRsBatchAction) -> Unit +) { + TopAppBar( + title = { Text(text = stringResource(R.string.cab_selected, selectedCount)) }, + navigationIcon = { + IconButton(onClick = onClearSelection) { + Icon( + Icons.Default.Close, + contentDescription = stringResource(R.string.clear) + ) + } + }, + actions = { + actions.forEach { action -> + TextButton(onClick = { onBatchAction(action) }) { + Text( + text = stringResource(action.labelResId), + color = if (action.isDestructive) { + MaterialTheme.colorScheme.error + } else { + MaterialTheme.colorScheme.primary + } + ) + } + } + } + ) +} + +@Composable +private fun ConfirmationDialog( + @StringRes titleResId: Int, + message: String, + @StringRes confirmTextResId: Int, + isDestructive: Boolean = false, + onConfirm: () -> Unit, + onDismiss: () -> Unit +) { + AlertDialog( + onDismissRequest = onDismiss, + title = { Text(stringResource(titleResId)) }, + text = { Text(message) }, + confirmButton = { + TextButton(onClick = onConfirm) { + Text( + stringResource(confirmTextResId), + color = if (isDestructive) { + MaterialTheme.colorScheme.error + } else { + Color.Unspecified + } + ) + } + }, + dismissButton = { + TextButton(onClick = onDismiss) { + Text(stringResource(R.string.cancel)) + } + } + ) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt index ea83ebaf4806..da87afc71425 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt @@ -44,9 +44,11 @@ import org.wordpress.android.ui.compose.components.ShimmerBox fun CommentsRsTabListScreen( state: CommentsTabUiState, emptyMessageResId: Int, + selectedIds: Set, onRefresh: () -> Unit, onLoadMore: () -> Unit, onCommentClick: (Long) -> Unit, + onCommentLongClick: (Long) -> Unit, modifier: Modifier = Modifier ) { val pullToRefreshState = rememberPullToRefreshState() @@ -76,10 +78,12 @@ fun CommentsRsTabListScreen( ) else -> CommentListContent( comments = state.comments, + selectedIds = selectedIds, isLoadingMore = state.isLoadingMore, canLoadMore = state.canLoadMore, onLoadMore = onLoadMore, - onCommentClick = onCommentClick + onCommentClick = onCommentClick, + onCommentLongClick = onCommentLongClick ) } } @@ -88,10 +92,12 @@ fun CommentsRsTabListScreen( @Composable private fun CommentListContent( comments: List, + selectedIds: Set, isLoadingMore: Boolean, canLoadMore: Boolean, onLoadMore: () -> Unit, - onCommentClick: (Long) -> Unit + onCommentClick: (Long) -> Unit, + onCommentLongClick: (Long) -> Unit ) { val listState = rememberLazyListState() @@ -121,7 +127,9 @@ private fun CommentListContent( ) { comment -> CommentsRsListItem( comment = comment, + isSelected = comment.remoteCommentId in selectedIds, onClick = { onCommentClick(comment.remoteCommentId) }, + onLongClick = { onCommentLongClick(comment.remoteCommentId) }, modifier = Modifier.animateItem() ) } diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index c200d82bd0a2..e9e1ea696a59 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -434,6 +434,7 @@ No unreplied comments No spam comments No trashed comments + %1$d of %2$d comments couldn\'t be updated Reply to %s Comment trashed Comment marked as spam From 63510493815c81f0585e00531a8184776f3c084c Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Fri, 3 Jul 2026 15:09:22 -0400 Subject: [PATCH 12/21] Remove unused onConfirm field from ConfirmationDialogState The confirm button is wired directly to onConfirmPendingAction in the screen, so this field was never set or read. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt index 4c9afe4ab86f..16fadfac2bb8 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt @@ -36,7 +36,6 @@ sealed interface PendingConfirmation { data class ConfirmationDialogState( val pending: PendingConfirmation? = null, - val onConfirm: () -> Unit = {}, val onDismiss: () -> Unit = {} ) From a1fa82b8cd264bec835c92d9dedbf1ae8c99db7c Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Fri, 3 Jul 2026 15:30:01 -0400 Subject: [PATCH 13/21] Simplify batch-moderation dialog wiring - Drop the single-use ConfirmationDialogState wrapper; pass pending confirmation and dismiss callback directly to the screen - Extract the confirmation dialogs into a private BatchConfirmationDialogs composable, removing the LongMethod suppression - Remove a redundant trailing semicolon in CommentsRsBatchAction Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/commentsrs/CommentsRsListActivity.kt | 6 ++-- .../ui/commentsrs/CommentsRsListUiState.kt | 7 +---- .../screens/CommentsRsListScreen.kt | 30 ++++++++++++++----- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt index c0383c77bd42..21f143d13c28 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt @@ -47,10 +47,8 @@ class CommentsRsListActivity : BaseAppCompatActivity() { CommentsRsListScreen( tabStates = tabStates, selectedIds = selectedIds, - confirmationDialog = ConfirmationDialogState( - pending = confirmation, - onDismiss = viewModel::onDismissPendingAction - ), + pendingConfirmation = confirmation, + onDismissConfirmation = viewModel::onDismissPendingAction, snackbarMessages = viewModel.snackbarMessages, onInitTab = viewModel::initTab, onTabChanged = viewModel::onTabChanged, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt index 16fadfac2bb8..f7d1434a7a3e 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt @@ -34,11 +34,6 @@ sealed interface PendingConfirmation { data class Delete(val commentIds: List) : PendingConfirmation } -data class ConfirmationDialogState( - val pending: PendingConfirmation? = null, - val onDismiss: () -> Unit = {} -) - /** * Batch moderation actions offered in selection mode. Each maps to a target [CommentStatus] * applied to every selected comment (except [DELETE], which deletes permanently). @@ -54,7 +49,7 @@ enum class CommentsRsBatchAction( NOT_SPAM(R.string.mnu_comment_unspam, CommentStatus.APPROVED), TRASH(R.string.mnu_comment_trash, CommentStatus.TRASH, isDestructive = true), UNTRASH(R.string.mnu_comment_untrash, CommentStatus.APPROVED), - DELETE(R.string.mnu_comment_delete_permanently, CommentStatus.DELETED, isDestructive = true); + DELETE(R.string.mnu_comment_delete_permanently, CommentStatus.DELETED, isDestructive = true) } /** The batch actions offered for a tab's selection, mirroring the legacy action mode. */ diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt index 338e7ac6579a..9108c3a79de7 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -38,18 +38,17 @@ import org.wordpress.android.R import org.wordpress.android.ui.commentsrs.CommentsRsBatchAction import org.wordpress.android.ui.commentsrs.CommentsRsListTab import org.wordpress.android.ui.commentsrs.CommentsTabUiState -import org.wordpress.android.ui.commentsrs.ConfirmationDialogState import org.wordpress.android.ui.commentsrs.PendingConfirmation import org.wordpress.android.ui.commentsrs.batchActions import org.wordpress.android.ui.postsrs.SnackbarMessage -@Suppress("LongMethod") @OptIn(ExperimentalMaterial3Api::class) @Composable fun CommentsRsListScreen( tabStates: Map, selectedIds: Set, - confirmationDialog: ConfirmationDialogState, + pendingConfirmation: PendingConfirmation?, + onDismissConfirmation: () -> Unit, snackbarMessages: Flow, onInitTab: (CommentsRsListTab) -> Unit, onTabChanged: (CommentsRsListTab) -> Unit, @@ -150,14 +149,29 @@ fun CommentsRsListScreen( } } - when (val pending = confirmationDialog.pending) { + BatchConfirmationDialogs( + pending = pendingConfirmation, + activeTab = activeTab, + onConfirm = onConfirmPendingAction, + onDismiss = onDismissConfirmation + ) +} + +@Composable +private fun BatchConfirmationDialogs( + pending: PendingConfirmation?, + activeTab: CommentsRsListTab, + onConfirm: (CommentsRsListTab) -> Unit, + onDismiss: () -> Unit +) { + when (pending) { is PendingConfirmation.Trash -> ConfirmationDialog( titleResId = R.string.trash, message = stringResource(R.string.dlg_confirm_trash_comments), confirmTextResId = R.string.dlg_confirm_action_trash, isDestructive = true, - onConfirm = { onConfirmPendingAction(activeTab) }, - onDismiss = confirmationDialog.onDismiss + onConfirm = { onConfirm(activeTab) }, + onDismiss = onDismiss ) is PendingConfirmation.Delete -> ConfirmationDialog( titleResId = R.string.delete, @@ -170,8 +184,8 @@ fun CommentsRsListScreen( ), confirmTextResId = R.string.delete, isDestructive = true, - onConfirm = { onConfirmPendingAction(activeTab) }, - onDismiss = confirmationDialog.onDismiss + onConfirm = { onConfirm(activeTab) }, + onDismiss = onDismiss ) null -> {} } From 92f4ea75b46d2f38a6fff4ce1b1670ed5e8247b1 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Mon, 6 Jul 2026 08:01:50 -0400 Subject: [PATCH 14/21] Use icon actions in batch-moderation action bar Text labels ("Approve", etc.) crowded the contextual action bar and wrapped the "N selected" title. Render each batch action as an icon button reusing the legacy comments list's action-mode drawables (thumbs up/down, spam, trash), keeping the string label as the icon's content description for accessibility. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/commentsrs/CommentsRsListUiState.kt | 27 ++++++++++++------- .../screens/CommentsRsListScreen.kt | 12 +++++---- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt index f7d1434a7a3e..815b70b1a64b 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt @@ -1,5 +1,6 @@ package org.wordpress.android.ui.commentsrs +import androidx.annotation.DrawableRes import androidx.annotation.StringRes import org.wordpress.android.R import org.wordpress.android.fluxc.model.CommentStatus @@ -35,21 +36,29 @@ sealed interface PendingConfirmation { } /** - * Batch moderation actions offered in selection mode. Each maps to a target [CommentStatus] - * applied to every selected comment (except [DELETE], which deletes permanently). + * Batch moderation actions offered in selection mode, rendered as icon buttons in the contextual + * action bar (mirroring the legacy list's action mode). Each maps to a target [CommentStatus] + * applied to every selected comment (except [DELETE], which deletes permanently); [labelResId] + * doubles as the icon's content description. */ enum class CommentsRsBatchAction( @StringRes val labelResId: Int, + @DrawableRes val iconResId: Int, val targetStatus: CommentStatus, val isDestructive: Boolean = false ) { - APPROVE(R.string.mnu_comment_approve, CommentStatus.APPROVED), - UNAPPROVE(R.string.mnu_comment_unapprove, CommentStatus.UNAPPROVED), - SPAM(R.string.mnu_comment_spam, CommentStatus.SPAM), - NOT_SPAM(R.string.mnu_comment_unspam, CommentStatus.APPROVED), - TRASH(R.string.mnu_comment_trash, CommentStatus.TRASH, isDestructive = true), - UNTRASH(R.string.mnu_comment_untrash, CommentStatus.APPROVED), - DELETE(R.string.mnu_comment_delete_permanently, CommentStatus.DELETED, isDestructive = true) + APPROVE(R.string.mnu_comment_approve, R.drawable.ic_thumbs_up_white_24dp, CommentStatus.APPROVED), + UNAPPROVE(R.string.mnu_comment_unapprove, R.drawable.ic_thumbs_down_white_24dp, CommentStatus.UNAPPROVED), + SPAM(R.string.mnu_comment_spam, R.drawable.ic_spam_white_24dp, CommentStatus.SPAM), + NOT_SPAM(R.string.mnu_comment_unspam, R.drawable.ic_spam_white_24dp, CommentStatus.APPROVED), + TRASH(R.string.mnu_comment_trash, R.drawable.ic_trash_white_24dp, CommentStatus.TRASH, isDestructive = true), + UNTRASH(R.string.mnu_comment_untrash, R.drawable.ic_trash_white_24dp, CommentStatus.APPROVED), + DELETE( + R.string.mnu_comment_delete_permanently, + R.drawable.ic_trash_white_24dp, + CommentStatus.DELETED, + isDestructive = true + ) } /** The batch actions offered for a tab's selection, mirroring the legacy action mode. */ diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt index 9108c3a79de7..9f5f0cbf0f69 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -30,6 +30,7 @@ import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color +import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import kotlinx.coroutines.flow.Flow @@ -211,13 +212,14 @@ private fun SelectionTopBar( }, actions = { actions.forEach { action -> - TextButton(onClick = { onBatchAction(action) }) { - Text( - text = stringResource(action.labelResId), - color = if (action.isDestructive) { + IconButton(onClick = { onBatchAction(action) }) { + Icon( + painter = painterResource(action.iconResId), + contentDescription = stringResource(action.labelResId), + tint = if (action.isDestructive) { MaterialTheme.colorScheme.error } else { - MaterialTheme.colorScheme.primary + MaterialTheme.colorScheme.onSurfaceVariant } ) } From d8011ed8d7a67321b8f9c823280442b7a093c7ed Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Mon, 6 Jul 2026 09:07:37 -0400 Subject: [PATCH 15/21] Show only approve/unapprove as action-bar icons Match the legacy comment list's action mode: keep approve/unapprove as always-visible icon buttons and move the remaining actions (spam, not-spam, trash, restore, delete) into an overflow menu, so the "N selected" title no longer wraps. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/commentsrs/CommentsRsListUiState.kt | 24 ++++++--- .../screens/CommentsRsListScreen.kt | 54 ++++++++++++++++--- 2 files changed, 66 insertions(+), 12 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt index 815b70b1a64b..1d903aa01820 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt @@ -36,19 +36,31 @@ sealed interface PendingConfirmation { } /** - * Batch moderation actions offered in selection mode, rendered as icon buttons in the contextual - * action bar (mirroring the legacy list's action mode). Each maps to a target [CommentStatus] + * Batch moderation actions offered in selection mode. Each maps to a target [CommentStatus] * applied to every selected comment (except [DELETE], which deletes permanently); [labelResId] - * doubles as the icon's content description. + * doubles as the icon's content description. Mirroring the legacy list's action mode, only the + * [showAsIcon] actions (approve/unapprove) appear as top-bar icon buttons; the rest fall into the + * overflow menu. */ enum class CommentsRsBatchAction( @StringRes val labelResId: Int, @DrawableRes val iconResId: Int, val targetStatus: CommentStatus, - val isDestructive: Boolean = false + val isDestructive: Boolean = false, + val showAsIcon: Boolean = false ) { - APPROVE(R.string.mnu_comment_approve, R.drawable.ic_thumbs_up_white_24dp, CommentStatus.APPROVED), - UNAPPROVE(R.string.mnu_comment_unapprove, R.drawable.ic_thumbs_down_white_24dp, CommentStatus.UNAPPROVED), + APPROVE( + R.string.mnu_comment_approve, + R.drawable.ic_thumbs_up_white_24dp, + CommentStatus.APPROVED, + showAsIcon = true + ), + UNAPPROVE( + R.string.mnu_comment_unapprove, + R.drawable.ic_thumbs_down_white_24dp, + CommentStatus.UNAPPROVED, + showAsIcon = true + ), SPAM(R.string.mnu_comment_spam, R.drawable.ic_spam_white_24dp, CommentStatus.SPAM), NOT_SPAM(R.string.mnu_comment_unspam, R.drawable.ic_spam_white_24dp, CommentStatus.APPROVED), TRASH(R.string.mnu_comment_trash, R.drawable.ic_trash_white_24dp, CommentStatus.TRASH, isDestructive = true), diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt index 9f5f0cbf0f69..f5ac776fff9c 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -9,7 +9,10 @@ import androidx.compose.foundation.pager.rememberPagerState import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material.icons.filled.Close +import androidx.compose.material.icons.filled.MoreVert import androidx.compose.material3.AlertDialog +import androidx.compose.material3.DropdownMenu +import androidx.compose.material3.DropdownMenuItem import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -25,8 +28,11 @@ import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.setValue import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color @@ -200,6 +206,9 @@ private fun SelectionTopBar( onClearSelection: () -> Unit, onBatchAction: (CommentsRsBatchAction) -> Unit ) { + // Like the legacy action mode, approve/unapprove sit in the bar as icons and everything else + // falls into the overflow menu, keeping the selection count on a single line. + val (iconActions, menuActions) = actions.partition { it.showAsIcon } TopAppBar( title = { Text(text = stringResource(R.string.cab_selected, selectedCount)) }, navigationIcon = { @@ -211,23 +220,56 @@ private fun SelectionTopBar( } }, actions = { - actions.forEach { action -> + iconActions.forEach { action -> IconButton(onClick = { onBatchAction(action) }) { Icon( painter = painterResource(action.iconResId), contentDescription = stringResource(action.labelResId), - tint = if (action.isDestructive) { - MaterialTheme.colorScheme.error - } else { - MaterialTheme.colorScheme.onSurfaceVariant - } + tint = MaterialTheme.colorScheme.onSurfaceVariant ) } } + if (menuActions.isNotEmpty()) { + BatchActionsOverflowMenu(actions = menuActions, onBatchAction = onBatchAction) + } } ) } +@Composable +private fun BatchActionsOverflowMenu( + actions: List, + onBatchAction: (CommentsRsBatchAction) -> Unit +) { + var expanded by remember { mutableStateOf(false) } + IconButton(onClick = { expanded = true }) { + Icon(Icons.Default.MoreVert, contentDescription = stringResource(R.string.more)) + } + DropdownMenu(expanded = expanded, onDismissRequest = { expanded = false }) { + actions.forEach { action -> + val color = if (action.isDestructive) { + MaterialTheme.colorScheme.error + } else { + MaterialTheme.colorScheme.onSurface + } + DropdownMenuItem( + text = { Text(text = stringResource(action.labelResId), color = color) }, + leadingIcon = { + Icon( + painter = painterResource(action.iconResId), + contentDescription = null, + tint = color + ) + }, + onClick = { + expanded = false + onBatchAction(action) + } + ) + } + } +} + @Composable private fun ConfirmationDialog( @StringRes titleResId: Int, From 6d30cab455fe69a7d46ad1e5d2b96726cd8beb3d Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Mon, 6 Jul 2026 10:05:05 -0400 Subject: [PATCH 16/21] Simplify pending-confirmation wiring Collapse the PendingConfirmation sealed interface into a single data class that carries the CommentsRsBatchAction, so onBatchAction routes on isDestructive and onConfirmPendingAction reuses action.targetStatus instead of re-mapping the confirmation type back to a CommentStatus. Also use MutableStateFlow.update in toggleSelection. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/commentsrs/CommentsRsListUiState.kt | 7 ++---- .../ui/commentsrs/CommentsRsListViewModel.kt | 23 +++++++------------ .../screens/CommentsRsListScreen.kt | 9 ++++---- 3 files changed, 15 insertions(+), 24 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt index 1d903aa01820..a3c546076a46 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt @@ -29,11 +29,8 @@ data class CommentsTabUiState( val error: String? = null ) -/** Destructive batch actions awaiting user confirmation. */ -sealed interface PendingConfirmation { - data class Trash(val commentIds: List) : PendingConfirmation - data class Delete(val commentIds: List) : PendingConfirmation -} +/** A destructive batch [action] awaiting user confirmation, to be applied to [commentIds]. */ +data class PendingConfirmation(val action: CommentsRsBatchAction, val commentIds: List) /** * Batch moderation actions offered in selection mode. Each maps to a target [CommentStatus] diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt index 307d2da39e6e..b404672d5512 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt @@ -13,6 +13,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.receiveAsFlow +import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import org.wordpress.android.R @@ -216,22 +217,16 @@ class CommentsRsListViewModel @Inject constructor( fun onBatchAction(action: CommentsRsBatchAction, tab: CommentsRsListTab) { val ids = _selectedIds.value.toList() if (ids.isEmpty()) return - when (action) { - CommentsRsBatchAction.TRASH -> _pendingConfirmation.value = PendingConfirmation.Trash(ids) - CommentsRsBatchAction.DELETE -> _pendingConfirmation.value = PendingConfirmation.Delete(ids) - else -> performBatchModeration(ids, action.targetStatus, tab) + if (action.isDestructive) { + _pendingConfirmation.value = PendingConfirmation(action, ids) + } else { + performBatchModeration(ids, action.targetStatus, tab) } } @MainThread fun onConfirmPendingAction(tab: CommentsRsListTab) { - when (val confirmation = _pendingConfirmation.value) { - is PendingConfirmation.Trash -> - performBatchModeration(confirmation.commentIds, CommentStatus.TRASH, tab) - is PendingConfirmation.Delete -> - performBatchModeration(confirmation.commentIds, CommentStatus.DELETED, tab) - null -> Unit - } + _pendingConfirmation.value?.let { performBatchModeration(it.commentIds, it.action.targetStatus, tab) } _pendingConfirmation.value = null } @@ -241,10 +236,8 @@ class CommentsRsListViewModel @Inject constructor( } private fun toggleSelection(remoteCommentId: Long) { - _selectedIds.value = if (remoteCommentId in _selectedIds.value) { - _selectedIds.value - remoteCommentId - } else { - _selectedIds.value + remoteCommentId + _selectedIds.update { ids -> + if (remoteCommentId in ids) ids - remoteCommentId else ids + remoteCommentId } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt index f5ac776fff9c..8a4a1d1e3e2c 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -171,8 +171,9 @@ private fun BatchConfirmationDialogs( onConfirm: (CommentsRsListTab) -> Unit, onDismiss: () -> Unit ) { - when (pending) { - is PendingConfirmation.Trash -> ConfirmationDialog( + if (pending == null) return + when (pending.action) { + CommentsRsBatchAction.TRASH -> ConfirmationDialog( titleResId = R.string.trash, message = stringResource(R.string.dlg_confirm_trash_comments), confirmTextResId = R.string.dlg_confirm_action_trash, @@ -180,7 +181,7 @@ private fun BatchConfirmationDialogs( onConfirm = { onConfirm(activeTab) }, onDismiss = onDismiss ) - is PendingConfirmation.Delete -> ConfirmationDialog( + CommentsRsBatchAction.DELETE -> ConfirmationDialog( titleResId = R.string.delete, message = stringResource( if (pending.commentIds.size > 1) { @@ -194,7 +195,7 @@ private fun BatchConfirmationDialogs( onConfirm = { onConfirm(activeTab) }, onDismiss = onDismiss ) - null -> {} + else -> {} } } From ecaf07d601f01a863129c24eaa539f37cc8ffcd2 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Mon, 6 Jul 2026 10:28:53 -0400 Subject: [PATCH 17/21] Disable approve/unapprove when they'd be a no-op Approve and unapprove stay visible in the batch action bar but are disabled (dimmed) unless the selection actually contains a comment they can act on: approve needs an unapproved comment, unapprove needs an approved one. The active tab's selected statuses drive the enablement. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/commentsrs/CommentsRsListUiState.kt | 12 ++++++++++ .../screens/CommentsRsListScreen.kt | 22 +++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt index a3c546076a46..df370b550e15 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt @@ -97,3 +97,15 @@ internal fun CommentsRsListTab.batchActions(): List = whe CommentsRsBatchAction.DELETE ) } + +/** + * Whether [this] action can act on a selection whose comments have [selectedStatuses]. Approve and + * unapprove are disabled when they would be a no-op — nothing unapproved to approve, or nothing + * approved to unapprove; every other action is always enabled. + */ +internal fun CommentsRsBatchAction.isEnabledFor(selectedStatuses: Set): Boolean = + when (this) { + CommentsRsBatchAction.APPROVE -> CommentStatus.UNAPPROVED in selectedStatuses + CommentsRsBatchAction.UNAPPROVE -> CommentStatus.APPROVED in selectedStatuses + else -> true + } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt index 8a4a1d1e3e2c..8f9c2d1ae2d8 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -42,13 +42,19 @@ import androidx.compose.ui.unit.dp import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.launch import org.wordpress.android.R +import org.wordpress.android.fluxc.model.CommentStatus import org.wordpress.android.ui.commentsrs.CommentsRsBatchAction import org.wordpress.android.ui.commentsrs.CommentsRsListTab import org.wordpress.android.ui.commentsrs.CommentsTabUiState import org.wordpress.android.ui.commentsrs.PendingConfirmation import org.wordpress.android.ui.commentsrs.batchActions +import org.wordpress.android.ui.commentsrs.isEnabledFor import org.wordpress.android.ui.postsrs.SnackbarMessage +// Material's disabled-content alpha, used to dim batch-action icons that can't apply to the +// current selection while keeping them visible. +private const val DISABLED_ICON_ALPHA = 0.38f + @OptIn(ExperimentalMaterial3Api::class) @Composable fun CommentsRsListScreen( @@ -91,9 +97,15 @@ fun CommentsRsListScreen( snackbarHost = { SnackbarHost(snackbarHostState) }, topBar = { if (isSelectionActive) { + val selectedStatuses = tabStates[activeTab]?.comments + .orEmpty() + .filter { it.remoteCommentId in selectedIds } + .map { it.status } + .toSet() SelectionTopBar( selectedCount = selectedIds.size, actions = activeTab.batchActions(), + selectedStatuses = selectedStatuses, onClearSelection = onClearSelection, onBatchAction = { action -> onBatchAction(action, activeTab) } ) @@ -204,6 +216,7 @@ private fun BatchConfirmationDialogs( private fun SelectionTopBar( selectedCount: Int, actions: List, + selectedStatuses: Set, onClearSelection: () -> Unit, onBatchAction: (CommentsRsBatchAction) -> Unit ) { @@ -222,11 +235,16 @@ private fun SelectionTopBar( }, actions = { iconActions.forEach { action -> - IconButton(onClick = { onBatchAction(action) }) { + // An action that would be a no-op for the selection (e.g. approve when nothing is + // unapproved) stays visible but disabled. + val enabled = action.isEnabledFor(selectedStatuses) + IconButton(onClick = { onBatchAction(action) }, enabled = enabled) { Icon( painter = painterResource(action.iconResId), contentDescription = stringResource(action.labelResId), - tint = MaterialTheme.colorScheme.onSurfaceVariant + tint = MaterialTheme.colorScheme.onSurfaceVariant.copy( + alpha = if (enabled) 1f else DISABLED_ICON_ALPHA + ) ) } } From b28df1e7c23684564396e16d1d35c1fe015281e7 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Mon, 6 Jul 2026 12:37:07 -0400 Subject: [PATCH 18/21] Fix action-bar flicker and make confirmations data-driven Gate the contextual action bar on the active tab actually containing selected comments, so it no longer briefly flashes the next tab's actions (with approve/unapprove dimmed) mid-swipe before the selection clears. Drive the confirmation dialog off ConfirmationCopy carried by each destructive action instead of a non-exhaustive when: onBatchAction now routes to confirmation only when the action has copy, so a destructive action can no longer strand the selection with no dialog. This also replaces the isDestructive flag and drops the duplicated trash/delete dialog branches. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/commentsrs/CommentsRsListUiState.kt | 36 ++++++++++-- .../ui/commentsrs/CommentsRsListViewModel.kt | 2 +- .../screens/CommentsRsListScreen.kt | 55 ++++++++----------- 3 files changed, 54 insertions(+), 39 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt index df370b550e15..bcbab8fd4dce 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt @@ -32,19 +32,31 @@ data class CommentsTabUiState( /** A destructive batch [action] awaiting user confirmation, to be applied to [commentIds]. */ data class PendingConfirmation(val action: CommentsRsBatchAction, val commentIds: List) +/** + * Confirmation-dialog copy for a destructive batch action. [messagePluralResId] is shown when more + * than one comment is selected, otherwise [messageResId]. + */ +data class ConfirmationCopy( + @StringRes val titleResId: Int, + @StringRes val confirmButtonResId: Int, + @StringRes val messageResId: Int, + @StringRes val messagePluralResId: Int = messageResId +) + /** * Batch moderation actions offered in selection mode. Each maps to a target [CommentStatus] * applied to every selected comment (except [DELETE], which deletes permanently); [labelResId] * doubles as the icon's content description. Mirroring the legacy list's action mode, only the * [showAsIcon] actions (approve/unapprove) appear as top-bar icon buttons; the rest fall into the - * overflow menu. + * overflow menu. An action with [confirmation] copy is destructive: it asks before running and is + * tinted accordingly. */ enum class CommentsRsBatchAction( @StringRes val labelResId: Int, @DrawableRes val iconResId: Int, val targetStatus: CommentStatus, - val isDestructive: Boolean = false, - val showAsIcon: Boolean = false + val showAsIcon: Boolean = false, + val confirmation: ConfirmationCopy? = null ) { APPROVE( R.string.mnu_comment_approve, @@ -60,13 +72,27 @@ enum class CommentsRsBatchAction( ), SPAM(R.string.mnu_comment_spam, R.drawable.ic_spam_white_24dp, CommentStatus.SPAM), NOT_SPAM(R.string.mnu_comment_unspam, R.drawable.ic_spam_white_24dp, CommentStatus.APPROVED), - TRASH(R.string.mnu_comment_trash, R.drawable.ic_trash_white_24dp, CommentStatus.TRASH, isDestructive = true), + TRASH( + R.string.mnu_comment_trash, + R.drawable.ic_trash_white_24dp, + CommentStatus.TRASH, + confirmation = ConfirmationCopy( + titleResId = R.string.trash, + confirmButtonResId = R.string.dlg_confirm_action_trash, + messageResId = R.string.dlg_confirm_trash_comments + ) + ), UNTRASH(R.string.mnu_comment_untrash, R.drawable.ic_trash_white_24dp, CommentStatus.APPROVED), DELETE( R.string.mnu_comment_delete_permanently, R.drawable.ic_trash_white_24dp, CommentStatus.DELETED, - isDestructive = true + confirmation = ConfirmationCopy( + titleResId = R.string.delete, + confirmButtonResId = R.string.delete, + messageResId = R.string.dlg_sure_to_delete_comment, + messagePluralResId = R.string.dlg_sure_to_delete_comments + ) ) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt index b404672d5512..e8ee606a9dac 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt @@ -217,7 +217,7 @@ class CommentsRsListViewModel @Inject constructor( fun onBatchAction(action: CommentsRsBatchAction, tab: CommentsRsListTab) { val ids = _selectedIds.value.toList() if (ids.isEmpty()) return - if (action.isDestructive) { + if (action.confirmation != null) { _pendingConfirmation.value = PendingConfirmation(action, ids) } else { performBatchModeration(ids, action.targetStatus, tab) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt index 8f9c2d1ae2d8..cbabbb4dae8a 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -79,7 +79,15 @@ fun CommentsRsListScreen( val coroutineScope = rememberCoroutineScope() val activeTab = tabs[pagerState.settledPage] val snackbarHostState = remember { SnackbarHostState() } - val isSelectionActive = selectedIds.isNotEmpty() + // Statuses of the selected comments that live on the active tab. This is empty during a tab + // swipe (the selection still belongs to the previous tab and is about to be cleared), so gating + // the contextual bar on it keeps it from flashing the next tab's actions mid-transition. + val selectedStatuses = tabStates[activeTab]?.comments + .orEmpty() + .filter { it.remoteCommentId in selectedIds } + .map { it.status } + .toSet() + val isSelectionActive = selectedStatuses.isNotEmpty() LaunchedEffect(snackbarMessages) { snackbarMessages.collect { msg -> @@ -97,11 +105,6 @@ fun CommentsRsListScreen( snackbarHost = { SnackbarHost(snackbarHostState) }, topBar = { if (isSelectionActive) { - val selectedStatuses = tabStates[activeTab]?.comments - .orEmpty() - .filter { it.remoteCommentId in selectedIds } - .map { it.status } - .toSet() SelectionTopBar( selectedCount = selectedIds.size, actions = activeTab.batchActions(), @@ -183,32 +186,18 @@ private fun BatchConfirmationDialogs( onConfirm: (CommentsRsListTab) -> Unit, onDismiss: () -> Unit ) { - if (pending == null) return - when (pending.action) { - CommentsRsBatchAction.TRASH -> ConfirmationDialog( - titleResId = R.string.trash, - message = stringResource(R.string.dlg_confirm_trash_comments), - confirmTextResId = R.string.dlg_confirm_action_trash, - isDestructive = true, - onConfirm = { onConfirm(activeTab) }, - onDismiss = onDismiss - ) - CommentsRsBatchAction.DELETE -> ConfirmationDialog( - titleResId = R.string.delete, - message = stringResource( - if (pending.commentIds.size > 1) { - R.string.dlg_sure_to_delete_comments - } else { - R.string.dlg_sure_to_delete_comment - } - ), - confirmTextResId = R.string.delete, - isDestructive = true, - onConfirm = { onConfirm(activeTab) }, - onDismiss = onDismiss - ) - else -> {} - } + // Every action that reaches confirmation carries its copy (see onBatchAction), so any + // destructive action renders a dialog rather than silently stranding the selection. + val copy = pending?.action?.confirmation ?: return + val messageResId = if (pending.commentIds.size > 1) copy.messagePluralResId else copy.messageResId + ConfirmationDialog( + titleResId = copy.titleResId, + message = stringResource(messageResId), + confirmTextResId = copy.confirmButtonResId, + isDestructive = true, + onConfirm = { onConfirm(activeTab) }, + onDismiss = onDismiss + ) } @OptIn(ExperimentalMaterial3Api::class) @@ -266,7 +255,7 @@ private fun BatchActionsOverflowMenu( } DropdownMenu(expanded = expanded, onDismissRequest = { expanded = false }) { actions.forEach { action -> - val color = if (action.isDestructive) { + val color = if (action.confirmation != null) { MaterialTheme.colorScheme.error } else { MaterialTheme.colorScheme.onSurface From 8cd59ae3a31d5f71ee7110101671dfdc8089c662 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Mon, 6 Jul 2026 14:04:51 -0400 Subject: [PATCH 19/21] Add quick-win UX polish to the rs comments list - Animate the top-bar swap between the normal bar and the selection CAB - Make the avatar tappable to toggle selection, the discoverable path into selection mode (long-press still works) - Add Select All to the CAB overflow menu - Re-tapping the active tab scrolls its list back to the top (per-tab LazyListStates hoisted to the screen) Long-press haptics were considered but need no code: Compose Foundation 1.11.4's combinedClickable already performs them by default. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/commentsrs/CommentsRsListActivity.kt | 1 + .../ui/commentsrs/CommentsRsListViewModel.kt | 6 ++ .../commentsrs/screens/CommentsRsListItem.kt | 13 +++- .../screens/CommentsRsListScreen.kt | 77 ++++++++++++++----- .../screens/CommentsRsTabListScreen.kt | 7 +- 5 files changed, 77 insertions(+), 27 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt index 21f143d13c28..8dc8f7ef0f37 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt @@ -58,6 +58,7 @@ class CommentsRsListActivity : BaseAppCompatActivity() { onCommentClick = viewModel::onCommentClick, onCommentLongClick = viewModel::onCommentLongClick, onClearSelection = viewModel::onClearSelection, + onSelectAll = viewModel::onSelectAll, onBatchAction = viewModel::onBatchAction, onConfirmPendingAction = viewModel::onConfirmPendingAction ) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt index e8ee606a9dac..8488a1a99286 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt @@ -209,6 +209,12 @@ class CommentsRsListViewModel @Inject constructor( _selectedIds.value = emptySet() } + /** Selects every loaded comment on [tab]. */ + @MainThread + fun onSelectAll(tab: CommentsRsListTab) { + _selectedIds.value = getTabUiState(tab).comments.map { it.remoteCommentId }.toSet() + } + /** * Runs [action] on the selected comments, first asking for confirmation for the destructive * ones (trash, delete) like the legacy list. diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt index 5e7e79ee255f..16d826719c29 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt @@ -2,6 +2,7 @@ package org.wordpress.android.ui.commentsrs.screens import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.background +import androidx.compose.foundation.clickable import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -78,7 +79,9 @@ fun CommentsRsListItem( ) ) Row(modifier = Modifier.padding(start = 12.dp, top = 16.dp, end = 16.dp, bottom = 16.dp)) { - CommentAvatar(comment = comment, isSelected = isSelected) + // Tapping the avatar toggles selection (the discoverable path into selection + // mode); long-press anywhere on the row does the same. + CommentAvatar(comment = comment, isSelected = isSelected, onClick = onLongClick) Column( modifier = Modifier .padding(start = 16.dp) @@ -114,13 +117,16 @@ fun CommentsRsListItem( } @Composable -private fun CommentAvatar(comment: CommentRsUiModel, isSelected: Boolean) { +private fun CommentAvatar(comment: CommentRsUiModel, isSelected: Boolean, onClick: () -> Unit) { if (isSelected) { Icon( imageVector = Icons.Filled.CheckCircle, contentDescription = stringResource(R.string.comment_checkmark_desc), tint = MaterialTheme.colorScheme.primary, - modifier = Modifier.size(AVATAR_SIZE) + modifier = Modifier + .size(AVATAR_SIZE) + .clip(CircleShape) + .clickable(onClick = onClick) ) } else { val fallback = rememberVectorPainter(Icons.Filled.Person) @@ -133,6 +139,7 @@ private fun CommentAvatar(comment: CommentRsUiModel, isSelected: Boolean) { modifier = Modifier .size(AVATAR_SIZE) .clip(CircleShape) + .clickable(onClick = onClick) ) } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt index cbabbb4dae8a..e9fe5bbb11b0 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -1,18 +1,22 @@ package org.wordpress.android.ui.commentsrs.screens import androidx.annotation.StringRes +import androidx.compose.animation.AnimatedContent import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.pager.HorizontalPager import androidx.compose.foundation.pager.rememberPagerState import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material.icons.filled.Close +import androidx.compose.material.icons.filled.DoneAll import androidx.compose.material.icons.filled.MoreVert import androidx.compose.material3.AlertDialog import androidx.compose.material3.DropdownMenu import androidx.compose.material3.DropdownMenuItem +import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -71,12 +75,15 @@ fun CommentsRsListScreen( onCommentClick: (Long) -> Unit, onCommentLongClick: (Long) -> Unit, onClearSelection: () -> Unit, + onSelectAll: (CommentsRsListTab) -> Unit, onBatchAction: (CommentsRsBatchAction, CommentsRsListTab) -> Unit, onConfirmPendingAction: (CommentsRsListTab) -> Unit ) { val tabs = CommentsRsListTab.entries val pagerState = rememberPagerState(pageCount = { tabs.size }) val coroutineScope = rememberCoroutineScope() + // Hoisted so a re-tap on the active tab can scroll its list back to the top. + val listStates = remember { tabs.associateWith { LazyListState() } } val activeTab = tabs[pagerState.settledPage] val snackbarHostState = remember { SnackbarHostState() } // Statuses of the selected comments that live on the active tab. This is empty during a tab @@ -104,26 +111,29 @@ fun CommentsRsListScreen( Scaffold( snackbarHost = { SnackbarHost(snackbarHostState) }, topBar = { - if (isSelectionActive) { - SelectionTopBar( - selectedCount = selectedIds.size, - actions = activeTab.batchActions(), - selectedStatuses = selectedStatuses, - onClearSelection = onClearSelection, - onBatchAction = { action -> onBatchAction(action, activeTab) } - ) - } else { - TopAppBar( - title = { Text(text = stringResource(R.string.comments)) }, - navigationIcon = { - IconButton(onClick = onNavigateBack) { - Icon( - Icons.AutoMirrored.Filled.ArrowBack, - contentDescription = stringResource(R.string.back) - ) + AnimatedContent(targetState = isSelectionActive, label = "topBar") { selectionActive -> + if (selectionActive) { + SelectionTopBar( + selectedCount = selectedIds.size, + actions = activeTab.batchActions(), + selectedStatuses = selectedStatuses, + onClearSelection = onClearSelection, + onSelectAll = { onSelectAll(activeTab) }, + onBatchAction = { action -> onBatchAction(action, activeTab) } + ) + } else { + TopAppBar( + title = { Text(text = stringResource(R.string.comments)) }, + navigationIcon = { + IconButton(onClick = onNavigateBack) { + Icon( + Icons.AutoMirrored.Filled.ArrowBack, + contentDescription = stringResource(R.string.back) + ) + } } - } - ) + ) + } } } ) { contentPadding -> @@ -136,7 +146,14 @@ fun CommentsRsListScreen( Tab( selected = pagerState.settledPage == index, onClick = { - coroutineScope.launch { pagerState.animateScrollToPage(index) } + coroutineScope.launch { + if (pagerState.settledPage == index) { + // Re-tapping the active tab scrolls its list back to the top. + listStates.getValue(tab).animateScrollToItem(0) + } else { + pagerState.animateScrollToPage(index) + } + } }, unselectedContentColor = MaterialTheme.colorScheme.onSurfaceVariant, text = { Text(text = stringResource(tab.labelResId)) } @@ -162,6 +179,7 @@ fun CommentsRsListScreen( state = tabState, emptyMessageResId = tab.emptyMessageResId, selectedIds = selectedIds, + listState = listStates.getValue(tab), onRefresh = { onRefreshTab(tab) }, onLoadMore = { onLoadMore(tab) }, onCommentClick = onCommentClick, @@ -207,6 +225,7 @@ private fun SelectionTopBar( actions: List, selectedStatuses: Set, onClearSelection: () -> Unit, + onSelectAll: () -> Unit, onBatchAction: (CommentsRsBatchAction) -> Unit ) { // Like the legacy action mode, approve/unapprove sit in the bar as icons and everything else @@ -238,7 +257,11 @@ private fun SelectionTopBar( } } if (menuActions.isNotEmpty()) { - BatchActionsOverflowMenu(actions = menuActions, onBatchAction = onBatchAction) + BatchActionsOverflowMenu( + actions = menuActions, + onSelectAll = onSelectAll, + onBatchAction = onBatchAction + ) } } ) @@ -247,6 +270,7 @@ private fun SelectionTopBar( @Composable private fun BatchActionsOverflowMenu( actions: List, + onSelectAll: () -> Unit, onBatchAction: (CommentsRsBatchAction) -> Unit ) { var expanded by remember { mutableStateOf(false) } @@ -254,6 +278,17 @@ private fun BatchActionsOverflowMenu( Icon(Icons.Default.MoreVert, contentDescription = stringResource(R.string.more)) } DropdownMenu(expanded = expanded, onDismissRequest = { expanded = false }) { + DropdownMenuItem( + text = { Text(text = stringResource(R.string.select_all)) }, + leadingIcon = { + Icon(Icons.Default.DoneAll, contentDescription = null) + }, + onClick = { + expanded = false + onSelectAll() + } + ) + HorizontalDivider() actions.forEach { action -> val color = if (action.confirmation != null) { MaterialTheme.colorScheme.error diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt index da87afc71425..ee6a4f11c95d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt @@ -11,7 +11,7 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items -import androidx.compose.foundation.lazy.rememberLazyListState +import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.shape.RoundedCornerShape @@ -45,6 +45,7 @@ fun CommentsRsTabListScreen( state: CommentsTabUiState, emptyMessageResId: Int, selectedIds: Set, + listState: LazyListState, onRefresh: () -> Unit, onLoadMore: () -> Unit, onCommentClick: (Long) -> Unit, @@ -79,6 +80,7 @@ fun CommentsRsTabListScreen( else -> CommentListContent( comments = state.comments, selectedIds = selectedIds, + listState = listState, isLoadingMore = state.isLoadingMore, canLoadMore = state.canLoadMore, onLoadMore = onLoadMore, @@ -93,14 +95,13 @@ fun CommentsRsTabListScreen( private fun CommentListContent( comments: List, selectedIds: Set, + listState: LazyListState, isLoadingMore: Boolean, canLoadMore: Boolean, onLoadMore: () -> Unit, onCommentClick: (Long) -> Unit, onCommentLongClick: (Long) -> Unit ) { - val listState = rememberLazyListState() - // Also keyed on the list size: a refresh that truncates the list (or an appended page) restarts // the flow, so a `true` latched by distinctUntilChanged before the change can't suppress the // re-fire needed to resume paging. The ViewModel's busy/cursor guards make re-fires safe. From dd447939a712e386b1c5cb4829202e52df98a742 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Mon, 6 Jul 2026 14:31:57 -0400 Subject: [PATCH 20/21] Remove Select All from the selection overflow On a paginated list "Select all" only selects the comments loaded so far, which reads as selecting everything on the tab. Drop it rather than ship a misleading affordance. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/commentsrs/CommentsRsListActivity.kt | 1 - .../ui/commentsrs/CommentsRsListViewModel.kt | 6 ----- .../screens/CommentsRsListScreen.kt | 23 +------------------ 3 files changed, 1 insertion(+), 29 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt index 8dc8f7ef0f37..21f143d13c28 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt @@ -58,7 +58,6 @@ class CommentsRsListActivity : BaseAppCompatActivity() { onCommentClick = viewModel::onCommentClick, onCommentLongClick = viewModel::onCommentLongClick, onClearSelection = viewModel::onClearSelection, - onSelectAll = viewModel::onSelectAll, onBatchAction = viewModel::onBatchAction, onConfirmPendingAction = viewModel::onConfirmPendingAction ) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt index 8488a1a99286..e8ee606a9dac 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt @@ -209,12 +209,6 @@ class CommentsRsListViewModel @Inject constructor( _selectedIds.value = emptySet() } - /** Selects every loaded comment on [tab]. */ - @MainThread - fun onSelectAll(tab: CommentsRsListTab) { - _selectedIds.value = getTabUiState(tab).comments.map { it.remoteCommentId }.toSet() - } - /** * Runs [action] on the selected comments, first asking for confirmation for the destructive * ones (trash, delete) like the legacy list. diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt index e9fe5bbb11b0..b3205d63dc49 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -11,12 +11,10 @@ import androidx.compose.foundation.pager.rememberPagerState import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material.icons.filled.Close -import androidx.compose.material.icons.filled.DoneAll import androidx.compose.material.icons.filled.MoreVert import androidx.compose.material3.AlertDialog import androidx.compose.material3.DropdownMenu import androidx.compose.material3.DropdownMenuItem -import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -75,7 +73,6 @@ fun CommentsRsListScreen( onCommentClick: (Long) -> Unit, onCommentLongClick: (Long) -> Unit, onClearSelection: () -> Unit, - onSelectAll: (CommentsRsListTab) -> Unit, onBatchAction: (CommentsRsBatchAction, CommentsRsListTab) -> Unit, onConfirmPendingAction: (CommentsRsListTab) -> Unit ) { @@ -118,7 +115,6 @@ fun CommentsRsListScreen( actions = activeTab.batchActions(), selectedStatuses = selectedStatuses, onClearSelection = onClearSelection, - onSelectAll = { onSelectAll(activeTab) }, onBatchAction = { action -> onBatchAction(action, activeTab) } ) } else { @@ -225,7 +221,6 @@ private fun SelectionTopBar( actions: List, selectedStatuses: Set, onClearSelection: () -> Unit, - onSelectAll: () -> Unit, onBatchAction: (CommentsRsBatchAction) -> Unit ) { // Like the legacy action mode, approve/unapprove sit in the bar as icons and everything else @@ -257,11 +252,7 @@ private fun SelectionTopBar( } } if (menuActions.isNotEmpty()) { - BatchActionsOverflowMenu( - actions = menuActions, - onSelectAll = onSelectAll, - onBatchAction = onBatchAction - ) + BatchActionsOverflowMenu(actions = menuActions, onBatchAction = onBatchAction) } } ) @@ -270,7 +261,6 @@ private fun SelectionTopBar( @Composable private fun BatchActionsOverflowMenu( actions: List, - onSelectAll: () -> Unit, onBatchAction: (CommentsRsBatchAction) -> Unit ) { var expanded by remember { mutableStateOf(false) } @@ -278,17 +268,6 @@ private fun BatchActionsOverflowMenu( Icon(Icons.Default.MoreVert, contentDescription = stringResource(R.string.more)) } DropdownMenu(expanded = expanded, onDismissRequest = { expanded = false }) { - DropdownMenuItem( - text = { Text(text = stringResource(R.string.select_all)) }, - leadingIcon = { - Icon(Icons.Default.DoneAll, contentDescription = null) - }, - onClick = { - expanded = false - onSelectAll() - } - ) - HorizontalDivider() actions.forEach { action -> val color = if (action.confirmation != null) { MaterialTheme.colorScheme.error From 640989416728673e81190c472f0a59b99b9300e1 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Mon, 6 Jul 2026 14:40:50 -0400 Subject: [PATCH 21/21] Polish selection mode: back press, avatar crossfade, bar color - Back now dismisses the selection instead of leaving the screen, matching the legacy action mode - The avatar crossfades to the checkmark when a row is selected - The selection bar uses surfaceContainerHigh so entering selection mode visibly reads as a mode change Co-Authored-By: Claude Opus 4.8 (1M context) --- .../commentsrs/screens/CommentsRsListItem.kt | 49 ++++++++++--------- .../screens/CommentsRsListScreen.kt | 13 +++++ 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt index 16d826719c29..d97f42a47455 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt @@ -1,5 +1,6 @@ package org.wordpress.android.ui.commentsrs.screens +import androidx.compose.animation.Crossfade import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.background import androidx.compose.foundation.clickable @@ -118,29 +119,31 @@ fun CommentsRsListItem( @Composable private fun CommentAvatar(comment: CommentRsUiModel, isSelected: Boolean, onClick: () -> Unit) { - if (isSelected) { - Icon( - imageVector = Icons.Filled.CheckCircle, - contentDescription = stringResource(R.string.comment_checkmark_desc), - tint = MaterialTheme.colorScheme.primary, - modifier = Modifier - .size(AVATAR_SIZE) - .clip(CircleShape) - .clickable(onClick = onClick) - ) - } else { - val fallback = rememberVectorPainter(Icons.Filled.Person) - AsyncImage( - model = comment.avatarUrl.ifBlank { null }, - contentDescription = null, - contentScale = ContentScale.Crop, - fallback = fallback, - error = fallback, - modifier = Modifier - .size(AVATAR_SIZE) - .clip(CircleShape) - .clickable(onClick = onClick) - ) + Crossfade(targetState = isSelected, label = "avatar") { selected -> + if (selected) { + Icon( + imageVector = Icons.Filled.CheckCircle, + contentDescription = stringResource(R.string.comment_checkmark_desc), + tint = MaterialTheme.colorScheme.primary, + modifier = Modifier + .size(AVATAR_SIZE) + .clip(CircleShape) + .clickable(onClick = onClick) + ) + } else { + val fallback = rememberVectorPainter(Icons.Filled.Person) + AsyncImage( + model = comment.avatarUrl.ifBlank { null }, + contentDescription = null, + contentScale = ContentScale.Crop, + fallback = fallback, + error = fallback, + modifier = Modifier + .size(AVATAR_SIZE) + .clip(CircleShape) + .clickable(onClick = onClick) + ) + } } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt index b3205d63dc49..4a03ad446604 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt @@ -1,5 +1,6 @@ package org.wordpress.android.ui.commentsrs.screens +import androidx.activity.compose.BackHandler import androidx.annotation.StringRes import androidx.compose.animation.AnimatedContent import androidx.compose.foundation.layout.Column @@ -28,6 +29,7 @@ import androidx.compose.material3.Tab import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar +import androidx.compose.material3.TopAppBarDefaults import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue @@ -93,6 +95,13 @@ fun CommentsRsListScreen( .toSet() val isSelectionActive = selectedStatuses.isNotEmpty() + // Like the legacy action mode: the first back press dismisses the selection, the next one + // leaves the screen. Enabled on selectedIds (not selectedStatuses) so any live selection is + // cleared, including during the brief tab-swipe window before it clears itself. + BackHandler(enabled = selectedIds.isNotEmpty()) { + onClearSelection() + } + LaunchedEffect(snackbarMessages) { snackbarMessages.collect { msg -> val result = snackbarHostState.showSnackbar( @@ -227,6 +236,10 @@ private fun SelectionTopBar( // falls into the overflow menu, keeping the selection count on a single line. val (iconActions, menuActions) = actions.partition { it.showAsIcon } TopAppBar( + // A distinct container color so selection mode visibly reads as a mode change. + colors = TopAppBarDefaults.topAppBarColors( + containerColor = MaterialTheme.colorScheme.surfaceContainerHigh + ), title = { Text(text = stringResource(R.string.cab_selected, selectedCount)) }, navigationIcon = { IconButton(onClick = onClearSelection) {