From e82de3136b1ca02ec125c45c0cbd80bec96ea28c Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Tue, 10 Mar 2020 13:36:46 +0100 Subject: [PATCH] [IMP] runbot_merge, forwardport: unify tagging Genericise runbot_merge's tagging (move states to the "UI" but only store / manage actual tags), and remove forwardport.tagging as it's now redundant. Closes #232 --- forwardport/data/security.xml | 17 ---------- forwardport/models/project.py | 41 ++---------------------- runbot_merge/github.py | 7 ++-- runbot_merge/models/pull_requests.py | 48 +++++++++++++--------------- 4 files changed, 26 insertions(+), 87 deletions(-) diff --git a/forwardport/data/security.xml b/forwardport/data/security.xml index d25ae794..99548e0c 100644 --- a/forwardport/data/security.xml +++ b/forwardport/data/security.xml @@ -17,15 +17,6 @@ 1 1 - - Admin access to tagging - - - 1 - 1 - 1 - 1 - Admin access to branch remover @@ -36,14 +27,6 @@ 1 - - No normal access to tagging - - 1 - 0 - 0 - 0 - No normal access to batches diff --git a/forwardport/models/project.py b/forwardport/models/project.py index 424795e1..bde76204 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -83,30 +83,6 @@ class Project(models.Model): if not project.fp_github_email: raise UserError(_("The forward-port bot needs a primary email set up.")) - def _send_feedback(self): - super()._send_feedback() - ghs = {} - to_remove = [] - for f in self.env['forwardport.tagging'].search([]): - repo = f.repository - gh = ghs.get(repo) - if not gh: - gh = ghs[repo] = repo.github() - - try: - gh('POST', 'issues/{}/labels'.format(f.pull_request), json={ - 'labels': json.loads(f.to_add) - }) - except Exception: - _logger.exception( - "Error while trying to add the tags %s to %s#%s", - f.to_add, repo.name, f.pull_request - ) - else: - to_remove.append(f.id) - if to_remove: - self.env['forwardport.tagging'].browse(to_remove).unlink() - def write(self, vals): Branches = self.env['runbot_merge.branch'] # check on branches both active and inactive so disabling branches doesn't @@ -722,11 +698,10 @@ This PR targets %s and is part of the forward-port chain. Further PRs will be cr labels = ['forwardport'] if has_conflicts: labels.append('conflict') - self.env['forwardport.tagging'].create({ + self.env['runbot_merge.pull_requests.tagging'].create({ 'repository': new_pr.repository.id, 'pull_request': new_pr.number, - 'to_add': json.dumps(labels), - 'token_field': 'fp_github_token', + 'tags_add': labels, }) # batch the PRs so _validate can perform the followup FP properly @@ -1016,18 +991,6 @@ class Feedback(models.Model): token_field = fields.Selection(selection_add=[('fp_github_token', 'Forwardport Bot')]) -class Tagging(models.Model): - _name = 'forwardport.tagging' - _description = "ad-hoc forwardport tagging commands" - - token_field = fields.Selection([ - ('github_token', 'Mergebot'), - ('fp_github_token', 'Forwardport Bot'), - ], required=True) - repository = fields.Many2one('runbot_merge.repository', required=True) - pull_request = fields.Integer(string="PR number") - to_add = fields.Char(string="JSON-encoded array of labels to add") - def git(directory): return Repo(directory, check=True) class Repo: def __init__(self, directory, **config): diff --git a/runbot_merge/github.py b/runbot_merge/github.py index f164a243..ead3ca2c 100644 --- a/runbot_merge/github.py +++ b/runbot_merge/github.py @@ -166,13 +166,10 @@ class GH(object): def close(self, pr): self('PATCH', 'pulls/{}'.format(pr), json={'state': 'closed'}) - def change_tags(self, pr, to_): + def change_tags(self, pr, remove, add): labels_endpoint = 'issues/{}/labels'.format(pr) - from .models.pull_requests import _TAGS - mergebot_tags = set.union(*_TAGS.values()) tags_before = {label['name'] for label in self('GET', labels_endpoint).json()} - # remove all mergebot tags from the PR, then add just the ones which should be set - tags_after = (tags_before - mergebot_tags) | to_ + tags_after = (tags_before - remove) | add # replace labels entirely self('PUT', labels_endpoint, json={'labels': list(tags_after)}) diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index 10509707..e45ee531 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -89,26 +89,27 @@ class Project(models.Model): t.repository as repo_id, t.pull_request as pr_number, array_agg(t.id) as ids, - (array_agg(t.state_from ORDER BY t.id))[1] as state_from, - (array_agg(t.state_to ORDER BY t.id DESC))[1] as state_to + array_agg(t.tags_remove::json) as to_remove, + array_agg(t.tags_add::json) as to_add FROM runbot_merge_pull_requests_tagging t GROUP BY t.repository, t.pull_request """) to_remove = [] - for repo_id, pr, ids, from_, to_ in self.env.cr.fetchall(): + for repo_id, pr, ids, remove, add in self.env.cr.fetchall(): repo = Repos.browse(repo_id) - to_tags = _TAGS[to_ or False] gh = ghs.get(repo) if not gh: gh = ghs[repo] = repo.github() + remove = set().union(*remove) + add = set().union(*add) try: - gh.change_tags(pr, to_tags) + gh.change_tags(pr, remove, add) except Exception: _logger.exception( "Error while trying to change the tags of %s#%s from %s to %s", - repo.name, pr, _TAGS[from_ or False], to_tags, + repo.name, pr.display_name, remove, add, ) else: to_remove.extend(ids) @@ -1324,6 +1325,7 @@ _TAGS['staged'] = _TAGS['ready'] | {'merging 👷'} _TAGS['merged'] = _TAGS['ready'] | {'merged 🎉'} _TAGS['error'] = _TAGS['opened'] | {'error 🙅'} _TAGS['closed'] = _TAGS['opened'] | {'closed 💔'} +ALL_TAGS = set.union(*_TAGS.values()) class Tagging(models.Model): """ @@ -1341,26 +1343,20 @@ class Tagging(models.Model): # being deleted (retargeted to non-managed branches) pull_request = fields.Integer() - state_from = fields.Selection([ - ('opened', 'Opened'), - ('closed', 'Closed'), - ('validated', 'Validated'), - ('approved', 'Approved'), - ('ready', 'Ready'), - ('staged', 'Staged'), - ('merged', 'Merged'), - ('error', 'Error'), - ]) - state_to = fields.Selection([ - ('opened', 'Opened'), - ('closed', 'Closed'), - ('validated', 'Validated'), - ('approved', 'Approved'), - ('ready', 'Ready'), - ('staged', 'Staged'), - ('merged', 'Merged'), - ('error', 'Error'), - ]) + tags_remove = fields.Char(required=True, default='[]') + tags_add = fields.Char(required=True, defualt='[]') + + def create(self, values): + values.pop('state_from', None) + state_to = values.pop('state_to', None) + if state_to: + values['tags_remove'] = ALL_TAGS + values['tags_add'] = _TAGS[state_to] + if not isinstance(values.get('tags_remove', ''), str): + values['tags_remove'] = json.dumps(list(values['tags_remove'])) + if not isinstance(values.get('tags_add', ''), str): + values['tags_add'] = json.dumps(list(values['tags_add'])) + return super().create(values) class Feedback(models.Model): """ Queue of feedback comments to send to PR users