Fixed bug where PR author's own review was counted as first review. (#30815)
Fixes #29140 Only impacts metrics gathering. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Reviews made by the pull request creator are now filtered out in addition to bot reviews when viewing pull request review events. * **Tests** * Added and updated tests to verify correct filtering of both bot and pull request creator reviews, including improved logging checks. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -236,32 +236,50 @@ export class GitHubClient {
|
||||
}
|
||||
|
||||
/**
|
||||
* Filters out bot reviews from review events
|
||||
* Filters out bot reviews and PR creator reviews from review events
|
||||
* @param {Array} reviewEvents - Array of review events
|
||||
* @param {boolean} excludeBots - Whether to exclude bot reviews (default: false)
|
||||
* @param {Object} prCreator - PR creator user object (optional)
|
||||
* @returns {Array} Filtered review events
|
||||
*/
|
||||
filterBotReviews(reviewEvents, excludeBots = false) {
|
||||
if (!excludeBots) {
|
||||
filterBotReviews(reviewEvents, excludeBots = false, prCreator = null) {
|
||||
if (!excludeBots && !prCreator) {
|
||||
return reviewEvents;
|
||||
}
|
||||
|
||||
const filteredReviews = reviewEvents.filter((review) => {
|
||||
const botAnalysis = identifyBotUser(review.user);
|
||||
if (botAnalysis.isBot) {
|
||||
logger.debug(`Filtering out bot review from ${review.user.login}`, {
|
||||
confidence: botAnalysis.confidence,
|
||||
reasons: botAnalysis.reasons,
|
||||
});
|
||||
// Filter out bot reviews
|
||||
if (excludeBots) {
|
||||
const botAnalysis = identifyBotUser(review.user);
|
||||
if (botAnalysis.isBot) {
|
||||
logger.debug(`Filtering out bot review from ${review.user.login}`, {
|
||||
confidence: botAnalysis.confidence,
|
||||
reasons: botAnalysis.reasons,
|
||||
});
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// Filter out PR creator reviews
|
||||
if (prCreator && review.user.login === prCreator.login) {
|
||||
logger.debug(`Filtering out PR creator review from ${review.user.login}`);
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
});
|
||||
|
||||
const botCount = reviewEvents.length - filteredReviews.length;
|
||||
if (botCount > 0) {
|
||||
const botCount = excludeBots ? reviewEvents.filter(review => identifyBotUser(review.user).isBot).length : 0;
|
||||
const creatorCount = prCreator ? reviewEvents.filter(review => review.user.login === prCreator.login).length : 0;
|
||||
const totalFiltered = reviewEvents.length - filteredReviews.length;
|
||||
|
||||
if (totalFiltered > 0) {
|
||||
const filterReasons = [];
|
||||
if (botCount > 0) filterReasons.push(`${botCount} bot reviews`);
|
||||
if (creatorCount > 0) filterReasons.push(`${creatorCount} PR creator reviews`);
|
||||
|
||||
logger.info(
|
||||
`Filtered out ${botCount} bot reviews from ${reviewEvents.length} total reviews`
|
||||
`Filtered out ${totalFiltered} reviews (${filterReasons.join(', ')}) from ${reviewEvents.length} total reviews`
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -125,10 +125,11 @@ export class MetricsCollector {
|
||||
pr.number
|
||||
);
|
||||
|
||||
// Filter bot reviews if configured
|
||||
// Filter bot reviews and PR creator reviews if configured
|
||||
const reviewEvents = this.githubClient.filterBotReviews(
|
||||
rawReviewEvents,
|
||||
this.config.excludeBotReviews
|
||||
this.config.excludeBotReviews,
|
||||
pr.user
|
||||
);
|
||||
|
||||
// Collect enabled metrics for this PR
|
||||
|
||||
+119
-1
@@ -177,7 +177,7 @@ describe('Bot Detection', () => {
|
||||
githubClient.filterBotReviews(reviews, true);
|
||||
|
||||
expect(mockLogger.info).toHaveBeenCalledWith(
|
||||
'Filtered out 1 bot reviews from 2 total reviews'
|
||||
'Filtered out 1 reviews (1 bot reviews) from 2 total reviews'
|
||||
);
|
||||
});
|
||||
|
||||
@@ -207,4 +207,122 @@ describe('Bot Detection', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('PR Creator Filtering', () => {
|
||||
test('should filter out PR creator reviews', () => {
|
||||
const prCreator = { login: 'prauthor', type: 'User' };
|
||||
const reviews = [
|
||||
{
|
||||
user: { login: 'prauthor', type: 'User' },
|
||||
state: 'COMMENTED',
|
||||
submitted_at: '2023-06-15T12:00:00Z'
|
||||
},
|
||||
{
|
||||
user: { login: 'reviewer1', type: 'User' },
|
||||
state: 'APPROVED',
|
||||
submitted_at: '2023-06-15T13:00:00Z'
|
||||
},
|
||||
{
|
||||
user: { login: 'reviewer2', type: 'User' },
|
||||
state: 'CHANGES_REQUESTED',
|
||||
submitted_at: '2023-06-15T14:00:00Z'
|
||||
}
|
||||
];
|
||||
|
||||
const filtered = githubClient.filterBotReviews(reviews, false, prCreator);
|
||||
expect(filtered).toHaveLength(2);
|
||||
expect(filtered[0].user.login).toBe('reviewer1');
|
||||
expect(filtered[1].user.login).toBe('reviewer2');
|
||||
|
||||
expect(mockLogger.info).toHaveBeenCalledWith(
|
||||
'Filtered out 1 reviews (1 PR creator reviews) from 3 total reviews'
|
||||
);
|
||||
});
|
||||
|
||||
test('should filter out multiple PR creator reviews', () => {
|
||||
const prCreator = { login: 'prauthor', type: 'User' };
|
||||
const reviews = [
|
||||
{
|
||||
user: { login: 'prauthor', type: 'User' },
|
||||
state: 'COMMENTED',
|
||||
submitted_at: '2023-06-15T12:00:00Z'
|
||||
},
|
||||
{
|
||||
user: { login: 'reviewer1', type: 'User' },
|
||||
state: 'APPROVED',
|
||||
submitted_at: '2023-06-15T13:00:00Z'
|
||||
},
|
||||
{
|
||||
user: { login: 'prauthor', type: 'User' },
|
||||
state: 'COMMENTED',
|
||||
submitted_at: '2023-06-15T14:00:00Z'
|
||||
}
|
||||
];
|
||||
|
||||
const filtered = githubClient.filterBotReviews(reviews, false, prCreator);
|
||||
expect(filtered).toHaveLength(1);
|
||||
expect(filtered[0].user.login).toBe('reviewer1');
|
||||
|
||||
expect(mockLogger.info).toHaveBeenCalledWith(
|
||||
'Filtered out 2 reviews (2 PR creator reviews) from 3 total reviews'
|
||||
);
|
||||
});
|
||||
|
||||
test('should not filter when prCreator is null', () => {
|
||||
const reviews = [
|
||||
{
|
||||
user: { login: 'prauthor', type: 'User' },
|
||||
state: 'COMMENTED',
|
||||
submitted_at: '2023-06-15T12:00:00Z'
|
||||
},
|
||||
{
|
||||
user: { login: 'reviewer1', type: 'User' },
|
||||
state: 'APPROVED',
|
||||
submitted_at: '2023-06-15T13:00:00Z'
|
||||
}
|
||||
];
|
||||
|
||||
const filtered = githubClient.filterBotReviews(reviews, false, null);
|
||||
expect(filtered).toHaveLength(2);
|
||||
|
||||
expect(mockLogger.info).not.toHaveBeenCalledWith(
|
||||
expect.stringContaining('Filtered out')
|
||||
);
|
||||
});
|
||||
|
||||
test('should filter both bots and PR creator', () => {
|
||||
const prCreator = { login: 'prauthor', type: 'User' };
|
||||
const reviews = [
|
||||
{
|
||||
user: { login: 'prauthor', type: 'User' },
|
||||
state: 'COMMENTED',
|
||||
submitted_at: '2023-06-15T12:00:00Z'
|
||||
},
|
||||
{
|
||||
user: { login: 'coderabbitai[bot]', type: 'Bot' },
|
||||
state: 'COMMENTED',
|
||||
submitted_at: '2023-06-15T13:00:00Z'
|
||||
},
|
||||
{
|
||||
user: { login: 'reviewer1', type: 'User' },
|
||||
state: 'APPROVED',
|
||||
submitted_at: '2023-06-15T14:00:00Z'
|
||||
},
|
||||
{
|
||||
user: { login: 'dependabot[bot]', type: 'User' },
|
||||
state: 'COMMENTED',
|
||||
submitted_at: '2023-06-15T15:00:00Z'
|
||||
}
|
||||
];
|
||||
|
||||
const filtered = githubClient.filterBotReviews(reviews, true, prCreator);
|
||||
expect(filtered).toHaveLength(1);
|
||||
expect(filtered[0].user.login).toBe('reviewer1');
|
||||
|
||||
expect(mockLogger.info).toHaveBeenCalledWith(
|
||||
'Filtered out 3 reviews (2 bot reviews, 1 PR creator reviews) from 4 total reviews'
|
||||
);
|
||||
});
|
||||
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user