From 9eb47f108c8e9a3f186bc0b5a84d6eca816045d1 Mon Sep 17 00:00:00 2001 From: adnrs96 Date: Thu, 2 Mar 2017 20:51:46 +0530 Subject: [PATCH] Refactor: Change upload_avatar_image to accept two user profiles. In this commit we just change the upload_avatar_image function to accept two user_profiles acting_user_profile and target_user_profile. Basically email param is dropped for a target_user_profile so that avatar's could be moved lateron to user id based storage. --- zerver/lib/upload.py | 28 +++++++++---------- .../commands/gravatar_to_user_avatar.py | 20 +++++++------ zerver/views/user_settings.py | 2 +- zerver/views/users.py | 7 +++-- 4 files changed, 30 insertions(+), 27 deletions(-) diff --git a/zerver/lib/upload.py b/zerver/lib/upload.py index 81efcbe757..47a5022030 100644 --- a/zerver/lib/upload.py +++ b/zerver/lib/upload.py @@ -103,8 +103,8 @@ class ZulipUploadBackend(object): # type: (Text, int, Optional[Text], binary_type, UserProfile, Optional[Realm]) -> Text raise NotImplementedError() - def upload_avatar_image(self, user_file, user_profile, email): - # type: (File, UserProfile, Text) -> None + def upload_avatar_image(self, user_file, acting_user_profile, target_user_profile): + # type: (File, UserProfile, UserProfile) -> None raise NotImplementedError() def delete_message_image(self, path_id): @@ -239,18 +239,18 @@ class S3UploadBackend(ZulipUploadBackend): logging.warning("%s does not exist. Its entry in the database will be removed." % (file_name,)) return False - def upload_avatar_image(self, user_file, user_profile, email): - # type: (File, UserProfile, Text) -> None + def upload_avatar_image(self, user_file, acting_user_profile, target_user_profile): + # type: (File, UserProfile, UserProfile) -> None content_type = guess_type(user_file.name)[0] bucket_name = settings.S3_AVATAR_BUCKET - s3_file_name = user_avatar_hash(email) + s3_file_name = user_avatar_hash(target_user_profile.email) image_data = user_file.read() upload_image_to_s3( bucket_name, s3_file_name + ".original", content_type, - user_profile, + target_user_profile, image_data, ) @@ -260,7 +260,7 @@ class S3UploadBackend(ZulipUploadBackend): bucket_name, s3_file_name + "-medium.png", "image/png", - user_profile, + target_user_profile, resized_medium ) @@ -269,7 +269,7 @@ class S3UploadBackend(ZulipUploadBackend): bucket_name, s3_file_name, 'image/png', - user_profile, + target_user_profile, resized_data, ) # See avatar_url in avatar.py for URL. (That code also handles the case @@ -386,9 +386,9 @@ class LocalUploadBackend(ZulipUploadBackend): logging.warning("%s does not exist. Its entry in the database will be removed." % (file_name,)) return False - def upload_avatar_image(self, user_file, user_profile, email): - # type: (File, UserProfile, Text) -> None - email_hash = user_avatar_hash(email) + def upload_avatar_image(self, user_file, acting_user_profile, target_user_profile): + # type: (File, UserProfile, UserProfile) -> None + email_hash = user_avatar_hash(target_user_profile.email) image_data = user_file.read() write_local_file('avatars', email_hash+'.original', image_data) @@ -446,9 +446,9 @@ def delete_message_image(path_id): # type: (Text) -> bool return upload_backend.delete_message_image(path_id) -def upload_avatar_image(user_file, user_profile, email): - # type: (File, UserProfile, Text) -> None - upload_backend.upload_avatar_image(user_file, user_profile, email) +def upload_avatar_image(user_file, acting_user_profile, target_user_profile): + # type: (File, UserProfile, UserProfile) -> None + upload_backend.upload_avatar_image(user_file, acting_user_profile, target_user_profile) def upload_icon_image(user_file, user_profile): # type: (File, UserProfile) -> None diff --git a/zerver/management/commands/gravatar_to_user_avatar.py b/zerver/management/commands/gravatar_to_user_avatar.py index 7011efe2dd..58f4013c45 100644 --- a/zerver/management/commands/gravatar_to_user_avatar.py +++ b/zerver/management/commands/gravatar_to_user_avatar.py @@ -37,16 +37,18 @@ for both email addresses.""" try: user_profile = get_user_profile_by_email(old_email) + upload_avatar_image(gravatar_file, user_profile, user_profile) + user_profile.avatar_source = UserProfile.AVATAR_FROM_USER + user_profile.save(update_fields=['avatar_source']) except UserProfile.DoesNotExist: - try: - user_profile = get_user_profile_by_email(new_email) - except UserProfile.DoesNotExist: - raise CommandError("Could not find specified user") + raise CommandError("Could not find specified user for email %s" % (old_email)) - upload_avatar_image(gravatar_file, user_profile, old_email) if old_email != new_email: gravatar_file.seek(0) - upload_avatar_image(gravatar_file, user_profile, new_email) - - user_profile.avatar_source = UserProfile.AVATAR_FROM_USER - user_profile.save(update_fields=['avatar_source']) + try: + user_profile = get_user_profile_by_email(new_email) + upload_avatar_image(gravatar_file, user_profile, user_profile) + user_profile.avatar_source = UserProfile.AVATAR_FROM_USER + user_profile.save(update_fields=['avatar_source']) + except UserProfile.DoesNotExist: + raise CommandError("Could not find specified user for email %s" % (new_email)) diff --git a/zerver/views/user_settings.py b/zerver/views/user_settings.py index 8a268c14a5..5ac217867e 100644 --- a/zerver/views/user_settings.py +++ b/zerver/views/user_settings.py @@ -260,7 +260,7 @@ def set_avatar_backend(request, user_profile): return json_error(_("You must upload exactly one avatar.")) user_file = list(request.FILES.values())[0] - upload_avatar_image(user_file, user_profile, user_profile.email) + upload_avatar_image(user_file, user_profile, user_profile) do_change_avatar_fields(user_profile, UserProfile.AVATAR_FROM_USER) user_avatar_url = avatar_url(user_profile) diff --git a/zerver/views/users.py b/zerver/views/users.py index bdf1dd49e9..2fcf63072e 100644 --- a/zerver/views/users.py +++ b/zerver/views/users.py @@ -192,7 +192,7 @@ def patch_bot_backend(request, user_profile, email, pass elif len(request.FILES) == 1: user_file = list(request.FILES.values())[0] - upload_avatar_image(user_file, user_profile, bot.email) + upload_avatar_image(user_file, user_profile, bot) avatar_source = UserProfile.AVATAR_FROM_USER do_change_avatar_fields(bot, avatar_source) else: @@ -255,8 +255,6 @@ def add_bot_backend(request, user_profile, full_name_raw=REQ("full_name"), short elif len(request.FILES) != 1: return json_error(_("You may only upload one file at a time")) else: - user_file = list(request.FILES.values())[0] - upload_avatar_image(user_file, user_profile, email) avatar_source = UserProfile.AVATAR_FROM_USER default_sending_stream = None @@ -278,6 +276,9 @@ def add_bot_backend(request, user_profile, full_name_raw=REQ("full_name"), short default_sending_stream=default_sending_stream, default_events_register_stream=default_events_register_stream, default_all_public_streams=default_all_public_streams) + if len(request.FILES) == 1: + user_file = list(request.FILES.values())[0] + upload_avatar_image(user_file, user_profile, bot_profile) json_result = dict( api_key=bot_profile.api_key, avatar_url=avatar_url(bot_profile),