From 36026f46e4e312881028436bedfc4d9cc98bb7d2 Mon Sep 17 00:00:00 2001
From: Xavier Morel <xmo@odoo.com>
Date: Wed, 22 Jul 2020 11:56:33 +0200
Subject: [PATCH] [FIX] runbot_merge: don't warn on unrecognized commands

This is a regression due to the implementation details of
odoo/runbot#376: previously _parse_command would only yield the
commands it had specifically recognised (from a whitelist).

22e18e752b948a0ef2160d584b24fcd77a7a3e3a simplified the implementation
and (for convenience when adding new commands) now passes through any
command to the executor instead of skipping the unknown one.

But I forgot to update the executor to ignore unknown commands, so it
treats them as *failed* (since the success flag doesn't get set) and
assumes it's an ACL issue, so notifies the user that they can't do the
thing they never really asked for.

Add an end-case which skips the feedback bit for unrecognized
commands, which restores the old behavior.

Fixes #390
---
 runbot_merge/models/pull_requests.py |  5 ++++-
 runbot_merge/tests/test_basic.py     | 16 ++++++++++++++++
 runbot_merge/tests/test_oddities.py  |  2 +-
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py
index 3c9298c6..8d3524d7 100644
--- a/runbot_merge/models/pull_requests.py
+++ b/runbot_merge/models/pull_requests.py
@@ -950,7 +950,10 @@ class PullRequests(models.Model):
                         c.create({'sha': self.head, 'statuses': '{}'})
                     ok = True
                 else:
-                    msg = "You are not allowed to do that."
+                    msg = f"You are not allowed to override this status."
+            else:
+                # ignore unknown commands
+                continue
 
             _logger.info(
                 "%s %s(%s) on %s by %s (%s)",
diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py
index cacab7a9..c179a4ab 100644
--- a/runbot_merge/tests/test_basic.py
+++ b/runbot_merge/tests/test_basic.py
@@ -2857,6 +2857,22 @@ class TestRecognizeCommands:
             prx.post_comment('%shansen r+' % indent, config['role_reviewer']['token'])
         assert pr.state == 'approved'
 
+    def test_unknown_commands(self, repo, env, config, users):
+        with repo:
+            m = repo.make_commit(None, 'initial', None, tree={'m': 'm'})
+            repo.make_ref('heads/master', m)
+
+            c = repo.make_commit(m, 'first', None, tree={'m': 'c'})
+            pr = repo.make_pr(title='title', body=None, target='master', head=c)
+            pr.post_comment("hansen do the thing", config['role_reviewer']['token'])
+            pr.post_comment('hansen @bobby-b r+ :+1:', config['role_reviewer']['token'])
+        env.run_crons()
+
+        assert pr.comments == [
+            (users['reviewer'], "hansen do the thing"),
+            (users['reviewer'], "hansen @bobby-b r+ :+1:"),
+        ]
+
 class TestRMinus:
     def test_rminus_approved(self, repo, env, config):
         """ approved -> r- -> opened
diff --git a/runbot_merge/tests/test_oddities.py b/runbot_merge/tests/test_oddities.py
index fdce2444..9f94225c 100644
--- a/runbot_merge/tests/test_oddities.py
+++ b/runbot_merge/tests/test_oddities.py
@@ -79,7 +79,7 @@ def test_override(env, project, make_repo, users, setreviewers, config):
     assert comments == [
         (users['reviewer'], 'hansen r+'),
         (users['reviewer'], 'hansen override=l/int'),
-        (users['user'], "I'm sorry, @{}. You are not allowed to do that.".format(users['reviewer'])),
+        (users['user'], "I'm sorry, @{}. You are not allowed to override this status.".format(users['reviewer'])),
         (users['other'], "hansen override=l/int"),
     ]
     assert pr_id.statuses == '{}'