From c91f6f9579c8392f279f2e595e99af5357445161 Mon Sep 17 00:00:00 2001 From: Valentin Date: Mon, 28 Oct 2024 01:41:52 +0200 Subject: [PATCH] New points attribution for new prs in new repos (#4902) * New points attribution for new prs in new repos * Don't distribute 100 points if pr owner is the same with repo owner * fix logic --------- Co-authored-by: mattcasey --- .../__tests__/recordMergedPullRequest.spec.ts | 108 ++++++++++++++++++ .../github/getRecentPullRequestsByUser.ts | 12 +- .../recordMergedPullRequest.ts | 24 +++- packages/scoutgame/src/testing/database.ts | 4 +- 4 files changed, 141 insertions(+), 7 deletions(-) diff --git a/apps/scoutgamecron/src/tasks/processBuilderActivity/__tests__/recordMergedPullRequest.spec.ts b/apps/scoutgamecron/src/tasks/processBuilderActivity/__tests__/recordMergedPullRequest.spec.ts index 80a79a8d28..1757bc43e7 100644 --- a/apps/scoutgamecron/src/tasks/processBuilderActivity/__tests__/recordMergedPullRequest.spec.ts +++ b/apps/scoutgamecron/src/tasks/processBuilderActivity/__tests__/recordMergedPullRequest.spec.ts @@ -511,4 +511,112 @@ describe('recordMergedPullRequest', () => { }); expect(scoutActivities).toBe(0); }); + + it('should award correct points for first and second PRs in different repos and in the last 7 days', async () => { + const builder = await mockBuilder(); + const repo1 = await mockRepo(); + const repo2 = await mockRepo(); + const scout = await mockScout(); + + await mockBuilderNft({ + builderId: builder.id, + season: currentSeason, + owners: [scout] + }); + + // First PR in repo1 + const firstPR = mockPullRequest({ + mergedAt: DateTime.now().minus({ days: 2 }).toISO(), + createdAt: DateTime.now().minus({ days: 2 }).toISO(), + state: 'MERGED', + author: builder.githubUser, + repo: repo1 + }); + + (getRecentPullRequestsByUser as jest.Mock).mockResolvedValue([]); + + await recordMergedPullRequest({ pullRequest: firstPR, repo: repo1, season: currentSeason }); + + // Second PR in repo2 + const secondPR = mockPullRequest({ + mergedAt: new Date().toISOString(), + createdAt: new Date().toISOString(), + state: 'MERGED', + author: builder.githubUser, + repo: repo2 + }); + + (getRecentPullRequestsByUser as jest.Mock).mockResolvedValue([]); + + await recordMergedPullRequest({ pullRequest: secondPR, repo: repo2, season: currentSeason }); + + const gemsReceipts = await prisma.gemsReceipt.findMany({ + where: { + event: { + builderId: builder.id + } + }, + orderBy: { + createdAt: 'asc' + } + }); + + expect(gemsReceipts).toHaveLength(2); + expect(gemsReceipts[0].value).toBe(100); // First PR in a new repo should award 100 points + expect(gemsReceipts[1].value).toBe(10); // Second PR in a new repo (in the last 7 days) should award only 10 points + }); + + it('should award full points for PRs in different repos when created more than 7 days apart', async () => { + const builder = await mockBuilder(); + const repo1 = await mockRepo(); + const repo2 = await mockRepo(); + const scout = await mockScout(); + + await mockBuilderNft({ + builderId: builder.id, + season: currentSeason, + owners: [scout] + }); + + // First PR in repo1 + const firstPR = mockPullRequest({ + mergedAt: DateTime.now().minus({ days: 10 }).toISO(), + createdAt: DateTime.now().minus({ days: 10 }).toISO(), + state: 'MERGED', + author: builder.githubUser, + repo: repo1 + }); + + (getRecentPullRequestsByUser as jest.Mock).mockResolvedValue([]); + + await recordMergedPullRequest({ pullRequest: firstPR, repo: repo1, season: currentSeason }); + + // Second PR in repo2, more than 7 days later + const secondPR = mockPullRequest({ + mergedAt: new Date().toISOString(), + createdAt: new Date().toISOString(), + state: 'MERGED', + author: builder.githubUser, + repo: repo2 + }); + + (getRecentPullRequestsByUser as jest.Mock).mockResolvedValue([]); + + await recordMergedPullRequest({ pullRequest: secondPR, repo: repo2, season: currentSeason }); + + const gemsReceipts = await prisma.gemsReceipt.findMany({ + where: { + event: { + builderId: builder.id + } + }, + orderBy: { + createdAt: 'asc' + } + }); + + expect(gemsReceipts).toHaveLength(2); + expect(gemsReceipts[0].value).toBe(100); // First PR should award 100 points + expect(gemsReceipts[1].value).toBe(100); // Second PR should also award 100 points since it's after 7 days + }); }); diff --git a/apps/scoutgamecron/src/tasks/processBuilderActivity/github/getRecentPullRequestsByUser.ts b/apps/scoutgamecron/src/tasks/processBuilderActivity/github/getRecentPullRequestsByUser.ts index 0e5af5faca..b578f39851 100644 --- a/apps/scoutgamecron/src/tasks/processBuilderActivity/github/getRecentPullRequestsByUser.ts +++ b/apps/scoutgamecron/src/tasks/processBuilderActivity/github/getRecentPullRequestsByUser.ts @@ -14,6 +14,11 @@ type PullRequestByUser = { createdAt: string; mergedAt?: string; state: 'CLOSED' | 'MERGED'; + repository: { + owner: { + login: string; + }; + }; }; const getPrsByUser = ` @@ -29,7 +34,12 @@ const getPrsByUser = ` closedAt createdAt mergedAt - state + state, + repository { + owner { + login + } + } } } } diff --git a/apps/scoutgamecron/src/tasks/processBuilderActivity/recordMergedPullRequest.ts b/apps/scoutgamecron/src/tasks/processBuilderActivity/recordMergedPullRequest.ts index 8800d9a420..03cd5a2d65 100644 --- a/apps/scoutgamecron/src/tasks/processBuilderActivity/recordMergedPullRequest.ts +++ b/apps/scoutgamecron/src/tasks/processBuilderActivity/recordMergedPullRequest.ts @@ -7,7 +7,7 @@ import type { } from '@charmverse/core/prisma-client'; import { prisma } from '@charmverse/core/prisma-client'; import type { Season } from '@packages/scoutgame/dates'; -import { getStartOfSeason, getWeekFromDate, isToday, streakWindow } from '@packages/scoutgame/dates'; +import { getStartOfSeason, getWeekStartEnd, getWeekFromDate, isToday, streakWindow } from '@packages/scoutgame/dates'; import { isTruthy } from '@packages/utils/types'; import { DateTime } from 'luxon'; @@ -43,6 +43,7 @@ export async function recordMergedPullRequest({ throw new Error('Pull request was not merged'); } const week = getWeekFromDate(now.toJSDate()); + const { start: startOfWeek } = getWeekStartEnd(now.toJSDate()); const start = getStartOfSeason(season as Season); const previousGitEvents = await prisma.githubEvent.findMany({ @@ -86,7 +87,7 @@ export async function recordMergedPullRequest({ return; } - // check our data to see if this is the first merged PR, and if so, check the Github API to confirm + // check our data to see if this is the first merged PR in a repo in the last 7 days, and if so, check the Github API to confirm const totalMergedPullRequests = await prisma.githubEvent.count({ where: { createdBy: pullRequest.author.id, @@ -95,7 +96,19 @@ export async function recordMergedPullRequest({ } }); - let isFirstMergedPullRequest = totalMergedPullRequests === 0; + const recentFirstMergedPullRequests = await prisma.githubEvent.count({ + where: { + createdBy: pullRequest.author.id, + type: 'merged_pull_request', + isFirstPullRequest: true, + createdAt: { + gte: startOfWeek.toJSDate() + } + } + }); + const hasFirstMergedPullRequestAlreadyThisWeek = recentFirstMergedPullRequests > 0; + + let isFirstMergedPullRequest = totalMergedPullRequests === 0 && !hasFirstMergedPullRequestAlreadyThisWeek; if (isFirstMergedPullRequest && !skipFirstMergedPullRequestCheck) { // double-check using Github API in case the previous PR was not recorded by us const prs = await getRecentPullRequestsByUser({ @@ -103,7 +116,10 @@ export async function recordMergedPullRequest({ repoNameWithOwner: pullRequest.repository.nameWithOwner, username: pullRequest.author.login }); - if (prs.filter((pr) => pr.number !== pullRequest.number).length > 0) { + if ( + prs.filter((pr) => pr.number !== pullRequest.number || pr.repository.owner.login === pullRequest.author.login) + .length > 0 + ) { isFirstMergedPullRequest = false; } } diff --git a/packages/scoutgame/src/testing/database.ts b/packages/scoutgame/src/testing/database.ts index f309df6b54..d0926e7645 100644 --- a/packages/scoutgame/src/testing/database.ts +++ b/packages/scoutgame/src/testing/database.ts @@ -200,8 +200,8 @@ export function mockRepo(fields: Partial & { owner?: string } = {}) data: { ...fields, id: fields.id ?? randomLargeInt(), - name: fields.name ?? `test_repo_${+Math.random()}`, - owner: fields.owner ?? `test_owner_${Math.random()}`, + name: fields.name ?? `test_repo_${Math.floor(Math.random() * 1000) + 1}`, + owner: fields.owner ?? `test_owner_${Math.floor(Math.random() * 1000) + 1}`, ownerType: fields.ownerType ?? 'org', defaultBranch: fields.defaultBranch ?? 'main' }