Delete old user avatar images on upload

misago/users/avatars/store.py

@@ -18,13 +18,13 @@ def normalize_image(image):
 
 def delete_avatar(user):
     if user.avatar_tmp:
-        user.avatar_tmp.delete(False)
+        user.avatar_tmp.delete(save=False)
 
     if user.avatar_src:
-        user.avatar_src.delete(False)
+        user.avatar_src.delete(save=False)
 
     for avatar in user.avatar_set.all():
-        avatar.image.delete(False)
+        avatar.image.delete(save=False)
     user.avatar_set.all().delete()
 
 
@@ -50,6 +50,13 @@ def store_avatar(user, image):
     user.avatars = [{'size': a.size, 'url': a.url} for a in avatars]
     user.save(update_fields=['avatars'])
 
+    # delete old user avatar images
+    new_avatars_ids = [i.id for i in avatars]
+    old_avatars_queryset = user.avatar_set.exclude(id__in=new_avatars_ids)
+    for avatar in old_avatars_queryset:
+        avatar.image.delete(save=False)
+    old_avatars_queryset.delete()
+
 
 def store_new_avatar(user, image):
     delete_avatar(user)
@@ -63,7 +70,7 @@ def store_temporary_avatar(user, image):
     image.save(image_stream, "PNG")
 
     if user.avatar_tmp:
-        user.avatar_tmp.delete(False)
+        user.avatar_tmp.delete(save=False)
 
     user.avatar_tmp = ContentFile(image_stream.getvalue(), 'avatar')
     user.save(update_fields=['avatar_tmp'])
@@ -71,7 +78,7 @@ def store_temporary_avatar(user, image):
 
 def store_original_avatar(user):
     if user.avatar_src:
-        user.avatar_src.delete(False)
+        user.avatar_src.delete(save=False)
     user.avatar_src = user.avatar_tmp
     user.avatar_tmp = None
     user.save(update_fields=['avatar_tmp', 'avatar_src'])

misago/users/tests/test_user_avatar_api.py

@@ -6,6 +6,7 @@ from path import Path
 from django.contrib.auth import get_user_model
 
 from misago.acl.testutils import override_acl
+from misago.conf import settings
 from misago.users.avatars import gallery, store
 from misago.users.models import AvatarGallery
 from misago.users.testutils import AuthenticatedUserTestCase
@@ -23,10 +24,16 @@ class UserAvatarTests(AuthenticatedUserTestCase):
     def setUp(self):
         super(UserAvatarTests, self).setUp()
         self.link = '/api/users/%s/avatar/' % self.user.pk
+        self.client.post(self.link, data={'avatar': 'generated'})
 
     def get_current_user(self):
         return UserModel.objects.get(pk=self.user.pk)
 
+    def assertOldAvatarsAreDeleted(self, user):
+        self.assertEqual(
+            user.avatar_set.count(), len(settings.MISAGO_AVATARS_SIZES)
+        )
+
     def test_avatars_off(self):
         """custom avatars are not allowed"""
         with self.settings(allow_custom_avatars=False):
@@ -109,16 +116,17 @@ class UserAvatarTests(AuthenticatedUserTestCase):
         response = self.client.post(self.link, data={'avatar': 'gravatar'})
         self.assertContains(response, "Gravatar was downloaded and set")
 
+        self.assertOldAvatarsAreDeleted(self.user)
+
     def test_generation_request(self):
         """generated avatar is set"""
         response = self.client.post(self.link, data={'avatar': 'generated'})
         self.assertContains(response, "New avatar based on your account")
 
+        self.assertOldAvatarsAreDeleted(self.user)
+
     def test_avatar_upload_and_crop(self):
         """avatar can be uploaded and cropped"""
-        response = self.client.post(self.link, data={'avatar': 'generated'})
-        self.assertEqual(response.status_code, 200)
-
         response = self.client.post(self.link, data={'avatar': 'upload'})
         self.assertContains(response, "No file was sent.", status_code=400)
 
@@ -156,6 +164,7 @@ class UserAvatarTests(AuthenticatedUserTestCase):
         self.assertContains(response, "Uploaded avatar was set.")
 
         self.assertFalse(self.get_current_user().avatar_tmp)
+        self.assertOldAvatarsAreDeleted(self.user)
 
         avatar = Path(self.get_current_user().avatar_src.path)
         self.assertTrue(avatar.exists())
@@ -192,6 +201,7 @@ class UserAvatarTests(AuthenticatedUserTestCase):
             content_type="application/json",
         )
         self.assertContains(response, "Avatar was re-cropped.")
+        self.assertOldAvatarsAreDeleted(self.user)
 
         # delete user avatars, test if it deletes src and tmp
         store.delete_avatar(self.get_current_user())
@@ -280,6 +290,7 @@ class UserAvatarTests(AuthenticatedUserTestCase):
         )
 
         self.assertContains(response, "Avatar from gallery was set.")
+        self.assertOldAvatarsAreDeleted(self.user)
 
 
 class UserAvatarModerationTests(AuthenticatedUserTestCase):