Skip to content

Commit d587ead

Browse files
committed
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.
1 parent 669c3b4 commit d587ead

8 files changed

Lines changed: 57 additions & 25 deletions

File tree

WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostRepository.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ class EditPostRepository
7373
val password: String
7474
get() = post!!.password
7575
val status: PostStatus
76-
get() = post?.let { fromPost(it) } ?: PostStatus.UNKNOWN
76+
get() = fromPost(post!!)
7777
val isPage: Boolean
7878
get() = post!!.isPage
7979
val isLocalDraft: Boolean

WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingBottomSheetFragment.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ class PrepublishingBottomSheetFragment : WPBottomSheetDialogFragment(),
191191
KEY_SCREEN_STATE
192192
)
193193
val site = requireNotNull(arguments?.getSerializableCompat<SiteModel>(SITE))
194-
viewModel.start(site, prepublishingScreenState)
194+
viewModel.start(site, prepublishingScreenState, getEditorHook().editPostRepository.hasPost())
195195
}
196196

197197
private fun navigateToScreen(navigationTarget: PrepublishingNavigationTarget) {

WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModel.kt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,21 @@ class PrepublishingViewModel @Inject constructor(private val dispatcher: Dispatc
6262

6363
fun start(
6464
site: SiteModel,
65-
currentScreenFromSavedState: PrepublishingScreen?
65+
currentScreenFromSavedState: PrepublishingScreen?,
66+
hasPost: Boolean = true
6667
) {
6768
this.site = site
6869

70+
// The sheet is a DialogFragment, so the FragmentManager restores it after a config change or
71+
// process death even when the host hasn't loaded a post. Guarding here rather than in an
72+
// individual screen's ViewModel covers every screen the sheet can restore into, since the
73+
// saved screen below may be TAGS, CATEGORIES or PUBLISH rather than HOME.
74+
if (!hasPost) {
75+
AppLog.e(T.POSTS, "Prepublishing sheet started without a post; dismissing.")
76+
_dismissBottomSheet.postValue(Event(Unit))
77+
return
78+
}
79+
6980
// Set screen: use saved state if available (config change), otherwise reset to HOME (dismissal + reopen)
7081
val targetScreen = currentScreenFromSavedState ?: HOME
7182
this.currentScreen = targetScreen

WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeFragment.kt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package org.wordpress.android.ui.posts.prepublishing.home
33
import android.content.Context
44
import android.os.Bundle
55
import android.view.View
6-
import androidx.fragment.app.DialogFragment
76
import androidx.fragment.app.Fragment
87
import androidx.lifecycle.ViewModelProvider
98
import androidx.recyclerview.widget.LinearLayoutManager
@@ -103,11 +102,6 @@ class PrepublishingHomeFragment : Fragment(R.layout.post_prepublishing_home_frag
103102
actionClickedListener?.onSubmitButtonClicked(publishPost)
104103
}
105104

106-
viewModel.dismissSheet.observeEvent(viewLifecycleOwner) {
107-
// allowing state loss because this can run while the host activity is already finishing
108-
(parentFragment as? DialogFragment)?.dismissAllowingStateLoss()
109-
}
110-
111105
viewModel.start(getEditPostRepository(), getSite())
112106
}
113107

WordPress/src/main/java/org/wordpress/android/ui/posts/prepublishing/home/PrepublishingHomeViewModel.kt

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,6 @@ class PrepublishingHomeViewModel @Inject constructor(
5050
private val _onSubmitButtonClicked = MutableLiveData<Event<PublishPost>>()
5151
val onSubmitButtonClicked: LiveData<Event<PublishPost>> = _onSubmitButtonClicked
5252

53-
private val _dismissSheet = MutableLiveData<Event<Unit>>()
54-
val dismissSheet: LiveData<Event<Unit>> = _dismissSheet
55-
5653
private val _uiState = MutableLiveData<List<PrepublishingHomeItemUiState>>()
5754
private var _socialUiState: MutableLiveData<SocialUiState> = MutableLiveData(SocialUiState.Hidden)
5855

@@ -72,12 +69,11 @@ class PrepublishingHomeViewModel @Inject constructor(
7269
fun start(editPostRepository: EditPostRepository, site: SiteModel) {
7370
this.editPostRepository = editPostRepository
7471
if (isStarted) return
75-
// The sheet is a DialogFragment, so the FragmentManager can restore it after a config
76-
// change or process death before the host activity has loaded a post. Close the sheet
77-
// rather than building its UI against an empty repository.
72+
// PrepublishingViewModel already dismisses the sheet when the host has no post, but that
73+
// dismissal is asynchronous and the FragmentManager can restore this fragment and create its
74+
// view before it commits. Skip the UI rather than reading a post that isn't there.
7875
if (!editPostRepository.hasPost()) {
79-
AppLog.e(AppLog.T.POSTS, "Prepublishing sheet opened without a post; dismissing.")
80-
_dismissSheet.postValue(Event(Unit))
76+
AppLog.e(AppLog.T.POSTS, "Prepublishing home started without a post; skipping setup.")
8177
return
8278
}
8379
isStarted = true

WordPress/src/test/java/org/wordpress/android/ui/posts/EditPostRepositoryTest.kt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import org.wordpress.android.fluxc.model.post.PostLocation
1919
import org.wordpress.android.fluxc.model.post.PostStatus.DRAFT
2020
import org.wordpress.android.fluxc.model.post.PostStatus.PENDING
2121
import org.wordpress.android.fluxc.model.post.PostStatus.PUBLISHED
22-
import org.wordpress.android.fluxc.model.post.PostStatus.UNKNOWN
2322
import org.wordpress.android.fluxc.store.PostStore
2423
import org.wordpress.android.ui.posts.EditPostRepository.UpdatePostResult
2524
import org.wordpress.android.util.LocaleManagerWrapper
@@ -70,11 +69,6 @@ class EditPostRepositoryTest : BaseUnitTest() {
7069
assertThat(editPostRepository.isPostPublishable()).isFalse()
7170
}
7271

73-
@Test
74-
fun `status is UNKNOWN before initialization`() {
75-
assertThat(editPostRepository.status).isEqualTo(UNKNOWN)
76-
}
77-
7872
@Test
7973
fun `reads post for undo correctly`() {
8074
val post = PostModel()

WordPress/src/test/java/org/wordpress/android/ui/posts/PrepublishingHomeViewModelTest.kt

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -399,18 +399,31 @@ class PrepublishingHomeViewModelTest : BaseUnitTest() {
399399
}
400400

401401
@Test
402-
fun `given a repository with no post, when the viewModel is started, then the sheet is dismissed`() {
402+
fun `given a repository with no post, when the viewModel is started, then no ui state is built`() {
403403
// arrange
404404
whenever(editPostRepository.hasPost()).thenReturn(false)
405405

406406
// act
407407
viewModel.start(editPostRepository, site)
408408

409409
// assert
410-
assertThat(viewModel.dismissSheet.value?.peekContent()).isNotNull
411410
assertThat(viewModel.uiState.value).isNull()
412411
}
413412

413+
@Test
414+
fun `given a start with no post, when started again once the post loads, then the ui state is built`() {
415+
// arrange
416+
whenever(editPostRepository.hasPost()).thenReturn(false)
417+
viewModel.start(editPostRepository, site)
418+
419+
// act - the guard must not latch isStarted, or the sheet would stay empty forever
420+
whenever(editPostRepository.hasPost()).thenReturn(true)
421+
viewModel.start(editPostRepository, site)
422+
423+
// assert
424+
assertThat(viewModel.uiState.value).isNotNull
425+
}
426+
414427
private fun getHeaderUiState() = viewModel.uiState.value?.filterIsInstance(HeaderUiState::class.java)?.first()
415428

416429
private fun getButtonUiState(): ButtonUiState? {

WordPress/src/test/java/org/wordpress/android/ui/posts/prepublishing/PrepublishingViewModelTest.kt

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,30 @@ class PrepublishingViewModelTest : BaseUnitTest() {
5454
assertThat(event?.peekContent()?.targetScreen).isEqualTo(expectedScreen)
5555
}
5656

57+
@Test
58+
fun `when viewModel start is called without a post, dismiss the sheet instead of navigating`() {
59+
var dismissEvent: Event<Unit>? = null
60+
var navigationEvent: Event<PrepublishingNavigationTarget>? = null
61+
viewModel.dismissBottomSheet.observeForever { dismissEvent = it }
62+
viewModel.navigationTarget.observeForever { navigationEvent = it }
63+
64+
// TAGS rather than HOME because a restored sheet can land on any screen
65+
viewModel.start(mock(), TAGS, hasPost = false)
66+
67+
assertThat(dismissEvent).isNotNull
68+
assertThat(navigationEvent).isNull()
69+
}
70+
71+
@Test
72+
fun `when viewModel start is called with a post, don't dismiss the sheet`() {
73+
var dismissEvent: Event<Unit>? = null
74+
viewModel.dismissBottomSheet.observeForever { dismissEvent = it }
75+
76+
viewModel.start(mock(), TAGS, hasPost = true)
77+
78+
assertThat(dismissEvent).isNull()
79+
}
80+
5781
@Test
5882
fun `when onBackClicked is pressed and currentScreen isn't HOME, navigate to HOME`() {
5983
val expectedScreen = HOME

0 commit comments

Comments
 (0)