Fix oppia#2553: Change the "index all explorations" job in the admin …

…dashb… (oppia#3831) * Fix oppia#2553: Change the "index all explorations" job in the admin dashboard to an "index all activities" job * Address review comments * Address review comments * Update docstring to correct parameter name
shashaBot · Sep 5, 2017 · dd4d7c3 · dd4d7c3
1 parent abe5541
commit dd4d7c3
Show file tree

Hide file tree

Showing 18 changed files with 801 additions and 589 deletions.
diff --git a/core/controllers/collection_editor.py b/core/controllers/collection_editor.py
@@ -20,6 +20,7 @@
 from core.domain import acl_decorators
 from core.domain import collection_services
 from core.domain import rights_manager
+from core.domain import search_services
 from core.domain import summary_services
 from core.platform import models
 import feconf
@@ -203,7 +204,7 @@ def put(self, collection_id):
         _require_valid_version(version, collection.version)
 
         rights_manager.unpublish_collection(self.user, collection_id)
-        collection_services.delete_documents_from_search_index([
+        search_services.delete_collections_from_search_index([
             collection_id])
 
         collection_rights = rights_manager.get_collection_rights(

diff --git a/core/controllers/cron.py b/core/controllers/cron.py
@@ -21,8 +21,8 @@
 from core import jobs
 from core.controllers import base
 from core.domain import acl_decorators
+from core.domain import activity_jobs_one_off
 from core.domain import email_manager
-from core.domain import exp_jobs_one_off
 from core.domain import recommendations_jobs_one_off
 from core.domain import user_jobs_one_off
 from core.platform import models
@@ -96,14 +96,14 @@ def get(self):
         job_class.enqueue(job_class.create_new())
 
 
-class CronExplorationSearchRankHandler(base.BaseHandler):
-    """Handler for computing exploration search ranks."""
+class CronActivitySearchRankHandler(base.BaseHandler):
+    """Handler for computing activity search ranks."""
 
     @acl_decorators.can_perform_cron_tasks
     def get(self):
         """Handles GET requests."""
-        exp_jobs_one_off.IndexAllExplorationsJobManager.enqueue(
-            exp_jobs_one_off.IndexAllExplorationsJobManager.create_new())
+        activity_jobs_one_off.IndexAllActivitiesJobManager.enqueue(
+            activity_jobs_one_off.IndexAllActivitiesJobManager.create_new())
 
 
 class CronMapreduceCleanupHandler(base.BaseHandler):

diff --git a/core/controllers/editor.py b/core/controllers/editor.py
@@ -38,6 +38,7 @@
 from core.domain import obj_services
 from core.domain import rights_manager
 from core.domain import rte_component_registry
+from core.domain import search_services
 from core.domain import stats_services
 from core.domain import user_services
 from core.domain import value_generators_domain
@@ -409,7 +410,7 @@ def put(self, exploration_id):
         # Perform the moderator action.
         if action == 'unpublish_exploration':
             rights_manager.unpublish_exploration(self.user, exploration_id)
-            exp_services.delete_documents_from_search_index([
+            search_services.delete_explorations_from_search_index([
                 exploration_id])
         else:
             raise self.InvalidInputException(

diff --git a/core/domain/activity_jobs_one_off.py b/core/domain/activity_jobs_one_off.py
@@ -0,0 +1,47 @@
+# coding: utf-8
+#
+# Copyright 2017 The Oppia Authors. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS-IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""One-off jobs for activities."""
+
+from core import jobs
+from core.domain import search_services
+from core.platform import models
+
+(exp_models, collection_models,) = models.Registry.import_models([
+    models.NAMES.exploration, models.NAMES.collection])
+
+
+class IndexAllActivitiesJobManager(jobs.BaseMapReduceOneOffJobManager):
+    """Job that indexes all explorations and collections and compute their
+    ranks.
+    """
+
+    @classmethod
+    def entity_classes_to_map_over(cls):
+        return [exp_models.ExpSummaryModel,
+                collection_models.CollectionSummaryModel]
+
+    @staticmethod
+    def map(item):
+        if not item.deleted:
+            if isinstance(item, exp_models.ExpSummaryModel):
+                search_services.index_exploration_summaries([item])
+            else:
+                search_services.index_collection_summaries([item])
+
+    @staticmethod
+    def reduce(key, values):
+        pass
diff --git a/core/domain/activity_jobs_one_off_test.py b/core/domain/activity_jobs_one_off_test.py
@@ -0,0 +1,91 @@
+# coding: utf-8
+#
+# Copyright 2017 The Oppia Authors. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS-IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from core.domain import activity_jobs_one_off
+from core.domain import collection_services
+from core.domain import collection_domain
+from core.domain import exp_domain
+from core.domain import exp_services
+from core.domain import rights_manager
+from core.domain import search_services
+from core.domain import user_services
+from core.platform import models
+from core.platform.taskqueue import gae_taskqueue_services as taskqueue_services
+from core.tests import test_utils
+
+gae_search_services = models.Registry.import_search_services()
+
+class OneOffReindexActivitiesJobTest(test_utils.GenericTestBase):
+
+    def setUp(self):
+        super(OneOffReindexActivitiesJobTest, self).setUp()
+
+        self.signup(self.OWNER_EMAIL, self.OWNER_USERNAME)
+        self.owner_id = self.get_user_id_from_email(self.OWNER_EMAIL)
+        self.owner = user_services.UserActionsInfo(self.owner_id)
+
+        explorations = [exp_domain.Exploration.create_default_exploration(
+            '%s' % i,
+            title='title %d' % i,
+            category='category%d' % i
+        ) for i in xrange(3)]
+
+        for exp in explorations:
+            exp_services.save_new_exploration(self.owner_id, exp)
+            rights_manager.publish_exploration(self.owner, exp.id)
+
+        collections = [collection_domain.Collection.create_default_collection(
+            '%s' % i,
+            title='title %d' % i,
+            category='category%d' % i
+        ) for i in xrange(3, 6)]
+
+        for collection in collections:
+            collection_services.save_new_collection(self.owner_id, collection)
+            rights_manager.publish_collection(self.owner, collection.id)
+
+        self.process_and_flush_pending_tasks()
+
+    def test_standard_operation(self):
+        job_id = (
+            activity_jobs_one_off.IndexAllActivitiesJobManager.create_new())
+        activity_jobs_one_off.IndexAllActivitiesJobManager.enqueue(job_id)
+
+        self.assertEqual(
+            self.count_jobs_in_taskqueue(
+                taskqueue_services.QUEUE_NAME_ONE_OFF_JOBS), 1)
+
+        indexed_docs = []
+
+        def add_docs_mock(docs, index):
+            indexed_docs.extend(docs)
+            self.assertIn(index, (search_services.SEARCH_INDEX_EXPLORATIONS,
+                                  search_services.SEARCH_INDEX_COLLECTIONS))
+
+        add_docs_swap = self.swap(
+            gae_search_services, 'add_documents_to_index', add_docs_mock)
+
+        with add_docs_swap:
+            self.process_and_flush_pending_tasks()
+
+        ids = [doc['id'] for doc in indexed_docs]
+        titles = [doc['title'] for doc in indexed_docs]
+        categories = [doc['category'] for doc in indexed_docs]
+
+        for index in xrange(5):
+            self.assertIn("%s" % index, ids)
+            self.assertIn('title %d' % index, titles)
+            self.assertIn('category%d' % index, categories)
diff --git a/core/domain/collection_jobs_one_off_test.py b/core/domain/collection_jobs_one_off_test.py
@@ -131,6 +131,13 @@ def test_migrate_colections_failing_strict_validation(self):
             'category': collection_category,
         }])
 
+        # Save a collection summary object for indexing. The explicit commit
+        # does not create a summary object, which is needed for the
+        # job to update the index after updating the collection.
+        collection_summary = collection_services.compute_summary_of_collection(
+            model, self.albert_id)
+        collection_services.save_collection_summary(collection_summary)
+
         # Start migration job on sample collection.
         job_id = (
             collection_jobs_one_off.CollectionMigrationJob.create_new())
@@ -178,6 +185,13 @@ def test_migration_job_migrates_collection_nodes(self):
             'category': collection_category,
         }])
 
+        # Save a collection summary object for indexing. The explicit commit
+        # does not create a summary object, which is needed for the
+        # job to update the index after updating the collection.
+        collection_summary = collection_services.compute_summary_of_collection(
+            model, self.albert_id)
+        collection_services.save_collection_summary(collection_summary)
+
         # Check that collection_contents is empty
         self.assertEqual(model.collection_contents, {})