Skip to content

Commit a390930

Browse files
nbradburyclaude
andauthored
RS Posts: Route mutations through PostService and improve UX (#22645)
* RS Posts: Show move-to-draft dialog when tapping a trashed post Tapping a trashed post in the RS post list now shows a confirmation dialog instead of opening the editor directly. On confirmation the post is moved to draft status before the editor opens, matching the behavior of the old post list. The active tab is passed through the click callback chain rather than the post status, since the status field may not be populated when the user taps. A post on the TRASHED tab is always trashed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Reuse executePostAction in moveToDraftAndEdit and add loading indicator Refactor moveToDraftAndEdit to use the shared executePostAction helper with new onSuccess/onError callbacks instead of duplicating the pattern. Show pull-to-refresh spinner on the Trashed tab during the network call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Show toast when FluxC post not found after move-to-draft If getFluxCPost returns null after successfully moving a trashed post to draft, show a "Post moved to drafts" toast instead of failing silently. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Route post mutations through PostService instead of WpApiClient PostService (from wordpress-rs PR #1180) automatically updates PostMetadataCollectionWithEditContext when posts are mutated, so the list UI updates locally without needing optimistic removal or a full tab refresh. - Update wordpress-rs to trunk-a557060 which includes PR #1180 - Replace restClient.trashPost/deletePost/updatePostStatus with postService.trashPost/deletePostPermanently/updatePost - Rewrite moveToDraftAndEdit to use PostService directly - Remove executePostAction, handleActionResult, removePostFromState, and refreshAllTabs from the ViewModel - Remove mutation methods and PostActionResult from PostRsRestClient Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Add better error messaging to post mutations Adds getErrorMessage() helper that checks network availability and parses WpApiException messages to provide more specific error feedback to users instead of generic error messages. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Refactor post mutation methods to reduce duplication Consolidates duplicate boilerplate across trash, delete, publish, and move-to-draft operations by extracting shared logic into helper functions. Changes: - Add executePostMutation() helper for network check, error handling - Add postStatusUpdate() helper for PostUpdateParams construction - Unify friendlyErrorMessage() to handle both generic and API errors - Remove explicit PostService type (use type inference) Reduces code by ~85 lines with no behavioral changes. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Removed unused import * Re-throw CancellationException in post mutation catch blocks Prevents coroutine cancellation from being swallowed by the generic Exception catch in executePostMutation and moveToDraftAndEdit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Consolidate duplicate friendlyErrorMessage overloads The single-param version was a strict subset of the two-param version with defaultResId=null. All call sites resolve correctly to the remaining overload. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add KDoc to undocumented private helpers in PostRsListViewModel Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix test compilation errors after wordpress-rs library update Add missing requestUrl and requestMethod parameters to WpRequestResult error constructors (UnknownError and WpError) in test files. These parameters became required after the wordpress-rs library update in commit 234bd8b. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Load cached items before refresh and guard loadMore during sync Show cached/stale data immediately when initializing a tab by calling loadItemsForTab before refreshTab, matching the recommended pattern from the collection developer guide. Also skip loadNextPage when a refresh is already in progress to prevent concurrent sync operations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix FluxC test compilation errors after wordpress-rs library update Add missing requestUrl and requestMethod parameters to WpRequestResult.UnknownError constructors in FluxC test files (MediaRsApiRestClientTest and TaxonomyRsApiRestClientTest). These parameters became required after the wordpress-rs library update. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix line length violations in MediaRsApiRestClientTest Break long WpRequestResult.UnknownError constructor calls across multiple lines to comply with 120-character line length limit. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Prevent raw API error messages from being shown to users The friendlyErrorMessage helper was returning raw WpApiException messages directly to users, which could include technical content. Remove this early return so errors always use localized string resources. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3295d05 commit a390930

8 files changed

Lines changed: 187 additions & 199 deletions

File tree

WordPress/src/main/java/org/wordpress/android/ui/postsrs/PostRsListViewModel.kt

Lines changed: 114 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import androidx.annotation.MainThread
44
import androidx.lifecycle.ViewModel
55
import androidx.lifecycle.viewModelScope
66
import dagger.hilt.android.lifecycle.HiltViewModel
7+
import kotlinx.coroutines.CancellationException
78
import kotlinx.coroutines.Dispatchers
89
import kotlinx.coroutines.FlowPreview
910
import kotlinx.coroutines.Job
@@ -26,7 +27,6 @@ import org.wordpress.android.ui.blaze.BlazeFeatureUtils
2627
import org.wordpress.android.ui.mysite.SelectedSiteRepository
2728
import org.wordpress.android.ui.posts.AuthorFilterSelection
2829
import org.wordpress.android.ui.postsrs.data.PostRsRestClient
29-
import org.wordpress.android.ui.postsrs.data.PostRsRestClient.PostActionResult
3030
import org.wordpress.android.ui.postsrs.data.WpServiceProvider
3131
import org.wordpress.android.ui.prefs.AppPrefsWrapper
3232
import org.wordpress.android.util.AppLog
@@ -38,6 +38,7 @@ import rs.wordpress.cache.kotlin.getObservablePostMetadataCollectionWithEditCont
3838
import rs.wordpress.cache.kotlin.hasMorePages
3939
import uniffi.wp_api.PostEndpointType
4040
import uniffi.wp_api.PostStatus
41+
import uniffi.wp_api.PostUpdateParams
4142
import uniffi.wp_api.RequestExecutionErrorReason
4243
import uniffi.wp_api.WpApiException
4344
import uniffi.wp_api.WpApiParamPostsOrderBy
@@ -88,6 +89,7 @@ class PostRsListViewModel @Inject constructor(
8889
private val _site: SiteModel? = selectedSiteRepository.getSelectedSite()
8990
private val site: SiteModel
9091
get() = requireNotNull(_site) { "No selected site — Activity should have finished" }
92+
private val postService by lazy { serviceProvider.getService(site).posts() }
9193

9294
val avatarUrl: String? = accountStore.account?.avatarUrl
9395

@@ -237,19 +239,19 @@ class PostRsListViewModel @Inject constructor(
237239
_pendingConfirmation.value = PendingConfirmation.Trash(remotePostId)
238240
PostRsMenuAction.DELETE_PERMANENTLY ->
239241
_pendingConfirmation.value = PendingConfirmation.Delete(remotePostId)
240-
PostRsMenuAction.PUBLISH -> publishPost(site, remotePostId)
241-
PostRsMenuAction.MOVE_TO_DRAFT -> moveToDraft(site, remotePostId)
242+
PostRsMenuAction.PUBLISH -> publishPost(remotePostId)
243+
PostRsMenuAction.MOVE_TO_DRAFT -> moveToDraft(remotePostId)
242244
PostRsMenuAction.DUPLICATE -> duplicatePost(remotePostId)
243245
}
244246
}
245247

246248
@MainThread
247249
fun onConfirmPendingAction() {
248250
when (val confirmation = _pendingConfirmation.value) {
249-
is PendingConfirmation.Trash -> trashPost(site, confirmation.postId)
250-
is PendingConfirmation.Delete -> deletePost(site, confirmation.postId)
251+
is PendingConfirmation.Trash -> trashPost(confirmation.postId)
252+
is PendingConfirmation.Delete -> deletePost(confirmation.postId)
251253
is PendingConfirmation.MoveToDraft ->
252-
moveToDraftAndEdit(site, confirmation.postId)
254+
moveToDraftAndEdit(confirmation.postId)
253255
null -> Unit
254256
}
255257
_pendingConfirmation.value = null
@@ -260,86 +262,77 @@ class PostRsListViewModel @Inject constructor(
260262
_pendingConfirmation.value = null
261263
}
262264

263-
private fun trashPost(site: SiteModel, postId: Long) = executePostAction(
264-
postId, R.string.post_rs_trashed, R.string.post_rs_error_trash
265-
) { restClient.trashPost(site, postId) }
265+
private fun trashPost(postId: Long) = executePostMutation(
266+
successMessageResId = R.string.post_rs_trashed,
267+
errorMessageResId = R.string.post_rs_error_trash,
268+
logTag = "Trash"
269+
) {
270+
postService.trashPost(PostEndpointType.Posts, postId)
271+
}
266272

267-
private fun deletePost(site: SiteModel, postId: Long) = executePostAction(
268-
postId, R.string.post_rs_deleted, R.string.post_rs_error_delete
269-
) { restClient.deletePost(site, postId) }
273+
private fun deletePost(postId: Long) = executePostMutation(
274+
successMessageResId = R.string.post_rs_deleted,
275+
errorMessageResId = R.string.post_rs_error_delete,
276+
logTag = "Delete"
277+
) {
278+
postService.deletePostPermanently(PostEndpointType.Posts, postId)
279+
}
270280

271-
private fun publishPost(site: SiteModel, postId: Long) = executePostAction(
272-
postId, R.string.post_rs_published, R.string.post_rs_error_update_status
273-
) { restClient.updatePostStatus(site, postId, PostStatus.Publish) }
281+
private fun publishPost(postId: Long) = executePostMutation(
282+
successMessageResId = R.string.post_rs_published,
283+
errorMessageResId = R.string.post_rs_error_update_status,
284+
logTag = "Publish"
285+
) {
286+
postService.updatePost(
287+
PostEndpointType.Posts, postId,
288+
postStatusUpdate(PostStatus.Publish)
289+
)
290+
}
274291

275-
private fun moveToDraft(site: SiteModel, postId: Long) = executePostAction(
276-
postId, R.string.post_rs_moved_to_draft, R.string.post_rs_error_update_status
277-
) { restClient.updatePostStatus(site, postId, PostStatus.Draft) }
292+
private fun moveToDraft(postId: Long) = executePostMutation(
293+
successMessageResId = R.string.post_rs_moved_to_draft,
294+
errorMessageResId = R.string.post_rs_error_update_status,
295+
logTag = "Move to draft"
296+
) {
297+
postService.updatePost(
298+
PostEndpointType.Posts, postId,
299+
postStatusUpdate(PostStatus.Draft)
300+
)
301+
}
278302

279-
private fun moveToDraftAndEdit(site: SiteModel, postId: Long) {
280-
updateTabUiState(PostRsListTab.TRASHED) {
281-
copy(isRefreshing = true)
282-
}
283-
val clearRefreshing = {
284-
updateTabUiState(PostRsListTab.TRASHED) {
285-
copy(isRefreshing = false)
286-
}
287-
}
288-
executePostAction(
289-
postId = postId,
290-
errorResId = R.string.post_rs_error_update_status,
291-
onSuccess = {
292-
refreshAllTabs()
303+
@Suppress("TooGenericExceptionCaught")
304+
private fun moveToDraftAndEdit(postId: Long) {
305+
if (!checkNetwork()) return
306+
updateTabUiState(PostRsListTab.TRASHED) { copy(isRefreshing = true) }
307+
viewModelScope.launch {
308+
try {
309+
withContext(Dispatchers.IO) {
310+
postService.updatePost(
311+
PostEndpointType.Posts, postId,
312+
postStatusUpdate(PostStatus.Draft)
313+
)
314+
}
293315
val post = getFluxCPost(postId)
294316
if (post != null) {
295-
_events.trySend(
296-
PostRsListEvent.EditPost(site, post)
297-
)
317+
_events.trySend(PostRsListEvent.EditPost(site, post))
298318
} else {
299319
_events.trySend(
300-
PostRsListEvent.ShowToast(
301-
R.string.post_rs_moved_to_draft
302-
)
320+
PostRsListEvent.ShowToast(R.string.post_rs_moved_to_draft)
303321
)
304322
}
305-
},
306-
onError = clearRefreshing
307-
) { restClient.updatePostStatus(site, postId, PostStatus.Draft) }
308-
}
309-
310-
/**
311-
* Shared helper that runs a post mutation on IO.
312-
*
313-
* @param postId Remote ID of the post being acted on.
314-
* @param successResId String resource shown when [action] succeeds
315-
* (unused when [onSuccess] is provided).
316-
* @param errorResId String resource shown when [action] fails.
317-
* @param onSuccess Optional callback that replaces the default
318-
* success handling (remove from UI + snackbar
319-
* + refresh).
320-
* @param onError Optional callback invoked after the default
321-
* error handling (e.g. to clear loading state).
322-
* @param action Suspend lambda that performs the network call.
323-
*/
324-
private fun executePostAction(
325-
postId: Long,
326-
successResId: Int = 0,
327-
errorResId: Int,
328-
onSuccess: (() -> Unit)? = null,
329-
onError: (() -> Unit)? = null,
330-
action: suspend () -> PostActionResult
331-
) {
332-
if (!checkNetwork()) {
333-
onError?.invoke()
334-
return
335-
}
336-
viewModelScope.launch {
337-
val result = withContext(Dispatchers.IO) { action() }
338-
if (result is PostActionResult.Success && onSuccess != null) {
339-
onSuccess()
340-
} else {
341-
handleActionResult(result, postId, successResId, errorResId)
342-
if (result is PostActionResult.Error) onError?.invoke()
323+
} catch (e: CancellationException) {
324+
throw e
325+
} catch (e: Exception) {
326+
AppLog.e(AppLog.T.POSTS, "Move to draft and edit failed", e)
327+
_snackbarMessages.trySend(
328+
SnackbarMessage(
329+
friendlyErrorMessage(e, R.string.post_rs_error_update_status)
330+
)
331+
)
332+
} finally {
333+
updateTabUiState(PostRsListTab.TRASHED) {
334+
copy(isRefreshing = false)
335+
}
343336
}
344337
}
345338
}
@@ -364,6 +357,11 @@ class PostRsListViewModel @Inject constructor(
364357
_events.trySend(PostRsListEvent.EditPost(site, newPost))
365358
}
366359

360+
/**
361+
* Extracts the underlying [WpApiException] from a [FetchException.Api]
362+
* wrapper so that callers can inspect API-level error details (status
363+
* codes, error reasons) without knowing about the wrapper type.
364+
*/
367365
private fun unwrapException(e: Exception?): Exception? =
368366
(e as? FetchException.Api)?.v1 ?: e
369367

@@ -382,12 +380,23 @@ class PostRsListViewModel @Inject constructor(
382380
errorCode is WpErrorCode.NoAuthenticatedAppPassword
383381
}
384382

383+
private fun checkNetwork(): Boolean {
384+
if (!networkUtilsWrapper.isNetworkAvailable()) {
385+
_snackbarMessages.trySend(
386+
SnackbarMessage(resourceProvider.getString(R.string.no_network_message))
387+
)
388+
return false
389+
}
390+
return true
391+
}
392+
385393
/**
386394
* Returns a user-friendly error subtitle based on the exception type.
387-
* Detects offline, authentication, and generic errors. Raw
388-
* exception/API messages are never surfaced to the user.
395+
* Detects offline, authentication, and generic errors. When a default
396+
* resource ID is provided and the exception is a WpApiException with a
397+
* message, that message is used; otherwise falls back to the default.
389398
*/
390-
private fun friendlyErrorMessage(e: Exception? = null): String {
399+
private fun friendlyErrorMessage(e: Exception? = null, defaultResId: Int? = null): String {
391400
val apiException = unwrapException(e)
392401
val reason = (apiException as? WpApiException.RequestExecutionFailed)?.reason
393402

@@ -398,61 +407,47 @@ class PostRsListViewModel @Inject constructor(
398407

399408
isAuthError(e) -> R.string.post_rs_error_auth
400409

410+
defaultResId != null -> defaultResId
411+
401412
else -> R.string.request_failed_message
402413
}
403414
return resourceProvider.getString(resId)
404415
}
405416

406-
private fun checkNetwork(): Boolean {
407-
if (!networkUtilsWrapper.isNetworkAvailable()) {
408-
_snackbarMessages.trySend(
409-
SnackbarMessage(resourceProvider.getString(R.string.no_network_message))
410-
)
411-
return false
412-
}
413-
return true
414-
}
417+
/** Creates a PostUpdateParams for changing post status. */
418+
private fun postStatusUpdate(status: PostStatus) = PostUpdateParams(status = status, meta = null)
415419

416420
/**
417-
* Processes the outcome of a post mutation. On success, optimistically
418-
* removes the post from the UI, shows a toast, and refreshes all tabs.
419-
* On error, logs the failure and shows an error toast.
421+
* Executes a post mutation (trash, draft, etc.) with standard error handling.
422+
* Checks network, launches coroutine, executes operation on IO,
423+
* shows success/error messages.
420424
*/
421-
private fun handleActionResult(
422-
result: PostActionResult,
423-
postId: Long,
424-
successResId: Int,
425-
errorResId: Int
425+
@Suppress("TooGenericExceptionCaught")
426+
private fun executePostMutation(
427+
successMessageResId: Int,
428+
errorMessageResId: Int,
429+
logTag: String,
430+
operation: suspend () -> Unit
426431
) {
427-
when (result) {
428-
is PostActionResult.Success -> {
429-
removePostFromState(postId)
432+
if (!checkNetwork()) return
433+
viewModelScope.launch {
434+
try {
435+
withContext(Dispatchers.IO) { operation() }
430436
_snackbarMessages.trySend(
431-
SnackbarMessage(resourceProvider.getString(successResId))
437+
SnackbarMessage(resourceProvider.getString(successMessageResId))
432438
)
433-
refreshAllTabs()
434-
}
435-
is PostActionResult.Error -> {
436-
AppLog.e(AppLog.T.POSTS, "Post action failed: ${result.message}")
439+
} catch (e: CancellationException) {
440+
throw e
441+
} catch (e: Exception) {
442+
AppLog.e(AppLog.T.POSTS, "$logTag failed", e)
437443
_snackbarMessages.trySend(
438-
SnackbarMessage(resourceProvider.getString(errorResId))
444+
SnackbarMessage(friendlyErrorMessage(e, errorMessageResId))
439445
)
440446
}
441447
}
442448
}
443449

444-
private fun refreshAllTabs() {
445-
collections.keys.toList().forEach { tab -> refreshTab(tab) }
446-
}
447-
448-
/** Optimistically removes a post from every tab's UI state so it disappears immediately. */
449-
private fun removePostFromState(postId: Long) {
450-
_tabStates.value = _tabStates.value.mapValues { (_, state) ->
451-
val filtered = state.posts.filter { it.remotePostId != postId }
452-
if (filtered.size != state.posts.size) state.copy(posts = filtered) else state
453-
}
454-
}
455-
450+
/** Searches all tab states for a [PostRsUiModel] matching [remotePostId]. */
456451
private fun findPost(remotePostId: Long): PostRsUiModel? {
457452
for (state in _tabStates.value.values) {
458453
for (post in state.posts) {
@@ -559,6 +554,7 @@ class PostRsListViewModel @Inject constructor(
559554
collections[tab] = collection
560555
initializingTabs.remove(tab)
561556
registerObservers(tab, collection)
557+
loadItemsForTab(tab)
562558
refreshTab(tab)
563559
} catch (e: Exception) {
564560
AppLog.e(AppLog.T.POSTS, "Failed to init RS post list tab", e)
@@ -668,7 +664,7 @@ class PostRsListViewModel @Inject constructor(
668664
fun loadMorePosts(tab: PostRsListTab) {
669665
val collection = collections[tab] ?: return
670666
val current = getTabUiState(tab)
671-
if (current.isLoadingMore || !current.canLoadMore) return
667+
if (current.isLoadingMore || current.isRefreshing || !current.canLoadMore) return
672668

673669
updateTabUiState(tab) { copy(isLoadingMore = true) }
674670

0 commit comments

Comments
 (0)