diff --git a/src/js/background.js b/src/js/background.js index 413e34c..3d93ca5 100644 --- a/src/js/background.js +++ b/src/js/background.js @@ -24,16 +24,9 @@ const ServiceWorker = () => { throw error; } - let recordEntries; + let entries; try { - recordEntries = await Promise.all([ - github.getReviewRequested(), - github.getTeamReviewRequested(), - github.getNoReviewRequested(), - github.getAllReviewsDone(), - github.getMissingAssignee(), - github.getAllAssigned(), - ]); + entries = await github.getAll(); } catch (error) { if (error === tooManyRequestsError) return; @@ -41,12 +34,12 @@ const ServiceWorker = () => { } const record = { - [PullRequestRecordKey.reviewRequested]: recordEntries[0], - [PullRequestRecordKey.teamReviewRequested]: recordEntries[1], - [PullRequestRecordKey.noReviewRequested]: recordEntries[2], - [PullRequestRecordKey.allReviewsDone]: recordEntries[3], - [PullRequestRecordKey.missingAssignee]: recordEntries[4], - [PullRequestRecordKey.allAssigned]: recordEntries[5], + [PullRequestRecordKey.reviewRequested]: entries.reviewRequested, + [PullRequestRecordKey.teamReviewRequested]: entries.teamReviewRequested, + [PullRequestRecordKey.noReviewRequested]: entries.noReviewRequested, + [PullRequestRecordKey.allReviewsDone]: entries.allReviewsDone, + [PullRequestRecordKey.missingAssignee]: entries.missingAssignee, + [PullRequestRecordKey.allAssigned]: entries.allAssigned, }; await filterIgnoredPrs(record); diff --git a/src/js/services/github-api-wrapper.js b/src/js/services/github-api-wrapper.js index 4647a46..3b29f6f 100644 --- a/src/js/services/github-api-wrapper.js +++ b/src/js/services/github-api-wrapper.js @@ -1,98 +1,164 @@ import { noAccessTokenError, tooManyRequestsError } from './constants.js'; import SettingsStorageAccessor from './settings-storage-accessor.js'; -const GithubApiWrapper = async () => { - const getReviewRequested = async () => { - const query = encodeURIComponent(`is:open is:pr review-requested:${userName} archived:false`); - const pullRequests = await makeApiRequest('/search/issues', `q=${query}`); - const processedPullRequests = await processDataIntoPullRequests(pullRequests.items, false); +const GITHUB_GRAPHQL_URL = 'https://api.github.com/graphql'; + +// Fields requested for every pull request. `reviewRequests` and `assignees` +// carry the counts that used to require one REST `/requested_reviewers` call +// per pull request, so the whole poll is now a single GraphQL request. +const PULL_REQUEST_FIELDS = ` + number + title + url + createdAt + isDraft + author { login } + assignees(first: 1) { totalCount } + reviewRequests(first: 1) { totalCount } + repository { nameWithOwner url } +`; - const teamPullRequestUrls = (await getTeamReviewRequested()).map(pr => pr.url); - return processedPullRequests.filter((pr) => !teamPullRequestUrls.includes(pr.url)); +const GithubApiWrapper = async () => { + // Builds one aliased `search` field. GitHub search qualifiers are identical + // to the ones the REST implementation used. + const searchField = (alias, queryString) => ` + ${alias}: search(query: ${JSON.stringify(queryString)}, type: ISSUE, first: 100) { + nodes { ... on PullRequest { ${PULL_REQUEST_FIELDS} } } + }`; + + const buildQuery = (teams) => { + const fields = [ + 'viewer { login }', + searchField('reviewRequested', `is:open is:pr review-requested:${userName} archived:false`), + searchField('noReviewRequested', `is:open is:pr assignee:${userName} archived:false review:none`), + searchField('allReviewsDone', `is:open is:pr assignee:${userName} archived:false -review:none`), + searchField('missingAssignee', `is:open is:pr author:${userName} draft:false archived:false`), + searchField('allAssigned', `is:open is:pr assignee:${userName} archived:false`), + ...teams.map((team, index) => ( + searchField(`team${index}`, `is:open is:pr team-review-requested:${team} archived:false`) + )), + ]; + + return `query {${fields.join('\n')}}`; }; - const getTeamReviewRequested = async () => { + const teamNames = async () => { const teams = await SettingsStorageAccessor().loadTeams(); if (teams === '') return []; + return teams.replace(/ /g, '').split(',').filter((team) => team !== ''); + }; - let combinedPullRequests = []; - - for (const team of teams.replace(/ /g, '').split(',')) { - const query = encodeURIComponent(`is:open is:pr team-review-requested:${team} archived:false`); - const pullRequests = await makeApiRequest('/search/issues', `q=${query}`); + // A single network round-trip that resolves every category at once. + const fetchAll = async () => { + const teams = await teamNames(); + const data = await graphqlRequest(buildQuery(teams)); + + const teamPullRequests = teams.flatMap((_team, index) => nodesOf(data, `team${index}`)); + + return { + reviewRequested: nodesOf(data, 'reviewRequested'), + teamPullRequests, + noReviewRequested: nodesOf(data, 'noReviewRequested'), + allReviewsDone: nodesOf(data, 'allReviewsDone'), + missingAssignee: nodesOf(data, 'missingAssignee'), + allAssigned: nodesOf(data, 'allAssigned'), + }; + }; - if (!pullRequests.errors) combinedPullRequests = combinedPullRequests.concat(pullRequests.items); - } + const nodesOf = (data, alias) => (data[alias] && data[alias].nodes) || []; - return processDataIntoPullRequests(combinedPullRequests, false); - }; + const getReviewRequested = async () => { + const { reviewRequested, teamPullRequests } = await fetchAll(); + const processed = await processNodes(reviewRequested, false); - const getNoReviewRequested = async () => { - return processDataIntoPullRequests(await searchMyIssues('review:none')); + const teamUrls = (await processNodes(teamPullRequests, false)).map((pr) => pr.url); + return processed.filter((pr) => !teamUrls.includes(pr.url)); }; - const getAllReviewsDone = async () => { - return processDataIntoPullRequests(await searchMyIssues('-review:none')); + const getTeamReviewRequested = async () => { + const { teamPullRequests } = await fetchAll(); + return processNodes(teamPullRequests, false); }; - const searchMyIssues = async (reviewModifier) => { - const query = encodeURIComponent(`is:pr assignee:${userName} archived:false is:open ${reviewModifier}`); - const response = await makeApiRequest('/search/issues', `q=${query}`); - - return asyncFilterIssues(response.items, async (PullRequest) => { - const requestedReviewers = (await makeRequest(`${PullRequest.pull_request.url}/requested_reviewers`)); - return requestedReviewers.users.length + requestedReviewers.teams.length === 0; - }); + // A pull request assigned to me still counts as "no review requested" only + // when nobody (user or team) has been requested for review. + const getNoReviewRequested = async () => { + const { noReviewRequested } = await fetchAll(); + return processNodes(noReviewRequested.filter(hasNoReviewRequest)); }; - const asyncFilterIssues = async (pullRequests, filter) => { - const response = await Promise.all(pullRequests.map(filter)); - return pullRequests.filter((_item, index) => response[index]); + const getAllReviewsDone = async () => { + const { allReviewsDone } = await fetchAll(); + return processNodes(allReviewsDone.filter(hasNoReviewRequest)); }; const getMissingAssignee = async () => { - const query = encodeURIComponent(`is:open is:pr author:${userName} draft:false archived:false`); - let response = await makeApiRequest('/search/issues', `q=${query}`); - response = response.items.filter((s) => !s.assignee); - - return processDataIntoPullRequests(response); + const { missingAssignee } = await fetchAll(); + return processNodes(missingAssignee.filter((node) => node.assignees.totalCount === 0)); }; const getAllAssigned = async () => { - const query = encodeURIComponent(`is:open is:pr assignee:${userName} archived:false`); - const response = await makeApiRequest('/search/issues', `q=${query}`); - - return processDataIntoPullRequests(response.items); + const { allAssigned } = await fetchAll(); + return processNodes(allAssigned); }; - const makeApiRequest = async (path, params) => makeRequest(`https://api.github.com${path}`, params); + // Resolves all six categories from a single GraphQL round-trip. This is what + // the service worker calls every poll; the individual getters above exist for + // callers that need one category in isolation. + const getAll = async () => { + const raw = await fetchAll(); + + const teamReviewRequested = await processNodes(raw.teamPullRequests, false); + const teamUrls = teamReviewRequested.map((pr) => pr.url); + + return { + reviewRequested: (await processNodes(raw.reviewRequested, false)).filter((pr) => !teamUrls.includes(pr.url)), + teamReviewRequested, + noReviewRequested: await processNodes(raw.noReviewRequested.filter(hasNoReviewRequest)), + allReviewsDone: await processNodes(raw.allReviewsDone.filter(hasNoReviewRequest)), + missingAssignee: await processNodes(raw.missingAssignee.filter((node) => node.assignees.totalCount === 0)), + allAssigned: await processNodes(raw.allAssigned), + }; + }; + const hasNoReviewRequest = (node) => node.reviewRequests.totalCount === 0; - const makeRequest = async (path, params) => { - const response = await fetch(`${path}?${params}`, { - method: 'GET', + const graphqlRequest = async (query) => { + const response = await fetch(GITHUB_GRAPHQL_URL, { + method: 'POST', headers: { - 'Authorization': 'Basic ' + btoa(':' + accessToken), + 'Authorization': `Bearer ${accessToken}`, + 'Content-Type': 'application/json', }, + body: JSON.stringify({ query }), }); - if (response.status === 403) throw tooManyRequestsError; - else return response.json(); + + if (response.status === 403 || response.status === 429) throw tooManyRequestsError; + + const body = await response.json(); + if (isRateLimited(body)) throw tooManyRequestsError; + + return body.data || {}; }; - const processDataIntoPullRequests = async (issues, shouldFilterByMaximumAge = true) => { - issues = await filterByScope(issues); - const pullRequests = issues.map(issue => ({ + const isRateLimited = (body) => ( + Array.isArray(body.errors) && body.errors.some((error) => error.type === 'RATE_LIMITED') + ); + + const processNodes = async (nodes, shouldFilterByMaximumAge = true) => { + const filtered = await filterByScope(nodes); + const pullRequests = filtered.map((node) => ({ id: 12, assignee: undefined, - title: issue.title, - number: issue.number, - ownerAndName: readOwnerAndNameFromUrl(issue.pull_request.url), - createdAt: issue.created_at, - ageInDays: getDifferenceInDays(new Date(issue.created_at)), - url: issue.pull_request.url, - repositoryUrl: issue.pull_request.html_url.split('/pull')[0], - htmlUrl: issue.pull_request.html_url, - author: issue.user.login, + title: node.title, + number: node.number, + ownerAndName: node.repository.nameWithOwner, + createdAt: node.createdAt, + ageInDays: getDifferenceInDays(new Date(node.createdAt)), + url: node.url, + repositoryUrl: node.repository.url, + htmlUrl: node.url, + author: node.author ? node.author.login : undefined, ignored: false, })); @@ -100,13 +166,13 @@ const GithubApiWrapper = async () => { return shouldFilterByMaximumAge ? filterByMaximumAge(sorted) : sorted; }; - const filterByScope = async (issues) => { + const filterByScope = async (nodes) => { const scope = await SettingsStorageAccessor().loadScope(); - if (scope === '') return issues; + if (scope === '') return nodes; const individualScopes = (scope).replace(' ', '').toLowerCase().split(','); - return issues.filter(issue => ( - individualScopes.includes(readOwnerAndNameFromUrl(issue.pull_request.url).split('/')[0].toLowerCase()) + return nodes.filter((node) => ( + individualScopes.includes(node.repository.nameWithOwner.split('/')[0].toLowerCase()) )); }; @@ -115,12 +181,10 @@ const GithubApiWrapper = async () => { new Date(pullRequest2.createdAt).getTime() - new Date(pullRequest1.createdAt).getTime() )); - const readOwnerAndNameFromUrl = (url) => url.replace('https://api.github.com/repos/', '').split('/pulls/')[0]; - const filterByMaximumAge = async (pullRequests) => { const maximumAge = await SettingsStorageAccessor().loadMaximumAge(); - return pullRequests.filter(pullRequest => pullRequest.ageInDays < maximumAge); + return pullRequests.filter((pullRequest) => pullRequest.ageInDays < maximumAge); }; const getDifferenceInDays = (date2) => (Date.now() - date2.getTime()) / 86_400_000; // 1000 * 3600 * 24 @@ -128,14 +192,13 @@ const GithubApiWrapper = async () => { const accessToken = await SettingsStorageAccessor().loadAccessToken(); if (accessToken === '') { - console.error("no access token no party"); + console.error('no access token no party'); throw noAccessTokenError; } - // TODO: This should be cached to improve performance - const userName = (await makeApiRequest('/user')).login; + const userName = (await graphqlRequest('query { viewer { login } }')).viewer.login; - return { getReviewRequested, getTeamReviewRequested, getNoReviewRequested, getAllReviewsDone, getMissingAssignee, getAllAssigned }; + return { getAll, getReviewRequested, getTeamReviewRequested, getNoReviewRequested, getAllReviewsDone, getMissingAssignee, getAllAssigned }; }; export default GithubApiWrapper; diff --git a/test/background.test.js b/test/background.test.js index e41d09b..7948700 100644 --- a/test/background.test.js +++ b/test/background.test.js @@ -112,9 +112,9 @@ describe('ServiceWorker', () => { mockedFetch.mockImplementation((url) => globalMock(url, { pullRequestCount: 2 })); }); - describe('while fetching the user', () => { + describe('while resolving the viewer', () => { beforeAll(() => { - mockedFetch.mockImplementation(() => Promise.resolve({ status: 403 })); + mockedFetch.mockImplementation(() => Promise.resolve({ status: 403, json: () => Promise.resolve({}) })); }); it('doesn\'t call set', () => { @@ -124,12 +124,14 @@ describe('ServiceWorker', () => { describe('while fetching data', () => { beforeAll(() => { - mockedFetch.mockImplementation((url) => { - if (url.includes('/user')) { - return Promise.resolve({ json: () => ({ login: 'renuo' }) }); - } else { - return Promise.resolve({ status: 403 }); + let call = 0; + mockedFetch.mockImplementation(() => { + call += 1; + // First request resolves the viewer login; the batched query is rate-limited. + if (call === 1) { + return Promise.resolve({ status: 200, json: () => Promise.resolve({ data: { viewer: { login: 'renuo' } } }) }); } + return Promise.resolve({ status: 403, json: () => Promise.resolve({}) }); }); }); @@ -145,7 +147,7 @@ describe('ServiceWorker', () => { mockedFetch.mockImplementation((url) => globalMock(url, { pullRequestCount: 2 })); }); - describe('while fetching the user', () => { + describe('while resolving the viewer', () => { beforeAll(() => { mockedFetch.mockImplementation(() => Promise.resolve({})); }); @@ -157,12 +159,13 @@ describe('ServiceWorker', () => { describe('while fetching data', () => { beforeAll(() => { - mockedFetch.mockImplementation((url) => { - if (url.includes('/user')) { - return Promise.resolve({ json: () => ({ login: 'renuo' }) }); - } else { - return Promise.resolve({}); + let call = 0; + mockedFetch.mockImplementation(() => { + call += 1; + if (call === 1) { + return Promise.resolve({ status: 200, json: () => Promise.resolve({ data: { viewer: { login: 'renuo' } } }) }); } + return Promise.resolve({}); }); }); diff --git a/test/mocks/github-api-mock-data.js b/test/mocks/github-api-mock-data.js index a9ae1b6..b8938c7 100644 --- a/test/mocks/github-api-mock-data.js +++ b/test/mocks/github-api-mock-data.js @@ -29,6 +29,46 @@ export const mockListOfPullRequests = (count, params = {}) => { }; }; +// Builds the GraphQL PullRequest nodes returned by a `search(type: ISSUE)` +// field. `params.reviewRequests` / `params.assignees` set the totalCount used +// by the wrapper to reproduce the old `/requested_reviewers` filtering. +export const mockPullRequestNodes = (count, params = {}) => { + const nodes = []; + for (let i = 0; i < count; i++) { + nodes.push({ + number: i + 1, + title: 'PR Title', + url: `https://github.com/renuo/github-pull-request-counter/pull/${i + 1}`, + createdAt: params.createdAt || new Date(Date.now()).toISOString(), + isDraft: params.isDraft || false, + author: { login: 'coorasse' }, + assignees: { totalCount: params.assignees === undefined ? 0 : params.assignees }, + reviewRequests: { totalCount: params.reviewRequests === undefined ? 0 : params.reviewRequests }, + repository: { + nameWithOwner: 'renuo/github-pull-request-counter', + url: 'https://github.com/renuo/github-pull-request-counter', + }, + }); + } + return nodes; +}; + +// Builds a full GraphQL response. Every aliased search field returns the same +// set of nodes unless overridden via `aliases`. +export const mockGraphqlResponse = (nodes, aliases = {}) => { + const searchAliases = ['reviewRequested', 'noReviewRequested', 'allReviewsDone', 'missingAssignee', 'allAssigned']; + const data = { viewer: { login: 'sislr' } }; + + for (const alias of searchAliases) { + data[alias] = { nodes: aliases[alias] || nodes }; + } + for (const [alias, value] of Object.entries(aliases)) { + if (!searchAliases.includes(alias)) data[alias] = { nodes: value }; + } + + return { data }; +}; + export const mockRequestedReviewers = (usersCount, teamsCount) => { const createRandomID = () => Math.floor(Math.random() * 100); @@ -46,14 +86,11 @@ export const mockRequestedReviewers = (usersCount, teamsCount) => { }; }; -export const globalMock = (url, params = {}) => { - if (url.includes('/requested_reviewers')) { - return Promise.resolve({ - json: () => Promise.resolve(mockRequestedReviewers(params.openUserRequestCount || 0, params.openTeamRequestCount || 0)), - }); - } else { - return Promise.resolve({ - json: () => mockListOfPullRequests(params.pullRequestCount || 0), - }); - } -}; +// GraphQL-shaped mock used by the background/integration tests. Every fetch +// (the `viewer` lookup and the batched query) resolves to the same response. +export const globalMock = (_url, params = {}) => ( + Promise.resolve({ + status: 200, + json: () => Promise.resolve(mockGraphqlResponse(mockPullRequestNodes(params.pullRequestCount || 0))), + }) +); diff --git a/test/services/github-api-wrapper.test.js b/test/services/github-api-wrapper.test.js index 8412afd..0a35521 100644 --- a/test/services/github-api-wrapper.test.js +++ b/test/services/github-api-wrapper.test.js @@ -1,5 +1,5 @@ import GithubApiWrapper from '../../src/js/services/github-api-wrapper.js'; -import { mockListOfPullRequests, mockRequestedReviewers } from '../mocks/github-api-mock-data.js'; +import { mockPullRequestNodes, mockGraphqlResponse } from '../mocks/github-api-mock-data.js'; import fetch from 'node-fetch'; import MockDate from 'mockdate'; @@ -7,7 +7,6 @@ jest.mock('node-fetch'); const mockedFetch = fetch; global.fetch = mockedFetch; -global.btoa = (data) => Buffer.from(data).toString('base64'); global.chrome = { storage: { local: { @@ -16,6 +15,15 @@ global.chrome = { }, }; +// Every call to `fetch` (the constructor's `viewer` query and the main query) +// resolves to the same GraphQL response. +const respondWith = (response) => { + mockedFetch.mockResolvedValue(Promise.resolve({ + status: 200, + json: () => Promise.resolve(response), + })); +}; + describe('GithubApiWrapper', () => { let scope = ''; @@ -28,11 +36,7 @@ describe('GithubApiWrapper', () => { describe('#getReviewRequested', () => { let pullRequestCount; - beforeEach(() => { - mockedFetch.mockResolvedValue(Promise.resolve({ - json: () => Promise.resolve(mockListOfPullRequests(pullRequestCount)), - })); - }); + beforeEach(() => respondWith(mockGraphqlResponse(mockPullRequestNodes(pullRequestCount)))); describe('with no pull requests', () => { beforeAll(() => pullRequestCount = 0); @@ -74,6 +78,10 @@ describe('GithubApiWrapper', () => { global.chrome.storage.local.get = jest.fn().mockImplementation((_keys, callback) => callback({ 'teams': 'myTeam, myOtherTeam', 'accessToken': 'secret', })); + // The same three PRs are returned for both the personal review-requested + // search and the team searches, so all of them are filtered out. + const nodes = mockPullRequestNodes(3); + respondWith(mockGraphqlResponse(nodes, { team0: nodes, team1: nodes })); }); it('has no pull requests', async () => { @@ -86,12 +94,11 @@ describe('GithubApiWrapper', () => { describe('#getTeamReviewRequested', () => { let teamPullRequestCount; beforeEach(() => { - mockedFetch.mockResolvedValue(Promise.resolve({ - json: () => Promise.resolve(mockListOfPullRequests(teamPullRequestCount)), - })); global.chrome.storage.local.get = jest.fn().mockImplementation((_keys, callback) => callback({ 'teams': 'myTeam', 'accessToken': 'secret', })); + const nodes = mockPullRequestNodes(teamPullRequestCount); + respondWith(mockGraphqlResponse(nodes, { team0: nodes })); }); describe('with no pull requests', () => { @@ -113,11 +120,12 @@ describe('GithubApiWrapper', () => { }); }); - describe('with an error', () => { + describe('with no configured teams', () => { beforeEach(() => { - mockedFetch.mockResolvedValue(Promise.resolve({ - json: () => Promise.resolve({ errors: ['Something went wrong'] }), + global.chrome.storage.local.get = jest.fn().mockImplementation((_keys, callback) => callback({ + 'accessToken': 'secret', })); + respondWith(mockGraphqlResponse(mockPullRequestNodes(3))); }); it('doesn\'t contain any links', async () => { @@ -129,20 +137,9 @@ describe('GithubApiWrapper', () => { describe('#getNoReviewRequested', () => { let pullRequestCount; - let openUserRequestCount = 0; - let openTeamRequestCount = 0; - - beforeEach(() => { - mockedFetch.mockImplementation((url) => { - const value = url.includes('/requested_reviewers') ? - mockRequestedReviewers(openUserRequestCount, openTeamRequestCount) : - mockListOfPullRequests(pullRequestCount); + let reviewRequests = 0; - return Promise.resolve({ - json: () => Promise.resolve(value), - }); - }); - }); + beforeEach(() => respondWith(mockGraphqlResponse(mockPullRequestNodes(pullRequestCount, { reviewRequests })))); describe('with no pull requests', () => { beforeAll(() => pullRequestCount = 0); @@ -172,20 +169,9 @@ describe('GithubApiWrapper', () => { expect(result[2].htmlUrl).toEqual('https://github.com/renuo/github-pull-request-counter/pull/3'); }); - describe('with a requested review from a user', () => { - - beforeAll(() => openUserRequestCount = 1); - afterAll(() => openUserRequestCount = 0); - - it('doesn\'t contain any links', async () => { - const result = await (await GithubApiWrapper()).getNoReviewRequested(); - expect(result.length).toEqual(0); - }); - }); - - describe('with a requested review from a team', () => { - beforeAll(() => openTeamRequestCount = 1); - afterAll(() => openTeamRequestCount = 0); + describe('with an open review request', () => { + beforeAll(() => reviewRequests = 1); + afterAll(() => reviewRequests = 0); it('doesn\'t contain any links', async () => { const result = await (await GithubApiWrapper()).getNoReviewRequested(); @@ -204,10 +190,7 @@ describe('GithubApiWrapper', () => { describe('with a not matching scope', () => { beforeAll(() => scope = 'notMatchingScope'); - - afterAll(() => { - scope = ''; - }); + afterAll(() => { scope = ''; }); it('doesn\'t contain any links', async () => { expect((await (await GithubApiWrapper()).getNoReviewRequested()).length).toEqual(0); @@ -228,21 +211,12 @@ describe('GithubApiWrapper', () => { beforeAll(() => { scope = 'renuo,notMatchingScope'; }); afterAll(() => { scope = ''; }); - it('doesn\'t contain any links', async () => { - expect((await (await GithubApiWrapper()).getNoReviewRequested()).length).toEqual(3); - }); - }); - - describe(',renuo', () => { - beforeAll(() => { scope = ',renuo'; }); - afterAll(() => { scope = ''; }); - it('contains three links', async () => { expect((await (await GithubApiWrapper()).getNoReviewRequested()).length).toEqual(3); }); }); - describe('renuo,', () => { + describe(',renuo', () => { beforeAll(() => { scope = ',renuo'; }); afterAll(() => { scope = ''; }); @@ -256,20 +230,9 @@ describe('GithubApiWrapper', () => { describe('#getAllReviewsDone', () => { let pullRequestCount; - let openUserRequestCount = 0; - let openTeamRequestCount = 0; - - beforeEach(() => { - mockedFetch.mockImplementation((url) => { - const value = url.includes('/requested_reviewers') ? - mockRequestedReviewers(openUserRequestCount, openTeamRequestCount) : - mockListOfPullRequests(pullRequestCount); + let reviewRequests = 0; - return Promise.resolve({ - json: () => Promise.resolve(value), - }); - }); - }); + beforeEach(() => respondWith(mockGraphqlResponse(mockPullRequestNodes(pullRequestCount, { reviewRequests })))); describe('with no pull requests', () => { beforeAll(() => pullRequestCount = 0); @@ -280,16 +243,6 @@ describe('GithubApiWrapper', () => { }); }); - describe('with a single pull request', () => { - beforeAll(() => pullRequestCount = 1); - - it('has the correct link', async () => { - const result = await (await GithubApiWrapper()).getAllReviewsDone(); - expect(result.length).toEqual(1); - expect(result[0].htmlUrl).toEqual('https://github.com/renuo/github-pull-request-counter/pull/1'); - }); - }); - describe('with three pull requests', () => { beforeAll(() => pullRequestCount = 3); @@ -299,19 +252,9 @@ describe('GithubApiWrapper', () => { expect(result[2].htmlUrl).toEqual('https://github.com/renuo/github-pull-request-counter/pull/3'); }); - describe('with a requested review from a user', () => { - beforeAll(() => openUserRequestCount = 1); - afterAll(() => openUserRequestCount = 0); - - it('doesn\'t contain any links', async () => { - const result = await (await GithubApiWrapper()).getAllReviewsDone(); - expect(result.length).toEqual(0); - }); - }); - - describe('with a requested review from a team', () => { - beforeAll(() => openTeamRequestCount = 1); - afterAll(() => openTeamRequestCount = 0); + describe('with an open review request', () => { + beforeAll(() => reviewRequests = 1); + afterAll(() => reviewRequests = 0); it('doesn\'t contain any links', async () => { const result = await (await GithubApiWrapper()).getAllReviewsDone(); @@ -323,29 +266,17 @@ describe('GithubApiWrapper', () => { describe('#getMissingAssignee', () => { let pullRequestCount = 0; - let assignee; + let assignees = 0; - beforeEach(() => { - mockedFetch.mockResolvedValue(Promise.resolve({ - json: () => Promise.resolve(mockListOfPullRequests(pullRequestCount, { assignee, created_at: undefined })), - })); - }); + beforeEach(() => respondWith(mockGraphqlResponse(mockPullRequestNodes(pullRequestCount, { assignees })))); describe('with no pull requests', () => { + beforeAll(() => pullRequestCount = 0); + it('doesn\'t contain any links', async () => { const result = await (await GithubApiWrapper()).getMissingAssignee(); expect(result.length).toEqual(0); }); - - describe('with an assignee', () => { - beforeAll(() => assignee = 'Karl'); - afterAll(() => assignee = undefined); - - it('doesn\'t contain any links', async () => { - const result = await (await GithubApiWrapper()).getMissingAssignee(); - expect(result.length).toEqual(0); - }); - }); }); describe('with a single pull request', () => { @@ -358,8 +289,8 @@ describe('GithubApiWrapper', () => { }); describe('with an assignee', () => { - beforeAll(() => assignee = 'Karl'); - afterAll(() => assignee = undefined); + beforeAll(() => assignees = 1); + afterAll(() => assignees = 0); it('doesn\'t contain any links', async () => { const result = await (await GithubApiWrapper()).getMissingAssignee(); @@ -378,8 +309,8 @@ describe('GithubApiWrapper', () => { }); describe('with an assignee', () => { - beforeAll(() => assignee = 'Karl'); - afterAll(() => assignee = undefined); + beforeAll(() => assignees = 1); + afterAll(() => assignees = 0); it('doesn\'t contain any links', async () => { const result = await (await GithubApiWrapper()).getMissingAssignee(); @@ -391,11 +322,7 @@ describe('GithubApiWrapper', () => { describe('#getAllAssigned', () => { let pullRequestCount; - beforeEach(() => { - mockedFetch.mockResolvedValue(Promise.resolve({ - json: () => Promise.resolve(mockListOfPullRequests(pullRequestCount)), - })); - }); + beforeEach(() => respondWith(mockGraphqlResponse(mockPullRequestNodes(pullRequestCount)))); describe('with no pull requests', () => { beforeAll(() => pullRequestCount = 0); @@ -406,16 +333,6 @@ describe('GithubApiWrapper', () => { }); }); - describe('with a single pull request', () => { - beforeAll(() => pullRequestCount = 1); - - it('has the correct link', async () => { - const result = await (await GithubApiWrapper()).getAllAssigned(); - expect(result.length).toEqual(1); - expect(result[0].htmlUrl).toEqual('https://github.com/renuo/github-pull-request-counter/pull/1'); - }); - }); - describe('with three pull requests', () => { beforeAll(() => pullRequestCount = 3); @@ -427,15 +344,12 @@ describe('GithubApiWrapper', () => { }); describe('filterByMaximumAge', () => { - const pullRequest1 = mockListOfPullRequests(1, { assignee: undefined, created_at: Date.parse('2021-05-20T14:17:00Z') }); - const pullRequest2 = mockListOfPullRequests(1, { assignee: undefined, created_at: Date.parse('2021-06-25T14:17:00Z') }); - const pullRequest3 = mockListOfPullRequests(1, { assignee: undefined, created_at: Date.parse('2021-04-15T14:17:00Z') }); + const pr1 = mockPullRequestNodes(1, { createdAt: '2021-05-20T14:17:00Z' }); + const pr2 = mockPullRequestNodes(1, { createdAt: '2021-06-25T14:17:00Z' }); + const pr3 = mockPullRequestNodes(1, { createdAt: '2021-04-15T14:17:00Z' }); + const nodes = [...pr1, ...pr2, ...pr3]; - beforeEach(() => { - mockedFetch.mockResolvedValue(Promise.resolve({ - json: () => Promise.resolve({ total_count: 3, items: [...pullRequest1.items, ...pullRequest2.items, ...pullRequest3.items] }), - })); - }); + beforeEach(() => respondWith(mockGraphqlResponse(nodes))); beforeEach(() => MockDate.set('2021-07-08')); afterEach(() => MockDate.reset()); @@ -443,9 +357,9 @@ describe('GithubApiWrapper', () => { it('has the correct order (newest first)', async () => { const result = await (await GithubApiWrapper()).getAllAssigned(); - expect(result[0].createdAt).toEqual(Date.parse('2021-06-25T14:17:00Z')); - expect(result[1].createdAt).toEqual(Date.parse('2021-05-20T14:17:00Z')); - expect(result[2].createdAt).toEqual(Date.parse('2021-04-15T14:17:00Z')); + expect(new Date(result[0].createdAt).getTime()).toEqual(Date.parse('2021-06-25T14:17:00Z')); + expect(new Date(result[1].createdAt).getTime()).toEqual(Date.parse('2021-05-20T14:17:00Z')); + expect(new Date(result[2].createdAt).getTime()).toEqual(Date.parse('2021-04-15T14:17:00Z')); }); describe('with a maximum age of 10 days', () => { @@ -498,10 +412,38 @@ describe('GithubApiWrapper', () => { }); }); + describe('#getAll', () => { + beforeEach(() => respondWith(mockGraphqlResponse(mockPullRequestNodes(3)))); + + it('resolves every category from a single request', async () => { + mockedFetch.mockClear(); + const record = await (await GithubApiWrapper()).getAll(); + + expect(record.reviewRequested.length).toEqual(3); + expect(record.allAssigned.length).toEqual(3); + // One request for the `viewer` login lookup, one for the batched query. + expect(mockedFetch).toHaveBeenCalledTimes(2); + }); + }); + describe('Too many requests', () => { beforeEach(() => { mockedFetch.mockResolvedValue(Promise.resolve({ status: 403, + json: () => Promise.resolve({}), + })); + }); + + it('throws', async () => { + await expect(GithubApiWrapper()).rejects.toThrow('Too many requests'); + }); + }); + + describe('With a GraphQL rate-limit error', () => { + beforeEach(() => { + mockedFetch.mockResolvedValue(Promise.resolve({ + status: 200, + json: () => Promise.resolve({ errors: [{ type: 'RATE_LIMITED', message: 'API rate limit exceeded' }] }), })); });