Mercurial > kallithea
changeset 4359:cfd9115db2a5
pull requests: cleanup of PR status voting calculation
| author | Mads Kiilerich <madski@unity3d.com> |
|---|---|
| date | Tue, 10 Dec 2013 19:30:37 +0100 |
| parents | 068ca0afcf9f |
| children | 05af189da2ae |
| files | kallithea/controllers/pullrequests.py kallithea/model/changeset_status.py kallithea/templates/pullrequests/pullrequest_show.html |
| diffstat | 3 files changed, 45 insertions(+), 47 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/controllers/pullrequests.py Fri Jul 18 18:47:02 2014 +0200 +++ b/kallithea/controllers/pullrequests.py Tue Dec 10 19:30:37 2013 +0100 @@ -483,25 +483,6 @@ c.allowed_to_change_status = self._get_is_allowed_change_status(c.pull_request) cc_model = ChangesetCommentsModel() cs_model = ChangesetStatusModel() - _cs_statuses = cs_model.get_statuses(c.pull_request.org_repo, - pull_request=c.pull_request, - with_revisions=True) - - cs_statuses = defaultdict(list) - for st in _cs_statuses: - cs_statuses[st.author.username] += [st] - - c.pull_request_reviewers = [] - c.pull_request_pending_reviewers = [] - for o in c.pull_request.reviewers: - st = cs_statuses.get(o.user.username, None) - if st: - sorter = lambda k: k.version - st = [(x, list(y)[0]) - for x, y in (groupby(sorted(st, key=sorter), sorter))] - else: - c.pull_request_pending_reviewers.append(o.user) - c.pull_request_reviewers.append([o.user, st]) # pull_requests repo_name we opened it against # ie. other_repo must match @@ -526,9 +507,10 @@ pull_request=pull_request_id) # (badly named) pull-request status calculation based on reviewer votes - c.current_changeset_status = cs_model.calculate_status( - c.pull_request_reviewers, - ) + (c.pull_request_reviewers, + c.pull_request_pending_reviewers, + c.current_voting_result, + ) = cs_model.calculate_pull_request_result(c.pull_request) c.changeset_statuses = ChangesetStatus.STATUSES c.as_form = False
--- a/kallithea/model/changeset_status.py Fri Jul 18 18:47:02 2014 +0200 +++ b/kallithea/model/changeset_status.py Tue Dec 10 19:30:37 2013 +0100 @@ -65,26 +65,39 @@ q = q.order_by(ChangesetStatus.version.asc()) return q - def calculate_status(self, statuses_by_reviewers): + def calculate_pull_request_result(self, pull_request): """ - approved if consensus - (old description: leading one wins, if number of occurrences are equal than weaker wins) - - :param statuses_by_reviewers: + Policy: approve if consensus. Only approve and reject counts as valid votes. """ - votes = defaultdict(int) - reviewers_number = len(statuses_by_reviewers) - for user, statuses in statuses_by_reviewers: - if statuses: - ver, latest = statuses[0] - votes[latest.status] += 1 - else: - votes[ChangesetStatus.DEFAULT] += 1 - if votes.get(ChangesetStatus.STATUS_APPROVED) == reviewers_number: - return ChangesetStatus.STATUS_APPROVED - else: - return ChangesetStatus.STATUS_UNDER_REVIEW + # collect latest votes from all voters + cs_statuses = dict() + for st in reversed(self.get_statuses(pull_request.org_repo, + pull_request=pull_request, + with_revisions=True)): + cs_statuses[st.author.username] = st + # collect votes from official reviewers + pull_request_reviewers = [] + pull_request_pending_reviewers = [] + approved_votes = 0 + for r in pull_request.reviewers: + st = cs_statuses.get(r.user.username) + if st and st.status == ChangesetStatus.STATUS_APPROVED: + approved_votes += 1 + if not st or st.status in (ChangesetStatus.STATUS_NOT_REVIEWED, + ChangesetStatus.STATUS_UNDER_REVIEW): + st = None + pull_request_pending_reviewers.append(r.user) + pull_request_reviewers.append((r.user, st)) + + # calculate result + result = ChangesetStatus.STATUS_UNDER_REVIEW + if approved_votes and approved_votes == len(pull_request.reviewers): + result = ChangesetStatus.STATUS_APPROVED + + return (pull_request_reviewers, + pull_request_pending_reviewers, + result) def get_statuses(self, repo, revision=None, pull_request=None, with_revisions=False): @@ -156,6 +169,7 @@ def _create_status(user, repo, status, comment, revision, pull_request): new_status = ChangesetStatus() + new_status.version = 0 # default new_status.author = self._get_user(user) new_status.repo = self._get_repo(repo) new_status.status = status
--- a/kallithea/templates/pullrequests/pullrequest_show.html Fri Jul 18 18:47:02 2014 +0200 +++ b/kallithea/templates/pullrequests/pullrequest_show.html Tue Dec 10 19:30:37 2013 +0100 @@ -78,18 +78,18 @@ <div class="field"> <div class="label-summary"> - <label>${_('Review status')}:</label> + <label>${_('Reviewer voting result')}:</label> </div> <div class="input"> <div class="changeset-status-container" style="float:none;clear:both"> - %if c.current_changeset_status: + %if c.current_voting_result: <div class="changeset-status-ico" style="padding:0px 4px 0px 0px"> - <img src="${h.url('/images/icons/flag_status_%s.png' % c.current_changeset_status)}" title="${_('Pull request status calculated from votes')}"/></div> + <img src="${h.url('/images/icons/flag_status_%s.png' % c.current_voting_result)}" title="${_('Pull request status calculated from votes')}"/></div> <div class="changeset-status-lbl tooltip" title="${_('Pull request status calculated from votes')}"> %if c.pull_request.is_closed(): ${_('Closed')}, %endif - ${h.changeset_status_lbl(c.current_changeset_status)} + ${h.changeset_status_lbl(c.current_voting_result)} </div> %endif @@ -103,8 +103,10 @@ <div class="input"> % if len(c.pull_request_pending_reviewers) > 0: <div class="tooltip" title="${h.tooltip(', '.join([x.username for x in c.pull_request_pending_reviewers]))}">${ungettext('%d reviewer', '%d reviewers',len(c.pull_request_pending_reviewers)) % len(c.pull_request_pending_reviewers)}</div> + % elif len(c.pull_request_reviewers) > 0: + <div>${_('Pull request was reviewed by all reviewers')}</div> %else: - <div>${_('Pull request was reviewed by all reviewers')}</div> + <div>${_('There are no reviewers')}</div> %endif </div> </div> @@ -182,8 +184,8 @@ %for member,status in c.pull_request_reviewers: <li id="reviewer_${member.user_id}"> <div class="reviewers_member"> - <div class="reviewer_status tooltip" title="${h.tooltip(h.changeset_status_lbl(status[0][1].status if status else 'not_reviewed'))}"> - <img src="${h.url(str('/images/icons/flag_status_%s.png' % (status[0][1].status if status else 'not_reviewed')))}"/> + <div class="reviewer_status tooltip" title="${h.tooltip(h.changeset_status_lbl(status.status if status else 'not_reviewed'))}"> + <img src="${h.url(str('/images/icons/flag_status_%s.png' % (status.status if status else 'not_reviewed')))}"/> </div> <div class="reviewer_gravatar gravatar"><img alt="gravatar" src="${h.gravatar_url(member.email,14)}"/> </div> <div style="float:left;">${member.full_name} (${_('owner') if c.pull_request.user_id == member.user_id else _('reviewer')})</div> @@ -293,7 +295,7 @@ ## main comment form and it status ${comment.comments(h.url('pullrequest_comment', repo_name=c.repo_name, pull_request_id=c.pull_request.pull_request_id), - c.current_changeset_status, + c.current_voting_result, is_pr=True, change_status=c.allowed_to_change_status)} %endif