Record version rejections triggered by a block as a human review (#22817

)

* Record version rejections triggered by a block as a human review

The user that last updated the block should be the user set on the
corresponding activity.

src/olympia/blocklist/tests/test_admin.py

@@ -218,7 +218,7 @@ def setUp(self):
         self.submission_list_url = reverse(
             'admin:blocklist_blocklistsubmission_changelist'
         )
-        self.task_user = user_factory(id=settings.TASK_USER_ID)
+        self.task_user = user_factory(id=settings.TASK_USER_ID, display_name='Mozilla')
         responses.add_callback(
             responses.POST,
             f'{settings.CINDER_SERVER_URL}create_decision',
@@ -436,15 +436,17 @@ def test_add_single(self):
         assert Block.objects.count() == 1
         block = Block.objects.first()
         assert block.addon == addon
+        assert block.updated_by == user
         # Multiple versions rejection somehow forces us to go through multiple
         # add-on status updates, it all turns out to be ok in the end though...
         logs = ActivityLog.objects.for_addons(addon)
-        assert len(logs) == 4
+        assert len(logs) == 5
         assert logs[0].action == amo.LOG.CHANGE_STATUS.id
         assert logs[1].action == amo.LOG.CHANGE_STATUS.id
-        reject_log = logs[2]
+        assert logs[2].action == amo.LOG.CHANGE_STATUS.id
+        reject_log = logs[3]
         assert reject_log.action == amo.LOG.REJECT_VERSION.id
-        block_log = logs[3]
+        block_log = logs[4]
         assert block_log.action == amo.LOG.BLOCKLIST_BLOCK_ADDED.id
         assert block_log.arguments == [addon, addon.guid, block]
         assert block_log.details['blocked_versions'] == changed_version_strs
@@ -466,6 +468,7 @@ def test_add_single(self):
                 ActivityLog.objects.for_versions(version)
             )
             assert version_reject_log == reject_log
+            assert version_reject_log.user == user
             assert version_block_log.action == amo.LOG.BLOCKLIST_VERSION_BLOCKED.id
             assert version_block_log.arguments == [*changed_versions, block]
 
@@ -497,19 +500,29 @@ def test_add_single(self):
         assert f'versions hard-blocked [{", ".join(changed_version_strs)}].' in content
 
         addon.reload()
-        first_version.file.reload()
-        second_version.file.reload()
-        pending_version.file.reload()
-        disabled_version.file.reload()
-        deleted_version.file.reload()
+        for version in [
+            first_version,
+            second_version,
+            pending_version,
+            disabled_version,
+            deleted_version,
+        ]:
+            version.reload()
+            version.file.reload()
+
         assert addon.status != amo.STATUS_DISABLED  # not 0 - * so no change
         assert first_version.file.status == amo.STATUS_DISABLED
+        self.assertCloseToNow(first_version.human_review_date)
         assert second_version.file.status == amo.STATUS_DISABLED
+        self.assertCloseToNow(second_version.human_review_date)
         assert pending_version.file.status == (
             amo.STATUS_AWAITING_REVIEW
         )  # no change because not in Block
+        assert not pending_version.human_review_date  # no change
         assert disabled_version.file.status == amo.STATUS_DISABLED  # no change
+        assert not disabled_version.human_review_date  # no change
         assert deleted_version.file.status == amo.STATUS_DISABLED  # no change
+        assert not deleted_version.human_review_date  # no change
         disabled_version.reload()
         deleted_version.reload()
         deleted_addon_version.reload()
