From 80f3294782cfa6b5963fd1dc5fcaef23587bec35 Mon Sep 17 00:00:00 2001
From: Sean Mac Gillicuddy <seantheappdev@gmail.com>
Date: Fri, 14 Feb 2020 14:22:53 +0000
Subject: [PATCH] #1771 Crash Report 3.2: java.lang.IllegalArgumentException
 CompatFindActionModeCallback.findAll - don't use null and correct
 searchActivity view states

---
 .idea/codeStyles/Project.xml                  |  3 ---
 .../main/CompatFindActionModeCallback.java    |  8 +++---
 .../kiwixmobile/core/search/SearchActivity.kt | 17 +++++++------
 .../core/search/viewmodel/SearchViewModel.kt  | 12 +++------
 .../core/search/viewmodel/State.kt            |  7 +++---
 .../search/viewmodel/SearchViewModelTest.kt   | 25 +++++++------------
 6 files changed, 30 insertions(+), 42 deletions(-)

diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml
index 580e9334d..6b0453612 100644
--- a/.idea/codeStyles/Project.xml
+++ b/.idea/codeStyles/Project.xml
@@ -45,9 +45,6 @@
       <option name="IMPORT_NESTED_CLASSES" value="true" />
       <option name="CODE_STYLE_DEFAULTS" value="KOTLIN_OFFICIAL" />
     </JetCodeStyleSettings>
-    <MarkdownNavigatorCodeStyleSettings>
-      <option name="RIGHT_MARGIN" value="72" />
-    </MarkdownNavigatorCodeStyleSettings>
     <XML>
       <option name="XML_LEGACY_SETTINGS_IMPORTED" value="true" />
     </XML>
diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CompatFindActionModeCallback.java b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CompatFindActionModeCallback.java
index 326e13ce8..a31888acf 100644
--- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CompatFindActionModeCallback.java
+++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CompatFindActionModeCallback.java
@@ -132,12 +132,12 @@ public class CompatFindActionModeCallback
       throw new AssertionError("No WebView for CompatFindActionModeCallback::findAll");
     }
     CharSequence find = editText.getText();
-    if (find.length() == 0) {
+    if (find == null || find.length() == 0) {
       webView.clearMatches();
       if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN) {
-        webView.findAllAsync(null);
+        webView.findAllAsync("");
       } else {
-        webView.findAll(null);
+        webView.findAll("");
       }
     } else {
       if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN) {
@@ -169,9 +169,7 @@ public class CompatFindActionModeCallback
       editText.requestFocus();
       //show the keyboard
       input.showSoftInput(editText, 0);
-
     }, 100);
-
   }
 
   @Override
diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchActivity.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchActivity.kt
index 5075ce0d8..85a85b68f 100644
--- a/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchActivity.kt
+++ b/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchActivity.kt
@@ -52,8 +52,7 @@ import org.kiwix.kiwixmobile.core.search.viewmodel.Action.OnItemClick
 import org.kiwix.kiwixmobile.core.search.viewmodel.Action.OnItemLongClick
 import org.kiwix.kiwixmobile.core.search.viewmodel.SearchViewModel
 import org.kiwix.kiwixmobile.core.search.viewmodel.State
-import org.kiwix.kiwixmobile.core.search.viewmodel.State.Empty
-import org.kiwix.kiwixmobile.core.search.viewmodel.State.Initialising
+import org.kiwix.kiwixmobile.core.search.viewmodel.State.NoResults
 import org.kiwix.kiwixmobile.core.search.viewmodel.State.Results
 import org.kiwix.kiwixmobile.core.utils.SimpleTextListener
 import javax.inject.Inject
