test: false alarm, it only returned for merged pulls. added all pulls…

… as well
ubiquity-os-marketplace · Feb 27, 2024 · 5d7de6f · 5d7de6f
1 parent 7b5097c
commit 5d7de6f
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 36 deletions.
diff --git a/src/data-collection/collect-linked-pulls.test.ts b/src/data-collection/collect-linked-pulls.test.ts
@@ -1,6 +1,6 @@
 import { config } from "dotenv";
 import { IssueParams, getIssueEvents, parseGitHubUrl } from "../start";
-import collectLinkedPulls from "./collect-linked-pulls";
+import { collectLinkedMergedPulls, collectLinkedPulls } from "./collect-linked-pulls";
 
 import ISSUE_CROSS_REPO_LINK from "./fixtures/issue-89.json"; // pr188 is linked to this issue
 import ISSUE_SAME_REPO_LINK from "./fixtures/issue-90.json"; // pr91 is linked to this issue
@@ -25,19 +25,19 @@ process.argv = ["path/to/node", "path/to/script", `--auth=${GITHUB_TOKEN}`];
 
 describe("Artificial scenarios for linking pull requests to issues", () => {
   it("should return an empty array when the issue does not have any associated link events", async () => {
-    const result = await collectLinkedPulls(PARAMS_ISSUE_NO_LINK);
+    const result = await collectLinkedMergedPulls(PARAMS_ISSUE_NO_LINK);
     expect(result).toEqual([]);
   });
 
   it("should identify and return the merged, linked pull requests that originate from the same issue within the same repository", async () => {
-    const result = await collectLinkedPulls(PARAMS_ISSUE_SAME_REPO_LINK);
+    const result = await collectLinkedMergedPulls(PARAMS_ISSUE_SAME_REPO_LINK);
     const expectedUrl = PR_SAME_REPO_LINK.html_url;
     const matchingLinks = result.filter((link) => link.source.issue.html_url === expectedUrl);
     expect(matchingLinks.length).toBe(1);
   });
 
   it("should identify and return the merged, linked pull requests that originate from a specific issue, regardless of the repository they are located in within the organization", async () => {
-    const result = await collectLinkedPulls(PARAMS_ISSUE_CROSS_REPO_LINK);
+    const result = await collectLinkedMergedPulls(PARAMS_ISSUE_CROSS_REPO_LINK);
     const expectedUrl = PR_CROSS_REPO_LINK.html_url;
     const matchingLinks = result.filter((link) => link.source.issue.html_url === expectedUrl);
     expect(matchingLinks.length).toBe(1);
@@ -46,39 +46,39 @@ describe("Artificial scenarios for linking pull requests to issues", () => {
 
 describe("Real-world scenarios for linking pull requests to issues", () => {
   it("For the issue 'ubiquibot/comment-incentives/issues/22', the test should identify and return all the merged, linked pull requests that originate from this issue within the same repository 'ubiquibot/comment-incentives'", async () => {
-    const result = await collectLinkedPulls(parseGitHubUrl("https://github.com/ubiquibot/comment-incentives/issues/22"));
+    const result = await collectLinkedMergedPulls(parseGitHubUrl("https://github.com/ubiquibot/comment-incentives/issues/22"));
     const expectedUrl = "https://github.com/ubiquibot/comment-incentives/pull/25";
     result.forEach((res) => expect(res.source.issue.html_url).toMatch(/\/pull\/\d+$/));
     const matchingLinks = result.filter((res) => res.source.issue.html_url === expectedUrl);
     expect(matchingLinks.length).toBeGreaterThan(0);
   });
 
-  it("For the issue 'ubiquity/pay.ubq.fi/issues/138', the test should identify and return all the merged, linked pull requests that originate from this issue within the same repository 'ubiquity/pay.ubq.fi'", async () => {
+  it("For the issue 'ubiquity/pay.ubq.fi/issues/138', the test should identify and return all the linked pull requests that originate from this issue within the same repository 'ubiquity/pay.ubq.fi'", async () => {
     const result = await collectLinkedPulls(parseGitHubUrl("https://github.com/ubiquity/pay.ubq.fi/issues/138"));
-    const expectedUrl = "https://github.com/https://github.com/ubiquity/pay.ubq.fi/pull/173";
+    const expectedUrl = "https://github.com/ubiquity/pay.ubq.fi/pull/173";
     result.forEach((res) => expect(res.source.issue.html_url).toMatch(/\/pull\/\d+$/));
     const matchingLinks = result.filter((res) => res.source.issue.html_url === expectedUrl);
     expect(matchingLinks.length).toBeGreaterThan(0);
   });
 
   it("For the issue 'ubiquibot/comment-incentives/issues/3', the test should identify and return all the merged, linked pull requests that originate from this issue within the same repository 'ubiquibot/comment-incentives'", async () => {
-    const result = await collectLinkedPulls(parseGitHubUrl("https://github.com/ubiquibot/comment-incentives/issues/3"));
+    const result = await collectLinkedMergedPulls(parseGitHubUrl("https://github.com/ubiquibot/comment-incentives/issues/3"));
     const expectedUrl = "https://github.com/ubiquibot/comment-incentives/pull/4";
     result.forEach((res) => expect(res.source.issue.html_url).toMatch(/\/pull\/\d+$/));
     const matchingLinks = result.filter((res) => res.source.issue.html_url === expectedUrl);
     expect(matchingLinks.length).toBeGreaterThan(0);
   });
 
   it("For the issue 'ubiquibot/comment-incentives/issues/15', the test should identify and return all the merged, linked pull requests that originate from this issue within the same repository 'ubiquibot/comment-incentives'", async () => {
-    const result = await collectLinkedPulls(parseGitHubUrl("https://github.com/ubiquibot/comment-incentives/issues/15"));
+    const result = await collectLinkedMergedPulls(parseGitHubUrl("https://github.com/ubiquibot/comment-incentives/issues/15"));
     const expectedUrl = "https://github.com/ubiquibot/comment-incentives/pull/16";
     result.forEach((res) => expect(res.source.issue.html_url).toMatch(/\/pull\/\d+$/));
     const matchingLinks = result.filter((res) => res.source.issue.html_url === expectedUrl);
     expect(matchingLinks.length).toBeGreaterThan(0);
   });
 
   it("For the issue 'ubiquibot/comment-incentives/issues/19', the test should identify and return all the merged, linked pull requests that originate from this issue within the same repository 'ubiquibot/comment-incentives'", async () => {
-    const result = await collectLinkedPulls(parseGitHubUrl("https://github.com/ubiquibot/comment-incentives/issues/19"));
+    const result = await collectLinkedMergedPulls(parseGitHubUrl("https://github.com/ubiquibot/comment-incentives/issues/19"));
     const expectedUrls = ["https://github.com/ubiquibot/comment-incentives/pull/21", "https://github.com/ubiquibot/comment-incentives/pull/23"];
     expectedUrls.forEach((url) => {
       const matchingLinks = result.filter((res) => res.source.issue.html_url === url);

diff --git a/src/data-collection/collect-linked-pulls.ts b/src/data-collection/collect-linked-pulls.ts
@@ -1,11 +1,16 @@
 import { GitHubLinkEvent, isGitHubLinkEvent } from "../github-types";
 import { IssueParams, getAllTimelineEvents } from "../start";
 
-export default async function collectLinkedPulls(issue: IssueParams) {
+export async function collectLinkedMergedPulls(issue: IssueParams) {
+  // normally we should only use this one to calculate incentives, because this specifies that the pull requests are merged (accepted)
+  const onlyPullRequests = await collectLinkedPulls(issue);
+  return onlyPullRequests.filter((event) => isGitHubLinkEvent(event) && event.source.issue.pull_request?.merged_at);
+}
+export async function collectLinkedPulls(issue: IssueParams) {
+  // this one was created to help with tests, but probably should not be used in the main code
   const issueLinkEvents = await getLinkedEvents(issue);
   const onlyConnected = eliminateDisconnects(issueLinkEvents);
-  const onlyPullRequests = onlyConnected.filter((event) => isGitHubLinkEvent(event) && event.source.issue.pull_request);
-  return onlyPullRequests.filter((event) => isGitHubLinkEvent(event) && event.source.issue.pull_request?.merged_at);
+  return onlyConnected.filter((event) => isGitHubLinkEvent(event) && event.source.issue.pull_request);
 }
 
 function eliminateDisconnects(issueLinkEvents: GitHubLinkEvent[]) {
@@ -37,26 +42,3 @@ async function getLinkedEvents(params: IssueParams): Promise<GitHubLinkEvent[]>
   const issueEvents = await getAllTimelineEvents(params);
   return issueEvents.filter(isGitHubLinkEvent);
 }
-
-// function getLatestLinkEvent(events: GitHubLinkEvent[]) {
-//   if (events.length === 0) {
-//     return null;
-//   } else {
-//     const event = events.pop();
-//     return event ? event : null;
-//   }
-// }
-
-// function connectedOrCrossReferenced(event: GitHubTimelineEvent): event is GitHubLinkEvent {
-//   return event.event === "connected" || event.event === "cross-referenced";
-// }
-
-// async function fetchEvents(params: IssueParams, page: number = 1, perPage: number = 100): Promise<GitHubTimelineEvent[]> {
-//   const octokit = getOctokitInstance();
-//   const response = await octokit.rest.issues.listEventsForTimeline({
-//     ...params,
-//     page,
-//     per_page: perPage,
-//   });
-//   return response.data;
-// }