Disallow plain 'merge' command when deployments are enabled

src/Configuration.hs

@@ -43,6 +43,7 @@ data ProjectConfiguration = ProjectConfiguration
   , deployEnvironments :: Maybe [Text] -- The environments which the `deploy to <environment>` command should be enabled for
   , deploySubprojects :: Maybe [Text] -- The subprojects which the `deploy` command should be enabled for
   , safeForFriday :: Maybe Bool -- Whether it's safe to deploy this project on Friday without an "on Friday" check. default False
+  , allowPlainMerge :: Maybe Bool -- Whether to allow plain merges without explicitly saying the PR shouldn't be deployed. default True
   }
   deriving (Generic)
 

src/Logic.hs

@@ -719,6 +719,8 @@ handleCommentAdded triggerConfig mergeWindowExemption featureFreezeWindow prId a
                   then isReviewer author
                   else pure False
 
+              let plainMergeAllowed = fromMaybe True (Config.allowPlainMerge projectConfig)
+
               dateTime <- getDateTime
 
               -- To guard against accidental merges we make use of a merge window.
@@ -739,6 +741,22 @@ handleCommentAdded triggerConfig mergeWindowExemption featureFreezeWindow prId a
                   let (Config.MergeWindowExemptionConfiguration users) = mergeWindowExemption
                   in  elem user users
 
+                verifyMergeType :: MergeCommand -> Eff es ProjectState -> Eff es ProjectState
+                verifyMergeType (Approve Merge) action =
+                  if not plainMergeAllowed
+                    then do
+                      () <-
+                        leaveComment
+                          prId
+                          ( "Your merge request has been denied because \
+                            \this project can be automatically deployed. Use '"
+                              <> Pr.displayMergeCommand (Approve MergeWithoutDeploy)
+                              <> "' if you really don't want to deploy after merging."
+                          )
+                      pure state
+                    else action
+                verifyMergeType _ action = action
+
                 verifyMergeWindow :: MergeCommand -> MergeWindow -> Eff es ProjectState -> Eff es ProjectState
                 verifyMergeWindow _ _ action | exempted author = action
                 verifyMergeWindow command DuringFeatureFreeze action
@@ -816,7 +834,7 @@ handleCommentAdded triggerConfig mergeWindowExemption featureFreezeWindow prId a
                 -- Cases where the parse was successful
                 Success (command, mergeWindow, priority)
                   -- Author is a reviewer
-                  | isAllowed -> verifyMergeWindow command mergeWindow $ case command of
+                  | isAllowed -> verifyMergeType command $ verifyMergeWindow command mergeWindow $ case command of
                       Approve approval -> handleMergeRequested projectConfig prId author source state pr approval priority Nothing
                       Retry -> handleMergeRetry projectConfig prId author source priority state pr
                   -- Author is not a reviewer, so we ignore

src/Parser.hs

@@ -239,17 +239,20 @@ parseMergeCommand projectConfig triggerConfig = cvtParseResult . P.parse pCommen
   -- messages megaparsec gives us, and the parser will instead error out in
   -- 'pCommandSuffix' which would be confusing.
   --
-  -- When the comment isn't folowed by @ and @ this is treated as a plain
+  -- When the comment isn't followed by @ and @ this is treated as a plain
   -- merge command.
   pMergeApproval :: Parser ApprovedFor
-  pMergeApproval = P.string' "merge" *> P.option Merge pMergeAnd
+  pMergeApproval = P.string' "merge" *> P.option Merge (pMergeWithoutDeploy <|> pMergeAnd)
 
   -- NOTE: As mentioned above, only the @ and @ part will backtrack. This is
   --       needed so a) the custom error message in pDeploy works and b) so
   --       'merge on friday' can be parsed correctly.
   pMergeAnd :: Parser ApprovedFor
   pMergeAnd = P.try (pSpace1 *> P.string' "and" *> pSpace1) *> (pTag <|> pDeploy)
 
+  pMergeWithoutDeploy :: Parser ApprovedFor
+  pMergeWithoutDeploy = P.try (pSpace1 *> P.string' "without" *> (MergeWithoutDeploy <$ (pSpace1 *> (P.string' "deploying" <|> P.string' "deploy"))))
+
   -- Parses @merge and tag@ commands.
   pTag :: Parser ApprovedFor
   pTag = MergeAndTag <$ P.string' "tag"

src/Project.hs

@@ -200,6 +200,7 @@ data DeploySubprojects
 -- This enumeration distinguishes these cases.
 data ApprovedFor
   = Merge
+  | MergeWithoutDeploy
   | MergeAndDeploy DeploySubprojects DeployEnvironment
   | MergeAndTag
   deriving (Eq, Show, Generic)
@@ -595,6 +596,7 @@ candidatePullRequests state =
 -- friday@ merge window suffix.
 displayMergeCommand :: MergeCommand -> Text
 displayMergeCommand (Approve Merge) = "merge"
+displayMergeCommand (Approve MergeWithoutDeploy) = "merge without deploying"
 displayMergeCommand (Approve (MergeAndDeploy subprojects (DeployEnvironment env))) =
   case subprojects of
     EntireProject -> format "merge and deploy to {}" [env]
@@ -612,11 +614,13 @@ alwaysAddMergeCommit = needsTag
 
 needsDeploy :: ApprovedFor -> Bool
 needsDeploy Merge = False
+needsDeploy MergeWithoutDeploy = False
 needsDeploy MergeAndDeploy{} = True
 needsDeploy MergeAndTag = False
 
 needsTag :: ApprovedFor -> Bool
 needsTag Merge = False
+needsTag MergeWithoutDeploy = False
 needsTag MergeAndDeploy{} = True
 needsTag MergeAndTag = True
 

tests/EventLoopSpec.hs

@@ -208,6 +208,7 @@ buildProjectConfig repoDir stateFile =
     , Config.deployEnvironments = Just ["staging", "production"]
     , Config.deploySubprojects = Nothing
     , Config.safeForFriday = Nothing
+    , Config.allowPlainMerge = Just True
     }
 
 -- Dummy user configuration used in test environment.

tests/ParserSpec.hs

@@ -228,6 +228,7 @@ dummyProject =
     , deployEnvironments = Just ["production", "staging"]
     , deploySubprojects = Just ["aaa", "bbb"]
     , safeForFriday = Just True
+    , allowPlainMerge = Just True
     }
 
 dummyTrigger :: TriggerConfiguration

