From 669c3b40c706bb9d325f9f0ab8b8ce31903df41f Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Fri, 24 Jul 2026 16:01:29 -0400 Subject: [PATCH 1/3] Fix crash when the Prepublishing sheet is restored without a post The sheet is a DialogFragment, so the FragmentManager restores it after a config change or process death regardless of whether the host activity has loaded a post, and PrepublishingHomeFragment then built its UI against an empty EditPostRepository. Dismiss the sheet instead, and stop EditPostRepository.status from laundering a null post through the unannotated PostStatus.fromPost. Fixes JETPACK-ANDROID-1EVE --- .../android/ui/posts/EditPostRepository.kt | 2 +- .../home/PrepublishingHomeFragment.kt | 6 +++++ .../home/PrepublishingHomeViewModel.kt | 12 ++++++++++ .../ui/posts/EditPostRepositoryTest.kt | 6 +++++ .../posts/PrepublishingHomeViewModelTest.kt | 24 +++++++++++++++---- 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostRepository.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostRepository.kt index 48d4b4ed98e3..fadae0326be7 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostRepository.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostRepository.kt @@ -73,7 +73,7 @@ class EditPostRepository val password: String get() = post!!.password val status: PostStatus - get() = fromPost(getPost()) + get() = post?.let { fromPost(it) } ?: PostStatus.UNKNOWN val isPage: Boolean get() = post!!.isPage val isLocalDraft: Boolean diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeFragment.kt index ea89d63f7125..5d17bfc251a0 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeFragment.kt @@ -3,6 +3,7 @@ package org.wordpress.android.ui.posts.prepublishing.home import android.content.Context import android.os.Bundle import android.view.View +import androidx.fragment.app.DialogFragment import androidx.fragment.app.Fragment import androidx.lifecycle.ViewModelProvider import androidx.recyclerview.widget.LinearLayoutManager @@ -102,6 +103,11 @@ class PrepublishingHomeFragment : Fragment(R.layout.post_prepublishing_home_frag actionClickedListener?.onSubmitButtonClicked(publishPost) } + viewModel.dismissSheet.observeEvent(viewLifecycleOwner) { + // allowing state loss because this can run while the host activity is already finishing + (parentFragment as? DialogFragment)?.dismissAllowingStateLoss() + } + viewModel.start(getEditPostRepository(), getSite()) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeViewModel.kt index 30a19c97065c..94ab56ad5ebb 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeViewModel.kt @@ -23,6 +23,7 @@ import org.wordpress.android.ui.posts.prepublishing.home.usecases.GetButtonUiSta import org.wordpress.android.ui.posts.trackPrepublishingNudges import org.wordpress.android.ui.utils.UiString.UiStringRes import org.wordpress.android.ui.utils.UiString.UiStringText +import org.wordpress.android.util.AppLog import org.wordpress.android.util.StringUtils import org.wordpress.android.util.analytics.AnalyticsTrackerWrapper import org.wordpress.android.util.merge @@ -49,6 +50,9 @@ class PrepublishingHomeViewModel @Inject constructor( private val _onSubmitButtonClicked = MutableLiveData>() val onSubmitButtonClicked: LiveData> = _onSubmitButtonClicked + private val _dismissSheet = MutableLiveData>() + val dismissSheet: LiveData> = _dismissSheet + private val _uiState = MutableLiveData>() private var _socialUiState: MutableLiveData = MutableLiveData(SocialUiState.Hidden) @@ -68,6 +72,14 @@ class PrepublishingHomeViewModel @Inject constructor( fun start(editPostRepository: EditPostRepository, site: SiteModel) { this.editPostRepository = editPostRepository if (isStarted) return + // The sheet is a DialogFragment, so the FragmentManager can restore it after a config + // change or process death before the host activity has loaded a post. Close the sheet + // rather than building its UI against an empty repository. + if (!editPostRepository.hasPost()) { + AppLog.e(AppLog.T.POSTS, "Prepublishing sheet opened without a post; dismissing.") + _dismissSheet.postValue(Event(Unit)) + return + } isStarted = true setupHomeUiState(editPostRepository, site) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/EditPostRepositoryTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/EditPostRepositoryTest.kt index f034239cea36..106775bbc711 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/EditPostRepositoryTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/EditPostRepositoryTest.kt @@ -19,6 +19,7 @@ import org.wordpress.android.fluxc.model.post.PostLocation import org.wordpress.android.fluxc.model.post.PostStatus.DRAFT import org.wordpress.android.fluxc.model.post.PostStatus.PENDING import org.wordpress.android.fluxc.model.post.PostStatus.PUBLISHED +import org.wordpress.android.fluxc.model.post.PostStatus.UNKNOWN import org.wordpress.android.fluxc.store.PostStore import org.wordpress.android.ui.posts.EditPostRepository.UpdatePostResult import org.wordpress.android.util.LocaleManagerWrapper @@ -69,6 +70,11 @@ class EditPostRepositoryTest : BaseUnitTest() { assertThat(editPostRepository.isPostPublishable()).isFalse() } + @Test + fun `status is UNKNOWN before initialization`() { + assertThat(editPostRepository.status).isEqualTo(UNKNOWN) + } + @Test fun `reads post for undo correctly`() { val post = PostModel() diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/PrepublishingHomeViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/PrepublishingHomeViewModelTest.kt index cc7de7211ba7..aade40569f58 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/PrepublishingHomeViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/PrepublishingHomeViewModelTest.kt @@ -71,6 +71,7 @@ class PrepublishingHomeViewModelTest : BaseUnitTest() { ).doAnswer { PublishButtonUiState(it.arguments[2] as (PublishPost) -> Unit) } + whenever(editPostRepository.hasPost()).thenReturn(true) whenever(editPostRepository.getEditablePost()).thenReturn(PostModel()) whenever(postSettingsUtils.getPublishDateLabel(any())).thenReturn(("")) whenever(site.name).thenReturn("") @@ -86,7 +87,7 @@ class PrepublishingHomeViewModelTest : BaseUnitTest() { val expectedActionsAmount = 3 // act - viewModel.start(mock(), site) + viewModel.start(editPostRepository, site) // assert assertThat(viewModel.uiState.value?.filterIsInstance(HomeUiState::class.java)?.size).isEqualTo( @@ -163,7 +164,7 @@ class PrepublishingHomeViewModelTest : BaseUnitTest() { val expectedActionsAmount = 1 // act - viewModel.start(mock(), site) + viewModel.start(editPostRepository, site) // assert assertThat(viewModel.uiState.value?.filterIsInstance(HeaderUiState::class.java)?.size).isEqualTo( @@ -177,7 +178,7 @@ class PrepublishingHomeViewModelTest : BaseUnitTest() { val expectedActionsAmount = 1 // act - viewModel.start(mock(), site) + viewModel.start(editPostRepository, site) // assert assertThat(viewModel.uiState.value?.filterIsInstance(ButtonUiState::class.java)?.size).isEqualTo( @@ -191,7 +192,7 @@ class PrepublishingHomeViewModelTest : BaseUnitTest() { val expectedActionType = PrepublishingScreenNavigation.Publish // act - viewModel.start(mock(), site) + viewModel.start(editPostRepository, site) val publishAction = getHomeUiState(expectedActionType) publishAction?.onNavigationActionClicked?.invoke(expectedActionType) @@ -205,7 +206,7 @@ class PrepublishingHomeViewModelTest : BaseUnitTest() { val expectedActionType = PrepublishingScreenNavigation.Tags // act - viewModel.start(mock(), site) + viewModel.start(editPostRepository, site) val tagsAction = getHomeUiState(expectedActionType) tagsAction?.onNavigationActionClicked?.invoke(expectedActionType) @@ -397,6 +398,19 @@ class PrepublishingHomeViewModelTest : BaseUnitTest() { assertThat(uiSocialState).isEqualTo(SocialUiState.Hidden) } + @Test + fun `given a repository with no post, when the viewModel is started, then the sheet is dismissed`() { + // arrange + whenever(editPostRepository.hasPost()).thenReturn(false) + + // act + viewModel.start(editPostRepository, site) + + // assert + assertThat(viewModel.dismissSheet.value?.peekContent()).isNotNull + assertThat(viewModel.uiState.value).isNull() + } + private fun getHeaderUiState() = viewModel.uiState.value?.filterIsInstance(HeaderUiState::class.java)?.first() private fun getButtonUiState(): ButtonUiState? { From d587ead3ac1d7ab56939f6ef0d2b396ff233210d Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Sun, 26 Jul 2026 09:11:09 -0400 Subject: [PATCH 2/3] Move the no-post guard up to PrepublishingViewModel The guard added in the previous commit only covered the HOME screen, but a restored sheet navigates to whatever screen was saved in KEY_SCREEN_STATE. On a restore into TAGS, CATEGORIES or PUBLISH the sheet stayed open over an empty EditPostRepository and still crashed on the first interaction, via requireNotNull(post) in updateAsync. Guarding in PrepublishingViewModel.start() instead covers every screen the sheet can restore into, and reuses the existing dismissBottomSheet channel, so the dismissSheet LiveData and the parentFragment cast are no longer needed. The PrepublishingHomeViewModel early return stays as a last line of defense, since the dismissal is asynchronous and the FragmentManager can restore that fragment before it commits. Also reverts the EditPostRepository.status leniency: with the guard in place its only reader is unreachable with a null post, so returning UNKNOWN just hid the missing post from the other callers. --- .../android/ui/posts/EditPostRepository.kt | 2 +- .../PrepublishingBottomSheetFragment.kt | 2 +- .../prepublishing/PrepublishingViewModel.kt | 13 +++++++++- .../home/PrepublishingHomeFragment.kt | 6 ----- .../home/PrepublishingHomeViewModel.kt | 12 ++++------ .../ui/posts/EditPostRepositoryTest.kt | 6 ----- .../posts/PrepublishingHomeViewModelTest.kt | 17 +++++++++++-- .../PrepublishingViewModelTest.kt | 24 +++++++++++++++++++ 8 files changed, 57 insertions(+), 25 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostRepository.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostRepository.kt index fadae0326be7..0b6066f85e1d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostRepository.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostRepository.kt @@ -73,7 +73,7 @@ class EditPostRepository val password: String get() = post!!.password val status: PostStatus - get() = post?.let { fromPost(it) } ?: PostStatus.UNKNOWN + get() = fromPost(post!!) val isPage: Boolean get() = post!!.isPage val isLocalDraft: Boolean diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingBottomSheetFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingBottomSheetFragment.kt index 74415d70a1b0..7323af942cc6 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingBottomSheetFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingBottomSheetFragment.kt @@ -191,7 +191,7 @@ class PrepublishingBottomSheetFragment : WPBottomSheetDialogFragment(), KEY_SCREEN_STATE ) val site = requireNotNull(arguments?.getSerializableCompat(SITE)) - viewModel.start(site, prepublishingScreenState) + viewModel.start(site, prepublishingScreenState, getEditorHook().editPostRepository.hasPost()) } private fun navigateToScreen(navigationTarget: PrepublishingNavigationTarget) { diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModel.kt index 725130edb0b2..3a61cdfbe527 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModel.kt @@ -62,10 +62,21 @@ class PrepublishingViewModel @Inject constructor(private val dispatcher: Dispatc fun start( site: SiteModel, - currentScreenFromSavedState: PrepublishingScreen? + currentScreenFromSavedState: PrepublishingScreen?, + hasPost: Boolean = true ) { this.site = site + // The sheet is a DialogFragment, so the FragmentManager restores it after a config change or + // process death even when the host hasn't loaded a post. Guarding here rather than in an + // individual screen's ViewModel covers every screen the sheet can restore into, since the + // saved screen below may be TAGS, CATEGORIES or PUBLISH rather than HOME. + if (!hasPost) { + AppLog.e(T.POSTS, "Prepublishing sheet started without a post; dismissing.") + _dismissBottomSheet.postValue(Event(Unit)) + return + } + // Set screen: use saved state if available (config change), otherwise reset to HOME (dismissal + reopen) val targetScreen = currentScreenFromSavedState ?: HOME this.currentScreen = targetScreen diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeFragment.kt index 5d17bfc251a0..ea89d63f7125 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeFragment.kt @@ -3,7 +3,6 @@ package org.wordpress.android.ui.posts.prepublishing.home import android.content.Context import android.os.Bundle import android.view.View -import androidx.fragment.app.DialogFragment import androidx.fragment.app.Fragment import androidx.lifecycle.ViewModelProvider import androidx.recyclerview.widget.LinearLayoutManager @@ -103,11 +102,6 @@ class PrepublishingHomeFragment : Fragment(R.layout.post_prepublishing_home_frag actionClickedListener?.onSubmitButtonClicked(publishPost) } - viewModel.dismissSheet.observeEvent(viewLifecycleOwner) { - // allowing state loss because this can run while the host activity is already finishing - (parentFragment as? DialogFragment)?.dismissAllowingStateLoss() - } - viewModel.start(getEditPostRepository(), getSite()) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeViewModel.kt index 94ab56ad5ebb..87dbea59e99b 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeViewModel.kt @@ -50,9 +50,6 @@ class PrepublishingHomeViewModel @Inject constructor( private val _onSubmitButtonClicked = MutableLiveData>() val onSubmitButtonClicked: LiveData> = _onSubmitButtonClicked - private val _dismissSheet = MutableLiveData>() - val dismissSheet: LiveData> = _dismissSheet - private val _uiState = MutableLiveData>() private var _socialUiState: MutableLiveData = MutableLiveData(SocialUiState.Hidden) @@ -72,12 +69,11 @@ class PrepublishingHomeViewModel @Inject constructor( fun start(editPostRepository: EditPostRepository, site: SiteModel) { this.editPostRepository = editPostRepository if (isStarted) return - // The sheet is a DialogFragment, so the FragmentManager can restore it after a config - // change or process death before the host activity has loaded a post. Close the sheet - // rather than building its UI against an empty repository. + // PrepublishingViewModel already dismisses the sheet when the host has no post, but that + // dismissal is asynchronous and the FragmentManager can restore this fragment and create its + // view before it commits. Skip the UI rather than reading a post that isn't there. if (!editPostRepository.hasPost()) { - AppLog.e(AppLog.T.POSTS, "Prepublishing sheet opened without a post; dismissing.") - _dismissSheet.postValue(Event(Unit)) + AppLog.e(AppLog.T.POSTS, "Prepublishing home started without a post; skipping setup.") return } isStarted = true diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/EditPostRepositoryTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/EditPostRepositoryTest.kt index 106775bbc711..f034239cea36 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/EditPostRepositoryTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/EditPostRepositoryTest.kt @@ -19,7 +19,6 @@ import org.wordpress.android.fluxc.model.post.PostLocation import org.wordpress.android.fluxc.model.post.PostStatus.DRAFT import org.wordpress.android.fluxc.model.post.PostStatus.PENDING import org.wordpress.android.fluxc.model.post.PostStatus.PUBLISHED -import org.wordpress.android.fluxc.model.post.PostStatus.UNKNOWN import org.wordpress.android.fluxc.store.PostStore import org.wordpress.android.ui.posts.EditPostRepository.UpdatePostResult import org.wordpress.android.util.LocaleManagerWrapper @@ -70,11 +69,6 @@ class EditPostRepositoryTest : BaseUnitTest() { assertThat(editPostRepository.isPostPublishable()).isFalse() } - @Test - fun `status is UNKNOWN before initialization`() { - assertThat(editPostRepository.status).isEqualTo(UNKNOWN) - } - @Test fun `reads post for undo correctly`() { val post = PostModel() diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/PrepublishingHomeViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/PrepublishingHomeViewModelTest.kt index aade40569f58..6542b486233f 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/PrepublishingHomeViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/PrepublishingHomeViewModelTest.kt @@ -399,7 +399,7 @@ class PrepublishingHomeViewModelTest : BaseUnitTest() { } @Test - fun `given a repository with no post, when the viewModel is started, then the sheet is dismissed`() { + fun `given a repository with no post, when the viewModel is started, then no ui state is built`() { // arrange whenever(editPostRepository.hasPost()).thenReturn(false) @@ -407,10 +407,23 @@ class PrepublishingHomeViewModelTest : BaseUnitTest() { viewModel.start(editPostRepository, site) // assert - assertThat(viewModel.dismissSheet.value?.peekContent()).isNotNull assertThat(viewModel.uiState.value).isNull() } + @Test + fun `given a start with no post, when started again once the post loads, then the ui state is built`() { + // arrange + whenever(editPostRepository.hasPost()).thenReturn(false) + viewModel.start(editPostRepository, site) + + // act - the guard must not latch isStarted, or the sheet would stay empty forever + whenever(editPostRepository.hasPost()).thenReturn(true) + viewModel.start(editPostRepository, site) + + // assert + assertThat(viewModel.uiState.value).isNotNull + } + private fun getHeaderUiState() = viewModel.uiState.value?.filterIsInstance(HeaderUiState::class.java)?.first() private fun getButtonUiState(): ButtonUiState? { diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModelTest.kt index 352091399128..d9bade4ca2de 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModelTest.kt @@ -54,6 +54,30 @@ class PrepublishingViewModelTest : BaseUnitTest() { assertThat(event?.peekContent()?.targetScreen).isEqualTo(expectedScreen) } + @Test + fun `when viewModel start is called without a post, dismiss the sheet instead of navigating`() { + var dismissEvent: Event? = null + var navigationEvent: Event? = null + viewModel.dismissBottomSheet.observeForever { dismissEvent = it } + viewModel.navigationTarget.observeForever { navigationEvent = it } + + // TAGS rather than HOME because a restored sheet can land on any screen + viewModel.start(mock(), TAGS, hasPost = false) + + assertThat(dismissEvent).isNotNull + assertThat(navigationEvent).isNull() + } + + @Test + fun `when viewModel start is called with a post, don't dismiss the sheet`() { + var dismissEvent: Event? = null + viewModel.dismissBottomSheet.observeForever { dismissEvent = it } + + viewModel.start(mock(), TAGS, hasPost = true) + + assertThat(dismissEvent).isNull() + } + @Test fun `when onBackClicked is pressed and currentScreen isn't HOME, navigate to HOME`() { val expectedScreen = HOME From e9a0446386cc9a0de0a15bb24546c388c8139267 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Mon, 27 Jul 2026 10:36:36 -0400 Subject: [PATCH 3/3] Rebind the EditPostRepository when the posts list is recreated PostsListActivity has no configChanges, so it is recreated on rotation with a freshly injected EditPostRepository, while PostListMainViewModel survives. Its start() returned early on isStarted before assigning the repository and before loading the post into it, so after any rotation the ViewModel kept writing to the previous activity's dead repository while the restored Prepublishing sheet read the new empty one. That is the state the sheet guard was catching: publishing stayed broken for the rest of the activity's life, silently once the sheet began dismissing itself instead of crashing. Rebind on every start instead, and keep currentBottomSheetPostId in sync so the id onSaveInstanceState reads survives a second restore. Also makes PrepublishingViewModel.start()'s hasPost parameter required, so the guard can't be bypassed by omission, and corrects the comment: the guard dismisses on every restore path but does not stop the saved child fragment from being restored and started first, since the dismissal is async. --- .../android/ui/posts/PostListMainViewModel.kt | 40 +++++++++++++------ .../prepublishing/PrepublishingViewModel.kt | 10 +++-- .../ui/posts/PostListMainViewModelTest.kt | 27 +++++++++++++ .../PrepublishingViewModelTest.kt | 22 +++++----- 4 files changed, 72 insertions(+), 27 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListMainViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListMainViewModel.kt index a007e1746a81..13c92150c94d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListMainViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListMainViewModel.kt @@ -244,9 +244,14 @@ class PostListMainViewModel @Inject constructor( currentBottomSheetPostId: LocalId, editPostRepository: EditPostRepository ) { + // The activity is recreated on rotation with a freshly injected EditPostRepository while this + // ViewModel survives, so rebind on every start rather than only the first. Otherwise the + // restored Prepublishing sheet reads an empty repository and this ViewModel goes on writing + // to the previous activity's, which breaks publishing for the rest of the activity's life. + bindEditPostRepository(editPostRepository, site, currentBottomSheetPostId) + if (isStarted) return this.site = site - this.editPostRepository = editPostRepository val authorFilterSelection: AuthorFilterSelection = if (isFilteringByAuthorSupported) { prefs.postListAuthorSelection @@ -289,23 +294,34 @@ class PostListMainViewModel @Inject constructor( ) _previewState.value = _previewState.value ?: initPreviewState - currentBottomSheetPostId.let { postId -> - if (postId.value != 0) { - editPostRepository.loadPostByLocalPostId(postId.value) - } - } - lifecycleOwner.lifecycleRegistry.currentState = Lifecycle.State.STARTED uploadStarter.queueUploadFromSite(site) - editPostRepository.run { - postChanged.observe(lifecycleOwner, Observer { - savePostToDbUseCase.savePostToDb(editPostRepository, site) - }) + isStarted = true + } + + private fun bindEditPostRepository( + repository: EditPostRepository, + site: SiteModel, + bottomSheetPostId: LocalId + ) { + if (this::editPostRepository.isInitialized) { + if (this.editPostRepository === repository) return + // drop the previous activity's repository, or its observer would pin it for our lifetime + this.editPostRepository.postChanged.removeObservers(lifecycleOwner) + } + this.editPostRepository = repository + + if (bottomSheetPostId.value != 0) { + // keep the field in sync so onSaveInstanceState can round-trip it again + currentBottomSheetPostId = bottomSheetPostId + repository.loadPostByLocalPostId(bottomSheetPostId.value) } - isStarted = true + repository.postChanged.observe(lifecycleOwner, Observer { + savePostToDbUseCase.savePostToDb(repository, site) + }) } override fun onCleared() { diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModel.kt index 3a61cdfbe527..8ef6138a9f61 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModel.kt @@ -63,14 +63,16 @@ class PrepublishingViewModel @Inject constructor(private val dispatcher: Dispatc fun start( site: SiteModel, currentScreenFromSavedState: PrepublishingScreen?, - hasPost: Boolean = true + hasPost: Boolean ) { this.site = site // The sheet is a DialogFragment, so the FragmentManager restores it after a config change or - // process death even when the host hasn't loaded a post. Guarding here rather than in an - // individual screen's ViewModel covers every screen the sheet can restore into, since the - // saved screen below may be TAGS, CATEGORIES or PUBLISH rather than HOME. + // process death even when the host hasn't loaded a post. Every restore passes through here, + // whatever screen was saved in KEY_SCREEN_STATE, so this dismisses the sheet in all of them. + // Note it does not stop the saved child fragment from being restored and started first: the + // dismissal is async. The non-HOME screens tolerate an empty repository at start time, and + // HOME has its own early return. A new screen that reads the post at start needs one too. if (!hasPost) { AppLog.e(T.POSTS, "Prepublishing sheet started without a post; dismissing.") _dismissBottomSheet.postValue(Event(Unit)) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/PostListMainViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/PostListMainViewModelTest.kt index b80137daee86..d3dd0494ea51 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/PostListMainViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/PostListMainViewModelTest.kt @@ -159,6 +159,33 @@ class PostListMainViewModelTest : BaseUnitTest() { verify(editPostRepository, times(0)).loadPostByLocalPostId(any()) } + @Test + fun `when the activity is recreated, then the new EditPostRepository is rebound and reloaded`() { + // arrange - the activity is recreated on rotation with a freshly injected repository + val bottomSheetPostId = LocalId(2) + viewModel.start(site, PostListRemotePreviewState.NONE, bottomSheetPostId, editPostRepository) + val recreatedRepository = mock() + whenever(recreatedRepository.postChanged).thenReturn(MutableLiveData(Event(PostModel()))) + + // act - start() runs again on the surviving ViewModel + viewModel.start(site, PostListRemotePreviewState.NONE, bottomSheetPostId, recreatedRepository) + + // assert - the second repository must be loaded too, or the restored sheet reads an empty one + verify(recreatedRepository, times(1)).loadPostByLocalPostId(bottomSheetPostId.value) + } + + @Test + fun `given a restored bottom sheet post id, when started, then the id is kept for the next save`() { + // arrange + val bottomSheetPostId = LocalId(2) + + // act + viewModel.start(site, PostListRemotePreviewState.NONE, bottomSheetPostId, editPostRepository) + + // assert - onSaveInstanceState reads this field, so it has to survive a restore round-trip + assertThat(viewModel.currentBottomSheetPostId).isEqualTo(bottomSheetPostId) + } + @Test fun `if post in EditPostRepository is modified then the savePostToDbUseCase should update the post`() { // arrange diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModelTest.kt index d9bade4ca2de..cac0d9ca1651 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModelTest.kt @@ -35,7 +35,7 @@ class PrepublishingViewModelTest : BaseUnitTest() { event = it } - viewModel.start(mock(), null) + viewModel.start(mock(), null, hasPost = true) assertThat(event?.peekContent()?.targetScreen).isEqualTo(expectedScreen) } @@ -49,7 +49,7 @@ class PrepublishingViewModelTest : BaseUnitTest() { event = it } - viewModel.start(mock(), expectedScreen) + viewModel.start(mock(), expectedScreen, hasPost = true) assertThat(event?.peekContent()?.targetScreen).isEqualTo(expectedScreen) } @@ -87,7 +87,7 @@ class PrepublishingViewModelTest : BaseUnitTest() { event = it } - viewModel.start(mock(), TAGS) + viewModel.start(mock(), TAGS, hasPost = true) viewModel.onBackClicked() assertThat(event?.peekContent()?.targetScreen).isEqualTo(expectedScreen) @@ -100,7 +100,7 @@ class PrepublishingViewModelTest : BaseUnitTest() { event = it } - viewModel.start(mock(), HOME) + viewModel.start(mock(), HOME, hasPost = true) viewModel.onBackClicked() assertThat(event).isNotNull @@ -127,7 +127,7 @@ class PrepublishingViewModelTest : BaseUnitTest() { event = it } - viewModel.start(mock(), mock()) + viewModel.start(mock(), mock(), hasPost = true) viewModel.onActionClicked(PrepublishingScreenNavigation.Tags) assertThat(event?.peekContent()?.targetScreen).isEqualTo(expectedScreen) @@ -142,7 +142,7 @@ class PrepublishingViewModelTest : BaseUnitTest() { event = it } - viewModel.start(mock(), mock()) + viewModel.start(mock(), mock(), hasPost = true) viewModel.onActionClicked(PrepublishingScreenNavigation.Publish) assertThat(event?.peekContent()?.targetScreen).isEqualTo(expectedScreen) @@ -157,7 +157,7 @@ class PrepublishingViewModelTest : BaseUnitTest() { event = it } - viewModel.start(mock(), mock()) + viewModel.start(mock(), mock(), hasPost = true) viewModel.onActionClicked(PrepublishingScreenNavigation.Categories) assertThat(event?.peekContent()?.targetScreen).isEqualTo(expectedScreen) @@ -172,7 +172,7 @@ class PrepublishingViewModelTest : BaseUnitTest() { event = it } - viewModel.start(mock(), mock()) + viewModel.start(mock(), mock(), hasPost = true) viewModel.onActionClicked(PrepublishingScreenNavigation.AddCategory) assertThat(event?.peekContent()?.targetScreen).isEqualTo(expectedScreen) @@ -187,7 +187,7 @@ class PrepublishingViewModelTest : BaseUnitTest() { event = it } - viewModel.start(mock(), mock()) + viewModel.start(mock(), mock(), hasPost = true) viewModel.onActionClicked(PrepublishingScreenNavigation.Social) assertThat(event?.peekContent()?.targetScreen).isEqualTo(expectedScreen) @@ -201,7 +201,7 @@ class PrepublishingViewModelTest : BaseUnitTest() { event = it } - viewModel.start(mockSite, mock()) + viewModel.start(mockSite, mock(), hasPost = true) viewModel.onActionClicked(Action.NavigateToSharingSettings) assertThat(event?.peekContent()).isEqualTo(mockSite) @@ -209,7 +209,7 @@ class PrepublishingViewModelTest : BaseUnitTest() { @Test fun `when onSubmitButtonClicked is triggered then bottom sheet should close and listener is triggered`() { - viewModel.start(mock(), mock()) + viewModel.start(mock(), mock(), hasPost = true) viewModel.onSubmitButtonClicked(true)