Generalised Review System Milestone 2: Cron job to automatically acce…

…pt suggestions after a threshold (oppia#5155) * added cron job to accept suggestions weekly * fixed linting * updated last_updated field whenever a message is sent to the thread * linting fixed * review changes * linting fixes and minor change * remove print * review changes
quantumferret · Jul 12, 2018 · bd31875 · bd31875
1 parent 466eb86
commit bd31875
Show file tree

Hide file tree

Showing 10 changed files with 121 additions and 7 deletions.
diff --git a/core/controllers/cron.py b/core/controllers/cron.py
@@ -22,13 +22,16 @@
 from core.domain import activity_jobs_one_off
 from core.domain import email_manager
 from core.domain import recommendations_jobs_one_off
+from core.domain import suggestion_services
 from core.domain import user_jobs_one_off
 from core.platform import models
+import feconf
 import utils
 
 from pipeline import pipeline
 
-(job_models,) = models.Registry.import_models([models.NAMES.job])
+(job_models, suggestion_models) = models.Registry.import_models([
+    models.NAMES.job, models.NAMES.suggestion])
 
 # The default retention time is 2 days.
 MAX_MAPREDUCE_METADATA_RETENTION_MSECS = 2 * 24 * 60 * 60 * 1000
@@ -185,3 +188,19 @@ def get(self):
                     jobs.MAPPER_PARAM_MAX_START_TIME_MSEC: max_start_time_msec
                 })
             logging.warning('Deletion jobs for auxiliary entities kicked off.')
+
+
+class CronAcceptStaleSuggestionsHandler(base.BaseHandler):
+    """Handler to accept suggestions that have no activity on them for
+    THRESHOLD_TIME_BEFORE_ACCEPT time.
+    """
+
+    @acl_decorators.can_perform_cron_tasks
+    def get(self):
+        """Handles get requests."""
+        if feconf.ENABLE_AUTO_ACCEPT_OF_SUGGESTIONS:
+            suggestions = suggestion_services.get_all_stale_suggestions()
+            for suggestion in suggestions:
+                suggestion_services.accept_suggestion(
+                    suggestion, feconf.SUGGESTION_BOT_USER_ID,
+                    suggestion_models.DEFAULT_SUGGESTION_ACCEPT_MESSAGE, None)
diff --git a/core/domain/feedback_services.py b/core/domain/feedback_services.py
@@ -28,8 +28,9 @@
 import feconf
 import utils
 
-(feedback_models, email_models) = models.Registry.import_models(
-    [models.NAMES.feedback, models.NAMES.email])
+(feedback_models, email_models, suggestion_models) = (
+    models.Registry.import_models(
+        [models.NAMES.feedback, models.NAMES.email, models.NAMES.suggestion]))
 datastore_services = models.Registry.import_datastore_services()
 taskqueue_services = models.Registry.import_taskqueue_services()
 transaction_services = models.Registry.import_transaction_services()
@@ -93,7 +94,8 @@ def _create_models_for_thread_and_first_message(
 
 
 def create_thread(
-        exploration_id, state_name, original_author_id, subject, text):
+        exploration_id, state_name, original_author_id, subject, text,
+        has_suggestion=False):
     """Creates a thread and its first message.
 
     Args:
@@ -103,12 +105,15 @@ def create_thread(
         original_author_id: str. The author id who starts this thread.
         subject: str. The subject of this thread.
         text: str. The text of the feedback message. This may be ''.
+        has_suggestion: bool. Whether the thread has a suggestion attached to
+            it.
 
     Returns:
         str. The ID of the newly created thread.
     """
     thread_id = _create_models_for_thread_and_first_message(
-        exploration_id, state_name, original_author_id, subject, text, False)
+        exploration_id, state_name, original_author_id, subject, text,
+        has_suggestion)
     return thread_id
 
 
