Change existing handler classes to properly override request methods (#2342)

* Change methods to properly override

* Update comments_api.py

* Update comments_api.py

api/accounts_api.py

@@ -37,7 +37,7 @@ class AccountsAPI(basehandlers.APIHandler):
   # TODO(jrobbins): do_get
 
   @permissions.require_admin_site
-  def do_post(self):
+  def do_post(self, **kwargs):
     """Process a request to create an account."""
     email = self.get_param('email', required=True)
     is_admin = self.get_bool_param('isAdmin')
@@ -63,8 +63,9 @@ class AccountsAPI(basehandlers.APIHandler):
   # TODO(jrobbins): do_patch
 
   @permissions.require_admin_site
-  def do_delete(self, account_id):
+  def do_delete(self, **kwargs):
     """Process a request to delete the specified account."""
+    account_id = kwargs.get('account_id', None)
     if account_id:
       self.delete_account(account_id)
       return {'message': 'Done'}

api/accounts_api_test.py

@@ -160,7 +160,7 @@ class AccountsAPITest(testing_config.CustomTestCase):
     testing_config.sign_in('admin@example.com', 123567890)
 
     with test_app.test_request_context(self.request_path):
-      actual_json = self.handler.do_delete(self.appuser_id)
+      actual_json = self.handler.do_delete(account_id=self.appuser_id)
     self.assertEqual({'message': 'Done'}, actual_json)
 
     revised_appuser = user_models.AppUser.get_by_id(self.appuser_id)
@@ -172,7 +172,7 @@ class AccountsAPITest(testing_config.CustomTestCase):
 
     with test_app.test_request_context(self.request_path):
       with self.assertRaises(werkzeug.exceptions.Forbidden):
-        self.handler.do_delete(self.appuser_id)
+        self.handler.do_delete(account_id=self.appuser_id)
 
     unrevised_appuser = user_models.AppUser.get_by_id(self.appuser_id)
     self.assertEqual('user@example.com', unrevised_appuser.email)
@@ -183,7 +183,7 @@ class AccountsAPITest(testing_config.CustomTestCase):
 
     with test_app.test_request_context(self.request_path):
       with self.assertRaises(werkzeug.exceptions.BadRequest):
-        self.handler.do_delete(None)
+        self.handler.do_delete()
 
     unrevised_appuser = user_models.AppUser.get_by_id(self.appuser_id)
     self.assertEqual('user@example.com', unrevised_appuser.email)
@@ -195,7 +195,7 @@ class AccountsAPITest(testing_config.CustomTestCase):
 
     with test_app.test_request_context(self.request_path):
       with self.assertRaises(werkzeug.exceptions.NotFound):
-        self.handler.do_delete(self.appuser_id + 1)
+        self.handler.do_delete(account_id=self.appuser_id + 1)
 
     unrevised_appuser = user_models.AppUser.get_by_id(self.appuser_id)
     self.assertEqual('user@example.com', unrevised_appuser.email)

api/approvals_api.py

@@ -37,8 +37,10 @@ class ApprovalsAPI(basehandlers.APIHandler):
   """Users may see the set of approvals on a feature, and add their own,
   if allowed."""
 
-  def do_get(self, feature_id, field_id=None):
+  def do_get(self, **kwargs):
     """Return a list of all approval values on the given feature."""
+    feature_id = kwargs.get('feature_id', None)
+    field_id = kwargs.get('field_id', None)
     # Note: We assume that anyone may view approvals.
     approvals = review_models.Approval.get_approvals(
         feature_id=feature_id, field_id=field_id)
@@ -48,8 +50,9 @@ class ApprovalsAPI(basehandlers.APIHandler):
         }
     return data
 
-  def do_post(self, feature_id=None):
+  def do_post(self, **kwargs):
     """Set an approval value for the specified feature."""
+    feature_id = kwargs.get('feature_id', None)
     field_id = self.get_int_param('fieldId')
     new_state = self.get_int_param(
         'state', validator=review_models.Approval.is_valid_state)
@@ -93,8 +96,9 @@ def approval_config_to_json_dict(appr_cfg):
 class ApprovalConfigsAPI(basehandlers.APIHandler):
   """Get and set the approval configs for a feature."""
 
-  def do_get(self, feature_id):
+  def do_get(self, **kwargs):
     """Return a list of all approval configs on the given feature."""
+    feature_id = kwargs.get('feature_id', None)
     # Note: We assume that anyone may view approval configs.
     configs = review_models.ApprovalConfig.get_configs(feature_id)
     dicts = [approval_config_to_json_dict(ac) for ac in configs]
@@ -108,8 +112,9 @@ class ApprovalConfigsAPI(basehandlers.APIHandler):
         }
     return data
 
-  def do_post(self, feature_id):
+  def do_post(self, **kwargs):
     """Set an approval config for the specified feature."""
+    feature_id = kwargs['feature_id']
     field_id = self.get_int_param('fieldId')
     owners_str = self.get_param('owners')
     next_action_str = self.get_param('nextAction')

api/approvals_api_test.py

@@ -72,7 +72,7 @@ class ApprovalsAPITest(testing_config.CustomTestCase):
     """We can get all approvals for a given feature, even if there none."""
     testing_config.sign_out()
     with test_app.test_request_context(self.request_path):
-      actual_response = self.handler.do_get(self.feature_id)
+      actual_response = self.handler.do_get(feature_id=self.feature_id)
     self.assertEqual({"approvals": []}, actual_response)
 
   def test_get__all_some(self):
@@ -82,7 +82,7 @@ class ApprovalsAPITest(testing_config.CustomTestCase):
     self.appr_1_2.put()
 
     with test_app.test_request_context(self.request_path):
