Skip to content

Commit 3763c01

Browse files
thedroiddivthedroiddiv
authored
fix: insert field adds fields to the wrong card type
Fixes: #19304 - When attaching the InsertFieldDialog result listener to CardTemplateFragment, same key (REQUEST_FIELD_INSERT) was being used - the returned result from dialog was only sent to the last fragment created (as a new listener is attached with the same keys used by previous cards) - Fixed by adding unique key for each card fCardTemplateFragment - When inflating the result dialog, send the CARD_INDEX - When attaching listener and returning result from dialog, append the CARD_INDEX to ensure unique key for each card
1 parent e896602 commit 3763c01

File tree

3 files changed

+122
-17
lines changed

3 files changed

+122
-17
lines changed

‎AnkiDroid/src/main/java/com/ichi2/anki/CardTemplateEditor.kt‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ import com.ichi2.anki.dialogs.DeckSelectionDialog
7676
import com.ichi2.anki.dialogs.DeckSelectionDialog.DeckSelectionListener
7777
import com.ichi2.anki.dialogs.DiscardChangesDialog
7878
import com.ichi2.anki.dialogs.InsertFieldDialog
79-
import com.ichi2.anki.dialogs.InsertFieldDialog.Companion.REQUEST_FIELD_INSERT
8079
import com.ichi2.anki.libanki.CardTemplates
8180
import com.ichi2.anki.libanki.Collection
8281
import com.ichi2.anki.libanki.Note
@@ -522,6 +521,13 @@ open class CardTemplateEditor :
522521
private val refreshFragmentHandler = Handler(Looper.getMainLooper())
523522
private lateinit var editorEditText: FixedEditText
524523

524+
// Index of this card template fragment in ViewPager
525+
private val cardIndex
526+
get() = requireArguments().getInt(CARD_INDEX)
527+
528+
val insertFieldRequestKey
529+
get() = "request_field_insert_$cardIndex"
530+
525531
var currentEditorViewId = 0
526532

527533
private lateinit var templateEditor: CardTemplateEditor
@@ -536,7 +542,6 @@ open class CardTemplateEditor :
536542
// Storing a reference to the templateEditor allows us to use member variables
537543
templateEditor = activity as CardTemplateEditor
538544
val mainView = inflater.inflate(R.layout.card_template_editor_item, container, false)
539-
val cardIndex = requireArguments().getInt(CARD_INDEX)
540545
tempModel = templateEditor.tempNoteType!!
541546
// Load template
542547
val template: BackendCardTemplate =
@@ -729,7 +734,8 @@ open class CardTemplateEditor :
729734
)
730735
fun showInsertFieldDialog() {
731736
templateEditor.fieldNames?.let { fieldNames ->
732-
templateEditor.showDialogFragment(InsertFieldDialog.newInstance(fieldNames))
737+
val dialog = InsertFieldDialog.newInstance(fieldNames, insertFieldRequestKey)
738+
templateEditor.showDialogFragment(dialog)
733739
}
734740
}
735741

@@ -810,11 +816,10 @@ open class CardTemplateEditor :
810816
}
811817
},
812818
)
813-
parentFragmentManager.setFragmentResultListener(REQUEST_FIELD_INSERT, viewLifecycleOwner) { key, bundle ->
814-
if (key == REQUEST_FIELD_INSERT) {
815-
// this is guaranteed to be non null, as we put a non null value on the other side
816-
insertField(bundle.getString(InsertFieldDialog.KEY_INSERTED_FIELD)!!)
817-
}
819+
820+
parentFragmentManager.setFragmentResultListener(insertFieldRequestKey, viewLifecycleOwner) { key, bundle ->
821+
// this is guaranteed to be non null, as we put a non null value on the other side
822+
insertField(bundle.getString(InsertFieldDialog.KEY_INSERTED_FIELD)!!)
818823
}
819824
setupMenu()
820825
}

