release: bot v0.4.0, final results and leaderboard fixes

package.json

@@ -1,6 +1,6 @@
 {
   "name": "codercops-discord-bot",
-  "version": "0.3.0",
+  "version": "0.4.0",
   "private": true,
   "description": "The CODERCOPS community bot: posts releases, starter issues and merged PRs from our open source repos into Discord.",
   "license": "MIT",

src/commands.ts

@@ -3,9 +3,9 @@
 import { listCheckins, localDate, rankStreaks, recordCheckin } from "./checkin";
 import type { Config, Env } from "./config";
 import type { MessagePayload } from "./discord";
-import { countMergedPrs, GithubError, listStarterIssues } from "./github";
+import { countMergedPrs, getLoginById, GithubError, listStarterIssues } from "./github";
 import { getLeaderboard } from "./leaderboard";
-import { getLink, linkedMembers, removeLink } from "./links";
+import { getLink, linkedMembers, removeLink, saveLink } from "./links";
 import { checkinReply, helpReply, leaderboardMessage, linkPrompt, meReply, starterIssuesReply, streaksMessage } from "./messages";
 import { authorizeUrl, linkingEnabled, signState } from "./oauth";
 import { grantEarnedRoles, isMaintainer, removeTierRoles } from "./roles";
@@ -23,6 +23,7 @@ export interface CommandUser {
 }
 
 const IN_SERVER_ONLY: MessagePayload = { content: "Run this inside the server, not in a DM." };
+const RELINK_GITHUB: MessagePayload = { content: "I couldn't find your linked GitHub account. Run `/link` again to reconnect it." };
 
 export async function runCommand(
   data: CommandData,
@@ -83,7 +84,9 @@ export async function runCommand(
         return leaderboardMessage(board, program, {
           final: programStatus(program, now) === "ended",
           url: leaderboardUrl(cfg, program),
+          finalNote: cfg.finalResultsNote,
           members: await linkedMembers(env, board.rows.slice(0, 10).map((r) => r.login)),
+          linking: linkingEnabled(env),
         });
       }
       case "link": {
@@ -105,9 +108,31 @@ export async function runCommand(
       }
       case "me": {
         if (!user) return IN_SERVER_ONLY;
-        const link = await getLink(env, user.id);
+        let link = await getLink(env, user.id);
         if (!link) return { content: "You haven't linked a GitHub account yet. Run `/link` first." };
-        const merged = await countMergedPrs(env, cfg, link.githubLogin);
+        let merged: number;
+        try {
+          merged = await countMergedPrs(env, cfg, link.githubLogin);
+        } catch (err) {
+          if (!(err instanceof GithubError) || err.status !== 422) throw err;
+          let login: string | null;
+          try {
+            login = await getLoginById(env, link.githubId);
+          } catch (lookupError) {
+            if (lookupError instanceof GithubError && (lookupError.status === 403 || lookupError.status === 429)) throw lookupError;
+            return RELINK_GITHUB;
+          }
+          if (!login) return RELINK_GITHUB;
+          link = { ...link, githubLogin: login };
+          await saveLink(env, link);
+          // Retry once; another invalid login must not restart recovery.
+          try {
+            merged = await countMergedPrs(env, cfg, login);
+          } catch (retryError) {
+            if (retryError instanceof GithubError && retryError.status === 422) return RELINK_GITHUB;
+            throw retryError;
+          }
+        }
         // Catches up on a role if a merge was missed.
         const newRoles = await grantEarnedRoles(env, cfg, link, merged).catch((err) => {
           console.error("granting roles failed", err);

src/github.ts

@@ -43,6 +43,15 @@ async function gh<T>(env: Env, path: string): Promise<T> {
   return (await res.json()) as T;
 }
 
+/** Resolves a linked account's current login using the id that survives renames. */
+export async function getLoginById(env: Env, githubId: string): Promise<string | null> {
+  if (!/^[1-9]\d*$/.test(githubId)) return null;
+  const user = await gh<{ id?: number; login?: unknown } | null>(env, `/user/${githubId}`);
+  if (!user || String(user.id) !== githubId || typeof user.login !== "string") return null;
+  // Apply the same login constraints as the verified Discord connection in oauth.ts.
+  return /^[a-z\d](?:[a-z\d-]{0,38})$/i.test(user.login) ? user.login : null;
+}
+
 export interface StarterIssue {
   repo: string;
   number: number;
@@ -110,17 +119,27 @@ interface SearchResult {
   items: { html_url: string; number: number; title: string; repository_url: string; user: { login: string } | null; pull_request?: { merged_at: string | null } }[];
 }
 
-/** Every merged PR that counts for a program: labeled, in its repos, merged inside its window. */
-export async function searchMergedPrs(env: Env, cfg: Config, program: Program): Promise<MergedPr[]> {
-  const { label, start, end } = program;
-  const iso = (d: Date) => d.toISOString().replace(/\.\d{3}Z$/, "Z");
-  const q = [
+/** The GitHub search query for a program's merged PRs. */
+export function mergedPrQuery(
+  cfg: Config,
+  program: Program,
+  { author, dayOnly = false }: { author?: string; dayOnly?: boolean } = {}
+): string {
+  const date = (d: Date) =>
+    dayOnly ? d.toISOString().slice(0, 10) : d.toISOString().replace(/\.\d{3}Z$/, "Z");
+  return [
     "is:pr",
     "is:merged",
-    `label:"${label}"`,
-    `merged:${iso(start)}..${iso(end)}`,
-    ...program.repos.map((r) => `repo:${cfg.org}/${r}`),
+    ...(author ? [`author:${author}`] : []),
+    `label:"${program.label}"`,
+    `merged:${date(program.start)}..${date(program.end)}`,
+    ...program.repos.map((repo) => `repo:${cfg.org}/${repo}`),
   ].join(" ");
+}
+
+/** Every merged PR that counts for a program: labeled, in its repos, merged inside its window. */
+export async function searchMergedPrs(env: Env, cfg: Config, program: Program): Promise<MergedPr[]> {
+  const q = mergedPrQuery(cfg, program);
 
   const prs: MergedPr[] = [];
   // The search API returns at most 1000 results (10 pages of 100).

src/index.ts

@@ -3,7 +3,7 @@ import { runCommand, type CommandData, type CommandUser } from "./commands";
 import { DAILY_CRON, getConfig, WELCOME_CRON, type Config, type Env } from "./config";
 import { editInteractionReply, sendMessage } from "./discord";
 import { getLeaderboard } from "./leaderboard";
-import { GithubError } from "./github";
+import { GithubError, mergedPrQuery } from "./github";
 import { linkedMembers } from "./links";
 import { completeLink } from "./linking";
 import { leaderboardMessage, LINK_BUTTON_ID, streaksMessage } from "./messages";
@@ -106,7 +106,7 @@ async function handleLeaderboardPage(request: Request, env: Env, id: string | un
   const cfg = getConfig(env);
   const now = new Date();
   const program = id === undefined ? currentProgram(cfg, now) : findProgram(cfg, id);
-  if (!program) return new Response("not found", { status: 404 });
+  if (!program) return new Response("not found", { status: 404, headers: asJson ? { "Access-Control-Allow-Origin": "*" } : undefined });
   // Browsers and the edge may reuse the response for five minutes.
   const cache = "public, max-age=300";
   try {
@@ -123,8 +123,22 @@ async function handleLeaderboardPage(request: Request, env: Env, id: string | un
       .filter((p) => p.id !== program.id)
       .reverse()
       .map((p) => ({ title: p.title, path: `/leaderboard/${p.id}` }));
+
+    // When linking is enabled, look up which qualifying users have linked their GitHub account.
+    const linking = linkingEnabled(env);
+    let members: Map<string, string> | undefined;
+    if (linking) {
+      const qualifying = board.rows.filter((r) => r.qualifies).map((r) => r.login);
+      try {
+        members = await linkedMembers(env, qualifying);
+      } catch (err) {
+        console.error("failed to look up linked members", err);
+      }
+    }
+
     const html = leaderboardPage(board, program, { issues: env.ISSUES_URL, discord: env.DISCORD_INVITE, self, image, others }, now, {
-      linking: linkingEnabled(env),
+      linking,
+      members,
     });
     return new Response(html, {
       headers: {
@@ -141,11 +155,21 @@ async function handleLeaderboardPage(request: Request, env: Env, id: string | un
     console.error("leaderboard page failed", err);
     return new Response(limited ? "GitHub is rate limiting us. Try again in a minute." : "The leaderboard is unavailable right now.", {
       status: 503,
-      headers: { "Content-Type": "text/plain; charset=utf-8", "Retry-After": "60" },
+      headers: { "Content-Type": "text/plain; charset=utf-8", "Retry-After": "60", ...(asJson ? { "Access-Control-Allow-Origin": "*" } : {}) },
     });
   }
 }
 
+// The cache key for a drawn image: its path and the version worked out here,
+// whatever `?v=` the request carries. Keyed on the address asked for, every new
+// `?v=` value would mean a new picture to draw. The key is always a GET, because
+// the cache only takes those, so a HEAD finds and stores the image under the
+// same key as a GET (otherwise the cache would throw, and the image would be
+// left empty for the next GET).
+function imageKey(path: string, version: string, request: Request): Request {
+  return new Request(new URL(`${path}?v=${encodeURIComponent(version)}`, request.url).href);
+}
+
 // A program's leaderboard as a share image: /leaderboard/{id}.png. The page's
 // og:image points here.
 async function handleLeaderboardImage(request: Request, env: Env, ctx: ExecutionContext, id: string): Promise<Response> {
@@ -156,13 +180,6 @@ async function handleLeaderboardImage(request: Request, env: Env, ctx: Execution
 
   // The image is the same for everyone and costs some CPU to draw, so keep it at the edge.
   const cache = typeof caches !== "undefined" ? await caches.open("cards").catch(() => undefined) : undefined;
-  // The cache only takes GET requests, and it keys on the address. So a HEAD
-  // asks for the image under the key a GET uses, and stores it there too:
-  // otherwise the cache would throw, and the image would be left empty for the
-  // next GET.
-  const cacheKey = request.method === "GET" ? request : new Request(request.url);
-  const cached = await cache?.match(cacheKey);
-  if (cached) return cached;
 
   let board;
   try {
@@ -175,6 +192,10 @@ async function handleLeaderboardImage(request: Request, env: Env, ctx: Execution
     });
   }
 
+  const cacheKey = imageKey(`/leaderboard/${program.id}.png`, boardVersion(board.rows, program, now), request);
+  const cached = await cache?.match(cacheKey);
+  if (cached) return cached;
+
   // Loaded on demand: this module pulls in the rasterizer and the fonts.
   const { fetchAvatar, renderPng } = await import("./card-image");
   const avatars = await Promise.all(board.rows.slice(0, BOARD_ROWS).map((row) => fetchAvatar(row.login)));
@@ -234,16 +255,6 @@ async function handleContributor(
   const program = findProgram(cfg, programId);
   if (!program) return new Response("not found", { status: 404 });
 
-  // The card is the same for everyone and costs some CPU to draw, so keep it at the edge.
-  const cache = asPng && typeof caches !== "undefined" ? await caches.open("cards").catch(() => undefined) : undefined;
-  // The cache only takes GET requests, and it keys on the address. So a HEAD
-  // asks for the card under the key a GET uses, and stores it there too:
-  // otherwise the cache would throw, and the card would be left empty for the
-  // next GET.
-  const cacheKey = request.method === "GET" ? request : new Request(request.url);
-  const cached = await cache?.match(cacheKey);
-  if (cached) return cached;
-
   let board;
   try {
     board = await getLeaderboard(env, cfg, program);
@@ -257,26 +268,40 @@ async function handleContributor(
   const index = board.rows.findIndex((r) => r.login.toLowerCase() === login.toLowerCase());
   const row = board.rows[index];
   const boardPath = `/leaderboard/${program.id}`;
+  const status = programStatus(program, now);
   if (!row) {
     if (asPng) return new Response("not found", { status: 404 });
     const body = noticePage(
       "No card yet",
       [
         `${login} has no counted pull requests in ${program.title}.`,
-        `A card appears with the first merged pull request that counts. The leaderboard is at ${new URL(boardPath, request.url).href}`,
+        status === "ended"
+            ? `${program.title} is over, so no more pull requests can count.`
+          : "A card appears with the first merged pull request that counts.",
       ],
       false,
-      "No card"
+      "No card",
+      [
+        { label: "See the leaderboard", href: boardPath },
+        { label: "Find an issue", href: env.ISSUES_URL },
+      ]
     );
     return new Response(body, { status: 404, headers: { ...PAGE_HEADERS, "Cache-Control": "no-store", "Content-Security-Policy": CONTRIBUTOR_CSP } });
   }
 
   const rank = index + 1;
   const of = board.rows.length;
-  const status = programStatus(program, now);
   const path = `${boardPath}/u/${encodeURIComponent(row.login)}`;
+  // The version makes share previews pick up a new image when the numbers change.
+  const version = `${row.count}-${rank}-${of}-${status}`;
 
   if (asPng) {
+    // The card is the same for everyone and costs some CPU to draw, so keep it at the edge.
+    const cache = typeof caches !== "undefined" ? await caches.open("cards").catch(() => undefined) : undefined;
+    const cacheKey = imageKey(`${path}.png`, version, request);
+    const cached = await cache?.match(cacheKey);
+    if (cached) return cached;
+
     // Loaded on demand: this module pulls in the rasterizer and the fonts.
     const { fetchAvatar, renderPng } = await import("./card-image");
     const svg = cardSvg({
@@ -296,9 +321,7 @@ async function handleContributor(
     return response;
   }
 
-  // The version makes share previews pick up a new image when the numbers change.
-  const version = `${row.count}-${rank}-${of}-${status}`;
-  const prsQuery = `org:${cfg.org} is:pr is:merged author:${row.login} label:"${program.label}" merged:${program.start.toISOString().slice(0, 10)}..${program.end.toISOString().slice(0, 10)}`;
+  const prsQuery = mergedPrQuery(cfg, program, { author: row.login, dayOnly: true });
   const html = contributorPage(
     { program, row, rank, of, generatedAt: board.generatedAt },
     {

src/messages.ts

@@ -156,14 +156,21 @@ export function leaderboardMessage(
     const prs = `${row.count} merged ${row.count === 1 ? "PR" : "PRs"}`;
     // Same as the chips on the page: a countdown while it runs, nothing once it's over.
     const countdown = opts.final ? "" : ` · ${minPrs - row.count} more to qualify`;
-    const note = row.winner ? (opts.final ? " · winner" : " · in the top " + topN) : row.qualifies ? " · qualifies" : countdown;
+    const note = row.winner
+      ? opts.final ? " · winner" : " · in the top " + topN
+      : row.qualifies ? (opts.final ? " · qualified" : " · qualifies") : countdown;
     return `${i + 1}. ${who(row.login)} · ${prs}${note}`;
   });
   const winners = board.rows.filter((r) => r.winner);
 
   let description: string;
   if (lines.length === 0) {
-    description = "No merged PRs counted yet. Pick a starter issue and be the first.";
+    if (opts.final) {
+      description = "No merged pull requests were counted.";
+      if (opts.finalNote) description += `\n\n${opts.finalNote}`;
+    } else {
+      description = "No merged PRs counted yet. Pick a starter issue and be the first.";
+    }
   } else {
     description = lines.join("\n");
     if (opts.final) {