(Contributor Recognition Infrastructure) Milestone 1.3: Add required suggestion services to handle contributor stats #15917

chris7716 · 2022-08-17T18:52:10Z

Overview

This PR fixes or fixes part of n/a.
This PR does the following:3.

Added stats services and relevant unit tests
Doc: https://docs.google.com/document/d/16SciF9u-AngOdmvBaSyjYCAYrppR6lTE-S2vwhhh86A/edit#

Essential 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 linter/Karma presubmit checks have passed locally on your machine.
"Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)
- This lets reviewers restart your CircleCI tests for you.
The PR is made from a branch that's not called "develop".

Proof that changes are correct

Proof of changes on desktop with slow/throttled network

Proof of changes on mobile phone

Proof of changes in Arabic language

PR Pointers

Make sure to follow the instructions for making a code change.
If you need a review or an answer to a question, and don't have permissions to assign people, leave a comment like the following: "{{Question/comment}} @{{reviewer_username}} PTAL". Oppiabot will help assign that person for you.
For what code owners will expect, see the Code Owner's wiki page.
Make sure your PR follows conventions in the style guide, otherwise this will lead to review delays.
Never force push. If you do, your PR will be closed.
Some of the e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.

Merged with uostream

merged with develop

uptodated

Update develop

Update branch

Update local

…ne1.1 Update brancj

…ne1.1 Update branch

…ne1.3 Update branch

oppiabot · 2022-08-17T18:52:11Z

Hi @chris7716, can you complete the following:

The karma and linter checklist has not been checked, please make sure to run the frontend tests and lint tests before pushing.
Thanks!

sagangwee

@chris7716 Thanks, just a few comments.

sagangwee · 2022-09-12T22:01:14Z

core/domain/suggestion_services_test.py

+        # Steps required in the setup phase before testing.
+        # 1. Save new skills.
+        # 2. Save a topic assigning skills for it.
+        # 4. Create a question suggestion.


Suggested change

# 4. Create a question suggestion.

# 3. Create a question suggestion.

sagangwee · 2022-09-12T22:01:28Z

core/domain/suggestion_services_test.py

+        # Steps required in the setup phase before testing.
+        # 1. Save new skills.
+        # 2. Save a topic assigning skills for it.
+        # 4. Create a question suggestion.


Suggested change

# 4. Create a question suggestion.

# 3. Create a question suggestion.

sagangwee · 2022-09-12T22:04:58Z

core/domain/suggestion_services_test.py

+        suggestion_services.update_question_contribution_stats_at_submission(
+            initial_suggestion
+        )
+        suggestion_services.update_question_contribution_stats_at_submission(
+            latest_suggestion
+        )
+
+        question_contribution_stats_model = (
+            suggestion_models.QuestionContributionStatsModel.get(
+                self.author_id, topic_id
+            )
+        )
+        # Assert question contribution stats before the review.
+        # At this point we can confirm that there should be an associated
+        # question contribution stat object for the given IDs since we have
+        # called update_question_contribution_stats_at_submission function to
+        # create/update question contribution stats.
+        assert question_contribution_stats_model is not None
+        self.assertEqual(
+            question_contribution_stats_model.submitted_questions_count,
+            2
+        )
+        self.assertEqual(
+            question_contribution_stats_model.accepted_questions_count,
+            0
+        )


Right, but we've already tested the behavior when submitting in the test above, so it doesn't seem necessary to repeat it here, when the main thing we're testing in this test is the behavior after accepting a suggestion. Ditto below.

sagangwee · 2022-09-12T22:38:56Z

core/domain/suggestion_services_test.py

+        # Base verification of contributor stats before the review is made.
+        translation_contribution_stats_model = (
+            suggestion_models.TranslationContributionStatsModel.get(
+                'hi', self.author_id, '0'
+            )
+        )
+        # Assert translation contribution stats before the review.
+        # At this point we can confirm that there should be an associated
+        # translation contribution stat object for the given IDs since we have
+        # called update_translation_contribution_stats_at_submission function
+        # to create/update translation contribution stats.
+        assert translation_contribution_stats_model is not None
+        self.assertEqual(
+            translation_contribution_stats_model.submitted_translations_count,
+            2
+        )
+        self.assertEqual(
+            (
+                translation_contribution_stats_model
+                .submitted_translation_word_count
+            ),
+            6
+        )
+        self.assertEqual(
+            translation_contribution_stats_model.accepted_translations_count,
+            0
+        )