-      actual_response = self.handler.do_get(self.feature_id)
+      actual_response = self.handler.do_get(feature_id=self.feature_id)
 
     self.assertEqual(
         {"approvals": [self.expected1, self.expected2]},
@@ -92,7 +92,8 @@ class ApprovalsAPITest(testing_config.CustomTestCase):
     """We can get approvals for given feature and field, even if there none."""
     testing_config.sign_out()
     with test_app.test_request_context(self.request_path + '/1'):
-      actual_response = self.handler.do_get(self.feature_id, field_id=1)
+      actual_response = self.handler.do_get(
+          feature_id=self.feature_id, field_id=1)
     self.assertEqual({"approvals": []}, actual_response)
 
   def test_get__field_some(self):
@@ -102,7 +103,8 @@ class ApprovalsAPITest(testing_config.CustomTestCase):
     self.appr_1_2.put()
 
     with test_app.test_request_context(self.request_path + '/1'):
-      actual_response = self.handler.do_get(self.feature_id, field_id=1)
+      actual_response = self.handler.do_get(
+          feature_id=self.feature_id, field_id=1)
 
     self.assertEqual(
         {"approvals": [self.expected1]},
@@ -250,7 +252,7 @@ class ApprovalConfigsAPITest(testing_config.CustomTestCase):
     mock_get_approvers.return_value = ['owner@example.com']
     testing_config.sign_in('other@example.com', 123567890)
     with test_app.test_request_context(self.request_path):
-      actual = self.handler.do_get(self.feature_1_id)
+      actual = self.handler.do_get(feature_id=self.feature_1_id)
     self.assertEqual(
         {'configs': [{
             'feature_id': self.feature_1_id,
@@ -270,7 +272,7 @@ class ApprovalConfigsAPITest(testing_config.CustomTestCase):
 
     testing_config.sign_out()
     with test_app.test_request_context(self.request_path):
-      actual = self.handler.do_get(self.feature_2_id)
+      actual = self.handler.do_get(feature_id=self.feature_2_id)
     self.assertEqual(
         {'configs': [{
             'feature_id': self.feature_2_id,
@@ -293,7 +295,7 @@ class ApprovalConfigsAPITest(testing_config.CustomTestCase):
     """If there are no configs, we return an empty list."""
     mock_get_approvers.return_value = ['owner@example.com']
     with test_app.test_request_context(self.request_path):
-      actual = self.handler.do_get(self.feature_3_id)
+      actual = self.handler.do_get(feature_id=self.feature_3_id)
 
     self.assertEqual(
         {'configs': [],
@@ -317,7 +319,7 @@ class ApprovalConfigsAPITest(testing_config.CustomTestCase):
               'nextAction': '2021-11-30',
               'additionalReview': False}
     with test_app.test_request_context(self.request_path, json=params):
-      actual = self.handler.do_post(self.feature_1_id)
+      actual = self.handler.do_post(feature_id=self.feature_1_id)
 
     self.assertEqual({'message': 'Done'}, actual)
     revised_configs = review_models.ApprovalConfig.query(
@@ -340,7 +342,7 @@ class ApprovalConfigsAPITest(testing_config.CustomTestCase):
               'nextAction': '2021-11-30',
               'additionalReview': False}
     with test_app.test_request_context(self.request_path, json=params):
-      actual = self.handler.do_post(self.feature_1_id)
+      actual = self.handler.do_post(feature_id=self.feature_1_id)
 
     self.assertEqual({'message': 'Done'}, actual)
     revised_config = review_models.ApprovalConfig.query(
@@ -364,7 +366,7 @@ class ApprovalConfigsAPITest(testing_config.CustomTestCase):
               'nextAction': '',
               'additionalReview': False}
     with test_app.test_request_context(self.request_path, json=params):
-      actual = self.handler.do_post(self.feature_1_id)
+      actual = self.handler.do_post(feature_id=self.feature_1_id)
 
     self.assertEqual({'message': 'Done'}, actual)
     revised_config = review_models.ApprovalConfig.query(
@@ -386,7 +388,7 @@ class ApprovalConfigsAPITest(testing_config.CustomTestCase):
               'nextAction': '2021-11-30',
               'additionalReview': False}
     with test_app.test_request_context(self.request_path, json=params):
-      actual = self.handler.do_post(self.feature_3_id)
+      actual = self.handler.do_post(feature_id=self.feature_3_id)
 
     self.assertEqual({'message': 'Done'}, actual)
     new_config = review_models.ApprovalConfig.query(
@@ -410,7 +412,7 @@ class ApprovalConfigsAPITest(testing_config.CustomTestCase):
               'additionalReview': False}
     with test_app.test_request_context(self.request_path, json=params):
       with self.assertRaises(werkzeug.exceptions.BadRequest):
-        self.handler.do_post(self.feature_3_id)
+        self.handler.do_post(feature_id=self.feature_3_id)
 
     params = {'fieldId': 3,
               'owners': '',
@@ -418,4 +420,4 @@ class ApprovalConfigsAPITest(testing_config.CustomTestCase):
               'additionalReview': False}
     with test_app.test_request_context(self.request_path, json=params):
       with self.assertRaises(werkzeug.exceptions.BadRequest):
-        self.handler.do_post(self.feature_3_id)
+        self.handler.do_post(feature_id=self.feature_3_id)

api/blink_components_api.py

@@ -22,6 +22,6 @@ class BlinkComponentsAPI(basehandlers.APIHandler):
   """The list of blink components populates the "Blink component" select field
   in the guide form"""
 
-  def do_get(self):
+  def do_get(self, **kwargs):
     """Returns a dict with blink components as both keys and values."""
     return {x: [x, x] for x in user_models.BlinkComponent.fetch_all_components()}

api/channels_api.py

@@ -99,7 +99,7 @@ def construct_specified_milestones_details(start, end):
 class ChannelsAPI(basehandlers.APIHandler):
   """Channels are the Chrome Versions across platforms."""
 
-  def do_get(self):
+  def do_get(self, **kwargs):
     # Query-string parameters 'start' and 'end' are provided
     if (self.request.args.get('start') is not None and
         self.request.args.get('end') is not None):

api/comments_api.py

@@ -45,8 +45,9 @@ class CommentsAPI(basehandlers.APIHandler):
     """Check whether a comment should be visible to the user."""
     return comment.deleted_by is None or email == comment.deleted_by or is_admin
 
-  def do_get(self, feature_id: int,
-      field_id: Optional[int]=None) -> dict[str, dict[str, Any]]:
+  def do_get(self, **kwargs) -> dict[str, list[dict[str, Any]]]:
+    feature_id = kwargs['feature_id']
+    field_id = kwargs.get('field_id', None)
     """Return a list of all review comments on the given feature."""
     # Note: We assume that anyone may view approval comments.
     comments = Activity.get_activities(
@@ -61,9 +62,10 @@ class CommentsAPI(basehandlers.APIHandler):
     dicts = [comment_to_json_dict(c) for c in comments]
     return {'comments': dicts}
 
-  def do_post(
-      self, feature_id: int, gate_id: Optional[int]=None) -> dict[str, str]:
+  def do_post(self, **kwargs) -> dict[str, str]:
     """Add a review comment and possibly set a approval value."""
+    feature_id = kwargs['feature_id']
+    gate_id = kwargs.get('gate_id', None)
     new_state = self.get_int_param(
         'state', required=False,
         validator=Approval.is_valid_state)
@@ -99,7 +101,7 @@ class CommentsAPI(basehandlers.APIHandler):
     # Callers don't use the JSON response for this API call.
     return {'message': 'Done'}
 
-  def do_patch(self, feature_id: int) -> dict[str, str]:
+  def do_patch(self, **kwargs) -> dict[str, str]:
     comment_id = self.get_param('commentId', required=True)
     comment: Optional[Activity] = Activity.get_by_id(comment_id)
 

api/comments_api_test.py

@@ -70,7 +70,8 @@ class CommentsAPITest(testing_config.CustomTestCase):
     testing_config.sign_out()
     testing_config.sign_in('user7@example.com', 123567890)
     with test_app.test_request_context(self.request_path):
-      actual_response = self.handler.do_get(self.feature_id, self.gate_id)
+      actual_response = self.handler.do_get(
+          feature_id=self.feature_id, field_id=self.gate_id)
     testing_config.sign_out()
     self.assertEqual({'comments': []}, actual_response)
 
@@ -81,7 +82,8 @@ class CommentsAPITest(testing_config.CustomTestCase):
     self.act_1_1.put()
 
     with test_app.test_request_context(self.request_path):
-      actual_response = self.handler.do_get(self.feature_id, self.gate_id)
+      actual_response = self.handler.do_get(
+          feature_id=self.feature_id, field_id=self.gate_id)
     testing_config.sign_out()
     actual_comment = actual_response['comments'][0]
     del actual_comment['created']
@@ -98,7 +100,8 @@ class CommentsAPITest(testing_config.CustomTestCase):
     self.act_1_1.put()
 
     with test_app.test_request_context(self.request_path):
-      resp = self.handler.do_get(self.feature_id, self.gate_id)
+      resp = self.handler.do_get(
+        feature_id=self.feature_id, field_id=self.gate_id)
     testing_config.sign_out()
     self.assertEqual(resp['comments'], [])
 
@@ -111,7 +114,8 @@ class CommentsAPITest(testing_config.CustomTestCase):
     self.act_1_1.put()
 
     with test_app.test_request_context(self.request_path):
-      resp = self.handler.do_get(self.feature_id, self.gate_id)
+      resp = self.handler.do_get(
+          feature_id=self.feature_id, field_id=self.gate_id)
     testing_config.sign_out()
     comment = resp['comments'][0]
     self.assertNotEqual(comment['content'], '[Deleted]')
@@ -121,12 +125,12 @@ class CommentsAPITest(testing_config.CustomTestCase):
     params = {'state': 'not an int'}
     with test_app.test_request_context(self.request_path, json=params):
       with self.assertRaises(werkzeug.exceptions.BadRequest):
-        self.handler.do_post(self.feature_id, self.gate_id)
+        self.handler.do_post(feature_id=self.feature_id, gate_id=self.gate_id)
 
     params = {'state': 999}
     with test_app.test_request_context(self.request_path, json=params):
       with self.assertRaises(werkzeug.exceptions.BadRequest):
-        self.handler.do_post(self.feature_id, self.gate_id)
+        self.handler.do_post(feature_id=self.feature_id, gate_id=self.gate_id)
 
   def test_post__feature_not_found(self):
     """Handler rejects requests that don't match an existing feature."""
@@ -134,7 +138,7 @@ class CommentsAPITest(testing_config.CustomTestCase):
     params = {'state': review_models.Approval.NEEDS_WORK }
     with test_app.test_request_context(bad_path, json=params):
       with self.assertRaises(werkzeug.exceptions.NotFound):
-        self.handler.do_post(12345, self.gate_id)
+        self.handler.do_post(feature_id=12345, gate_id=self.gate_id)
 
   @mock.patch('internals.approval_defs.get_approvers')
   def test_post__forbidden(self, mock_get_approvers):
@@ -145,17 +149,17 @@ class CommentsAPITest(testing_config.CustomTestCase):
     testing_config.sign_out()
     with test_app.test_request_context(self.request_path, json=params):
       with self.assertRaises(werkzeug.exceptions.Forbidden):
-        self.handler.do_post(self.feature_id, self.gate_id)
+        self.handler.do_post(feature_id=self.feature_id, gate_id=self.gate_id)
 
     testing_config.sign_in('user7@example.com', 123567890)
     with test_app.test_request_context(self.request_path, json=params):
       with self.assertRaises(werkzeug.exceptions.Forbidden):
-        self.handler.do_post(self.feature_id, self.gate_id)
+        self.handler.do_post(feature_id=self.feature_id, gate_id=self.gate_id)
 
     testing_config.sign_in('user@google.com', 123567890)
     with test_app.test_request_context(self.request_path, json=params):
       with self.assertRaises(werkzeug.exceptions.Forbidden):
-        self.handler.do_post(self.feature_id, self.gate_id)
+        self.handler.do_post(feature_id=self.feature_id, gate_id=self.gate_id)
 
   def test_patch__forbidden(self):
     """Handler rejects requests from users who can't edit the given comment."""
@@ -165,12 +169,12 @@ class CommentsAPITest(testing_config.CustomTestCase):
     testing_config.sign_out()
     with test_app.test_request_context(self.request_path, json=params):
       with self.assertRaises(werkzeug.exceptions.Forbidden):
-        self.handler.do_patch(self.feature_id)
+        self.handler.do_patch(feature_id=self.feature_id)
 
     testing_config.sign_in('user7@example.com', 123567890)
     with test_app.test_request_context(self.request_path, json=params):
       with self.assertRaises(werkzeug.exceptions.Forbidden):
-        self.handler.do_patch(self.feature_id)
+        self.handler.do_patch(feature_id=self.feature_id)
 
   def test_patch__delete_comment(self):
     """Handler marks a comment as deleted as requested by authorized user."""
@@ -180,8 +184,9 @@ class CommentsAPITest(testing_config.CustomTestCase):
     params = {'commentId': self.act_1_1.key.id(), 'isUndelete': False}
     testing_config.sign_in(user_email, 123567890)
     with test_app.test_request_context(self.request_path, json=params):
-      resp = self.handler.do_patch(self.feature_id)
-      get_resp = self.handler.do_get(self.feature_id, self.gate_id)
+      resp = self.handler.do_patch(feature_id=self.feature_id)
+      get_resp = self.handler.do_get(
+          feature_id=self.feature_id, field_id=self.gate_id)
     testing_config.sign_out()
     self.assertEqual(get_resp['comments'][0]['deleted_by'], user_email)
     self.assertEqual(resp, {'message': 'Done'})
@@ -200,8 +205,9 @@ class CommentsAPITest(testing_config.CustomTestCase):
     params = {'commentId': self.act_1_1.key.id(), 'isUndelete': True}
     testing_config.sign_in(user_email, 123567890)
     with test_app.test_request_context(self.request_path, json=params):
-      resp = self.handler.do_patch(self.feature_id)
-      get_resp = self.handler.do_get(self.feature_id, self.gate_id)
+      resp = self.handler.do_patch(feature_id=self.feature_id)
+      get_resp = self.handler.do_get(
+          feature_id=self.feature_id, field_id=self.gate_id)
     testing_config.sign_out()
     self.assertEqual(get_resp['comments'][0]['deleted_by'], None)
     self.assertEqual(resp, {'message': 'Done'})
@@ -218,7 +224,7 @@ class CommentsAPITest(testing_config.CustomTestCase):
     testing_config.sign_in('owner2@example.com', 123567890)
     params = {'comment': 'Congratulations'}
     with test_app.test_request_context(self.request_path, json=params):
-      actual = self.handler.do_post(self.feature_id, self.gate_id)
+      actual = self.handler.do_post(feature_id=self.feature_id, gate_id=self.gate_id)
 
     self.assertEqual(actual, {'message': 'Done'})
     updated_approvals = review_models.Approval.get_approvals(

api/cues_api.py

@@ -31,7 +31,7 @@ class CuesAPI(basehandlers.APIHandler):
   # Note: there is no do_get yet because we decide to show cues
   # based on data that is include in the HTML page.
 
-  def do_post(self):
+  def do_post(self, **kwargs):
     """Dismisses a cue card for the signed in user."""
     cue = self.get_param('cue', allowed=ALLOWED_CUES)
     unused_user = self.get_current_user(required=True)
@@ -40,7 +40,7 @@ class CuesAPI(basehandlers.APIHandler):
     # Callers don't use the JSON response for this API call.
     return {'message': 'Done'}
 
-  def do_get(self):
+  def do_get(self, **kwargs):
     """Return a list of the dismissed cue cards"""
     user_pref = user_models.UserPref.get_signed_in_user_pref()
 

api/features_api.py

@@ -63,8 +63,9 @@ class FeaturesAPI(basehandlers.APIHandler):
         'features': features_on_page,
         }
 
-  def do_get(self, feature_id: Optional[int]=None) -> dict:
+  def do_get(self, **kwargs) -> dict:
     """Handle GET requests for a single feature or a search."""
+    feature_id = kwargs.get('feature_id', None)
     if feature_id:
       return self.get_one_feature(feature_id)
     return self.do_search()
@@ -74,9 +75,10 @@ class FeaturesAPI(basehandlers.APIHandler):
   # TODO(jrobbins): do_patch
 
   @permissions.require_admin_site
-  def do_delete(self, feature_id: int) -> dict[str, str]:
+  def do_delete(self, **kwargs) -> dict[str, str]:
     """Delete the specified feature."""
     # TODO(jrobbins): implement undelete UI.  For now, use cloud console.
+    feature_id = kwargs.get('feature_id', None)
     feature = self.get_specified_feature(feature_id=feature_id)
     feature.deleted = True
     feature.put()

api/features_api_test.py

@@ -56,7 +56,7 @@ class FeaturesAPITestDelete(testing_config.CustomTestCase):
     testing_config.sign_in('admin@example.com', 123567890)
 
     with test_app.test_request_context(self.request_path):
-      actual_json = self.handler.do_delete(self.feature_id)
+      actual_json = self.handler.do_delete(feature_id=self.feature_id)
     self.assertEqual({'message': 'Done'}, actual_json)
 
     revised_feature = core_models.Feature.get_by_id(self.feature_id)
@@ -68,7 +68,7 @@ class FeaturesAPITestDelete(testing_config.CustomTestCase):
 
     with test_app.test_request_context(self.request_path):
       with self.assertRaises(werkzeug.exceptions.Forbidden):
-        self.handler.do_delete(self.feature_id)
+        self.handler.do_delete(feature_id=self.feature_id)
 
     revised_feature = core_models.Feature.get_by_id(self.feature_id)
     self.assertFalse(revised_feature.deleted)
@@ -79,7 +79,7 @@ class FeaturesAPITestDelete(testing_config.CustomTestCase):
 
     with test_app.test_request_context(self.request_path):
       with self.assertRaises(werkzeug.exceptions.BadRequest):
-        self.handler.do_delete(None)
+        self.handler.do_delete()
 
     revised_feature = core_models.Feature.get_by_id(self.feature_id)
     self.assertFalse(revised_feature.deleted)
@@ -90,7 +90,7 @@ class FeaturesAPITestDelete(testing_config.CustomTestCase):
 
     with test_app.test_request_context(self.request_path):
       with self.assertRaises(werkzeug.exceptions.NotFound):
-        self.handler.do_delete(self.feature_id + 1)
+        self.handler.do_delete(feature_id=self.feature_id + 1)
 
     revised_feature = core_models.Feature.get_by_id(self.feature_id)
     self.assertFalse(revised_feature.deleted)

api/login_api.py

@@ -26,7 +26,7 @@ import settings
 class LoginAPI(basehandlers.APIHandler):
   """Create a session using the credential generated by Sign-In With Google."""
 
-  def do_post(self):
+  def do_post(self, **kwargs):
     # TODO(jrobbins): Remove id_token after next deployment.
     token = (self.get_param('id_token', required=False) or
              self.get_param('credential'))

api/logout_api.py

@@ -23,6 +23,6 @@ from framework import basehandlers
 class LogoutAPI(basehandlers.APIHandler):
   """Clear the session when the user signs out."""
 
-  def do_post(self):
+  def do_post(self, **kwargs):
     session.clear()
     return {'message': 'Done'}

api/metricsdata.py

@@ -68,7 +68,7 @@ class TimelineHandler(basehandlers.FlaskHandler):
           self.MODEL_CLASS.date >= datetime.datetime(2017, 10, 26))
     return query
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     try:
       bucket_id = int(self.request.args.get('bucket_id'))
     except:
@@ -96,7 +96,7 @@ class PopularityTimelineHandler(TimelineHandler):
   CACHE_KEY = TimelineHandler.CACHE_PREFIX + 'css_pop_timeline'
   MODEL_CLASS = metrics_models.StableInstance
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     return super(PopularityTimelineHandler, self).get_template_data()
 
 
@@ -105,7 +105,7 @@ class AnimatedTimelineHandler(TimelineHandler):
   CACHE_KEY = TimelineHandler.CACHE_PREFIX + 'css_animated_timeline'
   MODEL_CLASS = metrics_models.AnimatedProperty
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     return super(AnimatedTimelineHandler, self).get_template_data()
 
 
@@ -114,7 +114,7 @@ class FeatureObserverTimelineHandler(TimelineHandler):
   CACHE_KEY = TimelineHandler.CACHE_PREFIX + 'featureob_timeline'
   MODEL_CLASS = metrics_models.FeatureObserver
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     return super(FeatureObserverTimelineHandler, self).get_template_data()
 
 
@@ -166,7 +166,7 @@ class FeatureHandler(basehandlers.FlaskHandler):
     datapoints.sort(key=lambda x: x.day_percentage, reverse=True)
     return datapoints
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     if self.MODEL_CLASS == metrics_models.FeatureObserver:
       properties = rediscache.get(self.CACHE_KEY)
 
@@ -195,7 +195,7 @@ class CSSPopularityHandler(FeatureHandler):
   MODEL_CLASS = metrics_models.StableInstance
   PROPERTY_CLASS = metrics_models.CssPropertyHistogram
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     return super(CSSPopularityHandler, self).get_template_data()
 
 
@@ -205,7 +205,7 @@ class CSSAnimatedHandler(FeatureHandler):
   MODEL_CLASS = metrics_models.AnimatedProperty
   PROPERTY_CLASS = metrics_models.CssPropertyHistogram
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     return super(CSSAnimatedHandler, self).get_template_data()
 
 
@@ -215,7 +215,7 @@ class FeatureObserverPopularityHandler(FeatureHandler):
   MODEL_CLASS = metrics_models.FeatureObserver
   PROPERTY_CLASS = metrics_models.FeatureObserverHistogram
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     return super(FeatureObserverPopularityHandler, self).get_template_data()
 
 
@@ -223,7 +223,8 @@ class FeatureBucketsHandler(basehandlers.FlaskHandler):
   HTTP_CACHE_TYPE = 'private'
   JSONIFY = True
 
-  def get_template_data(self, prop_type):
+  def get_template_data(self, **kwargs):
+    prop_type = kwargs.get('prop_type', None)
     if prop_type == 'cssprops':
       properties = sorted(
           metrics_models.CssPropertyHistogram.get_all().items(),

api/metricsdata_test.py

@@ -133,14 +133,14 @@ class FeatureBucketsHandlerTest(testing_config.CustomTestCase):
 
   def test_get_template_data__css(self):
     with test_app.test_request_context('/data/blink/cssprops'):
-      actual_buckets = self.handler.get_template_data('cssprops')
+      actual_buckets = self.handler.get_template_data(prop_type='cssprops')
     self.assertEqual(
         [(2, 'a prop'), (1, 'b prop')],
         actual_buckets)
 
   def test_get_template_data__js(self):
     with test_app.test_request_context('/data/blink/features'):
-      actual_buckets = self.handler.get_template_data('features')
+      actual_buckets = self.handler.get_template_data(prop_type='features')
     self.assertEqual(
         [(4, 'a feat'), (3, 'b feat')],
         actual_buckets)

api/permissions_api.py

@@ -23,7 +23,7 @@ class PermissionsAPI(basehandlers.APIHandler):
   """Permissions determine whether a user can create, approve, 
   or edit any feature, or admin the site"""
 
-  def do_get(self):
+  def do_get(self, **kwargs):
     """Return the permissions and the email of the user."""
 
     # No user data if not signed in

api/processes_api.py

@@ -22,9 +22,10 @@ from internals import processes
 class ProcessesAPI(basehandlers.APIHandler):
   """Processes contain details about the feature status"""
 
-  def do_get(self, feature_id):
+  def do_get(self, **kwargs):
     """Return the process of the feature."""
     # Load feature directly from NDB so as to never get a stale cached copy.
+    feature_id = kwargs['feature_id']
     f = core_models.Feature.get_by_id(feature_id)
     if f is None:
       self.abort(404, msg=f'Feature {feature_id} not found')
@@ -40,8 +41,9 @@ class ProgressAPI(basehandlers.APIHandler):
   or a string that starts with "http:" or "https:" that contain details about
   the progress of a feature so far"""
 
-  def do_get(self, feature_id):
+  def do_get(self, **kwargs):
     """Return the progress of the feature."""
+    feature_id = kwargs['feature_id']
     f = core_models.Feature.get_by_id(feature_id)
     if f is None:
       self.abort(404, msg=f'Feature {feature_id} not found')

api/processes_api_test.py

@@ -42,7 +42,7 @@ class ProcessesAPITest(testing_config.CustomTestCase):
   def test_get__default_feature_type(self):
     """We can get process for features with the default feature type (New feature incubation)."""
     with test_app.test_request_context(self.request_path):
-      actual = self.handler.do_get(self.feature_id)
+      actual = self.handler.do_get(feature_id=self.feature_id)
     expected = processes.process_to_dict(processes.BLINK_LAUNCH_PROCESS)
     self.assertEqual(expected, actual)
 
@@ -50,7 +50,7 @@ class ProcessesAPITest(testing_config.CustomTestCase):
     """We can get process for features with feature type 0 (New feature incubation)."""
     self.feature_1.feature_type = 0
     with test_app.test_request_context(self.request_path):
-      actual = self.handler.do_get(self.feature_id)
+      actual = self.handler.do_get(feature_id=self.feature_id)
     expected = processes.process_to_dict(processes.BLINK_LAUNCH_PROCESS)
     self.assertEqual(expected, actual)
 
@@ -58,7 +58,7 @@ class ProcessesAPITest(testing_config.CustomTestCase):
     """We can get process for features with feature type 1 (Existing feature implementation)."""
     self.feature_1.feature_type = 1
     with test_app.test_request_context(self.request_path):
-      actual = self.handler.do_get(self.feature_id)
+      actual = self.handler.do_get(feature_id=self.feature_id)
     expected = processes.process_to_dict(processes.BLINK_FAST_TRACK_PROCESS)
     self.assertEqual(expected, actual)
 
@@ -66,7 +66,7 @@ class ProcessesAPITest(testing_config.CustomTestCase):
     """We can get process for features with feature type 2 (Web developer facing change to existing code)."""
     self.feature_1.feature_type = 2
     with test_app.test_request_context(self.request_path):
-      actual = self.handler.do_get(self.feature_id)
+      actual = self.handler.do_get(feature_id=self.feature_id)
     expected = processes.process_to_dict(processes.PSA_ONLY_PROCESS)
     self.assertEqual(expected, actual)
 
@@ -74,7 +74,7 @@ class ProcessesAPITest(testing_config.CustomTestCase):
     """We can get process for features with feature type 3 (Feature deprecation)."""
     self.feature_1.feature_type = 3
     with test_app.test_request_context(self.request_path):
-      actual = self.handler.do_get(self.feature_id)
+      actual = self.handler.do_get(feature_id=self.feature_id)
     expected = processes.process_to_dict(processes.DEPRECATION_PROCESS)
     self.assertEqual(expected, actual)
 
@@ -102,7 +102,7 @@ class ProgressAPITest(testing_config.CustomTestCase):
   def test_get___feature_progress(self):
     """We can get progress of a feature."""
     with test_app.test_request_context(self.request_path):
-      actual = self.handler.do_get(self.feature_id)
+      actual = self.handler.do_get(feature_id=self.feature_id)
 
     self.maxDiff = None
     self.assertEqual({

api/settings_api.py

@@ -22,7 +22,7 @@ class SettingsAPI(basehandlers.APIHandler):
   """Users can store their settings preferences such as whether to get
   notification from the features they starred."""
 
-  def do_post(self):
+  def do_post(self, **kwargs):
     """Set the user settings (currently only the notify_as_starrer)"""
     user_pref = user_models.UserPref.get_signed_in_user_pref()
     if not user_pref:
@@ -33,7 +33,7 @@ class SettingsAPI(basehandlers.APIHandler):
     # Callers don't use the JSON response for this API call.
     return {'message': 'Done'}
 
-  def do_get(self):
+  def do_get(self, **kwargs):
     """Return the user settings (currently only the notify_as_starrer)"""
     user_pref = user_models.UserPref.get_signed_in_user_pref()
     if not user_pref:

api/stars_api.py

@@ -28,7 +28,7 @@ class StarsAPI(basehandlers.APIHandler):
   logic to toggle the star icon.  When a user has starred a feature, they
   will be sent notification emails about changes to that feature."""
 
-  def do_get(self):
+  def do_get(self, **kwargs):
     """Return a list of all starred feature IDs for the signed-in user."""
     user = self.get_current_user()
     if user:
@@ -41,7 +41,7 @@ class StarsAPI(basehandlers.APIHandler):
         }
     return data
 
-  def do_post(self):
+  def do_post(self, **kwargs):
     """Set or clear a star on the specified feature."""
     feature = self.get_specified_feature()
     starred = self.get_bool_param('starred', default=True)

api/token_refresh_api.py

@@ -37,7 +37,7 @@ class TokenRefreshAPI(basehandlers.APIHandler):
     xsrf.validate_token(token, email, timeout=xsrf.REFRESH_TOKEN_TIMEOUT_SEC)
 
   # Note: we use only POST instead of GET to avoid attacks that use GETs.
-  def do_post(self):
+  def do_post(self, **kwargs):
     """Refresh the session and return a new XSRF token for the current user."""
     user = self.get_current_user()
     users.refresh_user_session()

framework/basehandlers.py

@@ -286,7 +286,7 @@ class FlaskHandler(BaseHandler):
     headers.update(self.get_cache_headers())
     return headers
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     """Subclasses should implement this method to handle a GET request."""
     raise NotImplementedError()
 
@@ -300,7 +300,7 @@ class FlaskHandler(BaseHandler):
         'No TEMPLATE_PATH was defined in %r or returned in template_data.' %
         self.__class__.__name__)
 
-  def process_post_data(self):
+  def process_post_data(self, **kwargs):
     """Subclasses should implement this method to handle a POST request."""
     self.abort(405, msg='Unexpected HTTP method', valid_methods=['GET'])
 
@@ -486,7 +486,8 @@ class Redirector(FlaskHandler):
      Specify the location in the third part of a routing rule using:
      {'location': '/path/to/page'}."""
 
-  def get_template_data(self, location='/'):
+  def get_template_data(self, **kwargs):
+    location = kwargs['location'] if 'location' in kwargs else '/'
     return flask.redirect(location), self.get_headers()
 
 

framework/basehandlers_test.py

@@ -46,7 +46,8 @@ class TestableFlaskHandler(basehandlers.FlaskHandler):
       template_data['status'] = special_status
     return template_data
 
-  def process_post_data(self, redirect_to=None):
+  def process_post_data(self, **kwargs):
+    redirect_to = kwargs.get('redirect_to', None)
     if redirect_to:
       return flask.redirect(redirect_to)
 

internals/data_backup.py

@@ -32,7 +32,7 @@ class MemoryCache(Cache):
 class BackupExportHandler(basehandlers.FlaskHandler):
   """Triggers a new Datastore export."""
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     self.require_cron_header()
     bucket = f'gs://{settings.BACKUP_BUCKET}'
     # The default cache (file_cache) is unavailable when using oauth2client >= 4.0.0 or google-auth,

internals/deprecate_field.py

@@ -6,7 +6,7 @@ from internals.core_models import Feature
 
 class WriteStandardMaturityHandler(FlaskHandler):
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     """Writes standard_maturity field from the old standardization field."""
     self.require_cron_header()
     q = Feature.query()

internals/detect_intent.py

@@ -160,7 +160,7 @@ class IntentEmailHandler(basehandlers.FlaskHandler):
 
   IS_INTERNAL_HANDLER = True
 
-  def process_post_data(self):
+  def process_post_data(self, **kwargs):
     self.require_task_header()
 
     from_addr = self.get_param('from_addr')

internals/fetchmetrics.py

@@ -179,7 +179,7 @@ UMA_QUERIES = [
 class YesterdayHandler(basehandlers.FlaskHandler):
   """Loads yesterday's UMA data."""
 
-  def get_template_data(self, today=None):
+  def get_template_data(self, **kwargs):
     """Loads the data file located at |filename|.
 
     Args:
@@ -198,7 +198,7 @@ class YesterdayHandler(basehandlers.FlaskHandler):
         self.abort(400, msg='Failed to parse date string.')
 
     else:
-      today = today or datetime.date.today()
+      today = kwargs.get('today', datetime.date.today())
       days = [today - datetime.timedelta(days_ago)
               for days_ago in [1, 2, 3, 4, 5]]
 
@@ -250,7 +250,7 @@ class HistogramsHandler(basehandlers.FlaskHandler):
       property_name=property_name
     )
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     self.require_cron_header()
     # Attempt to fetch enums mapping file.
     response = requests.get(HISTOGRAMS_URL, timeout=60)
@@ -285,7 +285,7 @@ class HistogramsHandler(basehandlers.FlaskHandler):
 
 class BlinkComponentHandler(basehandlers.FlaskHandler):
   """Updates the list of Blink components in the db."""
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     self.require_cron_header()
     user_models.BlinkComponent.update_db()
     return 'Blink components updated'

internals/inactive_users.py

@@ -22,11 +22,10 @@ class RemoveInactiveUsersHandler(FlaskHandler):
   DEFAULT_LAST_VISIT = datetime(2022, 8, 1)  # 2022-08-01
   INACTIVE_REMOVE_DAYS = 270
 
-  def get_template_data(self, now=None):
+  def get_template_data(self, **kwargs):
     """Removes any users that have been inactive for 9 months."""
     self.require_cron_header()
-    if now is None:
-      now = datetime.now()
+    now = kwargs.get('now', datetime.now())
 
     q = AppUser.query()
     users = q.fetch()

internals/notifier.py

@@ -266,9 +266,11 @@ class NotifyInactiveUsersHandler(basehandlers.FlaskHandler):
   INACTIVE_WARN_DAYS = 180
   EMAIL_TEMPLATE_PATH = 'inactive_user_email.html'
 
-  def get_template_data(self, now=None):
+  def get_template_data(self, **kwargs):
     """Notify any users that have been inactive for 6 months."""
     self.require_cron_header()
+    now = kwargs.get('now', datetime.now())
+
     users_to_notify = self._determine_users_to_notify(now)
     email_tasks = self._build_email_tasks(users_to_notify)
     send_emails(email_tasks)
@@ -328,7 +330,7 @@ class FeatureChangeHandler(basehandlers.FlaskHandler):
 
   IS_INTERNAL_HANDLER = True
 
-  def process_post_data(self):
+  def process_post_data(self, **kwargs):
     self.require_task_header()
 
     feature = self.get_param('feature')

internals/reminders.py

@@ -73,7 +73,7 @@ class AbstractReminderHandler(basehandlers.FlaskHandler):
   FUTURE_MILESTONES_TO_CONSIDER = 0
   MILESTONE_FIELDS = None  # Subclasses must override
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     """Sends notifications to users requesting feature updates for accuracy."""
     self.require_cron_header()
     current_milestone_info = get_current_milestone_info(self.ANCHOR_CHANNEL)

internals/schema_migration.py

@@ -58,7 +58,7 @@ def handle_migration(original_cls, new_cls, kwarg_mapping,
 
 class MigrateCommentsToActivities(FlaskHandler):
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     """Writes an Activity entity for each unmigrated Comment entity."""
     self.require_cron_header()
 
@@ -97,7 +97,7 @@ class MigrateCommentsToActivities(FlaskHandler):
 
 class MigrateEntities(FlaskHandler):
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     """Write FeatureEntry, Stage, Gate, and Vote entities"""
     self.require_cron_header()
 

pages/blink_handler.py

@@ -55,7 +55,7 @@ class BlinkHandler(basehandlers.FlaskHandler):
     return True
 
   @permissions.require_admin_site
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     components = user_models.BlinkComponent.query().order(
         user_models.BlinkComponent.name).fetch(None)
     possible_subscribers = user_models.FeatureOwner.query().order(
@@ -95,7 +95,7 @@ class BlinkHandler(basehandlers.FlaskHandler):
 
   # Add user to component subscribers.
   @permissions.require_admin_site
-  def process_post_data(self):
+  def process_post_data(self, **kwargs):
     params = self.request.get_json(force=True)
     self.__update_subscribers_list(True, user_id=params.get('userId'),
                                    blink_component=params.get('componentName'),
@@ -108,7 +108,7 @@ class SubscribersHandler(basehandlers.FlaskHandler):
   TEMPLATE_PATH = 'admin/subscribers.html'
 
   @permissions.require_admin_site
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     users = user_models.FeatureOwner.query().order(
         user_models.FeatureOwner.name).fetch(None)
     feature_list = core_models.Feature.get_chronological()

pages/featurelist.py

@@ -32,7 +32,8 @@ class FeaturesJsonHandler(basehandlers.FlaskHandler):
   HTTP_CACHE_TYPE = 'private'
   JSONIFY = True
 
-  def get_template_data(self, version=2):
+  def get_template_data(self, **kwargs):
+    version = kwargs.get('version', 2)
     user = users.get_current_user()
     feature_list = core_models.Feature.get_chronological(
         version=version,
@@ -44,7 +45,7 @@ class FeatureListHandler(basehandlers.FlaskHandler):
 
   TEMPLATE_PATH = 'features.html'
 
-  def get_template_data(self, feature_id=None):
+  def get_template_data(self, **kwargs):
     # Note: feature_id is not used here but JS gets it from the URL.
 
     # This template data is all for filtering.  The actual features
@@ -73,7 +74,7 @@ class FeatureListHandler(basehandlers.FlaskHandler):
 # TODO(jrobbins): Delete this some time after Oct 2022.
 class FeatureListXMLHandler(basehandlers.FlaskHandler):
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     status = self.request.args.get('status', None)
     if status:
       feature_list = core_models.Feature.get_all_with_statuses(status.split(','))

pages/guide.py

@@ -32,7 +32,7 @@ import settings
 class FeatureCreateHandler(basehandlers.FlaskHandler):
 
   @permissions.require_create_feature
-  def process_post_data(self):
+  def process_post_data(self, **kwargs):
     owners = self.split_emails('owner')
     editors = self.split_emails('editors')
     cc_emails = self.split_emails('cc_recipients')
@@ -123,7 +123,9 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
     # See TODO at top of this method.
     return param_name in self.form
 
-  def process_post_data(self, feature_id: int, stage_id: int=0):
+  def process_post_data(self, **kwargs):
+    feature_id = kwargs.get('feature_id', None)
+    stage_id = kwargs.get('stage_id', 0)
     # Validate the user has edit permissions and redirect if needed.
     redirect_resp = permissions.validate_feature_edit_permission(
         self, feature_id)

pages/guide_test.py

@@ -168,7 +168,7 @@ class FeatureEditHandlerTest(testing_config.CustomTestCase):
     with test_app.test_request_context(self.request_path, method='POST'):
       with self.assertRaises(werkzeug.exceptions.Forbidden):
         self.handler.process_post_data(
-            self.feature_1.key.integer_id(), self.stage)
+            feature_id=self.feature_1.key.integer_id(), stage_id=self.stage)
 
   def test_post__non_allowed(self):
     """Non-allowed cannot edit features, gets a 403."""
@@ -176,7 +176,7 @@ class FeatureEditHandlerTest(testing_config.CustomTestCase):
     with test_app.test_request_context(self.request_path, method='POST'):
       with self.assertRaises(werkzeug.exceptions.Forbidden):
         self.handler.process_post_data(
-            self.feature_1.key.integer_id(), self.stage)
+            feature_id=self.feature_1.key.integer_id(), stage_id=self.stage)
 
   def test_post__normal_valid(self):
     """Allowed user can edit a feature."""
@@ -208,7 +208,7 @@ class FeatureEditHandlerTest(testing_config.CustomTestCase):
             'intent_to_ship_url': new_intent_to_ship_url
         }):
       actual_response = self.handler.process_post_data(
-          self.feature_1.key.integer_id(), self.stage)
+          feature_id=self.feature_1.key.integer_id(), stage_id=self.stage)
 
     self.assertEqual('302 FOUND', actual_response.status)
     location = actual_response.headers['location']

pages/intentpreview.py

@@ -32,8 +32,11 @@ class IntentEmailPreviewHandler(basehandlers.FlaskHandler):
 
   TEMPLATE_PATH = 'admin/features/launch.html'
 
-  def get_template_data(self, feature_id=None, stage_id=None):
+  def get_template_data(self, **kwargs):
     # Validate the user has edit permissions and redirect if needed.
+    feature_id = kwargs.get('feature_id', None)
+    stage_id = kwargs.get('stage_id', None)
+
     redirect_resp = permissions.validate_feature_edit_permission(
         self, feature_id)
     if redirect_resp:

pages/intentpreview_test.py

@@ -198,7 +198,7 @@ class IntentEmailPreviewTemplateTest(testing_config.CustomTestCase):
 
     with test_app.test_request_context(self.request_path):
       self.template_data = self.handler.get_template_data(
-        self.feature_id)
+        feature_id=self.feature_id)
       page_data = self.handler.get_page_data(
           self.feature_id, self.feature_1, core_enums.INTENT_IMPLEMENT)
 

pages/metrics.py

@@ -22,6 +22,6 @@ class OmahaDataHandler(basehandlers.FlaskHandler):
 
   JSONIFY = True
 
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     omaha_data = fetchchannels.get_omaha_data()
     return omaha_data

pages/users.py

@@ -30,7 +30,7 @@ class UserListHandler(basehandlers.FlaskHandler):
   TEMPLATE_PATH = 'admin/users/new.html'
 
   @permissions.require_admin_site
-  def get_template_data(self):
+  def get_template_data(self, **kwargs):
     users = user_models.AppUser.query().fetch(None)
     user_list = [accounts_api.user_to_json_dict(user) for user in users]