Mercurial > kallithea
changeset 6588:0452c45a546c
pullrequests: fix broken "new PR iteration" handling of ancestor changes
An earlier refactor (5d60c9a391cd) flubbed this code and accidentally
assumed the most recent common ancestor would not change when iterating,
which is of course only the case when there are no merges from 'other' to
'org' among the new revisions.
This was not only not caught during manual testing (nor review), but
neither did the test suite test this. That has now also been rectified.
| author | Søren Løvborg <sorenl@unity3d.com> |
|---|---|
| date | Tue, 11 Apr 2017 14:37:43 +0200 |
| parents | e075f2cc4f8c |
| children | c184df63e470 |
| files | kallithea/controllers/pullrequests.py kallithea/model/pull_request.py kallithea/tests/functional/test_pullrequests.py |
| diffstat | 3 files changed, 87 insertions(+), 4 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/controllers/pullrequests.py Tue Apr 11 01:51:54 2017 +0200 +++ b/kallithea/controllers/pullrequests.py Tue Apr 11 14:37:43 2017 +0200 @@ -362,8 +362,9 @@ def create_new_iteration(self, old_pull_request, new_rev, title, description, reviewers): owner = User.get(request.authuser.user_id) new_org_rev = self._get_ref_rev(old_pull_request.org_repo, 'rev', new_rev) + new_other_rev = self._get_ref_rev(old_pull_request.other_repo, old_pull_request.other_ref_parts[0], old_pull_request.other_ref_parts[1]) try: - cmd = CreatePullRequestIterationAction(old_pull_request, new_org_rev, title, description, owner, reviewers) + cmd = CreatePullRequestIterationAction(old_pull_request, new_org_rev, new_other_rev, title, description, owner, reviewers) except CreatePullRequestAction.ValidationError as e: h.flash(str(e), category='error', logf=log.error) raise HTTPNotFound
--- a/kallithea/model/pull_request.py Tue Apr 11 01:51:54 2017 +0200 +++ b/kallithea/model/pull_request.py Tue Apr 11 14:37:43 2017 +0200 @@ -297,7 +297,7 @@ return True - def __init__(self, old_pull_request, new_org_rev, title, description, owner, reviewers): + def __init__(self, old_pull_request, new_org_rev, new_other_rev, title, description, owner, reviewers): self.old_pull_request = old_pull_request org_repo = old_pull_request.org_repo @@ -308,8 +308,9 @@ #assert other_ref_type == 'branch', other_ref_type # TODO: what if not? new_org_ref = '%s:%s:%s' % (org_ref_type, org_ref_name, new_org_rev) + new_other_ref = '%s:%s:%s' % (other_ref_type, other_ref_name, new_other_rev) - self.create_action = CreatePullRequestAction(org_repo, other_repo, new_org_ref, old_pull_request.other_ref, None, None, owner, reviewers) + self.create_action = CreatePullRequestAction(org_repo, other_repo, new_org_ref, new_other_ref, None, None, owner, reviewers) # Generate complete title/description
--- a/kallithea/tests/functional/test_pullrequests.py Tue Apr 11 01:51:54 2017 +0200 +++ b/kallithea/tests/functional/test_pullrequests.py Tue Apr 11 14:37:43 2017 +0200 @@ -5,7 +5,7 @@ from kallithea.tests.base import * from kallithea.tests.fixture import Fixture -from kallithea.model.db import User +from kallithea.model.db import PullRequest, User from kallithea.model.meta import Session from kallithea.controllers.pullrequests import PullrequestsController @@ -208,6 +208,87 @@ status=400) response.mustcontain('Invalid reviewer "%s" specified' % invalid_user_id) + + def test_iteration_refs(self): + # Repo graph excerpt: + # o fb95b340e0d0 webvcs + # /: + # o : 41d2568309a0 default + # : : + # : o 5ec21f21aafe webvcs + # : : + # : o 9e6119747791 webvcs + # : : + # o : 3d1091ee5a53 default + # :/ + # o 948da46b29c1 default + + self.log_user() + + # create initial PR + response = self.app.post( + url(controller='pullrequests', action='create', repo_name=HG_REPO), + { + 'org_repo': HG_REPO, + 'org_ref': 'rev:9e6119747791:9e6119747791ff886a5abe1193a730b6bf874e1c', + 'other_repo': HG_REPO, + 'other_ref': 'branch:default:3d1091ee5a533b1f4577ec7d8a226bb315fb1336', + 'pullrequest_title': 'title', + 'pullrequest_desc': 'description', + '_authentication_token': self.authentication_token(), + }, + status=302) + pr1_id = int(re.search('/pull-request/(\d+)/', response.location).group(1)) + pr1 = PullRequest.get(pr1_id) + + assert pr1.org_ref == 'branch:webvcs:9e6119747791ff886a5abe1193a730b6bf874e1c' + assert pr1.other_ref == 'branch:default:948da46b29c125838a717f6a8496eb409717078d' + + Session().rollback() # invalidate loaded PR objects before issuing next request. + + # create PR 2 (new iteration with same ancestor) + response = self.app.post( + url(controller='pullrequests', action='post', repo_name=HG_REPO, pull_request_id=pr1_id), + { + 'updaterev': '5ec21f21aafe95220f1fc4843a4a57c378498b71', + 'pullrequest_title': 'title', + 'pullrequest_desc': 'description', + 'owner': TEST_USER_REGULAR_LOGIN, + '_authentication_token': self.authentication_token(), + }, + status=302) + pr2_id = int(re.search('/pull-request/(\d+)/', response.location).group(1)) + pr1 = PullRequest.get(pr1_id) + pr2 = PullRequest.get(pr2_id) + + assert pr2_id != pr1_id + assert pr1.status == PullRequest.STATUS_CLOSED + assert pr2.org_ref == 'branch:webvcs:5ec21f21aafe95220f1fc4843a4a57c378498b71' + assert pr2.other_ref == pr1.other_ref + + Session().rollback() # invalidate loaded PR objects before issuing next request. + + # create PR 3 (new iteration with new ancestor) + response = self.app.post( + url(controller='pullrequests', action='post', repo_name=HG_REPO, pull_request_id=pr2_id), + { + 'updaterev': 'fb95b340e0d03fa51f33c56c991c08077c99303e', + 'pullrequest_title': 'title', + 'pullrequest_desc': 'description', + 'owner': TEST_USER_REGULAR_LOGIN, + '_authentication_token': self.authentication_token(), + }, + status=302) + pr3_id = int(re.search('/pull-request/(\d+)/', response.location).group(1)) + pr2 = PullRequest.get(pr2_id) + pr3 = PullRequest.get(pr3_id) + + assert pr3_id != pr2_id + assert pr2.status == PullRequest.STATUS_CLOSED + assert pr3.org_ref == 'branch:webvcs:fb95b340e0d03fa51f33c56c991c08077c99303e' + assert pr3.other_ref == 'branch:default:41d2568309a05f422cffb8008e599d385f8af439' + + @pytest.mark.usefixtures("test_context_fixture") # apply fixture for all test methods class TestPullrequestsGetRepoRefs(TestController):