user-settings: Make default `None` for name, email and password changes.

Updates `json_change_settings` so that the default value for the `email`,
`full_name`, `new_password` and `old_password` parameters is `None` instead
of an empty string, which also makes the type annotation `Optional[str]`.

Also, updates tests for email and full name changes to include an empty
string as one of the tested invalid values.

zerver/tests/test_email_change.py

@@ -244,11 +244,13 @@ class EmailChangeTestCase(ZulipTestCase):
         )
 
     def test_post_invalid_email(self) -> None:
-        data = {"email": "hamlet-new"}
+        invalid_emails = ["", "hamlet-new"]
-        self.login("hamlet")
+        for email in invalid_emails:
-        url = "/json/settings"
+            data = {"email": email}
-        result = self.client_patch(url, data)
+            self.login("hamlet")
-        self.assert_in_response("Invalid address", result)
+            url = "/json/settings"
+            result = self.client_patch(url, data)
+            self.assert_in_response("Invalid address", result)
 
     def test_post_same_email(self) -> None:
         data = {"email": self.example_email("hamlet")}

zerver/tests/test_settings.py

@@ -121,9 +121,11 @@ class ChangeSettingsTest(ZulipTestCase):
         json_result = self.client_patch("/json/settings", dict(full_name="x" * 1000))
         self.assert_json_error(json_result, "Name too long!")
 
-        # Now try a too-short name
+        # Now try too-short names
-        json_result = self.client_patch("/json/settings", dict(full_name="x"))
+        short_names = ["", "x"]
-        self.assert_json_error(json_result, "Name too short!")
+        for name in short_names:
+            json_result = self.client_patch("/json/settings", dict(full_name=name))
+            self.assert_json_error(json_result, "Name too short!")
 
     def test_illegal_characters_in_name_changes(self) -> None:
         self.login("hamlet")

zerver/views/user_settings.py

@@ -147,10 +147,10 @@ def check_settings_values(
 def json_change_settings(
     request: HttpRequest,
     user_profile: UserProfile,
-    full_name: str = REQ(default=""),
+    full_name: Optional[str] = REQ(default=None),
-    email: str = REQ(default=""),
+    email: Optional[str] = REQ(default=None),
-    old_password: str = REQ(default=""),
+    old_password: Optional[str] = REQ(default=None),
-    new_password: str = REQ(default=""),
+    new_password: Optional[str] = REQ(default=None),
     twenty_four_hour_time: Optional[bool] = REQ(json_validator=check_bool, default=None),
     dense_mode: Optional[bool] = REQ(json_validator=check_bool, default=None),
     starred_message_counts: Optional[bool] = REQ(json_validator=check_bool, default=None),
@@ -227,7 +227,7 @@ def json_change_settings(
             notification_sound, email_notifications_batching_period_seconds, default_language
         )
 
-    if new_password != "":
+    if new_password is not None:
         return_data: Dict[str, Any] = {}
         if email_belongs_to_ldap(user_profile.realm, user_profile.delivery_email):
             raise JsonableError(_("Your Zulip password is managed in LDAP"))
@@ -270,36 +270,38 @@ def json_change_settings(
         request.session.save()
 
     result: Dict[str, Any] = {}
-    new_email = email.strip()
-    if user_profile.delivery_email != new_email and new_email != "":
-        if user_profile.realm.email_changes_disabled and not user_profile.is_realm_admin:
-            raise JsonableError(_("Email address changes are disabled in this organization."))
 
-        error = validate_email_is_valid(
+    if email is not None:
-            new_email,
+        new_email = email.strip()
-            get_realm_email_validator(user_profile.realm),
+        if user_profile.delivery_email != new_email:
-        )
+            if user_profile.realm.email_changes_disabled and not user_profile.is_realm_admin:
-        if error:
+                raise JsonableError(_("Email address changes are disabled in this organization."))
-            raise JsonableError(error)
 
-        try:
+            error = validate_email_is_valid(
-            validate_email_not_already_in_realm(
-                user_profile.realm,
                 new_email,
-                verbose=False,
+                get_realm_email_validator(user_profile.realm),
             )
-        except ValidationError as e:
+            if error:
-            raise JsonableError(e.message)
+                raise JsonableError(error)
 
-        ratelimited, time_until_free = RateLimitedUser(
+            try:
-            user_profile, domain="email_change_by_user"
+                validate_email_not_already_in_realm(
-        ).rate_limit()
+                    user_profile.realm,
-        if ratelimited:
+                    new_email,
-            raise RateLimited(time_until_free)
+                    verbose=False,
+                )
+            except ValidationError as e:
+                raise JsonableError(e.message)
 
-        do_start_email_change_process(user_profile, new_email)
+            ratelimited, time_until_free = RateLimitedUser(
+                user_profile, domain="email_change_by_user"
+            ).rate_limit()
+            if ratelimited:
+                raise RateLimited(time_until_free)
 
-    if user_profile.full_name != full_name and full_name.strip() != "":
+            do_start_email_change_process(user_profile, new_email)
+
+    if full_name is not None and user_profile.full_name != full_name:
         if name_changes_disabled(user_profile.realm) and not user_profile.is_realm_admin:
             # Failingly silently is fine -- they can't do it through the UI, so
             # they'd have to be trying to break the rules.