‎AnkiDroid/src/main/java/com/ichi2/anki/dialogs/InsertFieldDialog.kt‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,15 @@ import com.ichi2.utils.title
3838
*/
3939
class InsertFieldDialog : DialogFragment() {
4040
private lateinit var fieldList: List<String>
41+
private lateinit var requestKey: String
4142

4243
/**
4344
* A dialog for inserting field in card template editor
4445
*/
4546
override fun onCreateDialog(savedInstanceState: Bundle?): AlertDialog {
4647
super.onCreate(savedInstanceState)
4748
fieldList = requireArguments().getStringArrayList(KEY_FIELD_ITEMS)!!
49+
requestKey = requireArguments().getString(KEY_REQUEST_KEY)!!
4850
val adapter: RecyclerView.Adapter<*> =
4951
object : RecyclerView.Adapter<RecyclerView.ViewHolder>() {
5052
override fun onCreateViewHolder(
@@ -75,28 +77,37 @@ class InsertFieldDialog : DialogFragment() {
7577

7678
private fun selectFieldAndClose(textView: TextView) {
7779
parentFragmentManager.setFragmentResult(
78-
REQUEST_FIELD_INSERT,
80+
requestKey,
7981
bundleOf(KEY_INSERTED_FIELD to textView.text.toString()),
8082
)
8183
dismiss()
8284
}
8385

8486
companion object {
85-
/**
86-
* Other fragments sharing the activity can use this with
87-
* [androidx.fragment.app.FragmentManager.setFragmentResultListener] to get a result back.
88-
*/
89-
const val REQUEST_FIELD_INSERT = "request_field_insert"
90-
9187
/**
9288
* This fragment requires that a list of fields names to be passed in.
9389
*/
9490
const val KEY_INSERTED_FIELD = "key_inserted_field"
9591
private const val KEY_FIELD_ITEMS = "key_field_items"
92+
private const val KEY_REQUEST_KEY = "key_request_key"
9693

97-
fun newInstance(fieldItems: List<String>): InsertFieldDialog =
94+
/**
95+
* Creates a new instance of [InsertFieldDialog]
96+
*
97+
* @param fieldItems The list of field names to be displayed in the dialog.
98+
* @param requestKey The key used to identify the result when returning the selected field
99+
* to the calling fragment.
100+
*/
101+
fun newInstance(
102+
fieldItems: List<String>,
103+
requestKey: String,
104+
): InsertFieldDialog =
98105
InsertFieldDialog().apply {
99-
arguments = bundleOf(KEY_FIELD_ITEMS to ArrayList(fieldItems))
106+
arguments =
107+
bundleOf(
108+
KEY_FIELD_ITEMS to ArrayList(fieldItems),
109+
KEY_REQUEST_KEY to requestKey,
110+
)
100111
}
101112
}
102113
}

‎AnkiDroid/src/test/java/com/ichi2/anki/CardTemplateEditorTest.kt‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import android.widget.EditText
2424
import androidx.test.ext.junit.runners.AndroidJUnit4
2525
import com.ichi2.anki.CardTemplateEditor.CardTemplateFragment.CardTemplate
2626
import com.ichi2.anki.CollectionManager.withCol
27+
import com.ichi2.anki.dialogs.InsertFieldDialog
2728
import com.ichi2.anki.libanki.NotetypeJson
2829
import com.ichi2.anki.libanki.testutils.ext.addNote
2930
import com.ichi2.anki.model.SelectableDeck
@@ -752,6 +753,94 @@ class CardTemplateEditorTest : RobolectricTest() {
752753
assumeThat(cardTemplateFragment.currentEditorViewId, Matchers.equalTo(R.id.styling_edit))
753754
}
754755

756+
@Test
757+
fun testInsertFieldInCorrectFragmentAfterNavigation() {
758+
val noteTypeName = "Basic (and reversed card)"
759+
760+
// Start the CardTemplateEditor with a note type that has multiple templates
761+
val collectionBasicNoteTypeOriginal = getCurrentDatabaseNoteTypeCopy(noteTypeName)
762+
val intent = Intent(Intent.ACTION_VIEW)
763+
intent.putExtra("noteTypeId", collectionBasicNoteTypeOriginal.id)
764+
val templateEditorController =
765+
Robolectric
766+
.buildActivity(CardTemplateEditor::class.java, intent)
767+
.create()
768+
.start()
769+
.resume()
770+
.visible()
771+
saveControllerForCleanup(templateEditorController)
772+
val testEditor = templateEditorController.get()
773+
774+
assertEquals("Note type should have 2 templates", 2, testEditor.tempNoteType?.templateCount)
775+
776+
// Start on first fragment (Card 1)
777+
assertEquals("Should start on first fragment", 0, testEditor.viewPager.currentItem)
778+
advanceRobolectricLooper()
779+
780+
val firstFragment = testEditor.currentFragment
781+
assertNotNull("First fragment should exist", firstFragment)
782+
val firstTemplateEditText = testEditor.findViewById<EditText>(R.id.editor_editText)
783+
val originalFirstContent = firstTemplateEditText.text.toString()
784+
785+
// Navigate to second fragment (Card 2)
786+
testEditor.viewPager.currentItem = 1
787+
advanceRobolectricLooper()
788+
789+
val secondFragment = testEditor.currentFragment
790+
assertNotNull("Second fragment should exist", secondFragment)
791+
val secondTemplateEditText = testEditor.findViewById<EditText>(R.id.editor_editText)
792+
val originalSecondContent = secondTemplateEditText.text.toString()
793+
794+
// Navigate back to first fragment
795+
testEditor.viewPager.currentItem = 0
796+
advanceRobolectricLooper()
797+
798+
val firstFragmentAgain = testEditor.currentFragment
799+
assertNotNull("First fragment should exist after navigation back", firstFragmentAgain)
800+
assertEquals("Should be back on first fragment", 0, testEditor.viewPager.currentItem)
801+
802+
// Insert a field into the first fragment
803+
val fieldToInsert = "Front"
804+
val expectedFieldText = "{{$fieldToInsert}}"
805+
806+
// Simulate showing the insert field dialog and selecting a field
807+
firstFragmentAgain!!.showInsertFieldDialog()
808+
advanceRobolectricLooper()
809+
810+
val resultBundle = Bundle()
811+
resultBundle.putString(InsertFieldDialog.KEY_INSERTED_FIELD, fieldToInsert)
812+
testEditor.supportFragmentManager.setFragmentResult(firstFragmentAgain.insertFieldRequestKey, resultBundle)
813+
advanceRobolectricLooper()
814+
815+
// Verify the field was inserted into the first fragment
816+
val updatedFirstContent = firstTemplateEditText.text.toString()
817+
assertTrue(
818+
"Field should be inserted into first fragment",
819+
updatedFirstContent.contains(expectedFieldText),
820+
)
821+
assertTrue(
822+
"First fragment content should have changed",
823+
updatedFirstContent != originalFirstContent,
824+
)
825+
826+
// Navigate to second fragment to verify it wasn't modified
827+
testEditor.viewPager.currentItem = 1
828+
advanceRobolectricLooper()
829+
830+
val secondTemplateEditTextAfter = testEditor.findViewById<EditText>(R.id.editor_editText)
831+
val secondContentAfter = secondTemplateEditTextAfter.text.toString()
832+
833+
assertEquals(
834+
"Second fragment should not have been modified",
835+
originalSecondContent,
836+
secondContentAfter,
837+
)
838+
assertFalse(
839+
"Field should NOT be in second fragment",
840+
secondContentAfter.contains(expectedFieldText),
841+
)
842+
}
843+
755844
private fun addCardType(
756845
testEditor: CardTemplateEditor,
757846
shadowTestEditor: ShadowActivity,

0 commit comments

Comments
 (0)