Moved test to base_test

oppia · seanlip · Nov 28, 2018 · Sep 13, 2018 · Nov 3, 2018 · Nov 10, 2018
commit dc28ba352f2b604178ddc3c568bb29847ee39245
@@ -322,15 +322,14 @@ def get(self):
         })
 
 
-class AdminTopicsCsvDownloadHandler(base.BaseHandler):
+class AdminTopicsCsvDownloadFileDownloader(base.BaseHandler):
     """Retrieves topic similarity data for download."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'downloadable'
+
     @acl_decorators.can_access_admin_page
     def get(self):
-        self.response.headers['Content-Type'] = 'text/csv'
-        self.response.headers['Content-Disposition'] = (
-            'attachment; filename=topic_similarities.csv')
-        self.response.write(
+        self.render_downloadable_file(
             recommendations_services.get_topic_similarities_as_csv())
 
 

@@ -281,6 +281,13 @@ def render_json(self, values):
         json_output = json.dumps(values, cls=utils.JSONEncoderForHTML)
         self.response.write('%s%s' % (feconf.XSSI_PREFIX, json_output))
 
+    def render_downloadable_file(self, values):
+        """Prepares downloadable content to be sent to the client."""
+        self.response.headers['Content-Type'] = 'text/csv'
+        self.response.headers['Content-Disposition'] = (
+            'attachment; filename=topic_similarities.csv')
+        self.response.write(values)
+
     def _get_logout_url(self, redirect_url_on_logout):
         """Prepares and returns logout url which will be handled
         by LogoutPage handler.

@@ -17,6 +17,8 @@
 """Tests for generic controller behavior."""
 
 import datetime
+import importlib
+import inspect
 import json
 import os
 import re
@@ -589,3 +591,64 @@ def test_get_items(self):
                 'param2=value%20with%20%26%20%2B%20-%20/&'
                 'param3=value%20with%20.%20%%20@%20123%20=%20!%20%3C%3E')
             self.assertDictContainsSubset(params, result)
+
+
+class ControllerClassNameTest(test_utils.GenericTestBase):
+
+    def test_controller_class_names(self):
+        """This function checks that all controller class names end with
+        either 'Handler' or 'Page'.
+        """
+        for url in main.URLS:
+            if not isinstance(url, tuple):
+                handler = str(url.handler)[8:-2].rsplit('.', 1)[1]
+                class_path = str(url.handler)[8:-2].rsplit('.', 1)[0]
+                file_name = class_path.replace('.', '/') + ".py"
+                module = importlib.import_module(class_path)
+                clazz = getattr(module, handler)
+                all_base_classes = [base_class.__name__ for base_class in
+                                    (inspect.getmro(clazz))]
+                if 'BaseHandler' in all_base_classes:
+                    class_return_type = clazz.GET_HANDLER_ERROR_RETURN_TYPE
+                    # Check that the name of the class ends with 'Handler'
+                    # if it renders JSON.
+                    if class_return_type == 'json' or (
+                            not 'get' in clazz.__dict__.keys()):
+                        message = (
+                            'Please ensure that the name of this class '
+                            'ends with \'Handler\'')
+                        error_message = (
+                            '%s --> Line %s: %s'
+                            % (file_name, inspect.getsourcelines(clazz)[-1],
+                               message))
+                        self.assertTrue(handler.endswith('Handler'),
+                                        msg=error_message)
+
+                    # Check that the name of the class ends with 'Page' if
+                    # it has a get function that renders a HTML page.
+                    elif class_return_type == 'html' and (
+                            'get' in clazz.__dict__.keys()):
+                        message = (
+                            'Please ensure that the name of this class '
+                            'ends with \'Page\'')
+                        error_message = (
+                            '%s --> Line %s: %s'
+                            % (file_name, inspect.getsourcelines(clazz)[-1],
+                               message))
+                        self.assertTrue(handler.endswith('Page'),
+                                        msg=error_message)
+
+                    # Check that the name of the class ends with
+                    # 'FileDownloader' if it has a get function
+                    # that prepares downloadable content.
+                    elif class_return_type == 'downloadable' and (
+                            'get' in clazz.__dict__.keys()):
+                        message = (
+                            'Please ensure that the name of this class '
+                            'ends with \'FileDownloader\'')
+                        error_message = (
+                            '%s --> Line %s: %s'
+                            % (file_name, inspect.getsourcelines(clazz)[-1],
+                               message))
+                        self.assertTrue(handler.endswith('FileDownloader'),
+                                        msg=error_message)
@@ -75,6 +75,8 @@ def get(self, collection_id):
 class EditableCollectionDataHandler(CollectionEditorHandler):
     """A data handler for collections which supports writing."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     def _require_valid_version(self, version_from_payload, collection_version):
         """Check that the payload version matches the given collection
         version.
