Wipeout 2.8: Add has_reference_to_user_id to remainig models #7893

vojtechjelinek · 2019-11-01T22:20:48Z

Explanation

Fixes Add has_reference_to_user_id to remaining models #7858: Add has_reference_to_user_id to remaining models
Add indexed=True to SendEmailModel and BulkEmailModel
Add EmailModelsIndexesOneOffJob to reindex SendEmailModel and BulkEmailModel
Remove ReviewerRotationTrackingModel

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 linter/Karma presubmit checks have passed.
- These should run automatically, but if not, you can manually trigger them locally using python -m scripts.pre_commit_linter and python -m scripts.run_frontend_tests.
The PR is made from a branch that's not called "develop".
The PR has an appropriate "PROJECT: ..." label (Please add this label for the first-pass review of the PR).
The PR has an appropriate "CHANGELOG: ..." label (If you are unsure of which label to add, ask the reviewers for guidance).
The PR follows the style guide.
The PR addresses the points mentioned in the codeowner checks for the files/folders changed. (See the codeowner's wiki page.)
The PR is assigned to an appropriate reviewer.
- If you're a new contributor, please ask on Gitter for someone to assign a reviewer and don't tick this checkbox.
- If you're not sure who the appropriate reviewer is, please assign to the issue's "owner" -- see the "talk-to" label on the issue. Do not only request the review but also add the reviewer as an assignee.

…-models � Conflicts: � core/storage/question/gae_models_test.py � core/storage/user/gae_models_test.py

…-models � Conflicts: � core/storage/storage_models_test.py

…-models

into wipeout-models

…-models

oppiabot · 2019-11-01T22:20:50Z

Hi, @vojtechjelinek. This pull request does not have a "CHANGELOG: ..." label as mentioned in the PR checkbox list. Please add this label. PRs without this label will not be merged. If you are unsure of which label to add, please ask the reviewers for guidance. Thanks!

seanlip · 2019-11-01T23:18:02Z

@vojtechjelinek the backend tests are failing.

codecov · 2019-11-03T12:51:12Z

Codecov Report

Merging #7893 into develop will decrease coverage by 0.01%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #7893      +/-   ##
===========================================
- Coverage    84.03%   84.02%   -0.01%     
===========================================
  Files         1128     1128              
  Lines        67783    67745      -38     
  Branches      3805     3805              
===========================================
- Hits         56961    56922      -39     
- Misses        9543     9544       +1     
  Partials      1279     1279

Flag	Coverage Δ
#backend	`100% <100%> (ø)`	⬆️
#frontend	`73.68% <ø> (ø)`	⬇️

Impacted Files	Coverage Δ
core/jobs_registry.py	`100% <ø> (ø)`	⬆️
core/domain/suggestion_services.py	`100% <ø> (ø)`	⬆️
core/storage/suggestion/gae_models.py	`100% <ø> (ø)`	⬆️
main_cron.py	`100% <ø> (ø)`	⬆️
core/storage/recommendations/gae_models.py	`100% <ø> (ø)`	⬆️
core/controllers/cron.py	`100% <ø> (ø)`	⬆️
core/domain/prod_validation_jobs_one_off.py	`100% <ø> (ø)`	⬆️
core/storage/topic/gae_models.py	`100% <100%> (ø)`	⬆️
core/storage/skill/gae_models.py	`100% <100%> (ø)`	⬆️
core/domain/email_jobs_one_off.py	`100% <100%> (ø)`	⬆️
... and 4 more

oppiabot · 2019-11-03T18:11:38Z

Assigning @seanlip for the first-pass review of this pull request. Thanks!

seanlip

Couple of comments, but no real concerns (though please see if you can figure out why some changes from previous PRs are still being shown here).

Thanks!

seanlip · 2019-11-04T05:27:13Z

core/domain/activity_jobs_one_off.py

@@ -47,3 +55,34 @@ def map(item):
    @staticmethod
    def reduce(key, values):
        pass
+
+
+class SnapshotMetadataModelsIndexesJob(jobs.BaseMapReduceOneOffJobManager):


Er, this is weird. Haven't I already reviewed this in #7874? I don't understand why it's showing up again here?

seanlip · 2019-11-04T05:28:10Z

core/domain/email_jobs_one_off_test.py

+
+    def _run_one_off_job(self):
+        """Runs the one-off MapReduce job."""
+        job_id = (email_jobs_one_off.EmailModelsIndexesOneOffJob.create_new())


Nit: You don't need the outer parens.

seanlip · 2019-11-04T05:31:48Z

core/storage/topic/gae_models_test.py

+    def test_has_reference_to_user_id(self):
+        self.assertFalse(
+            topic_models.TopicSummaryModel.has_reference_to_user_id('any_id'))
+


Drop one of these newlines (there should only be two newlines above top-level class definitions).

seanlip · 2019-11-04T05:32:40Z

@aks681 @ankita240796 @nithusha21 PTAL as codeowners. Thanks!

ankita240796

LGTM from code-owner's perspective, Thanks @vojtechjelinek!

aks681

lgtm as codeowner

nithusha21

LGTM as a codeowner. Thanks!

…-models7

…ppia into wipeout-models7

vojtechjelinek added 30 commits August 24, 2019 16:59

Add deletion policies for topic models

7b55aca

Add deletion policies for skill models

5207305

Add deletion policies for question models

9686678

Add deletion policies for email models

42e28a2

Add deletion policies for feedback models

afd51e8

Add test deletion policies

c391acc

Add get_deletion_policy to models

33a9916

Add skip to the get_deletion_policy test

e9b4ce0

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

cdb77a5

…-models � Conflicts: � core/storage/question/gae_models_test.py � core/storage/user/gae_models_test.py

Fix lint errors

2c94143

Add test

022e526

Modify test

10dcdd3

Change comments

7ab8032

Add get_deltion_policy to other models

aee987f

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

ac17ad8

…-models � Conflicts: � core/storage/storage_models_test.py

Add recommendations tests

5f142d8

Reorder stats tests

bd195b3

Add tests for get_deletion_policy in stats

12667db

Modify test

b33d11d

Fix lint issues

fbb976a

Change policy for question-skill link

4fa6da9

Edit and fix tests

71c6cf7

Add has_reference_to_user_id to some models

5a008d0

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

d030464

…-models

Add get_deletion_policy to remaining models

b12ef31

Merge branch 'wipeout-models' of https://github.com/vojtechjelinek/oppia

0057239

into wipeout-models

Fix lint

8251af8

Assert review comments

122fb2b

Assert review comments

54f99db

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

884f3a8

…-models

seanlip assigned vojtechjelinek Nov 1, 2019

Fix backend tests

3d41a14

vojtechjelinek assigned seanlip and unassigned vojtechjelinek Nov 3, 2019

kevinlee12 added the PROJECT: Data handling -- @seanlip label Nov 3, 2019

seanlip approved these changes Nov 4, 2019

View reviewed changes

seanlip assigned vojtechjelinek, nithusha21, ankita240796 and aks681 and unassigned seanlip Nov 4, 2019

ankita240796 approved these changes Nov 4, 2019

View reviewed changes

ankita240796 removed their assignment Nov 4, 2019

aks681 approved these changes Nov 4, 2019

View reviewed changes

aks681 removed their assignment Nov 4, 2019

nithusha21 approved these changes Nov 4, 2019

View reviewed changes

nithusha21 removed their assignment Nov 4, 2019

vojtechjelinek added CHANGELOG: Infrastructural changes PR: don't merge - HAS MERGE CONFLICTS labels Nov 4, 2019

Address comments

9bc118e

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Nov 4, 2019

vojtechjelinek added 2 commits November 5, 2019 17:11

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

819d472

…-models7

Merge branch 'wipeout-models7' of https://github.com/vojtechjelinek/o…

6c0f13f

…ppia into wipeout-models7

ankita240796 merged commit 7261d67 into oppia:develop Nov 6, 2019

vojtechjelinek deleted the wipeout-models7 branch March 12, 2020 05:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wipeout 2.8: Add has_reference_to_user_id to remainig models #7893

Wipeout 2.8: Add has_reference_to_user_id to remainig models #7893

vojtechjelinek commented Nov 1, 2019

oppiabot bot commented Nov 1, 2019

seanlip commented Nov 1, 2019

codecov bot commented Nov 3, 2019 •

edited

Loading

oppiabot bot commented Nov 3, 2019

seanlip left a comment

seanlip Nov 4, 2019

seanlip Nov 4, 2019

seanlip Nov 4, 2019

seanlip commented Nov 4, 2019

ankita240796 left a comment

aks681 left a comment

nithusha21 left a comment

Wipeout 2.8: Add has_reference_to_user_id to remainig models #7893

Wipeout 2.8: Add has_reference_to_user_id to remainig models #7893

Conversation

vojtechjelinek commented Nov 1, 2019

Explanation

Checklist

oppiabot bot commented Nov 1, 2019

seanlip commented Nov 1, 2019

codecov bot commented Nov 3, 2019 • edited Loading

Codecov Report

oppiabot bot commented Nov 3, 2019

seanlip left a comment

Choose a reason for hiding this comment

seanlip Nov 4, 2019

Choose a reason for hiding this comment

seanlip Nov 4, 2019

Choose a reason for hiding this comment

seanlip Nov 4, 2019

Choose a reason for hiding this comment

seanlip commented Nov 4, 2019

ankita240796 left a comment

Choose a reason for hiding this comment

aks681 left a comment

Choose a reason for hiding this comment

nithusha21 left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 3, 2019 •

edited

Loading