fix: —until and —since should apply when using —pr-filter

sorenlouv · Apr 4, 2022 · 35c4aa7 · 35c4aa7
1 parent 578ee6b
commit 35c4aa7
Show file tree

Hide file tree

Showing 18 changed files with 169 additions and 78 deletions.
diff --git 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
@@ -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
@@ -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<Commit[]> {
  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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -58,7 +58,6 @@ export async function fetchCommitByPullNumber(options: {
  },
  });
  } catch (e) {
- //@ts-expect-error
  res = swallowMissingConfigFileException<CommitByPullNumberResponse>(e);
  }
 

diff --git 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<CommitsByShaResponse>(e);
  }
 

diff --git 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<CommitByAuthorResponse>(e);
  }
 }

diff --git 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=<username>\` 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
@@ -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<Commit[]> {
  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<ResponseData>(e);
  }
 

diff --git a/src/lib/remoteConfig.ts b/src/lib/remoteConfig.ts
@@ -52,8 +52,12 @@ export function parseRemoteConfig(remoteConfig: RemoteConfig) {
 }
 
 export function swallowMissingConfigFileException<T>(
- error: GithubV4Exception<T>
+ error: GithubV4Exception<T> | 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<T>(
 
  // 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