Stop anonymizing user data on ban (#21443)

* Stop anonymizing user data on ban

src/olympia/users/models.py

@@ -453,31 +453,31 @@ def delete_picture(self):
         if self.picture_type:
             self.update(picture_type=None)
 
-    @classmethod
-    def anonymize_users(cls, users):
+    def anonymize_user(self):
         fields = {
-            field_name: cls._meta.get_field(field_name)
-            for field_name in cls.ANONYMIZED_FIELDS
+            field_name: self._meta.get_field(field_name)
+            for field_name in self.ANONYMIZED_FIELDS
         }
-        for user in users:
-            log.info('Anonymizing username for %s', user.pk)
-            for field_name, field in fields.items():
-                setattr(user, field_name, field.get_default())
-            user.delete_picture()
+        log.info('Anonymizing user %s', self.pk)
+        for field_name, field in fields.items():
+            setattr(self, field_name, field.get_default())
+        self.delete_picture()
 
     @classmethod
     def ban_and_disable_related_content_bulk(cls, users, move_files=False):
         """Admin method to ban users and disable the content they produced.
 
         Similar to deletion, except that the content produced by the user is
         forcibly disabled instead of being deleted where possible, and the user
-        is not fully anonymized: we keep their fxa_id and email so that they
-        are never able to log back in.
+        is not anonymized: we keep their data until hard-deletion kicks in
+        (see clear_old_user_data), including fxa_id and email so that they are
+        never able to log back in.
         """
         from olympia.addons.models import Addon, AddonUser
         from olympia.addons.tasks import index_addons
         from olympia.bandwagon.models import Collection
         from olympia.ratings.models import Rating
+        from olympia.users.tasks import delete_photo
 
         # collect affected addons
         addon_ids = set(
@@ -515,18 +515,18 @@ def ban_and_disable_related_content_bulk(cls, users, move_files=False):
         ids = []
         for user in users:
             log.info(
-                'User (%s: <%s>) is being anonymized and banned.',
+                'User (%s: <%s>) is being banned.',
                 user,
                 user.email,
                 extra={'sensitive': True},
             )
             user.banned = user.modified = datetime.now()
             user.deleted = True
             ids.append(user.pk)
-        cls.anonymize_users(users)
-        cls.objects.bulk_update(
-            users, fields=('banned', 'deleted', 'modified') + cls.ANONYMIZED_FIELDS
-        )
+            # To delete their photo, avoid delete_picture() that updates
+            # picture_type immediately.
+            delete_photo.delay(user.pk)
+        cls.objects.bulk_update(users, fields=('banned', 'deleted', 'modified'))
 
     def _prepare_delete_email(self):
         site_url = settings.EXTERNAL_SITE_URL
@@ -560,7 +560,7 @@ def delete(self, addon_msg=''):
         self._delete_related_content(addon_msg=addon_msg)
         log.info('User (%s: <%s>) is being anonymized.', self, self.email)
         email = self._prepare_delete_email() if send_delete_email else None
-        self.anonymize_users((self,))
+        self.anonymize_user()
         self.deleted = True
         self.save()
         if send_delete_email:

src/olympia/users/tests/test_models.py

@@ -204,13 +204,29 @@ def test_delete_email_says_fxa_before_mza_date_and_mza_after(frozen_time, self):
 
     def test_ban_and_disable_related_content_bulk(self):
         user_sole = user_factory(
-            email='sole@foo.baa', fxa_id='13579', last_login_ip='127.0.0.1'
+            email='sole@foo.baa',
+            fxa_id='13579',
+            last_login_ip='127.0.0.1',
+            averagerating=4.4,
+            biography='ban me',
+            bypass_upload_restrictions=True,
+            location='some where',
+            occupation='some job',
+            read_dev_agreement=datetime.now(),
         )
         addon_sole = addon_factory(users=[user_sole])
         addon_sole_file = addon_sole.current_version.file
         self.setup_user_to_be_have_content_disabled(user_sole)
         user_multi = user_factory(
-            email='multi@foo.baa', fxa_id='24680', last_login_ip='127.0.0.2'
+            email='multi@foo.baa',
+            fxa_id='24680',
+            last_login_ip='127.0.0.2',
+            averagerating=2.2,
+            biography='ban me too',
+            bypass_upload_restrictions=True,
+            location='some where too',
+            occupation='some job too',
+            read_dev_agreement=datetime.now(),
         )
         innocent_user = user_factory()
         addon_multi = addon_factory(
@@ -259,13 +275,26 @@ def test_ban_and_disable_related_content_bulk(self):
         assert user_sole.auth_id
         assert user_sole.fxa_id == '13579'
         assert user_sole.last_login_ip == '127.0.0.1'
+        assert user_sole.averagerating == 4.4
+        assert user_sole.biography == 'ban me'
+        assert user_sole.bypass_upload_restrictions
+        assert user_sole.location == 'some where'
+        assert user_sole.occupation == 'some job'
+        assert user_sole.read_dev_agreement
+
         assert user_multi.deleted
         self.assertCloseToNow(user_multi.banned)
         self.assertCloseToNow(user_multi.modified)
         assert user_multi.email == 'multi@foo.baa'
         assert user_multi.auth_id
         assert user_multi.fxa_id == '24680'
         assert user_multi.last_login_ip == '127.0.0.2'
+        assert user_multi.averagerating == 2.2
+        assert user_multi.biography == 'ban me too'
+        assert user_multi.bypass_upload_restrictions
+        assert user_multi.location == 'some where too'
+        assert user_multi.occupation == 'some job too'
+        assert user_multi.read_dev_agreement
 
     def setup_user_to_be_have_content_disabled(self, user):
         addon = user.addons.last()