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 part of #5344: Implement event logs for multiple classrooms #5456

Merged
merged 23 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.model.RecentlyPlayedActivityParams
import org.oppia.android.app.model.RecentlyPlayedActivityTitle
import org.oppia.android.app.model.ScreenName.CLASSROOM_LIST_ACTIVITY
import org.oppia.android.app.topic.TopicActivity
import org.oppia.android.app.topic.TopicActivity.Companion.createTopicActivityIntent
import org.oppia.android.app.topic.TopicActivity.Companion.createTopicPlayStoryActivityIntent
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.util.logging.CurrentAppScreenNameIntentDecorator.decorateWithScreenName
import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.decorateWithUserProfileId
Expand Down Expand Up @@ -100,7 +101,7 @@ class ClassroomListActivity :

override fun routeToTopic(internalProfileId: Int, classroomId: String, topicId: String) {
startActivity(
TopicActivity.createTopicActivityIntent(this, internalProfileId, classroomId, topicId)
createTopicActivityIntent(this, internalProfileId, classroomId, topicId)
)
}

Expand All @@ -111,7 +112,7 @@ class ClassroomListActivity :
storyId: String
) {
startActivity(
TopicActivity.createTopicPlayStoryActivityIntent(
createTopicPlayStoryActivityIntent(
this,
internalProfileId,
classroomId,
adhiamboperes marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,23 @@ class StoryFragment : InjectableFragment(), ExplorationSelectionListener, StoryF
savedInstanceState: Bundle?
): View? {
val arguments = checkNotNull(arguments) {
"Expected arguments to be passed to StoryFragment"
"Expected arguments to be passed to StoryFragment."
}
val args =
arguments.getProto(STORY_FRAGMENT_ARGUMENTS_KEY, StoryFragmentArguments.getDefaultInstance())

val internalProfileId = arguments.extractCurrentUserProfileId().internalId
val classroomId =
checkNotNull(args.classroomId) {
"Expected classroomId to be passed to StoryFragment"
"Expected classroomId to be passed to StoryFragment."
}
val topicId =
checkNotNull(args.topicId) {
"Expected topicId to be passed to StoryFragment"
"Expected topicId to be passed to StoryFragment."
}
val storyId =
checkNotNull(args.storyId) {
"Expected storyId to be passed to StoryFragment"
"Expected storyId to be passed to StoryFragment."
}
return storyFragmentPresenter.handleCreateView(
inflater,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,10 @@ class TopicActivity :
TOPIC_ACTIVITY_PARAMS_KEY,
TopicActivityParams.getDefaultInstance()
)
classroomId = checkNotNull(
args?.classroomId
) {
classroomId = checkNotNull(args?.classroomId) {
"Expected classroom ID to be included in intent for TopicActivity."
}
topicId = checkNotNull(
args?.topicId
) {
topicId = checkNotNull(args?.topicId) {
"Expected topic ID to be included in intent for TopicActivity."
}
storyId = args?.storyId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionMo
import org.oppia.android.app.player.state.itemviewmodel.StateItemViewModel
import org.oppia.android.app.recyclerview.RecyclerViewMatcher.Companion.atPositionOnView
import org.oppia.android.app.shim.ViewBindingShimModule
import org.oppia.android.app.testing.ExplorationInjectionActivity
import org.oppia.android.app.translation.testing.ActivityRecreatorTestModule
import org.oppia.android.app.utility.EspressoTestsMatchers.hasProtoExtra
import org.oppia.android.app.utility.EspressoTestsMatchers.withDrawable
Expand Down Expand Up @@ -251,27 +250,6 @@ class ExplorationActivityTest {
ApplicationProvider.getApplicationContext<TestApplication>().inject(this)
}

private fun getApplicationDependencies(
internalProfileId: Int,
classroomId: String,
topicId: String,
storyId: String,
explorationId: String
) {
launch(ExplorationInjectionActivity::class.java).use {
it.onActivity { activity ->
explorationDataController = activity.explorationDataController
explorationDataController.startPlayingNewExploration(
internalProfileId,
classroomId,
topicId,
storyId,
explorationId
)
}
}
}

// TODO(#388): Fill in remaining tests for this activity.
@get:Rule
var explorationActivityTestRule: ActivityTestRule<ExplorationActivity> = ActivityTestRule(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,20 @@ class ClassroomController @Inject constructor(
}
}

/**
* Returns the classroomId of the classroom to which the topic with the given [topicId]
* belongs to.
*/
fun getClassroomIdByTopicId(topicId: String): String {
var classroomId = ""
loadClassrooms().forEach {
if (it.topicPrerequisitesMap.keys.contains(topicId)) {
classroomId = it.id
}
}
return classroomId
}

private fun createClassroomList(
contentLocale: OppiaLocale.ContentLocale
): ClassroomList {
Expand Down Expand Up @@ -318,6 +332,81 @@ class ClassroomController @Inject constructor(
.setFirstStoryId(firstStoryId)
.build()
}

private fun loadClassrooms(): List<ClassroomRecord> {
return if (loadLessonProtosFromAssets) {
assetRepository.loadProtoFromLocalAssets(
assetName = "classrooms",
baseMessage = ClassroomIdList.getDefaultInstance()
).classroomIdsList.map { classroomId ->
loadClassroomById(classroomId)
}
} else loadClassroomsFromJson()
}

private fun loadClassroomsFromJson(): List<ClassroomRecord> {
// Load the classrooms.json file.
val classroomIdsObj = jsonAssetRetriever.loadJsonFromAsset("classrooms.json")
checkNotNull(classroomIdsObj) { "Failed to load classrooms.json." }
val classroomIds = classroomIdsObj.optJSONArray("classroom_id_list")
checkNotNull(classroomIds) { "classrooms.json is missing classroom IDs." }

// Initialize a list to store the [ClassroomRecord]s.
val classroomRecords = mutableListOf<ClassroomRecord>()

// Iterate over all classroomIds and load each classroom's JSON.
for (i in 0 until classroomIds.length()) {
val classroomId = checkNotNull(classroomIds.optString(i)) {
"Expected non-null classroom ID at index $i."
}
val classroomRecord = loadClassroomById(classroomId)
classroomRecords.add(classroomRecord)
}

return classroomRecords
}

private fun loadClassroomById(classroomId: String): ClassroomRecord {
return if (loadLessonProtosFromAssets) {
assetRepository.tryLoadProtoFromLocalAssets(
assetName = classroomId,
defaultMessage = ClassroomRecord.getDefaultInstance()
) ?: ClassroomRecord.getDefaultInstance()
} else loadClassroomByIdFromJson(classroomId)
}

private fun loadClassroomByIdFromJson(classroomId: String): ClassroomRecord {
// Load the classroom obj.
val classroomObj = jsonAssetRetriever.loadJsonFromAsset("$classroomId.json")
checkNotNull(classroomObj) { "Failed to load $classroomId.json." }

// Load the topic prerequisite map.
val topicPrereqsObj = checkNotNull(classroomObj.optJSONObject("topic_prerequisites")) {
"Expected classroom to have non-null topic_prerequisites."
}
val topicPrereqs = topicPrereqsObj.keys().asSequence().associateWith { topicId ->
val topicIdArray = checkNotNull(topicPrereqsObj.optJSONArray(topicId)) {
"Expected topic $topicId to have a non-null string list."
}
return@associateWith List(topicIdArray.length()) { index ->
checkNotNull(topicIdArray.optString(index)) {
"Expected topic $topicId to have non-null string at index $index."
}
}
}
return ClassroomRecord.newBuilder().apply {
id = checkNotNull(classroomObj.optString("classroom_id")) {
"Expected classroom to have ID."
}
putAllTopicPrerequisites(
topicPrereqs.mapValues { (_, topicIds) ->
ClassroomRecord.TopicIdList.newBuilder().apply {
addAllTopicIds(topicIds)
}.build()
}
)
}.build()
}
}

/** Creates a [LessonThumbnail] from a classroomJsonObject. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import org.json.JSONObject
import org.oppia.android.app.model.ChapterPlayState
import org.oppia.android.app.model.ChapterRecord
import org.oppia.android.app.model.ChapterSummary
import org.oppia.android.app.model.ClassroomIdList
import org.oppia.android.app.model.ClassroomRecord
import org.oppia.android.app.model.CompletedStory
import org.oppia.android.app.model.CompletedStoryList
import org.oppia.android.app.model.EphemeralChapterSummary
Expand All @@ -32,7 +30,7 @@ import org.oppia.android.app.model.Topic
import org.oppia.android.app.model.TopicPlayAvailability
import org.oppia.android.app.model.TopicProgress
import org.oppia.android.app.model.TopicRecord
import org.oppia.android.domain.classroom.TEST_CLASSROOM_ID_0
import org.oppia.android.domain.classroom.ClassroomController
import org.oppia.android.domain.question.QuestionRetriever
import org.oppia.android.domain.translation.TranslationController
import org.oppia.android.domain.util.JsonAssetRetriever
Expand Down Expand Up @@ -108,7 +106,8 @@ class TopicController @Inject constructor(
private val storyProgressController: StoryProgressController,
private val assetRepository: AssetRepository,
@LoadLessonProtosFromAssets private val loadLessonProtosFromAssets: Boolean,
private val translationController: TranslationController
private val translationController: TranslationController,
private val classroomController: ClassroomController,
) {

/**
Expand Down Expand Up @@ -562,12 +561,13 @@ class TopicController @Inject constructor(
contentId = "description"
html = topicData.getStringFromObject("topic_description")
}.build()
val classroomId = classroomController.getClassroomIdByTopicId(topicId)
// No written translations are included since none are retrieved from JSON.
return Topic.newBuilder()
.setTopicId(topicId)
.setTitle(topicTitle)
.setDescription(topicDescription)
.setClassroomId(getClassroomIdByTopicId(topicId))
.setClassroomId(classroomId)
.addAllStory(storySummaryList)
.setTopicThumbnail(createTopicThumbnailFromJson(topicData))
.setDiskSizeBytes(computeTopicSizeBytes(getJsonAssetFileNameList(topicId)).toLong())
Expand Down Expand Up @@ -917,105 +917,6 @@ class TopicController @Inject constructor(
}.build()
}

private fun getClassroomIdByTopicId(topicId: String): String {
var classroomId = TEST_CLASSROOM_ID_0
loadClassrooms().forEach {
if (it.topicPrerequisitesMap.keys.contains(topicId)) {
classroomId = it.id
}
}
return classroomId
}

// TODO(#5344): Remove this in favor of per-classroom data handling.
private fun loadClassrooms(): List<ClassroomRecord> {
return if (loadLessonProtosFromAssets) {
assetRepository.loadProtoFromLocalAssets(
assetName = "classrooms",
baseMessage = ClassroomIdList.getDefaultInstance()
).classroomIdsList.map { classroomId ->
loadClassroomById(classroomId)
}
} else loadClassroomsFromJson()
}

// TODO(#5344): Remove this in favor of per-classroom data handling.
private fun loadClassroomsFromJson(): List<ClassroomRecord> {
// Load the classrooms.json file.
val classroomIdsObj = jsonAssetRetriever.loadJsonFromAsset("classrooms.json")
checkNotNull(classroomIdsObj) { "Failed to load classrooms.json." }
val classroomIds = classroomIdsObj.optJSONArray("classroom_id_list")
checkNotNull(classroomIds) { "classrooms.json is missing classroom IDs." }

// Initialize a list to store the [ClassroomRecord]s.
val classroomRecords = mutableListOf<ClassroomRecord>()

// Iterate over all classroomIds and load each classroom's JSON.
for (i in 0 until classroomIds.length()) {
val classroomId = checkNotNull(classroomIds.optString(i)) {
"Expected non-null classroom ID at index $i."
}
val classroomRecord = loadClassroomById(classroomId)
classroomRecords.add(classroomRecord)
}

return classroomRecords
}

// TODO(#5344): Move this to classroom controller.
private fun loadClassroomById(classroomId: String): ClassroomRecord {
return if (loadLessonProtosFromAssets) {
assetRepository.tryLoadProtoFromLocalAssets(
assetName = classroomId,
defaultMessage = ClassroomRecord.getDefaultInstance()
) ?: ClassroomRecord.getDefaultInstance()
} else loadClassroomByIdFromJson(classroomId)
}

// TODO(#5344): Remove this in favor of per-classroom data handling.
private fun loadClassroomByIdFromJson(classroomId: String): ClassroomRecord {
// Load the classroom obj.
val classroomObj = jsonAssetRetriever.loadJsonFromAsset("$classroomId.json")
checkNotNull(classroomObj) { "Failed to load $classroomId.json." }

val classroomTitle = classroomObj.getJSONObject("classroom_title")

// Load the topic prerequisite map.
val topicPrereqsObj = checkNotNull(classroomObj.optJSONObject("topic_prerequisites")) {
"Expected classroom to have non-null topic_prerequisites."
}
val topicPrereqs = topicPrereqsObj.keys().asSequence().associateWith { topicId ->
val topicIdArray = checkNotNull(topicPrereqsObj.optJSONArray(topicId)) {
"Expected topic $topicId to have a non-null string list."
}
return@associateWith List(topicIdArray.length()) { index ->
checkNotNull(topicIdArray.optString(index)) {
"Expected topic $topicId to have non-null string at index $index."
}
}
}
return ClassroomRecord.newBuilder().apply {
id = checkNotNull(classroomObj.optString("classroom_id")) {
"Expected classroom to have ID."
}
translatableTitle = SubtitledHtml.newBuilder().apply {
contentId = classroomTitle.getStringFromObject("content_id")
html = classroomTitle.getStringFromObject("html")
}.build()
putAllTopicPrerequisites(
topicPrereqs.mapValues { (_, topicIds) ->
ClassroomRecord.TopicIdList.newBuilder().apply {
addAllTopicIds(topicIds)
}.build()
}
)
}.build()
}

// TODO(#5344): Remove this in favor of per-classroom data handling.
private fun loadCombinedClassroomsTopicIdList(): List<String> =
loadClassrooms().flatMap { it.topicPrerequisitesMap.keys.toList() }

private companion object {
private fun JSONObject.getRemovableOptionalString(name: String) =
optString(name).takeIf { it.isNotEmpty() && it != "<removed>" && it != "<unknown>" }
Expand Down
Loading
Loading