Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create an admin task when votes are 2 weeks old #2323

Closed
Closed
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
4f235c8
add thread_id to pydantic model
Nov 6, 2022
b1845e5
add thread mentions to review history
Nov 6, 2022
99d9662
add list of threads to a review's markdown file
Nov 6, 2022
18e7cbe
replace thread mention with jump url
Nov 7, 2022
71d6ebc
add style to thread not found message
Nov 7, 2022
293dac8
use get_or_fetch_channel to look from thread
Nov 7, 2022
f6f3e55
use jump_url in the review markdown file
Nov 7, 2022
33828c8
add emtpy task to track forgotten nominations
Nov 12, 2022
ecc29bd
fetch forgotten nominations from site
Nov 12, 2022
8bd6840
filter out untracked nominations in GitHub
Nov 12, 2022
c4be56d
Merge branch '2304-link-previous-nomination-threads' into 2226-track-…
Nov 12, 2022
346be1a
filter out untracked nominations in GitHub
Nov 12, 2022
ed728d4
return a nominationXMessage tuple
Nov 12, 2022
506fbb2
add dummy method that tracks vote in GitHub
Nov 12, 2022
0ba1ddf
use "🎫" as emoji
Nov 12, 2022
9773aca
add github_admin_repo section
Nov 13, 2022
0da9f16
add loader for the github_admin_repo section
Nov 13, 2022
fef37fe
log repo name when token hasn't been provided
Nov 13, 2022
9259f84
reset value of autoreview task
Nov 13, 2022
161df87
change repo name to 'admin-tasks'
shtlrs Nov 27, 2022
4b0d49b
add debug log upon creating Github issue
shtlrs Nov 27, 2022
154c51d
handle case where member cannot be found upon tracking vote
shtlrs Nov 27, 2022
802003b
handle case where thread cannot be found when filtering tracked nomin…
shtlrs Nov 27, 2022
5a916ac
handle case where thread starter message cannot be found when filteri…
shtlrs Nov 27, 2022
68c625c
replace usage of typing.List with native list
shtlrs Nov 27, 2022
3ca37a0
use log.warning instead of deprecetated log.warn
shtlrs Nov 27, 2022
1c8059a
update name of the method that identifies untracked nominations
shtlrs Nov 27, 2022
da3c904
add jump url to the issue's body
shtlrs Nov 27, 2022
59319b4
Merge branch 'main' into 2226-track-old-votes-in-github
shtlrs Jan 1, 2023
33415da
move Github token check to `cog_load`
Nov 6, 2022
babf906
use review start date instead of nomination inserted_at
shtlrs Jan 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions bot/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,14 @@ class VideoPermission(metaclass=YAMLGetter):
default_permission_duration: int


class GithubAdminRepo(metaclass=YAMLGetter):
section = "github_admin_repo"

owner: str
name: str
token: str


class ThreadArchiveTimes(Enum):
HOUR = 60
DAY = 1440
Expand Down
4 changes: 4 additions & 0 deletions bot/exts/recruitment/talentpool/_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class Nomination(BaseModel):
ended_at: datetime | None
entries: list[NominationEntry]
reviewed: bool
thread_id: int | None


