ci: Automatically add milestones to prs and issues (#44357)

* automatically add milestones to prs and issues

* test code

* test code

* test commit (#3138)

* test commit (#43320)

* test commit (#44357)

* remove test code

* add newlines

* prefer older version milestones

.github/workflows/check-milestone.yml

-name: Milestone Reminder
+name: Automatically set milestone
+run-name: Set milestone for ${{ github.event.head_commit.message }}
 
 on:
-  pull_request:
-    types: [closed]
+  push:
     branches:
-      - "master"
+      - release-x.**
 
 jobs:
-  milestone-reminder:
-    if: github.event.pull_request.merged == true
-    name: Remind to set milestone
+  set-milestone:
+    name: Automatically set milestone
     runs-on: ubuntu-22.04
     timeout-minutes: 5
     permissions:
       pull-requests: write
-      issues: read
+      issues: write
+      contents: read
     steps:
       - uses: actions/checkout@v4
         with:
           sparse-checkout: release
+          ref: master # we want the logic from master, even though we're triggering on a release branch
       - name: Prepare build scripts
         run: cd ${{ github.workspace }}/release && yarn && yarn build
       - uses: actions/github-script@v7
         with:
           script: | # js
-            const { getLinkedIssues } = require('${{ github.workspace }}/release/dist/index.cjs');
+            const { setMilestoneForCommits } = require('${{ github.workspace }}/release/dist/index.cjs');
             const owner = context.repo.owner;
             const repo = context.repo.repo;
 
-            const pullNumber = '${{ github.event.pull_request.number }}';
+            const branchName = '${{ github.ref }}';
+            const commitMessage = '${{ github.event.head_commit.message }}';
 
-            function getMilestone(pullOrIssue) {
-              // pull has pull.data.milestone, issue has issue.milestone
-              return pullOrIssue?.data?.milestone || pullOrIssue?.milestone;
-            }
-
-            const pull = await github.rest.pulls.get({
+            await setMilestoneForCommits({
+              github,
               owner,
               repo,
-              pull_number: pullNumber,
+              branchName,
+              commitMessages: [commitMessage],
             });
-
-            if (getMilestone(pull)) {
-              console.log("Pull request has milestone", pull.data.milestone?.title);
-              process.exit(0);
-            }
-
-            // the github API doesn't expose linked issues, so we have to parse the body ourselves
-            const prDescription = pull.data.body;
-            const linkedIssues = getLinkedIssues(prDescription);
-
-            if (Array.isArray(linkedIssues) && linkedIssues.length > 0) {
-              console.log('linked issue(s) found');
-
-              linkedIssues.forEach(async (issueId, index) => {
-                const issue = await github.rest.issues.get({
-                  owner,
-                  repo,
-                  issue_number: issueId,
-                });
-
-                const milestone = getMilestone(issue);
-
-                if (milestone) {
-                  console.log(`Linked issue #${issueId} has milestone`, milestone.title);
-                  index === linkedIssues.length -1 && process.exit(0);
-                } else {
-                    console.log("No milestone found");
-
-                    const author = pull.data.user.login;
-                    const guideLink = "https://www.notion.so/metabase/Metabase-Branching-Strategy-6eb577d5f61142aa960a626d6bbdfeb3?pvs=4#3dea255ffa3b4f74942a227844e889fa";
-                    const message = `@${author} Did you forget to add a milestone to the issue #${issueId} linked in this PR? _[When and where should I add a milestone?](${guideLink})_`;
-
-                    await github.rest.issues.createComment({
-                      owner,
-                      repo,
-                      issue_number: pullNumber,
-                      body: message,
-                    });
-
-                    // Exit as soon as we found an issue without a milestone and alerted the author.
-                    process.exit(0);
-                }
-              });
-            }

release/src/github.ts

@@ -7,7 +7,7 @@ import {
   isValidVersionString,
 } from "./version-helpers";
 
-const getMilestones = async ({
+export const getMilestones = async ({
   github,
   owner,
   repo,

release/src/index.ts

 export * from "./backports";
 export * from "./github";
 export * from "./linked-issues";
+export * from "./milestones";
 export * from "./release-notes";
 export * from "./release-status";
 export * from "./slack";

release/src/linked-issues.ts

@@ -12,3 +12,13 @@ export function getLinkedIssues(body: string) {
 
   return null;
 }
+
+export function getPRsFromCommitMessage(message: string) {
+  const result = [ ...message.matchAll(/\(#(\d+)\)/g) ];
+  if (!result.length) {
+    console.log('no pr found in commit message', message);
+    return null;
+  }
+
+  return result.map(r => Number(r[1]));
+}

release/src/linked-issues.unit.spec.ts

-import { getLinkedIssues } from "./linked-issues";
+import { getLinkedIssues, getPRsFromCommitMessage } from "./linked-issues";
 
 const closingKeywords = [
   "Close",
@@ -107,3 +107,27 @@ describe("getLinkedIssues", () => {
     });
   });
 });
+
+describe("getPRsFromCommitMessage", () => {
+  it("should return `null` when no PR is found", () => {
+    expect(getPRsFromCommitMessage("")).toBeNull();
+    expect(getPRsFromCommitMessage("Lorem ipsum dolor sit amet.")).toBeNull();
+    expect(getPRsFromCommitMessage("Fix #123.")).toBeNull();
+    expect(getPRsFromCommitMessage("Fix#123.")).toBeNull();
+    expect(getPRsFromCommitMessage("Fix # 123.")).toBeNull();
+    expect(getPRsFromCommitMessage("123 456")).toBeNull();
+    expect(getPRsFromCommitMessage("123 #456)")).toBeNull();
+    expect(getPRsFromCommitMessage("123 (#456")).toBeNull();
+    expect(getPRsFromCommitMessage("123 (#456.99)")).toBeNull();
+  });
+
+  it("should return the PR id for a single pr backport", () => {
+    expect(getPRsFromCommitMessage("Backport (#123)")).toEqual([123]);
+    expect(getPRsFromCommitMessage("Backport (#123456) !")).toEqual([123456]);
+  });
+
+  it("should return the PR id for a message with multiple backport PRs", () => {
+    expect(getPRsFromCommitMessage("Backport (#123) (#456)")).toEqual([123, 456]);
+    expect(getPRsFromCommitMessage("Backport (#1234) and (#4567)")).toEqual([1234, 4567]);
+  });
+});

release/src/milestones.ts

+import { getMilestones } from "./github";
+import { getLinkedIssues, getPRsFromCommitMessage } from "./linked-issues";
+import type { Issue, GithubProps, Milestone } from "./types";
+import {
+  getMajorVersion,
+  getVersionFromReleaseBranch,
+} from "./version-helpers";
+
+function isBackport(pullRequest: Issue) {
+  return pullRequest.title.includes('backport') ||
+    (
+      Array.isArray(pullRequest.labels) &&
+      pullRequest.labels.some((label) => label.name === 'was-backported')
+    );
+}
+
+// for auto-setting milestones, we don't ever want to auto-set a patch milestone
+// which we release VERY rarely
+function ignorePatches(version: string) {
+  return version.split('.').length < 4;
+}
+
+function versionSort(a: string, b: string) {
+  const [aMajor, aMinor] = a.split('.').map(Number);
+  const [bMajor, bMinor] = b.split('.').map(Number);
+
+  if (aMajor !== bMajor) {
+    return aMajor - bMajor;
+  }
+
+  if (aMinor !== bMinor) {
+    return aMinor - bMinor;
+  }
+
+  return 0;
+}
+
+const isNotNull = <T>(value: T | null): value is T => value !== null;
+
+
+async function getOriginalPR({
+  github,
+  repo,
+  owner,
+  pullRequestNumber,
+}: GithubProps & { pullRequestNumber: number }) {
+  // every PR in the release branch should have a pr number
+  // it could be a backport PR or an original PR
+  const pull = await github.rest.pulls.get({
+    owner,
+    repo,
+    pull_number: pullRequestNumber,
+  });
+
+  if (pull?.data && isBackport(pull.data)) {
+    return getOriginalPR({
+      github,
+      repo,
+      owner,
+      pullRequestNumber: pull.data.number
+    });
+  }
+
+  const linkedIssues = await getLinkedIssues(pull.data.body ?? '');
+
+  if (linkedIssues) {
+    console.log('found linked issue for PR', pull.data.number, linkedIssues);
+    return linkedIssues.map(Number);
+  }
+  console.log("no linked issues found in PR body", pull.data.number);
+  return [pull.data.number];
+}
+
+async function setMilestone({ github, owner, repo, issueNumber, milestone }: GithubProps & { issueNumber: number, milestone: Milestone }) {
+  // we can use this for both issues and PRs since they're the same for many purposes in github
+  const issue = await github.rest.issues.get({
+    owner,
+    repo,
+    issue_number: issueNumber,
+  });
+
+  if (!issue.data.milestone) {
+    return github.rest.issues.update({
+      owner,
+      repo,
+      issue_number: issueNumber,
+      milestone: milestone.number,
+    });
+  }
+
+  const existingMilestone = issue.data.milestone;
+
+  if (existingMilestone.number === milestone.number) {
+    console.log(`Issue ${issueNumber} is already tagged with this ${milestone.title} milestone`);
+    return;
+  }
+
+  const existingMilestoneIsNewer = versionSort(existingMilestone.title, milestone.title) > 0;
+
+  // if existing milestone is newer, change it
+  if (existingMilestoneIsNewer) {
+    console.log(`Changing milestone from ${existingMilestone.title} to ${milestone.title}`);
+
+    await github.rest.issues.update({
+      owner,
+      repo,
+      issue_number: issueNumber,
+      milestone: milestone.number,
+    });
+  }
+
+  const commentBody = existingMilestoneIsNewer
+    ? `🚀 This should also be released by [v${existingMilestone.title}](${existingMilestone.html_url})`
+    : `🚀 This should also be released by [v${milestone.title}](${milestone.html_url})`;
+
+  console.log(`Adding comment to issue ${issueNumber} that already has milestone ${existingMilestone.title}`);
+
+  return github.rest.issues.createComment({
+    owner,
+    repo,
+    issue_number: issueNumber,
+    body: commentBody,
+  });
+}
+
+// get the next open milestone (e.g. 0.57.8) for the given major version (e.g 57)
+export function getNextMilestone(
+  { openMilestones, majorVersion }:
+  { openMilestones: Milestone[], majorVersion: number | string }
+): Milestone | undefined {
+  const milestonesForThisMajorVersion = openMilestones
+    .filter(milestone => milestone.title.startsWith(`0.${majorVersion}`))
+    .filter(milestone => ignorePatches(milestone.title))
+    .sort((a, b) => versionSort(a.title, b.title));
+
+  const nextMilestone = milestonesForThisMajorVersion[0];
+
+  return nextMilestone;
+}
+
+export async function setMilestoneForCommits({
+  github,
+  owner,
+  repo,
+  branchName,
+  commitMessages,
+}: GithubProps & { commitMessages: string[], branchName: string}) {
+  // figure out milestone
+  const branchVersion = getVersionFromReleaseBranch(branchName);
+  const majorVersion = getMajorVersion(branchVersion);
+  const openMilestones = await getMilestones({ github, owner, repo });
+  const nextMilestone = getNextMilestone({ openMilestones, majorVersion });
+
+  if (!nextMilestone) {
+    throw new Error(`No open milestone found for major version ${majorVersion}`);
+  }
+
+  console.log('Next milestone:', nextMilestone.title);
+
+  // figure out issue or PR
+  const PRsToCheck = commitMessages.flatMap(getPRsFromCommitMessage).filter(isNotNull);
+
+  if (!PRsToCheck.length) {
+    throw new Error('No PRs found in commit messages');
+  }
+
+  console.log(`Checking ${PRsToCheck.length} PRs for issues to tag`);
+
+  const issuesToTag = [];
+
+  for (const prNumber of PRsToCheck) { // for loop to avoid rate limiting
+    issuesToTag.push(...(await getOriginalPR({
+      github,
+      owner,
+      repo,
+      pullRequestNumber: prNumber,
+    })));
+  }
+
+  console.log(`Tagging ${issuesToTag.length} issues with milestone ${nextMilestone.title}`)
+
+  for (const issueNumber of issuesToTag) { // for loop to avoid rate limiting
+    await setMilestone({ github, owner, repo, issueNumber, milestone: nextMilestone });
+  }
+}

release/src/milestones.unit.spec.ts

+import { getNextMilestone } from "./milestones";
+import type { Milestone } from "./types";
+
+describe('milestones', () => {
+  const testMilestones = [
+    { number: 577, title: '0.57.7' },
+    { number: 578, title: '0.57.8' },
+    { number: 580, title: '0.58' },
+    { number: 581, title: '0.58.1' },
+    { number: 599, title: '0.59.9' },
+    { number: 5910, title: '0.59.10' },
+    { number: 600, title: '0.60' },
+    { number: 6010, title: '0.60.10' },
+    { number: 6111, title: '0.61.1.1' },
+    { number: 612, title: '0.61.2' },
+    { number: 613, title: '0.61.3'},
+    { number: 621, title: '0.62.1'},
+    { number: 6220, title: '0.62.20'},
+    { number: 62300, title: '0.62.300'},
+    { number: 630, title: '0.63'},
+  ] as Milestone[];
+
+  describe('getNextMilestone', () => {
+    it('can detect the next milestone for minor versions', () => {
+      expect(getNextMilestone({
+        openMilestones: testMilestones,
+        majorVersion: 57,
+      })?.number).toBe(577);
+
+      expect(getNextMilestone({
+        openMilestones: testMilestones,
+        majorVersion: 59,
+      })?.number).toBe(599);
+
+      expect(getNextMilestone({
+        openMilestones: testMilestones,
+        majorVersion: 62,
+      })?.number).toBe(621);
+    });
+
+    it('can detect the next milestone for major versions', () => {
+      expect(getNextMilestone({
+        openMilestones: testMilestones,
+        majorVersion: 63,
+      })?.number).toBe(630);
+    });
+
+    it('can detect the next milestone for mixed major and minor versions', () => {
+      expect(getNextMilestone({
+        openMilestones: testMilestones,
+        majorVersion: 58,
+      })?.number).toBe(580);
+
+      expect(getNextMilestone({
+        openMilestones: testMilestones,
+        majorVersion: 60,
+      })?.number).toBe(600);
+    });
+
+    it('should return undefined when milestone is missing', () => {
+      expect(getNextMilestone({
+        openMilestones: testMilestones,
+        majorVersion: 56,
+      })?.number).toBe(undefined);
+    });
+
+    it('should ignore patch versions', () => {
+      expect(getNextMilestone({
+        openMilestones: testMilestones,
+        majorVersion: 61,
+      })?.number).toBe(612);
+    });
+  });
+});

release/src/types.ts

@@ -15,14 +15,16 @@ export interface CacheType {
   slackChannelId?: string;
   preReleaseActionId?: number;
 }
-
-export interface ReleaseProps {
+export interface GithubProps {
   owner: string;
   repo: string;
-  version: string;
   github: Octokit;
 }
 
+export interface ReleaseProps extends GithubProps {
+  version: string;
+}
+
 export interface VersionInfo {
   version: string;
   released: string;
@@ -43,3 +45,15 @@ export type Issue = {
   assignee: null |  { login: string };
   created_at: string;
 };
+
+export type Milestone =  {
+  url: string;
+  html_url: string;
+  labels_url: string;
+  id: number;
+  node_id: string;
+  number: number;
+  state: "open" | "closed";
+  title: string;
+  description: string | null;
+};

release/tsconfig.json

@@ -9,7 +9,8 @@
     "experimentalDecorators": true,
     "forceConsistentCasingInFileNames": true,
     "lib": ["dom", "dom.iterable", "esnext"],
-    "outDir": "./dist"
+    "outDir": "./dist",
+    "target": "esnext"
   },
   "include": ["src/**/*.ts", "src/index.mts"]
 }