feat: use mostly markdown for pr bodies (#1118)

Existing solution uses HTML bodies for PR descriptions, as that was the easiest way to get consistency between GitHub and GitLab. However, VSTS supports only markdown so we needed to refactor how this is done. Now, GitHub PR bodies uses only minimal HTML (for summary/details) while GitLab PR bodies are converted to HTML using GitHub flavoured markdown for maximum compatibility. VSTS will be able to strip out the minimal markdown.

Closes #1018

lib/platform/gitlab/index.js

@@ -347,15 +347,17 @@ async function findPr(branchName, prTitle, state = 'all') {
 
 // Pull Request
 
-async function createPr(branchName, title, body, labels, useDefaultBranch) {
+async function createPr(
+  branchName,
+  title,
+  description,
+  labels,
+  useDefaultBranch
+) {
   const targetBranch = useDefaultBranch
     ? config.defaultBranch
     : config.baseBranch;
   logger.debug(`Creating Merge Request: ${title}`);
-  const description = body
-    .replace('</h4>', ' </h4>') // See #954
-    .replace(/Pull Request/g, 'Merge Request')
-    .replace(/PR/g, 'MR');
   const res = await get.post(`projects/${config.repoName}/merge_requests`, {
     body: {
       source_branch: branchName,
@@ -400,11 +402,7 @@ async function getPr(prNo) {
   return pr;
 }
 
-async function updatePr(prNo, title, body) {
+async function updatePr(prNo, title, description) {
-  const description = body
-    .replace('</h4>', ' </h4>') // See #954
-    .replace(/Pull Request/g, 'Merge Request')
-    .replace(/PR/g, 'MR');
   await get.put(`projects/${config.repoName}/merge_requests/${prNo}`, {
     body: {
       title,

lib/workers/pr/index.js

@@ -129,22 +129,23 @@ async function ensurePr(prConfig) {
   }
 
   const prTitle = handlebars.compile(config.prTitle)(config);
-  let prBody;
+  let prBody = handlebars.compile(config.prBody)(config);
-  function trimPrBody() {
+  // Put a zero width space after every @ symbol to prevent unintended hyperlinking
-    let prBodyMarkdown = handlebars.compile(config.prBody)(config);
+  prBody = prBody.replace('@', '@​');
-    prBodyMarkdown = prBodyMarkdown.replace('@', '@​');
-    prBody = converter.makeHtml(prBodyMarkdown);
   // Public GitHub repos need links prevented - see #489
   prBody = prBody.replace(issueRe, '$1#​$2$3');
+  // convert escaped backticks back to `
   const backTickRe = /`([^/]*?)`/g;
-    prBody = prBody.replace(backTickRe, '<code>$1</code>');
+  prBody = prBody.replace(backTickRe, '`$1`');
-    // istanbul ignore if
+  // It would be nice to abstract this to the platform layer but made difficult due to our create/update check
-    if (prBody.length > 250000) {
+  if (config.platform === 'gitlab') {
-      config.upgrades.pop();
+    // Convert to HTML using GitHub-flavoured markdown as it is more feature-rich than GitLab's flavour
-      trimPrBody();
+    prBody = converter
+      .makeHtml(prBody)
+      .replace('</h4>', ' </h4>') // See #954
+      .replace(/Pull Request/g, 'Merge Request')
+      .replace(/PR/g, 'MR');
   }
-  }
-  trimPrBody();
 
   try {
     // Check if existing PR exists

test/workers/pr/__snapshots__/index.spec.js.snap

@@ -26,19 +26,71 @@ Array [
 ]
 `;
 
-exports[`workers/pr ensurePr should return modified existing PR 1`] = `
+exports[`workers/pr ensurePr should convert to HTML PR for gitlab 1`] = `
-Object {
+Array [
-  "body": "<p>This Pull 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>
+  "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>
 <h3 id=\\"commits\\">Commits</h3>
 <p><details><br />
 <summary>renovateapp/dummy</summary></p>
-<h4 id=\\"110\\">1.1.0</h4>
+<h4 id=\\"110\\">1.1.0 </h4>
 <ul>
 <li><a href=\\"https://github.com/renovateapp/dummy/commit/abcdefghijklmnopqrstuvwxyz\\"><code>abcdefg</code></a> foo <a href=\\"https://github.com/renovateapp/dummy/issues/3\\">#3</a></li>
 </ul>
 <p></details></p>
 <hr />
-<p>This PR has been generated by <a href=\\"https://renovateapp.com\\">Renovate Bot</a>.</p>",
+<p>This MR has been generated by <a href=\\"https://renovateapp.com\\">Renovate Bot</a>.</p>",
+  Array [],
+]
+`;
+
+exports[`workers/pr ensurePr should create PR if success 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\`
+
+
+### Commits
+
+<details>
+<summary>renovateapp/dummy</summary>
+
+#### 1.1.0
+-   [\`abcdefg\`](https://github.com/renovateapp/dummy/commit/abcdefghijklmnopqrstuvwxyz) foo [#3](https://github.com/renovateapp/dummy/issues/3)
+
+</details>
+
+
+
+---
+
+This PR has been generated by [Renovate Bot](https://renovateapp.com).",
+  Array [],
+]
+`;
+
+exports[`workers/pr ensurePr should return modified existing PR 1`] = `
+Object {
+  "body": "This Pull Request updates dependency [dummy](https://github.com/renovateapp/dummy) from \`v1.0.0\` to \`v1.1.0\`
+
+
+### Commits
+
+<details>
+<summary>renovateapp/dummy</summary>
+
+#### 1.1.0
+-   [\`abcdefg\`](https://github.com/renovateapp/dummy/commit/abcdefghijklmnopqrstuvwxyz) foo [#3](https://github.com/renovateapp/dummy/issues/3)
+
+</details>
+
+
+
+---
+
+This PR has been generated by [Renovate Bot](https://renovateapp.com).",
   "displayNumber": "Existing PR",
   "title": "Update dependency dummy to v1.1.0",
 }

test/workers/pr/index.spec.js

@@ -86,28 +86,24 @@ describe('workers/pr', () => {
   });
   describe('ensurePr', () => {
     let config;
-    let existingPr;
+    const existingPr = {
+      displayNumber: 'Existing PR',
+      title: 'Update dependency dummy to v1.1.0',
+    };
     beforeEach(() => {
       config = {
         ...defaultConfig,
       };
+      config.branchName = 'renovate/dummy-1.x';
+      config.prTitle = 'Update dependency dummy to v1.1.0';
+      config.depName = 'dummy';
+      config.isGitHub = true;
+      config.privateRepo = true;
+      config.currentVersion = '1.0.0';
+      config.newVersion = '1.1.0';
+      config.repositoryUrl = 'https://github.com/renovateapp/dummy';
       platform.createPr.mockReturnValue({ displayNumber: 'New Pull Request' });
       config.upgrades = [config];
-      existingPr = {
-        title: 'Update dependency dummy to v1.1.0',
-        body: `<p>This Pull 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>
-<h3 id="commits">Commits</h3>
-<p><details><br />
-<summary>renovateapp/dummy</summary></p>
-<h4 id="110">1.1.0</h4>
-<ul>
-<li><a href="https://github.com/renovateapp/dummy/commit/abcdefghijklmnopqrstuvwxyz"><code>abcdefg</code></a> foo <a href="https://github.com/renovateapp/dummy/issues/3">#3</a></li>
-</ul>
-<p></details></p>
-<hr />
-<p>This PR has been generated by <a href="https://renovateapp.com">Renovate Bot</a>.</p>`,
-        displayNumber: 'Existing PR',
-      };
     });
     afterEach(() => {
       jest.clearAllMocks();
@@ -130,6 +126,19 @@ describe('workers/pr', () => {
       config.prCreation = 'status-success';
       const pr = await prWorker.ensurePr(config);
       expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+      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.platform = 'gitlab';
+      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 MR')).not.toBe(
+        -1
+      );
     });
     it('should delete branch and return null if creating PR fails', async () => {
       platform.getBranchStatus.mockReturnValueOnce('success');
@@ -210,10 +219,10 @@ describe('workers/pr', () => {
       config.warnings = [{}];
       const pr = await prWorker.ensurePr(config);
       expect(
-        platform.createPr.mock.calls[0][2].indexOf('Errors</h3>')
+        platform.createPr.mock.calls[0][2].indexOf('### Errors')
       ).not.toEqual(-1);
       expect(
-        platform.createPr.mock.calls[0][2].indexOf('Warnings</h3>')
+        platform.createPr.mock.calls[0][2].indexOf('### Warnings')
       ).not.toEqual(-1);
       expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
     });
@@ -227,15 +236,9 @@ describe('workers/pr', () => {
       expect(platform.addReviewers.mock.calls.length).toBe(0);
     });
     it('should add assignees and reviewers to existing PR', async () => {
-      config.depName = 'dummy';
-      config.automerge = true;
       config.assignees = ['bar'];
       config.reviewers = ['baz'];
-      config.isGitHub = true;
+      config.automerge = true;
-      config.privateRepo = true;
-      config.currentVersion = '1.0.0';
-      config.newVersion = '1.1.0';
-      config.repositoryUrl = 'https://github.com/renovateapp/dummy';
       platform.getBranchPr.mockReturnValueOnce(existingPr);
       platform.getBranchStatus.mockReturnValueOnce('failure');
       config.semanticPrefix = '';
@@ -247,24 +250,15 @@ describe('workers/pr', () => {
       expect(pr).toMatchObject(existingPr);
     });
     it('should return unmodified existing PR', async () => {
-      config.depName = 'dummy';
-      config.isGitHub = true;
-      config.privateRepo = true;
-      config.currentVersion = '1.0.0';
-      config.newVersion = '1.1.0';
-      config.repositoryUrl = 'https://github.com/renovateapp/dummy';
       platform.getBranchPr.mockReturnValueOnce(existingPr);
       config.semanticPrefix = '';
       const pr = await prWorker.ensurePr(config);
       expect(platform.updatePr.mock.calls).toMatchSnapshot();
-      expect(platform.updatePr.mock.calls.length).toBe(0);
+      expect(platform.updatePr.mock.calls).toHaveLength(0);
       expect(pr).toMatchObject(existingPr);
     });
     it('should return modified existing PR', async () => {
-      config.depName = 'dummy';
-      config.currentVersion = '1.0.0';
       config.newVersion = '1.2.0';
-      config.isGitHub = true;
       platform.getBranchPr.mockReturnValueOnce(existingPr);
       const pr = await prWorker.ensurePr(config);
       expect(pr).toMatchSnapshot();