fix: re-uploading the same avatar doesn't delete it (#12823)

Fixes #12822.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12823
Reviewed-by: Gusted <gusted@noreply.codeberg.org>

services/user/avatar.go

@@ -35,15 +35,15 @@ func UploadAvatar(ctx context.Context, u *user_model.User, data []byte) error {
 		return fmt.Errorf("updateUser: %w", err)
 	}
 
-	if err := avatarstore.StoreAvatar(u.CustomAvatarRelativePath(), avatarData, img, storage.Avatars); err != nil {
-		return fmt.Errorf("Failed to store avatar at %s: %w", u.CustomAvatarRelativePath(), err)
-	}
 	if len(previousAvatar) > 0 {
 		err := avatarstore.DeleteAvatar(previousAvatar, storage.Avatars)
 		if err != nil {
 			return err
 		}
 	}
+	if err := avatarstore.StoreAvatar(u.CustomAvatarRelativePath(), avatarData, img, storage.Avatars); err != nil {
+		return fmt.Errorf("Failed to store avatar at %s: %w", u.CustomAvatarRelativePath(), err)
+	}
 
 	return committer.Commit()
 }

services/user/avatar_test.go

@@ -136,4 +136,26 @@ func TestUserReplaceAvatar(t *testing.T) {
 		require.Error(t, err)
 		assert.Nil(t, fi)
 	})
+
+	t.Run("ReplaceAvatarByItself", func(t *testing.T) {
+		require.NoError(t, unittest.PrepareTestDatabase())
+		user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+
+		err := UploadAvatar(db.DefaultContext, user, firstBuff.Bytes())
+		require.NoError(t, err)
+		user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+		firstImageHash := user.Avatar
+		assert.NotEmpty(t, firstImageHash)
+
+		err = UploadAvatar(db.DefaultContext, user, firstBuff.Bytes())
+		require.NoError(t, err)
+		user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+		secondImageHash := user.Avatar
+		assert.Equal(t, firstImageHash, secondImageHash)
+
+		// The avatar is still available in storage
+		fi, err := storage.Avatars.Stat(firstImageHash)
+		require.NoError(t, err)
+		assert.NotNil(t, fi)
+	})
 }