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

@@ -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()
 
 

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.")

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:
+def delete_message_attachment(path_id: str) -> bool:
-    return upload_backend.delete_message_image(path_id)
+    return upload_backend.delete_message_attachment(path_id)
 
 
 def get_avatar_url(hash_key: str, medium: bool = False) -> str:

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:

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:

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(

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)

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,
             [