Because we already test this above and to cut down a bit on these long tests, I think it's worth it to remove these lines. Ditto below.

oppiabot · 2022-09-12T22:42:28Z

Unassigning @sagangwee since the review is done.

oppiabot · 2022-09-12T22:42:32Z

Hi @chris7716, it looks like some changes were requested on this pull request by @sagangwee. PTAL. Thanks!

sagangwee

@chris7716 LGTM, thanks!

oppiabot · 2022-09-13T22:22:05Z

Unassigning @sagangwee since they have already approved the PR.

vojtechjelinek

LGTM!

oppiabot · 2022-09-15T08:05:13Z

Hi @chris7716, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

…ne1.3 Update branch

…suggestion services to handle contributor stats (oppia#15917) * service file name changed to backend-api * backend api file added * previous service file removed from CODEOWNERS * new backend service file added to CODEOWNERS * First pass * Fix conflicts * Fix backend tests * Fix lint * Increase coverage * Fix lint * Address comments * Removed unwanted fields * Address comments * First pass * Fix conflicts * Enhance backend coverage * Remove unwanted prints * Remove unnecessary conditions * Fix tests * Fix lint * Fix lint * Fix mypy and backend tests * Address comments * Fix lint * Fix lint * Fix lint * Fix mypy * Address comments * Fix mypy * Fix lint * Fix mypy * Fix mypy * Fix mypy * Fix mypy * Address all comments * Fix lint * Fix lint * Fix mypy * Fix comments * Fix lint * Fix mypy * Address comments * Fix lint * Address comments * Fix lint * Fix lint * Fix lint * Address comments * Fix lint * Fix lint * Fix lint * Address comments * Fix lint * Address comments * Fix lint * Fix lint * Fix lint * Fix lint * Fix lint * Fix lint * Address comments * Fix lint * Fix mypy * Address comments * Address comments * Fix lint * Fix minor mypy * Fix commnets * Fix commnets * Fix lint * Address comments * Fix mypy * Fix tests * Fix tests * Remove unnecessary comments * Fix tests * Fix tests * Fix coverage

chris7716 and others added 25 commits January 10, 2021 01:32

service file name changed to backend-api

c798e78

backend api file added

218b79c

previous service file removed from CODEOWNERS

d03d579

new backend service file added to CODEOWNERS

536a551

Merge remote-tracking branch 'upstream/develop' into develop

b853c9a

Merged with uostream

Merge remote-tracking branch 'upstream/develop' into develop

cab061f

merged with develop

Merge remote-tracking branch 'upstream/develop' into develop

7f746b9

uptodated

Merge branch 'oppia:develop' into develop

63a9988

Merge branch 'oppia:develop' into develop

40e6c29

Merge branch 'develop' of https://github.com/oppia/oppia into develop

0dd7b48

Update develop

Merge branch 'develop' of https://github.com/oppia/oppia into develop

bbfa22c

Update branch

Merge branch 'develop' of https://github.com/oppia/oppia into develop

8bcdecd

Update local

Merge branch 'develop' of https://github.com/oppia/oppia into milesto…

b693c80

…ne1.1 Update brancj

First pass

3f9b46c

Fix conflicts

5ebbfcc

Fix backend tests

9a46e21

Fix lint

b95ce3d

Increase coverage

958d302

Fix lint

878a917

Merge branch 'develop' of https://github.com/oppia/oppia into milesto…

e8520e3

…ne1.1 Update branch

Address comments

91538b8

Removed unwanted fields

9963027

Address comments

4980b09

Merge branch 'develop' of https://github.com/oppia/oppia into milesto…

2eb06d4

…ne1.3 Update branch

First pass

aca05ea

chris7716 requested review from a team as code owners August 17, 2022 18:52

chris7716 requested review from vojtechjelinek and seanlip August 17, 2022 18:52

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Sep 12, 2022

chris7716 mentioned this pull request Sep 12, 2022

(Contributor Recognition Infrastructure) Milestone 1.6: Implement controller layer for the project #16044

Merged

4 tasks

sagangwee suggested changes Sep 12, 2022

View reviewed changes

oppiabot bot unassigned sagangwee Sep 12, 2022

oppiabot bot assigned chris7716 Sep 12, 2022

Address comments

a3d28f3

chris7716 requested a review from sagangwee September 13, 2022 15:21

chris7716 assigned sagangwee and unassigned chris7716 Sep 13, 2022

Fix mypy

6ea0885

sagangwee approved these changes Sep 13, 2022

View reviewed changes

oppiabot bot unassigned sagangwee Sep 13, 2022

chris7716 added 2 commits September 15, 2022 00:19

Fix tests

b67ab1e

Fix tests

33e82c2

vojtechjelinek approved these changes Sep 15, 2022

View reviewed changes

vojtechjelinek assigned chris7716 and unassigned vojtechjelinek Sep 15, 2022

oppiabot bot added the PR: LGTM label Sep 15, 2022

chris7716 added 5 commits September 15, 2022 17:44

Remove unnecessary comments

acf8e42

Merge branch 'develop' of https://github.com/oppia/oppia into milesto…

5d25712

…ne1.3 Update branch

Fix tests

856d353

Fix tests

37d6f2c

Fix coverage

b3802ec

chris7716 merged commit 6c20d7a into oppia:develop Sep 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(Contributor Recognition Infrastructure) Milestone 1.3: Add required suggestion services to handle contributor stats #15917

(Contributor Recognition Infrastructure) Milestone 1.3: Add required suggestion services to handle contributor stats #15917

chris7716 commented Aug 17, 2022

oppiabot bot commented Aug 17, 2022

sagangwee left a comment

sagangwee Sep 12, 2022

chris7716 Sep 13, 2022

sagangwee Sep 12, 2022

chris7716 Sep 13, 2022

sagangwee Sep 12, 2022

sagangwee Sep 12, 2022

chris7716 Sep 13, 2022

oppiabot bot commented Sep 12, 2022

oppiabot bot commented Sep 12, 2022

sagangwee left a comment

oppiabot bot commented Sep 13, 2022

vojtechjelinek left a comment

oppiabot bot commented Sep 15, 2022

	# 4. Create a question suggestion.
	# 3. Create a question suggestion.

(Contributor Recognition Infrastructure) Milestone 1.3: Add required suggestion services to handle contributor stats #15917

(Contributor Recognition Infrastructure) Milestone 1.3: Add required suggestion services to handle contributor stats #15917

Conversation

chris7716 commented Aug 17, 2022

Overview

Essential Checklist

Proof that changes are correct

Proof of changes on desktop with slow/throttled network

Proof of changes on mobile phone

Proof of changes in Arabic language

PR Pointers

oppiabot bot commented Aug 17, 2022

sagangwee left a comment

Choose a reason for hiding this comment

sagangwee Sep 12, 2022

Choose a reason for hiding this comment

chris7716 Sep 13, 2022

Choose a reason for hiding this comment

sagangwee Sep 12, 2022

Choose a reason for hiding this comment

chris7716 Sep 13, 2022

Choose a reason for hiding this comment

sagangwee Sep 12, 2022

Choose a reason for hiding this comment

sagangwee Sep 12, 2022

Choose a reason for hiding this comment

chris7716 Sep 13, 2022

Choose a reason for hiding this comment

oppiabot bot commented Sep 12, 2022

oppiabot bot commented Sep 12, 2022

sagangwee left a comment

Choose a reason for hiding this comment

oppiabot bot commented Sep 13, 2022

vojtechjelinek left a comment

Choose a reason for hiding this comment

oppiabot bot commented Sep 15, 2022