@@ -136,15 +135,19 @@ class SearchActivity : BaseActivity() {
     is Results -> {
       searchViewAnimator.setDistinctDisplayedChild(0)
       searchAdapter.items = state.values
-      searchView.setQuery(state.searchString, false)
-      searchInTextMenuItem.isVisible = state.searchString.isNotBlank()
+      render(state.searchString)
     }
-    Empty -> searchViewAnimator.setDistinctDisplayedChild(1)
-    Initialising -> {
-      // do nothing
+    is NoResults -> {
+      searchViewAnimator.setDistinctDisplayedChild(1)
+      render(state.searchString)
     }
   }
 
+  private fun render(searchString: String) {
+    searchView.setQuery(searchString, false)
+    searchInTextMenuItem.isEnabled = searchString.isNotBlank()
+  }
+
   private fun onItemClick(it: SearchListItem) {
     searchViewModel.actions.offer(OnItemClick(it))
   }
diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModel.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModel.kt
index 66dcc24da..81ca39e7b 100644
--- a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModel.kt
+++ b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModel.kt
@@ -42,8 +42,7 @@ import org.kiwix.kiwixmobile.core.search.viewmodel.Action.OnItemClick
 import org.kiwix.kiwixmobile.core.search.viewmodel.Action.OnItemLongClick
 import org.kiwix.kiwixmobile.core.search.viewmodel.Action.ReceivedPromptForSpeechInput
 import org.kiwix.kiwixmobile.core.search.viewmodel.Action.StartSpeechInputFailed
-import org.kiwix.kiwixmobile.core.search.viewmodel.State.Empty
-import org.kiwix.kiwixmobile.core.search.viewmodel.State.Initialising
+import org.kiwix.kiwixmobile.core.search.viewmodel.State.NoResults
 import org.kiwix.kiwixmobile.core.search.viewmodel.State.Results
 import org.kiwix.kiwixmobile.core.search.viewmodel.effects.DeleteRecentSearch
 import org.kiwix.kiwixmobile.core.search.viewmodel.effects.Finish
@@ -64,7 +63,7 @@ class SearchViewModel @Inject constructor(
   private val searchResultGenerator: SearchResultGenerator
 ) : ViewModel() {
 
-  val state = MutableLiveData<State>().apply { value = Initialising }
+  val state = MutableLiveData<State>().apply { value = NoResults("") }
   val effects = PublishProcessor.create<SideEffect<*>>()
   val actions = PublishProcessor.create<Action>()
   private val filter = BehaviorProcessor.createDefault("")
@@ -108,10 +107,7 @@ class SearchViewModel @Inject constructor(
   }
 
   private fun searchPreviousScreenWhenStateIsValid(): Any =
-    when (val currentState = state.value) {
-      is Results -> effects.offer(SearchInPreviousScreen(currentState.searchString))
-      else -> Unit
-    }
+    effects.offer(SearchInPreviousScreen(state.value!!.searchString))
 
   private fun showDeleteDialog(longClick: OnItemLongClick) {
     effects.offer(ShowDeleteSearchDialog(longClick.searchListItem, actions))
@@ -144,7 +140,7 @@ class SearchViewModel @Inject constructor(
       Results(searchString, zimSearchResults)
     searchString.isEmpty() && recentSearchResults.isNotEmpty() ->
       Results(searchString, recentSearchResults)
-    else -> Empty
+    else -> NoResults(searchString)
   }
 
   private fun searchResultsFromZimReader() = filter
diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/State.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/State.kt
index bdcd11a9d..2546db534 100644
--- a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/State.kt
+++ b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/State.kt
@@ -21,7 +21,8 @@ package org.kiwix.kiwixmobile.core.search.viewmodel
 import org.kiwix.kiwixmobile.core.search.adapter.SearchListItem
 
 sealed class State {
-  data class Results(val searchString: String, val values: List<SearchListItem>) : State()
-  object Empty : State()
-  object Initialising : State()
+  abstract val searchString: String
+
+  data class Results(override val searchString: String, val values: List<SearchListItem>) : State()
+  data class NoResults(override val searchString: String) : State()
 }
diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt
index b8abd5126..ea2dc8ef8 100644
--- a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt
+++ b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModelTest.kt
@@ -47,8 +47,7 @@ import org.kiwix.kiwixmobile.core.search.viewmodel.Action.OnItemClick
 import org.kiwix.kiwixmobile.core.search.viewmodel.Action.OnItemLongClick
 import org.kiwix.kiwixmobile.core.search.viewmodel.Action.ReceivedPromptForSpeechInput
 import org.kiwix.kiwixmobile.core.search.viewmodel.Action.StartSpeechInputFailed
-import org.kiwix.kiwixmobile.core.search.viewmodel.State.Empty
-import org.kiwix.kiwixmobile.core.search.viewmodel.State.Initialising
+import org.kiwix.kiwixmobile.core.search.viewmodel.State.NoResults
 import org.kiwix.kiwixmobile.core.search.viewmodel.State.Results
 import org.kiwix.kiwixmobile.core.search.viewmodel.effects.DeleteRecentSearch
 import org.kiwix.kiwixmobile.core.search.viewmodel.effects.Finish
@@ -99,7 +98,7 @@ internal class SearchViewModelTest {
   inner class StateTests {
     @Test
     fun `initial state is Initialising`() {
-      viewModel.state.test().assertValue(Initialising)
+      viewModel.state.test().assertValue(NoResults(""))
     }
 
     @Test
@@ -115,13 +114,13 @@ internal class SearchViewModelTest {
     }
 
     @Test
-    fun `non empty search string with no search results is Empty`() {
+    fun `non empty search string with no search results is NoResults`() {
       emissionOf(
         searchTerm = "a",
         searchResults = emptyList(),
         databaseResults = listOf(RecentSearchListItem(""))
       )
-      resultsIn(Empty)
+      resultsIn(NoResults("a"))
     }
 
     @Test
@@ -136,13 +135,13 @@ internal class SearchViewModelTest {
     }
 
     @Test
-    fun `empty search string with no database results is Empty`() {
+    fun `empty search string with no database results is NoResults`() {
       emissionOf(
         searchTerm = "",
         searchResults = listOf(ZimSearchResultListItem("")),
         databaseResults = emptyList()
       )
-      resultsIn(Empty)
+      resultsIn(NoResults(""))
     }
 
     @Test
@@ -157,7 +156,7 @@ internal class SearchViewModelTest {
       viewModel.actions.offer(Filter(searchString))
       viewModel.state.test()
         .also { testScheduler.advanceTimeBy(100, MILLISECONDS) }
-        .assertValueHistory(Initialising, Results(searchString, listOf(item)))
+        .assertValueHistory(NoResults(""), Results(searchString, listOf(item)))
     }
 
     @Test
@@ -175,7 +174,7 @@ internal class SearchViewModelTest {
       )
       viewModel.state.test()
         .also { testScheduler.advanceTimeBy(100, MILLISECONDS) }
-        .assertValueHistory(Initialising, Results("b", listOf(item)))
+        .assertValueHistory(NoResults(""), Results("b", listOf(item)))
     }
   }
 
@@ -205,11 +204,6 @@ internal class SearchViewModelTest {
       )
     }
 
-    @Test
-    fun `ClickedSearchInText in invalid state does nothing`() {
-      actionResultsInEffects(ClickedSearchInText)
-    }
-
     @Test
     fun `ClickedSearchInText in Result state offers SearchInPreviousScreen`() {
       val item = ZimSearchResultListItem("")
@@ -288,8 +282,7 @@ internal class SearchViewModelTest {
     searchResults: List<ZimSearchResultListItem>,
     databaseResults: List<RecentSearchListItem>
   ) {
-    val item =
-      every { searchResultGenerator.generateSearchResults(searchTerm) } returns searchResults
+    every { searchResultGenerator.generateSearchResults(searchTerm) } returns searchResults
     viewModel.actions.offer(Filter(searchTerm))
     recentsFromDb.offer(databaseResults)
   }