Fix #2738 and #3067: Mastery calculations and tests #3156

TheRealJessicaLi · 2021-05-02T18:21:16Z

Explanation

Added mastery calculations (with tests) for practice sessions. This fixes #3067 and #2738, but cannot be merged until PR #3040 (score calculations) is.

Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
The PR follows the style guide.
The PR does not contain any unnecessary auto-generated code from Android Studio.
The PR is made from a branch that's not called "develop".
The PR is made from a branch that is up-to-date with "develop".
The PR's branch is based on "develop" and not on any other branch.
The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

linted again more linting :) linted

linted more linting

BenHenning · 2021-05-06T09:13:47Z

Sorry, been really busy with non-work items this week. Will need to take a look later in the week.

BenHenning · 2021-05-08T02:25:35Z

FYI that I'll be unavailable for reviews for the next ~18 days, so I probably won't be able to look at this until later. Please don't block on my review unless needed.

vinitamurthi · 2021-05-11T07:05:29Z

Hey @TheRealJessicaLi , assign me back once the bazel build passes and I will take a look!

vinitamurthi

LGTM thanks! Just a few comments that need to be added but otherwise looks good

vinitamurthi · 2021-06-09T05:19:07Z

.../src/test/java/org/oppia/android/domain/question/QuestionAssessmentProgressControllerTest.kt

+    // This will generate question 1 (skill 0), question 2 (skill 0), and question 3 (skill 1)
+    startTrainingSession(TEST_SKILL_ID_LIST_01)
+
+    // Question 1


Can you add a comment below that you are submitting a wrong answer here too?

vinitamurthi · 2021-06-09T05:19:22Z

.../src/test/java/org/oppia/android/domain/question/QuestionAssessmentProgressControllerTest.kt

+    submitCorrectAnswerForQuestion2()
+
+    // Question 3
+    submitIncorrectAnswerForQuestion3("3/4")


Same comment here

vinitamurthi · 2021-06-09T05:20:04Z

.../src/test/java/org/oppia/android/domain/question/QuestionAssessmentProgressControllerTest.kt

+    startTrainingSession(TEST_SKILL_ID_LIST_01)
+
+    // Question 1
+    submitIncorrectAnswerForQuestion1(2)


Add comments here too! Here and below

oppiabot · 2021-06-09T05:24:12Z

Unassigning @vinitamurthi since they have already approved the PR.

oppiabot · 2021-06-09T05:24:15Z

Assigning @rt4914 for code owner reviews. Thanks!

rt4914

LGTM. Thanks.

rt4914 · 2021-06-09T19:42:50Z

domain/src/main/java/org/oppia/android/domain/question/QuestionAssessmentCalculation.kt

+    }.mapValues { (_, masteryChanges) ->
+      // Convert to Map<String, Double> for each linked skill ID.
+      masteryChanges.sum().toDouble() / internalMasteryMultiplyFactor
+    }


TheRealJessicaLi · 2021-06-20T22:32:27Z

@vinitamurthi I've updated my PR and all the tests are passing now, can you help me merge this in?

vinitamurthi · 2021-06-22T14:37:50Z

Thanks !Merging now

TheRealJessicaLi added 20 commits April 3, 2021 21:48

added score calculations

6be0a7a

linted

2628b89

linted again more linting :) linted

addressed comments

d848ee0

use BigDecimal to avoid floating point error

419ef0c

linted more linting

fixed as per comments

c70a6e3

mock constants for other related tests

bf985f1

linted

7092c95

fixed merge conflicts

3816c7f

addressed comments (minus factory pattern)

096b024

refactored to use factory pattern

d46fba7

lint

b78454c

more lint

430e366

updated textproto with new hint

d7ebefa

fix json formatting

ee08db8

fixed as per comments

d4aa959

made calculated scores immutable

06d3506

linted

e75cced

added logic for computing masteries

0559c15

added tests

015895a

lint

de0a035

TheRealJessicaLi requested review from BenHenning, rt4914 and vinitamurthi as code owners May 2, 2021 18:21

TheRealJessicaLi assigned vinitamurthi, BenHenning and TheRealJessicaLi May 2, 2021

anandwana001 added the PR don't merge - NEEDS UPDATE Corresponds to PRs that need to be updated with the latest develop changes before they can be merged label May 7, 2021

BenHenning removed their assignment May 8, 2021

vinitamurthi removed their assignment May 11, 2021

Merge branch 'develop' into mastery-calculations

dd9bffa

TheRealJessicaLi assigned vinitamurthi May 16, 2021

vinitamurthi removed their assignment May 20, 2021

TheRealJessicaLi added 2 commits May 31, 2021 01:30

merged in calculations PR

6a5b0af

linted

b6b78b9

TheRealJessicaLi assigned vinitamurthi May 31, 2021

TheRealJessicaLi removed the PR don't merge - NEEDS UPDATE Corresponds to PRs that need to be updated with the latest develop changes before they can be merged label May 31, 2021

Merge branch 'develop' into mastery-calculations

fb5a159

TheRealJessicaLi assigned BenHenning Jun 2, 2021

vinitamurthi approved these changes Jun 9, 2021

View reviewed changes

oppiabot bot unassigned vinitamurthi Jun 9, 2021

oppiabot bot assigned rt4914 Jun 9, 2021

rt4914 approved these changes Jun 9, 2021

View reviewed changes

rt4914 removed their assignment Jun 9, 2021

TheRealJessicaLi added 2 commits June 20, 2021 15:37

updated comments in tests

a0d5f3b

Merge branch 'develop' into mastery-calculations

372cd39

TheRealJessicaLi assigned vinitamurthi Jun 20, 2021

vinitamurthi merged commit 5b9cd52 into oppia:develop Jun 22, 2021

vinitamurthi removed their assignment Jun 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #2738 and #3067: Mastery calculations and tests #3156

Fix #2738 and #3067: Mastery calculations and tests #3156

TheRealJessicaLi commented May 2, 2021 •

edited

BenHenning commented May 6, 2021

BenHenning commented May 8, 2021

vinitamurthi commented May 11, 2021

vinitamurthi left a comment

vinitamurthi Jun 9, 2021

TheRealJessicaLi Jun 20, 2021

vinitamurthi Jun 9, 2021

TheRealJessicaLi Jun 20, 2021

vinitamurthi Jun 9, 2021

TheRealJessicaLi Jun 20, 2021

oppiabot bot commented Jun 9, 2021

oppiabot bot commented Jun 9, 2021

rt4914 left a comment

rt4914 Jun 9, 2021

TheRealJessicaLi Jun 20, 2021

TheRealJessicaLi commented Jun 20, 2021

vinitamurthi commented Jun 22, 2021

Fix #2738 and #3067: Mastery calculations and tests #3156

Fix #2738 and #3067: Mastery calculations and tests #3156

Conversation

TheRealJessicaLi commented May 2, 2021 • edited

Explanation

Checklist

BenHenning commented May 6, 2021

BenHenning commented May 8, 2021

vinitamurthi commented May 11, 2021

vinitamurthi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oppiabot bot commented Jun 9, 2021

oppiabot bot commented Jun 9, 2021

rt4914 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheRealJessicaLi commented Jun 20, 2021

vinitamurthi commented Jun 22, 2021

TheRealJessicaLi commented May 2, 2021 •

edited