From bd80c048be1b0196b369a49ac9e1d629beef064f Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Tue, 28 Feb 2023 02:49:04 +0000 Subject: [PATCH] upload: Rename delete_message_image to use word "attachment". The table is named Attachment, and not all of them are images. --- zerver/actions/uploads.py | 6 +++--- zerver/lib/attachments.py | 4 ++-- zerver/lib/upload/__init__.py | 4 ++-- zerver/lib/upload/base.py | 2 +- zerver/lib/upload/local.py | 2 +- zerver/lib/upload/s3.py | 2 +- zerver/tests/test_attachments.py | 6 ++++-- zerver/tests/test_upload.py | 16 ++++++++-------- 8 files changed, 22 insertions(+), 20 deletions(-) diff --git a/zerver/actions/uploads.py b/zerver/actions/uploads.py index df8fd5ae15..756f2e119e 100644 --- a/zerver/actions/uploads.py +++ b/zerver/actions/uploads.py @@ -2,7 +2,7 @@ import logging from typing import Any, Dict, List from zerver.lib.markdown import MessageRenderingResult -from zerver.lib.upload import claim_attachment, delete_message_image +from zerver.lib.upload import claim_attachment, delete_message_attachment from zerver.models import ( Attachment, Message, @@ -69,10 +69,10 @@ def do_delete_old_unclaimed_attachments(weeks_ago: int) -> None: ) for attachment in old_unclaimed_attachments: - delete_message_image(attachment.path_id) + delete_message_attachment(attachment.path_id) attachment.delete() for archived_attachment in old_unclaimed_archived_attachments: - delete_message_image(archived_attachment.path_id) + delete_message_attachment(archived_attachment.path_id) archived_attachment.delete() diff --git a/zerver/lib/attachments.py b/zerver/lib/attachments.py index 66ba50f969..e7717e9161 100644 --- a/zerver/lib/attachments.py +++ b/zerver/lib/attachments.py @@ -3,7 +3,7 @@ from typing import Any, Dict, List from django.utils.translation import gettext as _ from zerver.lib.exceptions import JsonableError -from zerver.lib.upload import delete_message_image +from zerver.lib.upload import delete_message_attachment from zerver.models import Attachment, UserProfile @@ -27,7 +27,7 @@ def access_attachment_by_id( def remove_attachment(user_profile: UserProfile, attachment: Attachment) -> None: try: - delete_message_image(attachment.path_id) + delete_message_attachment(attachment.path_id) except Exception: raise JsonableError( _("An error occurred while deleting the attachment. Please try again later.") diff --git a/zerver/lib/upload/__init__.py b/zerver/lib/upload/__init__.py index ab1674a868..a70d6ac068 100644 --- a/zerver/lib/upload/__init__.py +++ b/zerver/lib/upload/__init__.py @@ -62,8 +62,8 @@ def get_public_upload_root_url() -> str: return upload_backend.get_public_upload_root_url() -def delete_message_image(path_id: str) -> bool: - return upload_backend.delete_message_image(path_id) +def delete_message_attachment(path_id: str) -> bool: + return upload_backend.delete_message_attachment(path_id) def get_avatar_url(hash_key: str, medium: bool = False) -> str: diff --git a/zerver/lib/upload/base.py b/zerver/lib/upload/base.py index eea053035f..762c6ad5b4 100644 --- a/zerver/lib/upload/base.py +++ b/zerver/lib/upload/base.py @@ -214,7 +214,7 @@ class ZulipUploadBackend: def delete_avatar_image(self, user: UserProfile) -> None: raise NotImplementedError - def delete_message_image(self, path_id: str) -> bool: + def delete_message_attachment(self, path_id: str) -> bool: raise NotImplementedError def get_avatar_url(self, hash_key: str, medium: bool = False) -> str: diff --git a/zerver/lib/upload/local.py b/zerver/lib/upload/local.py index 0cba498d24..edc47bbdc5 100644 --- a/zerver/lib/upload/local.py +++ b/zerver/lib/upload/local.py @@ -96,7 +96,7 @@ class LocalUploadBackend(ZulipUploadBackend): create_attachment(uploaded_file_name, path, user_profile, target_realm, uploaded_file_size) return "/user_uploads/" + path - def delete_message_image(self, path_id: str) -> bool: + def delete_message_attachment(self, path_id: str) -> bool: return delete_local_file("files", path_id) def write_avatar_images(self, file_path: str, image_data: bytes) -> None: diff --git a/zerver/lib/upload/s3.py b/zerver/lib/upload/s3.py index 7818d22be8..a0752e2fe7 100644 --- a/zerver/lib/upload/s3.py +++ b/zerver/lib/upload/s3.py @@ -230,7 +230,7 @@ class S3UploadBackend(ZulipUploadBackend): ) return url - def delete_message_image(self, path_id: str) -> bool: + def delete_message_attachment(self, path_id: str) -> bool: return self.delete_file_from_s3(path_id, self.uploads_bucket) def write_avatar_images( diff --git a/zerver/tests/test_attachments.py b/zerver/tests/test_attachments.py index 872143af27..86c0630b73 100644 --- a/zerver/tests/test_attachments.py +++ b/zerver/tests/test_attachments.py @@ -29,13 +29,15 @@ class AttachmentsTests(ZulipTestCase): def test_remove_attachment_exception(self) -> None: user_profile = self.example_user("cordelia") self.login_user(user_profile) - with mock.patch("zerver.lib.attachments.delete_message_image", side_effect=Exception()): + with mock.patch( + "zerver.lib.attachments.delete_message_attachment", side_effect=Exception() + ): result = self.client_delete(f"/json/attachments/{self.attachment.id}") self.assert_json_error( result, "An error occurred while deleting the attachment. Please try again later." ) - @mock.patch("zerver.lib.attachments.delete_message_image") + @mock.patch("zerver.lib.attachments.delete_message_attachment") def test_remove_attachment(self, ignored: Any) -> None: user_profile = self.example_user("cordelia") self.login_user(user_profile) diff --git a/zerver/tests/test_upload.py b/zerver/tests/test_upload.py index 13cd08de27..f7624e3cd8 100644 --- a/zerver/tests/test_upload.py +++ b/zerver/tests/test_upload.py @@ -47,7 +47,7 @@ from zerver.lib.test_helpers import ( ) from zerver.lib.upload import ( delete_export_tarball, - delete_message_image, + delete_message_attachment, upload_emoji_image, upload_export_tarball, upload_message_attachment, @@ -439,7 +439,7 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase): do_delete_old_unclaimed_attachments(2) self.assertTrue(not Attachment.objects.filter(path_id=d2_path_id).exists()) with self.assertLogs(level="WARNING") as warn_log: - self.assertTrue(not delete_message_image(d2_path_id)) + self.assertTrue(not delete_message_attachment(d2_path_id)) self.assertEqual( warn_log.output, ["WARNING:root:dummy_2.txt does not exist. Its entry in the database will be removed."], @@ -1878,7 +1878,7 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase): # Ensure the correct realm id of the target realm is used instead of the bot's realm. self.assertTrue(uri.startswith(f"/user_uploads/{zulip_realm.id}/")) - def test_delete_message_image_local(self) -> None: + def test_delete_message_attachment_local(self) -> None: self.login("hamlet") fp = StringIO("zulip!") fp.name = "zulip.txt" @@ -1886,7 +1886,7 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase): response_dict = self.assert_json_success(result) path_id = re.sub("/user_uploads/", "", response_dict["uri"]) - self.assertTrue(delete_message_image(path_id)) + self.assertTrue(delete_message_attachment(path_id)) def test_avatar_url_local(self) -> None: self.login("hamlet") @@ -2092,7 +2092,7 @@ class S3Test(ZulipTestCase): self.assert_length(b"zulip!", uploaded_file.size) @use_s3_backend - def test_message_image_delete_s3(self) -> None: + def test_delete_message_attachment_s3(self) -> None: create_s3_buckets(settings.S3_AUTH_UPLOADS_BUCKET) user_profile = self.example_user("hamlet") @@ -2101,12 +2101,12 @@ class S3Test(ZulipTestCase): ) path_id = re.sub("/user_uploads/", "", uri) - self.assertTrue(delete_message_image(path_id)) + self.assertTrue(delete_message_attachment(path_id)) @use_s3_backend - def test_message_image_delete_when_file_doesnt_exist(self) -> None: + def test_delete_message_attachment_when_file_doesnt_exist(self) -> None: with self.assertLogs(level="WARNING") as warn_log: - self.assertEqual(False, delete_message_image("non-existent-file")) + self.assertEqual(False, delete_message_attachment("non-existent-file")) self.assertEqual( warn_log.output, [