tests/Spec.hs

@@ -98,6 +98,7 @@ testProjectConfig =
     , Config.deployEnvironments = Just ["staging", "production"]
     , Config.deploySubprojects = Just ["aaa", "bbb"]
     , Config.safeForFriday = Nothing
+    , Config.allowPlainMerge = Just True
     }
 
 testmergeWindowExemptionConfig :: Config.MergeWindowExemptionConfiguration
@@ -6561,3 +6562,96 @@ main = hspec $ do
                       }
                    , ALeaveComment (PullRequestId 1) "<!-- Hoff: ignore -->\nRebased as 1b3, waiting for CI …"
                    ]
+
+    it "disallows plain merge on a repo configured with ProjectConfiguration.allowPlainMerge=False" $ do
+      let
+        prId = PullRequestId 1
+        state = singlePullRequestState prId (Branch "p") masterBranch (Sha "abc1234") "tyrell"
+
+        event = CommentAdded prId "deckard" Nothing "@bot merge"
+
+        results = defaultResults{resultIntegrate = [Right (Sha "def2345")]}
+        (_, actions) = runActionCustom' results (testProjectConfig{Config.allowPlainMerge = Just False}) $ handleEventTest event state
+
+      actions
+        `shouldBe` [ AIsReviewer "deckard"
+                   , ALeaveComment prId "Your merge request has been denied because this project can be automatically deployed. Use 'merge without deploying' if you really don't want to deploy after merging."
+                   ]
+
+    it "allows merge without deploying on a repo configured with ProjectConfiguration.allowPlainMerge=False" $ do
+      let
+        prId = PullRequestId 1
+        state = singlePullRequestState prId (Branch "p") masterBranch (Sha "abc1234") "tyrell"
+
+        event = CommentAdded prId "deckard" Nothing "@bot merge without deploying"
+
+        results = defaultResults{resultIntegrate = [Right (Sha "def2345")]}
+        (_, actions) = runActionCustom' results (testProjectConfig{Config.allowPlainMerge = Just False}) $ handleEventTest event state
+
+      actions
+        `shouldBe` [ AIsReviewer "deckard"
+                   , ALeaveComment
+                      prId
+                      "<!-- Hoff: ignore -->\nPull request approved for merge without deploying by @deckard, rebasing now."
+                   , ATryIntegrate
+                      "Merge #1: Untitled\n\n\
+                      \Approved-by: deckard\n\
+                      \Priority: Normal\n\
+                      \Auto-deploy: false\n"
+                      (prId, Branch "refs/pull/1/head", Sha "abc1234")
+                      []
+                      False
+                   , ALeaveComment prId "<!-- Hoff: ignore -->\nRebased as def2345, waiting for CI …"
+                   ]
+
+    it "allows merge without deploying on a repo configured with ProjectConfiguration.allowPlainMerge=True" $ do
+      let
+        prId = PullRequestId 1
+        state = singlePullRequestState prId (Branch "p") masterBranch (Sha "abc1234") "tyrell"
+
+        event = CommentAdded prId "deckard" Nothing "@bot merge without deploying"
+
+        results = defaultResults{resultIntegrate = [Right (Sha "def2345")]}
+        (_, actions) = runActionCustom results $ handleEventTest event state
+
+      actions
+        `shouldBe` [ AIsReviewer "deckard"
+                   , ALeaveComment
+                      prId
+                      "<!-- Hoff: ignore -->\nPull request approved for merge without deploying by @deckard, rebasing now."
+                   , ATryIntegrate
+                      "Merge #1: Untitled\n\n\
+                      \Approved-by: deckard\n\
+                      \Priority: Normal\n\
+                      \Auto-deploy: false\n"
+                      (prId, Branch "refs/pull/1/head", Sha "abc1234")
+                      []
+                      False
+                   , ALeaveComment prId "<!-- Hoff: ignore -->\nRebased as def2345, waiting for CI …"
+                   ]
+
+    it "allows the 'merge without deploy' alias" $ do
+      let
+        prId = PullRequestId 1
+        state = singlePullRequestState prId (Branch "p") masterBranch (Sha "abc1234") "tyrell"
+
+        event = CommentAdded prId "deckard" Nothing "@bot merge without deploy"
+
+        results = defaultResults{resultIntegrate = [Right (Sha "def2345")]}
+        (_, actions) = runActionCustom results $ handleEventTest event state
+
+      actions
+        `shouldBe` [ AIsReviewer "deckard"
+                   , ALeaveComment
+                      prId
+                      "<!-- Hoff: ignore -->\nPull request approved for merge without deploying by @deckard, rebasing now."
+                   , ATryIntegrate
+                      "Merge #1: Untitled\n\n\
+                      \Approved-by: deckard\n\
+                      \Priority: Normal\n\
+                      \Auto-deploy: false\n"
+                      (prId, Branch "refs/pull/1/head", Sha "abc1234")
+                      []
+                      False
+                   , ALeaveComment prId "<!-- Hoff: ignore -->\nRebased as def2345, waiting for CI …"
+                   ]