From 67d935492d9a75cd8fbd0a46cef4497400efa08e Mon Sep 17 00:00:00 2001 From: jNullj <15849761+jNullj@users.noreply.github.com> Date: Thu, 15 Jun 2023 21:20:16 +0300 Subject: [PATCH] feat: Add author filter option for [GithubCommitActivity] (#9251) * feat: Add author filter option for CommitActivity Add a new filter option to [GithubCommitActivity], allowing users to filter the commit activity by a specific author. To make the filter more explicit, The label display "commits by [author]" for the total amount of commits and "commit activity by [author]" for other intervals when an author filter is selected. To maintain a clear and organized code structure, The filtered author is added as an argument and not to the shield path. The request to find the number of commits by the author is made using the REST api rather then the GraphQL api to make it in 1 request rather then 2. Resolves #9215 * fix: solve eslint errors * Add tests for [GithubCommitActivity] filter by author Add tests for the new filter by author feature. * update [GithubCommitActivity] spec file for new author feat Add test for new transformAuthorFilter function of GithubCommitActivity added for the author filter feature. * Fix null string for label of GithubCommitActivity * Update GithubCommitActivity example * improve error handeling for GithubCommitActivity The author filter error handling removed was redundent as it would never execute, there is no way to seperate branch not found from repo not found. * update depricated functions PR #9233 replaced errorsMessages with httpErrors. This commit updates the new changes to stay up to date with that PR * remove test for nonexisting error this exception was removed in commit 9e358c8 and is not needed anymore * Fixed test for commit activity unexisting repo * Update example for GithubCommitActivity Picked a user with commits in the repo as an example that would work * Add test for invalid commit activity branch Add test for REST API calls in commit activity branch --------- Co-authored-by: jNullj --- .../github/github-commit-activity.service.js | 75 +++++++++++++++++-- .../github/github-commit-activity.tester.js | 59 +++++++++++++++ 2 files changed, 126 insertions(+), 8 deletions(-) diff --git a/services/github/github-commit-activity.service.js b/services/github/github-commit-activity.service.js index ca270a19cfb41..07adab1e1274a 100644 --- a/services/github/github-commit-activity.service.js +++ b/services/github/github-commit-activity.service.js @@ -1,10 +1,15 @@ import gql from 'graphql-tag' import Joi from 'joi' +import parseLinkHeader from 'parse-link-header' import { InvalidResponse } from '../index.js' import { metric } from '../text-formatters.js' import { nonNegativeInteger } from '../validators.js' import { GithubAuthV4Service } from './github-auth-service.js' -import { transformErrors, documentation } from './github-helpers.js' +import { + transformErrors, + documentation, + httpErrorsFor, +} from './github-helpers.js' const schema = Joi.object({ data: Joi.object({ @@ -18,11 +23,16 @@ const schema = Joi.object({ }).required(), }).required() +const queryParamSchema = Joi.object({ + authorFilter: Joi.string(), +}) + export default class GitHubCommitActivity extends GithubAuthV4Service { static category = 'activity' static route = { base: 'github/commit-activity', pattern: ':interval(t|y|m|4w|w)/:user/:repo/:branch*', + queryParamSchema, } static examples = [ @@ -31,6 +41,7 @@ export default class GitHubCommitActivity extends GithubAuthV4Service { // Override the pattern to omit the deprecated interval "4w". pattern: ':interval(t|y|m|w)/:user/:repo', namedParams: { interval: 'm', user: 'eslint', repo: 'eslint' }, + queryParams: { authorFilter: 'nzakas' }, staticPreview: this.render({ interval: 'm', commitCount: 457 }), keywords: ['commits'], documentation, @@ -45,6 +56,7 @@ export default class GitHubCommitActivity extends GithubAuthV4Service { repo: 'squint', branch: 'main', }, + queryParams: { authorFilter: 'calebcartwright' }, staticPreview: this.render({ interval: 'm', commitCount: 5 }), keywords: ['commits'], documentation, @@ -53,9 +65,10 @@ export default class GitHubCommitActivity extends GithubAuthV4Service { static defaultBadgeData = { label: 'commit activity', color: 'blue' } - static render({ interval, commitCount }) { + static render({ interval, commitCount, authorFilter }) { // If total commits selected change label from commit activity to commits - const label = interval === 't' ? 'commits' : undefined + const label = interval === 't' ? 'commits' : 'commit activity' + const authorFilterLabel = authorFilter ? ` by ${authorFilter}` : '' const intervalLabel = { t: '', @@ -66,7 +79,7 @@ export default class GitHubCommitActivity extends GithubAuthV4Service { }[interval] return { - label, + label: `${label}${authorFilterLabel}`, message: `${metric(commitCount)}${intervalLabel}`, } } @@ -103,6 +116,30 @@ export default class GitHubCommitActivity extends GithubAuthV4Service { }) } + async fetchAuthorFilter({ + interval, + user, + repo, + branch = 'HEAD', + authorFilter, + }) { + const since = + this.constructor.getIntervalQueryStartDate({ interval }) || undefined + + return this._request({ + url: `/repos/${user}/${repo}/commits`, + options: { + searchParams: { + sha: branch, + author: authorFilter, + per_page: '1', + since, + }, + }, + httpErrors: httpErrorsFor('repo or branch not found'), + }) + } + static transform({ data }) { const { repository: { object: repo }, @@ -115,6 +152,16 @@ export default class GitHubCommitActivity extends GithubAuthV4Service { return repo.history.totalCount } + static transformAuthorFilter({ res }) { + const parsed = parseLinkHeader(res.headers.link) + + if (!parsed) { + return 0 + } + + return parsed.last.page + } + static getIntervalQueryStartDate({ interval }) { const now = new Date() @@ -131,9 +178,21 @@ export default class GitHubCommitActivity extends GithubAuthV4Service { return now.toISOString() } - async handle({ interval, user, repo, branch }) { - const json = await this.fetch({ interval, user, repo, branch }) - const commitCount = this.constructor.transform(json) - return this.constructor.render({ interval, commitCount }) + async handle({ interval, user, repo, branch }, { authorFilter }) { + let commitCount + if (authorFilter) { + const authorFilterRes = await this.fetchAuthorFilter({ + interval, + user, + repo, + branch, + authorFilter, + }) + commitCount = this.constructor.transformAuthorFilter(authorFilterRes) + } else { + const json = await this.fetch({ interval, user, repo, branch }) + commitCount = this.constructor.transform(json) + } + return this.constructor.render({ interval, commitCount, authorFilter }) } } diff --git a/services/github/github-commit-activity.tester.js b/services/github/github-commit-activity.tester.js index bf99826fc55ae..b1e95470b8f14 100644 --- a/services/github/github-commit-activity.tester.js +++ b/services/github/github-commit-activity.tester.js @@ -12,21 +12,44 @@ const isCommitActivity = Joi.alternatives().try( isZeroOverTimePeriod ) +const authorFilterUser = 'jnullj' + t.create('commit activity (total)').get('/t/badges/shields.json').expectBadge({ label: 'commits', message: isMetric, }) +t.create('commit activity (total) by author') + .get(`/t/badges/shields.json?authorFilter=${authorFilterUser}`) + .expectBadge({ + label: `commits by ${authorFilterUser}`, + message: isMetric, + }) + t.create('commit activity (1 year)').get('/y/eslint/eslint.json').expectBadge({ label: 'commit activity', message: isMetricOverTimePeriod, }) +t.create('commit activity (1 year) by author') + .get(`/y/badges/shields.json?authorFilter=${authorFilterUser}`) + .expectBadge({ + label: `commit activity by ${authorFilterUser}`, + message: isMetricOverTimePeriod, + }) + t.create('commit activity (1 month)').get('/m/eslint/eslint.json').expectBadge({ label: 'commit activity', message: isMetricOverTimePeriod, }) +t.create('commit activity (1 month) by author') + .get(`/m/badges/shields.json?authorFilter=${authorFilterUser}`) + .expectBadge({ + label: `commit activity by ${authorFilterUser}`, + message: isMetricOverTimePeriod, + }) + t.create('commit activity (4 weeks)') .get('/4w/eslint/eslint.json') .expectBadge({ @@ -34,11 +57,25 @@ t.create('commit activity (4 weeks)') message: isMetricOverTimePeriod, }) +t.create('commit activity (4 weeks) by author') + .get(`/4w/badges/shields.json?authorFilter=${authorFilterUser}`) + .expectBadge({ + label: `commit activity by ${authorFilterUser}`, + message: isMetricOverTimePeriod, + }) + t.create('commit activity (1 week)').get('/w/eslint/eslint.json').expectBadge({ label: 'commit activity', message: isCommitActivity, }) +t.create('commit activity (1 week) by author') + .get(`/w/badges/shields.json?authorFilter=${authorFilterUser}`) + .expectBadge({ + label: `commit activity by ${authorFilterUser}`, + message: isCommitActivity, + }) + t.create('commit activity (custom branch)') .get('/y/badges/squint/main.json') .expectBadge({ @@ -46,6 +83,13 @@ t.create('commit activity (custom branch)') message: isCommitActivity, }) +t.create('commit activity (custom branch) by author') + .get(`/y/badges/squint/main.json?authorFilter=${authorFilterUser}`) + .expectBadge({ + label: `commit activity by ${authorFilterUser}`, + message: isCommitActivity, + }) + t.create('commit activity (repo not found)') .get('/w/badges/helmets.json') .expectBadge({ @@ -59,3 +103,18 @@ t.create('commit activity (invalid branch)') label: 'commit activity', message: 'invalid branch', }) + +// test for error handling of author filter as it uses REST and not GraphQL +t.create('commit activity (repo not found)') + .get('/w/badges/helmets.json?authorFilter=zaphod') + .expectBadge({ + label: 'commit activity', + message: 'repo or branch not found', + }) + +t.create('commit activity (invalid branch)') + .get('/w/badges/shields/invalidBranchName.json?authorFilter=zaphod') + .expectBadge({ + label: 'commit activity', + message: 'repo or branch not found', + })