From fed91fd6663f3517d204338512434a838f5a9e06 Mon Sep 17 00:00:00 2001 From: cooltey Date: Fri, 13 Sep 2024 12:33:57 -0700 Subject: [PATCH 1/3] Prevent possible crash when showing results for other languages --- .../wikipedia/search/SearchResultsFragment.kt | 23 ++++++++++--------- .../search/SearchResultsViewModel.kt | 20 ++++++---------- 2 files changed, 19 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/org/wikipedia/search/SearchResultsFragment.kt b/app/src/main/java/org/wikipedia/search/SearchResultsFragment.kt index f6a99196c2a..0defb026c9c 100644 --- a/app/src/main/java/org/wikipedia/search/SearchResultsFragment.kt +++ b/app/src/main/java/org/wikipedia/search/SearchResultsFragment.kt @@ -176,31 +176,32 @@ class SearchResultsFragment : Fragment() { private inner class NoSearchResultAdapter : RecyclerView.Adapter() { override fun onBindViewHolder(holder: NoSearchResultItemViewHolder, position: Int) { - holder.bindItem(position, viewModel.resultsCount[position]) + holder.bindItem(viewModel.resultPairList[position]) } override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): NoSearchResultItemViewHolder { return NoSearchResultItemViewHolder(ItemSearchNoResultsBinding.inflate(layoutInflater, parent, false)) } - override fun getItemCount(): Int { return viewModel.resultsCount.size } + override fun getItemCount(): Int { return viewModel.resultPairList.size } } private inner class NoSearchResultItemViewHolder(val itemBinding: ItemSearchNoResultsBinding) : DefaultViewHolder(itemBinding.root) { private val accentColorStateList = getThemedColorStateList(requireContext(), R.attr.progressive_color) private val secondaryColorStateList = getThemedColorStateList(requireContext(), R.attr.secondary_color) - fun bindItem(position: Int, resultsCount: Int) { - if (resultsCount == 0 && viewModel.invokeSource == Constants.InvokeSource.PLACES) { + fun bindItem(resultPair: Pair) { + val langCode = resultPair.first + val resultCount = resultPair.second + if (resultCount == 0 && viewModel.invokeSource == Constants.InvokeSource.PLACES) { PlacesEvent.logAction("no_results_impression", "search_view") } - val langCode = WikipediaApp.instance.languageState.appLanguageCodes[position] - itemBinding.resultsText.text = if (resultsCount == 0) getString(R.string.search_results_count_zero) else resources.getQuantityString(R.plurals.search_results_count, resultsCount, resultsCount) - itemBinding.resultsText.setTextColor(if (resultsCount == 0) secondaryColorStateList else accentColorStateList) - itemBinding.languageCode.visibility = if (viewModel.resultsCount.size == 1) View.GONE else View.VISIBLE + itemBinding.resultsText.text = if (resultCount == 0) getString(R.string.search_results_count_zero) else resources.getQuantityString(R.plurals.search_results_count, resultCount, resultCount) + itemBinding.resultsText.setTextColor(if (resultCount == 0) secondaryColorStateList else accentColorStateList) + itemBinding.languageCode.visibility = if (viewModel.resultPairList.size == 1) View.GONE else View.VISIBLE itemBinding.languageCode.setLangCode(langCode) - itemBinding.languageCode.setTextColor(if (resultsCount == 0) secondaryColorStateList else accentColorStateList) - itemBinding.languageCode.setBackgroundTint(if (resultsCount == 0) secondaryColorStateList else accentColorStateList) - view.isEnabled = resultsCount > 0 + itemBinding.languageCode.setTextColor(if (resultCount == 0) secondaryColorStateList else accentColorStateList) + itemBinding.languageCode.setBackgroundTint(if (resultCount == 0) secondaryColorStateList else accentColorStateList) + view.isEnabled = resultCount > 0 view.setOnClickListener { if (!isAdded) { return@setOnClickListener diff --git a/app/src/main/java/org/wikipedia/search/SearchResultsViewModel.kt b/app/src/main/java/org/wikipedia/search/SearchResultsViewModel.kt index 5093a864b64..23b790e281e 100644 --- a/app/src/main/java/org/wikipedia/search/SearchResultsViewModel.kt +++ b/app/src/main/java/org/wikipedia/search/SearchResultsViewModel.kt @@ -26,20 +26,20 @@ class SearchResultsViewModel : ViewModel() { private val batchSize = 20 private val delayMillis = 200L - var resultsCount = mutableListOf() + var resultPairList = mutableListOf>() var searchTerm: String? = null var languageCode: String? = null lateinit var invokeSource: Constants.InvokeSource @OptIn(FlowPreview::class) // TODO: revisit if the debounce method changed. val searchResultsFlow = Pager(PagingConfig(pageSize = batchSize, initialLoadSize = batchSize)) { - SearchResultsPagingSource(searchTerm, languageCode, resultsCount, invokeSource) + SearchResultsPagingSource(searchTerm, languageCode, resultPairList, invokeSource) }.flow.debounce(delayMillis).cachedIn(viewModelScope) class SearchResultsPagingSource( private val searchTerm: String?, private val languageCode: String?, - private var resultsCount: MutableList?, + private var resultPairList: MutableList>?, private var invokeSource: Constants.InvokeSource ) : PagingSource() { @@ -93,14 +93,12 @@ class SearchResultsViewModel : ViewModel() { } if (resultList.isEmpty() && response?.continuation == null) { - resultsCount?.clear() + resultPairList?.clear() WikipediaApp.instance.languageState.appLanguageCodes.forEach { langCode -> - if (langCode == languageCode) { - resultsCount?.add(0) - } else { + var countResultSize = 0 + if (langCode != languageCode) { val prefixSearchResponse = ServiceFactory.get(WikiSite.forLanguageCode(langCode)) .prefixSearch(searchTerm, params.loadSize, 0) - var countResultSize = 0 prefixSearchResponse.query?.pages?.let { countResultSize = it.size } @@ -109,12 +107,8 @@ class SearchResultsViewModel : ViewModel() { .fullTextSearch(searchTerm, params.loadSize, null) countResultSize = fullTextSearchResponse.query?.pages?.size ?: 0 } - resultsCount?.add(countResultSize) } - } - // make a singleton list if all results are empty. - if (resultsCount?.sum() == 0) { - resultsCount = mutableListOf(0) + resultPairList?.add(langCode to countResultSize) } } From 494edfa8d9b31e991cf4ffabb95440a5b6c8da78 Mon Sep 17 00:00:00 2001 From: cooltey Date: Fri, 13 Sep 2024 12:35:56 -0700 Subject: [PATCH 2/3] Non-null --- .../java/org/wikipedia/search/SearchResultsViewModel.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/wikipedia/search/SearchResultsViewModel.kt b/app/src/main/java/org/wikipedia/search/SearchResultsViewModel.kt index 23b790e281e..dcb62ca2026 100644 --- a/app/src/main/java/org/wikipedia/search/SearchResultsViewModel.kt +++ b/app/src/main/java/org/wikipedia/search/SearchResultsViewModel.kt @@ -39,7 +39,7 @@ class SearchResultsViewModel : ViewModel() { class SearchResultsPagingSource( private val searchTerm: String?, private val languageCode: String?, - private var resultPairList: MutableList>?, + private var resultPairList: MutableList>, private var invokeSource: Constants.InvokeSource ) : PagingSource() { @@ -93,7 +93,7 @@ class SearchResultsViewModel : ViewModel() { } if (resultList.isEmpty() && response?.continuation == null) { - resultPairList?.clear() + resultPairList.clear() WikipediaApp.instance.languageState.appLanguageCodes.forEach { langCode -> var countResultSize = 0 if (langCode != languageCode) { @@ -108,7 +108,7 @@ class SearchResultsViewModel : ViewModel() { countResultSize = fullTextSearchResponse.query?.pages?.size ?: 0 } } - resultPairList?.add(langCode to countResultSize) + resultPairList.add(langCode to countResultSize) } } From dbc72b381697fa013b8d056e232a208a51430a78 Mon Sep 17 00:00:00 2001 From: cooltey Date: Thu, 26 Sep 2024 09:37:31 -0700 Subject: [PATCH 3/3] Rename the variable --- .../java/org/wikipedia/search/SearchResultsFragment.kt | 6 +++--- .../org/wikipedia/search/SearchResultsViewModel.kt | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/org/wikipedia/search/SearchResultsFragment.kt b/app/src/main/java/org/wikipedia/search/SearchResultsFragment.kt index 0defb026c9c..a8157cff7de 100644 --- a/app/src/main/java/org/wikipedia/search/SearchResultsFragment.kt +++ b/app/src/main/java/org/wikipedia/search/SearchResultsFragment.kt @@ -176,14 +176,14 @@ class SearchResultsFragment : Fragment() { private inner class NoSearchResultAdapter : RecyclerView.Adapter() { override fun onBindViewHolder(holder: NoSearchResultItemViewHolder, position: Int) { - holder.bindItem(viewModel.resultPairList[position]) + holder.bindItem(viewModel.countsPerLanguageCode[position]) } override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): NoSearchResultItemViewHolder { return NoSearchResultItemViewHolder(ItemSearchNoResultsBinding.inflate(layoutInflater, parent, false)) } - override fun getItemCount(): Int { return viewModel.resultPairList.size } + override fun getItemCount(): Int { return viewModel.countsPerLanguageCode.size } } private inner class NoSearchResultItemViewHolder(val itemBinding: ItemSearchNoResultsBinding) : DefaultViewHolder(itemBinding.root) { @@ -197,7 +197,7 @@ class SearchResultsFragment : Fragment() { } itemBinding.resultsText.text = if (resultCount == 0) getString(R.string.search_results_count_zero) else resources.getQuantityString(R.plurals.search_results_count, resultCount, resultCount) itemBinding.resultsText.setTextColor(if (resultCount == 0) secondaryColorStateList else accentColorStateList) - itemBinding.languageCode.visibility = if (viewModel.resultPairList.size == 1) View.GONE else View.VISIBLE + itemBinding.languageCode.visibility = if (viewModel.countsPerLanguageCode.size == 1) View.GONE else View.VISIBLE itemBinding.languageCode.setLangCode(langCode) itemBinding.languageCode.setTextColor(if (resultCount == 0) secondaryColorStateList else accentColorStateList) itemBinding.languageCode.setBackgroundTint(if (resultCount == 0) secondaryColorStateList else accentColorStateList) diff --git a/app/src/main/java/org/wikipedia/search/SearchResultsViewModel.kt b/app/src/main/java/org/wikipedia/search/SearchResultsViewModel.kt index dcb62ca2026..a035a1e0e1e 100644 --- a/app/src/main/java/org/wikipedia/search/SearchResultsViewModel.kt +++ b/app/src/main/java/org/wikipedia/search/SearchResultsViewModel.kt @@ -26,20 +26,20 @@ class SearchResultsViewModel : ViewModel() { private val batchSize = 20 private val delayMillis = 200L - var resultPairList = mutableListOf>() + var countsPerLanguageCode = mutableListOf>() var searchTerm: String? = null var languageCode: String? = null lateinit var invokeSource: Constants.InvokeSource @OptIn(FlowPreview::class) // TODO: revisit if the debounce method changed. val searchResultsFlow = Pager(PagingConfig(pageSize = batchSize, initialLoadSize = batchSize)) { - SearchResultsPagingSource(searchTerm, languageCode, resultPairList, invokeSource) + SearchResultsPagingSource(searchTerm, languageCode, countsPerLanguageCode, invokeSource) }.flow.debounce(delayMillis).cachedIn(viewModelScope) class SearchResultsPagingSource( private val searchTerm: String?, private val languageCode: String?, - private var resultPairList: MutableList>, + private var countsPerLanguageCode: MutableList>, private var invokeSource: Constants.InvokeSource ) : PagingSource() { @@ -93,7 +93,7 @@ class SearchResultsViewModel : ViewModel() { } if (resultList.isEmpty() && response?.continuation == null) { - resultPairList.clear() + countsPerLanguageCode.clear() WikipediaApp.instance.languageState.appLanguageCodes.forEach { langCode -> var countResultSize = 0 if (langCode != languageCode) { @@ -108,7 +108,7 @@ class SearchResultsViewModel : ViewModel() { countResultSize = fullTextSearchResponse.query?.pages?.size ?: 0 } } - resultPairList.add(langCode to countResultSize) + countsPerLanguageCode.add(langCode to countResultSize) } }