@@ -141,6 +143,8 @@ def put(self, collection_id):
 class CollectionRightsHandler(CollectionEditorHandler):
     """Handles management of collection editing rights."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_edit_collection
     def get(self, collection_id):
         """Gets the editing rights for the given collection.
@@ -234,6 +238,8 @@ def put(self, collection_id):
 class ExplorationMetadataSearchHandler(base.BaseHandler):
     """Provides data for exploration search."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.open_access
     def get(self):
         """Handles GET requests."""

@@ -23,6 +23,8 @@
 class ConceptCardDataHandler(base.BaseHandler):
     """A card that shows the explanation of a skill's concept."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_view_skill
     def get(self, skill_id):
         """Handles GET requests."""

@@ -87,6 +87,8 @@ def _require_valid_version(version_from_payload, exploration_version):
 class EditorLogoutHandler(base.BaseHandler):
     """Handles logout from editor page."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.open_access
     def get(self):
         """Checks if exploration is published and redirects accordingly."""
@@ -631,6 +633,7 @@ class FetchIssuesHandler(EditorHandler):
     exploration. This removes the invalid issues and returns the remaining
     unresolved ones.
     """
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
 
     @acl_decorators.can_view_exploration_stats
     def get(self, exp_id):
@@ -652,6 +655,8 @@ def get(self, exp_id):
 class FetchPlaythroughHandler(EditorHandler):
     """Handler used for retrieving a playthrough."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_view_exploration_stats
     def get(self, unused_exploration_id, playthrough_id):
         """Handles GET requests."""
@@ -823,6 +828,8 @@ def post(self, exploration_id):
 class StateAnswerStatisticsHandler(EditorHandler):
     """Returns basic learner answer statistics for a state."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_view_exploration_stats
     def get(self, exploration_id):
         """Handles GET requests."""
@@ -841,6 +848,8 @@ def get(self, exploration_id):
 class TopUnresolvedAnswersHandler(EditorHandler):
     """Returns a list of top N unresolved answers."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_edit_exploration
     def get(self, exploration_id):
         """Handles GET requests for unresolved answers."""

@@ -40,6 +40,8 @@ def get(self):
 class EmailDashboardDataHandler(base.BaseHandler):
     """Query data handler."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_manage_email_dashboard
     def get(self):
         cursor = self.request.get('cursor')
@@ -126,6 +128,8 @@ def _validate(self, data):
 class QueryStatusCheck(base.BaseHandler):
     """Handler for checking status of individual queries."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_manage_email_dashboard
     def get(self):
         query_id = self.request.get('query_id')

@@ -62,6 +62,9 @@ def post(self, exploration_id):
 
 class ThreadListHandlerForTopics(base.BaseHandler):
     """Handles listing of suggestions threads linked to topics."""
+
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_edit_topic
     def get(self, topic_id):
         if not constants.ENABLE_NEW_STRUCTURES:

@@ -128,6 +128,7 @@ class LearnerDashboardIdsHandler(base.BaseHandler):
     the activities currently being pursued, and the activities present in
     the playlist.
     """
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
 
     @acl_decorators.can_access_learner_dashboard
     def get(self):
@@ -146,6 +147,8 @@ def get(self):
 class LearnerDashboardFeedbackThreadHandler(base.BaseHandler):
     """Gets all the messages in a thread."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_access_learner_dashboard
     def get(self, thread_id):
         """Handles GET requests."""

@@ -178,6 +178,8 @@ def get(self):
 class LibraryGroupIndexHandler(base.BaseHandler):
     """Provides data for categories such as top rated and recently published."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.open_access
     def get(self):
         """Handles GET requests for group pages."""

