diff options
| -rw-r--r-- | bot/constants.py | 1 | ||||
| -rw-r--r-- | bot/exts/recruitment/talentpool/_api.py | 21 | ||||
| -rw-r--r-- | bot/exts/recruitment/talentpool/_cog.py | 195 | ||||
| -rw-r--r-- | bot/exts/recruitment/talentpool/_review.py | 111 | ||||
| -rw-r--r-- | tests/bot/exts/recruitment/talentpool/test_review.py | 23 |
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) |