Address review comments.

lunrong · Apr 23, 2015 · 790b9a0 · 790b9a0
1 parent 70374c5
commit 790b9a0
Show file tree

Hide file tree

Showing 9 changed files with 250 additions and 202 deletions.
diff --git a/core/controllers/home.py b/core/controllers/home.py
@@ -130,19 +130,17 @@ def get(self):
             exp_services.get_at_least_editable_exploration_summaries(
                 self.user_id))
 
-        threads = {}
-        for exp_summary in editable_exp_summaries.values():
-            threads[exp_summary.id] = feedback_services.get_thread_count(
-                exp_summary.id)
-
         def _get_intro_card_color(category):
             return (
                 feconf.CATEGORIES_TO_COLORS[category] if
                 category in feconf.CATEGORIES_TO_COLORS else
                 feconf.DEFAULT_COLOR)
 
-        self.values.update({
-            'explorations_list': [{
+        explorations_list = []
+        for exp_summary in editable_exp_summaries.values():
+            feedback_thread_analytics = feedback_services.get_thread_analytics(
+                exp_summary.id)
+            explorations_list.append({
                 'id': exp_summary.id,
                 'title': exp_summary.title,
                 'category': exp_summary.category,
@@ -159,12 +157,19 @@ def _get_intro_card_color(category):
                     '/images/gallery/exploration_background_%s_small.png' %
                     _get_intro_card_color(exp_summary.category)),
                 'ratings': exp_summary.ratings,
-            } for exp_summary in editable_exp_summaries.values()],
-            'thread_count_list': [{
-                'id': exp_id,
-                'open_threads': threads[exp_id][0] if threads[exp_id] else 0,
-                'total_threads': threads[exp_id][1] if threads[exp_id] else 0,
-            } for exp_id in threads.keys()],
+                'num_open_threads': (
+                    feedback_thread_analytics['num_open_threads']),
+                'num_total_threads': (
+                    feedback_thread_analytics['num_total_threads']),
+            })
+
+        explorations_list = sorted(
+            explorations_list,
+            key=lambda x: (x['num_open_threads'], x['last_updated']),
+            reverse=True)
+
+        self.values.update({
+            'explorations_list': explorations_list,
         })
         self.render_json(self.values)
 

diff --git a/core/domain/feedback_jobs.py b/core/domain/feedback_jobs.py
@@ -22,84 +22,94 @@
     models.NAMES.base_model, models.NAMES.feedback, models.NAMES.exploration
 ])
 transaction_services = models.Registry.import_transaction_services()
-import feconf
-import utils
 
-from google.appengine.ext import ndb
 
-
-class OpenFeedbacksRealtimeModel(
+class FeedbackAnalyticsRealtimeModel(
         jobs.BaseRealtimeDatastoreClassForContinuousComputations):
     pass
 
 
-class OpenFeedbacksStatisticsAggregator(jobs.BaseContinuousComputationManager):
-    """A continuous-computation job that computes the number of open feedbacks
-    for explorations.
+class FeedbackAnalyticsAggregator(jobs.BaseContinuousComputationManager):
+    """A continuous-computation job that computes analytics for feedback
+    threads of explorations.
 
     This job does not have a realtime component. There will be a delay in
     propagating new updates to the dashboard; the length of the delay will be
     approximately the time it takes a batch job to run.
     """
+    # TODO(sll): Add a realtime component: listen to incoming feedback messages
+    # and adjust the counts appropriately.
+
     @classmethod
     def get_event_types_listened_to(cls):
         return []
 
     @classmethod
     def _get_realtime_datastore_class(cls):
-        return OpenFeedbacksRealtimeModel
+        return FeedbackAnalyticsRealtimeModel
 
     @classmethod
     def _get_batch_job_manager_class(cls):
-        return OpenFeedbacksMRJobManager
+        return FeedbackAnalyticsMRJobManager
 
     @classmethod
     def _handle_incoming_event(cls, active_realtime_layer, event_type, *args):
         pass
 
     # Public query methods.
     @classmethod
-    def get_thread_count(cls, exploration_id):
+    def get_thread_analytics(cls, exploration_id):
         """
         Args:
-          - exploration_id: id of the exploration to get statistics for
+          - exploration_id: id of the exploration to get statistics for.
 
-        Returns the number of open feedbacks.
+        Returns a dict with two keys: 'num_open_threads' and
+        'num_total_threads', representing the counts of open and all feedback
+        threads, respectively.
         """
+        feedback_thread_analytics_model = (
+            feedback_models.FeedbackAnalyticsModel.get(
+                exploration_id, strict=False))
+
+        num_open_threads = 0
+        num_total_threads = 0
+        if feedback_thread_analytics_model:
+            num_open_threads = feedback_thread_analytics_model.num_open_threads
+            num_total_threads = (
+                feedback_thread_analytics_model.num_total_threads)
 
-        feedback_thread_analytics_model = feedback_models.OpenFeedbacksModel.get(
-            exploration_id, strict=False)
-        return ([feedback_thread_analytics_model.num_of_open_threads, 
-                 feedback_thread_analytics_model.total_threads]
-            if feedback_thread_analytics_model else None)
+        return {
+            'num_open_threads': num_open_threads,
+            'num_total_threads': num_total_threads
+        }
 
 
-class OpenFeedbacksMRJobManager(
+class FeedbackAnalyticsMRJobManager(
         jobs.BaseMapReduceJobManagerForContinuousComputations):
-    """Job that creates OpenFeedbackModels for explorations by calculating the
-       number of open feedback threads per exploration.
-       Includes:
-       * number of open feedbacks for an exploration.
+    """Job that creates FeedbackAnalyticsModels for explorations by calculating
+    various analytics for feedback threads corresponding to an exploration.
+
+    Currently, this job calculates the number of open feedback threads, as well
+    as the total feedback thread count for each exploration.
     """
 
     @classmethod
     def _get_continuous_computation_class(cls):
-        return OpenFeedbacksStatisticsAggregator
+        return FeedbackAnalyticsAggregator
 
     @classmethod
     def entity_classes_to_map_over(cls):
         return [feedback_models.FeedbackThreadModel]
 
     @staticmethod
     def map(item):
-        if (item.status == feedback_models.STATUS_CHOICES_OPEN):
-            yield (item.exploration_id, 1)
-        else:
-            yield (item.exploration_id, 0)
+        yield (item.exploration_id, item.status)
 
     @staticmethod
     def reduce(key, stringified_values):
-        open_threads = stringified_values.count('1')
-        total_threads = open_threads + stringified_values.count('0')
-        feedback_models.OpenFeedbacksModel.create(key, open_threads, 
-            total_threads)
+        num_open_threads = stringified_values.count(
+            feedback_models.STATUS_CHOICES_OPEN)
+        num_total_threads = len(stringified_values)
+
+        feedback_models.FeedbackAnalyticsModel.create(
+            key, num_open_threads, num_total_threads)