From 1df8fe8afe38cbbe3f7660a44aae07e430f875d4 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Tue, 5 Oct 2021 12:12:16 +0300 Subject: [PATCH 1/6] Handles uninitialised site model gracefully --- .../viewmodel/mlp/ModalLayoutPickerViewModel.kt | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModel.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModel.kt index 512705255362..1735720ea441 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModel.kt @@ -61,15 +61,19 @@ class ModalLayoutPickerViewModel @Inject constructor( private val _onCreateNewPageRequested = SingleLiveEvent() val onCreateNewPageRequested: LiveData = _onCreateNewPageRequested - private val site: SiteModel by lazy { - requireNotNull(siteStore.getSiteByLocalId(selectedSiteRepository.getSelectedSiteLocalId())) + private val site: SiteModel? by lazy { + siteStore.getSiteByLocalId(selectedSiteRepository.getSelectedSiteLocalId()) } 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?) { @@ -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 } @@ -99,7 +104,7 @@ class ModalLayoutPickerViewModel @Inject constructor( } launch { val payload = FetchBlockLayoutsPayload( - site, + selectedSite, supportedBlocksProvider.fromAssets().supported, thumbDimensionProvider.previewWidth.toFloat(), thumbDimensionProvider.previewHeight.toFloat(), From 3613d887c532e743587af305226c4be3f6986ad3 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Tue, 5 Oct 2021 12:12:56 +0300 Subject: [PATCH 2/6] Adds test case for initialised site --- .../mlp/ModalLayoutPickerViewModelTest.kt | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModelTest.kt index dcf7b7770332..dcabd004c3e6 100644 --- a/WordPress/src/test/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModelTest.kt @@ -103,15 +103,22 @@ class ModalLayoutPickerViewModelTest { } @ExperimentalCoroutinesApi - private fun mockFetchingSelectedSite(isError: Boolean = false, block: suspend CoroutineScope.() -> T) { + private fun 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) @@ -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`() = + mockFetchingSelectedSite(isSiteUnavailable = true) { + viewModel.createPageFlowTriggered() + assertThat(viewModel.uiState.value is Error).isEqualTo(true) + } + @ExperimentalCoroutinesApi @Test fun `modal layout picker is shown when triggered`() = mockFetchingSelectedSite { From 7871b05dd010f50dc71a4be3846b033a1276d4d9 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Tue, 5 Oct 2021 16:09:59 +0300 Subject: [PATCH 3/6] Reverts to the old getSelectedSiteLocalId behavior --- .../android/viewmodel/mlp/ModalLayoutPickerViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModel.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModel.kt index 1735720ea441..6b0a09c48a0e 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModel.kt @@ -62,7 +62,7 @@ class ModalLayoutPickerViewModel @Inject constructor( val onCreateNewPageRequested: LiveData = _onCreateNewPageRequested private val site: SiteModel? by lazy { - siteStore.getSiteByLocalId(selectedSiteRepository.getSelectedSiteLocalId()) + siteStore.getSiteByLocalId(selectedSiteRepository.getSelectedSiteLocalId(true)) } override val useCachedData: Boolean = true From 1f8f1005aa5adf0ab228977c36c51dd6ffb29144 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Tue, 5 Oct 2021 16:10:34 +0300 Subject: [PATCH 4/6] Makes SiteUtils.isBlockEditorDefaultForNewPost parameter explicitly nullable --- .../src/main/java/org/wordpress/android/util/SiteUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/util/SiteUtils.java b/WordPress/src/main/java/org/wordpress/android/util/SiteUtils.java index 0ccf3ff579be..4c0c1d16fce1 100644 --- a/WordPress/src/main/java/org/wordpress/android/util/SiteUtils.java +++ b/WordPress/src/main/java/org/wordpress/android/util/SiteUtils.java @@ -205,7 +205,7 @@ public static void disableBlockEditor(Dispatcher dispatcher, SiteModel siteModel dispatcher.dispatch(SiteActionBuilder.newUpdateSiteAction(siteModel)); } - public static boolean isBlockEditorDefaultForNewPost(SiteModel site) { + public static boolean isBlockEditorDefaultForNewPost(@Nullable SiteModel site) { if (site == null) { return true; } From 391efede26d747ce1a5714050348031478ae7f27 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Tue, 5 Oct 2021 16:10:46 +0300 Subject: [PATCH 5/6] Adds release notes --- RELEASE-NOTES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 7069e5cc13bb..8a7eb902d294 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -2,7 +2,7 @@ 18.5 ----- - +* [*] Fixed a crash in Page Template chooser [https://github.com/wordpress-mobile/WordPress-Android/pull/15415] 18.4 ----- From 776bcd4379338fcebccec3ae793dcd90f67e7c73 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Tue, 5 Oct 2021 17:53:12 +0300 Subject: [PATCH 6/6] Fixes broken tests --- .../android/viewmodel/mlp/ModalLayoutPickerViewModelTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModelTest.kt index dcabd004c3e6..c5f7d23660ec 100644 --- a/WordPress/src/test/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModelTest.kt @@ -113,7 +113,7 @@ class ModalLayoutPickerViewModelTest { id = 1 mobileEditor = GB_EDITOR_NAME } - whenever(selectedSiteRepository.getSelectedSiteLocalId()).thenReturn(site.id) + whenever(selectedSiteRepository.getSelectedSiteLocalId(true)).thenReturn(site.id) if (isSiteUnavailable) { whenever(siteStore.getSiteByLocalId(site.id)).thenReturn(null) } else {