@@ -664,7 +677,7 @@ def _test_add_multiple_verify_blocks(
 
         assert reject_log.action == amo.LOG.REJECT_VERSION.id
         assert reject_log.arguments == [new_addon, new_addon.current_version]
-        assert reject_log.user == self.task_user
+        assert reject_log.user == new_block.updated_by
         assert (
             reject_log
             == ActivityLog.objects.for_versions(new_addon.current_version).order_by(
@@ -734,7 +747,7 @@ def _test_add_multiple_verify_blocks(
 
         assert reject_log.action == amo.LOG.REJECT_VERSION.id
         assert reject_log.arguments == [partial_addon, partial_addon.current_version]
-        assert reject_log.user == self.task_user
+        assert reject_log.user == new_block.updated_by
         assert change_status_log.action == amo.LOG.CHANGE_STATUS.id
 
         existing_and_full = existing_and_full.reload()
@@ -1353,7 +1366,7 @@ def test_signoff_approve(self):
 
         assert reject_log.action == amo.LOG.REJECT_VERSION.id
         assert reject_log.arguments == [addon, version]
-        assert reject_log.user == self.task_user
+        assert reject_log.user == new_block.updated_by
         assert (
             reject_log
             == ActivityLog.objects.for_versions(addon.current_version).first()

src/olympia/blocklist/tests/test_utils.py

@@ -1,6 +1,7 @@
 import json
 import uuid
 from datetime import datetime
+from unittest import mock
 
 from django.conf import settings
 
@@ -11,7 +12,7 @@
 from olympia.amo.tests import TestCase, addon_factory, block_factory, user_factory
 
 from ..models import Block, BlocklistSubmission, BlockType
-from ..utils import datetime_to_ts, save_versions_to_blocks
+from ..utils import datetime_to_ts, disable_versions_for_block, save_versions_to_blocks
 
 
 def test_datetime_to_ts():
@@ -21,7 +22,7 @@ def test_datetime_to_ts():
 
 class TestSaveVersionsToBlocks(TestCase):
     def setUp(self):
-        self.task_user = user_factory(pk=settings.TASK_USER_ID)
+        self.task_user = user_factory(pk=settings.TASK_USER_ID, display_name='Mozilla')
         responses.add_callback(
             responses.POST,
             f'{settings.CINDER_SERVER_URL}create_decision',
@@ -75,7 +76,7 @@ def test_log_entries_new_block(self):
 
         activity = ActivityLog.objects.latest('pk')
         assert activity.action == amo.LOG.REJECT_VERSION.id
-        assert activity.user == self.task_user
+        assert activity.user == user_new
         assert activity.details['comments'] == 'some reason'
         assert activity.details['is_addon_being_blocked']
         assert activity.details['is_addon_being_disabled']
@@ -130,7 +131,7 @@ def test_log_soft_block(self):
 
         activity = ActivityLog.objects.latest('pk')
         assert activity.action == amo.LOG.REJECT_VERSION.id
-        assert activity.user == self.task_user
+        assert activity.user == user_new
         assert activity.details['comments'] == 'some reason'
         assert activity.details['is_addon_being_blocked']
         assert activity.details['is_addon_being_disabled']
@@ -212,3 +213,86 @@ def test_save_blocks_override_existing_block_type_hard_to_soft(self):
             addon.current_version.blockversion.reload().block_type
             == BlockType.SOFT_BLOCKED
         )
+
+    @mock.patch('olympia.reviewers.utils.ReviewBase')
+    def test_reviewbase_human_review_is_true_if_block_was_updated_by_human(
+        self, review_base_mock
+    ):
+        user = user_factory()
+        addon = addon_factory()
+        submission = BlocklistSubmission.objects.create(
+            input_guids=addon.guid,
+            reason='some reason',
+            url=None,
+            updated_by=user,
+            disable_addon=True,
+            block_type=BlockType.BLOCKED,
+            changed_version_ids=[addon.current_version.pk],
+            signoff_state=BlocklistSubmission.SIGNOFF_PUBLISHED,
+        )
+        block = block_factory(addon=addon, updated_by=submission.updated_by)
+        disable_versions_for_block(block, submission)
+        assert review_base_mock.call_args[0] == ()
+        assert review_base_mock.call_args[1] == {
+            'addon': addon,
+            'version': None,
+            'user': user,
+            'review_type': 'pending',
+            'human_review': True,  # True because user is a human
+        }
+
+    @mock.patch('olympia.reviewers.utils.ReviewBase')
+    def test_reviewbase_human_review_is_false_if_block_was_updated_by_none(
+        self, review_base_mock
+    ):
+        addon = addon_factory()
+        submission = BlocklistSubmission.objects.create(
+            input_guids=addon.guid,
+            reason='some reason',
+            url=None,
+            updated_by=self.task_user,
+            disable_addon=True,
+            block_type=BlockType.BLOCKED,
+            changed_version_ids=[addon.current_version.pk],
+            signoff_state=BlocklistSubmission.SIGNOFF_PUBLISHED,
+        )
+        block = block_factory(addon=addon, updated_by=submission.updated_by)
+        # We can't save a block with updated_by = None, but check that we still
+        # correctly fall back to the task user if somehow we have to deal with
+        # a block in that state in disable_versions_for_block().
+        block.updated_by = None
+        disable_versions_for_block(block, submission)
+        assert review_base_mock.call_args[0] == ()
+        assert review_base_mock.call_args[1] == {
+            'addon': addon,
+            'version': None,
+            'user': self.task_user,  # We fell back to the task user.
+            'review_type': 'pending',
+            'human_review': False,  # False because it's the task user.
+        }
+
+    @mock.patch('olympia.reviewers.utils.ReviewBase')
+    def test_reviewbase_human_review_is_false_if_block_was_updated_by_task_user(
+        self, review_base_mock
+    ):
+        addon = addon_factory()
+        submission = BlocklistSubmission.objects.create(
+            input_guids=addon.guid,
+            reason='some reason',
+            url=None,
+            updated_by=self.task_user,
+            disable_addon=True,
+            block_type=BlockType.BLOCKED,
+            changed_version_ids=[addon.current_version.pk],
+            signoff_state=BlocklistSubmission.SIGNOFF_PUBLISHED,
+        )
+        block = block_factory(addon=addon, updated_by=submission.updated_by)
+        disable_versions_for_block(block, submission)
+        assert review_base_mock.call_args[0] == ()
+        assert review_base_mock.call_args[1] == {
+            'addon': addon,
+            'version': None,
+            'user': self.task_user,
+            'review_type': 'pending',
+            'human_review': False,  # False because it's the task user.
+        }

src/olympia/blocklist/utils.py

@@ -147,12 +147,15 @@ def disable_versions_for_block(block, submission):
     """Disable appropriate addon versions that are affected by the Block."""
     from olympia.reviewers.utils import ReviewBase
 
+    task_user = get_task_user()
+    activity_user = block.updated_by or get_task_user()
+    human_review = activity_user != task_user
     review = ReviewBase(
         addon=block.addon,
         version=None,
-        user=get_task_user(),
+        user=activity_user,
         review_type='pending',
-        human_review=False,
+        human_review=human_review,
     )
     versions_to_reject = [
         ver