Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Fixes IllegalArgumentException due to uninitialised site in page template picker #15415

Merged
merged 6 commits into from
Oct 5, 2021
Merged

Fixes IllegalArgumentException due to uninitialised site in page template picker #15415

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1df8fe8
Handles uninitialised site model gracefully
Oct 5, 2021
3613d88
Adds test case for initialised site
Oct 5, 2021
7871b05
Reverts to the old getSelectedSiteLocalId behavior
Oct 5, 2021
1f8f100
Makes SiteUtils.isBlockEditorDefaultForNewPost parameter explicitly n…
Oct 5, 2021
391efed
Adds release notes
Oct 5, 2021
776bcd4
Fixes broken tests
Oct 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions WordPress/src/main/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,19 @@ class ModalLayoutPickerViewModel @Inject constructor(
private val _onCreateNewPageRequested = SingleLiveEvent<PageRequest.Create>()
val onCreateNewPageRequested: LiveData<PageRequest.Create> = _onCreateNewPageRequested

private val site: SiteModel by lazy {
requireNotNull(siteStore.getSiteByLocalId(selectedSiteRepository.getSelectedSiteLocalId()))
private val site: SiteModel? by lazy {
siteStore.getSiteByLocalId(selectedSiteRepository.getSelectedSiteLocalId())
ParaskP7 marked this conversation as resolved.
Show resolved Hide resolved
}

override val useCachedData: Boolean = true

override val selectedLayout: LayoutModel?
get() = (uiState.value as? Content)?.let { state ->
state.selectedLayoutSlug?.let { siteStore.getBlockLayout(site, it) }?.let { LayoutModel(it) }
state.selectedLayoutSlug?.let { slug ->
site?.let { site ->
siteStore.getBlockLayout(site, slug)
}
}?.let { LayoutModel(it) }
}

sealed class PageRequest(val template: String?) {
Expand All @@ -90,7 +94,8 @@ class ModalLayoutPickerViewModel @Inject constructor(
}

override fun fetchLayouts(preferCache: Boolean) {
if (!networkUtils.isNetworkAvailable()) {
val selectedSite = site
if (!networkUtils.isNetworkAvailable() || selectedSite == null) {
setErrorState()
return
}
Expand All @@ -99,7 +104,7 @@ class ModalLayoutPickerViewModel @Inject constructor(
}
launch {
val payload = FetchBlockLayoutsPayload(
site,
selectedSite,
supportedBlocksProvider.fromAssets().supported,
thumbDimensionProvider.previewWidth.toFloat(),
thumbDimensionProvider.previewHeight.toFloat(),
Expand Down
21 changes: 18 additions & 3 deletions ...Press/src/test/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,22 @@ class ModalLayoutPickerViewModelTest {
}

@ExperimentalCoroutinesApi
private fun <T> mockFetchingSelectedSite(isError: Boolean = false, block: suspend CoroutineScope.() -> T) {
private fun <T> mockFetchingSelectedSite(
isError: Boolean = false,
isSiteUnavailable: Boolean = false,
block: suspend CoroutineScope.() -> T
) {
coroutineScope.runBlockingTest {
val site = SiteModel().apply {
id = 1
mobileEditor = GB_EDITOR_NAME
}
whenever(selectedSiteRepository.getSelectedSiteLocalId()).thenReturn(site.id)
whenever(siteStore.getSiteByLocalId(site.id)).thenReturn(site)
whenever(siteStore.getSiteByLocalId(site.id)).thenReturn(site)
if (isSiteUnavailable) {
whenever(siteStore.getSiteByLocalId(site.id)).thenReturn(null)
} else {
whenever(siteStore.getSiteByLocalId(site.id)).thenReturn(site)
}
whenever(siteStore.getBlockLayout(site, "about")).thenReturn(aboutLayout)
whenever(supportedBlocksProvider.fromAssets()).thenReturn(SupportedBlocks())
whenever(thumbDimensionProvider.previewWidth).thenReturn(136)
Expand Down Expand Up @@ -172,6 +179,14 @@ class ModalLayoutPickerViewModelTest {
assertThat(viewModel.uiState.value is Error).isEqualTo(true)
}

@ExperimentalCoroutinesApi
@Test
fun `when modal layout picker starts and the site is unavailable errors are handled`() =
antonis marked this conversation as resolved.
Show resolved Hide resolved
mockFetchingSelectedSite(isSiteUnavailable = true) {
viewModel.createPageFlowTriggered()
assertThat(viewModel.uiState.value is Error).isEqualTo(true)
}

@ExperimentalCoroutinesApi
@Test
fun `modal layout picker is shown when triggered`() = mockFetchingSelectedSite {
Expand Down