refactor: check for existing PR before sending event to dep graph int…

…egrator (#1334)

* refactor: move pull-requests files into dep graph integrator

* refactor: check for existing repo in send-to-sns

* refactor: move getExistingPr function into send-to-sns

* refactor: move tests for moved function

* refactor: add instructions to token text in setup script

packages/dependency-graph-integrator/src/index.ts

@@ -1,9 +1,5 @@
 import type { SNSHandler } from 'aws-lambda';
 import { parseEvent, stageAwareOctokit } from 'common/functions';
-import {
-	createPrAndAddToProject,
-	generateBranchName,
-} from 'common/src/pull-requests';
 import type { DependencyGraphIntegratorEvent } from 'common/src/types';
 import type { Config } from './config';
 import { getConfig } from './config';
@@ -12,6 +8,7 @@ import {
 	depGraphPackageManager,
 	generatePrBody,
 } from './file-generator';
+import { createPrAndAddToProject, generateBranchName } from './pull-requests';
 import { enableDependabotAlerts } from './repo-functions';
 import type { StatusCode } from './types';
 

packages/dependency-graph-integrator/src/pull-requests.test.ts

@@ -0,0 +1,10 @@
+import { generateBranchName } from './pull-requests';
+
+describe('generateBranchName', () => {
+	it('does not produce the same branch name twice', () => {
+		const prefix = 'hello';
+		const branch1 = generateBranchName(prefix);
+		const branch2 = generateBranchName(prefix);
+		expect(branch1).not.toEqual(branch2);
+	});
+});

packages/common/src/pull-requests.ts → packages/dependency-graph-integrator/src/pull-requests.ts

@@ -1,9 +1,8 @@
 import { randomBytes } from 'crypto';
-import type { Endpoints } from '@octokit/types';
+import { stageAwareOctokit } from 'common/src/functions';
+import { addPrToProject } from 'common/src/projects-graphql';
 import type { Octokit } from 'octokit';
 import { composeCreatePullRequest } from 'octokit-plugin-create-pull-request';
-import { stageAwareOctokit } from './functions';
-import { addPrToProject } from './projects-graphql';
 
 interface Change {
 	commitMessage: string;
@@ -91,37 +90,6 @@ export async function requestTeamReview(
 	}
 }
 
-type PullRequestParameters =
-	Endpoints['GET /repos/{owner}/{repo}/pulls']['parameters'];
-
-type PullRequest =
-	Endpoints['GET /repos/{owner}/{repo}/pulls']['response']['data'][number];
-
-function isGithubAuthor(pull: PullRequest, author: string) {
-	return pull.user?.login === author && pull.user.type === 'Bot';
-}
-
-export async function getExistingPullRequest(
-	octokit: Octokit,
-	repoName: string,
-	owner: string,
-	author: string,
-) {
-	const pulls = await octokit.paginate(octokit.rest.pulls.list, {
-		owner,
-		repo: repoName,
-		state: 'open',
-	} satisfies PullRequestParameters);
-
-	const found = pulls.filter((pull) => isGithubAuthor(pull, author));
-
-	if (found.length > 1) {
-		console.warn(`More than one PR found on ${repoName} - choosing the first.`);
-	}
-
-	return found[0];
-}
-
 export async function createPrAndAddToProject(
 	stage: string,
 	repoName: string,
@@ -139,53 +107,40 @@ export async function createPrAndAddToProject(
 ) {
 	if (stage === 'PROD') {
 		const ghClient = octokit ?? (await stageAwareOctokit(stage));
-		const existingPullRequest = await getExistingPullRequest(
-			ghClient,
+
+		const pullRequestResponse = await createPullRequest(ghClient, {
 			repoName,
 			owner,
-			`${author}[bot]`,
-		);
+			title: prTitle,
+			body: prBody,
+			branchName: branch,
+			changes: [
+				{
+					commitMessage,
+					files: {
+						[fileName]: fileContents,
+					},
+				},
+			],
+			admins,
+		});
+
+		if (pullRequestResponse?.html_url && pullRequestResponse.number) {
+			console.log(
+				'Pull request successfully created:',
+				pullRequestResponse.html_url,
+			);
 
-		if (!existingPullRequest) {
-			const pullRequestResponse = await createPullRequest(ghClient, {
+			await requestTeamReview(
+				ghClient,
 				repoName,
 				owner,
-				title: prTitle,
-				body: prBody,
-				branchName: branch,
-				changes: [
-					{
-						commitMessage,
-						files: {
-							[fileName]: fileContents,
-						},
-					},
-				],
+				pullRequestResponse.number,
 				admins,
-			});
-
-			if (pullRequestResponse?.html_url && pullRequestResponse.number) {
-				console.log(
-					'Pull request successfully created:',
-					pullRequestResponse.html_url,
-				);
-
-				await requestTeamReview(
-					ghClient,
-					repoName,
-					owner,
-					pullRequestResponse.number,
-					admins,
-				);
-
-				await addPrToProject(stage, repoName, boardNumber, author);
-				console.log('Updated project board');
-			}
-		} else {
-			console.log(
-				`Existing pull request found. Skipping creating a new one.`,
-				existingPullRequest.html_url,
 			);
+
+			await addPrToProject(stage, repoName, boardNumber, author);
+			console.log('Updated project board');
 		}
 	} else {
 		console.log(`Testing generation of ${fileName} for ${repoName}`);

packages/repocop/src/index.ts

@@ -169,6 +169,7 @@ export async function main() {
 		productionWorkflowUsages,
 		repoOwners,
 		dependencyGraphIntegratorRepoCount,
+		octokit,
 	);
 
 	await writeEvaluationTable(repocopRules, prisma);

packages/repocop/src/remediation/dependency_graph-integrator/send-to-sns.test.ts

@@ -8,11 +8,13 @@ import type {
 	Repository,
 	RepositoryWithDepGraphLanguage,
 } from 'common/src/types';
+import type { Octokit } from 'octokit';
 import { removeRepoOwner } from '../shared-utilities';
 import {
 	checkRepoForLanguage,
 	createSnsEventsForDependencyGraphIntegration,
 	doesRepoHaveDepSubmissionWorkflowForLanguage,
+	getExistingPullRequest,
 	getReposWithoutWorkflows,
 } from './send-to-sns';
 
@@ -229,3 +231,85 @@ describe('When getting suitable events to send to SNS', () => {
 		]);
 	});
 });
+
+/**
+ * Create a mocked version of the Octokit SDK that returns a given array of pull requests
+ */
+function mockOctokit(pulls: unknown[]) {
+	return {
+		// eslint-disable-next-line @typescript-eslint/no-unused-vars -- It's just a mock
+		paginate: (arg0: unknown, arg1: unknown) => Promise.resolve(pulls),
+		rest: {
+			pulls: {
+				list: () => {},
+			},
+		},
+	} as Octokit;
+}
+
+describe('getPullRequest', () => {
+	const featureBranch = {
+		head: {
+			ref: 'feature-branch',
+		},
+		user: {
+			login: 'some-user',
+			type: 'User',
+		},
+	};
+
+	const dependabotBranch = {
+		head: {
+			ref: 'integrate-dependabot-abcd',
+		},
+		user: {
+			login: 'gu-dependency-graph-integrator[bot]',
+			type: 'Bot',
+		},
+	};
+
+	const dependabotBranch2 = {
+		...dependabotBranch,
+		head: {
+			ref: 'integrate-dependabot-efgh',
+		},
+	};
+
+	it('should return undefined when no matching branch found', async () => {
+		const pulls = [featureBranch];
+		const foundPull = await getExistingPullRequest(
+			mockOctokit(pulls),
+			'repo',
+			'owner',
+			'gu-dependency-graph-integrator[bot]',
+		);
+		expect(foundPull).toBeUndefined();
+	});
+
+	it('should return pull request when author matches', async () => {
+		const pulls = [featureBranch, dependabotBranch];
+		const foundPull = await getExistingPullRequest(
+			mockOctokit(pulls),
+			'repo',
+			'owner',
+			'gu-dependency-graph-integrator[bot]',
+		);
+		expect(foundPull).toEqual(dependabotBranch);
+	});
+
+	it('should return first pull request that matches and log warning', async () => {
+		const warn = jest.spyOn(console, 'warn');
+		const pulls = [featureBranch, dependabotBranch, dependabotBranch2];
+		const foundPull = await getExistingPullRequest(
+			mockOctokit(pulls),
+			'repo',
+			'owner',
+			'gu-dependency-graph-integrator[bot]',
+		);
+		expect(foundPull).toEqual(dependabotBranch);
+		expect(warn).toHaveBeenCalledWith(
+			'More than one PR found on repo - choosing the first.',
+		);
+		warn.mockRestore();
+	});
+});