@@ -178,6 +183,20 @@ def create_message(
     new_status = thread.status
     thread.put()
 
+    # We do a put on the suggestion linked (if it exists) to the thread, so that
+    # the last_updated time changes to show that there is activity in the
+    # thread.
+    if thread.has_suggestion:
+        # TODO (nithesh): remove manual construction of suggestion ID after
+        # migrating feedback threads.
+        suggestion_id = 'exploration.' + thread_id
+        suggestion = suggestion_models.GeneralSuggestionModel.get_by_id(
+            suggestion_id)
+        # As the thread is created before the suggestion, for the first message
+        # we need not update the suggestion.
+        if suggestion:
+            suggestion.put()
+
     if (user_services.is_user_registered(author_id) and
             feconf.CAN_SEND_EMAILS and
             feconf.CAN_SEND_FEEDBACK_MESSAGE_EMAILS):

diff --git a/core/domain/suggestion_services.py b/core/domain/suggestion_services.py
@@ -62,7 +62,7 @@ def create_suggestion(
     if target_type == suggestion_models.TARGET_TYPE_EXPLORATION:
         thread_id = feedback_services.create_thread(
             target_id, None, author_id, description,
-            DEFAULT_SUGGESTION_THREAD_SUBJECT)
+            DEFAULT_SUGGESTION_THREAD_SUBJECT, has_suggestion=True)
         # This line and the if..else will be removed after the feedback thread
         # migration is complete and the IDs for both models match.
         thread_id = suggestion_models.TARGET_TYPE_EXPLORATION + '.' + thread_id
@@ -193,6 +193,19 @@ def get_suggestions_by_target_id(target_type, target_id):
             .get_suggestions_by_target_id(target_type, target_id)]
 
 
