From ca3ce90496f6284b649b28fd43a996c0ececdf5f Mon Sep 17 00:00:00 2001 From: jkiely Date: Thu, 17 May 2018 10:22:08 -0400 Subject: [PATCH] mypy: Enable strict optional in lib/avatar.py. Add assert to function and modify tests in order to pass under strict conditions. --- mypy.ini | 2 +- zerver/lib/avatar.py | 13 +++++++++---- zerver/tests/test_upload.py | 12 ++++++------ 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/mypy.ini b/mypy.ini index 40ecc2d3be..54f572c3c9 100644 --- a/mypy.ini +++ b/mypy.ini @@ -310,7 +310,7 @@ strict_optional = False [mypy-zerver.lib.notifications] # line 122: Using group from regex, which might return None strict_optional = True [mypy-zerver.lib.avatar] # str vs Optional[str] -strict_optional = False +strict_optional = True [mypy-zerver.lib.soft_deactivation] strict_optional = False [mypy-zerver.lib.events] # signup_notifications_stream is Optional, but accessing id property diff --git a/zerver/lib/avatar.py b/zerver/lib/avatar.py index 4c8e465e99..ecefa54207 100644 --- a/zerver/lib/avatar.py +++ b/zerver/lib/avatar.py @@ -10,7 +10,7 @@ from zerver.lib.upload import upload_backend, MEDIUM_AVATAR_SIZE from zerver.models import UserProfile import urllib -def avatar_url(user_profile: UserProfile, medium: bool=False, client_gravatar: bool=False) -> str: +def avatar_url(user_profile: UserProfile, medium: bool=False, client_gravatar: bool=False) -> Optional[str]: return get_avatar_field( user_id=user_profile.id, @@ -112,6 +112,11 @@ def _get_unversioned_avatar_url(user_profile_id: int, return _get_unversioned_gravatar_url(email, medium) def absolute_avatar_url(user_profile: UserProfile) -> str: - """Absolute URLs are used to simplify logic for applications that - won't be served by browsers, such as rendering GCM notifications.""" - return urllib.parse.urljoin(user_profile.realm.uri, avatar_url(user_profile)) + """ + Absolute URLs are used to simplify logic for applications that + won't be served by browsers, such as rendering GCM notifications. + """ + avatar = avatar_url(user_profile) + # avatar_url can return None if client_gravatar=True, however here we use the default value of False + assert avatar is not None + return urllib.parse.urljoin(user_profile.realm.uri, avatar) diff --git a/zerver/tests/test_upload.py b/zerver/tests/test_upload.py index 3d4fdcd61b..57167fa992 100644 --- a/zerver/tests/test_upload.py +++ b/zerver/tests/test_upload.py @@ -728,12 +728,12 @@ class AvatarTest(UploadSerializeMixin, ZulipTestCase): with self.settings(ENABLE_GRAVATAR=True): response = self.client_get("/avatar/cordelia@zulip.com?foo=bar") redirect_url = response['Location'] - self.assertEqual(redirect_url, avatar_url(cordelia) + '&foo=bar') + self.assertEqual(redirect_url, str(avatar_url(cordelia)) + '&foo=bar') with self.settings(ENABLE_GRAVATAR=False): response = self.client_get("/avatar/cordelia@zulip.com?foo=bar") redirect_url = response['Location'] - self.assertTrue(redirect_url.endswith(avatar_url(cordelia) + '&foo=bar')) + self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia)) + '&foo=bar')) def test_get_user_avatar(self) -> None: self.login(self.example_email("hamlet")) @@ -743,11 +743,11 @@ class AvatarTest(UploadSerializeMixin, ZulipTestCase): cordelia.save() response = self.client_get("/avatar/cordelia@zulip.com?foo=bar") redirect_url = response['Location'] - self.assertTrue(redirect_url.endswith(avatar_url(cordelia) + '&foo=bar')) + self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia)) + '&foo=bar')) response = self.client_get("/avatar/%s?foo=bar" % (cordelia.id)) redirect_url = response['Location'] - self.assertTrue(redirect_url.endswith(avatar_url(cordelia) + '&foo=bar')) + self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia)) + '&foo=bar')) response = self.client_get("/avatar/") self.assertEqual(response.status_code, 404) @@ -760,11 +760,11 @@ class AvatarTest(UploadSerializeMixin, ZulipTestCase): cordelia.save() response = self.client_get("/avatar/cordelia@zulip.com/medium?foo=bar") redirect_url = response['Location'] - self.assertTrue(redirect_url.endswith(avatar_url(cordelia, True) + '&foo=bar')) + self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia, True)) + '&foo=bar')) response = self.client_get("/avatar/%s/medium?foo=bar" % (cordelia.id,)) redirect_url = response['Location'] - self.assertTrue(redirect_url.endswith(avatar_url(cordelia, True) + '&foo=bar')) + self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia, True)) + '&foo=bar')) def test_non_valid_user_avatar(self) -> None: