Fix part of oppia#1366: Cron job to keep history of dashboard statist…

…ics (oppia#2173) * Cron job for recording creator stats * Fixes * Write getter and tests for weekly dasboard stats * Minor changes to model * Fix initial round of review comments * Fix call to one-off job through cron * Removed extra whiteline * Fix accidental line deletion * Partially addressed review comments * Add sample test for multiple weeks * Add test to check format of datetime string * Add schema version to user dashboard stats * Lint fix * Fix brackets around conditional * Fix continuous job and modify tests accordingly * Address review comments * Create new stats model if not already present * Address review comments * Use get_or_create for model * Reused get_or_create in method
quantumferret · Aug 8, 2016 · 5dcfae6 · 5dcfae6
1 parent b669bfa
commit 5dcfae6
Show file tree

Hide file tree

Showing 10 changed files with 487 additions and 16 deletions.
diff --git a/core/controllers/cron.py b/core/controllers/cron.py
@@ -20,6 +20,7 @@
 
 from core import jobs
 from core.controllers import base
+from core.domain import user_jobs_one_off
 from core.platform import models
 import utils
 
@@ -86,6 +87,16 @@ def get(self):
         email_services.send_mail_to_admin(email_subject, email_message)
 
 
+class CronDashboardStatsHandler(base.BaseHandler):
+    """Handler for appending dashboard stats to a list."""
+
+    @require_cron_or_superadmin
+    def get(self):
+        """Handles GET requests."""
+        user_jobs_one_off.DashboardStatsOneOffJob.enqueue(
+            user_jobs_one_off.DashboardStatsOneOffJob.create_new())
+
+
 class CronMapreduceCleanupHandler(base.BaseHandler):
 
     def get(self):

diff --git a/core/domain/user_jobs_continuous.py b/core/domain/user_jobs_continuous.py
@@ -428,15 +428,11 @@ def reduce(key, stringified_values):
         # Find the average of all average ratings
         ratings = [value['average_rating_for_owned_exp'] for value in values
                    if value.get('average_rating_for_owned_exp')]
+
+        mr_model = user_models.UserStatsModel.get_or_create(key)
+        mr_model.impact_score = user_impact_score
+        mr_model.total_plays = total_plays
         if len(ratings) != 0:
             average_ratings = (sum(ratings) / float(len(ratings)))
-            user_models.UserStatsModel(
-                id=key,
-                impact_score=user_impact_score,
-                total_plays=total_plays,
-                average_ratings=average_ratings).put()
-        else:
-            user_models.UserStatsModel(
-                id=key,
-                impact_score=user_impact_score,
-                total_plays=total_plays).put()
+            mr_model.average_ratings = average_ratings
+        mr_model.put()
diff --git a/core/domain/user_jobs_one_off.py b/core/domain/user_jobs_one_off.py
@@ -39,11 +39,10 @@ def entity_classes_to_map_over(cls):
 
     @staticmethod
     def map(item):
-        if isinstance(item, exp_models.ExplorationSnapshotMetadataModel):
-            yield (item.committer_id, {
-                'exploration_id': item.get_unversioned_instance_id(),
-                'version_string': item.get_version_string(),
-            })
+        yield (item.committer_id, {
+            'exploration_id': item.get_unversioned_instance_id(),
+            'version_string': item.get_version_string(),
+        })
 
     @staticmethod
     def reduce(key, version_and_exp_ids):
@@ -175,6 +174,23 @@ def reduce(key, stringified_values):
                 subscription_services.subscribe_to_collection(key, item['id'])
 
 
+class DashboardStatsOneOffJob(jobs.BaseMapReduceJobManager):
+    """One-off job for populating weekly dashboard stats for all registered
+    users who have a non-None value of UserStatsModel.
+    """
+    @classmethod
+    def entity_classes_to_map_over(cls):
+        return [user_models.UserSettingsModel]
+
+    @staticmethod
+    def map(item):
+        user_services.update_dashboard_stats_log(item.id)
+
+    @staticmethod
+    def reduce(item):
+        pass
+
+
 class UserFirstContributionMsecOneOffJob(jobs.BaseMapReduceJobManager):
     """One-off job that updates first contribution time in milliseconds for
     current users. This job makes the assumption that once an exploration is

diff --git a/core/domain/user_jobs_one_off_test.py b/core/domain/user_jobs_one_off_test.py
@@ -16,16 +16,24 @@
 
 """Tests for user-related one-off computations."""
 
+import datetime
+
 from core.domain import collection_domain
 from core.domain import collection_services
 from core.domain import exp_services
+from core.domain import event_services
 from core.domain import feedback_services
+from core.domain import rating_services
 from core.domain import rights_manager
 from core.domain import subscription_services
 from core.domain import user_jobs_one_off
+from core.domain import user_jobs_continuous_test
 from core.domain import user_services
 from core.platform import models
 from core.tests import test_utils
+
+import feconf
+
 (user_models, feedback_models) = models.Registry.import_models(
     [models.NAMES.user, models.NAMES.feedback])
 taskqueue_services = models.Registry.import_taskqueue_services()
@@ -486,6 +494,205 @@ def test_adding_exploration_to_collection(self):
             user_b_subscriptions_model.collection_ids, [self.COLLECTION_ID_1])
 
 
+class DashboardStatsOneOffJobTests(test_utils.GenericTestBase):
+    """Tests for the one-off dashboard stats job."""
+
+    CURRENT_DATE_AS_STRING = user_services.get_current_date_as_string()
+    DATE_AFTER_ONE_WEEK = (
+        (datetime.datetime.utcnow() + datetime.timedelta(7)).strftime(
+            feconf.DASHBOARD_STATS_DATETIME_STRING_FORMAT))
+
+    USER_SESSION_ID = 'session1'
+
+    EXP_ID_1 = 'exp_id_1'
+    EXP_ID_2 = 'exp_id_2'
+    EXP_VERSION = 1
+
+    def _run_one_off_job(self):
+        """Runs the one-off MapReduce job."""
+        job_id = user_jobs_one_off.DashboardStatsOneOffJob.create_new()
+        user_jobs_one_off.DashboardStatsOneOffJob.enqueue(job_id)
+        self.assertEqual(
+            self.count_jobs_in_taskqueue(
+                queue_name=taskqueue_services.QUEUE_NAME_DEFAULT),
+            1)
+        self.process_and_flush_pending_tasks()
+
+    def setUp(self):
+        super(DashboardStatsOneOffJobTests, self).setUp()
+
+        self.signup(self.OWNER_EMAIL, self.OWNER_USERNAME)
+        self.owner_id = self.get_user_id_from_email(self.OWNER_EMAIL)
+
+    def _mock_get_current_date_as_string(self):
+        return self.CURRENT_DATE_AS_STRING
+
+    def _rate_exploration(self, user_id, exp_id, rating):
+        rating_services.assign_rating_to_exploration(user_id, exp_id, rating)
+
+    def _record_play(self, exp_id, state):
+        event_services.StartExplorationEventHandler.record(
+            exp_id, self.EXP_VERSION, state, self.USER_SESSION_ID, {},
+            feconf.PLAY_TYPE_NORMAL)
+
+    def test_weekly_stats_if_continuous_stats_job_has_not_been_run(self):
+        exploration = self.save_new_valid_exploration(
+            self.EXP_ID_1, self.owner_id)
+        exp_id = exploration.id
+        init_state_name = exploration.init_state_name
+        self._record_play(exp_id, init_state_name)
+        self._rate_exploration('user1', exp_id, 5)
+
+        weekly_stats = user_services.get_weekly_dashboard_stats(self.owner_id)
+        self.assertEqual(weekly_stats, None)
+
+        with self.swap(user_services,
+                       'get_current_date_as_string',
+                       self._mock_get_current_date_as_string):
+            self._run_one_off_job()
+
+        weekly_stats = user_services.get_weekly_dashboard_stats(self.owner_id)
+        self.assertEqual(weekly_stats, [{
+            self._mock_get_current_date_as_string(): {
+                'average_ratings': None,
+                'total_plays': 0
+            }
+        }])
+
+    def test_weekly_stats_if_no_explorations(self):
+        (user_jobs_continuous_test.ModifiedUserStatsAggregator.
+         start_computation())
+        self.process_and_flush_pending_tasks()
+
+        with self.swap(user_services,
+                       'get_current_date_as_string',
+                       self._mock_get_current_date_as_string):
+            self._run_one_off_job()
+
+        weekly_stats = user_services.get_weekly_dashboard_stats(self.owner_id)
+        self.assertEqual(weekly_stats, [{
+            self._mock_get_current_date_as_string(): {
+                'average_ratings': None,
+                'total_plays': 0
+            }
+        }])
+
+    def test_weekly_stats_for_single_exploration(self):
+        exploration = self.save_new_valid_exploration(
+            self.EXP_ID_1, self.owner_id)
+        exp_id = exploration.id
+        init_state_name = exploration.init_state_name
+        self._record_play(exp_id, init_state_name)
+        self._rate_exploration('user1', exp_id, 5)
+
+        (user_jobs_continuous_test.ModifiedUserStatsAggregator.
+         start_computation())
+        self.process_and_flush_pending_tasks()
+
+        with self.swap(user_services,
+                       'get_current_date_as_string',
+                       self._mock_get_current_date_as_string):
+            self._run_one_off_job()
+
+        weekly_stats = user_services.get_weekly_dashboard_stats(self.owner_id)
+        self.assertEqual(weekly_stats, [{
+            self._mock_get_current_date_as_string(): {
+                'average_ratings': 5.0,
+                'total_plays': 1
+            }
+        }])
+
+    def test_weekly_stats_for_multiple_explorations(self):
+        exploration_1 = self.save_new_valid_exploration(
+            self.EXP_ID_1, self.owner_id)
+        exp_id_1 = exploration_1.id
+        exploration_2 = self.save_new_valid_exploration(
+            self.EXP_ID_2, self.owner_id)
+        exp_id_2 = exploration_2.id
+        init_state_name_1 = exploration_1.init_state_name
+        self._record_play(exp_id_1, init_state_name_1)
+        self._rate_exploration('user1', exp_id_1, 5)
+        self._rate_exploration('user2', exp_id_2, 4)
+
+        (user_jobs_continuous_test.ModifiedUserStatsAggregator.
+         start_computation())
+        self.process_and_flush_pending_tasks()
+
+        with self.swap(user_services,
+                       'get_current_date_as_string',
+                       self._mock_get_current_date_as_string):
+            self._run_one_off_job()
+
+        weekly_stats = user_services.get_weekly_dashboard_stats(self.owner_id)
+        self.assertEqual(weekly_stats, [{
+            self._mock_get_current_date_as_string(): {
+                'average_ratings': 4.5,
+                'total_plays': 1
+            }
+        }])
+
+    def test_stats_for_multiple_weeks(self):
+        exploration = self.save_new_valid_exploration(
+            self.EXP_ID_1, self.owner_id)
+        exp_id = exploration.id
+        init_state_name = exploration.init_state_name
+        self._rate_exploration('user1', exp_id, 4)
+        self._record_play(exp_id, init_state_name)
+        self._record_play(exp_id, init_state_name)
+
+        (user_jobs_continuous_test.ModifiedUserStatsAggregator.
+         start_computation())
+        self.process_and_flush_pending_tasks()
+
+        with self.swap(user_services,
+                       'get_current_date_as_string',
+                       self._mock_get_current_date_as_string):
+            self._run_one_off_job()
+
+        weekly_stats = user_services.get_weekly_dashboard_stats(self.owner_id)
+        self.assertEqual(weekly_stats, [{
+            self._mock_get_current_date_as_string(): {
+                'average_ratings': 4.0,
+                'total_plays': 2
+            }
+        }])
+
+        (user_jobs_continuous_test.ModifiedUserStatsAggregator.
+         stop_computation(self.owner_id))
+        self.process_and_flush_pending_tasks()
+
+        self._rate_exploration('user2', exp_id, 2)
+
+        (user_jobs_continuous_test.ModifiedUserStatsAggregator.
+         start_computation())
+        self.process_and_flush_pending_tasks()
+
+        def _mock_get_date_after_one_week():
+            """Returns the date of the next week."""
+            return self.DATE_AFTER_ONE_WEEK
+
+        with self.swap(user_services,
+                       'get_current_date_as_string',
+                       _mock_get_date_after_one_week):
+            self._run_one_off_job()
+
+        weekly_stats = user_services.get_weekly_dashboard_stats(self.owner_id)
+        self.assertEqual(weekly_stats, [
+            {
+                self._mock_get_current_date_as_string(): {
+                    'average_ratings': 4.0,
+                    'total_plays': 2
+                }
+            },
+            {
+                _mock_get_date_after_one_week(): {
+                    'average_ratings': 3.0,
+                    'total_plays': 2
+                }
+            }
+        ])
+
+
 class UserFirstContributionMsecOneOffJobTests(test_utils.GenericTestBase):
 
     EXP_ID = 'test_exp'