+def get_all_stale_suggestions():
+    """Gets a list of suggestions without any activity on them for
+    THRESHOLD_TIME_BEFORE_ACCEPT time.
+
+    Returns:
+        list(Suggestion). A list of suggestions linked to the entity.
+    """
+
+    return [get_suggestion_from_model(s)
+            for s in suggestion_models.GeneralSuggestionModel
+            .get_all_stale_suggestions()]
+
+
 def _update_suggestion(suggestion):
     """Updates the given suggestion.
 

diff --git a/core/domain/user_services.py b/core/domain/user_services.py
@@ -43,7 +43,8 @@
     'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEwAAABMCAYAAADHl1ErAAAAAXNSR0IArs4c6QAADhtJREFUeAHt%0AXHlwVdUZ/859jyxmIQESyCaglC0iAgkJIntrIpvKphSwY2ttxbFOp9R/cGGqdhykLaMVO2OtoyRS%0ACEKNEpYKyBIVQ1iNkBhNMCtb8shiQpJ3b7/fTW7m5uUlecu9L4nTM5Pce8895zvf93vnnPud833f%0AEdQLKXb5jsC6%2BuZERZbHKaSMYRbGKERxgpQQUkSIIigEbAmFavlfrUKiVhCVcFa%2BIJEvJOlCcNCA%0AnNKMFQ0o58vEfPgmhS5Mn0ot8n2KIs8lIZJJUfy8almIJqbxhRDSIbJKe2s%2BXvWlV/RcrGwqYGGp%0A20bI1LyaeVmjKMrodp4EycGBAy6MjgsrSxozqG7O5GgxcVREeEigNDAwwBpmsUiRKGu3y1caGlts%0AtQ3yjbOFV6sPnypXTuRXBReU2GLqGprHkUKSRlMIUcD3WyUakGbbt7JYyzf6agpgYfe9O8kui/U8%0AnB7UhJIkUTljwrBTTz449mZKUlyCEBTnjTCKQiX7T5ScfGP3Rf9j5ysny7IyTKXHPwYP690WSXnZ%0AtvcXp71pw1ldQwELm59%2BlyzbX%2BbeNL%2Btscb4EYOyNz2ZWD99wtAFnGdxxoQBefbs85f3rHsjJyiv%0AuGo60wsATe51WZJkWW/LWnXGgDZUEoYAFr58x0B7beOLPHGv5XnFIpGoS0mKOfze%2Bpmj/f2smNR9%0Alm42teQ/8vLRgv0nyuZwVwtm1Ows5BZLSMBz1RkrbnjLiNeAhaWmPWgn%2BxYeejwkRMu9idH7tm%2BY%0AE8/z0EhvmfOmPs9/RQ9tOJx3IKc8lUixkqBKC1nW2vat3u0NXY8Bi1%2B%2Bw6%2BktnETD7%2BnwEB4iP/p%0AL/5xf03U4IBZ3jBkdN2K641Hkn/7YWh17c1JoM3D9PW4kIB1eRkrmjxpyyPAeK4aLttbPuAhOIU5%0AaHpm1cTMZ1ffuRT8eMKED%2BooL6Wd%2B2Bj%2BtnFUGeYyVzJYl3Kc9sld9t2W8Dw%2BWkTWuz2fdxQ9ACr%0A9P3Jfy7%2BZuSw0HnuNtwb5Ysqaw4mPJb5k%2BYW%2BVZuv9xqsaRWZ60%2B7w4vbgEWnrJ1hp3kTO5ZYUPC%0AAnK%2B3bYiitWDWHca7O2yrI6U3r5yR8U1W2MiC2%2BzkLS4ev%2BaY67y1a749VQBYLUIZT/AGhUTduS7%0Af68Y39/AgozgGbxDBsgCmSBbT/Jr710CDMMQPYvHf2DC2Mj9p95efA8TCNKI9MNrEGSALJAJskFG%0AV%2BTocUhigrfbWz5jYtH4VdrAMksBdYVnI8vYJ/8q83hhmW0WEy23WKx39/Qh6LaHQXXA1xBgYc5i%0AsBL4/scCFoC3QCbIBhkhK2TGi65St4CpeharDvgaYoJnIv15GHaFQRBkg4w8p02BzF0VRH6XgEGD%0AV5VS1rOgOvTHCb47wfXvIBtkhE4JmSG7/r3%2B3ilg6toQyx1OUEr7i56lF8zde8gIWVEPSz1g4IyG%0AU8CwkMbaEMudNg3eWd0fXR5khcyQXcXAiYSdAMMWDY/ltVhIY23IdXr8kjqh21%2BzRKvMogUYAAtH%0AQToBhv0sbNFg16GvLaQdmTfjGTJDdmCgYuHQSIfe07pTSqewn3V9z6qrvb1F48Crzx6xNTR4QXoE%0A9tN4c2%2ByfufWqudC3VbmAYzNPwZrkf6dL%2B4LSm5Q9vkrVH79B6qs%2BoH8B1goatAtNCIqmOZOiabw%0A4G5VJMNYREdhDD7ae6J0USsmtEwj3t7DYLCwK83f8WbbzauZP7/kq53SxiY7vfmfC5R24Fv6prTr%0ADVEWgqbfEUlPLY2nlKkxGv%2BmXbFzG7H4/eE8g/tZyO92zbDSPoe1WncUgT14X4G189Nimvjobnrh%0AX6e6BQuo8DCho2crafnzB2n%2BMwe4PL5H5iVgACx4wEltli%2B1sXbA%2BGkNcmCwUN%2BY%2BI%2B3WOjZt3Lp%0Al68cpQoefu6m4%2Bcqae7TWfTfk%2BXuVnWrvA4LFRtUVockjKxKc8sJmMJsWWsiON/U9eJvNmXTtk%2B%2B%0AdYt5Z4WZX0p/bjYtmBbn7LURefaw%2BVuvwoQnBliTYCxu7WFskQb1WROjcvliKlibM/IMAQv8siD0%0A643H6etiGx7NSBbYUlXCbRipgKnme859Ysl4jwwDrnKaV2SjDe%2B0tu9qnZ7KsQWch/YxVpt6KunZ%0AexieUVPDSIJjCC86k3lwyikJ0di%2BMS09/3au2iuMbuDr4mpKN2CIO%2BMLVnpgA4yAlVRX1ziV4fOD%0ArwOv2k2bDM4UVvEkXeaMJ0PyXn3/nCF0HIkAE2ADjICVpChiLArBMcSxsJHPmdmXjCTXiVZRRS19%0AVVTdKd%2BIDA0bYCW1%2BWcRvGiMIN4Vjb1flHb1yrD8rM9LDKOlJ6RhA6ww6au%2BD3A50hcy%2Bt5sRRP8%0AFpSYo8zqsBnDPax13oJ/ltEgafSqam5SU7NdezTtWsHrTzOShg2wYtWP3SQ5wZnNjMZA80Z9s1mk%0AO9CtMakdDRtgJcGnFK3C869D6wY%2BRISp7loGUnROKtKkdtqxYawkzQGXdwNUN0nnrHiXGxxoJf40%0Ae0fEhdpRg29xoZT7RTRsgJV%2B8e0%2BJTdqJIwd4kZpz4pOGWN%2BG5Lq2s38wQHXMzZdq2XiAlllgP2%2B%0AaH6yOX4xGjbAinejlVq0CG9l10T3rNT99wwnf96KMyvNuHMoDR0UaAr5dmwYK1YrhAoYXLtNaa2N%0A6DAW5vFF6qLClGZeeHSyKXRBVMMGWLFaoUZYEPzgTWuxjfC6lROI/RgMb2bZ7JGUaOIcqWEDrDDp%0A50MCBA0YLokDQRgx0p%2BdTezH4PDG88dxI8LotaeneU7AhZo6bPK5hwkVMERYuFDX6yLT2JDx99/f%0ATVY2anibYiOCaPuGuayydDB%2BeUu2U30NG2AlCaFcRAmEo3QqaVLGynm30a6X5sHz2uMWksZH0pHX%0AF9CIYeb/zho2CAqTgoMDvoTXCmJ3EI7isQRuVpw9KYqytyykhxk8qASuJoD84mNTKGvjveSLFQQw%0AUeOaGCNE0Flqvs5o8b/9gZ8xwyMmj404NComZJyrzHtbLjTIjxZNv1X9C/S30pXqRrLVdd4lh7Ej%0AOX4oPfHAOHrzD9Np9l1RZMHnygeJ45kOZXxaPJ6byr6WueotdfAjhI73rGdu2ZXnn5oY7QM2OjZx%0Ax8hw%2BvPjCepf2bUfqJz/Llc1qHpb1OBAiosMpoFB5i%2BtOnLV%2BoTgL9ypYYZ8bZ0tOd6QmuUNbCiF%0AMoN9GPM0TCbeXYoZcgvhr48kOyLlVF6AESf1UwV7G88jBbC/ISqsjzDb62wAC9UmydhoAaz6b/tW%0AcIgQul7ntI8woMNCxQZstQOGSFYeqQriDeGI0Ud47jU2gIEae8kmtlZsWllpB6zNO2UXZwcg3rDX%0AOO0jDbdhEIDoXs1zB6y1A4YHhP3iiuBMOJXh3tfJzuZ/qBbfX65nR5UGqmto8TUL2OoqAgZoWMNE%0AY6KTMhOa%2Bt4ehCDfmxjz8c4X5y3UChp5hVk/j63Vpwuu0zdlNVTIrkuFfC1hkOobO%2B//Qw8LD/an%0A26JDaFRsKI2KCWU76kCaOi6CoHYYnZY9d/DjAzllC/lDmFWz75EFevqdFmGIkbbL9hREsiI40yg/%0A11wGhxex9PlXV%2BjEhatUU99ZQdUzpr%2BH08n1mkb1L%2BfiVf0rGs5Lo2nxkXT3HUPZ0S7WawAhsxrF%0Ay6HPwKJDY/zQqYehAPey1%2BDgDxfsSxkPwZPYaTmU7S7BPWDXkWLafayYLlWaaidW2cASK5nBWzJz%0AOD3AG5YebCgqw5dvP4PoXab1Oveu3znK5xQIOPW31DZchL/6M6vv2sn%2B68scK3b1jDlo%2B6Hv6G87%0A8ij/e1M3cbtiQc3HML4vKZbWrbyTpowe3G1Z7SVH7e7cmHZmGXePSmtI4FhnQfVOAQMBNfhdse/C%0AwvzsO/cf6ykapKlZpq0HCmlzxlc%2B6U2akK5c2XJNf3x4At3D29hdJUTrTnz0wxlwOrEIy5Kugum7%0ABAyEtaGJwKVrH63mrSDn0besEdNTmz9XJ%2B6uGOoL%2BbAr/OXJJIoM77jryx%2Bh0iGL0mSENnc1FDX%2B%0AO6gVWqZ2RfQ9I5oLQgj75fxO/q%2BvpJ9TnXTxlevr6cPjlyj5iUx2bb%2BsZ7UesqlgsayQWf/S8b7b%0AHobC3QWYrv3rZ%2BwuXuhIs88/Y4v8vfWz4BvrdoBpj4BBejWE2W4/yupTGMJ%2BD21O/emf3j1t2bTN%0ArYD8PgWkv7/FflvUwE8uFFelMAg2i8Uy05UTBlwCTAWtLUieJ8XA2MiQIxXX6xNYI%2B6XC3Wep%2Br5%0Axz/Jsszij1qDVREprp4s4DJgGmjaMQzcUA5bgaNkRTbH3GxSf5SEVMoxRBUMlrnHMIB//Arounxb%0AjgZZuWWtSzlokmyGkwWv4Bm8QwZ1GLpxZgUYcquHaRLgQ6A/SobJ4IiGpeyc7RE9ja55V/aKEOID%0A5s/3R8loQjkeVsTzwmmeF2oYuFlamT5xFeII/4qh3LMmgR/oWT4/rEgPhONxWEKifUJW4mWikfpy%0Avr5nBbNIkUQeD8BU7lm9fxyWHgDHA9fYQlzHg/0w/6qjuZzqdKwvb/J9PveiAl4Hz%2BE5q%2B8duKYX%0AHjHSjkf6sXkqWyEZK4QFLIQ51iihWrr2CJKCeE6fzm2pax8Grm8e6acHDffth0YSLdF9CCoZvFye%0A55okRU7gIetV1AkPuRJZSCfZUdefezJMYf3v0MhOwHVzLKlQxAWSRJlQlDr%2BzrPcUjjbGwbyBB2m%0ACKH62/K7KwywjWM8b5CQq%2BH9x%2B%2BCSVZiFKH8eI4ldQQOz4jJ/P/Bt86QcSFPPVqZA50Qu4NwFK7i%0A3tHK7HEEJ5reOFr5fwkK97jkk8ywAAAAAElFTkSuQmCC%0A') #pylint: disable=line-too-long
 SYSTEM_USERS = {
     feconf.SYSTEM_COMMITTER_ID: feconf.SYSTEM_COMMITTER_ID,
-    feconf.MIGRATION_BOT_USER_ID: feconf.MIGRATION_BOT_USERNAME
+    feconf.MIGRATION_BOT_USER_ID: feconf.MIGRATION_BOT_USERNAME,
+    feconf.SUGGESTION_BOT_USER_ID: feconf.SUGGESTION_BOT_USERNAME
 }
 
 

diff --git a/core/storage/suggestion/gae_models.py b/core/storage/suggestion/gae_models.py
@@ -14,6 +14,8 @@
 
 """Models for Oppia suggestions."""
 
+import datetime
+
 from core.platform import models
 import feconf
 
@@ -70,6 +72,17 @@
 # The delimiter to be used in score category field.
 SCORE_CATEGORY_DELIMITER = '.'
 
+# Threshold number of days after which suggestion will be accepted.
+THRESHOLD_DAYS_BEFORE_ACCEPT = 7
+
+# Threshold time after which suggestion is considered stale and auto-accepted.
+THRESHOLD_TIME_BEFORE_ACCEPT_IN_MSECS = (
+    THRESHOLD_DAYS_BEFORE_ACCEPT * 24 * 60 * 60 * 1000)
+
+# The default message to be shown when accepting stale suggestions.
+DEFAULT_SUGGESTION_ACCEPT_MESSAGE = ('Automatically accepting suggestion after'
+                                     ' %d days' % THRESHOLD_DAYS_BEFORE_ACCEPT)
+
 
 class GeneralSuggestionModel(base_models.BaseModel):
     """Model to store suggestions made by Oppia users.
