From f6b1f605de9fee2d29f9db38733a051c85d95da8 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 30 May 2024 08:39:37 +0530 Subject: [PATCH 01/36] Add ClassroomIdList & ClassroomSummary protos --- model/src/main/proto/topic.proto | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/model/src/main/proto/topic.proto b/model/src/main/proto/topic.proto index 004980d147a..85c78d12bf9 100755 --- a/model/src/main/proto/topic.proto +++ b/model/src/main/proto/topic.proto @@ -547,6 +547,12 @@ message EphemeralRevisionCard { WrittenTranslationContext written_translation_context = 2; } +// Corresponds to a local file cataloging all classrooms available to load. +message ClassroomIdList { + // The list of IDs corresponding to classrooms available on the local filesystem. + repeated string classroom_ids = 1; +} + // Corresponds to a local file cataloging all available classrooms in the app. message ClassroomList { // The list of classrooms available to the app. @@ -581,6 +587,25 @@ message ClassroomRecord { } } +// A summary of a classroom which contains a list of topic summaries. +message ClassroomSummary { + // The ID of the classroom. + string classroom_id = 1; + + // Mapping from content_id to a TranslationMapping for each SubtitledHtml in this classroom that + // has a corresponding translation. + map written_translations = 2; + + // The title of the classroom. + SubtitledHtml classroom_title = 3; + + // The thumbnail corresponding to this classroom. + LessonThumbnail classroom_thumbnail = 4; + + // A list of topic summaries contained within this classroom. + repeated TopicSummary topic_summary = 5; +} + // Corresponds to a local file cataloging all concept cards available to load. message ConceptCardList { // The list of concept cards stored on the local filesystem. From fdcf183cab904efa5ed0c044f757f24a5510ed15 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 30 May 2024 08:47:17 +0530 Subject: [PATCH 02/36] Add classroom_id & classroom_title fields --- model/src/main/proto/topic.proto | 37 ++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/model/src/main/proto/topic.proto b/model/src/main/proto/topic.proto index 85c78d12bf9..b5bfd38c67b 100755 --- a/model/src/main/proto/topic.proto +++ b/model/src/main/proto/topic.proto @@ -25,6 +25,12 @@ message Topic { // A brief description of the topic. SubtitledHtml description = 11; + // The ID of the classroom this topic is part of. + string classroom_id = 12; + + // The title of the classroom this topic is part of. + SubtitledHtml classroom_title = 13; + // A list of summarized stories contained within this topic. repeated StorySummary story = 4; @@ -242,6 +248,12 @@ message CompletedStory { // The title of the topic this story is part of. SubtitledHtml topic_title = 9; + // The ID of the classroom this story is part of. + string classroom_id = 10; + + // The title of the classroom this story is part of. + SubtitledHtml classroom_title = 11; + // The thumbnail that should be displayed for this completed story. LessonThumbnail lesson_thumbnail = 5; @@ -289,6 +301,12 @@ message PromotedStory { // The title of the next chapter (exploration title) to complete. SubtitledHtml next_chapter_title = 17; + // The ID of the classroom this story is part of. + string classroom_id = 18; + + // The title of the classroom this story is part of. + SubtitledHtml classroom_title = 19; + // The exploration id next chapter to complete. string exploration_id = 6; @@ -331,6 +349,13 @@ message TopicSummary { // The title of the topic. SubtitledHtml title = 8; + // The ID of the classroom this topic is part of. + string classroom_id = 10; + + // The title of the classroom this topic is part of. + SubtitledHtml classroom_title = 11; + + // The structural version of the topic. int32 version = 3; @@ -444,6 +469,12 @@ message UpcomingTopic { // The title of the topic. SubtitledHtml title = 7; + // The ID of the classroom this topic is part of. + string classroom_id = 8; + + // The title of the classroom this topic is part of. + SubtitledHtml classroom_title = 9; + // The structural version of the topic. int32 version = 3; @@ -628,6 +659,12 @@ message TopicRecord { // The topic's description. SubtitledHtml translatable_description = 10; + // The ID of the classroom this topic is part of. + string classroom_id = 11; + + // The title of the classroom this topic is part of. + SubtitledHtml translatable_classroom_title = 12; + // The list of canonical story IDs that can be used to load stories from the local filesystem. repeated string canonical_story_ids = 4; From a66a5ac2e5598bd97b58d8a17dc236b16924a83e Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 30 May 2024 09:43:57 +0530 Subject: [PATCH 03/36] Add json & textproto test data --- domain/src/main/assets/GJ2rLXRKD5hw.json | 2 ++ domain/src/main/assets/GJ2rLXRKD5hw.textproto | 5 +++ domain/src/main/assets/classrooms.json | 11 +----- domain/src/main/assets/classrooms.textproto | 34 ++---------------- domain/src/main/assets/omzF4oqgeTXd.json | 2 ++ domain/src/main/assets/omzF4oqgeTXd.textproto | 5 +++ .../src/main/assets/test_classroom_id_0.json | 13 +++++++ .../main/assets/test_classroom_id_0.textproto | 17 +++++++++ .../src/main/assets/test_classroom_id_1.json | 13 +++++++ .../main/assets/test_classroom_id_1.textproto | 20 +++++++++++ .../src/main/assets/test_classroom_id_2.json | 12 +++++++ .../main/assets/test_classroom_id_2.textproto | 12 +++++++ domain/src/main/assets/test_topic_id_0.json | 2 ++ .../src/main/assets/test_topic_id_0.textproto | 5 +++ domain/src/main/assets/test_topic_id_1.json | 2 ++ .../src/main/assets/test_topic_id_1.textproto | 5 +++ domain/src/main/assets/test_topic_id_2.json | 2 ++ .../src/main/assets/test_topic_id_2.textproto | 5 +++ .../domain/topic/TopicListController.kt | 36 ++++++++++++------- 19 files changed, 150 insertions(+), 53 deletions(-) create mode 100644 domain/src/main/assets/test_classroom_id_0.json create mode 100644 domain/src/main/assets/test_classroom_id_0.textproto create mode 100644 domain/src/main/assets/test_classroom_id_1.json create mode 100644 domain/src/main/assets/test_classroom_id_1.textproto create mode 100644 domain/src/main/assets/test_classroom_id_2.json create mode 100644 domain/src/main/assets/test_classroom_id_2.textproto diff --git a/domain/src/main/assets/GJ2rLXRKD5hw.json b/domain/src/main/assets/GJ2rLXRKD5hw.json index 39e0269741c..ae22cbf45c8 100644 --- a/domain/src/main/assets/GJ2rLXRKD5hw.json +++ b/domain/src/main/assets/GJ2rLXRKD5hw.json @@ -13,6 +13,8 @@ "topic_name": "Fractions", "topic_description": "You'll often need to talk about part of an object or group. For example, a jar of milk might be half-full, or some of the eggs in a box might have broken. In these lessons, you'll learn to use fractions to describe situations like these.", "topic_id": "GJ2rLXRKD5hw", + "classroom_id": "test_classroom_id_0", + "classroom_name": "Maths", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", "skill_descriptions": {}, diff --git a/domain/src/main/assets/GJ2rLXRKD5hw.textproto b/domain/src/main/assets/GJ2rLXRKD5hw.textproto index 8891207c36c..dc83371adb8 100644 --- a/domain/src/main/assets/GJ2rLXRKD5hw.textproto +++ b/domain/src/main/assets/GJ2rLXRKD5hw.textproto @@ -25,3 +25,8 @@ translatable_description { html: "You\'ll often need to talk about part of an object or group. For example, a jar of milk might be half-full, or some of the eggs in a box might have broken. In these lessons, you\'ll learn to use fractions to describe situations like these." content_id: "description" } +classroom_id: test_classroom_id_0 +translatable_classroom_title { + html: "Maths" + content_id: "classroom_title" +} diff --git a/domain/src/main/assets/classrooms.json b/domain/src/main/assets/classrooms.json index eb92a2f7094..8a0d9bb44c3 100644 --- a/domain/src/main/assets/classrooms.json +++ b/domain/src/main/assets/classrooms.json @@ -1,12 +1,3 @@ { - "classrooms": [{ - "id": "test_classroom_id_0", - "topic_prerequisites": { - "test_topic_id_0": ["GJ2rLXRKD5hw"], - "test_topic_id_1": ["test_topic_id_0", "omzF4oqgeTXd"], - "test_topic_id_2": [], - "GJ2rLXRKD5hw": [], - "omzF4oqgeTXd": [] - } - }] + "classroom_id_list": ["test_classroom_id_0", "test_classroom_id_1", "test_classroom_id_2"] } diff --git a/domain/src/main/assets/classrooms.textproto b/domain/src/main/assets/classrooms.textproto index 05b4af4eb0a..5db8407d9a8 100644 --- a/domain/src/main/assets/classrooms.textproto +++ b/domain/src/main/assets/classrooms.textproto @@ -1,31 +1,3 @@ -classrooms { - id: "test_classroom_id_0" - topic_prerequisites { - key: "test_topic_id_0" - value { - topic_ids: "GJ2rLXRKD5hw" - } - } - topic_prerequisites { - key: "test_topic_id_1" - value { - topic_ids: "test_topic_id_0" - topic_ids: "omzF4oqgeTXd" - } - } - topic_prerequisites { - key: "test_topic_id_2" - value { - } - } - topic_prerequisites { - key: "GJ2rLXRKD5hw" - value { - } - } - topic_prerequisites { - key: "omzF4oqgeTXd" - value { - } - } -} +classroom_ids: "test_classroom_id_0" +classroom_ids: "test_classroom_id_1" +classroom_ids: "test_classroom_id_2" diff --git a/domain/src/main/assets/omzF4oqgeTXd.json b/domain/src/main/assets/omzF4oqgeTXd.json index 897eafd56eb..724af2c88f6 100644 --- a/domain/src/main/assets/omzF4oqgeTXd.json +++ b/domain/src/main/assets/omzF4oqgeTXd.json @@ -21,6 +21,8 @@ "topic_name": "Ratios and Proportional Reasoning", "topic_description": "Many everyday problems involve thinking about proportions. For example, if 30 apples cost 6 dollars, how much would 12 apples cost? In these lessons, you'll learn about how to use ratios and proportional reasoning to solve problems like this.", "topic_id": "omzF4oqgeTXd", + "classroom_id": "test_classroom_id_0", + "classroom_name": "Maths", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", "skill_descriptions": {}, diff --git a/domain/src/main/assets/omzF4oqgeTXd.textproto b/domain/src/main/assets/omzF4oqgeTXd.textproto index 5701d42636d..a49319c4f01 100644 --- a/domain/src/main/assets/omzF4oqgeTXd.textproto +++ b/domain/src/main/assets/omzF4oqgeTXd.textproto @@ -23,3 +23,8 @@ translatable_description { html: "Many everyday problems involve thinking about proportions. For example, if 30 apples cost 6 dollars, how much would 12 apples cost? In these lessons, you\'ll learn about how to use ratios and proportional reasoning to solve problems like this." content_id: "description" } +classroom_id: test_classroom_id_0 +translatable_classroom_title { + html: "Maths" + content_id: "classroom_title" +} diff --git a/domain/src/main/assets/test_classroom_id_0.json b/domain/src/main/assets/test_classroom_id_0.json new file mode 100644 index 00000000000..041b44929c9 --- /dev/null +++ b/domain/src/main/assets/test_classroom_id_0.json @@ -0,0 +1,13 @@ +{ + "classroom_id": "test_classroom_id_0", + "classroom_title": { + "content_id": "classroom_title", + "html": "Maths" + }, + "thumbnail_bg_color": "#00FFFFFF", + "thumbnail_filename": "", + "topic_prerequisites": { + "GJ2rLXRKD5hw": [], + "omzF4oqgeTXd": [] + } +} diff --git a/domain/src/main/assets/test_classroom_id_0.textproto b/domain/src/main/assets/test_classroom_id_0.textproto new file mode 100644 index 00000000000..64e10685c7e --- /dev/null +++ b/domain/src/main/assets/test_classroom_id_0.textproto @@ -0,0 +1,17 @@ +classroom_id: "test_classroom_id_0", +classroom_title: { + content_id: "classroom_title", + html: "Maths" +}, +thumbnail_bg_color: "#00FFFFFF", +thumbnail_filename: "", +topic_prerequisites { + key: "GJ2rLXRKD5hw" + value { + } +} +topic_prerequisites { + key: "omzF4oqgeTXd" + value { + } +} diff --git a/domain/src/main/assets/test_classroom_id_1.json b/domain/src/main/assets/test_classroom_id_1.json new file mode 100644 index 00000000000..7f1d59b9c1d --- /dev/null +++ b/domain/src/main/assets/test_classroom_id_1.json @@ -0,0 +1,13 @@ +{ + "classroom_id": "test_classroom_id_1", + "classroom_title": { + "content_id": "classroom_title", + "html": "Science" + }, + "thumbnail_bg_color": "#00FFFFFF", + "thumbnail_filename": "", + "topic_prerequisites": { + "test_topic_id_0": ["GJ2rLXRKD5hw"], + "test_topic_id_1": ["test_topic_id_0", "omzF4oqgeTXd"] + } +} diff --git a/domain/src/main/assets/test_classroom_id_1.textproto b/domain/src/main/assets/test_classroom_id_1.textproto new file mode 100644 index 00000000000..66caea11775 --- /dev/null +++ b/domain/src/main/assets/test_classroom_id_1.textproto @@ -0,0 +1,20 @@ +classroom_id: "test_classroom_id_1", +classroom_title: { + content_id: "classroom_title", + html: "Science" +}, +thumbnail_bg_color: "#00FFFFFF", +thumbnail_filename: "", +topic_prerequisites { + key: "test_topic_id_0" + value { + topic_ids: "GJ2rLXRKD5hw" + } +} +topic_prerequisites { + key: "test_topic_id_1" + value { + topic_ids: "test_topic_id_0" + topic_ids: "omzF4oqgeTXd" + } +} diff --git a/domain/src/main/assets/test_classroom_id_2.json b/domain/src/main/assets/test_classroom_id_2.json new file mode 100644 index 00000000000..55e7e9d9fe7 --- /dev/null +++ b/domain/src/main/assets/test_classroom_id_2.json @@ -0,0 +1,12 @@ +{ + "classroom_id": "test_classroom_id_2", + "classroom_title": { + "content_id": "classroom_title", + "html": "English" + }, + "thumbnail_bg_color": "#00FFFFFF", + "thumbnail_filename": "", + "topic_prerequisites": { + "test_topic_id_2": [] + } +} diff --git a/domain/src/main/assets/test_classroom_id_2.textproto b/domain/src/main/assets/test_classroom_id_2.textproto new file mode 100644 index 00000000000..baa3c0c9cc4 --- /dev/null +++ b/domain/src/main/assets/test_classroom_id_2.textproto @@ -0,0 +1,12 @@ +classroom_id: "test_classroom_id_2", +classroom_title: { + content_id: "classroom_title", + html: "English" +}, +thumbnail_bg_color: "#00FFFFFF", +thumbnail_filename: "", +topic_prerequisites { + key: "test_topic_id_2" + value { + } +} diff --git a/domain/src/main/assets/test_topic_id_0.json b/domain/src/main/assets/test_topic_id_0.json index a9115679060..1e07a9553a2 100644 --- a/domain/src/main/assets/test_topic_id_0.json +++ b/domain/src/main/assets/test_topic_id_0.json @@ -13,6 +13,8 @@ "topic_name": "First Test Topic", "topic_description": "A topic investigating the interesting aspects of the Oppia Android app.", "topic_id": "test_topic_id_0", + "classroom_id": "test_classroom_id_1", + "classroom_name": "Science", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", "skill_descriptions": {}, diff --git a/domain/src/main/assets/test_topic_id_0.textproto b/domain/src/main/assets/test_topic_id_0.textproto index ce47f844943..16c6a4222b3 100644 --- a/domain/src/main/assets/test_topic_id_0.textproto +++ b/domain/src/main/assets/test_topic_id_0.textproto @@ -22,3 +22,8 @@ translatable_description { html: "A topic investigating the interesting aspects of the Oppia Android app." content_id: "description" } +classroom_id: "test_classroom_id_1" +translatable_classroom_title { + html: "Science" + content_id: "classroom_title" +} diff --git a/domain/src/main/assets/test_topic_id_1.json b/domain/src/main/assets/test_topic_id_1.json index 6b3cc156914..fb8d038e3ee 100644 --- a/domain/src/main/assets/test_topic_id_1.json +++ b/domain/src/main/assets/test_topic_id_1.json @@ -13,6 +13,8 @@ "topic_name": "Second Test Topic", "topic_description": "A topic considering the various implications of having especially long topic descriptions. These descriptions almost certainly need to wrap, which should be interesting in the UI (especially on small screens). Consider also that there may even be multiple points pertaining to a topic, some of which may require expanding the description section in order to read the whole topic description.", "topic_id": "test_topic_id_1", + "classroom_id": "test_classroom_id_1", + "classroom_name": "Science", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", "skill_descriptions": {}, diff --git a/domain/src/main/assets/test_topic_id_1.textproto b/domain/src/main/assets/test_topic_id_1.textproto index fa4a012e25c..23c0195d9a5 100644 --- a/domain/src/main/assets/test_topic_id_1.textproto +++ b/domain/src/main/assets/test_topic_id_1.textproto @@ -21,3 +21,8 @@ translatable_description { html: "A topic considering the various implications of having especially long topic descriptions. These descriptions almost certainly need to wrap, which should be interesting in the UI (especially on small screens). Consider also that there may even be multiple points pertaining to a topic, some of which may require expanding the description section in order to read the whole topic description." content_id: "description" } +classroom_id: "test_classroom_id_1" +translatable_classroom_title { + html: "Science" + content_id: "classroom_title" +} diff --git a/domain/src/main/assets/test_topic_id_2.json b/domain/src/main/assets/test_topic_id_2.json index 469617b8c56..2a5a20640ef 100644 --- a/domain/src/main/assets/test_topic_id_2.json +++ b/domain/src/main/assets/test_topic_id_2.json @@ -5,6 +5,8 @@ "topic_description": "A topic that's not yet ready to be played.", "topic_name": "Third Test Topic", "topic_id": "test_topic_id_2", + "classroom_id": "test_classroom_id_2", + "classroom_name": "English", "version": -1, "published": false, "skill_descriptions": {}, diff --git a/domain/src/main/assets/test_topic_id_2.textproto b/domain/src/main/assets/test_topic_id_2.textproto index 5d9e7360478..63a1d005792 100644 --- a/domain/src/main/assets/test_topic_id_2.textproto +++ b/domain/src/main/assets/test_topic_id_2.textproto @@ -7,3 +7,8 @@ translatable_description { html: "A topic that\'s not yet ready to be played." content_id: "description" } +classroom_id: "test_classroom_id_2" +translatable_classroom_title { + html: "English" + content_id: "classroom_title" +} diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index 8936feb3d98..8f1ae1c31fc 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -5,7 +5,7 @@ import org.json.JSONObject import org.oppia.android.app.model.ChapterPlayState import org.oppia.android.app.model.ChapterProgress import org.oppia.android.app.model.ChapterSummary -import org.oppia.android.app.model.ClassroomList +import org.oppia.android.app.model.ClassroomIdList import org.oppia.android.app.model.ClassroomRecord import org.oppia.android.app.model.ClassroomRecord.TopicIdList import org.oppia.android.app.model.ComingSoonTopicList @@ -796,22 +796,34 @@ class TopicListController @Inject constructor( // TODO(#5344): Remove this in favor of per-classroom data handling. private fun loadClassroom(): ClassroomRecord { return if (loadLessonProtosFromAssets) { - return assetRepository.loadProtoFromLocalAssets( + val defaultClassroomId = assetRepository.loadProtoFromLocalAssets( assetName = "classrooms", - baseMessage = ClassroomList.getDefaultInstance() - ).classroomsList.single() // Only one record is currently expected. + baseMessage = ClassroomIdList.getDefaultInstance() + ).getClassroomIds(0) // Only one record is currently loaded. + assetRepository.loadProtoFromLocalAssets( + assetName = defaultClassroomId, + baseMessage = ClassroomRecord.getDefaultInstance() + ) } else loadClassroomFromJson() } // TODO(#5344): Remove this in favor of per-classroom data handling. private fun loadClassroomFromJson(): ClassroomRecord { - val classroomsObj = jsonAssetRetriever.loadJsonFromAsset("classrooms.json") - checkNotNull(classroomsObj) { "Failed to load classrooms.json." } - val classroomArray = classroomsObj.optJSONArray("classrooms") - checkNotNull(classroomArray) { "classrooms.json missing classrooms array." } - check(classroomArray.length() == 1) { "Expected classrooms.json to have one single classroom." } - val classroom = checkNotNull(classroomArray.optJSONObject(0)) { "Expected non-null classroom." } - val topicPrereqsObj = checkNotNull(classroom.optJSONObject("topic_prerequisites")) { + // 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 missing classroom IDs." } + + // Fetch the first classroom ID of the list. + val defaultClassroomId = checkNotNull(classroomIds.get(0)) { + "Expected non-null classroom ID." + } + + // Load the classroom obj of the first classroom. + val defaultClassroomObj = jsonAssetRetriever.loadJsonFromAsset("$defaultClassroomId.json") + checkNotNull(defaultClassroomObj) { "Failed to load $defaultClassroomId.json." } + val topicPrereqsObj = checkNotNull(defaultClassroomObj.optJSONObject("topic_prerequisites")) { "Expected classroom to have non-null topic_prerequisites." } val topicPrereqs = topicPrereqsObj.keys().asSequence().associateWith { topicId -> @@ -825,7 +837,7 @@ class TopicListController @Inject constructor( } } return ClassroomRecord.newBuilder().apply { - this.id = checkNotNull(classroom.optString("id")) { "Expected classroom to have ID." } + this.id = checkNotNull(defaultClassroomObj.optString("id")) { "Expected classroom to have ID." } this.putAllTopicPrerequisites( topicPrereqs.mapValues { (_, topicIds) -> TopicIdList.newBuilder().apply { From b2925941aa71907e3563241a49727f163bd74bcf Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 30 May 2024 09:50:59 +0530 Subject: [PATCH 04/36] Update controllers to add classroom references in topics and stories --- .../android/domain/topic/TopicController.kt | 11 +++++++ .../domain/topic/TopicListController.kt | 32 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt index 64c22d4e7af..d030eda4e07 100755 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt @@ -394,6 +394,8 @@ class TopicController @Inject constructor( .setStoryTitle(storySummary.storyTitle) .setTopicId(topic.topicId) .setTopicTitle(topic.title) + .setClassroomId(topic.classroomId) + .setClassroomTitle(topic.classroomTitle) .setLessonThumbnail(storySummary.storyThumbnail) completedStoryList.add(completedStoryBuilder.build()) } @@ -473,6 +475,8 @@ class TopicController @Inject constructor( putAllWrittenTranslations(topicRecord.writtenTranslationsMap) title = topicRecord.translatableTitle description = topicRecord.translatableDescription + classroomId = topicRecord.classroomId + classroomTitle = topicRecord.translatableClassroomTitle addAllStory(stories) topicThumbnail = createTopicThumbnailFromProto(topicId, topicRecord.topicThumbnail) diskSizeBytes = computeTopicSizeBytes(getProtoAssetFileNameList(topicId)).toLong() @@ -554,6 +558,11 @@ class TopicController @Inject constructor( contentId = "title" html = topicData.getStringFromObject("topic_name") }.build() + val classroomId = topicData.getStringFromObject("classroom_id") + val classroomTitle = SubtitledHtml.newBuilder().apply { + contentId = "classroom_title" + html = topicData.getStringFromObject("classroom_name") + } val topicDescription = SubtitledHtml.newBuilder().apply { contentId = "description" html = topicData.getStringFromObject("topic_description") @@ -562,6 +571,8 @@ class TopicController @Inject constructor( return Topic.newBuilder() .setTopicId(topicId) .setTitle(topicTitle) + .setClassroomId(classroomId) + .setClassroomTitle(classroomTitle) .setDescription(topicDescription) .addAllStory(storySummaryList) .setTopicThumbnail(createTopicThumbnailFromJson(topicData)) diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index 8f1ae1c31fc..f77d973a90c 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -209,6 +209,8 @@ class TopicListController @Inject constructor( this.topicId = topicId putAllWrittenTranslations(topicRecord.writtenTranslationsMap) title = topicRecord.translatableTitle + classroomId = topicRecord.classroomId + classroomTitle = topicRecord.translatableClassroomTitle totalChapterCount = storyRecords.map { it.chaptersList.size }.sum() topicThumbnail = topicRecord.topicThumbnail topicPlayAvailability = if (topicRecord.isPublished) { @@ -250,10 +252,17 @@ class TopicListController @Inject constructor( contentId = "title" html = jsonObject.getStringFromObject("topic_name") }.build() + val classroomId = jsonObject.getStringFromObject("classroom_id") + val classroomTitle = SubtitledHtml.newBuilder().apply { + contentId = "classroom_title" + html = jsonObject.getStringFromObject("classroom_name") + }.build() // No written translations are included since none are retrieved from JSON. return TopicSummary.newBuilder() .setTopicId(topicId) .setTitle(topicTitle) + .setClassroomId(classroomId) + .setClassroomTitle(classroomTitle) .setVersion(jsonObject.optInt("version")) .setTotalChapterCount(totalChapterCount) .setTopicThumbnail(createTopicThumbnailFromJson(jsonObject)) @@ -285,9 +294,17 @@ class TopicListController @Inject constructor( html = jsonObject.getStringFromObject("topic_name") }.build() + val classroomId = jsonObject.getStringFromObject("classroom_id") + val classroomTitle = SubtitledHtml.newBuilder().apply { + contentId = "classroom_title" + html = jsonObject.getStringFromObject("classroom_name") + }.build() + // No written translations are included since none are retrieved from JSON. return UpcomingTopic.newBuilder().setTopicId(topicId) .setTitle(topicTitle) + .setClassroomId(classroomId) + .setClassroomTitle(classroomTitle) .setVersion(jsonObject.optInt("version")) .setTopicPlayAvailability(topicPlayAvailability) .setLessonThumbnail(createTopicThumbnailFromJson(jsonObject)) @@ -684,6 +701,8 @@ class TopicListController @Inject constructor( storyTitle = storyRecord.translatableStoryName this.topicId = topicId topicTitle = topicRecord.translatableTitle + classroomId = topicRecord.classroomId + classroomTitle = topicRecord.translatableClassroomTitle completedChapterCount = 0 totalChapterCount = storyRecord.chaptersCount lessonThumbnail = storyRecord.storyThumbnail @@ -731,6 +750,15 @@ class TopicListController @Inject constructor( html = it }.build() } ?: SubtitledHtml.getDefaultInstance() + + val classroomId = topicJson.optString("classroom_id") + val classroomTitle = topicJson.optString("classroom_name").takeIf { it.isNotEmpty() }?.let { + SubtitledHtml.newBuilder().apply { + contentId = "classroom_title" + html = it + }.build() + } ?: SubtitledHtml.getDefaultInstance() + // No written translations are included for the topic since its name is directly fetched from // the JSON (and the JSON doesn't include translations for these properties, anyway). val promotedStoryBuilder = PromotedStory.newBuilder() @@ -739,6 +767,8 @@ class TopicListController @Inject constructor( .setLessonThumbnail(storySummary.storyThumbnail) .setTopicId(topicId) .setTopicTitle(topicTitle) + .setClassroomId(classroomId) + .setClassroomTitle(classroomTitle) .setCompletedChapterCount(0) .setTotalChapterCount(totalChapterCount) if (storySummary.chapterList.isNotEmpty()) { @@ -784,6 +814,8 @@ class TopicListController @Inject constructor( .setLessonThumbnail(storySummary.storyThumbnail) .setTopicId(topic.topicId) .setTopicTitle(topic.title) + .setClassroomId(topic.classroomId) + .setClassroomTitle(topic.classroomTitle) .setCompletedChapterCount(completedChapterCount) .setTotalChapterCount(totalChapterCount) .setIsTopicLearned(isTopicConsideredCompleted) From d68d1b508e4091b89485a6e84e1d2051a3e1cfc8 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 30 May 2024 11:15:03 +0530 Subject: [PATCH 05/36] Add and update tests for controller changes --- .../domain/topic/TopicListController.kt | 6 ++ .../domain/topic/TopicControllerTest.kt | 39 ++++++++ .../domain/topic/TopicListControllerTest.kt | 93 ++++++++++++++++++- 3 files changed, 134 insertions(+), 4 deletions(-) diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index f77d973a90c..d4541ec3e29 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -59,6 +59,12 @@ const val FRACTIONS_TOPIC_ID = "GJ2rLXRKD5hw" const val SUBTOPIC_TOPIC_ID = 1 const val SUBTOPIC_TOPIC_ID_2 = 2 const val RATIOS_TOPIC_ID = "omzF4oqgeTXd" + +// TODO(#5344): Move these classroom ids to [ClassroomController]. +const val TEST_CLASSROOM_ID_0 = "test_classroom_id_0" +const val TEST_CLASSROOM_ID_1 = "test_classroom_id_1" +const val TEST_CLASSROOM_ID_2 = "test_classroom_id_2" + val TOPIC_THUMBNAILS = mapOf( FRACTIONS_TOPIC_ID to createTopicThumbnail0(), RATIOS_TOPIC_ID to createTopicThumbnail1(), diff --git a/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt index 02cd5660933..0a1b6cb3c95 100755 --- a/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt @@ -130,6 +130,15 @@ class TopicControllerTest { assertThat(topic.description.html).contains("You'll often need to talk about") } + @Test + fun testRetrieveTopic_fractionsTopic_hasCorrectClassroomInfo() { + val topicProvider = topicController.getTopic(profileId1, FRACTIONS_TOPIC_ID) + + val topic = monitorFactory.waitForNextSuccessfulResult(topicProvider).topic + assertThat(topic.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(topic.classroomTitle.html).contains("Maths") + } + @Test fun testRetrieveTopic_ratiosTopic_returnsCorrectTopic() { val topicProvider = topicController.getTopic(profileId1, RATIOS_TOPIC_ID) @@ -150,6 +159,15 @@ class TopicControllerTest { ) } + @Test + fun testRetrieveTopic_ratiosTopic_hasCorrectClassroomInfo() { + val topicProvider = topicController.getTopic(profileId1, RATIOS_TOPIC_ID) + + val topic = monitorFactory.waitForNextSuccessfulResult(topicProvider).topic + assertThat(topic.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(topic.classroomTitle.html).contains("Maths") + } + @Test fun testRetrieveTopic_invalidTopic_returnsFailure() { val topicProvider = topicController.getTopic(profileId1, INVALID_TOPIC_ID_1) @@ -877,6 +895,27 @@ class TopicControllerTest { assertThat(completedStoryList.completedStoryList[1].storyId).isEqualTo(RATIOS_STORY_ID_0) } + @Test + fun testCompletedStoryList_finishTwoStories_completedStoryListHasCorrectClassroomInfo() { + markFractionsStory0Chapter0AsCompleted() + markFractionsStory0Chapter1AsCompleted() + markRatiosStory0Chapter0AsCompleted() + markRatiosStory0Chapter1AsCompleted() + + val storyList = topicController.getCompletedStoryList(profileId1) + + val completedStoryList = monitorFactory.waitForNextSuccessfulResult(storyList) + assertThat(completedStoryList.completedStoryCount).isEqualTo(2) + completedStoryList.completedStoryList[0].also { completedFractionsStory -> + assertThat(completedFractionsStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(completedFractionsStory.classroomTitle.html).isEqualTo("Maths") + } + completedStoryList.completedStoryList[1].also { completedRatiosStory -> + assertThat(completedRatiosStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(completedRatiosStory.classroomTitle.html).isEqualTo("Maths") + } + } + /* Localization-based tests. */ @Test diff --git a/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt index 28bcf2e44cd..62cf8a2dc3d 100644 --- a/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt @@ -10,6 +10,7 @@ import dagger.Component import dagger.Module import dagger.Provides import org.junit.Before +import org.junit.Ignore import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -100,6 +101,8 @@ class TopicListControllerTest { } @Test + @Ignore("Test failing due to changes in data files") + // TODO(#5344): Migrate the test to ClassroomControllerTest. fun testRetrieveTopicList_firstTopic_hasCorrectTopicInfo() { val topicList = retrieveTopicList() @@ -109,6 +112,19 @@ class TopicListControllerTest { } @Test + @Ignore("Test failing due to changes in data files") + // TODO(#5344): Migrate the test to ClassroomControllerTest. + fun testRetrieveTopicList_firstTopic_hasCorrectClassroomInfo() { + val topicList = retrieveTopicList() + + val firstTopic = topicList.getTopicSummary(0).topicSummary + assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + assertThat(firstTopic.classroomTitle.html).isEqualTo("Science") + } + + @Test + @Ignore("Test failing due to changes in data files") + // TODO(#5344): Migrate the test to ClassroomControllerTest. fun testRetrieveTopicList_firstTopic_hasCorrectLessonCount() { val topicList = retrieveTopicList() @@ -117,6 +133,8 @@ class TopicListControllerTest { } @Test + @Ignore("Test failing due to changes in data files") + // TODO(#5344): Migrate the test to ClassroomControllerTest. fun testRetrieveTopicList_secondTopic_hasCorrectTopicInfo() { val topicList = retrieveTopicList() @@ -126,6 +144,19 @@ class TopicListControllerTest { } @Test + @Ignore("Test failing due to changes in data files") + // TODO(#5344): Migrate the test to ClassroomControllerTest. + fun testRetrieveTopicList_secondTopic_hasCorrectClassroomInfo() { + val topicList = retrieveTopicList() + + val firstTopic = topicList.getTopicSummary(1).topicSummary + assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + assertThat(firstTopic.classroomTitle.html).isEqualTo("Science") + } + + @Test + @Ignore("Test failing due to changes in data files") + // TODO(#5344): Migrate the test to ClassroomControllerTest. fun testRetrieveTopicList_secondTopic_hasCorrectLessonCount() { val topicList = retrieveTopicList() @@ -137,16 +168,25 @@ class TopicListControllerTest { fun testRetrieveTopicList_fractionsTopic_hasCorrectTopicInfo() { val topicList = retrieveTopicList() - val fractionsTopic = topicList.getTopicSummary(2).topicSummary + val fractionsTopic = topicList.getTopicSummary(0).topicSummary assertThat(fractionsTopic.topicId).isEqualTo(FRACTIONS_TOPIC_ID) assertThat(fractionsTopic.title.html).isEqualTo("Fractions") } + @Test + fun testRetrieveTopicList_fractionsTopic_hasCorrectClassroomInfo() { + val topicList = retrieveTopicList() + + val firstTopic = topicList.getTopicSummary(0).topicSummary + assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(firstTopic.classroomTitle.html).isEqualTo("Maths") + } + @Test fun testRetrieveTopicList_fractionsTopic_hasCorrectLessonCount() { val topicList = retrieveTopicList() - val fractionsTopic = topicList.getTopicSummary(2).topicSummary + val fractionsTopic = topicList.getTopicSummary(0).topicSummary assertThat(fractionsTopic.totalChapterCount).isEqualTo(2) } @@ -154,16 +194,25 @@ class TopicListControllerTest { fun testRetrieveTopicList_ratiosTopic_hasCorrectTopicInfo() { val topicList = retrieveTopicList() - val ratiosTopic = topicList.getTopicSummary(3).topicSummary + val ratiosTopic = topicList.getTopicSummary(1).topicSummary assertThat(ratiosTopic.topicId).isEqualTo(RATIOS_TOPIC_ID) assertThat(ratiosTopic.title.html).isEqualTo("Ratios and Proportional Reasoning") } + @Test + fun testRetrieveTopicList_ratiosTopic_hasCorrectClassroomInfo() { + val topicList = retrieveTopicList() + + val firstTopic = topicList.getTopicSummary(1).topicSummary + assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(firstTopic.classroomTitle.html).isEqualTo("Maths") + } + @Test fun testRetrieveTopicList_ratiosTopic_hasCorrectLessonCount() { val topicList = retrieveTopicList() - val ratiosTopic = topicList.getTopicSummary(3).topicSummary + val ratiosTopic = topicList.getTopicSummary(1).topicSummary assertThat(ratiosTopic.totalChapterCount).isEqualTo(4) } @@ -285,6 +334,8 @@ class TopicListControllerTest { } @Test + @Ignore("Test failing as only the topics of the default classroom is being considered (i.e., Fractions & Ratios)") + // TODO(#5344): Update recommendation logic. fun testGetPromotedStoryList_markAllChapsDoneInFractions_suggestedStoryListIsCorrect() { storyProgressTestHelper.markCompletedFractionsStory0( profileId0, @@ -342,6 +393,8 @@ class TopicListControllerTest { } @Test + @Ignore("Test failing as only the topics of the default classroom is being considered (i.e., Fractions & Ratios)") + // TODO(#5344): Update recommendation logic. fun testGetStoryList_markStoryDoneOfRatiosAndFirstTestTopic_suggestedStoryListIsCorrect() { storyProgressTestHelper.markCompletedTestTopic0Story0( profileId0, @@ -404,6 +457,8 @@ class TopicListControllerTest { } @Test + @Ignore("Test failing as only the topics of the default classroom is being considered (i.e., Fractions & Ratios)") + // TODO(#5344): Update recommendation logic. fun testRetrievePromotedActivityList_markAllChapDoneInAllTopics_comingSoonTopicListIsCorrect() { storyProgressTestHelper.markAllTopicsAsCompleted( profileId0, @@ -417,6 +472,8 @@ class TopicListControllerTest { } @Test + @Ignore("Test failing as only the topics of the default classroom is being considered (i.e., Fractions & Ratios)") + // TODO(#5344): Update recommendation logic. fun testGetStoryList_markAllChapDoneInSecondTestTopic_doesNotPromoteAnyStories() { storyProgressTestHelper.markCompletedTestTopic1Story0( profileId0, @@ -433,6 +490,8 @@ class TopicListControllerTest { } @Test + @Ignore("Test failing as only the topics of the default classroom is being considered (i.e., Fractions & Ratios)") + // TODO(#5344): Update recommendation logic. fun testGetStoryList_markAllChapDoneInSecondTestTopic_comingSoonTopicListIsCorrect() { storyProgressTestHelper.markCompletedTestTopic1Story0( profileId0, @@ -525,6 +584,8 @@ class TopicListControllerTest { } @Test + @Ignore("Test failing as only the topics of the default classroom is being considered (i.e., Fractions & Ratios)") + // TODO(#5344): Update recommendation logic. fun testGetStoryList_markOneStoryDoneForFirstTestTopic_suggestedStoryListIsCorrect() { storyProgressTestHelper.markCompletedTestTopic0Story0( profileId0, @@ -679,6 +740,8 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(TEST_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(TEST_TOPIC_ID_0) assertThat(promotedStory.topicTitle.html).isEqualTo("First Test Topic") + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + assertThat(promotedStory.classroomTitle.html).isEqualTo("Science") assertThat(promotedStory.nextChapterTitle.html).isEqualTo("Prototype Exploration") assertThat(promotedStory.completedChapterCount).isEqualTo(0) assertThat(promotedStory.isTopicLearned).isFalse() @@ -690,6 +753,8 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(TEST_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(TEST_TOPIC_ID_0) assertThat(promotedStory.topicTitle.html).isEqualTo("First Test Topic") + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + assertThat(promotedStory.classroomTitle.html).isEqualTo("Science") assertThat(promotedStory.nextChapterTitle.html).isEqualTo("Prototype Exploration") assertThat(promotedStory.completedChapterCount).isEqualTo(0) assertThat(promotedStory.isTopicLearned).isFalse() @@ -701,6 +766,8 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(TEST_STORY_ID_2) assertThat(promotedStory.topicId).isEqualTo(TEST_TOPIC_ID_1) assertThat(promotedStory.topicTitle.html).isEqualTo("Second Test Topic") + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + assertThat(promotedStory.classroomTitle.html).isEqualTo("Science") assertThat(promotedStory.nextChapterTitle.html).isEqualTo("Fifth Exploration") assertThat(promotedStory.completedChapterCount).isEqualTo(0) assertThat(promotedStory.isTopicLearned).isFalse() @@ -712,6 +779,8 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(FRACTIONS_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(FRACTIONS_TOPIC_ID) assertThat(promotedStory.topicTitle.html).isEqualTo("Fractions") + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.nextChapterTitle.html).isEqualTo("What is a Fraction?") assertThat(promotedStory.completedChapterCount).isEqualTo(0) assertThat(promotedStory.isTopicLearned).isFalse() @@ -723,6 +792,8 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(FRACTIONS_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(FRACTIONS_TOPIC_ID) assertThat(promotedStory.topicTitle.html).isEqualTo("Fractions") + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.nextChapterTitle.html).isEqualTo("What is a Fraction?") assertThat(promotedStory.completedChapterCount).isEqualTo(0) assertThat(promotedStory.isTopicLearned).isFalse() @@ -734,6 +805,8 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(FRACTIONS_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(FRACTIONS_TOPIC_ID) assertThat(promotedStory.topicTitle.html).isEqualTo("Fractions") + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.nextChapterTitle.html).isEqualTo("The Meaning of Equal Parts") assertThat(promotedStory.completedChapterCount).isEqualTo(1) assertThat(promotedStory.totalChapterCount).isEqualTo(2) @@ -744,6 +817,8 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(RATIOS_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(RATIOS_TOPIC_ID) assertThat(promotedStory.nextChapterTitle.html).isEqualTo("What is a Ratio?") + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.topicTitle.html).isEqualTo("Ratios and Proportional Reasoning") assertThat(promotedStory.completedChapterCount).isEqualTo(0) assertThat(promotedStory.isTopicLearned).isFalse() @@ -755,6 +830,8 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(RATIOS_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(RATIOS_TOPIC_ID) assertThat(promotedStory.nextChapterTitle.html).isEqualTo("What is a Ratio?") + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.topicTitle.html).isEqualTo("Ratios and Proportional Reasoning") assertThat(promotedStory.completedChapterCount).isEqualTo(0) assertThat(promotedStory.isTopicLearned).isFalse() @@ -767,6 +844,8 @@ class TopicListControllerTest { assertThat(promotedStory.topicId).isEqualTo(RATIOS_TOPIC_ID) assertThat(promotedStory.nextChapterTitle.html).isEqualTo("Order is important") assertThat(promotedStory.topicTitle.html).isEqualTo("Ratios and Proportional Reasoning") + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.completedChapterCount).isEqualTo(1) assertThat(promotedStory.isTopicLearned).isFalse() assertThat(promotedStory.totalChapterCount).isEqualTo(2) @@ -775,6 +854,8 @@ class TopicListControllerTest { private fun verifyUpcomingTopic1(upcomingTopic: UpcomingTopic) { assertThat(upcomingTopic.topicId).isEqualTo(UPCOMING_TOPIC_ID_1) assertThat(upcomingTopic.title.html).isEqualTo("Third Test Topic") + assertThat(upcomingTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_2) + assertThat(upcomingTopic.classroomTitle.html).isEqualTo("English") } private fun verifyOngoingStoryAsRatioStory1Exploration2( @@ -786,6 +867,8 @@ class TopicListControllerTest { assertThat(promotedStory.topicId).isEqualTo(RATIOS_TOPIC_ID) assertThat(promotedStory.nextChapterTitle.html).isEqualTo("Equivalent Ratios") assertThat(promotedStory.topicTitle.html).isEqualTo("Ratios and Proportional Reasoning") + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.completedChapterCount).isEqualTo(0) assertThat(promotedStory.isTopicLearned).isEqualTo(expectedToBeLearned) assertThat(promotedStory.totalChapterCount).isEqualTo(2) @@ -797,6 +880,8 @@ class TopicListControllerTest { assertThat(promotedStory.topicId).isEqualTo(RATIOS_TOPIC_ID) assertThat(promotedStory.nextChapterTitle.html).isEqualTo("Writing Ratios in Simplest Form") assertThat(promotedStory.topicTitle.html).isEqualTo("Ratios and Proportional Reasoning") + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.completedChapterCount).isEqualTo(1) assertThat(promotedStory.isTopicLearned).isFalse() assertThat(promotedStory.totalChapterCount).isEqualTo(2) From 69352df181b44952953b4f197e91b615331dae4e Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 30 May 2024 11:37:57 +0530 Subject: [PATCH 06/36] Remove ClassroomList model --- model/src/main/proto/topic.proto | 6 ------ 1 file changed, 6 deletions(-) diff --git a/model/src/main/proto/topic.proto b/model/src/main/proto/topic.proto index b5bfd38c67b..734bd18f2a6 100755 --- a/model/src/main/proto/topic.proto +++ b/model/src/main/proto/topic.proto @@ -584,12 +584,6 @@ message ClassroomIdList { repeated string classroom_ids = 1; } -// Corresponds to a local file cataloging all available classrooms in the app. -message ClassroomList { - // The list of classrooms available to the app. - repeated ClassroomRecord classrooms = 1; -} - // Corresponds to a loadable classroom. message ClassroomRecord { // The classroom's ID. From 700a001c6224fa55fc1fd96405784e15db9cb48e Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 30 May 2024 11:49:16 +0530 Subject: [PATCH 07/36] Fix ktlint issues --- .../android/domain/topic/TopicListController.kt | 4 +++- .../android/domain/topic/TopicListControllerTest.kt | 12 ++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index d4541ec3e29..a4681215239 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -875,7 +875,9 @@ class TopicListController @Inject constructor( } } return ClassroomRecord.newBuilder().apply { - this.id = checkNotNull(defaultClassroomObj.optString("id")) { "Expected classroom to have ID." } + this.id = checkNotNull(defaultClassroomObj.optString("id")) { + "Expected classroom to have ID." + } this.putAllTopicPrerequisites( topicPrereqs.mapValues { (_, topicIds) -> TopicIdList.newBuilder().apply { diff --git a/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt index 62cf8a2dc3d..5e32e0e2b98 100644 --- a/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt @@ -334,7 +334,7 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing as only the topics of the default classroom is being considered (i.e., Fractions & Ratios)") + @Ignore("Test failing because only default classroom topics (Fractions & Ratios) are considered") // TODO(#5344): Update recommendation logic. fun testGetPromotedStoryList_markAllChapsDoneInFractions_suggestedStoryListIsCorrect() { storyProgressTestHelper.markCompletedFractionsStory0( @@ -393,7 +393,7 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing as only the topics of the default classroom is being considered (i.e., Fractions & Ratios)") + @Ignore("Test failing because only default classroom topics (Fractions & Ratios) are considered") // TODO(#5344): Update recommendation logic. fun testGetStoryList_markStoryDoneOfRatiosAndFirstTestTopic_suggestedStoryListIsCorrect() { storyProgressTestHelper.markCompletedTestTopic0Story0( @@ -457,7 +457,7 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing as only the topics of the default classroom is being considered (i.e., Fractions & Ratios)") + @Ignore("Test failing because only default classroom topics (Fractions & Ratios) are considered") // TODO(#5344): Update recommendation logic. fun testRetrievePromotedActivityList_markAllChapDoneInAllTopics_comingSoonTopicListIsCorrect() { storyProgressTestHelper.markAllTopicsAsCompleted( @@ -472,7 +472,7 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing as only the topics of the default classroom is being considered (i.e., Fractions & Ratios)") + @Ignore("Test failing because only default classroom topics (Fractions & Ratios) are considered") // TODO(#5344): Update recommendation logic. fun testGetStoryList_markAllChapDoneInSecondTestTopic_doesNotPromoteAnyStories() { storyProgressTestHelper.markCompletedTestTopic1Story0( @@ -490,7 +490,7 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing as only the topics of the default classroom is being considered (i.e., Fractions & Ratios)") + @Ignore("Test failing because only default classroom topics (Fractions & Ratios) are considered") // TODO(#5344): Update recommendation logic. fun testGetStoryList_markAllChapDoneInSecondTestTopic_comingSoonTopicListIsCorrect() { storyProgressTestHelper.markCompletedTestTopic1Story0( @@ -584,7 +584,7 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing as only the topics of the default classroom is being considered (i.e., Fractions & Ratios)") + @Ignore("Test failing because only default classroom topics (Fractions & Ratios) are considered") // TODO(#5344): Update recommendation logic. fun testGetStoryList_markOneStoryDoneForFirstTestTopic_suggestedStoryListIsCorrect() { storyProgressTestHelper.markCompletedTestTopic0Story0( From 9063bcd7a7003f1ec98e83e7b4732aae98395c97 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 30 May 2024 11:59:32 +0530 Subject: [PATCH 08/36] Fix textproto syntax issue --- domain/src/main/assets/GJ2rLXRKD5hw.textproto | 2 +- domain/src/main/assets/omzF4oqgeTXd.textproto | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/domain/src/main/assets/GJ2rLXRKD5hw.textproto b/domain/src/main/assets/GJ2rLXRKD5hw.textproto index dc83371adb8..3f469760138 100644 --- a/domain/src/main/assets/GJ2rLXRKD5hw.textproto +++ b/domain/src/main/assets/GJ2rLXRKD5hw.textproto @@ -25,7 +25,7 @@ translatable_description { html: "You\'ll often need to talk about part of an object or group. For example, a jar of milk might be half-full, or some of the eggs in a box might have broken. In these lessons, you\'ll learn to use fractions to describe situations like these." content_id: "description" } -classroom_id: test_classroom_id_0 +classroom_id: "test_classroom_id_0" translatable_classroom_title { html: "Maths" content_id: "classroom_title" diff --git a/domain/src/main/assets/omzF4oqgeTXd.textproto b/domain/src/main/assets/omzF4oqgeTXd.textproto index a49319c4f01..6ee875cc912 100644 --- a/domain/src/main/assets/omzF4oqgeTXd.textproto +++ b/domain/src/main/assets/omzF4oqgeTXd.textproto @@ -23,7 +23,7 @@ translatable_description { html: "Many everyday problems involve thinking about proportions. For example, if 30 apples cost 6 dollars, how much would 12 apples cost? In these lessons, you\'ll learn about how to use ratios and proportional reasoning to solve problems like this." content_id: "description" } -classroom_id: test_classroom_id_0 +classroom_id: "test_classroom_id_0" translatable_classroom_title { html: "Maths" content_id: "classroom_title" From 8ce636d6e72dbaa8786267baf5607bf5e6214930 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 30 May 2024 20:53:32 +0530 Subject: [PATCH 09/36] Fix textproto syntax issue --- domain/domain_assets.bzl | 4 ++-- domain/src/main/assets/test_classroom_id_0.textproto | 8 +++++--- domain/src/main/assets/test_classroom_id_1.textproto | 8 +++++--- domain/src/main/assets/test_classroom_id_2.textproto | 8 +++++--- .../org/oppia/android/domain/topic/TopicListController.kt | 2 +- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/domain/domain_assets.bzl b/domain/domain_assets.bzl index 39d7ea486c8..d924face7f5 100644 --- a/domain/domain_assets.bzl +++ b/domain/domain_assets.bzl @@ -31,8 +31,8 @@ def generate_assets_list_from_text_protos( name = name, names = classroom_list_file_names, proto_dep_name = "topic", - proto_type_name = "ClassroomList", - name_prefix = "classroom_list", + proto_type_name = "ClassroomIdList", + name_prefix = "classroom_id_list", asset_dir = "src/main/assets", proto_dep_bazel_target_prefix = "//model/src/main/proto", proto_package = "model", diff --git a/domain/src/main/assets/test_classroom_id_0.textproto b/domain/src/main/assets/test_classroom_id_0.textproto index 64e10685c7e..552d9a0664f 100644 --- a/domain/src/main/assets/test_classroom_id_0.textproto +++ b/domain/src/main/assets/test_classroom_id_0.textproto @@ -1,10 +1,12 @@ classroom_id: "test_classroom_id_0", -classroom_title: { +translatable_title { content_id: "classroom_title", html: "Maths" }, -thumbnail_bg_color: "#00FFFFFF", -thumbnail_filename: "", +classroom_thumbnail { + thumbnail_filename: "", + background_color_rgb: "#00FFFFFF", +} topic_prerequisites { key: "GJ2rLXRKD5hw" value { diff --git a/domain/src/main/assets/test_classroom_id_1.textproto b/domain/src/main/assets/test_classroom_id_1.textproto index 66caea11775..469a82f2686 100644 --- a/domain/src/main/assets/test_classroom_id_1.textproto +++ b/domain/src/main/assets/test_classroom_id_1.textproto @@ -1,10 +1,12 @@ classroom_id: "test_classroom_id_1", -classroom_title: { +translatable_title { content_id: "classroom_title", html: "Science" }, -thumbnail_bg_color: "#00FFFFFF", -thumbnail_filename: "", +classroom_thumbnail { + thumbnail_filename: "", + background_color_rgb: "#00FFFFFF", +} topic_prerequisites { key: "test_topic_id_0" value { diff --git a/domain/src/main/assets/test_classroom_id_2.textproto b/domain/src/main/assets/test_classroom_id_2.textproto index baa3c0c9cc4..6333230e79f 100644 --- a/domain/src/main/assets/test_classroom_id_2.textproto +++ b/domain/src/main/assets/test_classroom_id_2.textproto @@ -1,10 +1,12 @@ classroom_id: "test_classroom_id_2", -classroom_title: { +translatable_title { content_id: "classroom_title", html: "English" }, -thumbnail_bg_color: "#00FFFFFF", -thumbnail_filename: "", +classroom_thumbnail { + thumbnail_filename: "", + background_color_rgb: "#00FFFFFF", +} topic_prerequisites { key: "test_topic_id_2" value { diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index a4681215239..5ccd9e4994b 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -837,7 +837,7 @@ class TopicListController @Inject constructor( val defaultClassroomId = assetRepository.loadProtoFromLocalAssets( assetName = "classrooms", baseMessage = ClassroomIdList.getDefaultInstance() - ).getClassroomIds(0) // Only one record is currently loaded. + ).classroomIdsList[0] // Only one record is currently loaded. assetRepository.loadProtoFromLocalAssets( assetName = defaultClassroomId, baseMessage = ClassroomRecord.getDefaultInstance() From dbe98423cdaad474f14f3c261fc234a42de6119c Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Fri, 31 May 2024 08:21:57 +0530 Subject: [PATCH 10/36] TopicListController is updated to load all topics irrespective of the classroom. IDs of Science & Maths classrooms is interchanged to make the relative ordering of the topics same as previous. This is required by several UI tests to pass. --- domain/src/main/assets/GJ2rLXRKD5hw.json | 2 +- domain/src/main/assets/GJ2rLXRKD5hw.textproto | 2 +- domain/src/main/assets/omzF4oqgeTXd.json | 2 +- domain/src/main/assets/omzF4oqgeTXd.textproto | 2 +- .../src/main/assets/test_classroom_id_0.json | 6 +- .../main/assets/test_classroom_id_0.textproto | 9 +- .../src/main/assets/test_classroom_id_1.json | 6 +- .../main/assets/test_classroom_id_1.textproto | 9 +- domain/src/main/assets/test_topic_id_0.json | 2 +- .../src/main/assets/test_topic_id_0.textproto | 2 +- domain/src/main/assets/test_topic_id_1.json | 2 +- .../src/main/assets/test_topic_id_1.textproto | 2 +- .../domain/topic/TopicListController.kt | 109 ++++++++++-------- .../domain/topic/TopicListControllerTest.kt | 66 ++++------- 14 files changed, 107 insertions(+), 114 deletions(-) diff --git a/domain/src/main/assets/GJ2rLXRKD5hw.json b/domain/src/main/assets/GJ2rLXRKD5hw.json index ae22cbf45c8..39987949e3a 100644 --- a/domain/src/main/assets/GJ2rLXRKD5hw.json +++ b/domain/src/main/assets/GJ2rLXRKD5hw.json @@ -13,7 +13,7 @@ "topic_name": "Fractions", "topic_description": "You'll often need to talk about part of an object or group. For example, a jar of milk might be half-full, or some of the eggs in a box might have broken. In these lessons, you'll learn to use fractions to describe situations like these.", "topic_id": "GJ2rLXRKD5hw", - "classroom_id": "test_classroom_id_0", + "classroom_id": "test_classroom_id_1", "classroom_name": "Maths", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", diff --git a/domain/src/main/assets/GJ2rLXRKD5hw.textproto b/domain/src/main/assets/GJ2rLXRKD5hw.textproto index 3f469760138..6f5bc4a307a 100644 --- a/domain/src/main/assets/GJ2rLXRKD5hw.textproto +++ b/domain/src/main/assets/GJ2rLXRKD5hw.textproto @@ -25,7 +25,7 @@ translatable_description { html: "You\'ll often need to talk about part of an object or group. For example, a jar of milk might be half-full, or some of the eggs in a box might have broken. In these lessons, you\'ll learn to use fractions to describe situations like these." content_id: "description" } -classroom_id: "test_classroom_id_0" +classroom_id: "test_classroom_id_1" translatable_classroom_title { html: "Maths" content_id: "classroom_title" diff --git a/domain/src/main/assets/omzF4oqgeTXd.json b/domain/src/main/assets/omzF4oqgeTXd.json index 724af2c88f6..c2755332bdc 100644 --- a/domain/src/main/assets/omzF4oqgeTXd.json +++ b/domain/src/main/assets/omzF4oqgeTXd.json @@ -21,7 +21,7 @@ "topic_name": "Ratios and Proportional Reasoning", "topic_description": "Many everyday problems involve thinking about proportions. For example, if 30 apples cost 6 dollars, how much would 12 apples cost? In these lessons, you'll learn about how to use ratios and proportional reasoning to solve problems like this.", "topic_id": "omzF4oqgeTXd", - "classroom_id": "test_classroom_id_0", + "classroom_id": "test_classroom_id_1", "classroom_name": "Maths", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", diff --git a/domain/src/main/assets/omzF4oqgeTXd.textproto b/domain/src/main/assets/omzF4oqgeTXd.textproto index 6ee875cc912..d50da8a980a 100644 --- a/domain/src/main/assets/omzF4oqgeTXd.textproto +++ b/domain/src/main/assets/omzF4oqgeTXd.textproto @@ -23,7 +23,7 @@ translatable_description { html: "Many everyday problems involve thinking about proportions. For example, if 30 apples cost 6 dollars, how much would 12 apples cost? In these lessons, you\'ll learn about how to use ratios and proportional reasoning to solve problems like this." content_id: "description" } -classroom_id: "test_classroom_id_0" +classroom_id: "test_classroom_id_1" translatable_classroom_title { html: "Maths" content_id: "classroom_title" diff --git a/domain/src/main/assets/test_classroom_id_0.json b/domain/src/main/assets/test_classroom_id_0.json index 041b44929c9..97c33f70bf2 100644 --- a/domain/src/main/assets/test_classroom_id_0.json +++ b/domain/src/main/assets/test_classroom_id_0.json @@ -2,12 +2,12 @@ "classroom_id": "test_classroom_id_0", "classroom_title": { "content_id": "classroom_title", - "html": "Maths" + "html": "Science" }, "thumbnail_bg_color": "#00FFFFFF", "thumbnail_filename": "", "topic_prerequisites": { - "GJ2rLXRKD5hw": [], - "omzF4oqgeTXd": [] + "test_topic_id_0": ["GJ2rLXRKD5hw"], + "test_topic_id_1": ["test_topic_id_0", "omzF4oqgeTXd"] } } diff --git a/domain/src/main/assets/test_classroom_id_0.textproto b/domain/src/main/assets/test_classroom_id_0.textproto index 552d9a0664f..39acf642db8 100644 --- a/domain/src/main/assets/test_classroom_id_0.textproto +++ b/domain/src/main/assets/test_classroom_id_0.textproto @@ -1,19 +1,22 @@ classroom_id: "test_classroom_id_0", translatable_title { content_id: "classroom_title", - html: "Maths" + html: "Science" }, classroom_thumbnail { thumbnail_filename: "", background_color_rgb: "#00FFFFFF", } topic_prerequisites { - key: "GJ2rLXRKD5hw" + key: "test_topic_id_0" value { + topic_ids: "GJ2rLXRKD5hw" } } topic_prerequisites { - key: "omzF4oqgeTXd" + key: "test_topic_id_1" value { + topic_ids: "test_topic_id_0" + topic_ids: "omzF4oqgeTXd" } } diff --git a/domain/src/main/assets/test_classroom_id_1.json b/domain/src/main/assets/test_classroom_id_1.json index 7f1d59b9c1d..53043956ff0 100644 --- a/domain/src/main/assets/test_classroom_id_1.json +++ b/domain/src/main/assets/test_classroom_id_1.json @@ -2,12 +2,12 @@ "classroom_id": "test_classroom_id_1", "classroom_title": { "content_id": "classroom_title", - "html": "Science" + "html": "Maths" }, "thumbnail_bg_color": "#00FFFFFF", "thumbnail_filename": "", "topic_prerequisites": { - "test_topic_id_0": ["GJ2rLXRKD5hw"], - "test_topic_id_1": ["test_topic_id_0", "omzF4oqgeTXd"] + "GJ2rLXRKD5hw": [], + "omzF4oqgeTXd": [] } } diff --git a/domain/src/main/assets/test_classroom_id_1.textproto b/domain/src/main/assets/test_classroom_id_1.textproto index 469a82f2686..409b760b5b2 100644 --- a/domain/src/main/assets/test_classroom_id_1.textproto +++ b/domain/src/main/assets/test_classroom_id_1.textproto @@ -1,22 +1,19 @@ classroom_id: "test_classroom_id_1", translatable_title { content_id: "classroom_title", - html: "Science" + html: "Maths" }, classroom_thumbnail { thumbnail_filename: "", background_color_rgb: "#00FFFFFF", } topic_prerequisites { - key: "test_topic_id_0" + key: "GJ2rLXRKD5hw" value { - topic_ids: "GJ2rLXRKD5hw" } } topic_prerequisites { - key: "test_topic_id_1" + key: "omzF4oqgeTXd" value { - topic_ids: "test_topic_id_0" - topic_ids: "omzF4oqgeTXd" } } diff --git a/domain/src/main/assets/test_topic_id_0.json b/domain/src/main/assets/test_topic_id_0.json index 1e07a9553a2..d0893eeb7dc 100644 --- a/domain/src/main/assets/test_topic_id_0.json +++ b/domain/src/main/assets/test_topic_id_0.json @@ -13,7 +13,7 @@ "topic_name": "First Test Topic", "topic_description": "A topic investigating the interesting aspects of the Oppia Android app.", "topic_id": "test_topic_id_0", - "classroom_id": "test_classroom_id_1", + "classroom_id": "test_classroom_id_0", "classroom_name": "Science", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", diff --git a/domain/src/main/assets/test_topic_id_0.textproto b/domain/src/main/assets/test_topic_id_0.textproto index 16c6a4222b3..2109c268e4f 100644 --- a/domain/src/main/assets/test_topic_id_0.textproto +++ b/domain/src/main/assets/test_topic_id_0.textproto @@ -22,7 +22,7 @@ translatable_description { html: "A topic investigating the interesting aspects of the Oppia Android app." content_id: "description" } -classroom_id: "test_classroom_id_1" +classroom_id: "test_classroom_id_0" translatable_classroom_title { html: "Science" content_id: "classroom_title" diff --git a/domain/src/main/assets/test_topic_id_1.json b/domain/src/main/assets/test_topic_id_1.json index fb8d038e3ee..05ddb34ac12 100644 --- a/domain/src/main/assets/test_topic_id_1.json +++ b/domain/src/main/assets/test_topic_id_1.json @@ -13,7 +13,7 @@ "topic_name": "Second Test Topic", "topic_description": "A topic considering the various implications of having especially long topic descriptions. These descriptions almost certainly need to wrap, which should be interesting in the UI (especially on small screens). Consider also that there may even be multiple points pertaining to a topic, some of which may require expanding the description section in order to read the whole topic description.", "topic_id": "test_topic_id_1", - "classroom_id": "test_classroom_id_1", + "classroom_id": "test_classroom_id_0", "classroom_name": "Science", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", diff --git a/domain/src/main/assets/test_topic_id_1.textproto b/domain/src/main/assets/test_topic_id_1.textproto index 23c0195d9a5..5de5bf665f3 100644 --- a/domain/src/main/assets/test_topic_id_1.textproto +++ b/domain/src/main/assets/test_topic_id_1.textproto @@ -21,7 +21,7 @@ translatable_description { html: "A topic considering the various implications of having especially long topic descriptions. These descriptions almost certainly need to wrap, which should be interesting in the UI (especially on small screens). Consider also that there may even be multiple points pertaining to a topic, some of which may require expanding the description section in order to read the whole topic description." content_id: "description" } -classroom_id: "test_classroom_id_1" +classroom_id: "test_classroom_id_0" translatable_classroom_title { html: "Science" content_id: "classroom_title" diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index 5ccd9e4994b..76db4871a7d 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -141,7 +141,7 @@ class TopicListController @Inject constructor( private fun createTopicList(contentLocale: OppiaLocale.ContentLocale): TopicList { return if (loadLessonProtosFromAssets) { - val topicIdList = loadCombinedClassroomTopicList() + val topicIdList = loadCombinedClassroomsTopicIdList() return TopicList.newBuilder().apply { // Only include topics currently playable in the topic list. addAllTopicSummary( @@ -156,7 +156,7 @@ class TopicListController @Inject constructor( } private fun loadTopicListFromJson(contentLocale: OppiaLocale.ContentLocale): TopicList { - val topicIdList = loadCombinedClassroomTopicList() + val topicIdList = loadCombinedClassroomsTopicIdList() val topicListBuilder = TopicList.newBuilder() for (topicId in topicIdList) { val ephemeralSummary = createEphemeralTopicSummary(topicId, contentLocale) @@ -170,7 +170,7 @@ class TopicListController @Inject constructor( } private fun computeComingSoonTopicList(): ComingSoonTopicList { - val topicIdList = loadCombinedClassroomTopicList() + val topicIdList = loadCombinedClassroomsTopicIdList() val comingSoonTopicListBuilder = ComingSoonTopicList.newBuilder() for (topicId in topicIdList) { val upcomingTopicSummary = createUpcomingTopicSummary(topicId) @@ -545,8 +545,15 @@ class TopicListController @Inject constructor( * Returns a list of topic IDs for which the specified topic ID expects to be completed before * being suggested. */ - private fun retrieveTopicDependencies(topicId: String): List = - loadClassroom().topicPrerequisitesMap.getValue(topicId).topicIdsList + private fun retrieveTopicDependencies(topicId: String): List { + val classrooms = loadClassrooms() + for (classroom in classrooms) { + if (classroom.topicPrerequisitesMap.containsKey(topicId)) { + return classroom.topicPrerequisitesMap.getValue(topicId).topicIdsList + } + } + throw IllegalArgumentException("Topic ID $topicId not found in any classroom.") + } /* * Explanation for logic: @@ -572,7 +579,7 @@ class TopicListController @Inject constructor( contentLocale: OppiaLocale.ContentLocale ): List { return if (loadLessonProtosFromAssets) { - val topicIdList = loadCombinedClassroomTopicList() + val topicIdList = loadCombinedClassroomsTopicIdList() return computeSuggestedStoriesForTopicIds(topicProgressList, topicIdList, contentLocale) } else computeSuggestedStoriesFromJson(topicProgressList, contentLocale) } @@ -582,7 +589,7 @@ class TopicListController @Inject constructor( contentLocale: OppiaLocale.ContentLocale ): List { // All topics that could potentially be recommended. - val topicIdList = loadCombinedClassroomTopicList() + val topicIdList = loadCombinedClassroomsTopicIdList() return computeSuggestedStoriesForTopicIds(topicProgressList, topicIdList, contentLocale) } @@ -832,65 +839,75 @@ class TopicListController @Inject constructor( } // TODO(#5344): Remove this in favor of per-classroom data handling. - private fun loadClassroom(): ClassroomRecord { + private fun loadClassrooms(): List { return if (loadLessonProtosFromAssets) { - val defaultClassroomId = assetRepository.loadProtoFromLocalAssets( + assetRepository.loadProtoFromLocalAssets( assetName = "classrooms", baseMessage = ClassroomIdList.getDefaultInstance() - ).classroomIdsList[0] // Only one record is currently loaded. - assetRepository.loadProtoFromLocalAssets( - assetName = defaultClassroomId, - baseMessage = ClassroomRecord.getDefaultInstance() - ) - } else loadClassroomFromJson() + ).classroomIdsList.map { + assetRepository.loadProtoFromLocalAssets( + assetName = it, + baseMessage = ClassroomRecord.getDefaultInstance() + ) + } + } else loadClassroomsFromJson() } // TODO(#5344): Remove this in favor of per-classroom data handling. - private fun loadClassroomFromJson(): ClassroomRecord { + private fun loadClassroomsFromJson(): List { // 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 missing classroom IDs." } - // Fetch the first classroom ID of the list. - val defaultClassroomId = checkNotNull(classroomIds.get(0)) { - "Expected non-null classroom ID." - } + // Initialize a list to store the classroom records. + val classroomRecords = mutableListOf() - // Load the classroom obj of the first classroom. - val defaultClassroomObj = jsonAssetRetriever.loadJsonFromAsset("$defaultClassroomId.json") - checkNotNull(defaultClassroomObj) { "Failed to load $defaultClassroomId.json." } - val topicPrereqsObj = checkNotNull(defaultClassroomObj.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." + // Iterate over all classroom IDs 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." } - return@associateWith List(topicIdArray.length()) { index -> - checkNotNull(topicIdArray.optString(index)) { - "Expected topic $topicId to have non-null string at index $index." - } + + // Load the classroom obj of the current classroom. + val classroomObj = jsonAssetRetriever.loadJsonFromAsset("$classroomId.json") + checkNotNull(classroomObj) { "Failed to load $classroomId.json." } + val topicPrereqsObj = checkNotNull(classroomObj.optJSONObject("topic_prerequisites")) { + "Expected classroom to have non-null topic_prerequisites." } - } - return ClassroomRecord.newBuilder().apply { - this.id = checkNotNull(defaultClassroomObj.optString("id")) { - "Expected classroom to have ID." + 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." + } + } } - this.putAllTopicPrerequisites( - topicPrereqs.mapValues { (_, topicIds) -> - TopicIdList.newBuilder().apply { - addAllTopicIds(topicIds) - }.build() + val classroomRecord = ClassroomRecord.newBuilder().apply { + this.id = checkNotNull(classroomObj.optString("id")) { + "Expected classroom to have ID." } - ) - }.build() + this.putAllTopicPrerequisites( + topicPrereqs.mapValues { (_, topicIds) -> + TopicIdList.newBuilder().apply { + addAllTopicIds(topicIds) + }.build() + } + ) + }.build() + + classroomRecords.add(classroomRecord) + } + + return classroomRecords } // TODO(#5344): Remove this in favor of per-classroom data handling. - private fun loadCombinedClassroomTopicList(): List = - loadClassroom().topicPrerequisitesMap.keys.toList() + private fun loadCombinedClassroomsTopicIdList(): List = + loadClassrooms().flatMap { it.topicPrerequisitesMap.keys.toList() } } internal fun createTopicThumbnailFromJson(topicJsonObject: JSONObject): LessonThumbnail { diff --git a/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt index 5e32e0e2b98..50dc4714202 100644 --- a/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt @@ -101,8 +101,6 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing due to changes in data files") - // TODO(#5344): Migrate the test to ClassroomControllerTest. fun testRetrieveTopicList_firstTopic_hasCorrectTopicInfo() { val topicList = retrieveTopicList() @@ -112,19 +110,15 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing due to changes in data files") - // TODO(#5344): Migrate the test to ClassroomControllerTest. fun testRetrieveTopicList_firstTopic_hasCorrectClassroomInfo() { val topicList = retrieveTopicList() val firstTopic = topicList.getTopicSummary(0).topicSummary - assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) assertThat(firstTopic.classroomTitle.html).isEqualTo("Science") } @Test - @Ignore("Test failing due to changes in data files") - // TODO(#5344): Migrate the test to ClassroomControllerTest. fun testRetrieveTopicList_firstTopic_hasCorrectLessonCount() { val topicList = retrieveTopicList() @@ -133,8 +127,6 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing due to changes in data files") - // TODO(#5344): Migrate the test to ClassroomControllerTest. fun testRetrieveTopicList_secondTopic_hasCorrectTopicInfo() { val topicList = retrieveTopicList() @@ -144,19 +136,15 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing due to changes in data files") - // TODO(#5344): Migrate the test to ClassroomControllerTest. fun testRetrieveTopicList_secondTopic_hasCorrectClassroomInfo() { val topicList = retrieveTopicList() val firstTopic = topicList.getTopicSummary(1).topicSummary - assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) assertThat(firstTopic.classroomTitle.html).isEqualTo("Science") } @Test - @Ignore("Test failing due to changes in data files") - // TODO(#5344): Migrate the test to ClassroomControllerTest. fun testRetrieveTopicList_secondTopic_hasCorrectLessonCount() { val topicList = retrieveTopicList() @@ -168,7 +156,7 @@ class TopicListControllerTest { fun testRetrieveTopicList_fractionsTopic_hasCorrectTopicInfo() { val topicList = retrieveTopicList() - val fractionsTopic = topicList.getTopicSummary(0).topicSummary + val fractionsTopic = topicList.getTopicSummary(2).topicSummary assertThat(fractionsTopic.topicId).isEqualTo(FRACTIONS_TOPIC_ID) assertThat(fractionsTopic.title.html).isEqualTo("Fractions") } @@ -177,8 +165,8 @@ class TopicListControllerTest { fun testRetrieveTopicList_fractionsTopic_hasCorrectClassroomInfo() { val topicList = retrieveTopicList() - val firstTopic = topicList.getTopicSummary(0).topicSummary - assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + val firstTopic = topicList.getTopicSummary(2).topicSummary + assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(firstTopic.classroomTitle.html).isEqualTo("Maths") } @@ -186,7 +174,7 @@ class TopicListControllerTest { fun testRetrieveTopicList_fractionsTopic_hasCorrectLessonCount() { val topicList = retrieveTopicList() - val fractionsTopic = topicList.getTopicSummary(0).topicSummary + val fractionsTopic = topicList.getTopicSummary(2).topicSummary assertThat(fractionsTopic.totalChapterCount).isEqualTo(2) } @@ -194,7 +182,7 @@ class TopicListControllerTest { fun testRetrieveTopicList_ratiosTopic_hasCorrectTopicInfo() { val topicList = retrieveTopicList() - val ratiosTopic = topicList.getTopicSummary(1).topicSummary + val ratiosTopic = topicList.getTopicSummary(3).topicSummary assertThat(ratiosTopic.topicId).isEqualTo(RATIOS_TOPIC_ID) assertThat(ratiosTopic.title.html).isEqualTo("Ratios and Proportional Reasoning") } @@ -203,8 +191,8 @@ class TopicListControllerTest { fun testRetrieveTopicList_ratiosTopic_hasCorrectClassroomInfo() { val topicList = retrieveTopicList() - val firstTopic = topicList.getTopicSummary(1).topicSummary - assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + val firstTopic = topicList.getTopicSummary(3).topicSummary + assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(firstTopic.classroomTitle.html).isEqualTo("Maths") } @@ -212,7 +200,7 @@ class TopicListControllerTest { fun testRetrieveTopicList_ratiosTopic_hasCorrectLessonCount() { val topicList = retrieveTopicList() - val ratiosTopic = topicList.getTopicSummary(1).topicSummary + val ratiosTopic = topicList.getTopicSummary(3).topicSummary assertThat(ratiosTopic.totalChapterCount).isEqualTo(4) } @@ -334,8 +322,6 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing because only default classroom topics (Fractions & Ratios) are considered") - // TODO(#5344): Update recommendation logic. fun testGetPromotedStoryList_markAllChapsDoneInFractions_suggestedStoryListIsCorrect() { storyProgressTestHelper.markCompletedFractionsStory0( profileId0, @@ -393,8 +379,6 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing because only default classroom topics (Fractions & Ratios) are considered") - // TODO(#5344): Update recommendation logic. fun testGetStoryList_markStoryDoneOfRatiosAndFirstTestTopic_suggestedStoryListIsCorrect() { storyProgressTestHelper.markCompletedTestTopic0Story0( profileId0, @@ -457,8 +441,6 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing because only default classroom topics (Fractions & Ratios) are considered") - // TODO(#5344): Update recommendation logic. fun testRetrievePromotedActivityList_markAllChapDoneInAllTopics_comingSoonTopicListIsCorrect() { storyProgressTestHelper.markAllTopicsAsCompleted( profileId0, @@ -472,8 +454,6 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing because only default classroom topics (Fractions & Ratios) are considered") - // TODO(#5344): Update recommendation logic. fun testGetStoryList_markAllChapDoneInSecondTestTopic_doesNotPromoteAnyStories() { storyProgressTestHelper.markCompletedTestTopic1Story0( profileId0, @@ -490,8 +470,6 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing because only default classroom topics (Fractions & Ratios) are considered") - // TODO(#5344): Update recommendation logic. fun testGetStoryList_markAllChapDoneInSecondTestTopic_comingSoonTopicListIsCorrect() { storyProgressTestHelper.markCompletedTestTopic1Story0( profileId0, @@ -584,8 +562,6 @@ class TopicListControllerTest { } @Test - @Ignore("Test failing because only default classroom topics (Fractions & Ratios) are considered") - // TODO(#5344): Update recommendation logic. fun testGetStoryList_markOneStoryDoneForFirstTestTopic_suggestedStoryListIsCorrect() { storyProgressTestHelper.markCompletedTestTopic0Story0( profileId0, @@ -740,7 +716,7 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(TEST_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(TEST_TOPIC_ID_0) assertThat(promotedStory.topicTitle.html).isEqualTo("First Test Topic") - assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) assertThat(promotedStory.classroomTitle.html).isEqualTo("Science") assertThat(promotedStory.nextChapterTitle.html).isEqualTo("Prototype Exploration") assertThat(promotedStory.completedChapterCount).isEqualTo(0) @@ -753,7 +729,7 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(TEST_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(TEST_TOPIC_ID_0) assertThat(promotedStory.topicTitle.html).isEqualTo("First Test Topic") - assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) assertThat(promotedStory.classroomTitle.html).isEqualTo("Science") assertThat(promotedStory.nextChapterTitle.html).isEqualTo("Prototype Exploration") assertThat(promotedStory.completedChapterCount).isEqualTo(0) @@ -766,7 +742,7 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(TEST_STORY_ID_2) assertThat(promotedStory.topicId).isEqualTo(TEST_TOPIC_ID_1) assertThat(promotedStory.topicTitle.html).isEqualTo("Second Test Topic") - assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) assertThat(promotedStory.classroomTitle.html).isEqualTo("Science") assertThat(promotedStory.nextChapterTitle.html).isEqualTo("Fifth Exploration") assertThat(promotedStory.completedChapterCount).isEqualTo(0) @@ -779,7 +755,7 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(FRACTIONS_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(FRACTIONS_TOPIC_ID) assertThat(promotedStory.topicTitle.html).isEqualTo("Fractions") - assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.nextChapterTitle.html).isEqualTo("What is a Fraction?") assertThat(promotedStory.completedChapterCount).isEqualTo(0) @@ -792,7 +768,7 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(FRACTIONS_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(FRACTIONS_TOPIC_ID) assertThat(promotedStory.topicTitle.html).isEqualTo("Fractions") - assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.nextChapterTitle.html).isEqualTo("What is a Fraction?") assertThat(promotedStory.completedChapterCount).isEqualTo(0) @@ -805,7 +781,7 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(FRACTIONS_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(FRACTIONS_TOPIC_ID) assertThat(promotedStory.topicTitle.html).isEqualTo("Fractions") - assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.nextChapterTitle.html).isEqualTo("The Meaning of Equal Parts") assertThat(promotedStory.completedChapterCount).isEqualTo(1) @@ -817,7 +793,7 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(RATIOS_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(RATIOS_TOPIC_ID) assertThat(promotedStory.nextChapterTitle.html).isEqualTo("What is a Ratio?") - assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.topicTitle.html).isEqualTo("Ratios and Proportional Reasoning") assertThat(promotedStory.completedChapterCount).isEqualTo(0) @@ -830,7 +806,7 @@ class TopicListControllerTest { assertThat(promotedStory.storyId).isEqualTo(RATIOS_STORY_ID_0) assertThat(promotedStory.topicId).isEqualTo(RATIOS_TOPIC_ID) assertThat(promotedStory.nextChapterTitle.html).isEqualTo("What is a Ratio?") - assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.topicTitle.html).isEqualTo("Ratios and Proportional Reasoning") assertThat(promotedStory.completedChapterCount).isEqualTo(0) @@ -844,7 +820,7 @@ class TopicListControllerTest { assertThat(promotedStory.topicId).isEqualTo(RATIOS_TOPIC_ID) assertThat(promotedStory.nextChapterTitle.html).isEqualTo("Order is important") assertThat(promotedStory.topicTitle.html).isEqualTo("Ratios and Proportional Reasoning") - assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.completedChapterCount).isEqualTo(1) assertThat(promotedStory.isTopicLearned).isFalse() @@ -867,7 +843,7 @@ class TopicListControllerTest { assertThat(promotedStory.topicId).isEqualTo(RATIOS_TOPIC_ID) assertThat(promotedStory.nextChapterTitle.html).isEqualTo("Equivalent Ratios") assertThat(promotedStory.topicTitle.html).isEqualTo("Ratios and Proportional Reasoning") - assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.completedChapterCount).isEqualTo(0) assertThat(promotedStory.isTopicLearned).isEqualTo(expectedToBeLearned) @@ -880,7 +856,7 @@ class TopicListControllerTest { assertThat(promotedStory.topicId).isEqualTo(RATIOS_TOPIC_ID) assertThat(promotedStory.nextChapterTitle.html).isEqualTo("Writing Ratios in Simplest Form") assertThat(promotedStory.topicTitle.html).isEqualTo("Ratios and Proportional Reasoning") - assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(promotedStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(promotedStory.classroomTitle.html).isEqualTo("Maths") assertThat(promotedStory.completedChapterCount).isEqualTo(1) assertThat(promotedStory.isTopicLearned).isFalse() From 238d96cb4cc226d4b7aa7e259dca3ed7f60b5f6c Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Fri, 31 May 2024 08:26:30 +0530 Subject: [PATCH 11/36] Fix failing tests of TopicControllerTest --- .../org/oppia/android/domain/topic/TopicControllerTest.kt | 8 ++++---- .../oppia/android/domain/topic/TopicListControllerTest.kt | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt index 0a1b6cb3c95..7ef3a305f43 100755 --- a/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt @@ -135,7 +135,7 @@ class TopicControllerTest { val topicProvider = topicController.getTopic(profileId1, FRACTIONS_TOPIC_ID) val topic = monitorFactory.waitForNextSuccessfulResult(topicProvider).topic - assertThat(topic.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(topic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(topic.classroomTitle.html).contains("Maths") } @@ -164,7 +164,7 @@ class TopicControllerTest { val topicProvider = topicController.getTopic(profileId1, RATIOS_TOPIC_ID) val topic = monitorFactory.waitForNextSuccessfulResult(topicProvider).topic - assertThat(topic.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(topic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(topic.classroomTitle.html).contains("Maths") } @@ -907,11 +907,11 @@ class TopicControllerTest { val completedStoryList = monitorFactory.waitForNextSuccessfulResult(storyList) assertThat(completedStoryList.completedStoryCount).isEqualTo(2) completedStoryList.completedStoryList[0].also { completedFractionsStory -> - assertThat(completedFractionsStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(completedFractionsStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(completedFractionsStory.classroomTitle.html).isEqualTo("Maths") } completedStoryList.completedStoryList[1].also { completedRatiosStory -> - assertThat(completedRatiosStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + assertThat(completedRatiosStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(completedRatiosStory.classroomTitle.html).isEqualTo("Maths") } } diff --git a/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt index 50dc4714202..74ac9733da9 100644 --- a/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt @@ -10,7 +10,6 @@ import dagger.Component import dagger.Module import dagger.Provides import org.junit.Before -import org.junit.Ignore import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith From d5a8bc867594f4da98bed23ebc25e3ef38b2d4e0 Mon Sep 17 00:00:00 2001 From: "Mr. 17" Date: Fri, 31 May 2024 14:28:40 +0530 Subject: [PATCH 12/36] Update test_classroom_id_0.textproto --- domain/src/main/assets/test_classroom_id_0.textproto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/domain/src/main/assets/test_classroom_id_0.textproto b/domain/src/main/assets/test_classroom_id_0.textproto index 39acf642db8..c48ebd1a692 100644 --- a/domain/src/main/assets/test_classroom_id_0.textproto +++ b/domain/src/main/assets/test_classroom_id_0.textproto @@ -1,4 +1,4 @@ -classroom_id: "test_classroom_id_0", +id: "test_classroom_id_0", translatable_title { content_id: "classroom_title", html: "Science" From db676669bffc3f813e4675c71de44ccdba97d6df Mon Sep 17 00:00:00 2001 From: "Mr. 17" Date: Fri, 31 May 2024 14:29:15 +0530 Subject: [PATCH 13/36] Update test_classroom_id_1.textproto --- domain/src/main/assets/test_classroom_id_1.textproto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/domain/src/main/assets/test_classroom_id_1.textproto b/domain/src/main/assets/test_classroom_id_1.textproto index 409b760b5b2..13967de6124 100644 --- a/domain/src/main/assets/test_classroom_id_1.textproto +++ b/domain/src/main/assets/test_classroom_id_1.textproto @@ -1,4 +1,4 @@ -classroom_id: "test_classroom_id_1", +id: "test_classroom_id_1", translatable_title { content_id: "classroom_title", html: "Maths" From 405f1600d6865bd7dd0c9fa3b05551e541f9aedd Mon Sep 17 00:00:00 2001 From: "Mr. 17" Date: Fri, 31 May 2024 14:29:32 +0530 Subject: [PATCH 14/36] Update test_classroom_id_2.textproto --- domain/src/main/assets/test_classroom_id_2.textproto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/domain/src/main/assets/test_classroom_id_2.textproto b/domain/src/main/assets/test_classroom_id_2.textproto index 6333230e79f..972a156d6fe 100644 --- a/domain/src/main/assets/test_classroom_id_2.textproto +++ b/domain/src/main/assets/test_classroom_id_2.textproto @@ -1,4 +1,4 @@ -classroom_id: "test_classroom_id_2", +id: "test_classroom_id_2", translatable_title { content_id: "classroom_title", html: "English" From ff3444fcac88031a899634758e12358a7a8b4d88 Mon Sep 17 00:00:00 2001 From: "Mr. 17" Date: Fri, 31 May 2024 15:45:29 +0530 Subject: [PATCH 15/36] Add classroom file names in domain assets list --- domain/BUILD.bazel | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/domain/BUILD.bazel b/domain/BUILD.bazel index 0ff999536a3..b37faf8bc87 100755 --- a/domain/BUILD.bazel +++ b/domain/BUILD.bazel @@ -37,6 +37,11 @@ MIGRATED_PROD_FILES = glob([ DOMAIN_ASSETS = generate_assets_list_from_text_protos( name = "domain_assets", + classroom_file_names = [ + "test_classroom_id_0", + "test_classroom_id_1", + "test_classroom_id_2", + ], classroom_list_file_names = [ "classrooms", ], From 4c5f322d6648b571ba3a2df9eef45c764c8338c7 Mon Sep 17 00:00:00 2001 From: "Mr. 17" Date: Fri, 31 May 2024 15:51:18 +0530 Subject: [PATCH 16/36] Update domain_assets.bzl --- domain/domain_assets.bzl | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/domain/domain_assets.bzl b/domain/domain_assets.bzl index d924face7f5..9e56affa694 100644 --- a/domain/domain_assets.bzl +++ b/domain/domain_assets.bzl @@ -6,6 +6,7 @@ load("//model:text_proto_assets.bzl", "generate_proto_binary_assets") def generate_assets_list_from_text_protos( name, + classroom_file_names, classroom_list_file_names, topic_file_names, subtopic_file_names, @@ -17,6 +18,7 @@ def generate_assets_list_from_text_protos( Args: name: str. The name of this generation instance. This will be a prefix for derived targets. + classroom_file_names: list of str. The list of classroom file names. classroom_list_file_names: list of str. The classroom list file names. topic_file_names: list of str. The list of topic file names. subtopic_file_names: list of str. The list of subtopic file names. @@ -37,6 +39,15 @@ def generate_assets_list_from_text_protos( proto_dep_bazel_target_prefix = "//model/src/main/proto", proto_package = "model", ) + generate_proto_binary_assets( + name = name, + names = classroom_file_names, + proto_dep_name = "topic", + proto_type_name = "ClassroomRecord", + name_prefix = "classroom_record", + asset_dir = "src/main/assets", + proto_dep_bazel_target_prefix = "//model/src/main/proto", + proto_package = "model", + )+ generate_proto_binary_assets( name = name, names = topic_file_names, proto_dep_name = "topic", From 2a26a2bbffd9231cf9ff2d8ed3264d408ab7a225 Mon Sep 17 00:00:00 2001 From: "Mr. 17" Date: Fri, 31 May 2024 16:01:11 +0530 Subject: [PATCH 17/36] Remove classroom thumbnail data from test_classroom_id_0 --- domain/src/main/assets/test_classroom_id_0.textproto | 2 -- 1 file changed, 2 deletions(-) diff --git a/domain/src/main/assets/test_classroom_id_0.textproto b/domain/src/main/assets/test_classroom_id_0.textproto index c48ebd1a692..397ac9ef987 100644 --- a/domain/src/main/assets/test_classroom_id_0.textproto +++ b/domain/src/main/assets/test_classroom_id_0.textproto @@ -4,8 +4,6 @@ translatable_title { html: "Science" }, classroom_thumbnail { - thumbnail_filename: "", - background_color_rgb: "#00FFFFFF", } topic_prerequisites { key: "test_topic_id_0" From c22feadde9022e97120ee1e0fb9e5ec208359dfb Mon Sep 17 00:00:00 2001 From: "Mr. 17" Date: Fri, 31 May 2024 16:01:47 +0530 Subject: [PATCH 18/36] Remove classroom thumbnail data from test_classroom_id_1 --- domain/src/main/assets/test_classroom_id_1.textproto | 2 -- 1 file changed, 2 deletions(-) diff --git a/domain/src/main/assets/test_classroom_id_1.textproto b/domain/src/main/assets/test_classroom_id_1.textproto index 13967de6124..dfafc4a28bb 100644 --- a/domain/src/main/assets/test_classroom_id_1.textproto +++ b/domain/src/main/assets/test_classroom_id_1.textproto @@ -4,8 +4,6 @@ translatable_title { html: "Maths" }, classroom_thumbnail { - thumbnail_filename: "", - background_color_rgb: "#00FFFFFF", } topic_prerequisites { key: "GJ2rLXRKD5hw" From 6cb914609423427e3722052b96d439bf74dac97a Mon Sep 17 00:00:00 2001 From: "Mr. 17" Date: Fri, 31 May 2024 16:02:13 +0530 Subject: [PATCH 19/36] Remove classroom thumbnail data from test_classroom_id_2 --- domain/src/main/assets/test_classroom_id_2.textproto | 2 -- 1 file changed, 2 deletions(-) diff --git a/domain/src/main/assets/test_classroom_id_2.textproto b/domain/src/main/assets/test_classroom_id_2.textproto index 972a156d6fe..0b570ca4421 100644 --- a/domain/src/main/assets/test_classroom_id_2.textproto +++ b/domain/src/main/assets/test_classroom_id_2.textproto @@ -4,8 +4,6 @@ translatable_title { html: "English" }, classroom_thumbnail { - thumbnail_filename: "", - background_color_rgb: "#00FFFFFF", } topic_prerequisites { key: "test_topic_id_2" From b21420638ba264651ecd28d4f1e95e00331a7ff3 Mon Sep 17 00:00:00 2001 From: "Mr. 17" Date: Fri, 31 May 2024 16:39:54 +0530 Subject: [PATCH 20/36] Fix bazel lint issue --- domain/domain_assets.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/domain/domain_assets.bzl b/domain/domain_assets.bzl index 9e56affa694..fe853e6707e 100644 --- a/domain/domain_assets.bzl +++ b/domain/domain_assets.bzl @@ -47,7 +47,7 @@ def generate_assets_list_from_text_protos( asset_dir = "src/main/assets", proto_dep_bazel_target_prefix = "//model/src/main/proto", proto_package = "model", - )+ generate_proto_binary_assets( + ) + generate_proto_binary_assets( name = name, names = topic_file_names, proto_dep_name = "topic", From 884a67502df99ed9f04b1265aef8607c8d5b8e57 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Fri, 31 May 2024 21:22:48 +0530 Subject: [PATCH 21/36] Introduce topic_ids field for classrooms --- domain/src/main/assets/test_classroom_id_0.json | 3 ++- .../src/main/assets/test_classroom_id_0.textproto | 2 ++ domain/src/main/assets/test_classroom_id_1.json | 3 ++- .../src/main/assets/test_classroom_id_1.textproto | 2 ++ domain/src/main/assets/test_classroom_id_2.json | 3 ++- .../src/main/assets/test_classroom_id_2.textproto | 1 + .../android/domain/topic/TopicListController.kt | 15 ++++++++++++++- model/src/main/proto/topic.proto | 3 +++ 8 files changed, 28 insertions(+), 4 deletions(-) diff --git a/domain/src/main/assets/test_classroom_id_0.json b/domain/src/main/assets/test_classroom_id_0.json index 97c33f70bf2..0fafae68ccb 100644 --- a/domain/src/main/assets/test_classroom_id_0.json +++ b/domain/src/main/assets/test_classroom_id_0.json @@ -9,5 +9,6 @@ "topic_prerequisites": { "test_topic_id_0": ["GJ2rLXRKD5hw"], "test_topic_id_1": ["test_topic_id_0", "omzF4oqgeTXd"] - } + }, + "topic_ids": ["test_topic_id_0", "test_topic_id_1"] } diff --git a/domain/src/main/assets/test_classroom_id_0.textproto b/domain/src/main/assets/test_classroom_id_0.textproto index 397ac9ef987..8c4fbabaf0b 100644 --- a/domain/src/main/assets/test_classroom_id_0.textproto +++ b/domain/src/main/assets/test_classroom_id_0.textproto @@ -18,3 +18,5 @@ topic_prerequisites { topic_ids: "omzF4oqgeTXd" } } +topic_ids: "test_topic_id_0" +topic_ids: "test_topic_id_1" diff --git a/domain/src/main/assets/test_classroom_id_1.json b/domain/src/main/assets/test_classroom_id_1.json index 53043956ff0..eb0aa03b32e 100644 --- a/domain/src/main/assets/test_classroom_id_1.json +++ b/domain/src/main/assets/test_classroom_id_1.json @@ -9,5 +9,6 @@ "topic_prerequisites": { "GJ2rLXRKD5hw": [], "omzF4oqgeTXd": [] - } + }, + "topic_ids": ["GJ2rLXRKD5hw", "omzF4oqgeTXd"] } diff --git a/domain/src/main/assets/test_classroom_id_1.textproto b/domain/src/main/assets/test_classroom_id_1.textproto index dfafc4a28bb..628d3324070 100644 --- a/domain/src/main/assets/test_classroom_id_1.textproto +++ b/domain/src/main/assets/test_classroom_id_1.textproto @@ -15,3 +15,5 @@ topic_prerequisites { value { } } +topic_ids: "GJ2rLXRKD5hw" +topic_ids: "omzF4oqgeTXd" diff --git a/domain/src/main/assets/test_classroom_id_2.json b/domain/src/main/assets/test_classroom_id_2.json index 55e7e9d9fe7..5735e9597fc 100644 --- a/domain/src/main/assets/test_classroom_id_2.json +++ b/domain/src/main/assets/test_classroom_id_2.json @@ -8,5 +8,6 @@ "thumbnail_filename": "", "topic_prerequisites": { "test_topic_id_2": [] - } + }, + "topic_ids": ["test_topic_id_2"] } diff --git a/domain/src/main/assets/test_classroom_id_2.textproto b/domain/src/main/assets/test_classroom_id_2.textproto index 0b570ca4421..8803f2604c4 100644 --- a/domain/src/main/assets/test_classroom_id_2.textproto +++ b/domain/src/main/assets/test_classroom_id_2.textproto @@ -10,3 +10,4 @@ topic_prerequisites { value { } } +topic_ids: "test_topic_id_2" diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index 76db4871a7d..ceddff1d84c 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -873,6 +873,8 @@ class TopicListController @Inject constructor( // Load the classroom obj of the current classroom. val classroomObj = jsonAssetRetriever.loadJsonFromAsset("$classroomId.json") checkNotNull(classroomObj) { "Failed to load $classroomId.json." } + + // Load the topic prerequisite map of the current classroom. val topicPrereqsObj = checkNotNull(classroomObj.optJSONObject("topic_prerequisites")) { "Expected classroom to have non-null topic_prerequisites." } @@ -886,6 +888,16 @@ class TopicListController @Inject constructor( } } } + + // Load the topic ids of the current classroom. + val topicIdJsonArray = checkNotNull(classroomObj.getJSONArray("topic_ids")) { + "Expected classroom to have non-null topic_ids." + } + val topicIdListBuilder = TopicIdList.newBuilder() + for (i in 0 until topicIdJsonArray.length()) { + topicIdListBuilder.addTopicIds(topicIdJsonArray.optString(i)!!) + } + val classroomRecord = ClassroomRecord.newBuilder().apply { this.id = checkNotNull(classroomObj.optString("id")) { "Expected classroom to have ID." @@ -897,6 +909,7 @@ class TopicListController @Inject constructor( }.build() } ) + this.topicIds = topicIdListBuilder.build() }.build() classroomRecords.add(classroomRecord) @@ -907,7 +920,7 @@ class TopicListController @Inject constructor( // TODO(#5344): Remove this in favor of per-classroom data handling. private fun loadCombinedClassroomsTopicIdList(): List = - loadClassrooms().flatMap { it.topicPrerequisitesMap.keys.toList() } + loadClassrooms().flatMap { it.topicIds.topicIdsList } } internal fun createTopicThumbnailFromJson(topicJsonObject: JSONObject): LessonThumbnail { diff --git a/model/src/main/proto/topic.proto b/model/src/main/proto/topic.proto index 734bd18f2a6..fff39bcb41e 100755 --- a/model/src/main/proto/topic.proto +++ b/model/src/main/proto/topic.proto @@ -605,6 +605,9 @@ message ClassroomRecord { // include topics outside of this classroom. map topic_prerequisites = 5; + // A list of topics IDs that belong to this classroom. + TopicIdList topic_ids = 6; + // Represents a list of topic IDs (to be used in the context of topic deps in a classroom). message TopicIdList { // A list of topics IDs. From 9fbfda3ee1399664762669bc7590a0c3235a7909 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Fri, 31 May 2024 21:51:57 +0530 Subject: [PATCH 22/36] Fix textproto formatting issue --- domain/src/main/assets/test_classroom_id_0.textproto | 12 +++++++----- domain/src/main/assets/test_classroom_id_1.textproto | 12 +++++++----- domain/src/main/assets/test_classroom_id_2.textproto | 10 ++++++---- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/domain/src/main/assets/test_classroom_id_0.textproto b/domain/src/main/assets/test_classroom_id_0.textproto index 8c4fbabaf0b..1d5a8439eaa 100644 --- a/domain/src/main/assets/test_classroom_id_0.textproto +++ b/domain/src/main/assets/test_classroom_id_0.textproto @@ -1,8 +1,8 @@ -id: "test_classroom_id_0", +id: "test_classroom_id_0" translatable_title { - content_id: "classroom_title", + content_id: "classroom_title" html: "Science" -}, +} classroom_thumbnail { } topic_prerequisites { @@ -18,5 +18,7 @@ topic_prerequisites { topic_ids: "omzF4oqgeTXd" } } -topic_ids: "test_topic_id_0" -topic_ids: "test_topic_id_1" +topic_ids: { + topic_ids: "test_topic_id_0" + topic_ids: "test_topic_id_1" +} diff --git a/domain/src/main/assets/test_classroom_id_1.textproto b/domain/src/main/assets/test_classroom_id_1.textproto index 628d3324070..67746510f39 100644 --- a/domain/src/main/assets/test_classroom_id_1.textproto +++ b/domain/src/main/assets/test_classroom_id_1.textproto @@ -1,8 +1,8 @@ -id: "test_classroom_id_1", +id: "test_classroom_id_1" translatable_title { - content_id: "classroom_title", + content_id: "classroom_title" html: "Maths" -}, +} classroom_thumbnail { } topic_prerequisites { @@ -15,5 +15,7 @@ topic_prerequisites { value { } } -topic_ids: "GJ2rLXRKD5hw" -topic_ids: "omzF4oqgeTXd" +topic_ids: { + topic_ids: "GJ2rLXRKD5hw" + topic_ids: "omzF4oqgeTXd" +} diff --git a/domain/src/main/assets/test_classroom_id_2.textproto b/domain/src/main/assets/test_classroom_id_2.textproto index 8803f2604c4..b11f4b903b8 100644 --- a/domain/src/main/assets/test_classroom_id_2.textproto +++ b/domain/src/main/assets/test_classroom_id_2.textproto @@ -1,8 +1,8 @@ -id: "test_classroom_id_2", +id: "test_classroom_id_2" translatable_title { - content_id: "classroom_title", + content_id: "classroom_title" html: "English" -}, +} classroom_thumbnail { } topic_prerequisites { @@ -10,4 +10,6 @@ topic_prerequisites { value { } } -topic_ids: "test_topic_id_2" +topic_ids: { + topic_ids: "test_topic_id_2" +} From e1c9254fb91eade9296d35e28c7242543f70e4ee Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Wed, 5 Jun 2024 18:12:30 +0530 Subject: [PATCH 23/36] Add EphemeralClassroomSummary & ClassroomList objects --- model/src/main/proto/topic.proto | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/model/src/main/proto/topic.proto b/model/src/main/proto/topic.proto index fff39bcb41e..3498e847576 100755 --- a/model/src/main/proto/topic.proto +++ b/model/src/main/proto/topic.proto @@ -634,6 +634,21 @@ message ClassroomSummary { repeated TopicSummary topic_summary = 5; } +// Corresponds to a classroom summary that is currently being viewed. +message EphemeralClassroomSummary { + // The classroom summary to view. + ClassroomSummary classroom_summary = 1; + + // The translation context that should be used for this classroom summary. + WrittenTranslationContext written_translation_context = 2; +} + +// Corresponds to the list of classrooms that can be shown on the classroom list screen. +message ClassroomList { + // All classrooms that are available to the player. + repeated EphemeralClassroomSummary classroom_summary = 1; +} + // Corresponds to a local file cataloging all concept cards available to load. message ConceptCardList { // The list of concept cards stored on the local filesystem. From c82994eb584c94d9967b0f904cc343ef0e0a58ea Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 6 Jun 2024 17:17:03 +0530 Subject: [PATCH 24/36] Remove classroom info from CompletedStory --- .../android/domain/topic/TopicController.kt | 2 -- .../domain/topic/TopicControllerTest.kt | 21 ------------------- model/src/main/proto/topic.proto | 6 ------ 3 files changed, 29 deletions(-) diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt index d030eda4e07..ce2e556e575 100755 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt @@ -394,8 +394,6 @@ class TopicController @Inject constructor( .setStoryTitle(storySummary.storyTitle) .setTopicId(topic.topicId) .setTopicTitle(topic.title) - .setClassroomId(topic.classroomId) - .setClassroomTitle(topic.classroomTitle) .setLessonThumbnail(storySummary.storyThumbnail) completedStoryList.add(completedStoryBuilder.build()) } diff --git a/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt index 7ef3a305f43..ae33434a682 100755 --- a/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt @@ -895,27 +895,6 @@ class TopicControllerTest { assertThat(completedStoryList.completedStoryList[1].storyId).isEqualTo(RATIOS_STORY_ID_0) } - @Test - fun testCompletedStoryList_finishTwoStories_completedStoryListHasCorrectClassroomInfo() { - markFractionsStory0Chapter0AsCompleted() - markFractionsStory0Chapter1AsCompleted() - markRatiosStory0Chapter0AsCompleted() - markRatiosStory0Chapter1AsCompleted() - - val storyList = topicController.getCompletedStoryList(profileId1) - - val completedStoryList = monitorFactory.waitForNextSuccessfulResult(storyList) - assertThat(completedStoryList.completedStoryCount).isEqualTo(2) - completedStoryList.completedStoryList[0].also { completedFractionsStory -> - assertThat(completedFractionsStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) - assertThat(completedFractionsStory.classroomTitle.html).isEqualTo("Maths") - } - completedStoryList.completedStoryList[1].also { completedRatiosStory -> - assertThat(completedRatiosStory.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) - assertThat(completedRatiosStory.classroomTitle.html).isEqualTo("Maths") - } - } - /* Localization-based tests. */ @Test diff --git a/model/src/main/proto/topic.proto b/model/src/main/proto/topic.proto index 3498e847576..ca6f44aa523 100755 --- a/model/src/main/proto/topic.proto +++ b/model/src/main/proto/topic.proto @@ -248,12 +248,6 @@ message CompletedStory { // The title of the topic this story is part of. SubtitledHtml topic_title = 9; - // The ID of the classroom this story is part of. - string classroom_id = 10; - - // The title of the classroom this story is part of. - SubtitledHtml classroom_title = 11; - // The thumbnail that should be displayed for this completed story. LessonThumbnail lesson_thumbnail = 5; From 26494613527ac46261fe54a0a7790db6ee1c6a72 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 6 Jun 2024 17:18:59 +0530 Subject: [PATCH 25/36] Fix error message --- .../java/org/oppia/android/domain/topic/TopicListController.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index ceddff1d84c..8502344d95d 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -859,7 +859,7 @@ class TopicListController @Inject constructor( val classroomIdsObj = jsonAssetRetriever.loadJsonFromAsset("classrooms.json") checkNotNull(classroomIdsObj) { "Failed to load classrooms.json." } val classroomIds = classroomIdsObj.optJSONArray("classroom_id_list") - checkNotNull(classroomIds) { "classrooms.json missing classroom IDs." } + checkNotNull(classroomIds) { "classrooms.json is missing classroom IDs." } // Initialize a list to store the classroom records. val classroomRecords = mutableListOf() From 857d296604eea1b823cf4829125220c1ddbc678a Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 6 Jun 2024 17:34:38 +0530 Subject: [PATCH 26/36] Update comments --- .../org/oppia/android/domain/topic/TopicListController.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index 8502344d95d..b371d881acd 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -861,10 +861,10 @@ class TopicListController @Inject constructor( val classroomIds = classroomIdsObj.optJSONArray("classroom_id_list") checkNotNull(classroomIds) { "classrooms.json is missing classroom IDs." } - // Initialize a list to store the classroom records. + // Initialize a list to store the [ClassroomRecord]s. val classroomRecords = mutableListOf() - // Iterate over all classroom IDs and load each classroom's JSON. + // 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." From 693bfe8b1d63c827ece911a25651ac646f16a255 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 6 Jun 2024 17:52:20 +0530 Subject: [PATCH 27/36] Revert "Introduce topic_ids field for classrooms" This reverts commit 884a6750 --- domain/src/main/assets/test_classroom_id_0.json | 3 +-- .../src/main/assets/test_classroom_id_0.textproto | 4 ---- domain/src/main/assets/test_classroom_id_1.json | 3 +-- .../src/main/assets/test_classroom_id_1.textproto | 4 ---- domain/src/main/assets/test_classroom_id_2.json | 3 +-- .../src/main/assets/test_classroom_id_2.textproto | 3 --- .../android/domain/topic/TopicListController.kt | 13 +------------ model/src/main/proto/topic.proto | 3 --- 8 files changed, 4 insertions(+), 32 deletions(-) diff --git a/domain/src/main/assets/test_classroom_id_0.json b/domain/src/main/assets/test_classroom_id_0.json index 0fafae68ccb..97c33f70bf2 100644 --- a/domain/src/main/assets/test_classroom_id_0.json +++ b/domain/src/main/assets/test_classroom_id_0.json @@ -9,6 +9,5 @@ "topic_prerequisites": { "test_topic_id_0": ["GJ2rLXRKD5hw"], "test_topic_id_1": ["test_topic_id_0", "omzF4oqgeTXd"] - }, - "topic_ids": ["test_topic_id_0", "test_topic_id_1"] + } } diff --git a/domain/src/main/assets/test_classroom_id_0.textproto b/domain/src/main/assets/test_classroom_id_0.textproto index 1d5a8439eaa..c98e7ba4ac3 100644 --- a/domain/src/main/assets/test_classroom_id_0.textproto +++ b/domain/src/main/assets/test_classroom_id_0.textproto @@ -18,7 +18,3 @@ topic_prerequisites { topic_ids: "omzF4oqgeTXd" } } -topic_ids: { - topic_ids: "test_topic_id_0" - topic_ids: "test_topic_id_1" -} diff --git a/domain/src/main/assets/test_classroom_id_1.json b/domain/src/main/assets/test_classroom_id_1.json index eb0aa03b32e..53043956ff0 100644 --- a/domain/src/main/assets/test_classroom_id_1.json +++ b/domain/src/main/assets/test_classroom_id_1.json @@ -9,6 +9,5 @@ "topic_prerequisites": { "GJ2rLXRKD5hw": [], "omzF4oqgeTXd": [] - }, - "topic_ids": ["GJ2rLXRKD5hw", "omzF4oqgeTXd"] + } } diff --git a/domain/src/main/assets/test_classroom_id_1.textproto b/domain/src/main/assets/test_classroom_id_1.textproto index 67746510f39..84d10414493 100644 --- a/domain/src/main/assets/test_classroom_id_1.textproto +++ b/domain/src/main/assets/test_classroom_id_1.textproto @@ -15,7 +15,3 @@ topic_prerequisites { value { } } -topic_ids: { - topic_ids: "GJ2rLXRKD5hw" - topic_ids: "omzF4oqgeTXd" -} diff --git a/domain/src/main/assets/test_classroom_id_2.json b/domain/src/main/assets/test_classroom_id_2.json index 5735e9597fc..55e7e9d9fe7 100644 --- a/domain/src/main/assets/test_classroom_id_2.json +++ b/domain/src/main/assets/test_classroom_id_2.json @@ -8,6 +8,5 @@ "thumbnail_filename": "", "topic_prerequisites": { "test_topic_id_2": [] - }, - "topic_ids": ["test_topic_id_2"] + } } diff --git a/domain/src/main/assets/test_classroom_id_2.textproto b/domain/src/main/assets/test_classroom_id_2.textproto index b11f4b903b8..a95ec162147 100644 --- a/domain/src/main/assets/test_classroom_id_2.textproto +++ b/domain/src/main/assets/test_classroom_id_2.textproto @@ -10,6 +10,3 @@ topic_prerequisites { value { } } -topic_ids: { - topic_ids: "test_topic_id_2" -} diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index b371d881acd..49315ff974a 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -888,16 +888,6 @@ class TopicListController @Inject constructor( } } } - - // Load the topic ids of the current classroom. - val topicIdJsonArray = checkNotNull(classroomObj.getJSONArray("topic_ids")) { - "Expected classroom to have non-null topic_ids." - } - val topicIdListBuilder = TopicIdList.newBuilder() - for (i in 0 until topicIdJsonArray.length()) { - topicIdListBuilder.addTopicIds(topicIdJsonArray.optString(i)!!) - } - val classroomRecord = ClassroomRecord.newBuilder().apply { this.id = checkNotNull(classroomObj.optString("id")) { "Expected classroom to have ID." @@ -909,7 +899,6 @@ class TopicListController @Inject constructor( }.build() } ) - this.topicIds = topicIdListBuilder.build() }.build() classroomRecords.add(classroomRecord) @@ -920,7 +909,7 @@ class TopicListController @Inject constructor( // TODO(#5344): Remove this in favor of per-classroom data handling. private fun loadCombinedClassroomsTopicIdList(): List = - loadClassrooms().flatMap { it.topicIds.topicIdsList } + loadClassrooms().flatMap { it.topicPrerequisitesMap.keys.toList() } } internal fun createTopicThumbnailFromJson(topicJsonObject: JSONObject): LessonThumbnail { diff --git a/model/src/main/proto/topic.proto b/model/src/main/proto/topic.proto index ca6f44aa523..9b8fedfda77 100755 --- a/model/src/main/proto/topic.proto +++ b/model/src/main/proto/topic.proto @@ -599,9 +599,6 @@ message ClassroomRecord { // include topics outside of this classroom. map topic_prerequisites = 5; - // A list of topics IDs that belong to this classroom. - TopicIdList topic_ids = 6; - // Represents a list of topic IDs (to be used in the context of topic deps in a classroom). message TopicIdList { // A list of topics IDs. From 73fb79eef1d70d95d573a2ff3df58274569edb85 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 13 Jun 2024 17:00:39 +0530 Subject: [PATCH 28/36] Remove extra new line --- model/src/main/proto/topic.proto | 1 - 1 file changed, 1 deletion(-) diff --git a/model/src/main/proto/topic.proto b/model/src/main/proto/topic.proto index 9b8fedfda77..54eecc97c17 100755 --- a/model/src/main/proto/topic.proto +++ b/model/src/main/proto/topic.proto @@ -349,7 +349,6 @@ message TopicSummary { // The title of the classroom this topic is part of. SubtitledHtml classroom_title = 11; - // The structural version of the topic. int32 version = 3; From 2432150a190e8923d9d00b34cbd168185e898810 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Fri, 14 Jun 2024 11:37:32 +0530 Subject: [PATCH 29/36] Move classroom titles to ephemeral objects and pass the class written translation contexts --- .../android/domain/topic/TopicController.kt | 63 ++++++- .../domain/topic/TopicListController.kt | 163 +++++++++++------- .../domain/topic/TopicControllerTest.kt | 2 - .../domain/topic/TopicListControllerTest.kt | 16 +- model/src/main/proto/topic.proto | 27 ++- 5 files changed, 188 insertions(+), 83 deletions(-) diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt index 59d99a52c3a..1821ca5989b 100755 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt @@ -6,6 +6,7 @@ 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.ClassroomRecord import org.oppia.android.app.model.CompletedStory import org.oppia.android.app.model.CompletedStoryList import org.oppia.android.app.model.EphemeralChapterSummary @@ -474,7 +475,6 @@ class TopicController @Inject constructor( title = topicRecord.translatableTitle description = topicRecord.translatableDescription classroomId = topicRecord.classroomId - classroomTitle = topicRecord.translatableClassroomTitle addAllStory(stories) topicThumbnail = createTopicThumbnailFromProto(topicId, topicRecord.topicThumbnail) diskSizeBytes = computeTopicSizeBytes(getProtoAssetFileNameList(topicId)).toLong() @@ -557,10 +557,6 @@ class TopicController @Inject constructor( html = topicData.getStringFromObject("topic_name") }.build() val classroomId = topicData.getStringFromObject("classroom_id") - val classroomTitle = SubtitledHtml.newBuilder().apply { - contentId = "classroom_title" - html = topicData.getStringFromObject("classroom_name") - } val topicDescription = SubtitledHtml.newBuilder().apply { contentId = "description" html = topicData.getStringFromObject("topic_description") @@ -570,7 +566,6 @@ class TopicController @Inject constructor( .setTopicId(topicId) .setTitle(topicTitle) .setClassroomId(classroomId) - .setClassroomTitle(classroomTitle) .setDescription(topicDescription) .addAllStory(storySummaryList) .setTopicThumbnail(createTopicThumbnailFromJson(topicData)) @@ -871,14 +866,20 @@ class TopicController @Inject constructor( private fun Topic.toEphemeral( contentLocale: OppiaLocale.ContentLocale ): EphemeralTopic { + val classroomRecord = loadClassroomById(classroomId) return EphemeralTopic.newBuilder().apply { topic = this@toEphemeral writtenTranslationContext = translationController.computeWrittenTranslationContext( topic.writtenTranslationsMap, contentLocale ) + classroomWrittenTranslationContext = + translationController.computeWrittenTranslationContext( + classroomRecord.writtenTranslationsMap, contentLocale + ) addAllStories(topic.storyList.map { it.toEphemeral(contentLocale) }) addAllSubtopics(topic.subtopicList.map { it.toEphemeral(contentLocale) }) + classroomTitle = classroomRecord.translatableTitle }.build() } @@ -921,6 +922,56 @@ class TopicController @Inject constructor( }.build() } + // TODO(#5344): Move this to classroom controller. + fun loadClassroomById(classroomId: String): ClassroomRecord { + return if (loadLessonProtosFromAssets) { + assetRepository.loadProtoFromLocalAssets( + assetName = classroomId, + baseMessage = 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("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() + } + private companion object { private fun JSONObject.getRemovableOptionalString(name: String) = optString(name).takeIf { it.isNotEmpty() && it != "" && it != "" } diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index 49315ff974a..8c9d128ac59 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -189,12 +189,18 @@ class TopicListController @Inject constructor( contentLocale: OppiaLocale.ContentLocale ): EphemeralTopicSummary { val topicSummary = createTopicSummary(topicId) + val classroomRecord = loadClassroomById(topicSummary.classroomId) return EphemeralTopicSummary.newBuilder().apply { this.topicSummary = topicSummary writtenTranslationContext = translationController.computeWrittenTranslationContext( topicSummary.writtenTranslationsMap, contentLocale ) + classroomWrittenTranslationContext = + translationController.computeWrittenTranslationContext( + classroomRecord.writtenTranslationsMap, contentLocale + ) + classroomTitle = classroomRecord.translatableTitle }.build() } @@ -216,7 +222,6 @@ class TopicListController @Inject constructor( putAllWrittenTranslations(topicRecord.writtenTranslationsMap) title = topicRecord.translatableTitle classroomId = topicRecord.classroomId - classroomTitle = topicRecord.translatableClassroomTitle totalChapterCount = storyRecords.map { it.chaptersList.size }.sum() topicThumbnail = topicRecord.topicThumbnail topicPlayAvailability = if (topicRecord.isPublished) { @@ -259,16 +264,11 @@ class TopicListController @Inject constructor( html = jsonObject.getStringFromObject("topic_name") }.build() val classroomId = jsonObject.getStringFromObject("classroom_id") - val classroomTitle = SubtitledHtml.newBuilder().apply { - contentId = "classroom_title" - html = jsonObject.getStringFromObject("classroom_name") - }.build() // No written translations are included since none are retrieved from JSON. return TopicSummary.newBuilder() .setTopicId(topicId) .setTitle(topicTitle) .setClassroomId(classroomId) - .setClassroomTitle(classroomTitle) .setVersion(jsonObject.optInt("version")) .setTotalChapterCount(totalChapterCount) .setTopicThumbnail(createTopicThumbnailFromJson(jsonObject)) @@ -301,10 +301,14 @@ class TopicListController @Inject constructor( }.build() val classroomId = jsonObject.getStringFromObject("classroom_id") - val classroomTitle = SubtitledHtml.newBuilder().apply { - contentId = "classroom_title" - html = jsonObject.getStringFromObject("classroom_name") - }.build() + + val classroomJsonObject = jsonAssetRetriever.loadJsonFromAsset("$classroomId.json")!! + val classroomTitle = classroomJsonObject.getJSONObject("classroom_title").let { + SubtitledHtml.newBuilder().apply { + contentId = it.getString("content_id") + html = it.getString("html") + }.build() + } // No written translations are included since none are retrieved from JSON. return UpcomingTopic.newBuilder().setTopicId(topicId) @@ -368,6 +372,8 @@ class TopicListController @Inject constructor( sortedTopicProgressList.forEach { topicProgress -> val topic = topicController.retrieveTopic(topicProgress.topicId) + val classroom = topic?.classroomId?.let { loadClassroomById(it) } + ?: ClassroomRecord.getDefaultInstance() // Ignore topics that are no longer on the device, or that have been unpublished. if (topic?.topicPlayAvailability?.availabilityCase == AVAILABLE_TO_PLAY_NOW) { val isTopicConsideredCompleted = topic.hasAtLeastOneStoryCompleted(topicProgress) @@ -396,7 +402,8 @@ class TopicListController @Inject constructor( topic, isTopicConsideredCompleted, storyProgress.chapterProgressMap, - contentLocale + contentLocale, + classroom )?.let { promotedStory -> playedPromotedStoryList.add(promotedStory) } @@ -416,7 +423,8 @@ class TopicListController @Inject constructor( topic, isTopicConsideredCompleted, storyProgress.chapterProgressMap, - contentLocale + contentLocale, + classroom )?.let { promotedStory -> playedPromotedStoryList.add(promotedStory) } @@ -483,7 +491,8 @@ class TopicListController @Inject constructor( topic: Topic, isTopicConsideredCompleted: Boolean, chapterProgressMap: Map, - contentLocale: OppiaLocale.ContentLocale + contentLocale: OppiaLocale.ContentLocale, + classroom: ClassroomRecord, ): PromotedStory? { val recentlyPlayerChapterSummary: ChapterSummary? = story.chapterList.find { chapterSummary -> @@ -498,7 +507,8 @@ class TopicListController @Inject constructor( recentlyPlayerChapterSummary, isTopicConsideredCompleted, chapterProgressMap[recentlyPlayerChapterSummary.explorationId], - contentLocale + contentLocale, + classroom ) } return null @@ -512,7 +522,8 @@ class TopicListController @Inject constructor( topic: Topic, isTopicConsideredCompleted: Boolean, chapterProgressMap: Map, - contentLocale: OppiaLocale.ContentLocale + contentLocale: OppiaLocale.ContentLocale, + classroom: ClassroomRecord, ): PromotedStory? { val lastChapterSummary: ChapterSummary? = story.chapterList.find { chapterSummary -> @@ -530,7 +541,8 @@ class TopicListController @Inject constructor( nextChapterSummary, isTopicConsideredCompleted, chapterProgressMap[nextChapterSummary.explorationId], - contentLocale + contentLocale, + classroom ) } } @@ -701,6 +713,11 @@ class TopicListController @Inject constructor( assetName = firstStoryId, baseMessage = StoryRecord.getDefaultInstance() ) + val classroomRecord = + assetRepository.loadProtoFromLocalAssets( + assetName = topicRecord.classroomId, + baseMessage = ClassroomRecord.getDefaultInstance() + ) return PromotedStory.newBuilder().apply { storyId = firstStoryId storyWrittenTranslationContext = @@ -711,11 +728,15 @@ class TopicListController @Inject constructor( translationController.computeWrittenTranslationContext( topicRecord.writtenTranslationsMap, contentLocale ) + classroomWrittenTranslationContext = + translationController.computeWrittenTranslationContext( + classroomRecord.writtenTranslationsMap, contentLocale + ) storyTitle = storyRecord.translatableStoryName this.topicId = topicId topicTitle = topicRecord.translatableTitle classroomId = topicRecord.classroomId - classroomTitle = topicRecord.translatableClassroomTitle + classroomTitle = classroomRecord.translatableTitle completedChapterCount = 0 totalChapterCount = storyRecord.chaptersCount lessonThumbnail = storyRecord.storyThumbnail @@ -765,12 +786,16 @@ class TopicListController @Inject constructor( } ?: SubtitledHtml.getDefaultInstance() val classroomId = topicJson.optString("classroom_id") - val classroomTitle = topicJson.optString("classroom_name").takeIf { it.isNotEmpty() }?.let { + + val classroomJson = jsonAssetRetriever.loadJsonFromAsset("$classroomId.json") + if (classroomJson!!.optString("classroom_title").isNullOrEmpty()) return null + + val classroomTitle = classroomJson.getJSONObject("classroom_title").let { SubtitledHtml.newBuilder().apply { - contentId = "classroom_title" - html = it + contentId = it.getString("content_id") + html = it.getString("html") }.build() - } ?: SubtitledHtml.getDefaultInstance() + } // No written translations are included for the topic since its name is directly fetched from // the JSON (and the JSON doesn't include translations for these properties, anyway). @@ -801,7 +826,8 @@ class TopicListController @Inject constructor( nextChapterSummary: ChapterSummary, isTopicConsideredCompleted: Boolean, nextChapterProgress: ChapterProgress?, - contentLocale: OppiaLocale.ContentLocale + contentLocale: OppiaLocale.ContentLocale, + classroom: ClassroomRecord, ): PromotedStory { val storySummary = topic.storyList.find { summary -> summary.storyId == storyId }!! // If the chapterProgress equals null that means the chapter has no progress associated with @@ -823,12 +849,17 @@ class TopicListController @Inject constructor( nextChapterSummary.writtenTranslationsMap, contentLocale ) ) + .setClassroomWrittenTranslationContext( + translationController.computeWrittenTranslationContext( + classroom.writtenTranslationsMap, contentLocale + ) + ) .setStoryTitle(storySummary.storyTitle) .setLessonThumbnail(storySummary.storyThumbnail) .setTopicId(topic.topicId) .setTopicTitle(topic.title) .setClassroomId(topic.classroomId) - .setClassroomTitle(topic.classroomTitle) + .setClassroomTitle(classroom.translatableTitle) .setCompletedChapterCount(completedChapterCount) .setTotalChapterCount(totalChapterCount) .setIsTopicLearned(isTopicConsideredCompleted) @@ -844,11 +875,8 @@ class TopicListController @Inject constructor( assetRepository.loadProtoFromLocalAssets( assetName = "classrooms", baseMessage = ClassroomIdList.getDefaultInstance() - ).classroomIdsList.map { - assetRepository.loadProtoFromLocalAssets( - assetName = it, - baseMessage = ClassroomRecord.getDefaultInstance() - ) + ).classroomIdsList.map { classroomId -> + loadClassroomById(classroomId) } } else loadClassroomsFromJson() } @@ -869,42 +897,61 @@ class TopicListController @Inject constructor( val classroomId = checkNotNull(classroomIds.optString(i)) { "Expected non-null classroom ID at index $i." } + val classroomRecord = loadClassroomById(classroomId) + classroomRecords.add(classroomRecord) + } - // Load the classroom obj of the current classroom. - val classroomObj = jsonAssetRetriever.loadJsonFromAsset("$classroomId.json") - checkNotNull(classroomObj) { "Failed to load $classroomId.json." } + return classroomRecords + } - // Load the topic prerequisite map of the current classroom. - val topicPrereqsObj = checkNotNull(classroomObj.optJSONObject("topic_prerequisites")) { - "Expected classroom to have non-null topic_prerequisites." + // TODO(#5344): Move this to classroom controller. + private fun loadClassroomById(classroomId: String): ClassroomRecord { + return if (loadLessonProtosFromAssets) { + assetRepository.loadProtoFromLocalAssets( + assetName = classroomId, + baseMessage = 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." } - 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@associateWith List(topicIdArray.length()) { index -> + checkNotNull(topicIdArray.optString(index)) { + "Expected topic $topicId to have non-null string at index $index." } } - val classroomRecord = ClassroomRecord.newBuilder().apply { - this.id = checkNotNull(classroomObj.optString("id")) { - "Expected classroom to have ID." - } - this.putAllTopicPrerequisites( - topicPrereqs.mapValues { (_, topicIds) -> - TopicIdList.newBuilder().apply { - addAllTopicIds(topicIds) - }.build() - } - ) - }.build() - - classroomRecords.add(classroomRecord) } - - return classroomRecords + return ClassroomRecord.newBuilder().apply { + id = checkNotNull(classroomObj.optString("id")) { + "Expected classroom to have ID." + } + translatableTitle = SubtitledHtml.newBuilder().apply { + contentId = classroomTitle.getStringFromObject("content_id") + html = classroomTitle.getStringFromObject("html") + }.build() + putAllTopicPrerequisites( + topicPrereqs.mapValues { (_, topicIds) -> + TopicIdList.newBuilder().apply { + addAllTopicIds(topicIds) + }.build() + } + ) + }.build() } // TODO(#5344): Remove this in favor of per-classroom data handling. diff --git a/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt index ae33434a682..439a331d4ea 100755 --- a/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt @@ -136,7 +136,6 @@ class TopicControllerTest { val topic = monitorFactory.waitForNextSuccessfulResult(topicProvider).topic assertThat(topic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) - assertThat(topic.classroomTitle.html).contains("Maths") } @Test @@ -165,7 +164,6 @@ class TopicControllerTest { val topic = monitorFactory.waitForNextSuccessfulResult(topicProvider).topic assertThat(topic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) - assertThat(topic.classroomTitle.html).contains("Maths") } @Test diff --git a/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt index 74ac9733da9..29006f7e528 100644 --- a/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt @@ -112,8 +112,8 @@ class TopicListControllerTest { fun testRetrieveTopicList_firstTopic_hasCorrectClassroomInfo() { val topicList = retrieveTopicList() - val firstTopic = topicList.getTopicSummary(0).topicSummary - assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + val firstTopic = topicList.getTopicSummary(0) + assertThat(firstTopic.topicSummary.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) assertThat(firstTopic.classroomTitle.html).isEqualTo("Science") } @@ -138,8 +138,8 @@ class TopicListControllerTest { fun testRetrieveTopicList_secondTopic_hasCorrectClassroomInfo() { val topicList = retrieveTopicList() - val firstTopic = topicList.getTopicSummary(1).topicSummary - assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) + val firstTopic = topicList.getTopicSummary(1) + assertThat(firstTopic.topicSummary.classroomId).isEqualTo(TEST_CLASSROOM_ID_0) assertThat(firstTopic.classroomTitle.html).isEqualTo("Science") } @@ -164,8 +164,8 @@ class TopicListControllerTest { fun testRetrieveTopicList_fractionsTopic_hasCorrectClassroomInfo() { val topicList = retrieveTopicList() - val firstTopic = topicList.getTopicSummary(2).topicSummary - assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + val firstTopic = topicList.getTopicSummary(2) + assertThat(firstTopic.topicSummary.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(firstTopic.classroomTitle.html).isEqualTo("Maths") } @@ -190,8 +190,8 @@ class TopicListControllerTest { fun testRetrieveTopicList_ratiosTopic_hasCorrectClassroomInfo() { val topicList = retrieveTopicList() - val firstTopic = topicList.getTopicSummary(3).topicSummary - assertThat(firstTopic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) + val firstTopic = topicList.getTopicSummary(3) + assertThat(firstTopic.topicSummary.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) assertThat(firstTopic.classroomTitle.html).isEqualTo("Maths") } diff --git a/model/src/main/proto/topic.proto b/model/src/main/proto/topic.proto index 54eecc97c17..a1ccaabed34 100755 --- a/model/src/main/proto/topic.proto +++ b/model/src/main/proto/topic.proto @@ -28,9 +28,6 @@ message Topic { // The ID of the classroom this topic is part of. string classroom_id = 12; - // The title of the classroom this topic is part of. - SubtitledHtml classroom_title = 13; - // A list of summarized stories contained within this topic. repeated StorySummary story = 4; @@ -61,11 +58,17 @@ message EphemeralTopic { // The translation context that should be used for this topic. WrittenTranslationContext written_translation_context = 2; + // The written translation context for the topic's classroom strings. + WrittenTranslationContext classroom_written_translation_context = 5; + // The EphemeralStorySummarys corresponding to the stories contained within this topic. repeated EphemeralStorySummary stories = 3; // The EphemeralSubtopics corresponding to the subtopics contained within this topic. repeated EphemeralSubtopic subtopics = 4; + + // The title of the classroom this topic is part of. + SubtitledHtml classroom_title = 6; } // Corresponds to details around whether a particular topic is playable. @@ -283,6 +286,9 @@ message PromotedStory { // The written translation context strings corresponding to the next chapter of this story. WrittenTranslationContext next_chapter_written_translation_context = 14; + // The written translation context for the story's classroom strings. + WrittenTranslationContext classroom_written_translation_context = 20; + // The title of the story being promoted. SubtitledHtml story_title = 15; @@ -346,9 +352,6 @@ message TopicSummary { // The ID of the classroom this topic is part of. string classroom_id = 10; - // The title of the classroom this topic is part of. - SubtitledHtml classroom_title = 11; - // The structural version of the topic. int32 version = 3; @@ -376,6 +379,12 @@ message EphemeralTopicSummary { // The translation context that should be used for this topic summary. WrittenTranslationContext written_translation_context = 2; + // The written translation context for the topic's classroom strings. + WrittenTranslationContext classroom_written_translation_context = 3; + + // The title of the classroom this topic is part of. + SubtitledHtml classroom_title = 4; + // The ID of the first story of this topic. string first_story_id = 7; } @@ -459,6 +468,9 @@ message UpcomingTopic { // The written translation context for this topic view. WrittenTranslationContext written_translation_context = 6; + // The written translation context for the topic's classroom strings. + WrittenTranslationContext classroom_written_translation_context = 10; + // The title of the topic. SubtitledHtml title = 7; @@ -664,9 +676,6 @@ message TopicRecord { // The ID of the classroom this topic is part of. string classroom_id = 11; - // The title of the classroom this topic is part of. - SubtitledHtml translatable_classroom_title = 12; - // The list of canonical story IDs that can be used to load stories from the local filesystem. repeated string canonical_story_ids = 4; From d2a454df37ee412c74c162359dd94b15685303c8 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Fri, 14 Jun 2024 14:47:12 +0530 Subject: [PATCH 30/36] Remove classroom title from topic data files --- domain/src/main/assets/GJ2rLXRKD5hw.json | 1 - domain/src/main/assets/GJ2rLXRKD5hw.textproto | 4 ---- domain/src/main/assets/omzF4oqgeTXd.json | 1 - domain/src/main/assets/omzF4oqgeTXd.textproto | 4 ---- domain/src/main/assets/test_topic_id_0.json | 1 - domain/src/main/assets/test_topic_id_0.textproto | 4 ---- domain/src/main/assets/test_topic_id_1.json | 1 - domain/src/main/assets/test_topic_id_1.textproto | 4 ---- domain/src/main/assets/test_topic_id_2.json | 1 - domain/src/main/assets/test_topic_id_2.textproto | 4 ---- 10 files changed, 25 deletions(-) diff --git a/domain/src/main/assets/GJ2rLXRKD5hw.json b/domain/src/main/assets/GJ2rLXRKD5hw.json index 39987949e3a..06a4a1db257 100644 --- a/domain/src/main/assets/GJ2rLXRKD5hw.json +++ b/domain/src/main/assets/GJ2rLXRKD5hw.json @@ -14,7 +14,6 @@ "topic_description": "You'll often need to talk about part of an object or group. For example, a jar of milk might be half-full, or some of the eggs in a box might have broken. In these lessons, you'll learn to use fractions to describe situations like these.", "topic_id": "GJ2rLXRKD5hw", "classroom_id": "test_classroom_id_1", - "classroom_name": "Maths", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", "skill_descriptions": {}, diff --git a/domain/src/main/assets/GJ2rLXRKD5hw.textproto b/domain/src/main/assets/GJ2rLXRKD5hw.textproto index 6f5bc4a307a..6690f0d15dc 100644 --- a/domain/src/main/assets/GJ2rLXRKD5hw.textproto +++ b/domain/src/main/assets/GJ2rLXRKD5hw.textproto @@ -26,7 +26,3 @@ translatable_description { content_id: "description" } classroom_id: "test_classroom_id_1" -translatable_classroom_title { - html: "Maths" - content_id: "classroom_title" -} diff --git a/domain/src/main/assets/omzF4oqgeTXd.json b/domain/src/main/assets/omzF4oqgeTXd.json index c2755332bdc..111d0465a91 100644 --- a/domain/src/main/assets/omzF4oqgeTXd.json +++ b/domain/src/main/assets/omzF4oqgeTXd.json @@ -22,7 +22,6 @@ "topic_description": "Many everyday problems involve thinking about proportions. For example, if 30 apples cost 6 dollars, how much would 12 apples cost? In these lessons, you'll learn about how to use ratios and proportional reasoning to solve problems like this.", "topic_id": "omzF4oqgeTXd", "classroom_id": "test_classroom_id_1", - "classroom_name": "Maths", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", "skill_descriptions": {}, diff --git a/domain/src/main/assets/omzF4oqgeTXd.textproto b/domain/src/main/assets/omzF4oqgeTXd.textproto index d50da8a980a..2f21eb840e4 100644 --- a/domain/src/main/assets/omzF4oqgeTXd.textproto +++ b/domain/src/main/assets/omzF4oqgeTXd.textproto @@ -24,7 +24,3 @@ translatable_description { content_id: "description" } classroom_id: "test_classroom_id_1" -translatable_classroom_title { - html: "Maths" - content_id: "classroom_title" -} diff --git a/domain/src/main/assets/test_topic_id_0.json b/domain/src/main/assets/test_topic_id_0.json index d0893eeb7dc..63ed5945d1b 100644 --- a/domain/src/main/assets/test_topic_id_0.json +++ b/domain/src/main/assets/test_topic_id_0.json @@ -14,7 +14,6 @@ "topic_description": "A topic investigating the interesting aspects of the Oppia Android app.", "topic_id": "test_topic_id_0", "classroom_id": "test_classroom_id_0", - "classroom_name": "Science", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", "skill_descriptions": {}, diff --git a/domain/src/main/assets/test_topic_id_0.textproto b/domain/src/main/assets/test_topic_id_0.textproto index 2109c268e4f..91a6285f651 100644 --- a/domain/src/main/assets/test_topic_id_0.textproto +++ b/domain/src/main/assets/test_topic_id_0.textproto @@ -23,7 +23,3 @@ translatable_description { content_id: "description" } classroom_id: "test_classroom_id_0" -translatable_classroom_title { - html: "Science" - content_id: "classroom_title" -} diff --git a/domain/src/main/assets/test_topic_id_1.json b/domain/src/main/assets/test_topic_id_1.json index 05ddb34ac12..df38a675e50 100644 --- a/domain/src/main/assets/test_topic_id_1.json +++ b/domain/src/main/assets/test_topic_id_1.json @@ -14,7 +14,6 @@ "topic_description": "A topic considering the various implications of having especially long topic descriptions. These descriptions almost certainly need to wrap, which should be interesting in the UI (especially on small screens). Consider also that there may even be multiple points pertaining to a topic, some of which may require expanding the description section in order to read the whole topic description.", "topic_id": "test_topic_id_1", "classroom_id": "test_classroom_id_0", - "classroom_name": "Science", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", "skill_descriptions": {}, diff --git a/domain/src/main/assets/test_topic_id_1.textproto b/domain/src/main/assets/test_topic_id_1.textproto index 5de5bf665f3..960afc7a355 100644 --- a/domain/src/main/assets/test_topic_id_1.textproto +++ b/domain/src/main/assets/test_topic_id_1.textproto @@ -22,7 +22,3 @@ translatable_description { content_id: "description" } classroom_id: "test_classroom_id_0" -translatable_classroom_title { - html: "Science" - content_id: "classroom_title" -} diff --git a/domain/src/main/assets/test_topic_id_2.json b/domain/src/main/assets/test_topic_id_2.json index 2a5a20640ef..1ffc6387d65 100644 --- a/domain/src/main/assets/test_topic_id_2.json +++ b/domain/src/main/assets/test_topic_id_2.json @@ -6,7 +6,6 @@ "topic_name": "Third Test Topic", "topic_id": "test_topic_id_2", "classroom_id": "test_classroom_id_2", - "classroom_name": "English", "version": -1, "published": false, "skill_descriptions": {}, diff --git a/domain/src/main/assets/test_topic_id_2.textproto b/domain/src/main/assets/test_topic_id_2.textproto index 63a1d005792..c703d1f6821 100644 --- a/domain/src/main/assets/test_topic_id_2.textproto +++ b/domain/src/main/assets/test_topic_id_2.textproto @@ -8,7 +8,3 @@ translatable_description { content_id: "description" } classroom_id: "test_classroom_id_2" -translatable_classroom_title { - html: "English" - content_id: "classroom_title" -} From ac24de1367bf6e2c14981b23920e838b97c7f4e3 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Fri, 14 Jun 2024 14:55:40 +0530 Subject: [PATCH 31/36] Fix regex validation checks --- .../org/oppia/android/domain/topic/TopicListController.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index 8c9d128ac59..e19dec7cc7b 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -305,8 +305,8 @@ class TopicListController @Inject constructor( val classroomJsonObject = jsonAssetRetriever.loadJsonFromAsset("$classroomId.json")!! val classroomTitle = classroomJsonObject.getJSONObject("classroom_title").let { SubtitledHtml.newBuilder().apply { - contentId = it.getString("content_id") - html = it.getString("html") + contentId = it.getStringFromObject("content_id") + html = it.getStringFromObject("html") }.build() } @@ -792,8 +792,8 @@ class TopicListController @Inject constructor( val classroomTitle = classroomJson.getJSONObject("classroom_title").let { SubtitledHtml.newBuilder().apply { - contentId = it.getString("content_id") - html = it.getString("html") + contentId = it.getStringFromObject("content_id") + html = it.getStringFromObject("html") }.build() } From 6cf8ae4aaf56ed2845ff1e942773db750adfd510 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Sat, 15 Jun 2024 16:59:04 +0530 Subject: [PATCH 32/36] Handle empty classroom id when the proto asset is overridden for tests --- .../org/oppia/android/domain/topic/TopicListController.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index e19dec7cc7b..c61cd97f58e 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -907,10 +907,10 @@ class TopicListController @Inject constructor( // TODO(#5344): Move this to classroom controller. private fun loadClassroomById(classroomId: String): ClassroomRecord { return if (loadLessonProtosFromAssets) { - assetRepository.loadProtoFromLocalAssets( + assetRepository.tryLoadProtoFromLocalAssets( assetName = classroomId, - baseMessage = ClassroomRecord.getDefaultInstance() - ) + defaultMessage = ClassroomRecord.getDefaultInstance() + ) ?: ClassroomRecord.getDefaultInstance() } else loadClassroomByIdFromJson(classroomId) } From 3d91ac200be07f3324e3421a88ccf9fd96bb22e5 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 20 Jun 2024 10:52:30 +0530 Subject: [PATCH 33/36] Remove classroom ID from topic data files --- domain/src/main/assets/GJ2rLXRKD5hw.json | 1 - domain/src/main/assets/GJ2rLXRKD5hw.textproto | 1 - domain/src/main/assets/omzF4oqgeTXd.json | 1 - domain/src/main/assets/omzF4oqgeTXd.textproto | 1 - domain/src/main/assets/test_topic_id_0.json | 1 - .../src/main/assets/test_topic_id_0.textproto | 1 - domain/src/main/assets/test_topic_id_1.json | 1 - .../src/main/assets/test_topic_id_1.textproto | 1 - domain/src/main/assets/test_topic_id_2.json | 1 - .../src/main/assets/test_topic_id_2.textproto | 1 - .../android/domain/topic/TopicController.kt | 58 ------------------- .../domain/topic/TopicListController.kt | 25 +++++--- .../domain/topic/TopicControllerTest.kt | 16 ----- model/src/main/proto/topic.proto | 12 ---- 14 files changed, 18 insertions(+), 103 deletions(-) diff --git a/domain/src/main/assets/GJ2rLXRKD5hw.json b/domain/src/main/assets/GJ2rLXRKD5hw.json index 06a4a1db257..39e0269741c 100644 --- a/domain/src/main/assets/GJ2rLXRKD5hw.json +++ b/domain/src/main/assets/GJ2rLXRKD5hw.json @@ -13,7 +13,6 @@ "topic_name": "Fractions", "topic_description": "You'll often need to talk about part of an object or group. For example, a jar of milk might be half-full, or some of the eggs in a box might have broken. In these lessons, you'll learn to use fractions to describe situations like these.", "topic_id": "GJ2rLXRKD5hw", - "classroom_id": "test_classroom_id_1", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", "skill_descriptions": {}, diff --git a/domain/src/main/assets/GJ2rLXRKD5hw.textproto b/domain/src/main/assets/GJ2rLXRKD5hw.textproto index 6690f0d15dc..8891207c36c 100644 --- a/domain/src/main/assets/GJ2rLXRKD5hw.textproto +++ b/domain/src/main/assets/GJ2rLXRKD5hw.textproto @@ -25,4 +25,3 @@ translatable_description { html: "You\'ll often need to talk about part of an object or group. For example, a jar of milk might be half-full, or some of the eggs in a box might have broken. In these lessons, you\'ll learn to use fractions to describe situations like these." content_id: "description" } -classroom_id: "test_classroom_id_1" diff --git a/domain/src/main/assets/omzF4oqgeTXd.json b/domain/src/main/assets/omzF4oqgeTXd.json index 111d0465a91..897eafd56eb 100644 --- a/domain/src/main/assets/omzF4oqgeTXd.json +++ b/domain/src/main/assets/omzF4oqgeTXd.json @@ -21,7 +21,6 @@ "topic_name": "Ratios and Proportional Reasoning", "topic_description": "Many everyday problems involve thinking about proportions. For example, if 30 apples cost 6 dollars, how much would 12 apples cost? In these lessons, you'll learn about how to use ratios and proportional reasoning to solve problems like this.", "topic_id": "omzF4oqgeTXd", - "classroom_id": "test_classroom_id_1", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", "skill_descriptions": {}, diff --git a/domain/src/main/assets/omzF4oqgeTXd.textproto b/domain/src/main/assets/omzF4oqgeTXd.textproto index 2f21eb840e4..5701d42636d 100644 --- a/domain/src/main/assets/omzF4oqgeTXd.textproto +++ b/domain/src/main/assets/omzF4oqgeTXd.textproto @@ -23,4 +23,3 @@ translatable_description { html: "Many everyday problems involve thinking about proportions. For example, if 30 apples cost 6 dollars, how much would 12 apples cost? In these lessons, you\'ll learn about how to use ratios and proportional reasoning to solve problems like this." content_id: "description" } -classroom_id: "test_classroom_id_1" diff --git a/domain/src/main/assets/test_topic_id_0.json b/domain/src/main/assets/test_topic_id_0.json index 63ed5945d1b..a9115679060 100644 --- a/domain/src/main/assets/test_topic_id_0.json +++ b/domain/src/main/assets/test_topic_id_0.json @@ -13,7 +13,6 @@ "topic_name": "First Test Topic", "topic_description": "A topic investigating the interesting aspects of the Oppia Android app.", "topic_id": "test_topic_id_0", - "classroom_id": "test_classroom_id_0", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", "skill_descriptions": {}, diff --git a/domain/src/main/assets/test_topic_id_0.textproto b/domain/src/main/assets/test_topic_id_0.textproto index 91a6285f651..ce47f844943 100644 --- a/domain/src/main/assets/test_topic_id_0.textproto +++ b/domain/src/main/assets/test_topic_id_0.textproto @@ -22,4 +22,3 @@ translatable_description { html: "A topic investigating the interesting aspects of the Oppia Android app." content_id: "description" } -classroom_id: "test_classroom_id_0" diff --git a/domain/src/main/assets/test_topic_id_1.json b/domain/src/main/assets/test_topic_id_1.json index df38a675e50..6b3cc156914 100644 --- a/domain/src/main/assets/test_topic_id_1.json +++ b/domain/src/main/assets/test_topic_id_1.json @@ -13,7 +13,6 @@ "topic_name": "Second Test Topic", "topic_description": "A topic considering the various implications of having especially long topic descriptions. These descriptions almost certainly need to wrap, which should be interesting in the UI (especially on small screens). Consider also that there may even be multiple points pertaining to a topic, some of which may require expanding the description section in order to read the whole topic description.", "topic_id": "test_topic_id_1", - "classroom_id": "test_classroom_id_0", "thumbnail_bg_color": "#000000", "thumbnail_filename": "", "skill_descriptions": {}, diff --git a/domain/src/main/assets/test_topic_id_1.textproto b/domain/src/main/assets/test_topic_id_1.textproto index 960afc7a355..fa4a012e25c 100644 --- a/domain/src/main/assets/test_topic_id_1.textproto +++ b/domain/src/main/assets/test_topic_id_1.textproto @@ -21,4 +21,3 @@ translatable_description { html: "A topic considering the various implications of having especially long topic descriptions. These descriptions almost certainly need to wrap, which should be interesting in the UI (especially on small screens). Consider also that there may even be multiple points pertaining to a topic, some of which may require expanding the description section in order to read the whole topic description." content_id: "description" } -classroom_id: "test_classroom_id_0" diff --git a/domain/src/main/assets/test_topic_id_2.json b/domain/src/main/assets/test_topic_id_2.json index 1ffc6387d65..469617b8c56 100644 --- a/domain/src/main/assets/test_topic_id_2.json +++ b/domain/src/main/assets/test_topic_id_2.json @@ -5,7 +5,6 @@ "topic_description": "A topic that's not yet ready to be played.", "topic_name": "Third Test Topic", "topic_id": "test_topic_id_2", - "classroom_id": "test_classroom_id_2", "version": -1, "published": false, "skill_descriptions": {}, diff --git a/domain/src/main/assets/test_topic_id_2.textproto b/domain/src/main/assets/test_topic_id_2.textproto index c703d1f6821..5d9e7360478 100644 --- a/domain/src/main/assets/test_topic_id_2.textproto +++ b/domain/src/main/assets/test_topic_id_2.textproto @@ -7,4 +7,3 @@ translatable_description { html: "A topic that\'s not yet ready to be played." content_id: "description" } -classroom_id: "test_classroom_id_2" diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt index 1821ca5989b..72e756049e0 100755 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt @@ -474,7 +474,6 @@ class TopicController @Inject constructor( putAllWrittenTranslations(topicRecord.writtenTranslationsMap) title = topicRecord.translatableTitle description = topicRecord.translatableDescription - classroomId = topicRecord.classroomId addAllStory(stories) topicThumbnail = createTopicThumbnailFromProto(topicId, topicRecord.topicThumbnail) diskSizeBytes = computeTopicSizeBytes(getProtoAssetFileNameList(topicId)).toLong() @@ -565,7 +564,6 @@ class TopicController @Inject constructor( return Topic.newBuilder() .setTopicId(topicId) .setTitle(topicTitle) - .setClassroomId(classroomId) .setDescription(topicDescription) .addAllStory(storySummaryList) .setTopicThumbnail(createTopicThumbnailFromJson(topicData)) @@ -866,20 +864,14 @@ class TopicController @Inject constructor( private fun Topic.toEphemeral( contentLocale: OppiaLocale.ContentLocale ): EphemeralTopic { - val classroomRecord = loadClassroomById(classroomId) return EphemeralTopic.newBuilder().apply { topic = this@toEphemeral writtenTranslationContext = translationController.computeWrittenTranslationContext( topic.writtenTranslationsMap, contentLocale ) - classroomWrittenTranslationContext = - translationController.computeWrittenTranslationContext( - classroomRecord.writtenTranslationsMap, contentLocale - ) addAllStories(topic.storyList.map { it.toEphemeral(contentLocale) }) addAllSubtopics(topic.subtopicList.map { it.toEphemeral(contentLocale) }) - classroomTitle = classroomRecord.translatableTitle }.build() } @@ -922,56 +914,6 @@ class TopicController @Inject constructor( }.build() } - // TODO(#5344): Move this to classroom controller. - fun loadClassroomById(classroomId: String): ClassroomRecord { - return if (loadLessonProtosFromAssets) { - assetRepository.loadProtoFromLocalAssets( - assetName = classroomId, - baseMessage = 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("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() - } - private companion object { private fun JSONObject.getRemovableOptionalString(name: String) = optString(name).takeIf { it.isNotEmpty() && it != "" && it != "" } diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index c61cd97f58e..b8e2333d58f 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -221,7 +221,6 @@ class TopicListController @Inject constructor( this.topicId = topicId putAllWrittenTranslations(topicRecord.writtenTranslationsMap) title = topicRecord.translatableTitle - classroomId = topicRecord.classroomId totalChapterCount = storyRecords.map { it.chaptersList.size }.sum() topicThumbnail = topicRecord.topicThumbnail topicPlayAvailability = if (topicRecord.isPublished) { @@ -372,8 +371,10 @@ class TopicListController @Inject constructor( sortedTopicProgressList.forEach { topicProgress -> val topic = topicController.retrieveTopic(topicProgress.topicId) - val classroom = topic?.classroomId?.let { loadClassroomById(it) } - ?: ClassroomRecord.getDefaultInstance() + val classroom = topic?.topicId?.let { topicId -> + val classroomId = getClassroomIdByTopicId(topicId) + loadClassroomById(classroomId) + } ?: ClassroomRecord.getDefaultInstance() // Ignore topics that are no longer on the device, or that have been unpublished. if (topic?.topicPlayAvailability?.availabilityCase == AVAILABLE_TO_PLAY_NOW) { val isTopicConsideredCompleted = topic.hasAtLeastOneStoryCompleted(topicProgress) @@ -715,7 +716,7 @@ class TopicListController @Inject constructor( ) val classroomRecord = assetRepository.loadProtoFromLocalAssets( - assetName = topicRecord.classroomId, + assetName = getClassroomIdByTopicId(topicId), baseMessage = ClassroomRecord.getDefaultInstance() ) return PromotedStory.newBuilder().apply { @@ -735,7 +736,7 @@ class TopicListController @Inject constructor( storyTitle = storyRecord.translatableStoryName this.topicId = topicId topicTitle = topicRecord.translatableTitle - classroomId = topicRecord.classroomId + classroomId = classroomRecord.id classroomTitle = classroomRecord.translatableTitle completedChapterCount = 0 totalChapterCount = storyRecord.chaptersCount @@ -858,7 +859,7 @@ class TopicListController @Inject constructor( .setLessonThumbnail(storySummary.storyThumbnail) .setTopicId(topic.topicId) .setTopicTitle(topic.title) - .setClassroomId(topic.classroomId) + .setClassroomId(classroom.id) .setClassroomTitle(classroom.translatableTitle) .setCompletedChapterCount(completedChapterCount) .setTotalChapterCount(totalChapterCount) @@ -869,6 +870,16 @@ class TopicListController @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 { return if (loadLessonProtosFromAssets) { @@ -937,7 +948,7 @@ class TopicListController @Inject constructor( } } return ClassroomRecord.newBuilder().apply { - id = checkNotNull(classroomObj.optString("id")) { + id = checkNotNull(classroomObj.optString("classroom_id")) { "Expected classroom to have ID." } translatableTitle = SubtitledHtml.newBuilder().apply { diff --git a/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt index 439a331d4ea..02cd5660933 100755 --- a/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt @@ -130,14 +130,6 @@ class TopicControllerTest { assertThat(topic.description.html).contains("You'll often need to talk about") } - @Test - fun testRetrieveTopic_fractionsTopic_hasCorrectClassroomInfo() { - val topicProvider = topicController.getTopic(profileId1, FRACTIONS_TOPIC_ID) - - val topic = monitorFactory.waitForNextSuccessfulResult(topicProvider).topic - assertThat(topic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) - } - @Test fun testRetrieveTopic_ratiosTopic_returnsCorrectTopic() { val topicProvider = topicController.getTopic(profileId1, RATIOS_TOPIC_ID) @@ -158,14 +150,6 @@ class TopicControllerTest { ) } - @Test - fun testRetrieveTopic_ratiosTopic_hasCorrectClassroomInfo() { - val topicProvider = topicController.getTopic(profileId1, RATIOS_TOPIC_ID) - - val topic = monitorFactory.waitForNextSuccessfulResult(topicProvider).topic - assertThat(topic.classroomId).isEqualTo(TEST_CLASSROOM_ID_1) - } - @Test fun testRetrieveTopic_invalidTopic_returnsFailure() { val topicProvider = topicController.getTopic(profileId1, INVALID_TOPIC_ID_1) diff --git a/model/src/main/proto/topic.proto b/model/src/main/proto/topic.proto index a1ccaabed34..6ec81bca22d 100755 --- a/model/src/main/proto/topic.proto +++ b/model/src/main/proto/topic.proto @@ -25,9 +25,6 @@ message Topic { // A brief description of the topic. SubtitledHtml description = 11; - // The ID of the classroom this topic is part of. - string classroom_id = 12; - // A list of summarized stories contained within this topic. repeated StorySummary story = 4; @@ -58,17 +55,11 @@ message EphemeralTopic { // The translation context that should be used for this topic. WrittenTranslationContext written_translation_context = 2; - // The written translation context for the topic's classroom strings. - WrittenTranslationContext classroom_written_translation_context = 5; - // The EphemeralStorySummarys corresponding to the stories contained within this topic. repeated EphemeralStorySummary stories = 3; // The EphemeralSubtopics corresponding to the subtopics contained within this topic. repeated EphemeralSubtopic subtopics = 4; - - // The title of the classroom this topic is part of. - SubtitledHtml classroom_title = 6; } // Corresponds to details around whether a particular topic is playable. @@ -673,9 +664,6 @@ message TopicRecord { // The topic's description. SubtitledHtml translatable_description = 10; - // The ID of the classroom this topic is part of. - string classroom_id = 11; - // The list of canonical story IDs that can be used to load stories from the local filesystem. repeated string canonical_story_ids = 4; From 8296e2e5d6abbc6d0c06b1ab59b2af5c790ba92d Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 20 Jun 2024 10:58:32 +0530 Subject: [PATCH 34/36] Remove unused import & variable --- .../main/java/org/oppia/android/domain/topic/TopicController.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt index 72e756049e0..f893e87a4b2 100755 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt @@ -6,7 +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.ClassroomRecord import org.oppia.android.app.model.CompletedStory import org.oppia.android.app.model.CompletedStoryList import org.oppia.android.app.model.EphemeralChapterSummary @@ -555,7 +554,6 @@ class TopicController @Inject constructor( contentId = "title" html = topicData.getStringFromObject("topic_name") }.build() - val classroomId = topicData.getStringFromObject("classroom_id") val topicDescription = SubtitledHtml.newBuilder().apply { contentId = "description" html = topicData.getStringFromObject("topic_description") From 8c5a77b03d95f12a7ca7c5a82f91266c73b49652 Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 20 Jun 2024 16:32:31 +0530 Subject: [PATCH 35/36] Fetch classroom ID from topic ID --- .../java/org/oppia/android/domain/topic/TopicListController.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index b8e2333d58f..c698f67f0ff 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -223,6 +223,7 @@ class TopicListController @Inject constructor( title = topicRecord.translatableTitle totalChapterCount = storyRecords.map { it.chaptersList.size }.sum() topicThumbnail = topicRecord.topicThumbnail + classroomId = getClassroomIdByTopicId(topicId) topicPlayAvailability = if (topicRecord.isPublished) { TopicPlayAvailability.newBuilder().setAvailableToPlayNow(true).build() } else { @@ -262,7 +263,7 @@ class TopicListController @Inject constructor( contentId = "title" html = jsonObject.getStringFromObject("topic_name") }.build() - val classroomId = jsonObject.getStringFromObject("classroom_id") + val classroomId = getClassroomIdByTopicId(topicId) // No written translations are included since none are retrieved from JSON. return TopicSummary.newBuilder() .setTopicId(topicId) From 37dc6a31b1fe2e580906887d1313f27febcb0d9b Mon Sep 17 00:00:00 2001 From: Saptak Manna Date: Thu, 20 Jun 2024 17:10:00 +0530 Subject: [PATCH 36/36] Update left out classroom ID fetching --- .../org/oppia/android/domain/topic/TopicListController.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt index c698f67f0ff..067763a9963 100644 --- a/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt @@ -221,9 +221,9 @@ class TopicListController @Inject constructor( this.topicId = topicId putAllWrittenTranslations(topicRecord.writtenTranslationsMap) title = topicRecord.translatableTitle + classroomId = getClassroomIdByTopicId(topicId) totalChapterCount = storyRecords.map { it.chaptersList.size }.sum() topicThumbnail = topicRecord.topicThumbnail - classroomId = getClassroomIdByTopicId(topicId) topicPlayAvailability = if (topicRecord.isPublished) { TopicPlayAvailability.newBuilder().setAvailableToPlayNow(true).build() } else { @@ -300,7 +300,7 @@ class TopicListController @Inject constructor( html = jsonObject.getStringFromObject("topic_name") }.build() - val classroomId = jsonObject.getStringFromObject("classroom_id") + val classroomId = getClassroomIdByTopicId(topicId) val classroomJsonObject = jsonAssetRetriever.loadJsonFromAsset("$classroomId.json")!! val classroomTitle = classroomJsonObject.getJSONObject("classroom_title").let { @@ -787,7 +787,7 @@ class TopicListController @Inject constructor( }.build() } ?: SubtitledHtml.getDefaultInstance() - val classroomId = topicJson.optString("classroom_id") + val classroomId = getClassroomIdByTopicId(topicId) val classroomJson = jsonAssetRetriever.loadJsonFromAsset("$classroomId.json") if (classroomJson!!.optString("classroom_title").isNullOrEmpty()) return null