From 6ae51f9da8f0fcf5e3dfb5fc4f1960d1f0b6bd1e Mon Sep 17 00:00:00 2001
From: Mark Browning <mabrowningrr@gmail.com>
Date: Thu, 17 Oct 2019 17:08:07 -0500
Subject: [PATCH] Fix #499 by prioritizing trigger phrase over retest phrase

---
 .../plugins/ghprb/GhprbPullRequest.java       | 22 +++++++++++--------
 .../plugins/ghprb/GhprbRepositoryTest.java    |  1 +
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java
index a21c64d74..cc5b2f1b2 100644
--- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java
+++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java
@@ -607,16 +607,11 @@ private void checkComment(GHIssueComment comment) throws IOException {
         } else if (helper.isOktotestPhrase(body) && helper.isAdmin(sender)) { // ok to test
             LOGGER.log(Level.FINEST, "Admin {0} gave OK to test", sender);
             setAccepted(true);
-        } else if (helper.isRetestPhrase(body)) { // test this please
-            LOGGER.log(Level.FINEST, "Retest phrase");
-            if (helper.isAdmin(sender)) {
-                LOGGER.log(Level.FINEST, "Admin {0} gave retest phrase", sender);
-                shouldRun = true;
-            } else if (accepted && helper.isWhitelisted(sender)) {
-                LOGGER.log(Level.FINEST, "Retest accepted and user {0} is whitelisted", sender);
-                shouldRun = true;
-            }
         } else if (helper.isTriggerPhrase(body)) { // trigger phrase
+            // The trigger phrase is prioritized over retest phrase because it
+            // encomposes the logic of retestPhrase (shouldRun = true), but also
+            // sets the triggered flag, which is important when the job is
+            // configured to "Only use trigger phrase for build triggering"
             LOGGER.log(Level.FINEST, "Trigger phrase");
             if (helper.isAdmin(sender)) {
                 LOGGER.log(Level.FINEST, "Admin {0} ran trigger phrase", sender);
@@ -627,6 +622,15 @@ private void checkComment(GHIssueComment comment) throws IOException {
                 shouldRun = true;
                 triggered = true;
             }
+        } else if (helper.isRetestPhrase(body)) { // test this please
+            LOGGER.log(Level.FINEST, "Retest phrase");
+            if (helper.isAdmin(sender)) {
+                LOGGER.log(Level.FINEST, "Admin {0} gave retest phrase", sender);
+                shouldRun = true;
+            } else if (accepted && helper.isWhitelisted(sender)) {
+                LOGGER.log(Level.FINEST, "Retest accepted and user {0} is whitelisted", sender);
+                shouldRun = true;
+            }
         }
 
         if (shouldRun) {
diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java
index 32653c7c3..fcf106ae5 100644
--- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java
+++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java
@@ -694,6 +694,7 @@ public void testCheckMethodWhenPrWasUpdatedWithRetestPhrase() throws Exception {
 
         verify(helper).isWhitelistPhrase(eq("test this please"));
         verify(helper).isOktotestPhrase(eq("test this please"));
+        verify(helper).isTriggerPhrase(eq("test this please"));
         verify(helper).isRetestPhrase(eq("test this please"));
         verify(helper).isAdmin(eq(ghUser));
         verify(helper, times(4)).isProjectDisabled();