cache existing Blocks so reversing delayed block actions doesn't fail

src/olympia/abuse/actions.py

@@ -19,7 +19,7 @@
 from olympia.amo.templatetags.jinja_helpers import absolutify
 from olympia.amo.utils import send_mail
 from olympia.bandwagon.models import Collection
-from olympia.blocklist.models import BlocklistSubmission, BlockType
+from olympia.blocklist.models import Block, BlocklistSubmission, BlockType
 from olympia.blocklist.utils import delete_versions_from_blocks, save_versions_to_blocks
 from olympia.constants.abuse import DECISION_ACTIONS
 from olympia.constants.permissions import ADDONS_HIGH_IMPACT_APPROVE
@@ -742,6 +742,18 @@ def __init__(self, decision):
                 f'{self.__class__.__name__} requires delay_days to be set'
             )
 
+    @classmethod
+    def get_existing_blocks_from_decision(cls, decision):
+        """Get existing blocks for the decision's target and cache them on the decision.
+        We're caching them on the decision to avoid hitting the database multiple times
+        when reversing a decision, and because get_blocks_from_guids uses replicas, so
+        the data can be stale when we're running in a transaction"""
+        if not hasattr(decision, '_existing_blocks'):
+            decision._existing_blocks = Block.get_blocks_from_guids(
+                [decision.target.guid]
+            )
+        return decision._existing_blocks
+
     def process_action(self, release_hold=False):
         versions_qs = self.versions_block_will_affect
         # if this is a followup action, and the primary action is rejecting specific
@@ -795,7 +807,7 @@ def reverse_action(cls, original_decision):
             )
         )
         delete_versions_from_blocks(
-            [guid],
+            cls.get_existing_blocks_from_decision(original_decision),
             BlocklistSubmission(
                 input_guids=guid,
                 action=BlocklistSubmission.ACTIONS.DELETE,

src/olympia/abuse/tests/test_actions.py

@@ -2285,6 +2285,28 @@ def test_approve_appeal_success_followup(self):
         )
         self._test_approve_appeal_or_override(ContentActionTargetAppealApprove)
 
+    def test_approve_appeal_success_followup_with_multiple_followups(self):
+        self.past_negative_decision.update(
+            appeal_job=self.cinder_job, action=DECISION_ACTIONS.AMO_DISABLE_ADDON
+        )
+        ContentDecisionFollowupAction.objects.create(
+            decision=self.past_negative_decision,
+            action=self.takedown_decision_action,
+            action_date=datetime.now(),
+        )
+        # These follow-up actions are redundant, but shouldn't cause errors.
+        ContentDecisionFollowupAction.objects.create(
+            decision=self.past_negative_decision,
+            action=DECISION_ACTIONS.AMO_FU_DELAY_LONG_SOFT_BLOCK_ADDON,
+            action_date=datetime.now(),
+        )
+        ContentDecisionFollowupAction.objects.create(
+            decision=self.past_negative_decision,
+            action=DECISION_ACTIONS.AMO_FU_DELAY_LONG_HARD_BLOCK_ADDON,
+            action_date=datetime.now(),
+        )
+        self._test_approve_appeal_or_override(ContentActionTargetAppealApprove)
+
     def test_approve_override_success_followup(self):
         self.decision.update(override_of=self.past_negative_decision)
         self.past_negative_decision.update(action=DECISION_ACTIONS.AMO_DISABLE_ADDON)

src/olympia/blocklist/models.py

@@ -536,7 +536,8 @@ def delete_block_objects(self):
 
         for guids_chunk in chunked(all_guids_to_block, 100):
             # This function will remove BlockVersions and delete the Block if empty
-            delete_versions_from_blocks(guids_chunk, self)
+            blocks = Block.get_blocks_from_guids(guids_chunk)
+            delete_versions_from_blocks(blocks, self)
             self.save()
 
         self.update(signoff_state=self.SIGNOFF_STATES.PUBLISHED)

src/olympia/blocklist/tests/test_utils.py

@@ -430,7 +430,7 @@ def test_activity_attributed_to_user_deleting_the_block(self):
         addon = addon_factory()
         version_factory(addon=addon)
         user = user_factory()
-        block_factory(guid=addon.guid, updated_by=self.task_user)
+        block = block_factory(guid=addon.guid, updated_by=self.task_user)
         changed_version_ids = list(
             addon.versions.values_list('pk', flat=True).order_by('pk')
         )
@@ -445,7 +445,7 @@ def test_activity_attributed_to_user_deleting_the_block(self):
             signoff_state=BlocklistSubmission.SIGNOFF_STATES.PUBLISHED,
         )
         ActivityLog.objects.all().delete()
-        delete_versions_from_blocks([addon.guid], submission)
+        delete_versions_from_blocks([block], submission)
         for version in addon.versions.all():
             assert not version.is_blocked
 
@@ -468,7 +468,7 @@ def test_activity_attributed_to_user_deleting_the_block_with_signoff(self):
         version_factory(addon=addon)
         user = user_factory()
         signoffer = user_factory()
-        block_factory(guid=addon.guid, updated_by=self.task_user)
+        block = block_factory(guid=addon.guid, updated_by=self.task_user)
         changed_version_ids = list(
             addon.versions.values_list('pk', flat=True).order_by('pk')
         )
@@ -484,7 +484,7 @@ def test_activity_attributed_to_user_deleting_the_block_with_signoff(self):
             signoff_by=signoffer,
         )
         ActivityLog.objects.all().delete()
-        delete_versions_from_blocks([addon.guid], submission)
+        delete_versions_from_blocks([block], submission)
         for version in addon.versions.all():
             assert not version.is_blocked
 

src/olympia/blocklist/utils.py

@@ -257,12 +257,11 @@ def save_versions_to_blocks(guids, submission):
     return blocks
 
 
-def delete_versions_from_blocks(guids, submission):
-    from .models import Block, BlockVersion
+def delete_versions_from_blocks(blocks, submission):
+    from .models import BlockVersion
 
     modified_datetime = datetime.now()
 
-    blocks = Block.get_blocks_from_guids(guids)
     should_override_block_metadata = not submission.preserve_block_metadata
     for block in blocks:
         if not block.id: