Merge pull request #2418 from python-discord/2302-activity-in-reviews

Take activity into account in talentpool

5 files changed, 279 insertions, 72 deletions

diff --git a/bot/constants.py b/bot/constants.py
index 3d0ae9542..eefae11e2 100644
--- a/bot/constants.py
+++ b/bot/constants.py
@@ -94,6 +94,7 @@ class _Channels(EnvConfig):
     mod_meta = 775412552795947058
     mods = 305126844661760000
     nominations = 822920136150745168
+    nomination_discussion = 798959130634747914
     nomination_voting = 822853512709931008
     organisation = 551789653284356126
 
diff --git a/bot/exts/recruitment/talentpool/_api.py b/bot/exts/recruitment/talentpool/_api.py
index c00c8c09c..e12111de5 100644
--- a/bot/exts/recruitment/talentpool/_api.py
+++ b/bot/exts/recruitment/talentpool/_api.py
@@ -112,3 +112,24 @@ class NominationAPI:
         }
         result = await self.site_api.post("bot/nominations", json=data)
         return Nomination.parse_obj(result)
+
+    async def get_activity(
+        self,
+        user_ids: list[int],
+        *,
+        days: int,
+    ) -> dict[int, int]:
+        """
+        Get the number of messages sent in the past `days` days by users with the given IDs.
+
+        Returns a dictionary mapping user ID to message count.
+        """
+        if not user_ids:
+            return {}
+
+        result = await self.site_api.post(
+            "bot/users/metricity_activity_data",
+            json=user_ids,
+            params={"days": str(days)}
+        )
+        return {int(user_id): message_count for user_id, message_count in result.items()}
diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py
index a41d9e8c5..f3716140a 100644
--- a/bot/exts/recruitment/talentpool/_cog.py
+++ b/bot/exts/recruitment/talentpool/_cog.py
@@ -1,5 +1,6 @@
 import asyncio
 import textwrap
+from datetime import datetime, timezone
 from io import StringIO
 from typing import Optional, Union
 
@@ -13,6 +14,7 @@ from pydis_core.site_api import ResponseCodeError
 from bot.bot import Bot
 from bot.constants import Bot as BotConfig, Channels, Emojis, Guild, MODERATION_ROLES, Roles, STAFF_ROLES
 from bot.converters import MemberOrUser, UnambiguousMemberOrUser
+from bot.exts.recruitment.talentpool._api import Nomination, NominationAPI
 from bot.exts.recruitment.talentpool._review import Reviewer
 from bot.log import get_logger
 from bot.pagination import LinePaginator
@@ -20,11 +22,13 @@ from bot.utils import time
 from bot.utils.channel import get_or_fetch_channel
 from bot.utils.members import get_or_fetch_member
 
-from ._api import Nomination, NominationAPI
-
 AUTOREVIEW_ENABLED_KEY = "autoreview_enabled"
 REASON_MAX_CHARS = 1000
 
+# The number of days that a user can have no activity (no messages sent)
+# until they should be removed from the talentpool.
+DAYS_UNTIL_INACTIVE = 45
+
 log = get_logger(__name__)
 
 
@@ -47,6 +51,8 @@ class TalentPool(Cog, name="Talentpool"):
         if await self.autoreview_enabled():
             self.autoreview_loop.start()
 
+        self.prune_talentpool.start()
+
     async def autoreview_enabled(self) -> bool:
         """Return whether automatic posting of nomination reviews is enabled."""
         return await self.talentpool_settings.get(AUTOREVIEW_ENABLED_KEY, True)
@@ -122,69 +128,177 @@ class TalentPool(Cog, name="Talentpool"):
             log.info("Running check for users to nominate.")
             await self.reviewer.maybe_review_user()
 
-    @nomination_group.command(
-        name="nominees",
-        aliases=("nominated", "all", "list", "watched"),
-        root_aliases=("nominees",)
+    @tasks.loop(hours=24)
+    async def prune_talentpool(self) -> None:
+        """
+        Prune any inactive users from the talentpool.
+
+        A user is considered inactive if they have sent no messages on the server
+        in the past `DAYS_UNTIL_INACTIVE` days.
+        """
+        log.info("Running task to prune users from talent pool")
+        nominations = await self.api.get_nominations(active=True)
+
+        if not nominations:
+            return
+
+        messages_per_user = await self.api.get_activity(
+            [nomination.user_id for nomination in nominations],
+            days=DAYS_UNTIL_INACTIVE
+        )
+
+        nomination_discussion = await get_or_fetch_channel(Channels.nomination_discussion)
+        for nomination in nominations:
+            if messages_per_user[nomination.user_id] > 0:
+                continue
+
+            if nomination.reviewed:
+                continue
+
+            log.info("Removing %s from the talent pool due to inactivity", nomination.user_id)
+
+            await nomination_discussion.send(
+                f":warning: <@{nomination.user_id}> ({nomination.user_id})"
+                " was removed from the talentpool as they have sent no messages"
+                f" in the past {DAYS_UNTIL_INACTIVE} days."
+            )
+            await self.api.edit_nomination(
+                nomination.id,
+                active=False,
+                end_reason=f"Automatic removal: User was inactive for more than {DAYS_UNTIL_INACTIVE}"
+            )
+
+    @nomination_group.group(
+        name="list",
+        aliases=("nominated", "nominees"),
+        invoke_without_command=True
     )
     @has_any_role(*MODERATION_ROLES)
-    async def list_command(
+    async def list_group(
         self,
         ctx: Context,
-        oldest_first: bool = False,
     ) -> None:
         """
         Shows the users that are currently in the talent pool.
 
-        The optional kwarg `oldest_first` can be used to order the list by oldest nomination.
+        The "Recent Nominations" sections shows users nominated in the past 7 days,
+        so will not be considered for autoreview.
+
+        In the "Autoreview Priority" section a :zzz: emoji will be shown next to
+        users that have not been active recently enough to be considered for autoreview.
+        Note that the order in this section will change over time so should not be relied upon.
         """
-        await self.list_nominated_users(ctx, oldest_first=oldest_first)
+        await self.show_nominations_list(ctx, grouped_view=True)
+
+    @list_group.command(name="oldest")
+    async def list_oldest(self, ctx: Context) -> None:
+        """Shows the users that are currently in the talent pool, ordered by oldest nomination."""
+        await self.show_nominations_list(ctx, oldest_first=True)
+
+    @list_group.command(name='newest')
+    async def list_newest(self, ctx: Context) -> None:
+        """Shows the users that are currently in the talent pool, ordered by newest nomination."""
+        await self.show_nominations_list(ctx, oldest_first=False)
 
-    async def list_nominated_users(
+    async def show_nominations_list(
         self,
         ctx: Context,
+        *,
         oldest_first: bool = False,
+        grouped_view: bool = False,
     ) -> None:
         """
-        Gives an overview of the nominated users list.
+        Lists the currently nominated users.
 
-        It specifies the user's mention, name, how long ago they were nominated, and whether their
-        review was posted.
+        If `grouped_view` is passed, nominations will be displayed in the groups
+        being reviewed, recent nominations, and others by autoreview priority.
 
-        The optional kwarg `oldest_first` orders the list by oldest entry.
+        Otherwise, nominations will be sorted by age
+        (ordered based on the value of `oldest_first`).
         """
+        now = datetime.now(tz=timezone.utc)
         nominations = await self.api.get_nominations(active=True)
-        if oldest_first:
-            nominations = reversed(nominations)
+        messages_per_user = await self.api.get_activity(
+            [nomination.user_id for nomination in nominations],
+            days=DAYS_UNTIL_INACTIVE
+        )
 
-        lines = []
+        if grouped_view:
+            reviewed_nominations = []
+            recent_nominations = []
+            other_nominations = []
+            for nomination in nominations:
+                if nomination.reviewed:
+                    reviewed_nominations.append(nomination)
+                elif not self.reviewer.is_nomination_old_enough(nomination, now):
+                    recent_nominations.append(nomination)
+                else:
+                    other_nominations.append(nomination)
+
+            other_nominations = await self.reviewer.sort_nominations_to_review(other_nominations, now)
+
+            lines = [
+                "**Being Reviewed:**",
+                *await self.list_nominations(ctx, reviewed_nominations, messages_per_user),
+                "**Recent Nominations:**",
+                *await self.list_nominations(ctx, recent_nominations, messages_per_user),
+                "**Other Nominations by Autoreview Priority:**",
+                *await self.list_nominations(ctx, other_nominations, messages_per_user, show_inactive=True)
+            ]
+        else:
+            if oldest_first:
+                nominations.reverse()
+            lines = await self.list_nominations(ctx, nominations, messages_per_user, show_reviewed=True)
+
+            if not lines:
+                lines = ["There are no active nominations"]
+
+        embed = Embed(
+            title="Talent Pool active nominations",
+            color=Color.blue()
+        )
+        await LinePaginator.paginate(lines, ctx, embed, empty=False)
+
+    async def list_nominations(
+        self,
+        ctx: Context,
+        nominations: list[Nomination],
+        messages_per_user: dict[int, int],
+        *,
+        show_reviewed: bool = False,
+        show_inactive: bool = False,
+    ) -> list[str]:
+        """
+        Formats the given nominations into a list.
+
+        Pass `show_reviewed` to indicate reviewed nominations, and `show_inactive` to
+        indicate if the user doesn't have recent enough activity to be autoreviewed.
+        """
+        lines: list[str] = []
+
+        if not nominations:
+            return ["*None*"]
 
         for nomination in nominations:
-            member = await get_or_fetch_member(ctx.guild, nomination.user_id)
             line = f"• `{nomination.user_id}`"
+
+            member = await get_or_fetch_member(ctx.guild, nomination.user_id)
             if member:
                 line += f" ({member.name}#{member.discriminator})"
+            else:
+                line += " (not on server)"
+
             line += f", added {time.format_relative(nomination.inserted_at)}"
-            if not member:  # Cross off users who left the server.
-                line = f"~~{line}~~"
-            if nomination.reviewed:
-                line += " *(reviewed)*"
-            lines.append(line)
 
-        if not lines:
-            lines = ("There's nothing here yet.",)
+            if show_reviewed and nomination.reviewed:
+                line += " *(reviewed)*"
 
-        embed = Embed(
-            title="Talent Pool active nominations",
-            color=Color.blue()
-        )
-        await LinePaginator.paginate(lines, ctx, embed, empty=False)
+            is_active = self.reviewer.is_user_active_enough(messages_per_user[nomination.user_id])
+            if show_inactive and not is_active:
+                line += " :zzz:"
 
-    @nomination_group.command(name='oldest')
-    @has_any_role(*MODERATION_ROLES)
-    async def oldest_command(self, ctx: Context) -> None:
-        """Shows the users that are currently in the talent pool, ordered by oldest nomination."""
-        await self.list_nominated_users(ctx, oldest_first=True)
+            lines.append(line)
+        return lines
 
     @nomination_group.command(
         name="forcenominate",
@@ -435,7 +549,12 @@ class TalentPool(Cog, name="Talentpool"):
     @Cog.listener()
     async def on_member_ban(self, guild: Guild, user: MemberOrUser) -> None:
         """Remove `user` from the talent pool after they are banned."""
-        await self.end_nomination(user.id, "User was banned.")
+        if await self.end_nomination(user.id, "Automatic removal: User was banned"):
+            nomination_discussion = await get_or_fetch_channel(Channels.nomination_discussion)
+            await nomination_discussion.send(
+                f":warning: <@{user.id}> ({user.id})"
+                " was removed from the talentpool due to being banned."
+            )
 
     @Cog.listener()
     async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None:
@@ -539,3 +658,5 @@ class TalentPool(Cog, name="Talentpool"):
         # Only cancel the loop task when the autoreview code is not running
         async with self.autoreview_lock:
             self.autoreview_loop.cancel()
+
+        self.prune_talentpool.cancel()
diff --git a/bot/exts/recruitment/talentpool/_review.py b/bot/exts/recruitment/talentpool/_review.py
index efc9c7a4d..f46e565ed 100644
--- a/bot/exts/recruitment/talentpool/_review.py
+++ b/bot/exts/recruitment/talentpool/_review.py
@@ -38,6 +38,8 @@ MAX_ONGOING_REVIEWS = 3
 MIN_REVIEW_INTERVAL = timedelta(days=1)
 # Minimum time between nomination and sending a review
 MIN_NOMINATION_TIME = timedelta(days=7)
+# Number of days ago that the user must have activity since
+RECENT_ACTIVITY_DAYS = 7
 
 # A constant for weighting number of nomination entries against nomination age when selecting a user to review.
 # The higher this is, the lower the effect of review age. At 1, age and number of entries are weighted equally.
@@ -115,56 +117,101 @@ class Reviewer:
 
         return True
 
-    async def get_nomination_to_review(self) -> Optional[Nomination]:
+    @staticmethod
+    def is_nomination_old_enough(nomination: Nomination, now: datetime) -> bool:
+        """Check if a nomination is old enough to autoreview."""
+        time_since_nomination = now - nomination.inserted_at
+        return time_since_nomination > MIN_NOMINATION_TIME
+
+    @staticmethod
+    def is_user_active_enough(user_message_count: int) -> bool:
+        """Check if a user's message count is enough for them to be autoreviewed."""
+        return user_message_count > 0
+
+    async def is_nomination_ready_for_review(
+        self,
+        nomination: Nomination,
+        user_message_count: int,
+        now: datetime,
+    ) -> bool:
         """
-        Returns the Nomination of the next user to review, or None if there are no users ready.
+        Returns a boolean representing whether a nomination should be reviewed.
 
         Users will only be selected for review if:
          - They have not already been reviewed.
          - They have been nominated for longer than `MIN_NOMINATION_TIME`.
+         - They have sent at least one message in the server recently.
+         - They are still a member of the server.
+        """
+        guild = self.bot.get_guild(Guild.id)
+        return (
+            # Must be an active nomination
+            nomination.active and
+            # ... that has not already been reviewed
+            not nomination.reviewed and
+            # ... and has been nominated for long enough
+            self.is_nomination_old_enough(nomination, now) and
+            # ... and is for a user that has been active recently
+            self.is_user_active_enough(user_message_count) and
+            # ... and is currently a member of the server
+            await get_or_fetch_member(guild, nomination.user_id) is not None
+        )
 
-        The priority of the review is determined by how many nominations the user has
-        (more nominations = higher priority).
-        For users with equal priority the oldest nomination will be reviewed first.
+    async def sort_nominations_to_review(self, nominations: list[Nomination], now: datetime) -> list[Nomination]:
         """
-        now = datetime.now(timezone.utc)
+        Sorts a list of nominations by priority for review.
 
-        possible_nominations: list[Nomination] = []
-        nominations = await self.api.get_nominations(active=True)
-        for nomination in nominations:
-            time_since_nomination = now - nomination.inserted_at
-            if (
-                not nomination.reviewed
-                and time_since_nomination > MIN_NOMINATION_TIME
-            ):
-                possible_nominations.append(nomination)
+        The priority of the review is determined based on how many nominations the user has
+        (more nominations = higher priority), and the age of the nomination.
+        """
+        if not nominations:
+            return []
 
-        if not possible_nominations:
-            log.debug("No users ready to review.")
-            return None
+        oldest_date = min(nomination.inserted_at for nomination in nominations)
+        max_entries = max(len(nomination.entries) for nomination in nominations)
 
-        oldest_date = min(nomination.inserted_at for nomination in possible_nominations)
-        max_entries = max(len(nomination.entries) for nomination in possible_nominations)
+        def score_nomination(nomination: Nomination) -> float:
+            """
+            Scores a nomination based on age and number of nomination entries.
 
-        def sort_key(nomination: Nomination) -> float:
-            return self.score_nomination(nomination, oldest_date, now, max_entries)
+            The higher the score, the higher the priority for being put up for review should be.
+            """
+            num_entries = len(nomination.entries)
+            entries_score = num_entries / max_entries
 
-        return max(possible_nominations, key=sort_key)
+            nomination_date = nomination.inserted_at
+            age_score = (nomination_date - now) / (oldest_date - now)
 
-    @staticmethod
-    def score_nomination(nomination: Nomination, oldest_date: datetime, now: datetime, max_entries: int) -> float:
+            return entries_score * REVIEW_SCORE_WEIGHT + age_score
+
+        return sorted(nominations, key=score_nomination, reverse=True)
+
+    async def get_nomination_to_review(self) -> Nomination | None:
         """
-        Scores a nomination based on age and number of nomination entries.
+        Returns the Nomination of the next user to review, or None if there are no users ready.
 
-        The higher the score, the higher the priority for being put up for review should be.
+        See `is_ready_for_review` for the criteria for a user to be ready for review.
+        See `sort_nominations_to_review` for the criteria for a user to be prioritised for review.
         """
-        num_entries = len(nomination.entries)
-        entries_score = num_entries / max_entries
+        now = datetime.now(timezone.utc)
+        nominations = await self.api.get_nominations(active=True)
+        if not nominations:
+            return None
 
-        nomination_date = nomination.inserted_at
-        age_score = (nomination_date - now) / (oldest_date - now)
+        messages_per_user = await self.api.get_activity(
+            [nomination.user_id for nomination in nominations],
+            days=RECENT_ACTIVITY_DAYS,
+        )
+        possible_nominations = [
+            nomination for nomination in nominations
+            if await self.is_nomination_ready_for_review(nomination, messages_per_user[nomination.user_id], now)
+        ]
+        if not possible_nominations:
+            log.info("No nominations are ready to review")
+            return None
 
-        return entries_score * REVIEW_SCORE_WEIGHT + age_score
+        sorted_nominations = await self.sort_nominations_to_review(possible_nominations, now)
+        return sorted_nominations[0]
 
     async def post_review(self, nomination: Nomination) -> None:
         """Format the review of a user and post it to the nomination voting channel."""
diff --git a/tests/bot/exts/recruitment/talentpool/test_review.py b/tests/bot/exts/recruitment/talentpool/test_review.py
index 1ddb73ab0..f726fccc7 100644
--- a/tests/bot/exts/recruitment/talentpool/test_review.py
+++ b/tests/bot/exts/recruitment/talentpool/test_review.py
@@ -30,13 +30,17 @@ def nomination(
     inserted_at: datetime,
     num_entries: int,
     reviewed: bool = False,
-    id: int | None = None
+    id: int | None = None,
+    msg_count: int = 1000,
 ) -> Mock:
+    id = id or MockMember().id
     return Mock(
         id=id or MockMember().id,
+        user_id=id,
         inserted_at=inserted_at,
         entries=[Mock() for _ in range(num_entries)],
-        reviewed=reviewed
+        reviewed=reviewed,
+        _msg_count=msg_count,
     )
 
 
@@ -127,10 +131,13 @@ class ReviewerTests(unittest.IsolatedAsyncioTestCase):
         # Each case contains a list of nominations, followed by the index in that list
         # of the one that should be selected, or None if None should be returned
         cases = [
-            # One nomination, too recent so don't send.
+            # Don't send if too recent, already reviewed, or no recent messages.
             (
                 [
                     nomination(now - timedelta(days=1), 5),
+                    nomination(now - timedelta(days=10), 5, reviewed=True),
+                    nomination(now - timedelta(days=10), 5, msg_count=0),
+
                 ],
                 None,
             ),
@@ -161,6 +168,11 @@ class ReviewerTests(unittest.IsolatedAsyncioTestCase):
             with self.subTest(case_num=case_num):
                 get_nominations_mock = AsyncMock(return_value=nominations)
                 self.nomination_api.get_nominations = get_nominations_mock
+
+                activity = {nomination.id: nomination._msg_count for nomination in nominations}
+                get_activity_mock = AsyncMock(return_value=activity)
+                self.nomination_api.get_activity = get_activity_mock
+
                 res = await self.reviewer.get_nomination_to_review()
 
                 if expected is None:
@@ -168,6 +180,7 @@ class ReviewerTests(unittest.IsolatedAsyncioTestCase):
                 else:
                     self.assertEqual(res, nominations[expected])
                 get_nominations_mock.assert_called_once_with(active=True)
+                get_activity_mock.assert_called_once()
 
     @patch("bot.exts.recruitment.talentpool._review.MIN_NOMINATION_TIME", timedelta(days=0))
     async def test_get_nomination_to_review_order(self):
@@ -211,6 +224,10 @@ class ReviewerTests(unittest.IsolatedAsyncioTestCase):
                         get_nominations_mock = AsyncMock(return_value=case[i:])
                         self.nomination_api.get_nominations = get_nominations_mock
 
+                        activity = {nomination.id: nomination._msg_count for nomination in case}
+                        get_activity_mock = AsyncMock(return_value=activity)
+                        self.nomination_api.get_activity = get_activity_mock
+
                         res = await self.reviewer.get_nomination_to_review()
                         self.assertEqual(res, case[i])
                         get_nominations_mock.assert_called_once_with(active=True)