@@ -218,3 +231,18 @@ def get_suggestions_by_target_id(cls, target_type, target_id):
         """
         return cls.get_all().filter(cls.target_type == target_type).filter(
             cls.target_id == target_id).fetch(feconf.DEFAULT_QUERY_LIMIT)
+
+    @classmethod
+    def get_all_stale_suggestions(cls):
+        """Gets all suggestions which were last updated before the threshold
+        time.
+
+        Returns:
+            list(SuggestionModel). A list of suggestions that are stale.
+        """
+        threshold_time = (
+            datetime.datetime.utcnow() - datetime.timedelta(
+                0, 0, 0, THRESHOLD_TIME_BEFORE_ACCEPT_IN_MSECS))
+        return cls.get_all().filter(
+            cls.status.IN([STATUS_IN_REVIEW, STATUS_RECEIVED])).filter(
+                cls.last_updated < threshold_time).fetch()
diff --git a/core/storage/suggestion/gae_models_test.py b/core/storage/suggestion/gae_models_test.py
@@ -202,3 +202,17 @@ def test_get_suggestions_by_target_id(self):
                 .get_suggestions_by_target_id(
                     suggestion_models.TARGET_TYPE_EXPLORATION, 'exp_invalid')),
             0)
+
+    def test_get_all_stale_suggestions(self):
+        with self.swap(
+            suggestion_models, 'THRESHOLD_TIME_BEFORE_ACCEPT_IN_MSECS', 0):
+            self.assertEqual(len(
+                suggestion_models.GeneralSuggestionModel
+                .get_all_stale_suggestions()), 1)
+
+        with self.swap(
+            suggestion_models, 'THRESHOLD_TIME_BEFORE_ACCEPT_IN_MSECS',
+            7 * 24 * 60 * 60 * 1000):
+            self.assertEqual(len(
+                suggestion_models.GeneralSuggestionModel
+                .get_all_stale_suggestions()), 0)
diff --git a/cron.yaml b/cron.yaml
@@ -14,3 +14,6 @@ cron:
 - description: weekly exploration search rank computation
   url: /cron/explorations/search_rank
   schedule: every wednesday 9:00
+- description: accept all stale suggestions every week
+  url: /cron/suggestions/accept_stale_suggestions
+  schedule: every thursday 9:00
diff --git a/feconf.py b/feconf.py
@@ -376,6 +376,9 @@ def get_empty_ratings():
 # Disables all the new structures' pages, till they are completed.
 ENABLE_NEW_STRUCTURES = False
 
+# Whether to automatically accept suggestions after a threshold time.
+ENABLE_AUTO_ACCEPT_OF_SUGGESTIONS = False
+
 EMAIL_INTENT_SIGNUP = 'signup'
 EMAIL_INTENT_DAILY_BATCH = 'daily_batch'
 EMAIL_INTENT_EDITOR_ROLE_NOTIFICATION = 'editor_role_notification'
@@ -445,6 +448,11 @@ def get_empty_ratings():
 MIGRATION_BOT_USER_ID = 'OppiaMigrationBot'
 MIGRATION_BOT_USERNAME = 'OppiaMigrationBot'
 
+# User id and username for suggestion bot. This bot will be used to accept
+# suggestions automatically after a threshold time.
+SUGGESTION_BOT_USER_ID = 'OppiaSuggestionBot'
+SUGGESTION_BOT_USERNAME = 'OppiaSuggestionBot'
+
 # Ids and locations of the permitted extensions.
 ALLOWED_RTE_EXTENSIONS = {
     'Collapsible': {

diff --git a/index.yaml b/index.yaml
@@ -112,6 +112,12 @@ indexes:
   - name: exploration_id
   - name: last_updated
 
+- kind: GeneralSuggestionModel
+  properties:
+  - name: deleted
+  - name: status
+  - name: last_updated
+
 - kind: InteractionAnswerSummariesRealtimeModel
   properties:
   - name: realtime_layer

diff --git a/main_cron.py b/main_cron.py
@@ -41,6 +41,9 @@
         cron.CronActivitySearchRankHandler),
     main.get_redirect_route(
         r'/cron/jobs/cleanup', cron.CronMapreduceCleanupHandler),
+    main.get_redirect_route(
+        r'/cron/suggestions/accept_stale_suggestions',
+        cron.CronAcceptStaleSuggestionsHandler),
 ]
 
 app = transaction_services.toplevel_wrapper(  # pylint: disable=invalid-name