Fix #1508: Changed string type to int for subtopic-id (#1510)

* Changed string type to int for subtopic-id

* Nit changes

* Nit changes

* Updated test case

Co-authored-by: Rajat Talesra <[email protected]>

app/src/main/java/org/oppia/app/testing/TopicRevisionTestActivity.kt

@@ -19,7 +19,7 @@ class TopicRevisionTestActivity : InjectableAppCompatActivity(), RouteToRevision
     topicRevisionTestActivityPresenter.handleOnCreate()
   }
 
-  override fun routeToRevisionCard(topicId: String, subtopicId: String) {
+  override fun routeToRevisionCard(topicId: String, subtopicId: Int) {
     startActivity(
       RevisionCardActivity.createRevisionCardActivityIntent(
         this,

app/src/main/java/org/oppia/app/testing/TopicTestActivity.kt

@@ -74,7 +74,7 @@ class TopicTestActivity :
     )
   }
 
-  override fun routeToRevisionCard(topicId: String, subtopicId: String) {
+  override fun routeToRevisionCard(topicId: String, subtopicId: Int) {
     startActivity(
       RevisionCardActivity.createRevisionCardActivityIntent(
         this, topicId, subtopicId

app/src/main/java/org/oppia/app/testing/TopicTestActivityForStory.kt

@@ -75,7 +75,7 @@ class TopicTestActivityForStory :
     )
   }
 
-  override fun routeToRevisionCard(topicId: String, subtopicId: String) {
+  override fun routeToRevisionCard(topicId: String, subtopicId: Int) {
     startActivity(
       RevisionCardActivity.createRevisionCardActivityIntent(
         this, topicId, subtopicId

app/src/main/java/org/oppia/app/topic/RouteToRevisionCardListener.kt

@@ -2,5 +2,5 @@ package org.oppia.app.topic
 
 /** Listener for when an [TopicActivity] should route to a [RevisionCardFragment]. */
 interface RouteToRevisionCardListener {
-  fun routeToRevisionCard(topicId: String, subtopicId: String)
+  fun routeToRevisionCard(topicId: String, subtopicId: Int)
 }

app/src/main/java/org/oppia/app/topic/TopicActivity.kt

@@ -62,7 +62,7 @@ class TopicActivity :
     )
   }
 
-  override fun routeToRevisionCard(topicId: String, subtopicId: String) {
+  override fun routeToRevisionCard(topicId: String, subtopicId: Int) {
     startActivity(
       RevisionCardActivity.createRevisionCardActivityIntent(
         this,

app/src/main/java/org/oppia/app/topic/practice/SubtopicSelector.kt

@@ -3,8 +3,8 @@ package org.oppia.app.topic.practice
 /** Interface to update the selectedSubtopicList in [TopicPracticeFragmentPresenter]. */
 interface SubtopicSelector {
   /** This subtopic and skill will get added to selectedSubtopicList in [TopicPracticeFragmentPresenter]. */
-  fun subtopicSelected(subtopicId: String, skillIdList: MutableList<String>)
+  fun subtopicSelected(subtopicId: Int, skillIdList: MutableList<String>)
 
   /** This subtopic and skill will get removed from selectedSubtopicList in [TopicPracticeFragmentPresenter]. */
-  fun subtopicUnselected(subtopicId: String, skillIdList: MutableList<String>)
+  fun subtopicUnselected(subtopicId: Int, skillIdList: MutableList<String>)
 }

app/src/main/java/org/oppia/app/topic/practice/TopicPracticeFragment.kt

@@ -10,11 +10,11 @@ import org.oppia.app.topic.PROFILE_ID_ARGUMENT_KEY
 import org.oppia.app.topic.TOPIC_ID_ARGUMENT_KEY
 import javax.inject.Inject
 
-private const val KEY_SKILL_ID_LIST = "SKILL_ID_LIST"
-
 /** Fragment that displays skills for topic practice mode. */
 class TopicPracticeFragment : InjectableFragment() {
   companion object {
+    internal const val SUBTOPIC_ID_LIST_ARGUMENT_KEY = "SUBTOPIC_ID_LIST_ARGUMENT_KEY"
+
     /** Returns a new [TopicPracticeFragment]. */
     fun newInstance(internalProfileId: Int, topicId: String): TopicPracticeFragment {
       val topicPracticeFragment = TopicPracticeFragment()
@@ -39,9 +39,9 @@ class TopicPracticeFragment : InjectableFragment() {
     container: ViewGroup?,
     savedInstanceState: Bundle?
   ): View? {
-    var selectedIdList = ArrayList<String>()
+    var selectedIdList = ArrayList<Int>()
     if (savedInstanceState != null) {
-      selectedIdList = savedInstanceState.getStringArrayList(KEY_SKILL_ID_LIST)
+      selectedIdList = savedInstanceState.getIntegerArrayList(SUBTOPIC_ID_LIST_ARGUMENT_KEY)!!
     }
     val internalProfileId = arguments?.getInt(PROFILE_ID_ARGUMENT_KEY, -1)!!
     val topicId = checkNotNull(arguments?.getString(TOPIC_ID_ARGUMENT_KEY)) {
@@ -58,9 +58,9 @@ class TopicPracticeFragment : InjectableFragment() {
 
   override fun onSaveInstanceState(outState: Bundle) {
     super.onSaveInstanceState(outState)
-    outState.putStringArrayList(
-      KEY_SKILL_ID_LIST,
-      topicPracticeFragmentPresenter.selectedSkillIdList
+    outState.putIntegerArrayList(
+      SUBTOPIC_ID_LIST_ARGUMENT_KEY,
+      topicPracticeFragmentPresenter.selectedSubtopicIdList
     )
   }
 }

app/src/main/java/org/oppia/app/topic/practice/TopicPracticeFragmentPresenter.kt

@@ -18,9 +18,7 @@ import org.oppia.app.topic.practice.practiceitemviewmodel.TopicPracticeHeaderVie
 import org.oppia.app.topic.practice.practiceitemviewmodel.TopicPracticeItemViewModel
 import org.oppia.app.topic.practice.practiceitemviewmodel.TopicPracticeSubtopicViewModel
 import org.oppia.app.viewmodel.ViewModelProvider
-import org.oppia.domain.oppialogger.analytics.AnalyticsController
 import org.oppia.util.logging.ConsoleLogger
-import org.oppia.util.system.OppiaClock
 import javax.inject.Inject
 
 /** The presenter for [TopicPracticeFragment]. */
@@ -29,22 +27,20 @@ class TopicPracticeFragmentPresenter @Inject constructor(
   private val activity: AppCompatActivity,
   private val fragment: Fragment,
   private val logger: ConsoleLogger,
-  private val analyticsController: AnalyticsController,
-  private val oppiaClock: OppiaClock,
   private val viewModelProvider: ViewModelProvider<TopicPracticeViewModel>
 ) : SubtopicSelector {
   private lateinit var binding: TopicPracticeFragmentBinding
   private lateinit var linearLayoutManager: LinearLayoutManager
-  lateinit var selectedSkillIdList: ArrayList<String>
-  private var skillIdHashMap = HashMap<String, MutableList<String>>()
+  lateinit var selectedSubtopicIdList: ArrayList<Int>
+  private var skillIdHashMap = HashMap<Int, MutableList<String>>()
   private lateinit var topicId: String
   private lateinit var topicPracticeFooterViewBinding: TopicPracticeFooterViewBinding
   private val routeToQuestionPlayerListener = activity as RouteToQuestionPlayerListener
 
   fun handleCreateView(
     inflater: LayoutInflater,
     container: ViewGroup?,
-    skillList: ArrayList<String>,
+    subtopicList: ArrayList<Int>,
     internalProfileId: Int,
     topicId: String
   ): View? {
@@ -53,7 +49,7 @@ class TopicPracticeFragmentPresenter @Inject constructor(
     viewModel.setTopicId(this.topicId)
     viewModel.setInternalProfileId(internalProfileId)
 
-    selectedSkillIdList = skillList
+    selectedSubtopicIdList = subtopicList
     binding = TopicPracticeFragmentBinding.inflate(
       inflater,
       container,
@@ -110,7 +106,7 @@ class TopicPracticeFragmentPresenter @Inject constructor(
     model: TopicPracticeSubtopicViewModel
   ) {
     binding.viewModel = model
-    binding.isChecked = selectedSkillIdList.contains(model.subtopic.subtopicId)
+    binding.isChecked = selectedSubtopicIdList.contains(model.subtopic.subtopicId)
     binding.subtopicCheckBox.setOnCheckedChangeListener { _, isChecked ->
       if (isChecked) {
         subtopicSelected(model.subtopic.subtopicId, model.subtopic.skillIdsList)
@@ -126,7 +122,7 @@ class TopicPracticeFragmentPresenter @Inject constructor(
   ) {
     topicPracticeFooterViewBinding = binding
     binding.viewModel = model
-    binding.isSubmitButtonActive = selectedSkillIdList.isNotEmpty()
+    binding.isSubmitButtonActive = selectedSubtopicIdList.isNotEmpty()
     binding.topicPracticeStartButton.setOnClickListener {
       val skillIdList = ArrayList(skillIdHashMap.values)
       logger.d("TopicPracticeFragmentPresenter", "Skill Ids = " + skillIdList.flatten())
@@ -146,20 +142,20 @@ class TopicPracticeFragmentPresenter @Inject constructor(
     VIEW_TYPE_FOOTER
   }
 
-  override fun subtopicSelected(subtopicId: String, skillIdList: MutableList<String>) {
-    if (!selectedSkillIdList.contains(subtopicId)) {
-      selectedSkillIdList.add(subtopicId)
-      skillIdHashMap.put(subtopicId, skillIdList)
+  override fun subtopicSelected(subtopicId: Int, skillIdList: MutableList<String>) {
+    if (!selectedSubtopicIdList.contains(subtopicId)) {
+      selectedSubtopicIdList.add(subtopicId)
+      skillIdHashMap[subtopicId] = skillIdList
     }
 
     if (::topicPracticeFooterViewBinding.isInitialized) {
       topicPracticeFooterViewBinding.isSubmitButtonActive = skillIdHashMap.isNotEmpty()
     }
   }
 
-  override fun subtopicUnselected(subtopicId: String, skillIdList: MutableList<String>) {
-    if (selectedSkillIdList.contains(subtopicId)) {
-      selectedSkillIdList.remove(subtopicId)
+  override fun subtopicUnselected(subtopicId: Int, skillIdList: MutableList<String>) {
+    if (selectedSubtopicIdList.contains(subtopicId)) {
+      selectedSubtopicIdList.remove(subtopicId)
       skillIdHashMap.remove(subtopicId)
     }
     if (::topicPracticeFooterViewBinding.isInitialized) {

app/src/main/java/org/oppia/app/topic/revisioncard/RevisionCardActivity.kt

@@ -6,9 +6,6 @@ import android.os.Bundle
 import org.oppia.app.activity.InjectableAppCompatActivity
 import javax.inject.Inject
 
-const val TOPIC_ID_ARGUMENT_KEY = "TOPIC_ID_"
-const val SUBTOPIC_ID_ARGUMENT_KEY = "SUBTOPIC_ID"
-
 /** Activity for revision card. */
 class RevisionCardActivity : InjectableAppCompatActivity(), ReturnToTopicClickListener {
 
@@ -18,19 +15,28 @@ class RevisionCardActivity : InjectableAppCompatActivity(), ReturnToTopicClickLi
   override fun onCreate(savedInstanceState: Bundle?) {
     super.onCreate(savedInstanceState)
     activityComponent.inject(this)
-    revisionCardActivityPresenter.handleOnCreate()
+    intent?.let { intent ->
+      val topicId = checkNotNull(intent.getStringExtra(TOPIC_ID_EXTRA_KEY)) {
+        "Expected topic ID to be included in intent for RevisionCardActivity."
+      }
+      val subtopicId = intent.getIntExtra(SUBTOPIC_ID_EXTRA_KEY, -1)
+      revisionCardActivityPresenter.handleOnCreate(topicId, subtopicId)
+    }
   }
 
   companion object {
+    internal const val TOPIC_ID_EXTRA_KEY = "TOPIC_ID_EXTRA_KEY"
+    internal const val SUBTOPIC_ID_EXTRA_KEY = "SUBTOPIC_ID_EXTRA_KEY"
+
     /** Returns a new [Intent] to route to [RevisionCardActivity]. */
     fun createRevisionCardActivityIntent(
       context: Context,
       topicId: String,
-      subtopicId: String
+      subtopicId: Int
     ): Intent {
       val intent = Intent(context, RevisionCardActivity::class.java)
-      intent.putExtra(TOPIC_ID_ARGUMENT_KEY, topicId)
-      intent.putExtra(SUBTOPIC_ID_ARGUMENT_KEY, subtopicId)
+      intent.putExtra(TOPIC_ID_EXTRA_KEY, topicId)
+      intent.putExtra(SUBTOPIC_ID_EXTRA_KEY, subtopicId)
       return intent
     }
   }

app/src/main/java/org/oppia/app/topic/revisioncard/RevisionCardActivityPresenter.kt

@@ -8,12 +8,12 @@ import javax.inject.Inject
 /** The presenter for [RevisionCardActivity]. */
 @ActivityScope
 class RevisionCardActivityPresenter @Inject constructor(private val activity: AppCompatActivity) {
-  fun handleOnCreate() {
+  fun handleOnCreate(topicId: String, subtopicId: Int) {
     activity.setContentView(R.layout.revision_card_activity)
     if (getReviewCardFragment() == null) {
       activity.supportFragmentManager.beginTransaction().add(
         R.id.revision_card_fragment_placeholder,
-        RevisionCardFragment()
+        RevisionCardFragment.newInstance(topicId, subtopicId)
       ).commitNow()
     }
   }

app/src/main/java/org/oppia/app/topic/revisioncard/RevisionCardFragment.kt

@@ -10,6 +10,20 @@ import javax.inject.Inject
 
 /* Fragment that displays revision card */
 class RevisionCardFragment : InjectableDialogFragment() {
+  companion object {
+    internal const val TOPIC_ID_ARGUMENT_KEY = "TOPIC_ID_ARGUMENT_KEY"
+    internal const val SUBTOPIC_ID_ARGUMENT_KEY = "SUBOPIC_ID_ARGUMENT_KEY"
+
+    /** Returns a new [RevisionCardFragment] to display the subtopic content.. */
+    fun newInstance(topicId: String, subtopicId: Int): RevisionCardFragment {
+      val revisionCardFragment = RevisionCardFragment()
+      val args = Bundle()
+      args.putString(TOPIC_ID_ARGUMENT_KEY, topicId)
+      args.putInt(SUBTOPIC_ID_ARGUMENT_KEY, subtopicId)
+      revisionCardFragment.arguments = args
+      return revisionCardFragment
+    }
+  }
 
   @Inject
   lateinit var revisionCardFragmentPresenter: RevisionCardFragmentPresenter
@@ -25,6 +39,14 @@ class RevisionCardFragment : InjectableDialogFragment() {
     savedInstanceState: Bundle?
   ): View? {
     super.onCreateView(inflater, container, savedInstanceState)
-    return revisionCardFragmentPresenter.handleCreateView(inflater, container)
+    val args = checkNotNull(arguments) {
+      "Expected arguments to be passed to StoryFragment"
+    }
+    val topicId =
+      checkNotNull(args.getString(TOPIC_ID_ARGUMENT_KEY)) {
+        "Expected topicId to be passed to RevisionCardFragment"
+      }
+    val subtopicId = args.getInt(SUBTOPIC_ID_ARGUMENT_KEY, -1)
+    return revisionCardFragmentPresenter.handleCreateView(inflater, container, topicId, subtopicId)
   }
 }

app/src/main/java/org/oppia/app/topic/revisioncard/RevisionCardFragmentPresenter.kt

@@ -12,18 +12,21 @@ import org.oppia.domain.oppialogger.analytics.AnalyticsController
 import org.oppia.util.system.OppiaClock
 import javax.inject.Inject
 
-/** Presenter for [RevisionCardFragment], sets up bindings from ViewModel */
+/** Presenter for [RevisionCardFragment], sets up bindings from ViewModel. */
 @FragmentScope
 class RevisionCardFragmentPresenter @Inject constructor(
   private val fragment: Fragment,
   private val analyticsController: AnalyticsController,
   private val oppiaClock: OppiaClock,
   private val viewModelProvider: ViewModelProvider<RevisionCardViewModel>
 ) {
-  private lateinit var topicId: String
-  private lateinit var subtopicId: String
 
-  fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? {
+  fun handleCreateView(
+    inflater: LayoutInflater,
+    container: ViewGroup?,
+    topicId: String,
+    subtopicId: Int
+  ): View? {
     val binding =
       RevisionCardFragmentBinding.inflate(
         inflater,
@@ -32,9 +35,6 @@ class RevisionCardFragmentPresenter @Inject constructor(
       )
     val viewModel = getReviewCardViewModel()
 
-    topicId = fragment.activity!!.intent.getStringExtra(TOPIC_ID_ARGUMENT_KEY)
-    subtopicId = fragment.activity!!.intent.getStringExtra(SUBTOPIC_ID_ARGUMENT_KEY)
-
     viewModel.setSubtopicIdAndBinding(topicId, subtopicId, binding)
     logRevisionCardEvent(topicId, subtopicId)
 
@@ -49,7 +49,7 @@ class RevisionCardFragmentPresenter @Inject constructor(
     return viewModelProvider.getForFragment(fragment, RevisionCardViewModel::class.java)
   }
 
-  private fun logRevisionCardEvent(topicId: String, subTopicId: String) {
+  private fun logRevisionCardEvent(topicId: String, subTopicId: Int) {
     analyticsController.logTransitionEvent(
       oppiaClock.getCurrentCalendar().timeInMillis,
       EventLog.EventAction.OPEN_REVISION_CARD,

app/src/main/java/org/oppia/app/topic/revisioncard/RevisionCardViewModel.kt

@@ -27,7 +27,7 @@ class RevisionCardViewModel @Inject constructor(
   @RevisionCardHtmlParserEntityType private val entityType: String
 ) : ViewModel() {
   private lateinit var topicId: String
-  private lateinit var subtopicId: String
+  private var subtopicId: Int = 0
   private lateinit var binding: RevisionCardFragmentBinding
   private val returnToTopicClickListener: ReturnToTopicClickListener =
     activity as ReturnToTopicClickListener
@@ -42,10 +42,14 @@ class RevisionCardViewModel @Inject constructor(
     returnToTopicClickListener.onReturnToTopicClicked()
   }
 
-  /** Sets the value of subtopicId and binding. Must be called before setting ViewModel to binding. */
-  fun setSubtopicIdAndBinding(topicId: String, id: String, binding: RevisionCardFragmentBinding) {
-    subtopicId = id
+  /** Sets the value of topicId, subtopicId and binding before anything else. */
+  fun setSubtopicIdAndBinding(
+    topicId: String,
+    subtopicId: Int,
+    binding: RevisionCardFragmentBinding
+  ) {
     this.topicId = topicId
+    this.subtopicId = subtopicId
     this.binding = binding
   }
 
@@ -72,7 +76,7 @@ class RevisionCardViewModel @Inject constructor(
     )
     subtopicTitle = revisionCard.subtopicTitle
     return htmlParserFactory.create(
-      resourceBucketName, entityType, subtopicId, /* imageCenterAlign= */ true
+      resourceBucketName, entityType, topicId, /* imageCenterAlign= */ true
     ).parseOppiaHtml(revisionCard.pageContents.html, binding.revisionCardExplanationText)
   }
 }

app/src/sharedTest/java/org/oppia/app/topic/revision/TopicRevisionFragmentTest.kt

@@ -100,7 +100,7 @@ class TopicRevisionFragmentTest {
           isDescendantOfA(withId(R.id.topic_tabs_container))
         )
       ).perform(click())
-      onView(atPosition(R.id.revision_recycler_view, 0)).perform(click())
+      onView(atPosition(R.id.revision_recycler_view, 1)).perform(click())
       onView(withId(R.id.revision_card_explanation_text))
         .check(
           matches(