From 05aff3f2710de0ffeeeb816e360ad223dcd9ed7f Mon Sep 17 00:00:00 2001 From: sahil839 Date: Thu, 15 Jul 2021 20:27:07 +0530 Subject: [PATCH] api: Add "ignored_parameters_unsupported" to response of '/settings'. We add "ignored_parameters_unsupported" field to the response object of 'PATCH /settings' endpoint. This will contain the parameters passed to the endpoint which are not changed by the endpoint and are ignored. This will help in removing the other fields like "full_name" from response which was essentially present to specify that only these fields were updated by the endpoint and rest were ignored. We will also change other endpoints to follow this in future. --- templates/zerver/api/changelog.md | 6 ++++++ zerver/lib/request.py | 7 +++++++ zerver/tests/test_settings.py | 11 +++++++++++ zerver/views/user_settings.py | 11 +++++++++++ 4 files changed, 35 insertions(+) diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index dddc0b9c11..ccaa5cb525 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -11,6 +11,12 @@ below features are supported. ## Changes in Zulip 5.0 +**Feature level 78** + +* `PATCH /settings`: Added `ignored_parameters_unsupported` field, + which is a list of parameters that were ignored by the endpoint, + to the response object. + **Feature level 77** * [`GET /events`](/api/get-events): Removed `recipient_id` and diff --git a/zerver/lib/request.py b/zerver/lib/request.py index 9f3f747fd2..578a53c667 100644 --- a/zerver/lib/request.py +++ b/zerver/lib/request.py @@ -13,6 +13,7 @@ from typing import ( MutableMapping, Optional, Sequence, + Set, TypeVar, Union, cast, @@ -62,6 +63,8 @@ class ZulipRequestNotes: placeholder_open_graph_description: Optional[str] = None saved_response: Optional[HttpResponse] = None tornado_handler: Optional["handlers.AsyncDjangoHandler"] = None + processed_parameters: Set[str] = field(default_factory=set) + ignored_parameters: Set[str] = field(default_factory=set) request_notes_map: MutableMapping[HttpRequest, ZulipRequestNotes] = weakref.WeakKeyDictionary() @@ -353,6 +356,7 @@ def has_request_variables(view_func: ViewFuncT) -> ViewFuncT: @wraps(view_func) def _wrapped_view_func(request: HttpRequest, *args: object, **kwargs: object) -> HttpResponse: + request_notes = get_request_notes(request) for param in post_params: func_var_name = param.func_var_name if param.path_only: @@ -386,10 +390,13 @@ def has_request_variables(view_func: ViewFuncT) -> ViewFuncT: post_var_name: Optional[str] = None for req_var in post_var_names: + assert req_var is not None if req_var in request.POST: val = request.POST[req_var] + request_notes.processed_parameters.add(req_var) elif req_var in request.GET: val = request.GET[req_var] + request_notes.processed_parameters.add(req_var) else: # This is covered by test_REQ_aliases, but coverage.py # fails to recognize this for some reason. diff --git a/zerver/tests/test_settings.py b/zerver/tests/test_settings.py index 78bf3f3da9..e0612e022f 100644 --- a/zerver/tests/test_settings.py +++ b/zerver/tests/test_settings.py @@ -409,6 +409,17 @@ class ChangeSettingsTest(ZulipTestCase): result = self.client_post("/json/users/me/avatar", {"f1": fp1}) self.assert_json_error(result, "Avatar changes are disabled in this organization.", 400) + def test_invalid_setting_name(self) -> None: + self.login("hamlet") + + # Now try an invalid setting name + json_result = self.client_patch("/json/settings", dict(invalid_setting="value")) + self.assert_json_success(json_result) + + result = orjson.loads(json_result.content) + self.assertIn("ignored_parameters_unsupported", result) + self.assertEqual(result["ignored_parameters_unsupported"], ["invalid_setting"]) + class UserChangesTest(ZulipTestCase): def test_update_api_key(self) -> None: diff --git a/zerver/views/user_settings.py b/zerver/views/user_settings.py index 5ae4325dca..36c944dfd0 100644 --- a/zerver/views/user_settings.py +++ b/zerver/views/user_settings.py @@ -173,6 +173,17 @@ def json_change_settings( # Note that check_change_full_name strips the passed name automatically result["full_name"] = check_change_full_name(user_profile, full_name, user_profile) + # TODO: Do this more generally. + from zerver.lib.request import get_request_notes + + request_notes = get_request_notes(request) + for req_var in request.POST: + if req_var not in request_notes.processed_parameters: + request_notes.ignored_parameters.add(req_var) + + if len(request_notes.ignored_parameters) > 0: + result["ignored_parameters_unsupported"] = list(request_notes.ignored_parameters) + return json_success(result)