Merge pull request #2305 from python-discord/2301-fix-voting-conditions

Store time of last vote in redis to prevent vote triggering early

2 files changed, 32 insertions, 11 deletions

diff --git a/bot/exts/recruitment/talentpool/_review.py b/bot/exts/recruitment/talentpool/_review.py
index f41e08fe1..b9e76b60f 100644
--- a/bot/exts/recruitment/talentpool/_review.py
+++ b/bot/exts/recruitment/talentpool/_review.py
@@ -9,6 +9,7 @@ from datetime import datetime, timedelta, timezone
 from typing import List, Optional, Union
 
 import discord
+from async_rediscache import RedisCache
 from discord import Embed, Emoji, Member, Message, NotFound, PartialMessage, TextChannel
 from pydis_core.site_api import ResponseCodeError
 
@@ -52,6 +53,11 @@ NOMINATION_MESSAGE_REGEX = re.compile(
 class Reviewer:
     """Manages, formats, and publishes reviews of helper nominees."""
 
+    # RedisCache[
+    #    "last_vote_date": float   | POSIX UTC timestamp.
+    # ]
+    status_cache = RedisCache()
+
     def __init__(self, bot: Bot, nomination_api: NominationAPI):
         self.bot = bot
         self.api = nomination_api
@@ -82,8 +88,18 @@ class Reviewer:
         """
         voting_channel = self.bot.get_channel(Channels.nomination_voting)
 
+        last_vote_timestamp = await self.status_cache.get("last_vote_date")
+        if last_vote_timestamp:
+            last_vote_date = datetime.fromtimestamp(last_vote_timestamp, tz=timezone.utc)
+            time_since_last_vote = datetime.now(timezone.utc) - last_vote_date
+
+            if time_since_last_vote < MIN_REVIEW_INTERVAL:
+                log.debug("Most recent review was less than %s ago, cancelling check", MIN_REVIEW_INTERVAL)
+                return False
+        else:
+            log.info("Date of last vote not found in cache, a vote may be sent early")
+
         review_count = 0
-        is_first_message = True
         async for msg in voting_channel.history():
             # Try and filter out any non-review messages. We also only want to count
             # one message from reviews split over multiple messages. We use fixed text
@@ -91,14 +107,6 @@ class Reviewer:
             if not msg.author.bot or "for Helper!" not in msg.content:
                 continue
 
-            if is_first_message:
-                time_since_message_created = datetime.now(timezone.utc) - msg.created_at
-                if time_since_message_created < MIN_REVIEW_INTERVAL:
-                    log.debug("Most recent review was less than %s ago, cancelling check", MIN_REVIEW_INTERVAL)
-                    return False
-
-                is_first_message = False
-
             review_count += 1
 
             if review_count >= MAX_ONGOING_REVIEWS:
@@ -182,6 +190,9 @@ class Reviewer:
         )
         message = await thread.send(f"<@&{Roles.mod_team}> <@&{Roles.admins}>")
 
+        now = datetime.now(tz=timezone.utc)
+        await self.status_cache.set("last_vote_date", now.timestamp())
+
         await self.api.edit_nomination(nomination.id, reviewed=True, thread_id=thread.id)
 
         bump_cog: ThreadBumper = self.bot.get_cog("ThreadBumper")
diff --git a/tests/bot/exts/recruitment/talentpool/test_review.py b/tests/bot/exts/recruitment/talentpool/test_review.py
index 03c78ab08..1ddb73ab0 100644
--- a/tests/bot/exts/recruitment/talentpool/test_review.py
+++ b/tests/bot/exts/recruitment/talentpool/test_review.py
@@ -67,6 +67,7 @@ class ReviewerTests(unittest.IsolatedAsyncioTestCase):
                     MockMessage(author=self.bot_user, content="Not a review", created_at=not_too_recent),
                     MockMessage(author=self.bot_user, content="Not a review", created_at=not_too_recent),
                 ],
+                not_too_recent.timestamp(),
                 True,
             ),
 
@@ -77,6 +78,7 @@ class ReviewerTests(unittest.IsolatedAsyncioTestCase):
                     MockMessage(author=self.bot_user, content="Zig for Helper!", created_at=not_too_recent),
                     MockMessage(author=self.bot_user, content="Scaleios for Helper!", created_at=not_too_recent),
                 ],
+                not_too_recent.timestamp(),
                 False,
             ),
 
@@ -85,6 +87,7 @@ class ReviewerTests(unittest.IsolatedAsyncioTestCase):
                 [
                     MockMessage(author=self.bot_user, content="Chrisjl for Helper!", created_at=too_recent),
                 ],
+                too_recent.timestamp(),
                 False,
             ),
 
@@ -96,18 +99,25 @@ class ReviewerTests(unittest.IsolatedAsyncioTestCase):
                     MockMessage(author=self.bot_user, content="wookie for Helper!", created_at=not_too_recent),
                     MockMessage(author=self.bot_user, content="Not a review", created_at=not_too_recent),
                 ],
+                not_too_recent.timestamp(),
                 True,
             ),
 
             # No messages, so ready.
-            ([], True),
+            ([], None, True),
         )
 
-        for messages, expected in cases:
+        for messages, last_review_timestamp, expected in cases:
             with self.subTest(messages=messages, expected=expected):
                 self.voting_channel.history = AsyncIterator(messages)
+
+                cache_get_mock = AsyncMock(return_value=last_review_timestamp)
+                self.reviewer.status_cache.get = cache_get_mock
+
                 res = await self.reviewer.is_ready_for_review()
+
                 self.assertIs(res, expected)
+                cache_get_mock.assert_called_with("last_vote_date")
 
     @patch("bot.exts.recruitment.talentpool._review.MIN_NOMINATION_TIME", timedelta(days=7))
     async def test_get_nomination_to_review(self):