From 35c4aa7823566db21dd48703269798ad437c58ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Mon, 4 Apr 2022 08:38:23 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20`=E2=80=94until`=20and=20`=E2=80=94since?= =?UTF-8?q?`=20should=20apply=20when=20using=20`=E2=80=94pr-filter`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/configuration.md | 14 ++++- src/entrypoint.module.private.test.ts | 18 +++--- src/entrypoint.module.ts | 6 +- src/lib/getCommits.ts | 13 ++-- src/lib/github/v4/apiRequestV4.ts | 2 +- .../fetchCommitsByAuthor.private.test.ts.snap | 2 +- .../fetchCommitsByAuthor.test.ts.snap | 2 +- .../fetchCommits/allFetchers.private.test.ts | 8 ++- .../fetchCommits/fetchCommitByPullNumber.ts | 1 - .../v4/fetchCommits/fetchCommitBySha.ts | 1 - .../v4/fetchCommits/fetchCommitsByAuthor.ts | 17 +++--- ...chPullRequestBySearchQuery.private.test.ts | 10 +++- .../fetchPullRequestsBySearchQuery.ts | 38 +++++++++--- src/lib/remoteConfig.ts | 8 ++- src/options/cliArgs.ts | 22 ++++--- src/options/options.ts | 1 - src/test/e2e/cli/date-filters.private.test.ts | 59 +++++++++++++++++++ .../e2e/cli/entrypoint.cli.private.test.ts | 25 +------- 18 files changed, 169 insertions(+), 78 deletions(-) create mode 100644 src/test/e2e/cli/date-filters.private.test.ts diff --git a/docs/configuration.md b/docs/configuration.md index f45341dd..e954d78f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -237,6 +237,18 @@ Config: } ``` +#### `dateSince` + +Only display commits newer than the specified date + +CLI: `--since=2020-12-10` + +#### `dateUntil` + +Only display commits older than the specified date + +CLI: `--until=2020-12-15` + #### `fork` `true`: Create backport branch in users own fork @@ -245,7 +257,7 @@ Config: Default: `true` -CLI: `--fork=false` +CLI: `--no-fork` Config: diff --git a/src/entrypoint.module.private.test.ts b/src/entrypoint.module.private.test.ts index a8894a20..6a2eb508 100644 --- a/src/entrypoint.module.private.test.ts +++ b/src/entrypoint.module.private.test.ts @@ -297,19 +297,19 @@ describe('entrypoint.module', () => { expect(commitMessage).toMatchInlineSnapshot(` Array [ Object { - "committedDate": "2021-06-01T15:53:07Z", - "message": "Upgrade EUI to v33.0.0 (#99382)", - "sha": "38fd8a268ad7661d92f0d84c52d6f0a3d84c9801", + "committedDate": "2021-05-28T12:41:42Z", + "message": "[Observability] Fix typo in readme for new navigation (#100861)", + "sha": "79945fe0275b2ec9c93747e26154110133ec51fb", }, Object { - "committedDate": "2021-02-08T09:19:54Z", - "message": "Migrate most plugins to synchronous lifecycle (#89562)", - "sha": "3b3327dbc3c3041c9681e0cd86bd31cf411dc460", + "committedDate": "2021-05-28T19:43:30Z", + "message": "[APM] Move APM tutorial from apm_oss to x-pack/apm (#100780)", + "sha": "0bcd78b0e999feb95057f5e6eafdb572b9b2fe39", }, Object { - "committedDate": "2021-04-01T12:40:47Z", - "message": "TS Incremental build exclude test files (#95610)", - "sha": "b6e582c53ebb9c496c232408066b128d2ca2f92c", + "committedDate": "2021-05-18T10:33:16Z", + "message": "Migrate from Joi to @kbn/config-schema in \\"home\\" and \\"features\\" plugins (#100201)", + "sha": "574f6595ad2e5452fa90e6a3111220a599e473c0", }, ] `); diff --git a/src/entrypoint.module.ts b/src/entrypoint.module.ts index 22627b09..ba9306bf 100644 --- a/src/entrypoint.module.ts +++ b/src/entrypoint.module.ts @@ -61,6 +61,8 @@ export async function getCommits(options: { // optional author?: string; branchLabelMapping?: ValidConfigOptions['branchLabelMapping']; + dateSince?: string; + dateUntil?: string; githubApiBaseUrlV4?: string; maxNumber?: number; onlyMissing?: boolean; @@ -69,8 +71,6 @@ export async function getCommits(options: { sha?: string | string[]; skipRemoteConfig?: boolean; sourceBranch?: string; - dateUntil?: string; - dateSince?: string; }): Promise { initLogger({ interactive: false, accessToken: options.accessToken }); @@ -102,6 +102,8 @@ export async function getCommits(options: { ...options, prFilter: options.prFilter, author: options.author ?? null, + dateSince: options.dateSince ?? null, + dateUntil: options.dateUntil ?? null, }); } diff --git a/src/lib/getCommits.ts b/src/lib/getCommits.ts index f3bf0f6b..a272838a 100644 --- a/src/lib/getCommits.ts +++ b/src/lib/getCommits.ts @@ -68,12 +68,13 @@ export async function getCommits(options: ValidConfigOptions) { ? 'Loading pull requests...' : `Loading commits from branch "${options.sourceBranch}"...`; - const commitChoices = options.prFilter - ? await fetchPullRequestsBySearchQuery({ - ...options, - prFilter: options.prFilter, - }) - : await fetchCommitsByAuthor(options); + const commitChoices = + options.prFilter !== undefined + ? await fetchPullRequestsBySearchQuery({ + ...options, + prFilter: options.prFilter, + }) + : await fetchCommitsByAuthor(options); spinner.stop(); if (options.ls) { diff --git a/src/lib/github/v4/apiRequestV4.ts b/src/lib/github/v4/apiRequestV4.ts index d06c5978..5cc790cf 100644 --- a/src/lib/github/v4/apiRequestV4.ts +++ b/src/lib/github/v4/apiRequestV4.ts @@ -92,7 +92,7 @@ function addDebugLogs({ }${didThrow ? ', EXCEPTION THROWN' : ''})` ); - logger.verbose(`Query: ${query}`); + logger.verbose(`Query: ${print(query)}`); logger.verbose('Variables:', variables); logger.verbose('Response headers:', githubResponse.headers); logger.verbose('Response data:', githubResponse.data); diff --git a/src/lib/github/v4/fetchCommits/__snapshots__/fetchCommitsByAuthor.private.test.ts.snap b/src/lib/github/v4/fetchCommits/__snapshots__/fetchCommitsByAuthor.private.test.ts.snap index 381502c0..14753ef5 100644 --- a/src/lib/github/v4/fetchCommits/__snapshots__/fetchCommitsByAuthor.private.test.ts.snap +++ b/src/lib/github/v4/fetchCommits/__snapshots__/fetchCommitsByAuthor.private.test.ts.snap @@ -9,7 +9,7 @@ exports[`fetchCommitsByAuthor snapshot request/response makes the right queries: `; exports[`fetchCommitsByAuthor snapshot request/response makes the right queries: Query: CommitsByAuthor 1`] = ` -"query CommitsByAuthor($repoOwner: String!, $repoName: String!, $maxNumber: Int!, $sourceBranch: String!, $authorId: ID, $commitPath: String, $dateSince: GitTimestamp, $dateUntil: GitTimestamp) { +"query CommitsByAuthor($authorId: ID, $commitPath: String, $dateSince: GitTimestamp, $dateUntil: GitTimestamp, $maxNumber: Int!, $repoName: String!, $repoOwner: String!, $sourceBranch: String!) { repository(owner: $repoOwner, name: $repoName) { ref(qualifiedName: $sourceBranch) { target { diff --git a/src/lib/github/v4/fetchCommits/__snapshots__/fetchCommitsByAuthor.test.ts.snap b/src/lib/github/v4/fetchCommits/__snapshots__/fetchCommitsByAuthor.test.ts.snap index 88cf7581..9dc89e7d 100644 --- a/src/lib/github/v4/fetchCommits/__snapshots__/fetchCommitsByAuthor.test.ts.snap +++ b/src/lib/github/v4/fetchCommits/__snapshots__/fetchCommitsByAuthor.test.ts.snap @@ -18,7 +18,7 @@ Array [ exports[`fetchCommitsByAuthor when commit has an associated pull request should call with correct args to fetch commits 1`] = ` Array [ Object { - "query": "query CommitsByAuthor($repoOwner: String!, $repoName: String!, $maxNumber: Int!, $sourceBranch: String!, $authorId: ID, $commitPath: String, $dateSince: GitTimestamp, $dateUntil: GitTimestamp) { + "query": "query CommitsByAuthor($authorId: ID, $commitPath: String, $dateSince: GitTimestamp, $dateUntil: GitTimestamp, $maxNumber: Int!, $repoName: String!, $repoOwner: String!, $sourceBranch: String!) { repository(owner: $repoOwner, name: $repoName) { ref(qualifiedName: $sourceBranch) { target { diff --git a/src/lib/github/v4/fetchCommits/allFetchers.private.test.ts b/src/lib/github/v4/fetchCommits/allFetchers.private.test.ts index 96779d39..991f8468 100644 --- a/src/lib/github/v4/fetchCommits/allFetchers.private.test.ts +++ b/src/lib/github/v4/fetchCommits/allFetchers.private.test.ts @@ -56,13 +56,15 @@ describe('allFetchers', () => { it('matches commitByAuthor with commitBySearchQuery', async () => { const commitsBySearchQuery = await fetchPullRequestsBySearchQuery({ - repoOwner: 'elastic', - repoName: 'kibana', accessToken, + author: 'sqren', + dateSince: null, + dateUntil: null, maxNumber: 1, prFilter: `created:2021-12-20..2021-12-20`, + repoName: 'kibana', + repoOwner: 'elastic', sourceBranch: 'main', - author: 'sqren', }); const commitBySearchQuery = commitsBySearchQuery[0]; diff --git a/src/lib/github/v4/fetchCommits/fetchCommitByPullNumber.ts b/src/lib/github/v4/fetchCommits/fetchCommitByPullNumber.ts index e2fd61f4..dfb05106 100644 --- a/src/lib/github/v4/fetchCommits/fetchCommitByPullNumber.ts +++ b/src/lib/github/v4/fetchCommits/fetchCommitByPullNumber.ts @@ -58,7 +58,6 @@ export async function fetchCommitByPullNumber(options: { }, }); } catch (e) { - //@ts-expect-error res = swallowMissingConfigFileException(e); } diff --git a/src/lib/github/v4/fetchCommits/fetchCommitBySha.ts b/src/lib/github/v4/fetchCommits/fetchCommitBySha.ts index 69296b1b..f5fc3b0d 100644 --- a/src/lib/github/v4/fetchCommits/fetchCommitBySha.ts +++ b/src/lib/github/v4/fetchCommits/fetchCommitBySha.ts @@ -53,7 +53,6 @@ export async function fetchCommitBySha(options: { }, }); } catch (e) { - //@ts-expect-error res = swallowMissingConfigFileException(e); } diff --git a/src/lib/github/v4/fetchCommits/fetchCommitsByAuthor.ts b/src/lib/github/v4/fetchCommits/fetchCommitsByAuthor.ts index cc3a3467..6c04500a 100644 --- a/src/lib/github/v4/fetchCommits/fetchCommitsByAuthor.ts +++ b/src/lib/github/v4/fetchCommits/fetchCommitsByAuthor.ts @@ -21,38 +21,38 @@ async function fetchByCommitPath({ }: { options: { accessToken: string; + dateSince: string | null; + dateUntil: string | null; githubApiBaseUrlV4?: string; maxNumber?: number; repoName: string; repoOwner: string; sourceBranch: string; - dateSince: string | null; - dateUntil: string | null; }; authorId: string | null; commitPath: string | null; }) { const { accessToken, + dateSince, + dateUntil, githubApiBaseUrlV4 = 'https://api.github.com/graphql', maxNumber = 10, repoName, repoOwner, sourceBranch, - dateSince, - dateUntil, } = options; const query = gql` query CommitsByAuthor( - $repoOwner: String! - $repoName: String! - $maxNumber: Int! - $sourceBranch: String! $authorId: ID $commitPath: String $dateSince: GitTimestamp $dateUntil: GitTimestamp + $maxNumber: Int! + $repoName: String! + $repoOwner: String! + $sourceBranch: String! ) { repository(owner: $repoOwner, name: $repoName) { ref(qualifiedName: $sourceBranch) { @@ -99,7 +99,6 @@ async function fetchByCommitPath({ variables, }); } catch (e) { - //@ts-expect-error return swallowMissingConfigFileException(e); } } diff --git a/src/lib/github/v4/fetchCommits/fetchPullRequestBySearchQuery.private.test.ts b/src/lib/github/v4/fetchCommits/fetchPullRequestBySearchQuery.private.test.ts index 8829862f..d227b226 100644 --- a/src/lib/github/v4/fetchCommits/fetchPullRequestBySearchQuery.private.test.ts +++ b/src/lib/github/v4/fetchCommits/fetchPullRequestBySearchQuery.private.test.ts @@ -9,18 +9,20 @@ describe('fetchPullRequestsBySearchQuery', () => { it('throws an error', async () => { const options = { accessToken, + author: 'sqren', + dateSince: null, + dateUntil: null, maxNumber: 10, prFilter: 'label:non-existing', repoName: 'backport-e2e', repoOwner: 'backport-org', sourceBranch: 'master', - author: 'sqren', }; await expect(fetchPullRequestsBySearchQuery(options)).rejects .toThrowErrorMatchingInlineSnapshot(` "No commits found for query: - type:pr is:merged sort:updated-desc repo:backport-org/backport-e2e author:sqren base:master label:non-existing + type:pr is:merged sort:created-desc repo:backport-org/backport-e2e author:sqren base:master label:non-existing Use \`--all\` to see commits by all users or \`--author=\` for commits from a specific user" `); @@ -31,12 +33,14 @@ describe('fetchPullRequestsBySearchQuery', () => { it('returns the merge commits for those PRs', async () => { const options = { accessToken, + author: 'sqren', + dateSince: null, + dateUntil: null, maxNumber: 10, prFilter: 'label:v7.8.0', repoName: 'backport-e2e', repoOwner: 'backport-org', sourceBranch: 'master', - author: 'sqren', }; const expectedCommits: Commit[] = [ diff --git a/src/lib/github/v4/fetchCommits/fetchPullRequestsBySearchQuery.ts b/src/lib/github/v4/fetchCommits/fetchPullRequestsBySearchQuery.ts index 15b808c5..25d2612b 100644 --- a/src/lib/github/v4/fetchCommits/fetchPullRequestsBySearchQuery.ts +++ b/src/lib/github/v4/fetchCommits/fetchPullRequestsBySearchQuery.ts @@ -14,6 +14,8 @@ import { apiRequestV4 } from '../apiRequestV4'; export async function fetchPullRequestsBySearchQuery(options: { accessToken: string; author: string | null; + dateSince: string | null; + dateUntil: string | null; githubApiBaseUrlV4?: string; maxNumber?: number; onlyMissing?: boolean; @@ -24,13 +26,15 @@ export async function fetchPullRequestsBySearchQuery(options: { }): Promise { const { accessToken, + author, + dateSince, + dateUntil, githubApiBaseUrlV4 = 'https://api.github.com/graphql', maxNumber = 10, prFilter, repoName, repoOwner, sourceBranch, - author, } = options; const query = gql` @@ -49,11 +53,32 @@ export async function fetchPullRequestsBySearchQuery(options: { ${SourceCommitWithTargetPullRequestFragment} `; - const authorFilter = options.author ? ` author:${options.author}` : ''; - const sourceBranchFilter = prFilter.includes('base:') - ? '' - : ` base:${sourceBranch}`; - const searchQuery = `type:pr is:merged sort:updated-desc repo:${repoOwner}/${repoName}${authorFilter}${sourceBranchFilter} ${prFilter} `; + function dateFilter() { + if (dateUntil && dateSince) { + return [`merged:${dateSince}..${dateUntil}`]; + } + + if (dateUntil) { + return [`merged:<${dateUntil}`]; + } + + if (dateSince) { + return [`merged:>${dateSince}`]; + } + + return []; + } + + const searchQuery = [ + 'type:pr', + 'is:merged', + 'sort:created-desc', + `repo:${repoOwner}/${repoName}`, + ...(options.author ? [`author:${options.author}`] : []), + ...(prFilter.includes('base:') ? [] : [`base:${sourceBranch}`]), + ...dateFilter(), + prFilter, + ].join(' '); const variables = { query: searchQuery, @@ -69,7 +94,6 @@ export async function fetchPullRequestsBySearchQuery(options: { variables, }); } catch (e) { - //@ts-expect-error res = swallowMissingConfigFileException(e); } diff --git a/src/lib/remoteConfig.ts b/src/lib/remoteConfig.ts index 2a84829f..6110c933 100644 --- a/src/lib/remoteConfig.ts +++ b/src/lib/remoteConfig.ts @@ -52,8 +52,12 @@ export function parseRemoteConfig(remoteConfig: RemoteConfig) { } export function swallowMissingConfigFileException( - error: GithubV4Exception + error: GithubV4Exception | unknown ) { + if (!(error instanceof GithubV4Exception)) { + throw error; + } + const { data, errors } = error.githubResponse.data; const missingConfigError = errors?.some((error) => { @@ -62,7 +66,7 @@ export function swallowMissingConfigFileException( // swallow error if it's just the config file that's missing if (missingConfigError && data != null) { - return data; + return data as T; } // Throw unexpected error diff --git a/src/options/cliArgs.ts b/src/options/cliArgs.ts index 14501a79..ae23216a 100644 --- a/src/options/cliArgs.ts +++ b/src/options/cliArgs.ts @@ -86,14 +86,26 @@ export function getOptionsFromCliArgs(processArgs: readonly string[]) { type: 'string', }) - .option('since', { + .option('dateSince', { + alias: 'since', description: 'ISO-8601 date for filtering commits', type: 'string', + coerce: (since) => { + if (since) { + return new Date(since).toISOString(); + } + }, }) - .option('until', { + .option('dateUntil', { + alias: 'until', description: 'ISO-8601 date for filtering commits', type: 'string', + coerce: (until) => { + if (until) { + return new Date(until).toISOString(); + } + }, }) .option('dir', { @@ -274,7 +286,7 @@ export function getOptionsFromCliArgs(processArgs: readonly string[]) { }) .option('prFilter', { - conflicts: ['pullNumber', 'sha'], + conflicts: ['pullNumber', 'sha', 'path'], description: `Filter source pull requests by a query`, type: 'string', }) @@ -401,8 +413,6 @@ export function getOptionsFromCliArgs(processArgs: readonly string[]) { // filters author, - since, - until, // negations noCherrypickRef, @@ -438,8 +448,6 @@ export function getOptionsFromCliArgs(processArgs: readonly string[]) { // filters author: all ? null : author, - dateSince: since ? new Date(since).toISOString() : undefined, - dateUntil: until ? new Date(until).toISOString() : undefined, // `multiple` is a cli-only flag to override `multipleBranches` and `multipleCommits` multipleBranches: multiple ?? multipleBranches, diff --git a/src/options/options.ts b/src/options/options.ts index 88aaaf61..16f825e5 100644 --- a/src/options/options.ts +++ b/src/options/options.ts @@ -181,7 +181,6 @@ function throwForRequiredOptions( 'githubApiBaseUrlV4', 'logFilePath', 'prDescription', - 'prFilter', 'projectConfigFile', 'prTitle', 'repoForkOwner', diff --git a/src/test/e2e/cli/date-filters.private.test.ts b/src/test/e2e/cli/date-filters.private.test.ts new file mode 100644 index 00000000..39427a65 --- /dev/null +++ b/src/test/e2e/cli/date-filters.private.test.ts @@ -0,0 +1,59 @@ +import { getDevAccessToken } from '../../private/getDevAccessToken'; +import { runBackportViaCli } from './runBackportViaCli'; + +const accessToken = getDevAccessToken(); + +describe('date filters (dateSince, dateUntil)', () => { + it(`filters commits by "since" and "until"`, async () => { + const { output } = await runBackportViaCli( + [ + '--branch=7.x', + '--repo=backport-org/backport-e2e', + `--accessToken=${accessToken}`, + '--since=2020-08-15T10:00:00.000Z', + '--until=2020-08-15T10:30:00.000Z', + ], + { waitForString: 'Select commit' } + ); + + expect(output).toMatchInlineSnapshot(` + "? Select commit (Use arrow keys) + ❯ 1. Bump to 8.0.0 + 2. Add package.json + 3. Update .backportrc.json + 4. Create .backportrc.json" + `); + }); + + it('combined with --pr-filter', async () => { + const { output } = await runBackportViaCli( + [ + '--branch=7.x', + '--repo=elastic/kibana', + `--accessToken=${accessToken}`, + `--author=sqren`, + '--since=2021-09-20', + '--until=2021-10-01', + ], + { waitForString: 'Select commit' } + ); + + const { output: outputFromPrFilter } = await runBackportViaCli( + [ + '--branch=7.x', + '--repo=elastic/kibana', + `--accessToken=${accessToken}`, + `--pr-filter="author:sqren"`, + '--since=2021-09-20', + '--until=2021-10-01', + '--source-branch=master', + ], + { waitForString: 'Select commit' } + ); + expect(output).toMatchInlineSnapshot(` + "? Select commit (Use arrow keys) + ❯ 1. [APM] Add link to officials docs for APM UI settings (#113396) 7.x" + `); + expect(output).toEqual(outputFromPrFilter); + }); +}); diff --git a/src/test/e2e/cli/entrypoint.cli.private.test.ts b/src/test/e2e/cli/entrypoint.cli.private.test.ts index 290f7634..4d1e65e7 100644 --- a/src/test/e2e/cli/entrypoint.cli.private.test.ts +++ b/src/test/e2e/cli/entrypoint.cli.private.test.ts @@ -48,8 +48,8 @@ describe('entrypoint cli', () => { [boolean] --projectConfigFile, --config Path to project config [string] --globalConfigFile Path to global config [string] - --since ISO-8601 date for filtering commits [string] - --until ISO-8601 date for filtering commits [string] + --dateSince, --since ISO-8601 date for filtering commits [string] + --dateUntil, --until ISO-8601 date for filtering commits [string] --dir Path to temporary backport repo [string] --details Show details about each commit [boolean] --dryRun Run backport locally without pushing to Github [boolean] @@ -157,27 +157,6 @@ describe('entrypoint cli', () => { `); }); - it(`filters commits by "since" and "until"`, async () => { - const { output } = await runBackportViaCli( - [ - '--branch=7.x', - '--repo=backport-org/backport-e2e', - `--accessToken=${accessToken}`, - '--since=2020-08-15T10:00:00.000Z', - '--until=2020-08-15T10:30:00.000Z', - ], - { waitForString: 'Select commit' } - ); - - expect(output).toMatchInlineSnapshot(` - "? Select commit (Use arrow keys) - ❯ 1. Bump to 8.0.0 - 2. Add package.json - 3. Update .backportrc.json - 4. Create .backportrc.json" - `); - }); - it(`lists commits from 7.x`, async () => { const { output } = await runBackportViaCli( [