From a384ea440539ec3497ae31922de17a15821d6e30 Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Thu, 17 Jun 2021 11:38:11 -0700 Subject: [PATCH 1/4] Fix StagingDeployCash bug --- .../createOrUpdateStagingDeploy.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/actions/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js b/.github/actions/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js index 5484a14c2f0b..61192b3ce8f0 100644 --- a/.github/actions/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js +++ b/.github/actions/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js @@ -74,6 +74,7 @@ const run = function () { // There is an open StagingDeployCash, so we'll be updating it, not creating a new one const currentStagingDeployCashData = GithubUtils.getStagingDeployCashData(stagingDeployResponse.data[0]); + console.log('Parsed the following data from the current StagingDeployCash:', currentStagingDeployCashData); currentStagingDeployCashIssueNumber = currentStagingDeployCashData.number; const newDeployBlockers = _.map(deployBlockerResponse.data, ({url}) => ({ @@ -88,9 +89,9 @@ const run = function () { // Generate the PR list, preserving the previous state of `isVerified` for existing PRs const PRList = _.sortBy( _.unique( - _.union(currentStagingDeployCashData.PRList, _.map(mergedPRs, url => ({ - url, - number: GithubUtils.getPullRequestNumberFromURL(url), + _.union(currentStagingDeployCashData.PRList, _.map(mergedPRs, number => ({ + number, + url: GithubUtils.getPullRequestURLFromNumber(number), // Since this is the second argument to _.union, // it will appear later in the array than any duplicate. From f623d1044405d596bcd6247d0d581c6c9baa8c1e Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Thu, 17 Jun 2021 11:40:03 -0700 Subject: [PATCH 2/4] Rebuild GH actions --- .github/actions/createOrUpdateStagingDeploy/index.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/actions/createOrUpdateStagingDeploy/index.js b/.github/actions/createOrUpdateStagingDeploy/index.js index 2c1fc3d73568..e46f63ec0b0d 100644 --- a/.github/actions/createOrUpdateStagingDeploy/index.js +++ b/.github/actions/createOrUpdateStagingDeploy/index.js @@ -84,6 +84,7 @@ const run = function () { // There is an open StagingDeployCash, so we'll be updating it, not creating a new one const currentStagingDeployCashData = GithubUtils.getStagingDeployCashData(stagingDeployResponse.data[0]); + console.log('Parsed the following data from the current StagingDeployCash:', currentStagingDeployCashData); currentStagingDeployCashIssueNumber = currentStagingDeployCashData.number; const newDeployBlockers = _.map(deployBlockerResponse.data, ({url}) => ({ @@ -98,9 +99,9 @@ const run = function () { // Generate the PR list, preserving the previous state of `isVerified` for existing PRs const PRList = _.sortBy( _.unique( - _.union(currentStagingDeployCashData.PRList, _.map(mergedPRs, url => ({ - url, - number: GithubUtils.getPullRequestNumberFromURL(url), + _.union(currentStagingDeployCashData.PRList, _.map(mergedPRs, number => ({ + number, + url: GithubUtils.getPullRequestURLFromNumber(number), // Since this is the second argument to _.union, // it will appear later in the array than any duplicate. From 81a7c7917b631bd815e6aeddfb03f3fcd666ca2f Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Thu, 17 Jun 2021 12:03:12 -0700 Subject: [PATCH 3/4] Fix tests --- .github/actions/checkDeployBlockers/index.js | 2 +- .../createOrUpdateStagingDeploy.js | 5 ++++- .../actions/createOrUpdateStagingDeploy/index.js | 7 +++++-- .../actions/getMergeCommitForPullRequest/index.js | 2 +- .github/actions/getReleaseBody/index.js | 2 +- .github/actions/isPullRequestMergeable/index.js | 2 +- .github/actions/isStagingDeployLocked/index.js | 2 +- .github/actions/markPullRequestsAsDeployed/index.js | 2 +- .github/actions/reopenIssueWithComment/index.js | 2 +- .github/actions/triggerWorkflowAndWait/index.js | 2 +- .github/libs/GithubUtils.js | 2 +- tests/unit/createOrUpdateStagingDeployTest.js | 13 +++---------- 12 files changed, 21 insertions(+), 22 deletions(-) diff --git a/.github/actions/checkDeployBlockers/index.js b/.github/actions/checkDeployBlockers/index.js index eb39d3a08daa..12c95c8c76f1 100644 --- a/.github/actions/checkDeployBlockers/index.js +++ b/.github/actions/checkDeployBlockers/index.js @@ -313,7 +313,7 @@ class GithubUtils { let issueBody = `**Release Version:** \`${tag}\`\r\n**Compare Changes:** https://github.com/Expensify/Expensify.cash/compare/production...staging\r\n`; // PR list - if (!_.isEmpty(PRList)) { + if (!_.isEmpty(sortedPRList)) { issueBody += '\r\n**This release contains changes from the following pull requests:**\r\n'; _.each(sortedPRList, (URL) => { issueBody += _.contains(verifiedPRList, URL) ? '- [x]' : '- [ ]'; diff --git a/.github/actions/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js b/.github/actions/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js index 61192b3ce8f0..f05e26440c7d 100644 --- a/.github/actions/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js +++ b/.github/actions/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js @@ -69,7 +69,10 @@ const run = function () { // We're in the create flow, not update // TODO: if there are open DeployBlockers and we are opening a new checklist, // then we should close / remove the DeployBlockerCash label from those - return GithubUtils.generateStagingDeployCashBody(newVersion, mergedPRs); + return GithubUtils.generateStagingDeployCashBody( + newVersion, + _.map(mergedPRs, GithubUtils.getPullRequestURLFromNumber) + ); } // There is an open StagingDeployCash, so we'll be updating it, not creating a new one diff --git a/.github/actions/createOrUpdateStagingDeploy/index.js b/.github/actions/createOrUpdateStagingDeploy/index.js index e46f63ec0b0d..c5904c03f529 100644 --- a/.github/actions/createOrUpdateStagingDeploy/index.js +++ b/.github/actions/createOrUpdateStagingDeploy/index.js @@ -79,7 +79,10 @@ const run = function () { // We're in the create flow, not update // TODO: if there are open DeployBlockers and we are opening a new checklist, // then we should close / remove the DeployBlockerCash label from those - return GithubUtils.generateStagingDeployCashBody(newVersion, mergedPRs); + return GithubUtils.generateStagingDeployCashBody( + newVersion, + _.map(mergedPRs, GithubUtils.getPullRequestURLFromNumber) + ); } // There is an open StagingDeployCash, so we'll be updating it, not creating a new one @@ -432,7 +435,7 @@ class GithubUtils { let issueBody = `**Release Version:** \`${tag}\`\r\n**Compare Changes:** https://github.com/Expensify/Expensify.cash/compare/production...staging\r\n`; // PR list - if (!_.isEmpty(PRList)) { + if (!_.isEmpty(sortedPRList)) { issueBody += '\r\n**This release contains changes from the following pull requests:**\r\n'; _.each(sortedPRList, (URL) => { issueBody += _.contains(verifiedPRList, URL) ? '- [x]' : '- [ ]'; diff --git a/.github/actions/getMergeCommitForPullRequest/index.js b/.github/actions/getMergeCommitForPullRequest/index.js index 281144981729..cbb10320c6a2 100644 --- a/.github/actions/getMergeCommitForPullRequest/index.js +++ b/.github/actions/getMergeCommitForPullRequest/index.js @@ -341,7 +341,7 @@ class GithubUtils { let issueBody = `**Release Version:** \`${tag}\`\r\n**Compare Changes:** https://github.com/Expensify/Expensify.cash/compare/production...staging\r\n`; // PR list - if (!_.isEmpty(PRList)) { + if (!_.isEmpty(sortedPRList)) { issueBody += '\r\n**This release contains changes from the following pull requests:**\r\n'; _.each(sortedPRList, (URL) => { issueBody += _.contains(verifiedPRList, URL) ? '- [x]' : '- [ ]'; diff --git a/.github/actions/getReleaseBody/index.js b/.github/actions/getReleaseBody/index.js index bd52404c57cc..4321be9b8293 100644 --- a/.github/actions/getReleaseBody/index.js +++ b/.github/actions/getReleaseBody/index.js @@ -284,7 +284,7 @@ class GithubUtils { let issueBody = `**Release Version:** \`${tag}\`\r\n**Compare Changes:** https://github.com/Expensify/Expensify.cash/compare/production...staging\r\n`; // PR list - if (!_.isEmpty(PRList)) { + if (!_.isEmpty(sortedPRList)) { issueBody += '\r\n**This release contains changes from the following pull requests:**\r\n'; _.each(sortedPRList, (URL) => { issueBody += _.contains(verifiedPRList, URL) ? '- [x]' : '- [ ]'; diff --git a/.github/actions/isPullRequestMergeable/index.js b/.github/actions/isPullRequestMergeable/index.js index 8abd0df83937..c1c4c59e8bd9 100644 --- a/.github/actions/isPullRequestMergeable/index.js +++ b/.github/actions/isPullRequestMergeable/index.js @@ -287,7 +287,7 @@ class GithubUtils { let issueBody = `**Release Version:** \`${tag}\`\r\n**Compare Changes:** https://github.com/Expensify/Expensify.cash/compare/production...staging\r\n`; // PR list - if (!_.isEmpty(PRList)) { + if (!_.isEmpty(sortedPRList)) { issueBody += '\r\n**This release contains changes from the following pull requests:**\r\n'; _.each(sortedPRList, (URL) => { issueBody += _.contains(verifiedPRList, URL) ? '- [x]' : '- [ ]'; diff --git a/.github/actions/isStagingDeployLocked/index.js b/.github/actions/isStagingDeployLocked/index.js index d85fc16db06f..1808c24b2b7f 100644 --- a/.github/actions/isStagingDeployLocked/index.js +++ b/.github/actions/isStagingDeployLocked/index.js @@ -263,7 +263,7 @@ class GithubUtils { let issueBody = `**Release Version:** \`${tag}\`\r\n**Compare Changes:** https://github.com/Expensify/Expensify.cash/compare/production...staging\r\n`; // PR list - if (!_.isEmpty(PRList)) { + if (!_.isEmpty(sortedPRList)) { issueBody += '\r\n**This release contains changes from the following pull requests:**\r\n'; _.each(sortedPRList, (URL) => { issueBody += _.contains(verifiedPRList, URL) ? '- [x]' : '- [ ]'; diff --git a/.github/actions/markPullRequestsAsDeployed/index.js b/.github/actions/markPullRequestsAsDeployed/index.js index f7206fc4fbcf..9ce5a031d0d7 100644 --- a/.github/actions/markPullRequestsAsDeployed/index.js +++ b/.github/actions/markPullRequestsAsDeployed/index.js @@ -337,7 +337,7 @@ class GithubUtils { let issueBody = `**Release Version:** \`${tag}\`\r\n**Compare Changes:** https://github.com/Expensify/Expensify.cash/compare/production...staging\r\n`; // PR list - if (!_.isEmpty(PRList)) { + if (!_.isEmpty(sortedPRList)) { issueBody += '\r\n**This release contains changes from the following pull requests:**\r\n'; _.each(sortedPRList, (URL) => { issueBody += _.contains(verifiedPRList, URL) ? '- [x]' : '- [ ]'; diff --git a/.github/actions/reopenIssueWithComment/index.js b/.github/actions/reopenIssueWithComment/index.js index c0e4bd2822ba..9e09931d78b3 100644 --- a/.github/actions/reopenIssueWithComment/index.js +++ b/.github/actions/reopenIssueWithComment/index.js @@ -276,7 +276,7 @@ class GithubUtils { let issueBody = `**Release Version:** \`${tag}\`\r\n**Compare Changes:** https://github.com/Expensify/Expensify.cash/compare/production...staging\r\n`; // PR list - if (!_.isEmpty(PRList)) { + if (!_.isEmpty(sortedPRList)) { issueBody += '\r\n**This release contains changes from the following pull requests:**\r\n'; _.each(sortedPRList, (URL) => { issueBody += _.contains(verifiedPRList, URL) ? '- [x]' : '- [ ]'; diff --git a/.github/actions/triggerWorkflowAndWait/index.js b/.github/actions/triggerWorkflowAndWait/index.js index f592f53a454b..559a754eff75 100644 --- a/.github/actions/triggerWorkflowAndWait/index.js +++ b/.github/actions/triggerWorkflowAndWait/index.js @@ -427,7 +427,7 @@ class GithubUtils { let issueBody = `**Release Version:** \`${tag}\`\r\n**Compare Changes:** https://github.com/Expensify/Expensify.cash/compare/production...staging\r\n`; // PR list - if (!_.isEmpty(PRList)) { + if (!_.isEmpty(sortedPRList)) { issueBody += '\r\n**This release contains changes from the following pull requests:**\r\n'; _.each(sortedPRList, (URL) => { issueBody += _.contains(verifiedPRList, URL) ? '- [x]' : '- [ ]'; diff --git a/.github/libs/GithubUtils.js b/.github/libs/GithubUtils.js index d8cda484ba70..025a9c9281b2 100644 --- a/.github/libs/GithubUtils.js +++ b/.github/libs/GithubUtils.js @@ -225,7 +225,7 @@ class GithubUtils { let issueBody = `**Release Version:** \`${tag}\`\r\n**Compare Changes:** https://github.com/Expensify/Expensify.cash/compare/production...staging\r\n`; // PR list - if (!_.isEmpty(PRList)) { + if (!_.isEmpty(sortedPRList)) { issueBody += '\r\n**This release contains changes from the following pull requests:**\r\n'; _.each(sortedPRList, (URL) => { issueBody += _.contains(verifiedPRList, URL) ? '- [x]' : '- [ ]'; diff --git a/tests/unit/createOrUpdateStagingDeployTest.js b/tests/unit/createOrUpdateStagingDeployTest.js index d19cc6a1c570..fce7a09a9775 100644 --- a/tests/unit/createOrUpdateStagingDeployTest.js +++ b/tests/unit/createOrUpdateStagingDeployTest.js @@ -87,11 +87,7 @@ describe('createOrUpdateStagingDeployCash', () => { state: 'closed', }; - const baseNewPullRequests = [ - 'https://github.com/Expensify/Expensify.cash/pull/6', - 'https://github.com/Expensify/Expensify.cash/pull/7', - 'https://github.com/Expensify/Expensify.cash/pull/8', - ]; + const baseNewPullRequests = ['6', '7', '8']; test('creates new issue when there is none open', () => { mockGetInput.mockImplementation((arg) => { @@ -179,10 +175,7 @@ describe('createOrUpdateStagingDeployCash', () => { }); // New pull requests to add to open StagingDeployCash - const newPullRequests = [ - 'https://github.com/Expensify/Expensify.cash/pull/9', - 'https://github.com/Expensify/Expensify.cash/pull/10', - ]; + const newPullRequests = ['9', '10']; mockGetPullRequestsMergedBetween.mockImplementation((fromRef, toRef) => { if (fromRef === '1.0.1-0' && toRef === '1.0.2-2') { return [ @@ -229,7 +222,7 @@ describe('createOrUpdateStagingDeployCash', () => { // eslint-disable-next-line max-len html_url: `https://github.com/Expensify/Expensify.cash/issues/${openStagingDeployCashBefore.number}`, // eslint-disable-next-line max-len - body: `**Release Version:** \`1.0.2-2\`\r\n**Compare Changes:** https://github.com/Expensify/Expensify.cash/compare/production...staging\r\n\r\n**This release contains changes from the following pull requests:**\r\n- [ ] https://github.com/Expensify/Expensify.cash/pull/6\r\n- [x] https://github.com/Expensify/Expensify.cash/pull/7\r\n- [ ] https://github.com/Expensify/Expensify.cash/pull/8\r\n- [ ] ${newPullRequests[0]}\r\n- [ ] ${newPullRequests[1]}\r\n\r\n**Deploy Blockers:**\r\n- [ ] https://github.com/Expensify/Expensify.cash/pull/6\r\n- [ ] https://github.com/Expensify/Expensify.cash/issues/9\r\n- [x] https://github.com/Expensify/Expensify.cash/issues/10\r\n- [ ] https://github.com/Expensify/Expensify.cash/issues/11\r\n- [ ] https://github.com/Expensify/Expensify.cash/issues/12\r\n\r\ncc @Expensify/applauseleads\r\n`, + body: `**Release Version:** \`1.0.2-2\`\r\n**Compare Changes:** https://github.com/Expensify/Expensify.cash/compare/production...staging\r\n\r\n**This release contains changes from the following pull requests:**\r\n- [ ] https://github.com/Expensify/Expensify.cash/pull/6\r\n- [x] https://github.com/Expensify/Expensify.cash/pull/7\r\n- [ ] https://github.com/Expensify/Expensify.cash/pull/8\r\n- [ ] https://github.com/Expensify/Expensify.cash/pull/${newPullRequests[0]}\r\n- [ ] https://github.com/Expensify/Expensify.cash/pull/${newPullRequests[1]}\r\n\r\n**Deploy Blockers:**\r\n- [ ] https://github.com/Expensify/Expensify.cash/pull/6\r\n- [ ] https://github.com/Expensify/Expensify.cash/issues/9\r\n- [x] https://github.com/Expensify/Expensify.cash/issues/10\r\n- [ ] https://github.com/Expensify/Expensify.cash/issues/11\r\n- [ ] https://github.com/Expensify/Expensify.cash/issues/12\r\n\r\ncc @Expensify/applauseleads\r\n`, }); }); }); From 53ab90fc2bf7fd33b7a45afd04cf86271fbc6de8 Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Thu, 17 Jun 2021 12:09:13 -0700 Subject: [PATCH 4/4] Fix JS style --- .../createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js | 2 +- .github/actions/createOrUpdateStagingDeploy/index.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js b/.github/actions/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js index f05e26440c7d..75b890934476 100644 --- a/.github/actions/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js +++ b/.github/actions/createOrUpdateStagingDeploy/createOrUpdateStagingDeploy.js @@ -71,7 +71,7 @@ const run = function () { // then we should close / remove the DeployBlockerCash label from those return GithubUtils.generateStagingDeployCashBody( newVersion, - _.map(mergedPRs, GithubUtils.getPullRequestURLFromNumber) + _.map(mergedPRs, GithubUtils.getPullRequestURLFromNumber), ); } diff --git a/.github/actions/createOrUpdateStagingDeploy/index.js b/.github/actions/createOrUpdateStagingDeploy/index.js index c5904c03f529..4bc71ff41a46 100644 --- a/.github/actions/createOrUpdateStagingDeploy/index.js +++ b/.github/actions/createOrUpdateStagingDeploy/index.js @@ -81,7 +81,7 @@ const run = function () { // then we should close / remove the DeployBlockerCash label from those return GithubUtils.generateStagingDeployCashBody( newVersion, - _.map(mergedPRs, GithubUtils.getPullRequestURLFromNumber) + _.map(mergedPRs, GithubUtils.getPullRequestURLFromNumber), ); }