From 5c95d319fa4bf338abc6e68c828f0cd17c4f03c0 Mon Sep 17 00:00:00 2001
From: Thomas Neidhart <thomas.neidhart@gmail.com>
Date: Wed, 22 Nov 2023 08:39:17 +0100
Subject: [PATCH] Cleanup and adding FIXME comment.

---
 lib/group-pull-requests.js | 59 ++++++++++++++++++++------------------
 lib/template.js            |  4 +++
 test/index.test.js         |  4 +--
 3 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/lib/group-pull-requests.js b/lib/group-pull-requests.js
index f1d68b18c..0e3ab94c2 100644
--- a/lib/group-pull-requests.js
+++ b/lib/group-pull-requests.js
@@ -5,46 +5,49 @@ const groupPullRequests = (pullRequests) => {
     const dependabotPRs = {}
     const otherPRs = []
 
-    const dependabot = new RegExp("Bump ([^\S]+) from ([^\S]+) to ([^\S]+)", "gm");
+    const bumpVersionRegExp = new RegExp("Bump ([^\S]+) from ([^\S]+) to ([^\S]+)", "g");
+
+    // group all matching dependabot PRs
     for (let i in pullRequests) {
-        const currentPullRequest = pullRequests[i]
-
-        if (currentPullRequest.title.match(dependabot)) {
-            const matches = [...pullRequests[i].title.matchAll(dependabot)]
-            if (dependabotPRs[matches[0][1]]) {
-                const curr = dependabotPRs[matches[0][1]]
-                const from = (compareVersions(matches[0][2], curr.from) < 0) ? matches[0][2] : curr.from
-                const to = (compareVersions(matches[0][3], curr.to) > 0) ? matches[0][3] : curr.to
-                curr.number.push(currentPullRequest.number)
-                dependabotPRs[matches[0][1]] = {
-                    from: from,
-                    to: to,
-                    number: curr.number,
-                    pr: currentPullRequest
+        const currentPR = pullRequests[i]
+
+        if (currentPR.title.match(bumpVersionRegExp)) {
+            const match = [...pullRequests[i].title.matchAll(bumpVersionRegExp)][0]
+
+            const artifact = match[1]
+            const fromVersion = match[2]
+            const toVersion = match[3]
+
+            if (dependabotPRs[artifact]) {
+                const prevDependabotPR = dependabotPRs[artifact]
+                dependabotPRs[artifact] = {
+                    from: (compareVersions(fromVersion, prevDependabotPR.from) < 0) ? fromVersion : prevDependabotPR.from,
+                    to: (compareVersions(toVersion, prevDependabotPR.to) > 0) ? toVersion : prevDependabotPR.to,
+                    pullRequests: [...prevDependabotPR.pullRequests, currentPR.number],
+                    // use the latest PR for creating the final changelog entry
+                    pr: (compareVersions(toVersion, prevDependabotPR.to) > 0) ? currentPR : prevDependabotPR.pr
                 }
             } else {
-                dependabotPRs[matches[0][1]] = {
-                    from: matches[0][2],
-                    to: matches[0][3],
-                    number: [currentPullRequest.number],
-                    pr: currentPullRequest
+                dependabotPRs[artifact] = {
+                    from: fromVersion,
+                    to: toVersion,
+                    pullRequests: [currentPR.number],
+                    pr: currentPR
                 }
             }
         } else {
-            otherPRs.push(currentPullRequest)
+            otherPRs.push(currentPR)
         }
     }
 
+    // reconstruct all PRs
     const result = [...otherPRs]
-
     for (const [key, value] of Object.entries(dependabotPRs)) {
-        const item = value.pr
-        item.title = `Bump ${key} from ${value.from} to ${value.to}`
-        item.number = value.number
-        result.push(item)
+        const pr = value.pr
+        pr.title = `Bump ${key} from ${value.from} to ${value.to}`
+        pr.number = value.pullRequests
+        result.push(pr)
     }
-    console.log(result)
-
     return result
 }
 
diff --git a/lib/template.js b/lib/template.js
index 29c74543a..5f359af02 100644
--- a/lib/template.js
+++ b/lib/template.js
@@ -13,6 +13,10 @@ const template = (string, object, customReplacers) => {
     if (object[k] === undefined || object[k] === null) {
       result = k
     } else if (Array.isArray(object[k])) {
+      // FIXME: this is basically a hack to support changelog entries to refer to multiple PRs
+      //        we assume that a list of values refers to PR numbers. If somebody has a better
+      //        idea how to handle that in a clean way, i.e. to support the change template for
+      //        each individual PR number.
       result = object[k].map(item => `${item}`).join(',#')
     } else if (typeof object[k] === 'object') {
       result = template(object[k].template, object[k])
diff --git a/test/index.test.js b/test/index.test.js
index 5e66eb63f..b81b29794 100644
--- a/test/index.test.js
+++ b/test/index.test.js
@@ -3406,11 +3406,11 @@ describe('release-drafter', () => {
                 Object {
                   "body": "# What's Changed
 
+                * Bump org.owasp:dependency-check-maven from 8.4.0 to 8.4.3 (#272,#259) @dependabot
+                * Bump com.fasterxml.jackson.core:jackson-databind from 2.13.1 to 2.16.0 (#273,#261,#256) @dependabot
                 * Bump the quarkus group with 1 update (#271) @dependabot
                 * Bump org.cyclonedx:cyclonedx-maven-plugin from 2.7.9 to 2.7.10 (#258) @dependabot
-                * Bump org.owasp:dependency-check-maven from 8.4.0 to 8.4.3 (#272,#259) @dependabot
                 * Bump surefire-plugin.version from 3.1.2 to 3.2.1 (#260) @dependabot
-                * Bump com.fasterxml.jackson.core:jackson-databind from 2.13.1 to 2.16.0 (#273,#261,#256) @dependabot
                 ",
                   "draft": true,
                   "make_latest": "true",