diff --git a/conftest.py b/conftest.py index e800258c..e7cdeda2 100644 --- a/conftest.py +++ b/conftest.py @@ -826,6 +826,12 @@ mutation setDraft($pid: ID!) { } } ''' +def state_prop(name: str) -> property: + @property + def _prop(self): + return self._pr[name] + return _prop.setter(lambda self, v: self._set_prop(name, v)) + class PR: def __init__(self, repo, number): self.repo = repo @@ -850,15 +856,9 @@ class PR: caching['If-Modified-Since']= r.headers['Last-Modified'] return contents - @property - def title(self): - return self._pr['title'] - title = title.setter(lambda self, v: self._set_prop('title', v)) - - @property - def base(self): - return self._pr['base'] - base = base.setter(lambda self, v: self._set_prop('base', v)) + title = state_prop('title') + body = state_prop('body') + base = state_prop('base') @property def draft(self): @@ -888,10 +888,6 @@ class PR: def state(self): return self._pr['state'] - @property - def body(self): - return self._pr['body'] - @property def comments(self): r = self.repo._session.get('https://api.github.com/repos/{}/issues/{}/comments'.format(self.repo.name, self.number)) diff --git a/runbot_merge/changelog/2022-06/empty-body.md b/runbot_merge/changelog/2022-06/empty-body.md new file mode 100644 index 00000000..093962f5 --- /dev/null +++ b/runbot_merge/changelog/2022-06/empty-body.md @@ -0,0 +1,4 @@ +FIX: correctly handle PR empty PR descriptions + +Github's webhook for this case are weird, and weren't handled correctly, +updating a PR's description to *or from* empty might be mishandled. diff --git a/runbot_merge/controllers/__init__.py b/runbot_merge/controllers/__init__.py index ecee99f3..3b0b02c0 100644 --- a/runbot_merge/controllers/__init__.py +++ b/runbot_merge/controllers/__init__.py @@ -120,15 +120,25 @@ def handle_pr(env, event): if not source_branch: return handle_pr(env, dict(event, action='opened')) + pr_obj = find(source_branch) updates = {} if source_branch != branch: - updates['target'] = branch.id - updates['squash'] = pr['commits'] == 1 - if event['changes'].keys() & {'title', 'body'}: - updates['message'] = "{}\n\n{}".format(pr['title'].strip(), pr['body'].strip()) + if branch != pr_obj.target: + updates['target'] = branch.id + updates['squash'] = pr['commits'] == 1 + + # turns out github doesn't bother sending a change key if the body is + # changing from empty (None), therefore ignore that entirely, just + # generate the message and check if it changed + message = pr['title'].strip() + body = (pr['body'] or '').strip() + if body: + message += f"\n\n{body}" + if message != pr_obj.message: + updates['message'] = message + _logger.info("update: %s#%d = %s (by %s)", repo.name, pr['number'], updates, event['sender']['login']) if updates: - pr_obj = find(source_branch) pr_obj.write(updates) return 'Updated {}'.format(pr_obj.id) return "Nothing to update ({})".format(event['changes'].keys()) diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py index 1f254960..5049fcda 100644 --- a/runbot_merge/tests/test_basic.py +++ b/runbot_merge/tests/test_basic.py @@ -728,6 +728,8 @@ class TestPREdition: assert pr.message == 'title\n\nbody' with repo: prx.title = "title 2" assert pr.message == 'title 2\n\nbody' + with repo: prx.body = None + assert pr.message == "title 2" assert pr.staging_id, \ "message edition does not affect staging of rebased PRs" with repo: prx.base = '1.0'