class NominationAPI:
Expand Down Expand Up @@ -65,6 +66,7 @@ async def edit_nomination(
end_reason: str | None = None,
active: bool | None = None,
reviewed: bool | None = None,
thread_id: int | None = None,
) -> Nomination:
"""
Edit a nomination.
Expand All @@ -78,6 +80,8 @@ async def edit_nomination(
data["active"] = active
if reviewed is not None:
data["reviewed"] = reviewed
if thread_id is not None:
data["thread_id"] = thread_id

result = await self.site_api.patch(f"bot/nominations/{nomination_id}", json=data)
return Nomination.parse_obj(result)
Expand Down
96 changes: 94 additions & 2 deletions bot/exts/recruitment/talentpool/_cog.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import asyncio
import textwrap
from io import StringIO
from typing import Optional, Union
from typing import List, Optional, Tuple, Union

import arrow
import discord
from async_rediscache import RedisCache
from botcore.site_api import ResponseCodeError
Expand All @@ -11,18 +12,23 @@
from discord.ext.commands import BadArgument, Cog, Context, group, has_any_role

from bot.bot import Bot
from bot.constants import Bot as BotConfig, Channels, Emojis, Guild, MODERATION_ROLES, Roles, STAFF_ROLES
from bot.constants import (
Bot as BotConfig, Channels, Emojis, GithubAdminRepo, Guild, MODERATION_ROLES, Roles, STAFF_ROLES
)
from bot.converters import MemberOrUser, UnambiguousMemberOrUser
from bot.exts.recruitment.talentpool._review import Reviewer
from bot.log import get_logger
from bot.pagination import LinePaginator
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"
FLAG_EMOJI = "🎫"
REASON_MAX_CHARS = 1000
OLD_NOMINATIONS_THRESHOLD_IN_DAYS = 14

log = get_logger(__name__)

Expand All @@ -43,6 +49,7 @@ def __init__(self, bot: Bot) -> None:

async def cog_load(self) -> None:
"""Start autoreview loop if enabled."""
self.track_forgotten_nominations.start()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make this configurable to run just like we do for autoreview ?

if await self.autoreview_enabled():
self.autoreview_loop.start()

Expand Down Expand Up @@ -111,6 +118,81 @@ async def autoreview_status(self, ctx: Context) -> None:
else:
await ctx.send("Autoreview is currently disabled.")

@tasks.loop(hours=72)
async def track_forgotten_nominations(self) -> None:
"""Track active nominations who are more than 2 weeks old."""
old_nominations = await self._get_forgotten_nominations()
nomination_details = await self._filter_out_tracked_nominations(old_nominations)
for nomination, nomination_vote_message in nomination_details:
issue_created = await self._track_vote_in_github(nomination)
if issue_created:
await nomination_vote_message.add_reaction(FLAG_EMOJI)

async def _get_forgotten_nominations(self) -> List[Nomination]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
async def _get_forgotten_nominations(self) -> List[Nomination]:
async def _get_forgotten_nominations(self) -> list[Nomination]:

This can be done since Python 3.9. There are more examples in this PR where typing is used where built-ins could be used instead, please replace them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 68c625c

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't resolved.

"""Get active nominations that are more than 2 weeks old."""
now = arrow.utcnow()
nominations = [
nomination
for nomination in await self.api.get_nominations(active=True)
if (now - nomination.inserted_at).days >= OLD_NOMINATIONS_THRESHOLD_IN_DAYS
shtlrs marked this conversation as resolved.
Show resolved Hide resolved
]
return nominations

async def _filter_out_tracked_nominations(
self,
nominations: List[Nomination]
) -> List[Tuple[Nomination, discord.Message]]:
"""Filter the forgotten nominations that are still untracked in GitHub."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't just a filter, since it returns more data that it was given. This should be described in the docstring, and maybe change the function name to make it clear it isn't just a filter

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alrighty, fixed in 1c8059a

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't resolved.

untracked_nominations = []

for nomination in nominations:
# We avoid the scenario of this task run & nomination created at the same time
if not nomination.thread_id:
continue

thread = await get_or_fetch_channel(nomination.thread_id)
shtlrs marked this conversation as resolved.
Show resolved Hide resolved
if not thread:
# Thread was deleted
continue

