fix: whitelist/blacklist algorithm

maxkomarychev · Nov 10, 2019 · 4f1ebfa · 4f1ebfa
1 parent a6c0a8d
commit 4f1ebfa
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 23 deletions.
diff --git a/README.md b/README.md
@@ -45,7 +45,7 @@ with the content:
           - test1-that
         steps:
           - uses: actions/checkout@v1
-          - uses: maxkomarychev/merge-pal-action@v0.4.0
+          - uses: maxkomarychev/merge-pal-action@v0.4.1
             with:
               token: ${{ secrets.GITHUB_TOKEN }}
 
@@ -78,7 +78,7 @@ separate workflow: `.github/workflows/merge-pal-other.yml`
         runs-on: ubuntu-latest
         steps:
           - uses: actions/checkout@v1
-          - uses: maxkomarychev/merge-pal-action@v0.4.0
+          - uses: maxkomarychev/merge-pal-action@v0.4.1
             with:
               token: ${{ secrets.GITHUB_TOKEN }}
 

diff --git a/bin/index.js b/bin/index.js
@@ -10163,19 +10163,15 @@ function canMergeByLabels(pr, whitelist, blacklist) {
         return true;
     }
     const labels = pr.labels.map((label) => label.name);
-    if (blacklist.length) {
-        const matchedBlack = labels.filter((label) => blacklist.includes(label));
-        if (matchedBlack.length > 0) {
-            return false;
-        }
-        else {
-            return true;
-        }
+    const matchedBlack = labels.filter((label) => blacklist.includes(label));
+    const matchedWhite = labels.filter((label) => whitelist.includes(label));
+    if (blacklist.length > 0 && matchedBlack.length > 0) {
+        return false;
     }
-    if (whitelist.length) {
-        const matchedWhite = labels.filter((label) => whitelist.includes(label));
-        return matchedWhite.length > 0;
+    if (whitelist.length > 0 && matchedWhite.length === 0) {
+        return false;
     }
+    return true;
 }
 exports.canMergeByLabels = canMergeByLabels;
 function canMerge(pr, whitelist, blacklist) {

diff --git a/src/__tests__/canMerge.ts b/src/__tests__/canMerge.ts
@@ -77,6 +77,13 @@ describe('canMergeByLabels', () => {
             ),
         ).toEqual(false)
     })
+    it('disallows merge when whitelist and blacklists exist but labels are empty', () => {
+        const whitelist = ['white']
+        const blacklist = ['black']
+        expect(
+            canMergeByLabels(createPR(false, '', []), whitelist, blacklist),
+        ).toEqual(false)
+    })
     it('disallows merge even if one label matches blacklist', () => {
         const whitelist = []
         const blacklist = ['black']

diff --git a/src/canMerge.ts b/src/canMerge.ts
@@ -15,18 +15,15 @@ export function canMergeByLabels(
         return true
     }
     const labels = pr.labels.map((label) => label.name)
-    if (blacklist.length) {
-        const matchedBlack = labels.filter((label) => blacklist.includes(label))
-        if (matchedBlack.length > 0) {
-            return false
-        } else {
-            return true
-        }
+    const matchedBlack = labels.filter((label) => blacklist.includes(label))
+    const matchedWhite = labels.filter((label) => whitelist.includes(label))
+    if (blacklist.length > 0 && matchedBlack.length > 0) {
+        return false
     }
-    if (whitelist.length) {
-        const matchedWhite = labels.filter((label) => whitelist.includes(label))
-        return matchedWhite.length > 0
+    if (whitelist.length > 0 && matchedWhite.length === 0) {
+        return false
     }
+    return true
 }
 
 export default function canMerge(