From 981d5a1cce93b450a7749dc6c7967267f1d485ab Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Sun, 22 Jul 2018 07:47:23 +0200
Subject: [PATCH] feat: set prBody formatting per-platform
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Pushes “getPrBody†logic into platform level to allow platforms to individually do things like text replacements and body length limiting.
Closes #2267
---
lib/platform/github/index.js | 13 ++++
lib/platform/gitlab/index.js | 19 ++++++
lib/platform/vsts/index.js | 10 +++
lib/workers/pr/index.js | 67 +++++--------------
.../platform/__snapshots__/index.spec.js.snap | 3 +
.../github/__snapshots__/index.spec.js.snap | 2 +
test/platform/github/index.spec.js | 7 ++
.../gitlab/__snapshots__/index.spec.js.snap | 2 +
test/platform/gitlab/index.spec.js | 7 ++
.../vsts/__snapshots__/index.spec.js.snap | 2 +
test/platform/vsts/index.spec.js | 8 ++-
.../pr/__snapshots__/index.spec.js.snap | 40 -----------
test/workers/pr/index.spec.js | 21 +-----
13 files changed, 88 insertions(+), 113 deletions(-)
diff --git a/lib/platform/github/index.js b/lib/platform/github/index.js
index 2eb8d3ac22..71824311f8 100644
--- a/lib/platform/github/index.js
+++ b/lib/platform/github/index.js
@@ -45,6 +45,7 @@ module.exports = {
getPrFiles,
updatePr,
mergePr,
+ getPrBody,
// file
commitFilesToBranch,
getFile,
@@ -1108,6 +1109,18 @@ async function mergePr(prNo, branchName) {
return true;
}
+function getPrBody(input) {
+ let prBody = input.replace(
+ /(https?:\/\/github.com\/[^/]*\/[^/]*\/(issues|pull)\/\w+)/g,
+ '`$1`'
+ );
+ prBody = prBody.replace(
+ /]\(https?:\/\/github.com\/[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}\)/gi,
+ ']'
+ );
+ return prBody.substring(0, 60000);
+}
+
// Generic File operations
async function getFile(filePath, branchName) {
diff --git a/lib/platform/gitlab/index.js b/lib/platform/gitlab/index.js
index d299f395e6..859e3bcfcf 100644
--- a/lib/platform/gitlab/index.js
+++ b/lib/platform/gitlab/index.js
@@ -1,8 +1,12 @@
const is = require('@sindresorhus/is');
const addrs = require('email-addresses');
+const showdown = require('showdown');
const get = require('./gl-got-wrapper');
const endpoints = require('../../util/endpoints');
+const converter = new showdown.Converter();
+converter.setFlavor('github');
+
let config = {};
module.exports = {
@@ -40,6 +44,7 @@ module.exports = {
getPrFiles,
updatePr,
mergePr,
+ getPrBody,
// file
commitFilesToBranch,
getFile,
@@ -697,6 +702,20 @@ async function mergePr(iid) {
return true;
}
+function getPrBody(input) {
+ // Convert to HTML using GitHub-flavoured markdown as it is more feature-rich than GitLab's flavour
+ return converter
+ .makeHtml(input)
+ .replace(/<\/?h4[^>]*>/g, '**') // See #954
+ .replace(/Pull Request/g, 'Merge Request')
+ .replace(/PR/g, 'MR')
+ .replace(/<summary>/g, '<h1>')
+ .replace(/<\/summary><\/p>/g, '</h1>')
+ .replace(/<p><details><br \/>\n/g, '')
+ .replace(/<p><\/details><\/p>\n/g, '');
+ // TODO: set maximum length
+}
+
// Generic File operations
async function getFile(filePath, branchName) {
diff --git a/lib/platform/vsts/index.js b/lib/platform/vsts/index.js
index d0be6f6f7d..b1de220485 100644
--- a/lib/platform/vsts/index.js
+++ b/lib/platform/vsts/index.js
@@ -41,6 +41,7 @@ module.exports = {
getPrFiles,
updatePr,
mergePr,
+ getPrBody,
// file
commitFilesToBranch,
getFile,
@@ -493,6 +494,15 @@ async function mergePr(pr) {
await null;
}
+function getPrBody(input) {
+ // Remove any HTML we use
+ return input
+ .replace('<summary>', '**')
+ .replace('</summary>', '**')
+ .replace('<details>', '')
+ .replace('</details>', '');
+}
+
function ensureIssue() {
// istanbul ignore next
logger.warn(`ensureIssue() is not implemented`);
diff --git a/lib/workers/pr/index.js b/lib/workers/pr/index.js
index 15c1af0e7f..2bb6a0afc8 100644
--- a/lib/workers/pr/index.js
+++ b/lib/workers/pr/index.js
@@ -1,10 +1,6 @@
const handlebars = require('handlebars');
-const showdown = require('showdown');
const changelogHelper = require('./changelog');
-const converter = new showdown.Converter();
-converter.setFlavor('github');
-
module.exports = {
ensurePr,
checkAutoMerge,
@@ -139,12 +135,6 @@ async function ensurePr(prConfig) {
upgrade.releases.push(release);
});
}
- // istanbul ignore next
- for (let i = 15; i < upgrade.releases.length; i += 1) {
- if (upgrade.releases[i].releaseNotes) {
- upgrade.releases[i].releaseNotes.body = null;
- }
- }
}
config.upgrades.push(upgrade);
}
@@ -184,48 +174,21 @@ async function ensurePr(prConfig) {
}
}
prBody = prBody.trim();
- if (config.isGitHub) {
- // Put a zero width space after every # followed by a digit
- prBody = prBody.replace(/#(\d)/gi, '#​$1');
- // Put a zero width space after every @ symbol to prevent unintended hyperlinking
- prBody = prBody.replace(/@/g, '@​');
- prBody = prBody.replace(/(`\[?@)​/g, '$1');
- // Public GitHub repos need links prevented - see #489
- prBody = prBody.replace(issueRe, '$1#​$2$3');
- prBody = prBody.replace(
- /(https?:\/\/github.com\/[^/]*\/[^/]*\/(issues|pull)\/\w+)/g,
- '`$1`'
- );
- logger.trace('Escaping links to GitHub users');
- prBody = prBody.replace(
- /]\(https?:\/\/github.com\/[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}\)/gi,
- ']'
- );
- // convert escaped backticks back to `
- const backTickRe = /`([^/]*?)`/g;
- prBody = prBody.replace(backTickRe, '`$1`');
- prBody = prBody.replace(/`#​(\d+)`/g, '`#$1`');
- }
- // It would be nice to abstract this to the platform layer but made difficult due to our create/update check
- if (config.isGitLab) {
- // Convert to HTML using GitHub-flavoured markdown as it is more feature-rich than GitLab's flavour
- prBody = converter
- .makeHtml(prBody)
- .replace(/<\/?h4[^>]*>/g, '**') // See #954
- .replace(/Pull Request/g, 'Merge Request')
- .replace(/PR/g, 'MR')
- .replace(/<summary>/g, '<h1>')
- .replace(/<\/summary><\/p>/g, '</h1>')
- .replace(/<p><details><br \/>\n/g, '')
- .replace(/<p><\/details><\/p>\n/g, '');
- } else if (config.isVsts) {
- // Remove any HTML we use
- prBody = prBody
- .replace('<summary>', '**')
- .replace('</summary>', '**')
- .replace('<details>', '')
- .replace('</details>', '');
- }
+
+ // Generic replacements/link-breakers
+
+ // Put a zero width space after every # followed by a digit
+ prBody = prBody.replace(/#(\d)/gi, '#​$1');
+ // Put a zero width space after every @ symbol to prevent unintended hyperlinking
+ prBody = prBody.replace(/@/g, '@​');
+ prBody = prBody.replace(/(`\[?@)​/g, '$1');
+ prBody = prBody.replace(issueRe, '$1#​$2$3');
+ // convert escaped backticks back to `
+ const backTickRe = /`([^/]*?)`/g;
+ prBody = prBody.replace(backTickRe, '`$1`');
+ prBody = prBody.replace(/`#​(\d+)`/g, '`#$1`');
+
+ prBody = platform.getPrBody(prBody);
try {
if (existingPr) {
diff --git a/test/platform/__snapshots__/index.spec.js.snap b/test/platform/__snapshots__/index.spec.js.snap
index e7232e687d..406bdafa74 100644
--- a/test/platform/__snapshots__/index.spec.js.snap
+++ b/test/platform/__snapshots__/index.spec.js.snap
@@ -31,6 +31,7 @@ Array [
"getPrFiles",
"updatePr",
"mergePr",
+ "getPrBody",
"commitFilesToBranch",
"getFile",
"getCommitMessages",
@@ -68,6 +69,7 @@ Array [
"getPrFiles",
"updatePr",
"mergePr",
+ "getPrBody",
"commitFilesToBranch",
"getFile",
"getCommitMessages",
@@ -105,6 +107,7 @@ Array [
"getPrFiles",
"updatePr",
"mergePr",
+ "getPrBody",
"commitFilesToBranch",
"getFile",
"getCommitMessages",
diff --git a/test/platform/github/__snapshots__/index.spec.js.snap b/test/platform/github/__snapshots__/index.spec.js.snap
index 2352841cfe..161db623cc 100644
--- a/test/platform/github/__snapshots__/index.spec.js.snap
+++ b/test/platform/github/__snapshots__/index.spec.js.snap
@@ -454,6 +454,8 @@ Object {
}
`;
+exports[`platform/github getPrBody(input) returns updated pr body 1`] = `"\`https://github.com/foo/bar/issues/5\` plus also [a link](\`https://github.com/foo/bar/issues/5\`)"`;
+
exports[`platform/github getPrFiles() returns files 1`] = `
Array [
"renovate.json",
diff --git a/test/platform/github/index.spec.js b/test/platform/github/index.spec.js
index 245138fb3c..9a7c16c1f4 100644
--- a/test/platform/github/index.spec.js
+++ b/test/platform/github/index.spec.js
@@ -1481,6 +1481,13 @@ describe('platform/github', () => {
expect(get.mock.calls).toHaveLength(3);
});
});
+ describe('getPrBody(input)', () => {
+ it('returns updated pr body', () => {
+ const input =
+ 'https://github.com/foo/bar/issues/5 plus also [a link](https://github.com/foo/bar/issues/5)';
+ expect(github.getPrBody(input)).toMatchSnapshot();
+ });
+ });
describe('mergePr(prNo) - autodetection', () => {
beforeEach(async () => {
function guessInitRepo(...args) {
diff --git a/test/platform/gitlab/__snapshots__/index.spec.js.snap b/test/platform/gitlab/__snapshots__/index.spec.js.snap
index f192d986d0..972298c3ac 100644
--- a/test/platform/gitlab/__snapshots__/index.spec.js.snap
+++ b/test/platform/gitlab/__snapshots__/index.spec.js.snap
@@ -243,6 +243,8 @@ Object {
}
`;
+exports[`platform/gitlab getPrBody(input) returns updated pr body 1`] = `"<p><a href=\\"https://github.com/foo/bar/issues/5\\">https://github.com/foo/bar/issues/5</a> plus also <a href=\\"https://github.com/foo/bar/issues/5\\">a link</a></p>"`;
+
exports[`platform/gitlab getPrFiles() returns files 1`] = `
Array [
"renovate.json",
diff --git a/test/platform/gitlab/index.spec.js b/test/platform/gitlab/index.spec.js
index 8b255dae51..e6f034b908 100644
--- a/test/platform/gitlab/index.spec.js
+++ b/test/platform/gitlab/index.spec.js
@@ -842,6 +842,13 @@ describe('platform/gitlab', () => {
expect(get.put.mock.calls.length).toEqual(1);
});
});
+ describe('getPrBody(input)', () => {
+ it('returns updated pr body', () => {
+ const input =
+ 'https://github.com/foo/bar/issues/5 plus also [a link](https://github.com/foo/bar/issues/5)';
+ expect(gitlab.getPrBody(input)).toMatchSnapshot();
+ });
+ });
describe('getFile(filePath, branchName)', () => {
beforeEach(async () => {
get.mockReturnValueOnce({ body: [{ type: 'blob', path: 'some-path' }] });
diff --git a/test/platform/vsts/__snapshots__/index.spec.js.snap b/test/platform/vsts/__snapshots__/index.spec.js.snap
index 2e8400ff0f..87cefbc924 100644
--- a/test/platform/vsts/__snapshots__/index.spec.js.snap
+++ b/test/platform/vsts/__snapshots__/index.spec.js.snap
@@ -114,6 +114,8 @@ Object {
}
`;
+exports[`platform/vsts getPrBody(input) returns updated pr body 1`] = `"https://github.com/foo/bar/issues/5 plus also [a link](https://github.com/foo/bar/issues/5)"`;
+
exports[`platform/vsts getRepos should return an array of repos 1`] = `
Array [
Array [],
diff --git a/test/platform/vsts/index.spec.js b/test/platform/vsts/index.spec.js
index 681c9acddb..193e68d5a2 100644
--- a/test/platform/vsts/index.spec.js
+++ b/test/platform/vsts/index.spec.js
@@ -691,7 +691,13 @@ describe('platform/vsts', () => {
expect(vstsApi.gitApi.mock.calls.length).toBe(3);
});
});
-
+ describe('getPrBody(input)', () => {
+ it('returns updated pr body', () => {
+ const input =
+ '<details>https://github.com/foo/bar/issues/5 plus also [a link](https://github.com/foo/bar/issues/5)';
+ expect(vsts.getPrBody(input)).toMatchSnapshot();
+ });
+ });
describe('Not supported by VSTS (yet!)', () => {
it('setBranchStatus', () => {
const res = vsts.setBranchStatus();
diff --git a/test/workers/pr/__snapshots__/index.spec.js.snap b/test/workers/pr/__snapshots__/index.spec.js.snap
index 9580360786..f01ce8fe58 100644
--- a/test/workers/pr/__snapshots__/index.spec.js.snap
+++ b/test/workers/pr/__snapshots__/index.spec.js.snap
@@ -26,22 +26,6 @@ Array [
]
`;
-exports[`workers/pr ensurePr should convert to HTML PR for gitlab 1`] = `
-Array [
- "renovate/dummy-1.x",
- "Update dependency dummy to v1.1.0",
- "<p>This Merge Request updates dependency <a href=\\"https://github.com/renovateapp/dummy\\">dummy</a> from <code>v1.0.0</code> to <code>v1.1.0</code></p>
-<h1>Release Notes</h1>
-<h3 id=\\"v110httpsgithubcomrenovateappdummycomparev100v110\\"><a href=\\"https://github.com/renovateapp/dummy/compare/v1.0.0…v1.1.0\\"><code>v1.1.0</code></a></h3>
-<p><a href=\\"https://github.com/renovateapp/dummy/compare/v1.0.0…v1.1.0\\">Compare Source</a></p>
-<hr />
-<p></details></p>",
- Array [],
- false,
- false,
-]
-`;
-
exports[`workers/pr ensurePr should create PR if success 1`] = `
Array [
"renovate/dummy-1.x",
@@ -133,27 +117,3 @@ Object {
`;
exports[`workers/pr ensurePr should return unmodified existing PR 1`] = `Array []`;
-
-exports[`workers/pr ensurePr should strip HTML PR for vsts 1`] = `
-Array [
- "renovate/dummy-1.x",
- "Update dependency dummy to v1.1.0",
- "This Pull Request updates dependency [dummy](https://github.com/renovateapp/dummy) from \`v1.0.0\` to \`v1.1.0\`
-
-
-
-
-**Release Notes**
-
-### [\`v1.1.0\`](https://github.com/renovateapp/dummy/compare/v1.0.0...v1.1.0)
-[Compare Source](https://github.com/renovateapp/dummy/compare/v1.0.0...v1.1.0)
-
-
----
-
-",
- Array [],
- false,
- false,
-]
-`;
diff --git a/test/workers/pr/index.spec.js b/test/workers/pr/index.spec.js
index 12a9459e3c..be24b1a835 100644
--- a/test/workers/pr/index.spec.js
+++ b/test/workers/pr/index.spec.js
@@ -118,6 +118,7 @@ describe('workers/pr', () => {
config.repositoryUrl = 'https://github.com/renovateapp/dummy';
platform.createPr.mockReturnValue({ displayNumber: 'New Pull Request' });
config.upgrades = [config];
+ platform.getPrBody = jest.fn(input => input);
});
afterEach(() => {
jest.clearAllMocks();
@@ -145,26 +146,6 @@ describe('workers/pr', () => {
expect(platform.createPr.mock.calls[0]).toMatchSnapshot();
existingPr.body = platform.createPr.mock.calls[0][2];
});
- it('should convert to HTML PR for gitlab', async () => {
- platform.getBranchStatus.mockReturnValueOnce('success');
- config.prCreation = 'status-success';
- config.isGitLab = true;
- const pr = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
- expect(platform.createPr.mock.calls[0]).toMatchSnapshot();
- expect(
- platform.createPr.mock.calls[0][2].indexOf('<p>This Merge Request')
- ).not.toBe(-1);
- });
- it('should strip HTML PR for vsts', async () => {
- platform.getBranchStatus.mockReturnValueOnce('success');
- config.prCreation = 'status-success';
- config.isVsts = true;
- const pr = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
- expect(platform.createPr.mock.calls[0]).toMatchSnapshot();
- expect(platform.createPr.mock.calls[0][2].indexOf('<details>')).toBe(-1);
- });
it('should return null if creating PR fails', async () => {
platform.getBranchStatus.mockReturnValueOnce('success');
platform.createPr = jest.fn();
--
GitLab