@@ -96,6 +96,8 @@ def delete(self, question_id, skill_id):
 class EditableQuestionDataHandler(base.BaseHandler):
     """A data handler for questions which supports writing."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_view_question_editor
     def get(self, question_id):
         """Gets the data for the question overview page."""

@@ -313,6 +313,8 @@ def get(self, exploration_id):
 class PretestHandler(base.BaseHandler):
     """Provides subsequent pretest questions after initial batch."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_play_exploration
     def get(self, exploration_id):
         """Handles GET request."""

@@ -27,6 +27,8 @@
 class ValueGeneratorHandler(base.BaseHandler):
     """Retrieves the HTML template for a value generator editor."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.open_access
     def get(self, generator_id):
         """Handles GET requests."""
@@ -47,6 +49,8 @@ class ImageDevHandler(base.BaseHandler):
 
     _IMAGE_PATH_PREFIX = 'image'
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.open_access
     def get(self, exploration_id, encoded_filename):
         """Returns an image.
@@ -85,6 +89,7 @@ class AudioDevHandler(base.BaseHandler):
     """
 
     _AUDIO_PATH_PREFIX = 'audio'
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
 
     @acl_decorators.open_access
     def get(self, exploration_id, encoded_filename):

@@ -95,6 +95,8 @@ def check_can_edit_skill_description(user):
 class SkillRightsHandler(base.BaseHandler):
     """A handler for returning skill rights."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_edit_skill
     def get(self, skill_id):
         """Returns the SkillRights object of a skill."""
@@ -118,6 +120,8 @@ def get(self, skill_id):
 class EditableSkillDataHandler(base.BaseHandler):
     """A data handler for skills which supports writing."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_edit_skill
     def get(self, skill_id):
         """Populates the data on the individual skill page."""

@@ -59,6 +59,8 @@ def get(self, topic_id, story_id):
 class EditableStoryDataHandler(base.BaseHandler):
     """A data handler for stories which support writing."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     def _require_valid_version(self, version_from_payload, story_version):
         """Check that the payload version matches the given story
         version.

@@ -152,6 +152,8 @@ def put(self, target_id, suggestion_id):
 class SuggestionListHandler(base.BaseHandler):
     """Handles list operations on suggestions."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.open_access
     def get(self):
         # The query_fields_and_values variable is a list of tuples. The first

@@ -42,6 +42,7 @@ class TopicEditorStoryHandler(base.BaseHandler):
     """Manages the creation of a story and receiving of all story summaries for
     display in topic editor page.
     """
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
 
     @acl_decorators.can_view_any_topic_editor
     def get(self, topic_id):
@@ -97,6 +98,7 @@ class TopicEditorQuestionHandler(base.BaseHandler):
     """Manages the creation of a question and receiving of all question
     summaries for display in topic editor page.
     """
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
 
     @acl_decorators.can_view_any_topic_editor
     def get(self, topic_id):
@@ -177,6 +179,8 @@ def get(self, topic_id):
 class EditableSubtopicPageDataHandler(base.BaseHandler):
     """The data handler for subtopic pages."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     def _require_valid_version(
             self, version_from_payload, subtopic_page_version):
         """Check that the payload version matches the given subtopic page
@@ -218,6 +222,8 @@ def get(self, topic_id, subtopic_id):
 class EditableTopicDataHandler(base.BaseHandler):
     """A data handler for topics which supports writing."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     def _require_valid_version(self, version_from_payload, topic_version):
         """Check that the payload version matches the given topic
         version.
@@ -329,6 +335,8 @@ def delete(self, topic_id):
 class TopicRightsHandler(base.BaseHandler):
     """A handler for returning topic rights."""
 
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
+
     @acl_decorators.can_view_any_topic_editor
     def get(self, topic_id):
         """Returns the TopicRights object of a topic."""

@@ -43,6 +43,7 @@ class TopicPageDataHandler(base.BaseHandler):
     """Manages the data that needs to be displayed to a learner on the topic
     viewer page.
     """
+    GET_HANDLER_ERROR_RETURN_TYPE = 'json'
 
     @acl_decorators.can_access_topic_viewer_page
     def get(self, topic_name):

diff --git a/feconf.py b/feconf.py
@@ -916,6 +916,7 @@ def get_empty_ratings():
 # The type of the response returned by a handler when an exception is raised.
 HANDLER_TYPE_HTML = 'html'
 HANDLER_TYPE_JSON = 'json'
+HANDLER_TYPE_DOWNLOADABLE = 'downloadable'
 
 # Following are the constants for the role IDs.
 ROLE_ID_GUEST = 'GUEST'