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.

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

Fix crash during deck filtering #11880

Merged
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
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
1 change: 1 addition & 0 deletions .idea/dictionaries/usernames.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
<w>Freidle</w>
<w>Goel</w>
<w>Gruber</w>
<w>Guelman</w>
<w>Houssam</w>
<w>Jadhav</w>
<w>Jolta</w>
Expand Down
8 changes: 5 additions & 3 deletions AnkiDroid/src/main/java/com/ichi2/anki/widgets/DeckAdapter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import android.view.View
import android.view.View.OnLongClickListener
import android.view.ViewGroup
import android.widget.*
import androidx.annotation.VisibleForTesting
import androidx.core.content.ContextCompat
import androidx.recyclerview.widget.RecyclerView
import com.ichi2.anki.R
Expand Down Expand Up @@ -343,7 +344,8 @@ class DeckAdapter(private val layoutInflater: LayoutInflater, context: Context)
return DeckFilter()
}

private inner class DeckFilter : TypedFilter<TreeNode<AbstractDeckTreeNode>>(mDeckList) {
@VisibleForTesting
inner class DeckFilter(deckList: List<TreeNode<AbstractDeckTreeNode>> = mDeckList) : TypedFilter<TreeNode<AbstractDeckTreeNode>>(deckList) {
override fun filterResults(constraint: CharSequence, items: List<TreeNode<AbstractDeckTreeNode>>): List<TreeNode<AbstractDeckTreeNode>> {
val filterPattern = constraint.toString().lowercase(Locale.getDefault()).trim { it <= ' ' }
return items.mapNotNull { t: TreeNode<AbstractDeckTreeNode> -> filterDeckInternal(filterPattern, t) }
Expand Down Expand Up @@ -375,13 +377,13 @@ class DeckAdapter(private val layoutInflater: LayoutInflater, context: Context)

// we have a root, and a list of trees with the counts already calculated.
return TreeNode(root.value).apply {
children.addAll(ret)
this.children.addAll(ret)
shaiguelman marked this conversation as resolved.
Show resolved Hide resolved
}
}

private fun containsFilterString(filterPattern: String, root: AbstractDeckTreeNode): Boolean {
val deckName = root.fullDeckName
return deckName.lowercase(Locale.getDefault()).contains(filterPattern) || deckName.lowercase(Locale.ROOT).contains(filterPattern)
return deckName.contains(filterPattern, ignoreCase = true)
shaiguelman marked this conversation as resolved.
Show resolved Hide resolved
david-allison marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
92 changes: 92 additions & 0 deletions AnkiDroid/src/test/java/com/ichi2/anki/DeckAdapterFilterTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/****************************************************************************************
* Copyright (c) 2022 Shai Guelman <[email protected]> *
* *
* This program is free software; you can redistribute it and/or modify it under *
* the terms of the GNU General Public License as published by the Free Software *
* Foundation; either version 3 of the License, or (at your option) any later *
* version. *
* *
* This program is distributed in the hope that it will be useful, but WITHOUT ANY *
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A *
* PARTICULAR PURPOSE. See the GNU General Public License for more details. *
* *
* You should have received a copy of the GNU General Public License along with *
* this program. If not, see <http://www.gnu.org/licenses/>. *
****************************************************************************************/
package com.ichi2.anki

import com.ichi2.anki.widgets.DeckAdapter
import com.ichi2.libanki.sched.AbstractDeckTreeNode
import com.ichi2.libanki.sched.DeckTreeNode
import com.ichi2.libanki.sched.TreeNode
import org.junit.Assert
import org.junit.Before
import org.junit.Test
import org.mockito.Mock
import org.mockito.MockitoAnnotations

class DeckAdapterFilterTest {
david-allison marked this conversation as resolved.
Show resolved Hide resolved

@Mock
private lateinit var adapter: DeckAdapter

private lateinit var filter: DeckAdapter.DeckFilter

@Before
fun setUp() {
MockitoAnnotations.openMocks(this)
filter = adapter.DeckFilter(deckList)
}

@Test
fun verifyFilterResultsReturnsCorrectList() {
val pattern = "Math"

val actual = filter.filterResults(pattern, deckList)
val expected = deckList.getByDids(0, 4, 5, 6, 8)

Assert.assertEquals(expected, actual)
}

@Test
fun verifyFilterResultsReturnsNullWhenForNoMatches() {
val deckList = deckList
val pattern = "geometry"

val actual = filter.filterResults(pattern, deckList)

Assert.assertTrue(actual.isEmpty())
}

private val deckList: MutableList<TreeNode<AbstractDeckTreeNode>>
get() {
val deckList: MutableList<TreeNode<AbstractDeckTreeNode>> = mutableListOf(
TreeNode(DeckTreeNode("Chanson", 0)),
shaiguelman marked this conversation as resolved.
Show resolved Hide resolved
TreeNode(DeckTreeNode("Chanson::A Vers", 1)),
TreeNode(DeckTreeNode("Chanson::A Vers::1", 2)),
TreeNode(DeckTreeNode("Chanson::A Vers::Other", 3)),
TreeNode(DeckTreeNode("Chanson::Math HW", 4)),
TreeNode(DeckTreeNode("Chanson::Math HW::Theory", 5)),
TreeNode(DeckTreeNode("Chanson::Important", 6)),
TreeNode(DeckTreeNode("Chanson::Important::Stuff", 7)),
TreeNode(DeckTreeNode("Chanson::Important::Math", 8)),
TreeNode(DeckTreeNode("Chanson::Important::Stuff::Other Stuff", 9)),
)

deckList.getByDid(0).children.addAll(deckList.getByDids(1, 4, 6))
deckList.getByDid(1).children.addAll(deckList.getByDids(2, 3))
deckList.getByDid(4).children.addAll(deckList.getByDids(5))
deckList.getByDid(6).children.addAll(deckList.getByDids(7, 8))
deckList.getByDid(7).children.addAll(deckList.getByDids(9))

return deckList
}

private fun List<TreeNode<AbstractDeckTreeNode>>.getByDid(did: Long): TreeNode<AbstractDeckTreeNode> {
return this.first { it.value.did == did }
}

private fun List<TreeNode<AbstractDeckTreeNode>>.getByDids(vararg dids: Long): List<TreeNode<AbstractDeckTreeNode>> {
return this.filter { it.value.did in dids }
}
}