Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix part of #2132: Chapter List Page Issues #2232

Merged
merged 18 commits into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class StoryChapterSummaryViewModel(
val name: String = chapterSummary.name
val summary: String = chapterSummary.summary
val chapterThumbnail: LessonThumbnail = chapterSummary.chapterThumbnail
val missingPrerequisiteChapter: ChapterSummary = chapterSummary.missingPrerequisiteChapter

fun onExplorationClicked() {
explorationDataController.stopPlayingExploration()
Expand Down
4 changes: 1 addition & 3 deletions app/src/main/res/layout-land/story_chapter_view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,8 @@
android:layout_marginTop="8dp"
android:layout_marginEnd="8dp"
android:layout_marginBottom="4dp"
android:ellipsize="end"
android:fontFamily="sans-serif"
android:maxLines="3"
android:text="@{htmlContent}"
android:text="@{viewModel.chapterSummary.chapterPlayState != ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES ? htmlContent : @string/chapter_prerequisite_title_label(viewModel.index, viewModel.missingPrerequisiteChapter.name)}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="16sp"
app:layout_constraintBottom_toBottomOf="parent"
Expand Down
4 changes: 1 addition & 3 deletions app/src/main/res/layout-sw600dp/story_chapter_view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,8 @@
android:layout_marginTop="8dp"
android:layout_marginEnd="8dp"
android:layout_marginBottom="4dp"
android:ellipsize="end"
android:fontFamily="sans-serif"
android:maxLines="4"
android:text="@{htmlContent}"
android:text="@{viewModel.chapterSummary.chapterPlayState != ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES ? htmlContent : @string/chapter_prerequisite_title_label(viewModel.index, viewModel.missingPrerequisiteChapter.name)}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="16sp"
app:layout_constraintBottom_toBottomOf="parent"
Expand Down
4 changes: 1 addition & 3 deletions app/src/main/res/layout/story_chapter_view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,8 @@
android:layout_marginTop="8dp"
android:layout_marginEnd="8dp"
android:layout_marginBottom="4dp"
android:ellipsize="end"
android:fontFamily="sans-serif"
android:maxLines="4"
android:text="@{htmlContent}"
android:text="@{viewModel.chapterSummary.chapterPlayState != ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES ? htmlContent : @string/chapter_prerequisite_title_label(viewModel.index, viewModel.missingPrerequisiteChapter.name)}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="16sp"
app:layout_constraintBottom_toBottomOf="parent"
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/res/values/dimens.xml
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@

<dimen name="story_chapter_card_playable_state_elevation">4dp</dimen>
<dimen name="ongoing_topic_card_elevation">4dp</dimen>
<dimen name="story_chapter_card_not_playable_state_elevation">0dp</dimen>
<dimen name="story_chapter_card_not_playable_state_elevation">2dp</dimen>
<dimen name="profile_chooser_padding_bottom_profile_already_added">224dp</dimen>

<dimen name="continue_item_exploration_split_view_margin_top">72dp</dimen>
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
<string name="topic_play_chapter_index">%s.</string>
<string name="chapter_name">Chapter %s: %s</string>
<string name="chapter_completed">Chapter %s: %s is completed.</string>
<string name="chapter_prerequisite_title_label">Complete Chapter %s: %s to unlock this chapter.</string>
<string name="text_input_default_content_description">Enter text.</string>
<string name="fractions_default_hint_text">Enter a fraction in the form x/x, or a mixed number in the form x x/x.</string>
<string name="fractions_default_hint_text_no_integer">Enter a fraction in the form x/x.</string>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,6 @@ class StoryFragmentTest {
profileTestHelper.initializeProfiles()
FirebaseApp.initializeApp(context)
profileId = ProfileId.newBuilder().setInternalId(internalProfileId).build()
storyProgressTestHelper.markPartialStoryProgressForFractions(
profileId,
timestampOlderThanAWeek = false
)
testCoroutineDispatchers.runCurrent()
}

Expand Down Expand Up @@ -159,6 +155,10 @@ class StoryFragmentTest {
@Test
fun testStoryFragment_correctStoryCountLoadedInHeader() {
launch<StoryActivity>(createStoryActivityIntent()).use {
storyProgressTestHelper.markPartialStoryProgressForFractions(
anandwana001 marked this conversation as resolved.
Show resolved Hide resolved
anandwana001 marked this conversation as resolved.
Show resolved Hide resolved
profileId,
timestampOlderThanAWeek = false
)
testCoroutineDispatchers.runCurrent()
val headerString: String =
getResources().getQuantityString(R.plurals.story_total_chapters, 2, 1, 2)
Expand Down Expand Up @@ -242,6 +242,41 @@ class StoryFragmentTest {
}
}

@Test
fun testStoryFragment_chapterMissingPrerequisiteIsShownCorrectly() {
launch<StoryActivity>(createStoryActivityIntent()).use {
testCoroutineDispatchers.runCurrent()
onView(allOf(withId(R.id.story_chapter_list))).perform(
scrollToPosition<RecyclerView.ViewHolder>(
2
)
)
onView(atPositionOnView(R.id.story_chapter_list, 2, R.id.chapter_summary)).check(
anandwana001 marked this conversation as resolved.
Show resolved Hide resolved
matches(
withText("Complete Chapter 1: What is a Fraction? to unlock this chapter.")
)
)
}
}

@Test
fun testStoryFragment_changeConfiguration_chapterMissingPrerequisiteIsShownCorrectly() {
anandwana001 marked this conversation as resolved.
Show resolved Hide resolved
launch<StoryActivity>(createStoryActivityIntent()).use {
testCoroutineDispatchers.runCurrent()
onView(isRoot()).perform(orientationLandscape())
onView(allOf(withId(R.id.story_chapter_list))).perform(
scrollToPosition<RecyclerView.ViewHolder>(
2
)
)
onView(atPositionOnView(R.id.story_chapter_list, 2, R.id.chapter_summary)).check(
matches(
withText("Complete Chapter 1: What is a Fraction? to unlock this chapter.")
)
)
}
}

@Test
fun testStoryFragment_changeConfiguration_explorationStartCorrectly() {
launch<StoryActivity>(createStoryActivityIntent()).use {
Expand All @@ -263,6 +298,10 @@ class StoryFragmentTest {
@Test
fun testStoryFragment_changeConfiguration_correctStoryCountInHeader() {
launch<StoryActivity>(createStoryActivityIntent()).use {
storyProgressTestHelper.markPartialStoryProgressForFractions(
profileId,
timestampOlderThanAWeek = false
)
testCoroutineDispatchers.runCurrent()
onView(isRoot()).perform(orientationLandscape())
val headerString: String =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,24 +333,20 @@ class TopicController @Inject constructor(
if (storyProgress.chapterProgressMap.isNotEmpty()) {
val storyBuilder = storySummary.toBuilder()
storySummary.chapterList.forEachIndexed { chapterIndex, chapterSummary ->
val chapterBuilder = chapterSummary.toBuilder()
if (storyProgress.chapterProgressMap.containsKey(chapterSummary.explorationId)) {
val chapterBuilder = chapterSummary.toBuilder()
chapterBuilder.chapterPlayState =
storyProgress.chapterProgressMap[chapterSummary.explorationId]!!.chapterPlayState
storyBuilder.setChapter(chapterIndex, chapterBuilder)
} else {
if (storyBuilder.getChapter(chapterIndex - 1).chapterPlayState ==
ChapterPlayState.COMPLETED
) {
val chapterBuilder = chapterSummary.toBuilder()
val prerequisiteChapter = storyBuilder.getChapter(chapterIndex - 1)
if (prerequisiteChapter.chapterPlayState == ChapterPlayState.COMPLETED) {
chapterBuilder.chapterPlayState = ChapterPlayState.NOT_STARTED
storyBuilder.setChapter(chapterIndex, chapterBuilder)
} else {
val chapterBuilder = chapterSummary.toBuilder()
chapterBuilder.chapterPlayState = ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES
storyBuilder.setChapter(chapterIndex, chapterBuilder)
chapterBuilder.missingPrerequisiteChapter = prerequisiteChapter
}
}
storyBuilder.setChapter(chapterIndex, chapterBuilder)
}
return storyBuilder.build()
} else {
Expand Down Expand Up @@ -386,10 +382,11 @@ class TopicController @Inject constructor(
val storyBuilder = storySummary.toBuilder()
storySummary.chapterList.forEachIndexed { index, chapterSummary ->
val chapterBuilder = chapterSummary.toBuilder()
chapterBuilder.chapterPlayState = if (index != 0) {
ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES
if (index != 0) {
chapterBuilder.chapterPlayState = ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES
chapterBuilder.missingPrerequisiteChapter = storySummary.chapterList.get(index - 1)
} else {
ChapterPlayState.NOT_STARTED
chapterBuilder.chapterPlayState = ChapterPlayState.NOT_STARTED
}
storyBuilder.setChapter(index, chapterBuilder)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ class TopicControllerTest {
}

@Test
fun testRetrieveStory_validSecondStory_returnsStoryWithProgress() {
fun testRetrieveStory_validSecondStory_returnsStoryWithoutProgress() {
topicController.getStory(profileId1, TEST_TOPIC_ID_0, TEST_STORY_ID_1).toLiveData()
.observeForever(mockStorySummaryObserver)
testCoroutineDispatchers.runCurrent()
Expand All @@ -424,8 +424,31 @@ class TopicControllerTest {
.isEqualTo(ChapterPlayState.NOT_STARTED)
assertThat(story.getChapter(1).chapterPlayState)
.isEqualTo(ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES)
assertThat(story.getChapter(1).missingPrerequisiteChapter.name)
.isEqualTo(story.getChapter(0).name)
assertThat(story.getChapter(2).chapterPlayState)
.isEqualTo(ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES)
assertThat(story.getChapter(2).missingPrerequisiteChapter.name)
.isEqualTo(story.getChapter(1).name)
}

@Test
fun testRetrieveStory_validSecondStory_returnsStoryWithProgress() {
topicController.getStory(profileId1, TEST_TOPIC_ID_0, TEST_STORY_ID_1).toLiveData()
.observeForever(mockStorySummaryObserver)
markSecondStory1Chapter1AsCompleted()
testCoroutineDispatchers.runCurrent()

verifyGetStorySucceeded()
val story = storySummaryResultCaptor.value!!.getOrThrow()
assertThat(story.getChapter(0).chapterPlayState)
.isEqualTo(ChapterPlayState.COMPLETED)
assertThat(story.getChapter(1).chapterPlayState)
.isEqualTo(ChapterPlayState.NOT_STARTED)
assertThat(story.getChapter(2).chapterPlayState)
.isEqualTo(ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES)
assertThat(story.getChapter(2).missingPrerequisiteChapter.name)
.isEqualTo(story.getChapter(1).name)
}

@Test
Expand Down Expand Up @@ -884,6 +907,8 @@ class TopicControllerTest {
.isEqualTo(ChapterPlayState.NOT_STARTED)
assertThat(topic.storyList[0].chapterList[1].chapterPlayState)
.isEqualTo(ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES)
assertThat(topic.storyList[0].chapterList[1].missingPrerequisiteChapter.name)
.isEqualTo(topic.storyList[0].chapterList[0].name)
}

@Test
Expand Down Expand Up @@ -927,6 +952,8 @@ class TopicControllerTest {
.isEqualTo(ChapterPlayState.NOT_STARTED)
assertThat(storySummary.chapterList[1].chapterPlayState)
.isEqualTo(ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES)
assertThat(storySummary.chapterList[1].missingPrerequisiteChapter.name)
.isEqualTo(storySummary.chapterList[0].name)
}

@Test
Expand Down Expand Up @@ -1162,6 +1189,16 @@ class TopicControllerTest {
).toLiveData().observeForever(mockRecordProgressObserver)
}

private fun markSecondStory1Chapter1AsCompleted() {
storyProgressController.recordCompletedChapter(
profileId1,
TEST_TOPIC_ID_0,
TEST_STORY_ID_1,
TEST_EXPLORATION_ID_1,
currentTimestamp
).toLiveData().observeForever(mockRecordProgressObserver)
}

private fun markRatiosStory0Chapter0AsCompleted() {
storyProgressController.recordCompletedChapter(
profileId1,
Expand Down
4 changes: 4 additions & 0 deletions model/src/main/proto/topic.proto
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ message ChapterSummary {

// The thumbnail corresponding to this chapter.
LessonThumbnail chapter_thumbnail = 5;

// Prerequisite chapter to unlock this chapter. It is only defined when the play state of this
// chapter is NOT_PLAYABLE_MISSING_PREREQUISITES.
ChapterSummary missing_prerequisite_chapter = 6;
}

// A summary of a skill that can be trained or reviewed by the player.
Expand Down