From 5fb9d77bba18775bc50516fa081f4246ca278773 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Mon, 27 Aug 2018 06:25:17 +0200
Subject: [PATCH] feat: separate groups into major/minor/patch (#2426)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously if grouping, all major/minor updates within that group were combined into one. Instead, we now honour the “separateMajorMinor”, "separateMinorPatch", and "separateMultipleMajor" settings and keep the groups separate if necessary.

For maximum compatibility with existing PRs, we name branches like `renovate/group-name` whenever possible and only name them like `renovate/major-group-name` or `renovate/patch-group-name` if major or patch are found.

Closes #2425
---
 lib/config/definitions.js                     |  2 -
 .../repository/process/lookup/index.js        |  1 -
 lib/workers/repository/updates/branchify.js   | 10 +++
 lib/workers/repository/updates/generate.js    | 12 +++
 .../lookup/__snapshots__/index.spec.js.snap   | 76 ++++++++++++++++---
 .../repository/process/lookup/index.spec.js   | 36 ++++++++-
 .../__snapshots__/flatten.spec.js.snap        | 10 +++
 .../repository/updates/branchify.spec.js      | 59 ++++++++++++++
 8 files changed, 191 insertions(+), 15 deletions(-)

diff --git a/lib/config/definitions.js b/lib/config/definitions.js
index b98d0a1075..d0a7b4e221 100644
--- a/lib/config/definitions.js
+++ b/lib/config/definitions.js
@@ -459,7 +459,6 @@ const options = [
     name: 'separateMajorMinor',
     description:
       'If set to false, it will upgrade dependencies to latest release only, and not separate major/minor branches',
-    stage: 'package',
     type: 'boolean',
   },
   {
@@ -474,7 +473,6 @@ const options = [
     name: 'separateMinorPatch',
     description:
       'If set to true, it will separate minor and patch updates into separate branches',
-    stage: 'package',
     type: 'boolean',
     default: false,
   },
diff --git a/lib/workers/repository/process/lookup/index.js b/lib/workers/repository/process/lookup/index.js
index 32001e28ca..506d18d917 100644
--- a/lib/workers/repository/process/lookup/index.js
+++ b/lib/workers/repository/process/lookup/index.js
@@ -245,7 +245,6 @@ function getBucket(config, update) {
   const { updateType, newMajor } = update;
   if (
     !separateMajorMinor ||
-    config.groupName ||
     config.major.automerge === true ||
     (config.automerge && config.major.automerge !== false)
   ) {
diff --git a/lib/workers/repository/updates/branchify.js b/lib/workers/repository/updates/branchify.js
index 5d009a06cb..504dad30cb 100644
--- a/lib/workers/repository/updates/branchify.js
+++ b/lib/workers/repository/updates/branchify.js
@@ -48,6 +48,16 @@ function branchifyUpgrades(config, packageFiles) {
       update.groupSlug = slugify(update.groupSlug || update.groupName, {
         lower: true,
       });
+      if (update.updateType === 'major' && update.separateMajorMinor) {
+        if (update.separateMultipleMajor) {
+          update.groupSlug = `major-${update.newMajor}-${update.groupSlug}`;
+        } else {
+          update.groupSlug = `major-${update.groupSlug}`;
+        }
+      }
+      if (update.updateType === 'patch') {
+        update.groupSlug = `patch-${update.groupSlug}`;
+      }
       update.branchTopic = update.group.branchTopic || update.branchTopic;
       update.branchName = handlebars.compile(
         update.group.branchName || update.branchName
diff --git a/lib/workers/repository/updates/generate.js b/lib/workers/repository/updates/generate.js
index ff916fa9d8..c698a2c573 100644
--- a/lib/workers/repository/updates/generate.js
+++ b/lib/workers/repository/updates/generate.js
@@ -40,6 +40,7 @@ function generateBranchConfig(branchUpgrades) {
     if (useGroupSettings) {
       // Now overwrite original config with group config
       upgrade = mergeChildConfig(upgrade, upgrade.group);
+      upgrade.isGroup = true;
     } else {
       delete upgrade.groupName;
     }
@@ -113,6 +114,17 @@ function generateBranchConfig(branchUpgrades) {
       [upgrade.prTitle] = upgrade.commitMessage.split('\n');
     }
     upgrade.prTitle += upgrade.hasBaseBranches ? ' ({{baseBranch}})' : '';
+    if (upgrade.isGroup) {
+      upgrade.prTitle +=
+        upgrade.updateType === 'major' && upgrade.separateMajorMinor
+          ? ' (major)'
+          : '';
+      upgrade.prTitle +=
+        upgrade.updateType === 'minor' && upgrade.separateMinorPatch
+          ? ' (minor)'
+          : '';
+      upgrade.prTitle += upgrade.updateType === 'patch' ? ' (patch)' : '';
+    }
     logger.debug(`prTitle: ` + JSON.stringify(upgrade.prTitle));
     // Compile again to allow for nested handlebars templates
     upgrade.prTitle = handlebars.compile(upgrade.prTitle)(upgrade);
diff --git a/test/workers/repository/process/lookup/__snapshots__/index.spec.js.snap b/test/workers/repository/process/lookup/__snapshots__/index.spec.js.snap
index cf7287c0e1..5f6cf88de6 100644
--- a/test/workers/repository/process/lookup/__snapshots__/index.spec.js.snap
+++ b/test/workers/repository/process/lookup/__snapshots__/index.spec.js.snap
@@ -340,6 +340,44 @@ Array [
 ]
 `;
 
+exports[`manager/npm/lookup .lookupUpdates() returns additional update if grouping but separateMinorPatch=true 1`] = `
+Array [
+  Object {
+    "canBeUnpublished": false,
+    "fromVersion": "0.4.0",
+    "isSingleVersion": true,
+    "newMajor": 0,
+    "newMinor": 4,
+    "newValue": "0.4.4",
+    "releaseTimestamp": "2011-06-10T17:20:04.719Z",
+    "toVersion": "0.4.4",
+    "updateType": "patch",
+  },
+  Object {
+    "canBeUnpublished": false,
+    "fromVersion": "0.4.0",
+    "isSingleVersion": true,
+    "newMajor": 0,
+    "newMinor": 9,
+    "newValue": "0.9.7",
+    "releaseTimestamp": "2013-09-04T17:07:22.948Z",
+    "toVersion": "0.9.7",
+    "updateType": "minor",
+  },
+  Object {
+    "canBeUnpublished": false,
+    "fromVersion": "0.4.0",
+    "isSingleVersion": true,
+    "newMajor": 1,
+    "newMinor": 4,
+    "newValue": "1.4.1",
+    "releaseTimestamp": "2015-05-17T04:25:07.299Z",
+    "toVersion": "1.4.1",
+    "updateType": "major",
+  },
+]
+`;
+
 exports[`manager/npm/lookup .lookupUpdates() returns both updates if automerging minor 1`] = `
 Array [
   Object {
@@ -470,8 +508,19 @@ Array [
 ]
 `;
 
-exports[`manager/npm/lookup .lookupUpdates() returns only one update if automerging 1`] = `
+exports[`manager/npm/lookup .lookupUpdates() returns multiple updates if grouping but separateMajorMinor=true 1`] = `
 Array [
+  Object {
+    "canBeUnpublished": false,
+    "fromVersion": "0.4.0",
+    "isSingleVersion": true,
+    "newMajor": 0,
+    "newMinor": 9,
+    "newValue": "0.9.7",
+    "releaseTimestamp": "2013-09-04T17:07:22.948Z",
+    "toVersion": "0.9.7",
+    "updateType": "minor",
+  },
   Object {
     "canBeUnpublished": false,
     "fromVersion": "0.4.0",
@@ -486,18 +535,27 @@ Array [
 ]
 `;
 
-exports[`manager/npm/lookup .lookupUpdates() returns only one update if automerging major 1`] = `
+exports[`manager/npm/lookup .lookupUpdates() returns one update if grouping and separateMajorMinor=false 1`] = `
 Array [
   Object {
-    "isPin": true,
-    "newMajor": 0,
-    "newValue": "0.4.4",
-    "updateType": "pin",
+    "canBeUnpublished": false,
+    "fromVersion": "0.4.0",
+    "isSingleVersion": true,
+    "newMajor": 1,
+    "newMinor": 4,
+    "newValue": "1.4.1",
+    "releaseTimestamp": "2015-05-17T04:25:07.299Z",
+    "toVersion": "1.4.1",
+    "updateType": "major",
   },
+]
+`;
+
+exports[`manager/npm/lookup .lookupUpdates() returns only one update if automerging 1`] = `
+Array [
   Object {
-    "blockedByPin": true,
     "canBeUnpublished": false,
-    "fromVersion": "0.4.4",
+    "fromVersion": "0.4.0",
     "isSingleVersion": true,
     "newMajor": 1,
     "newMinor": 4,
@@ -509,7 +567,7 @@ Array [
 ]
 `;
 
-exports[`manager/npm/lookup .lookupUpdates() returns only one update if grouping 1`] = `
+exports[`manager/npm/lookup .lookupUpdates() returns only one update if automerging major 1`] = `
 Array [
   Object {
     "isPin": true,
diff --git a/test/workers/repository/process/lookup/index.spec.js b/test/workers/repository/process/lookup/index.spec.js
index 7f0e8a5e1a..7109cab770 100644
--- a/test/workers/repository/process/lookup/index.spec.js
+++ b/test/workers/repository/process/lookup/index.spec.js
@@ -52,16 +52,46 @@ describe('manager/npm/lookup', () => {
         .reply(200, qJson);
       expect((await lookup.lookupUpdates(config)).updates).toMatchSnapshot();
     });
-    it('returns only one update if grouping', async () => {
+    it('returns multiple updates if grouping but separateMajorMinor=true', async () => {
       config.groupName = 'somegroup';
-      config.currentValue = '^0.4.0';
+      config.currentValue = '0.4.0';
       config.rangeStrategy = 'pin';
       config.depName = 'q';
       config.purl = 'pkg:npm/q';
       nock('https://registry.npmjs.org')
         .get('/q')
         .reply(200, qJson);
-      expect((await lookup.lookupUpdates(config)).updates).toMatchSnapshot();
+      const res = await lookup.lookupUpdates(config);
+      expect(res.updates).toMatchSnapshot();
+      expect(res.updates).toHaveLength(2);
+    });
+    it('returns additional update if grouping but separateMinorPatch=true', async () => {
+      config.groupName = 'somegroup';
+      config.currentValue = '0.4.0';
+      config.rangeStrategy = 'pin';
+      config.depName = 'q';
+      config.separateMinorPatch = true;
+      config.purl = 'pkg:npm/q';
+      nock('https://registry.npmjs.org')
+        .get('/q')
+        .reply(200, qJson);
+      const res = await lookup.lookupUpdates(config);
+      expect(res.updates).toMatchSnapshot();
+      expect(res.updates).toHaveLength(3);
+    });
+    it('returns one update if grouping and separateMajorMinor=false', async () => {
+      config.groupName = 'somegroup';
+      config.currentValue = '0.4.0';
+      config.rangeStrategy = 'pin';
+      config.separateMajorMinor = false;
+      config.depName = 'q';
+      config.purl = 'pkg:npm/q';
+      nock('https://registry.npmjs.org')
+        .get('/q')
+        .reply(200, qJson);
+      const res = await lookup.lookupUpdates(config);
+      expect(res.updates).toMatchSnapshot();
+      expect(res.updates).toHaveLength(1);
     });
     it('returns only one update if automerging', async () => {
       config.automerge = true;
diff --git a/test/workers/repository/updates/__snapshots__/flatten.spec.js.snap b/test/workers/repository/updates/__snapshots__/flatten.spec.js.snap
index 9cc0140d6a..608b54cd52 100644
--- a/test/workers/repository/updates/__snapshots__/flatten.spec.js.snap
+++ b/test/workers/repository/updates/__snapshots__/flatten.spec.js.snap
@@ -56,6 +56,8 @@ Array [
     "semanticCommitScope": "deps",
     "semanticCommitType": "chore",
     "semanticCommits": null,
+    "separateMajorMinor": true,
+    "separateMinorPatch": false,
     "skipInstalls": true,
     "statusCheckVerify": false,
     "timezone": null,
@@ -124,6 +126,8 @@ Array [
     "semanticCommitScope": "deps",
     "semanticCommitType": "chore",
     "semanticCommits": null,
+    "separateMajorMinor": true,
+    "separateMinorPatch": false,
     "skipInstalls": true,
     "statusCheckVerify": false,
     "timezone": null,
@@ -205,6 +209,8 @@ Array [
     "semanticCommitScope": "deps",
     "semanticCommitType": "chore",
     "semanticCommits": null,
+    "separateMajorMinor": true,
+    "separateMinorPatch": false,
     "skipInstalls": true,
     "statusCheckVerify": false,
     "timezone": null,
@@ -275,6 +281,8 @@ Array [
     "semanticCommitScope": "deps",
     "semanticCommitType": "chore",
     "semanticCommits": null,
+    "separateMajorMinor": true,
+    "separateMinorPatch": false,
     "skipInstalls": true,
     "statusCheckVerify": false,
     "timezone": null,
@@ -344,6 +352,8 @@ Array [
     "semanticCommitScope": "deps",
     "semanticCommitType": "chore",
     "semanticCommits": null,
+    "separateMajorMinor": true,
+    "separateMinorPatch": false,
     "skipInstalls": true,
     "statusCheckVerify": false,
     "timezone": null,
diff --git a/test/workers/repository/updates/branchify.spec.js b/test/workers/repository/updates/branchify.spec.js
index 0c6bcf9fe1..0f0cc45c8c 100644
--- a/test/workers/repository/updates/branchify.spec.js
+++ b/test/workers/repository/updates/branchify.spec.js
@@ -38,6 +38,65 @@ describe('workers/repository/updates/branchify', () => {
       expect(res.branches[0].isMinor).toBe(true);
       expect(res.branches[0].upgrades[0].isMinor).toBe(true);
     });
+    it('uses major/minor/patch slugs', async () => {
+      flattenUpdates.mockReturnValueOnce([
+        {
+          depName: 'foo',
+          branchName: 'foo-{{version}}',
+          version: '2.0.0',
+          prTitle: 'some-title',
+          updateType: 'major',
+          groupName: 'some packages',
+          group: {},
+          separateMajorMinor: true,
+        },
+        {
+          depName: 'foo',
+          branchName: 'foo-{{version}}',
+          version: '1.1.0',
+          prTitle: 'some-title',
+          updateType: 'minor',
+          groupName: 'some packages',
+          group: {},
+          separateMajorMinor: true,
+          separateMinorPatch: true,
+        },
+        {
+          depName: 'foo',
+          branchName: 'foo-{{version}}',
+          version: '1.0.1',
+          prTitle: 'some-title',
+          updateType: 'patch',
+          groupName: 'some packages',
+          group: {},
+          separateMajorMinor: true,
+          separateMinorPatch: true,
+        },
+        {
+          depName: 'bar',
+          branchName: 'bar-{{version}}',
+          version: '2.0.0',
+          prTitle: 'some-title',
+          updateType: 'major',
+          groupName: 'other packages',
+          group: {},
+          separateMultipleMajor: true,
+          separateMajorMinor: true,
+          newMajor: 2,
+        },
+      ]);
+      config.repoIsOnboarded = true;
+      const res = await branchifyUpgrades(config);
+      expect(Object.keys(res.branches).length).toBe(4);
+      expect(res.branches[0].isMajor).toBe(true);
+      expect(res.branches[0].groupSlug).toBe(`major-some-packages`);
+      expect(res.branches[1].isMinor).toBe(true);
+      expect(res.branches[1].groupSlug).toBe(`some-packages`);
+      expect(res.branches[2].isPatch).toBe(true);
+      expect(res.branches[2].groupSlug).toBe(`patch-some-packages`);
+      expect(res.branches[3].isMajor).toBe(true);
+      expect(res.branches[3].groupSlug).toBe(`major-2-other-packages`);
+    });
     it('does not group if different compiled branch names', async () => {
       flattenUpdates.mockReturnValueOnce([
         {
-- 
GitLab