Skip to content

Commit 0c5c9c8

Browse files
feat(github-connector): exclude self-reviews from review counts (#20050)
## Summary - Adds an `isSelfReview` boolean to the consolidated `PullRequestReview` record (`true` when the reviewer is the same contributor as the PR author). - Filters `isSelfReview === true` rows out of the top-reviewers leaderboard (`top-contributors`) and per-contributor review stats (`contributor-stats`) so contributors are credited only for reviews on **other people's** PRs. - Both the live ingestion path (`fetch-prs`) and the recompute job (`recompute-pull-request-reviews`) now thread `prAuthorId` to `buildConsolidatedRow`, so re-running either is sufficient to backfill existing rows — no extra migration needed. ## Test plan - [x] `yarn test src/modules/github/pull-request-review/utils/consolidate-reviews.integration-test.ts` — 20 tests passing, including new coverage for the `isSelfReview` helper and `buildConsolidatedRow` payload. - [ ] After merge: re-run `fetch-prs` (or `recompute-pull-request-reviews`) once on a target workspace to backfill `isSelfReview` on existing `PullRequestReview` rows, then verify the top-reviewers leaderboard no longer credits self-reviews. Made with [Cursor](https://cursor.com)
1 parent 41571ea commit 0c5c9c8

11 files changed

Lines changed: 127 additions & 4 deletions

File tree

packages/twenty-apps/community/github-connector/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "github-connector",
3-
"version": "0.1.0",
3+
"version": "0.2.0",
44
"license": "MIT",
55
"engines": {
66
"node": "^24.5.0",

packages/twenty-apps/community/github-connector/src/__tests__/schema.integration-test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ describe('PullRequestReview object', () => {
7777
expect(names).toContain('firstSubmittedAt');
7878
expect(names).toContain('lastSubmittedAt');
7979
expect(names).toContain('eventCount');
80+
expect(names).toContain('isSelfReview');
8081
expect(names).toContain('reviewer');
8182
expect(names).toContain('pullRequest');
8283
expect(names).toContain('reviewEvents');

packages/twenty-apps/community/github-connector/src/modules/github/contributor/logic-functions/contributor-stats.logic-function.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,12 @@ const handler = async (
298298
const res = await client.query({
299299
pullRequestReviews: {
300300
__args: {
301-
filter: { reviewerId: { eq: contributorId } },
301+
filter: {
302+
and: [
303+
{ reviewerId: { eq: contributorId } },
304+
{ isSelfReview: { eq: false } },
305+
],
306+
},
302307
orderBy: [{ firstSubmittedAt: 'DescNullsLast' }],
303308
first: PAGE_SIZE,
304309
after: cursor,

packages/twenty-apps/community/github-connector/src/modules/github/contributor/logic-functions/top-contributors.logic-function.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ const handler = async (
198198
const res = await client.query({
199199
pullRequestReviews: {
200200
__args: {
201+
filter: { isSelfReview: { eq: false } },
201202
orderBy: [{ firstSubmittedAt: 'DescNullsLast' }],
202203
first: PAGE_SIZE,
203204
after: cursor,

packages/twenty-apps/community/github-connector/src/modules/github/pull-request-review/graphql/mutations/batch-upsert.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,6 @@ export async function batchUpsertConsolidatedReviews(
1515
eventCount: true,
1616
reviewerId: true,
1717
pullRequestId: true,
18+
isSelfReview: true,
1819
}) as Promise<PullRequestReviewRow[]>;
1920
}

packages/twenty-apps/community/github-connector/src/modules/github/pull-request-review/logic-functions/recompute-pull-request-reviews.logic-function.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,18 @@ type ReviewEventNode = {
2323
reviewerId: string | null;
2424
pullRequestId: string | null;
2525
reviewer: { ghLogin: string | null } | null;
26-
pullRequest: { githubNumber: number | null } | null;
26+
pullRequest: {
27+
githubNumber: number | null;
28+
authorId: string | null;
29+
} | null;
2730
};
2831

2932
type GroupContext = {
3033
pullRequestId: string;
3134
reviewerId: string | null;
3235
prNumber: number | null;
3336
reviewerLogin: string | null;
37+
prAuthorId: string | null;
3438
events: { state: ReviewEventState; submittedAt: string | null }[];
3539
};
3640

@@ -68,7 +72,7 @@ const handler = async (_event: RoutePayload<unknown>) => {
6872
reviewerId: true,
6973
pullRequestId: true,
7074
reviewer: { ghLogin: true },
71-
pullRequest: { githubNumber: true },
75+
pullRequest: { githubNumber: true, authorId: true },
7276
},
7377
},
7478
pageInfo: { hasNextPage: true, endCursor: true },
@@ -95,6 +99,7 @@ const handler = async (_event: RoutePayload<unknown>) => {
9599
reviewerId: node.reviewerId,
96100
prNumber: node.pullRequest?.githubNumber ?? null,
97101
reviewerLogin: node.reviewer?.ghLogin ?? null,
102+
prAuthorId: node.pullRequest?.authorId ?? null,
98103
events: [],
99104
};
100105
groups.set(key, group);
@@ -105,6 +110,9 @@ const handler = async (_event: RoutePayload<unknown>) => {
105110
if (group.prNumber === null && node.pullRequest?.githubNumber != null) {
106111
group.prNumber = node.pullRequest.githubNumber;
107112
}
113+
if (group.prAuthorId === null && node.pullRequest?.authorId) {
114+
group.prAuthorId = node.pullRequest.authorId;
115+
}
108116
group.events.push({
109117
state: node.state,
110118
submittedAt: node.submittedAt,
@@ -123,6 +131,7 @@ const handler = async (_event: RoutePayload<unknown>) => {
123131
prNumber: group.prNumber,
124132
reviewerLogin: group.reviewerLogin,
125133
events: group.events,
134+
prAuthorId: group.prAuthorId,
126135
}),
127136
);
128137
}

packages/twenty-apps/community/github-connector/src/modules/github/pull-request-review/objects/pull-request-review.object.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ export const REVIEW_EVENT_COUNT_FIELD_UNIVERSAL_IDENTIFIER =
2424
export const PULL_REQUEST_REVIEW_CREATED_AT_FIELD_UNIVERSAL_IDENTIFIER =
2525
'b4d61187-1b78-5d6e-9752-79f994ff6d55';
2626

27+
export const REVIEW_IS_SELF_REVIEW_FIELD_UNIVERSAL_IDENTIFIER =
28+
'c1f3e8a2-9b5d-4e7f-8a6c-2d4b6f8e1a93';
29+
2730
enum ReviewState {
2831
APPROVED = 'APPROVED',
2932
CHANGES_REQUESTED = 'CHANGES_REQUESTED',
@@ -116,5 +119,15 @@ export default defineObject({
116119
icon: 'IconHash',
117120
defaultValue: 0,
118121
},
122+
{
123+
universalIdentifier: REVIEW_IS_SELF_REVIEW_FIELD_UNIVERSAL_IDENTIFIER,
124+
name: 'isSelfReview',
125+
type: FieldType.BOOLEAN,
126+
label: 'Self review',
127+
description:
128+
'True when the reviewer is the same contributor as the PR author. Self reviews are excluded from review-count aggregations (top reviewers, contributor stats, etc.) so contributors are credited only for reviews on other people\u2019s PRs.',
129+
icon: 'IconUserCheck',
130+
defaultValue: false,
131+
},
119132
],
120133
});

packages/twenty-apps/community/github-connector/src/modules/github/pull-request-review/types/pull-request-review-row.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@ export type PullRequestReviewRow = {
88
eventCount?: number | null;
99
reviewerId?: string | null;
1010
pullRequestId?: string | null;
11+
isSelfReview?: boolean | null;
1112
};

packages/twenty-apps/community/github-connector/src/modules/github/pull-request-review/utils/build-consolidated-row.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export type ConsolidatedReviewUpsertInput = {
1212
eventCount: number;
1313
reviewerId: string | null;
1414
pullRequestId: string;
15+
isSelfReview: boolean;
1516
};
1617

1718
export type BuildConsolidatedRowParams = {
@@ -20,8 +21,23 @@ export type BuildConsolidatedRowParams = {
2021
prNumber: number | null;
2122
reviewerLogin: string | null;
2223
events: ReviewEventForConsolidation[];
24+
/**
25+
* Author of the PR being reviewed. When non-null and equal to `reviewerId`
26+
* the consolidated row is flagged as a self-review so downstream
27+
* aggregations (top reviewers, contributor stats, etc.) can exclude it.
28+
*/
29+
prAuthorId?: string | null;
2330
};
2431

32+
export const isSelfReview = (
33+
reviewerId: string | null,
34+
prAuthorId: string | null | undefined,
35+
): boolean =>
36+
reviewerId !== null &&
37+
prAuthorId !== null &&
38+
prAuthorId !== undefined &&
39+
reviewerId === prAuthorId;
40+
2541
export const buildReviewKey = (
2642
pullRequestId: string,
2743
reviewerId: string | null,
@@ -49,5 +65,6 @@ export const buildConsolidatedRow = (
4965
eventCount: verdict.eventCount,
5066
reviewerId: params.reviewerId,
5167
pullRequestId: params.pullRequestId,
68+
isSelfReview: isSelfReview(params.reviewerId, params.prAuthorId ?? null),
5269
};
5370
};

packages/twenty-apps/community/github-connector/src/modules/github/pull-request-review/utils/consolidate-reviews.integration-test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
buildConsolidatedRow,
99
buildReviewKey,
1010
buildConsolidatedTitle,
11+
isSelfReview,
1112
} from 'src/modules/github/pull-request-review/utils/build-consolidated-row';
1213

1314
const evt = (
@@ -140,6 +141,71 @@ describe('buildConsolidatedRow', () => {
140141
eventCount: 3,
141142
reviewerId: 'rev-2',
142143
pullRequestId: 'pr-1',
144+
isSelfReview: false,
143145
});
144146
});
147+
148+
it('flags isSelfReview when prAuthorId equals reviewerId', () => {
149+
const row = buildConsolidatedRow({
150+
pullRequestId: 'pr-1',
151+
reviewerId: 'alice',
152+
prNumber: 7,
153+
reviewerLogin: 'alice',
154+
prAuthorId: 'alice',
155+
events: [evt('COMMENTED', '2025-04-01T10:00:00Z')],
156+
});
157+
expect(row.isSelfReview).toBe(true);
158+
});
159+
160+
it('does not flag isSelfReview when prAuthorId differs from reviewerId', () => {
161+
const row = buildConsolidatedRow({
162+
pullRequestId: 'pr-1',
163+
reviewerId: 'alice',
164+
prNumber: 7,
165+
reviewerLogin: 'alice',
166+
prAuthorId: 'bob',
167+
events: [evt('APPROVED', '2025-04-01T10:00:00Z')],
168+
});
169+
expect(row.isSelfReview).toBe(false);
170+
});
171+
172+
it('does not flag isSelfReview when prAuthorId is missing', () => {
173+
const row = buildConsolidatedRow({
174+
pullRequestId: 'pr-1',
175+
reviewerId: 'alice',
176+
prNumber: 7,
177+
reviewerLogin: 'alice',
178+
events: [evt('APPROVED', '2025-04-01T10:00:00Z')],
179+
});
180+
expect(row.isSelfReview).toBe(false);
181+
});
182+
183+
it('does not flag isSelfReview when reviewerId is null (ghost reviewer)', () => {
184+
const row = buildConsolidatedRow({
185+
pullRequestId: 'pr-1',
186+
reviewerId: null,
187+
prNumber: 7,
188+
reviewerLogin: null,
189+
prAuthorId: null,
190+
events: [evt('COMMENTED', '2025-04-01T10:00:00Z')],
191+
});
192+
expect(row.isSelfReview).toBe(false);
193+
});
194+
});
195+
196+
describe('isSelfReview', () => {
197+
it('returns true only when both ids are present and equal', () => {
198+
expect(isSelfReview('a', 'a')).toBe(true);
199+
});
200+
201+
it('returns false when ids differ', () => {
202+
expect(isSelfReview('a', 'b')).toBe(false);
203+
});
204+
205+
it('returns false when either side is null/undefined', () => {
206+
expect(isSelfReview(null, 'a')).toBe(false);
207+
expect(isSelfReview('a', null)).toBe(false);
208+
expect(isSelfReview('a', undefined)).toBe(false);
209+
expect(isSelfReview(null, null)).toBe(false);
210+
});
145211
});

0 commit comments

Comments
 (0)