starter_message = thread.starter_message
if not starter_message:
# Starter message will be null if it's not cached
starter_message = await self.bot.get_channel(Channels.nomination_voting).fetch_message(thread.id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handle the case where this raises an error if the message doesn't exist

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5a916ac

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't resolved.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linking the commit here 5a916ac


if not starter_message:
# Starter message deleted
continue

if FLAG_EMOJI in [reaction.emoji for reaction in starter_message.reactions]:
# Nomination has been already tracked in GitHub
continue

untracked_nominations.append((nomination, starter_message))
return untracked_nominations

async def _track_vote_in_github(self, nomination: Nomination) -> bool:
"""
Adds an issue in GitHub to track dormant vote.

Returns True when the issue has been created, False otherwise.
"""
if not GithubAdminRepo.token:
log.warn(f"No token for the {GithubAdminRepo.name} repository was provided, skipping issue creation.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
log.warn(f"No token for the {GithubAdminRepo.name} repository was provided, skipping issue creation.")
log.warning(f"No token for the {GithubAdminRepo.name} repository was provided, skipping issue creation.")

warn is a deprecated alias to warning

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't resolved.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 3ca37a0

return False

url = f"https://api.github.com/repos/{GithubAdminRepo.owner}/{GithubAdminRepo.name}/issues"
headers = {
"Accept": "application/vnd.github.v3+json",
"Authorization": f"Bearer {GithubAdminRepo.token}"
}
member = await get_or_fetch_member(self.bot.get_guild(Guild.id), nomination.user_id)
ChrisLovering marked this conversation as resolved.
Show resolved Hide resolved
data = {"title": f"Nomination review needed. Id: {nomination.id}. User: {member.name}"}
ChrisLovering marked this conversation as resolved.
Show resolved Hide resolved

async with self.bot.http_session.post(url=url, raise_for_status=True, headers=headers, json=data) as response:
# REVIEW: Might be useful to add logs here ?
shtlrs marked this conversation as resolved.
Show resolved Hide resolved
return response.status == 201

@tasks.loop(hours=1)
async def autoreview_loop(self) -> None:
"""Send request to `reviewer` to send a nomination if ready."""
Expand Down Expand Up @@ -489,13 +571,22 @@ async def _nomination_to_string(self, nomination: Nomination) -> str:
entries_string = "\n\n".join(entries)

start_date = time.discord_timestamp(nomination.inserted_at)

thread = None

if nomination.thread_id:
thread = await get_or_fetch_channel(nomination.thread_id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle the case where this raises an error if the thread doesn't exist.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's actually handled in the original PR that adds the thread id to the nominations table.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be visible in e68d756


thread_jump_url = f'[Jump to thread!]({thread.jump_url})' if thread else "*Not created*"

if nomination.active:
lines = textwrap.dedent(
f"""
===============
Status: **Active**
Date: {start_date}
Nomination ID: `{nomination.id}`
Nomination vote thread: {thread_jump_url}

{entries_string}
===============
Expand All @@ -509,6 +600,7 @@ async def _nomination_to_string(self, nomination: Nomination) -> str:
Status: Inactive
Date: {start_date}
Nomination ID: `{nomination.id}`
Nomination vote thread: {thread_jump_url}

{entries_string}

Expand Down
12 changes: 11 additions & 1 deletion bot/exts/recruitment/talentpool/_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from bot.exts.recruitment.talentpool._api import Nomination, NominationAPI
from bot.log import get_logger
from bot.utils import time
from bot.utils.channel import get_or_fetch_channel
from bot.utils.members import get_or_fetch_member
from bot.utils.messages import count_unique_users_reaction, pin_no_system_message

Expand Down Expand Up @@ -180,7 +181,7 @@ async def post_review(self, nomination: Nomination) -> None:
)
message = await thread.send(f"<@&{Roles.mod_team}> <@&{Roles.admins}>")

await self.api.edit_nomination(nomination.id, reviewed=True)
await self.api.edit_nomination(nomination.id, reviewed=True, thread_id=thread.id)

bump_cog: ThreadBumper = self.bot.get_cog("ThreadBumper")
if bump_cog:
Expand Down Expand Up @@ -433,11 +434,20 @@ async def _previous_nominations_review(self, member: Member) -> Optional[str]:

nomination_times = f"{num_entries} times" if num_entries > 1 else "once"
rejection_times = f"{len(history)} times" if len(history) > 1 else "once"
threads = [await get_or_fetch_channel(nomination.thread_id) for nomination in history]

nomination_vote_threads = ", ".join(
[
f"[Thread-{thread_number}]({thread.jump_url})" if thread else ''
for thread_number, thread in enumerate(threads, start=1)
]
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change doesn't seem related to the issue, so might be worth splitting to another PR, since there's some more logic you'll need here.

get_or_fetch_channel will raise an error if the channel isn't found, rather than returning None that you are assuming here.

In your nomination_vote_threads list comp if the thread is non, you're adding '', which will result in odd output since, it will still add the comma separator.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That change is due to the fact that my other PR that hasn't been merged is needed, so I merged it here.
When it does get merged, I'll clean up the history.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think commit e68d756 should hold the answer

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And now that my previously mentioned PR has been merged, these changes no longer exist
Merge happened here 59319b4

end_time = time.format_relative(history[0].ended_at)

review = (
f"They were nominated **{nomination_times}** before"
f", but their nomination was called off **{rejection_times}**."
f"\nList of all of their nomination threads: {nomination_vote_threads}"
f"\nThe last one ended {end_time} with the reason: {history[0].end_reason}"
)

Expand Down
8 changes: 7 additions & 1 deletion config-default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ voice_gate:


branding:
cycle_frequency: 3 # How many days bot wait before refreshing server icon
cycle_frequency: 3 # How many days the bot waits before refreshing server icon


config:
Expand All @@ -583,3 +583,9 @@ config:

video_permission:
default_permission_duration: 5 # Default duration for stream command in minutes


github_admin_repo:
owner: 'python-discord'
name: 'admin'
shtlrs marked this conversation as resolved.
Show resolved Hide resolved
token: !ENV ["ADMIN_GITHUB_REPO_TOKEN", ""]