[IMP] runbot_merge, forwardport: variable-user feedback

Having all the feedback be sent by the mergebot user (github_token) is
confusing. Add a way to specify which field of project should be used to
source the token used when sending feedback.

Fixes #190

forwardport/data/crons.xml

@@ -41,6 +41,7 @@ for pr in env['runbot_merge.pull_requests'].search([
         'repository': pr.repository.id,
         'pull_request': pr.number,
         'message': "This pull request has forward-port PRs awaiting action",
+        'token_field': 'fp_github_token',
     })
         </field>
         <field name="interval_number">1</field>

forwardport/models/project.py

@@ -218,6 +218,7 @@ class PullRequests(models.Model):
                     'repository': self.repository.id,
                     'pull_request': self.number,
                     'message': msg,
+                    'token_field': 'fp_github_token',
                 })
 
     def _validate(self, statuses):
@@ -235,6 +236,7 @@ class PullRequests(models.Model):
                     self.env['runbot_merge.pull_requests.feedback'].create({
                         'repository': pr.repository.id,
                         'pull_request': pr.number,
+                        'token_field': 'fp_github_token',
                         'message': pr.source_id._pingline() + '\n\nCI failed on this forward-port PR'
                     })
                 continue
@@ -501,6 +503,7 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
                 'repository': new_pr.repository.id,
                 'pull_request': new_pr.number,
                 'message': message,
+                'token_field': 'fp_github_token',
             })
             # not great but we probably want to avoid the risk of the webhook
             # creating the PR from under us. There's still a "hole" between
@@ -717,6 +720,10 @@ class Stagings(models.Model):
                     })
         return r
 
+class Feedback(models.Model):
+    _inherit = 'runbot_merge.pull_requests.feedback'
+
+    token_field = fields.Selection(selection_add=[('fp_github_token', 'Forwardport Bot')])
 
 def git(directory): return Repo(directory, check=True)
 class Repo:

forwardport/tests/test_simple.py

@@ -626,6 +626,10 @@ def test_empty(env, config, make_repo, users):
 
     # should not have created any new PR
     assert env['runbot_merge.pull_requests'].search([], order='number') == prs
+    # change FP token to see if the feedback comes from the proper user
+    project = env['runbot_merge.project'].search([])
+    project.fp_github_token = config['role_other']['token']
+    assert project.fp_github_name == users['other']
 
     # check reminder
     env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron', context={'forwardport_updated_before': FAKE_PREV_WEEK})
@@ -634,8 +638,8 @@ def test_empty(env, config, make_repo, users):
     assert pr1.comments == [
         (users['reviewer'], 'hansen r+'),
         (users['user'], re_matches(r'Merged at [0-9a-f]{40}, thanks!')),
-        (users['user'], 'This pull request has forward-port PRs awaiting action'),
-        (users['user'], 'This pull request has forward-port PRs awaiting action'),
+        (users['other'], 'This pull request has forward-port PRs awaiting action'),
+        (users['other'], 'This pull request has forward-port PRs awaiting action'),
     ], "each cron run should trigger a new message on the ancestor"
     # check that this stops if we close the PR
     with prod:
@@ -644,8 +648,8 @@ def test_empty(env, config, make_repo, users):
     assert pr1.comments == [
         (users['reviewer'], 'hansen r+'),
         (users['user'], re_matches(r'Merged at [0-9a-f]{40}, thanks!')),
-        (users['user'], 'This pull request has forward-port PRs awaiting action'),
-        (users['user'], 'This pull request has forward-port PRs awaiting action'),
+        (users['other'], 'This pull request has forward-port PRs awaiting action'),
+        (users['other'], 'This pull request has forward-port PRs awaiting action'),
     ]
 
 def test_partially_empty(env, config, make_repo):
@@ -770,7 +774,8 @@ def test_limit_disable(env, config, make_repo, users, enabled):
     !fp_target (won't be forward-ported to).
     """
     prod, other = make_basic(env, config, make_repo)
-    bot_name = env['runbot_merge.project'].search([]).fp_github_name
+    project = env['runbot_merge.project'].search([])
+    bot_name = project.fp_github_name
     with prod:
         [c] = prod.make_commits('a', Commit('c 0', tree={'0': '0'}), ref='heads/branch0')
         pr = prod.make_pr(target='a', head='branch0')
@@ -797,6 +802,8 @@ def test_limit_disable(env, config, make_repo, users, enabled):
     assert p.parent_id == _1
     assert p.target.name == 'c'
 
+    project.fp_github_token = config['role_other']['token']
+    bot_name = project.fp_github_name
     with prod:
         [c] = prod.make_commits('a', Commit('c 2', tree={'2': '2'}), ref='heads/branch2')
         pr = prod.make_pr(target='a', head='branch2')
@@ -815,10 +822,10 @@ def test_limit_disable(env, config, make_repo, users, enabled):
         (users['reviewer'], "%s up to b" % bot_name),
         (users['reviewer'], "%s up to foo" % bot_name),
         (users['reviewer'], "%s up to c" % bot_name),
-        (users['user'], "Please provide a branch to forward-port to."),
-        (users['user'], "Branch 'b' is disabled, it can't be used as a forward port target."),
-        (users['user'], "There is no branch 'foo', it can't be used as a forward port target."),
-        (users['user'], "Forward-porting to 'c'."),
+        (users['other'], "Please provide a branch to forward-port to."),
+        (users['other'], "Branch 'b' is disabled, it can't be used as a forward port target."),
+        (users['other'], "There is no branch 'foo', it can't be used as a forward port target."),
+        (users['other'], "Forward-porting to 'c'."),
     }
 
 def test_default_disabled(env, config, make_repo, users):

runbot_merge/models/pull_requests.py

@@ -122,9 +122,9 @@ class Project(models.Model):
         to_remove = []
         for f in self.env['runbot_merge.pull_requests.feedback'].search([]):
             repo = f.repository
-            gh = ghs.get(repo)
+            gh = ghs.get((repo, f.token_field))
             if not gh:
-                gh = ghs[repo] = repo.github()
+                gh = ghs[(repo, f.token_field)] = repo.github(f.token_field)
 
             try:
                 if f.close:
@@ -186,8 +186,8 @@ class Repository(models.Model):
     name = fields.Char(required=True)
     project_id = fields.Many2one('runbot_merge.project', required=True)
 
-    def github(self):
-        return github.GH(self.project_id.github_token, self.name)
+    def github(self, token_field='github_token'):
+        return github.GH(self.project_id[token_field], self.name)
 
     def _auto_init(self):
         res = super(Repository, self)._auto_init()
@@ -1228,6 +1228,12 @@ class Feedback(models.Model):
     pull_request = fields.Integer()
     message = fields.Char()
     close = fields.Boolean()
+    token_field = fields.Selection(
+        [('github_token', "Mergebot")],
+        default='github_token',
+        string="Bot User",
+        help="Token field (from repo's project) to use to post messages"
+    )
 
 class Commit(models.Model):
     """Represents a commit onto which statuses might be posted,