soft-block versions on delete (#23768)

* soft-block versions on delete

* refactor block_activity_log_save as submission_obj is not optional

Author: eviljeff

docs/topics/api/addons.rst

@@ -416,6 +416,7 @@ Delete
 This endpoint allows an add-on to be deleted.
 Because add-on deletion is an irreversible and destructive action an additional token must be retrieved beforehand, and passed as a parameter to the delete endpoint.
 Deleting the add-on will permanently delete all versions and files submitted for this add-on, listed or not.
+All versions will be soft-blocked (restricted), which will disable and prevent any further installation in Firefox. Existing users can choose to re-enable the add-on.
 The add-on ID (``guid``) cannot be restored and will forever be unusable for submission.
 
     .. note::
@@ -736,7 +737,7 @@ Version Delete
 
 .. _version-delete:
 
-This endpoint allows a version to be deleted.
+This endpoint allows a version to be deleted. The version will be soft-blocked (restricted), which will disable and prevent any further installation in Firefox. Existing users can choose to re-enable the add-on.
 
     .. note::
         This API requires :doc:`authentication <auth>`, and for the user to be an author of the add-on.

src/olympia/addons/models.py

@@ -844,7 +844,7 @@ def _prepare_deletion_email(self, msg, reason):
         return subject, email_msg
 
     @transaction.atomic
-    def delete(self, msg='', reason='', send_delete_email=True):
+    def delete(self, *, msg='', reason='', send_delete_email=True):
         # To avoid a circular import
         from olympia.versions import tasks as version_tasks
 
@@ -919,6 +919,16 @@ def delete(self, msg='', reason='', send_delete_email=True):
 
             if send_delete_email:
                 send_mail(subject, email_msg, recipient_list=email_to)
+
+            all_versions = list(
+                self.versions(manager='unfiltered_for_relations').values_list(
+                    'id', flat=True
+                )
+            )
+            if all_versions:
+                version_tasks.soft_block_versions.delay(
+                    version_ids=all_versions, reason='Addon deleted'
+                )
         else:
             # Real deletion path.
             super().delete()

src/olympia/addons/tests/test_models.py

@@ -41,7 +41,7 @@
 from olympia.amo.tests.test_models import BasePreviewMixin
 from olympia.applications.models import AppVersion
 from olympia.bandwagon.models import Collection
-from olympia.blocklist.models import Block, BlocklistSubmission
+from olympia.blocklist.models import Block, BlocklistSubmission, BlockType, BlockVersion
 from olympia.constants.categories import CATEGORIES
 from olympia.constants.promoted import PROMOTED_GROUP_CHOICES
 from olympia.devhub.models import RssKey
@@ -598,7 +598,7 @@ def _delete(self, addon_id):
         addon_count = Addon.unfiltered.count()
         addon = Addon.objects.get(pk=addon_id)
         guid = addon.guid
-        addon.delete('bye')
+        addon.delete(msg='bye')
         assert addon_count == Addon.unfiltered.count()  # Soft deletion.
         assert addon.status == amo.STATUS_DELETED
         assert addon.slug is None
@@ -686,7 +686,7 @@ def test_delete_incomplete_no_versions(self):
         # The addon status will have been changed when we deleted the version,
         # and the instance should be the same, so we shouldn't need to reload.
         assert addon.status == amo.STATUS_NULL
-        addon.delete(None)
+        addon.delete(msg=None)
         assert len(mail.outbox) == 0
         assert Addon.unfiltered.count() == (count - 1)
 
@@ -696,7 +696,7 @@ def test_delete_incomplete_with_versions(self):
         a = Addon.objects.get(pk=3615)
         a.status = 0
         a.save()
-        a.delete('oh looky here')
+        a.delete(msg='oh looky here')
         assert len(mail.outbox) == 1
         assert count == Addon.unfiltered.count()
 
@@ -1331,7 +1331,7 @@ def test_slug_isdenied(self):
     def delete(self):
         addon = Addon.objects.get(id=3615)
         assert len(mail.outbox) == 0
-        addon.delete('so long and thanks for all the fish')
+        addon.delete(msg='so long and thanks for all the fish')
         assert len(mail.outbox) == 1
 
     def test_delete_to(self):
@@ -2620,6 +2620,9 @@ def test_update_all_due_dates(self):
 
 
 class TestAddonDelete(TestCase):
+    def setUp(self):
+        user_factory(pk=settings.TASK_USER_ID)
+
     def test_cascades(self):
         addon = Addon.objects.create(type=amo.ADDON_EXTENSION)
 
@@ -2667,6 +2670,34 @@ def test_delete_with_deleted_versions(self):
         addon.delete()
         assert Addon.unfiltered.filter(pk=addon.pk).exists()
 
+    def test_delete_soft_blocks_all_versions(self):
+        developer = user_factory()
+        addon = addon_factory(users=[developer])
+        version = addon.current_version
+        deleted_version = version_factory(addon=addon, deleted=True)
+        hard_blocked_version = version_factory(
+            addon=addon, file_kw={'status': amo.STATUS_DISABLED}
+        )
+        block = Block.objects.create(guid=addon.guid, updated_by=developer)
+        BlockVersion.objects.create(block=block, version=hard_blocked_version)
+
+        addon.delete(send_delete_email=False)
+
+        assert Block.objects.count() == 1
+        assert set(block.blockversion_set.values_list('version', flat=True)) == {
+            version.id,
+            deleted_version.id,
+            hard_blocked_version.id,
+        }
+        assert (
+            block.blockversion_set.filter(block_type=BlockType.SOFT_BLOCKED).count()
+            == 2
+        )
+        assert (
+            hard_blocked_version.blockversion.reload().block_type == BlockType.BLOCKED
+        )
+        assert len(mail.outbox) == 0
+
 
 class TestUpdateStatus(TestCase):
     def test_no_file_ends_with_NULL(self):

src/olympia/blocklist/tests/test_admin.py

@@ -379,8 +379,13 @@ def test_initial_values_from_add_from_addon_pk_view(self):
 
         addon = addon_factory(guid='guid@')
         ver = addon.current_version
-        ver_deleted = version_factory(addon=addon, channel=amo.CHANNEL_UNLISTED)
-        ver_deleted.delete()  # shouldn't affect it's inclusion in the choices
+        # being deleted shouldn't affect it's inclusion in the choices
+        ver_deleted = version_factory(
+            addon=addon,
+            channel=amo.CHANNEL_UNLISTED,
+            deleted=True,
+            file_kw={'status': amo.STATUS_DISABLED},
+        )
         ver_unlisted = version_factory(addon=addon, channel=amo.CHANNEL_UNLISTED)
         # these next two versions shouldn't be possible choices
         ver_add_subm = version_factory(addon=addon)
@@ -444,8 +449,13 @@ def test_version_checkboxes(self):
 
         addon = addon_factory(guid='guid@', average_daily_users=100)
         ver = addon.current_version
-        ver_deleted = version_factory(addon=addon, channel=amo.CHANNEL_UNLISTED)
-        ver_deleted.delete()  # shouldn't affect it's status
+        # being deleted shouldn't affect it's status
+        ver_deleted = version_factory(
+            addon=addon,
+            channel=amo.CHANNEL_UNLISTED,
+            deleted=True,
+            file_kw={'status': amo.STATUS_DISABLED},
+        )
         # these next three versions shouldn't be possible choices
         ver_add_subm = version_factory(addon=addon)
         add_submission = BlocklistSubmission.objects.create(
@@ -515,8 +525,12 @@ def test_version_checkboxes_hardening_action(self):
         self.client.force_login(user)
 
         addon = addon_factory(guid='guid@', average_daily_users=100)
-        ver_deleted = version_factory(addon=addon, channel=amo.CHANNEL_UNLISTED)
-        ver_deleted.delete()  # shouldn't affect it's status
+        version_factory(
+            addon=addon,
+            channel=amo.CHANNEL_UNLISTED,
+            deleted=True,
+            file_kw={'status': amo.STATUS_DISABLED},
+        )
         # these next three versions shouldn't be possible choices
         ver_add_subm = version_factory(addon=addon)
         add_submission = BlocklistSubmission.objects.create(
@@ -567,8 +581,12 @@ def test_version_checkboxes_softening_action(self):
         self.client.force_login(user)
 
         addon = addon_factory(guid='guid@', average_daily_users=100)
-        ver_deleted = version_factory(addon=addon, channel=amo.CHANNEL_UNLISTED)
-        ver_deleted.delete()  # shouldn't affect it's status
+        version_factory(
+            addon=addon,
+            channel=amo.CHANNEL_UNLISTED,
+            deleted=True,
+            file_kw={'status': amo.STATUS_DISABLED},
+        )
         # these next three versions shouldn't be possible choices
         ver_add_subm = version_factory(addon=addon)
         add_submission = BlocklistSubmission.objects.create(
@@ -621,7 +639,8 @@ def test_add_single(self):
         deleted_addon = addon_factory(version_kw={'version': '1.2.5'})
         deleted_addon_version = deleted_addon.current_version
         NeedsHumanReview.objects.create(version=deleted_addon_version)
-        deleted_addon.delete()
+        deleted_addon.update(status=amo.STATUS_DELETED)
+        deleted_addon_version.update(deleted=True)
         deleted_addon.addonguid.update(guid='guid@')
         addon = addon_factory(
             guid='guid@', name='Danger Danger', version_kw={'version': '1.2a'}
@@ -2098,8 +2117,9 @@ def test_blocked_deleted_keeps_addon_status(self):
         deleted_addon = addon_factory(guid='guid@', version_kw={'version': '1.2.5'})
         version = deleted_addon.current_version
         NeedsHumanReview.objects.create(version=version)
-        deleted_addon.delete()
-        assert deleted_addon.status == amo.STATUS_DELETED
+        deleted_addon.update(status=amo.STATUS_DELETED)
+        version.update(deleted=True)
+        version.file.update(status=amo.STATUS_DISABLED)
         assert not DeniedGuid.objects.filter(guid=deleted_addon.guid).exists()
 
         response = self.client.get(self.submission_url + '?guids=guid@', follow=True)
@@ -2145,8 +2165,8 @@ def test_blocking_addon_guid_already_denied(self):
 
         deleted_addon = addon_factory(guid='guid@', version_kw={'version': '1.2.5'})
         version = deleted_addon.current_version
-        deleted_addon.delete()
-        assert deleted_addon.status == amo.STATUS_DELETED
+        deleted_addon.update(status=amo.STATUS_DELETED)
+        version.update(deleted=True)
         deleted_addon.deny_resubmission()
         assert DeniedGuid.objects.filter(guid=deleted_addon.guid).exists()
 
@@ -2737,8 +2757,7 @@ def test_version_checkboxes(self):
             changed_version_ids=[ver_del_subm.id],
             action=BlocklistSubmission.ACTIONS.DELETE,
         )
-        ver_deleted = version_factory(addon=other_addon)
-        ver_deleted.delete()  # shouldn't affect it's status
+        ver_deleted = version_factory(addon=other_addon, deleted=True)
         ver_block = version_factory(addon=other_addon)
         ver_soft_block = version_factory(addon=other_addon)
         block_factory(

src/olympia/blocklist/utils.py

@@ -10,11 +10,7 @@
 log = olympia.core.logger.getLogger('z.amo.blocklist')
 
 
-def block_activity_log_save(
-    obj,
-    change,
-    submission_obj=None,
-):
+def block_activity_log_save(obj, *, change, submission_obj):
     from .models import BlockType
 
     action = amo.LOG.BLOCKLIST_BLOCK_EDITED if change else amo.LOG.BLOCKLIST_BLOCK_ADDED
@@ -23,11 +19,9 @@ def block_activity_log_save(
     blocked_versions = sorted(
         ver.version for ver in obj.addon_versions if ver.is_blocked
     )
-    changed_version_ids = (
-        [v_id for v_id in submission_obj.changed_version_ids if v_id in addon_versions]
-        if submission_obj
-        else sorted(ver.id for ver in obj.addon_versions if ver.is_blocked)
-    )
+    changed_version_ids = [
+        v_id for v_id in submission_obj.changed_version_ids if v_id in addon_versions
+    ]
     changed_versions = sorted(addon_versions[ver_id] for ver_id in changed_version_ids)
 
     details = {
@@ -39,15 +33,15 @@ def block_activity_log_save(
         'comments': f'{len(changed_versions)} versions added to block; '
         f'{len(blocked_versions)} total versions now blocked.',
     }
-    if submission_obj:
-        details['signoff_state'] = submission_obj.SIGNOFF_STATES.for_value(
-            submission_obj.signoff_state
-        ).display
-        if submission_obj.signoff_by:
-            details['signoff_by'] = submission_obj.signoff_by.id
-        details['block_type'] = submission_obj.block_type
-        if submission_obj.block_type == BlockType.SOFT_BLOCKED:
-            action_version = amo.LOG.BLOCKLIST_VERSION_SOFT_BLOCKED
+
+    details['signoff_state'] = submission_obj.SIGNOFF_STATES.for_value(
+        submission_obj.signoff_state
+    ).display
+    if submission_obj.signoff_by:
+        details['signoff_by'] = submission_obj.signoff_by.id
+    details['block_type'] = submission_obj.block_type
+    if submission_obj.block_type == BlockType.SOFT_BLOCKED:
+        action_version = amo.LOG.BLOCKLIST_VERSION_SOFT_BLOCKED
 
     log_create(action, obj.addon, obj.guid, obj, details=details, user=obj.updated_by)
     log_create(
@@ -58,7 +52,7 @@ def block_activity_log_save(
         user=obj.updated_by,
     )
 
-    if submission_obj and submission_obj.signoff_by:
+    if submission_obj.signoff_by:
         log_create(
             amo.LOG.BLOCKLIST_SIGNOFF,
             obj.addon,
@@ -161,6 +155,7 @@ def disable_versions_for_block(block, submission):
         if ver.addon == block.addon
         and ver.id in submission.changed_version_ids
         and ver.file.status != amo.STATUS_DISABLED
+        and ver.deleted is False
     ]
     if versions_to_reject:
         review.set_data(
@@ -173,14 +168,15 @@ def disable_versions_for_block(block, submission):
         )
         review.auto_reject_multiple_versions()
 
-    for version in block.addon_versions:
-        # Clear active NeedsHumanReview on all blocked versions, we consider
-        # that the admin looked at them before blocking (don't limit to
-        # versions we are rejecting, which is only a subset).
-        review.clear_specific_needs_human_review_flags(version)
+    if human_review:
+        for version in block.addon_versions:
+            # Clear active NeedsHumanReview on all blocked versions, if a human has
+            # reviewed we consider that the admin looked at them before blocking (don't
+            # limit to versions we are rejecting, which is only a subset).
+            review.clear_specific_needs_human_review_flags(version)
 
 
-def save_versions_to_blocks(guids, submission):
+def save_versions_to_blocks(guids, submission, *, overwrite_block_metadata=True):
     from olympia.addons.models import GuidAlreadyDeniedError
 
     from .models import Block, BlockVersion
@@ -192,10 +188,13 @@ def save_versions_to_blocks(guids, submission):
         change = bool(block.id)
         if change:
             block.modified = modified_datetime
+            should_override_block_metadata = overwrite_block_metadata
+        else:
+            should_override_block_metadata = True
         block.updated_by = submission.updated_by
-        if submission.reason is not None:
+        if submission.reason is not None and should_override_block_metadata:
             block.reason = submission.reason
-        if submission.url is not None:
+        if submission.url is not None and should_override_block_metadata:
             block.url = submission.url
         block.average_daily_users_snapshot = block.current_adu
         # And now update the BlockVersion instances - instances to add first
@@ -230,7 +229,7 @@ def save_versions_to_blocks(guids, submission):
         block_activity_log_save(
             block,
             change=change,
-            submission_obj=submission if submission.id else None,
+            submission_obj=submission,
         )
         disable_versions_for_block(block, submission)
         if submission.disable_addon:

src/olympia/devhub/templates/devhub/addons/listing/_delete_warning.html

@@ -3,6 +3,11 @@
   have submitted for this add-on, listed or not.
 {% endtrans %}
 
+{% trans %}
+  All versions will be Restricted, which will disable and prevent any further installation in Firefox.
+  Existing users can choose to re-enable the add-on.
+{% endtrans %}
+
 {% trans %}
   The add-on ID cannot be restored and will forever be unusable for submission.
 {% endtrans %}

src/olympia/devhub/templates/devhub/versions/list.html

@@ -285,10 +285,12 @@ <h3 data-tmpl="{{ _('Delete Version {version}') }}"></h3>
         <li id="del-files"></li>
       </ul>
       <p class="highlight warning">
+      <strong>{{ _('Important:') }}</strong>
       {% trans %}
-        <strong>Important:</strong>
         Once a version has been deleted, you may not upload a new
         version with the same version number.
+        The version will also be Restricted, which will disable and prevent any further installation in Firefox.
+        Existing users can choose to re-enable the add-on.
       {% endtrans %}
       </p>
       <p>{{ _('Are you sure you wish to delete this version?') }}</p>

src/olympia/devhub/tests/test_views_versions.py

@@ -137,6 +137,9 @@ def test_delete_message(self):
         assert doc('#modal-delete p').eq(0).text() == (
             'Deleting your add-on will permanently delete all versions and '
             'files you have submitted for this add-on, listed or not. '
+            'All versions will be Restricted, which will disable and prevent any '
+            'further installation in Firefox. '
+            'Existing users can choose to re-enable the add-on. '
             'The add-on ID cannot be restored and will forever be unusable '
             'for submission.'
         )

src/olympia/lib/settings_base.py

@@ -833,6 +833,7 @@ def get_language_url_map():
     'olympia.scanners.tasks.call_mad_api': {'queue': 'devhub'},
     'olympia.scanners.tasks.run_customs': {'queue': 'devhub'},
     'olympia.scanners.tasks.run_yara': {'queue': 'devhub'},
+    'olympia.versions.tasks.soft_block_versions': {'queue': 'devhub'},
     # Crons.
     'olympia.addons.tasks.update_addon_average_daily_users': {'queue': 'cron'},
     'olympia.addons.tasks.update_addon_hotness': {'queue': 'cron'},