default-reviewers: introduce new voting rule logic that allows

defining vote rules for user groups assigned to the default reviewers.

- allows to set per number votes on user groups
- group reviewers by groups
- store the rules and show them in the UI
- fixes #5409
This commit is contained in:
Marcin Kuzminski 2018-01-19 12:38:25 +01:00
parent b0d09dc6e5
commit 6f2703e4af
28 changed files with 2240 additions and 187 deletions

View file

@ -42,6 +42,8 @@
"<%= dirs.js.src %>/bootstrap.js",
"<%= dirs.js.src %>/i18n_utils.js",
"<%= dirs.js.src %>/deform.js",
"<%= dirs.js.src %>/ejs.js",
"<%= dirs.js.src %>/ejs_templates/utils.js",
"<%= dirs.js.src %>/plugins/jquery.pjax.js",
"<%= dirs.js.src %>/plugins/jquery.dataTables.js",
"<%= dirs.js.src %>/plugins/flavoured_checkbox.js",

View file

@ -51,7 +51,7 @@ PYRAMID_SETTINGS = {}
EXTENSIONS = {}
__version__ = ('.'.join((str(each) for each in VERSION[:3])))
__dbversion__ = 83 # defines current db version for migrations
__dbversion__ = 85 # defines current db version for migrations
__platform__ = platform.system()
__license__ = 'AGPLv3, and Commercial License'
__author__ = 'RhodeCode GmbH'

View file

@ -108,7 +108,7 @@ class TestGetPullRequest(object):
'reasons': reasons,
'review_status': st[0][1].status if st else 'not_reviewed',
}
for reviewer, reasons, mandatory, st in
for obj, reviewer, reasons, mandatory, st in
pull_request.reviewers_statuses()
]
}

View file

@ -133,7 +133,7 @@ class TestUpdatePullRequest(object):
removed = [a.username]
pull_request = pr_util.create_pull_request(
reviewers=[(a.username, ['added via API'], False)])
reviewers=[(a.username, ['added via API'], False, [])])
id_, params = build_data(
self.apikey, 'update_pull_request',

View file

@ -53,7 +53,6 @@ class AdminUserGroupsView(BaseAppView, DataGridAppView):
PermissionModel().set_global_permission_choices(
c, gettext_translator=self.request.translate)
return c
# permission check in data loading of

View file

@ -299,7 +299,7 @@ class TestPullrequestsView(object):
pull_request = pr_util.create_pull_request()
pull_request_id = pull_request.pull_request_id
PullRequestModel().update_reviewers(
pull_request_id, [(1, ['reason'], False), (2, ['reason2'], False)],
pull_request_id, [(1, ['reason'], False, []), (2, ['reason2'], False, [])],
pull_request.author)
author = pull_request.user_id
repo = pull_request.target_repo.repo_id
@ -376,6 +376,8 @@ class TestPullrequestsView(object):
('__start__', 'reasons:sequence'),
('reason', 'Some reason'),
('__end__', 'reasons:sequence'),
('__start__', 'rules:sequence'),
('__end__', 'rules:sequence'),
('mandatory', 'False'),
('__end__', 'reviewer:mapping'),
('__end__', 'review_members:sequence'),
@ -433,6 +435,8 @@ class TestPullrequestsView(object):
('__start__', 'reasons:sequence'),
('reason', 'Some reason'),
('__end__', 'reasons:sequence'),
('__start__', 'rules:sequence'),
('__end__', 'rules:sequence'),
('mandatory', 'False'),
('__end__', 'reviewer:mapping'),
('__end__', 'review_members:sequence'),
@ -460,7 +464,7 @@ class TestPullrequestsView(object):
# Change reviewers and check that a notification was made
PullRequestModel().update_reviewers(
pull_request.pull_request_id, [(1, [], False)],
pull_request.pull_request_id, [(1, [], False, [])],
pull_request.author)
assert len(notifications.all()) == 2
@ -497,6 +501,8 @@ class TestPullrequestsView(object):
('__start__', 'reasons:sequence'),
('reason', 'Some reason'),
('__end__', 'reasons:sequence'),
('__start__', 'rules:sequence'),
('__end__', 'rules:sequence'),
('mandatory', 'False'),
('__end__', 'reviewer:mapping'),
('__end__', 'review_members:sequence'),

View file

@ -22,7 +22,7 @@ from rhodecode.lib import helpers as h
from rhodecode.lib.utils2 import safe_int
def reviewer_as_json(user, reasons=None, mandatory=False):
def reviewer_as_json(user, reasons=None, mandatory=False, rules=None, user_group=None):
"""
Returns json struct of a reviewer for frontend
@ -34,10 +34,13 @@ def reviewer_as_json(user, reasons=None, mandatory=False):
return {
'user_id': user.user_id,
'reasons': reasons or [],
'rules': rules or [],
'mandatory': mandatory,
'user_group': user_group,
'username': user.username,
'first_name': user.first_name,
'last_name': user.last_name,
'user_link': h.link_to_user(user),
'gravatar_link': h.gravatar_url(user.email, 14),
}
@ -68,7 +71,7 @@ def validate_default_reviewers(review_members, reviewer_rules):
reviewer_by_id = {}
for r in review_members:
reviewer_user_id = safe_int(r['user_id'])
entry = (reviewer_user_id, r['reasons'], r['mandatory'])
entry = (reviewer_user_id, r['reasons'], r['mandatory'], r['rules'])
reviewer_by_id[reviewer_user_id] = entry
reviewers.append(entry)

View file

@ -0,0 +1,38 @@
import logging
from sqlalchemy import *
from rhodecode.model import meta
from rhodecode.lib.dbmigrate.versions import _reset_base, notify
log = logging.getLogger(__name__)
def upgrade(migrate_engine):
"""
Upgrade operations go here.
Don't create your own engine; bind migrate_engine to your metadata
"""
_reset_base(migrate_engine)
from rhodecode.lib.dbmigrate.schema import db_4_11_0_0 as db
reviewers_table = db.PullRequestReviewers.__table__
rule_data = Column(
'rule_data_json',
db.JsonType(dialect_map=dict(mysql=UnicodeText(16384))))
rule_data.create(table=reviewers_table)
# issue fixups
fixups(db, meta.Session)
def downgrade(migrate_engine):
meta = MetaData()
meta.bind = migrate_engine
def fixups(models, _SESSION):
pass

View file

@ -0,0 +1,37 @@
import logging
from sqlalchemy import *
from rhodecode.model import meta
from rhodecode.lib.dbmigrate.versions import _reset_base, notify
log = logging.getLogger(__name__)
def upgrade(migrate_engine):
"""
Upgrade operations go here.
Don't create your own engine; bind migrate_engine to your metadata
"""
_reset_base(migrate_engine)
from rhodecode.lib.dbmigrate.schema import db_4_11_0_0 as db
user_group_review_table = db.RepoReviewRuleUserGroup.__table__
vote_rule = Column("vote_rule", Integer(), nullable=True,
default=-1)
vote_rule.create(table=user_group_review_table)
# issue fixups
fixups(db, meta.Session)
def downgrade(migrate_engine):
meta = MetaData()
meta.bind = migrate_engine
def fixups(models, _SESSION):
pass

View file

@ -2070,3 +2070,8 @@ def go_import_header(request, db_repo=None):
# we have a repo and go-get flag,
return literal('<meta name="go-import" content="{} {} {}">'.format(
prefix, db_repo.repo_type, clone_url))
def reviewer_as_json(*args, **kwargs):
from rhodecode.apps.repository.utils import reviewer_as_json as _reviewer_as_json
return _reviewer_as_json(*args, **kwargs)

View file

@ -21,7 +21,7 @@
import itertools
import logging
from collections import defaultdict
import collections
from rhodecode.model import BaseModel
from rhodecode.model.db import (
@ -68,6 +68,107 @@ class ChangesetStatusModel(BaseModel):
q = q.order_by(ChangesetStatus.version.asc())
return q
def calculate_group_vote(self, group_id, group_statuses_by_reviewers,
trim_votes=True):
"""
Calculate status based on given group members, and voting rule
group1 - 4 members, 3 required for approval
user1 - approved
user2 - reject
user3 - approved
user4 - rejected
final_state: rejected, reasons not at least 3 votes
group1 - 4 members, 2 required for approval
user1 - approved
user2 - reject
user3 - approved
user4 - rejected
final_state: approved, reasons got at least 2 approvals
group1 - 4 members, ALL required for approval
user1 - approved
user2 - reject
user3 - approved
user4 - rejected
final_state: rejected, reasons not all approvals
group1 - 4 members, ALL required for approval
user1 - approved
user2 - approved
user3 - approved
user4 - approved
final_state: approved, reason all approvals received
group1 - 4 members, 5 required for approval
(approval should be shorted to number of actual members)
user1 - approved
user2 - approved
user3 - approved
user4 - approved
final_state: approved, reason all approvals received
"""
group_vote_data = {}
got_rule = False
members = collections.OrderedDict()
for review_obj, user, reasons, mandatory, statuses \
in group_statuses_by_reviewers:
if not got_rule:
group_vote_data = review_obj.rule_user_group_data()
got_rule = bool(group_vote_data)
members[user.user_id] = statuses
if not group_vote_data:
return []
required_votes = group_vote_data['vote_rule']
if required_votes == -1:
# -1 means all required, so we replace it with how many people
# are in the members
required_votes = len(members)
if trim_votes and required_votes > len(members):
# we require more votes than we have members in the group
# in this case we trim the required votes to the number of members
required_votes = len(members)
approvals = sum([
1 for statuses in members.values()
if statuses and
statuses[0][1].status == ChangesetStatus.STATUS_APPROVED])
calculated_votes = []
# we have all votes from users, now check if we have enough votes
# to fill other
fill_in = ChangesetStatus.STATUS_UNDER_REVIEW
if approvals >= required_votes:
fill_in = ChangesetStatus.STATUS_APPROVED
for member, statuses in members.items():
if statuses:
ver, latest = statuses[0]
if fill_in == ChangesetStatus.STATUS_APPROVED:
calculated_votes.append(fill_in)
else:
calculated_votes.append(latest.status)
else:
calculated_votes.append(fill_in)
return calculated_votes
def calculate_status(self, statuses_by_reviewers):
"""
Given the approval statuses from reviewers, calculates final approval
@ -76,21 +177,45 @@ class ChangesetStatusModel(BaseModel):
:param statuses_by_reviewers:
"""
votes = defaultdict(int)
reviewers_number = len(statuses_by_reviewers)
for user, reasons, mandatory, statuses in statuses_by_reviewers:
if statuses:
ver, latest = statuses[0]
votes[latest.status] += 1
else:
votes[ChangesetStatus.DEFAULT] += 1
# all approved
if votes.get(ChangesetStatus.STATUS_APPROVED) == reviewers_number:
def group_rule(element):
review_obj = element[0]
rule_data = review_obj.rule_user_group_data()
if rule_data and rule_data['id']:
return rule_data['id']
voting_groups = itertools.groupby(
sorted(statuses_by_reviewers, key=group_rule), group_rule)
voting_by_groups = [(x, list(y)) for x, y in voting_groups]
reviewers_number = len(statuses_by_reviewers)
votes = collections.defaultdict(int)
for group, group_statuses_by_reviewers in voting_by_groups:
if group:
# calculate how the "group" voted
for vote_status in self.calculate_group_vote(
group, group_statuses_by_reviewers):
votes[vote_status] += 1
else:
for review_obj, user, reasons, mandatory, statuses \
in group_statuses_by_reviewers:
# individual vote
if statuses:
ver, latest = statuses[0]
votes[latest.status] += 1
approved_votes_count = votes[ChangesetStatus.STATUS_APPROVED]
rejected_votes_count = votes[ChangesetStatus.STATUS_REJECTED]
# TODO(marcink): with group voting, how does rejected work,
# do we ever get rejected state ?
if approved_votes_count == reviewers_number:
return ChangesetStatus.STATUS_APPROVED
# all rejected
if votes.get(ChangesetStatus.STATUS_REJECTED) == reviewers_number:
if rejected_votes_count == reviewers_number:
return ChangesetStatus.STATUS_REJECTED
return ChangesetStatus.STATUS_UNDER_REVIEW
@ -234,7 +359,7 @@ class ChangesetStatusModel(BaseModel):
pull_request=pull_request,
with_revisions=True)
commit_statuses = defaultdict(list)
commit_statuses = collections.defaultdict(list)
for st in _commit_statuses:
commit_statuses[st.author.username] += [st]
@ -243,17 +368,18 @@ class ChangesetStatusModel(BaseModel):
def version(commit_status):
return commit_status.version
for o in pull_request.reviewers:
if not o.user:
for obj in pull_request.reviewers:
if not obj.user:
continue
statuses = commit_statuses.get(o.user.username, None)
statuses = commit_statuses.get(obj.user.username, None)
if statuses:
statuses = [(x, list(y)[0])
for x, y in (itertools.groupby(
sorted(statuses, key=version),version))]
status_groups = itertools.groupby(
sorted(statuses, key=version), version)
statuses = [(x, list(y)[0]) for x, y in status_groups]
pull_request_reviewers.append(
(o.user, o.reasons, o.mandatory, statuses))
(obj, obj.user, obj.reasons, obj.mandatory, statuses))
return pull_request_reviewers
def calculated_review_status(self, pull_request, reviewers_statuses=None):

View file

@ -59,8 +59,7 @@ from rhodecode.lib.utils2 import (
str2bool, safe_str, get_commit_safe, safe_unicode, md5_safe,
time_to_datetime, aslist, Optional, safe_int, get_clone_url, AttributeDict,
glob2re, StrictAttributeDict, cleaned_uri)
from rhodecode.lib.jsonalchemy import MutationObj, MutationList, JsonType, \
JsonRaw
from rhodecode.lib.jsonalchemy import MutationObj, MutationList, JsonType
from rhodecode.lib.ext_json import json
from rhodecode.lib.caching_query import FromCache
from rhodecode.lib.encrypt import AESCipher
@ -1327,7 +1326,7 @@ class UserGroup(Base, BaseModel):
@hybrid_property
def description_safe(self):
from rhodecode.lib import helpers as h
return h.escape(self.description)
return h.escape(self.user_group_description)
@hybrid_property
def group_data(self):
@ -3594,7 +3593,7 @@ class _PullRequestBase(BaseModel):
'reasons': reasons,
'review_status': st[0][1].status if st else 'not_reviewed',
}
for reviewer, reasons, mandatory, st in
for obj, reviewer, reasons, mandatory, st in
pull_request.reviewers_statuses()
]
}
@ -3790,10 +3789,34 @@ class PullRequestReviewers(Base, BaseModel):
_reasons = Column(
'reason', MutationList.as_mutable(
JsonType('list', dialect_map=dict(mysql=UnicodeText(16384)))))
mandatory = Column("mandatory", Boolean(), nullable=False, default=False)
user = relationship('User')
pull_request = relationship('PullRequest')
rule_data = Column(
'rule_data_json',
JsonType(dialect_map=dict(mysql=UnicodeText(16384))))
def rule_user_group_data(self):
"""
Returns the voting user group rule data for this reviewer
"""
if self.rule_data and 'vote_rule' in self.rule_data:
user_group_data = {}
if 'rule_user_group_entry_id' in self.rule_data:
# means a group with voting rules !
user_group_data['id'] = self.rule_data['rule_user_group_entry_id']
user_group_data['name'] = self.rule_data['rule_name']
user_group_data['vote_rule'] = self.rule_data['vote_rule']
return user_group_data
def __unicode__(self):
return u"<%s('id:%s')>" % (self.__class__.__name__,
self.pull_requests_reviewers_id)
class Notification(Base, BaseModel):
__tablename__ = 'notifications'
@ -4086,6 +4109,7 @@ class RepoReviewRuleUser(Base, BaseModel):
{'extend_existing': True, 'mysql_engine': 'InnoDB',
'mysql_charset': 'utf8', 'sqlite_autoincrement': True,}
)
repo_review_rule_user_id = Column('repo_review_rule_user_id', Integer(), primary_key=True)
repo_review_rule_id = Column("repo_review_rule_id", Integer(), ForeignKey('repo_review_rules.repo_review_rule_id'))
user_id = Column("user_id", Integer(), ForeignKey('users.user_id'), nullable=False)
@ -4104,17 +4128,28 @@ class RepoReviewRuleUserGroup(Base, BaseModel):
{'extend_existing': True, 'mysql_engine': 'InnoDB',
'mysql_charset': 'utf8', 'sqlite_autoincrement': True,}
)
VOTE_RULE_ALL = -1
repo_review_rule_users_group_id = Column('repo_review_rule_users_group_id', Integer(), primary_key=True)
repo_review_rule_id = Column("repo_review_rule_id", Integer(), ForeignKey('repo_review_rules.repo_review_rule_id'))
users_group_id = Column("users_group_id", Integer(),ForeignKey('users_groups.users_group_id'), nullable=False)
mandatory = Column("mandatory", Boolean(), nullable=False, default=False)
vote_rule = Column("vote_rule", Integer(), nullable=True, default=VOTE_RULE_ALL)
users_group = relationship('UserGroup')
def rule_data(self):
return {
'mandatory': self.mandatory
'mandatory': self.mandatory,
'vote_rule': self.vote_rule
}
@property
def vote_rule_label(self):
if not self.vote_rule or self.vote_rule == self.VOTE_RULE_ALL:
return 'all must vote'
else:
return 'min. vote {}'.format(self.vote_rule)
class RepoReviewRule(Base, BaseModel):
__tablename__ = 'repo_review_rules'
@ -4225,12 +4260,20 @@ class RepoReviewRule(Base, BaseModel):
for rule_user_group in self.rule_user_groups:
source_data = {
'user_group_id': rule_user_group.users_group.users_group_id,
'name': rule_user_group.users_group.users_group_name,
'members': len(rule_user_group.users_group.members)
}
for member in rule_user_group.users_group.members:
if member.user.active:
users[member.user.username] = {
key = member.user.username
if key in users:
# skip this member as we have him already
# this prevents from override the "first" matched
# users with duplicates in multiple groups
continue
users[key] = {
'user': member.user,
'source': 'user_group',
'source_data': source_data,
@ -4239,6 +4282,13 @@ class RepoReviewRule(Base, BaseModel):
return users
def user_group_vote_rule(self):
rules = []
if self.rule_user_groups:
for user_group in self.rule_user_groups:
rules.append(user_group)
return rules
def __repr__(self):
return '<RepoReviewerRule(id=%r, repo=%r)>' % (
self.repo_review_rule_id, self.repo)

View file

@ -584,6 +584,7 @@ def PullRequestForm(localizer, repo_id):
class ReviewerForm(formencode.Schema):
user_id = v.Int(not_empty=True)
reasons = All()
rules = All(v.UniqueList(localizer, convert=int)())
mandatory = v.StringBoolean()
class _PullRequestForm(formencode.Schema):

View file

@ -51,7 +51,7 @@ from rhodecode.model.changeset_status import ChangesetStatusModel
from rhodecode.model.comment import CommentsModel
from rhodecode.model.db import (
or_, PullRequest, PullRequestReviewers, ChangesetStatus,
PullRequestVersion, ChangesetComment, Repository)
PullRequestVersion, ChangesetComment, Repository, RepoReviewRule)
from rhodecode.model.meta import Session
from rhodecode.model.notification import NotificationModel, \
EmailNotificationModel
@ -468,7 +468,7 @@ class PullRequestModel(BaseModel):
reviewer_ids = set()
# members / reviewers
for reviewer_object in reviewers:
user_id, reasons, mandatory = reviewer_object
user_id, reasons, mandatory, rules = reviewer_object
user = self._get_user(user_id)
# skip duplicates
@ -482,6 +482,33 @@ class PullRequestModel(BaseModel):
reviewer.pull_request = pull_request
reviewer.reasons = reasons
reviewer.mandatory = mandatory
# NOTE(marcink): pick only first rule for now
rule_id = rules[0] if rules else None
rule = RepoReviewRule.get(rule_id) if rule_id else None
if rule:
review_group = rule.user_group_vote_rule()
if review_group:
# NOTE(marcink):
# again, can be that user is member of more,
# but we pick the first same, as default reviewers algo
review_group = review_group[0]
rule_data = {
'rule_name':
rule.review_rule_name,
'rule_user_group_entry_id':
review_group.repo_review_rule_users_group_id,
'rule_user_group_name':
review_group.users_group.users_group_name,
'rule_user_group_members':
[x.user.username for x in review_group.users_group.members],
}
# e.g {'vote_rule': -1, 'mandatory': True}
rule_data.update(review_group.rule_data())
reviewer.rule_data = rule_data
Session().add(reviewer)
# Set approval status to "Under Review" for all commits which are
@ -962,14 +989,14 @@ class PullRequestModel(BaseModel):
:param pull_request: the pr to update
:param reviewer_data: list of tuples
[(user, ['reason1', 'reason2'], mandatory_flag)]
[(user, ['reason1', 'reason2'], mandatory_flag, [rules])]
"""
pull_request = self.__get_pull_request(pull_request)
if pull_request.is_closed():
raise ValueError('This pull request is closed')
reviewers = {}
for user_id, reasons, mandatory in reviewer_data:
for user_id, reasons, mandatory, rules in reviewer_data:
if isinstance(user_id, (int, basestring)):
user_id = self._get_user(user_id).user_id
reviewers[user_id] = {

View file

@ -74,6 +74,7 @@ class UserModel(BaseModel):
'username': user.username,
'email': user.email,
'icon_link': h.gravatar_url(user.email, 30),
'profile_link': h.link_to_user(user),
'value_display': h.escape(h.person(user)),
'value': user.username,
'value_type': 'user',

View file

@ -26,6 +26,7 @@ class ReviewerSchema(colander.MappingSchema):
username = colander.SchemaNode(types.StrOrIntType())
reasons = colander.SchemaNode(colander.List(), missing=['no reason specified'])
mandatory = colander.SchemaNode(colander.Boolean(), missing=False)
rules = colander.SchemaNode(colander.List(), missing=[])
class ReviewerListSchema(colander.SequenceSchema):

View file

@ -1324,7 +1324,7 @@ table.integrations {
.reviewers ul li {
position: relative;
width: 100%;
margin-bottom: 8px;
padding-bottom: 8px;
}
.reviewer_entry {
@ -1335,19 +1335,15 @@ table.integrations {
width: 100%;
overflow: auto;
}
.reviewer_reason_container {
padding-left: 20px;
}
.reviewer_reason {
padding-left: 20px;
line-height: 1.5em;
}
.reviewer_status {
display: inline-block;
vertical-align: top;
width: 7%;
min-width: 20px;
width: 25px;
min-width: 25px;
height: 1.2em;
margin-top: 3px;
line-height: 1em;
@ -1370,7 +1366,17 @@ table.integrations {
}
}
.reviewer_member_mandatory,
.reviewer_member_mandatory {
position: absolute;
left: 15px;
top: 8px;
width: 16px;
font-size: 11px;
margin: 0;
padding: 0;
color: black;
}
.reviewer_member_mandatory_remove,
.reviewer_member_remove {
position: absolute;
@ -1386,10 +1392,6 @@ table.integrations {
color: @grey4;
}
.reviewer_member_mandatory {
padding-top:20px;
}
.reviewer_member_status {
margin-top: 5px;
}
@ -1849,13 +1851,6 @@ BIN_FILENODE = 7
}
}
.no-object-border {
text-align: center;
padding: 20px;
border-radius: @border-radius-base;
border: 1px solid @grey4;
color: @grey4;
}
.creation_in_progress {
color: @grey4

File diff suppressed because it is too large Load diff

View file

@ -0,0 +1,58 @@
// # Copyright (C) 2010-2017 RhodeCode GmbH
// #
// # This program is free software: you can redistribute it and/or modify
// # it under the terms of the GNU Affero General Public License, version 3
// # (only), as published by the Free Software Foundation.
// #
// # This program is distributed in the hope that it will be useful,
// # but WITHOUT ANY WARRANTY; without even the implied warranty of
// # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// # GNU General Public License for more details.
// #
// # You should have received a copy of the GNU Affero General Public License
// # along with this program. If not, see <http://www.gnu.org/licenses/>.
// #
// # This program is dual-licensed. If you wish to learn more about the
// # RhodeCode Enterprise Edition, including its added features, Support services,
// # and proprietary license terms, please see https://rhodecode.com/licenses/
var EJS_TEMPLATES = {};
var renderTemplate = function(tmplName, data) {
var tmplStr = getTemplate(tmplName);
var options = {};
var template = ejs.compile(tmplStr, options);
return template(data);
};
var registerTemplate = function (name) {
if (EJS_TEMPLATES[name] !== undefined) {
return
}
var template = $('#ejs_' + name);
if (template.get(0) !== undefined) {
EJS_TEMPLATES[name] = template.html();
} else {
console.log('Failed to register template', name)
}
};
var registerTemplates = function () {
$.each($('.ejsTemplate'), function(idx, value) {
var id = $(value).attr('id');
var tmplId = id.substring(0, 4);
var tmplName = id.substring(4);
if (tmplId === 'ejs_') {
registerTemplate(tmplName)
}
});
};
var getTemplate = function (name) {
return EJS_TEMPLATES[name]
};

View file

@ -137,10 +137,10 @@ ReviewersController = function () {
}
if (data.rules.voting !== undefined) {
if (data.rules.voting < 0){
if (data.rules.voting < 0) {
self.$rulesList.append(
self.addRule(
_gettext('All reviewers must vote.'))
_gettext('All individual reviewers must vote.'))
)
} else if (data.rules.voting === 1) {
self.$rulesList.append(
@ -155,6 +155,15 @@ ReviewersController = function () {
)
}
}
if (data.rules.voting_groups !== undefined) {
$.each(data.rules.voting_groups, function(index, rule_data) {
self.$rulesList.append(
self.addRule(rule_data.text)
)
});
}
if (data.rules.use_code_authors_for_review) {
self.$rulesList.append(
self.addRule(
@ -227,10 +236,7 @@ ReviewersController = function () {
for (var i = 0; i < data.reviewers.length; i++) {
var reviewer = data.reviewers[i];
self.addReviewMember(
reviewer.user_id, reviewer.first_name,
reviewer.last_name, reviewer.username,
reviewer.gravatar_link, reviewer.reasons,
reviewer.mandatory);
reviewer, reviewer.reasons, reviewer.mandatory);
}
$('.calculate-reviewers').hide();
prButtonLock(false, null, 'reviewers');
@ -260,64 +266,22 @@ ReviewersController = function () {
$('#reviewer_{0}'.format(reviewer_id)).remove();
}
};
this.reviewMemberEntry = function() {
this.addReviewMember = function(id, fname, lname, nname, gravatar_link, reasons, mandatory) {
};
this.addReviewMember = function(reviewer_obj, reasons, mandatory) {
var members = self.$reviewMembers.get(0);
var reasons_html = '';
var reasons_inputs = '';
var id = reviewer_obj.user_id;
var username = reviewer_obj.username;
var reasons = reasons || [];
var mandatory = mandatory || false;
if (reasons) {
for (var i = 0; i < reasons.length; i++) {
reasons_html += '<div class="reviewer_reason">- {0}</div>'.format(reasons[i]);
reasons_inputs += '<input type="hidden" name="reason" value="' + escapeHtml(reasons[i]) + '">';
}
}
var tmpl = '' +
'<li id="reviewer_{2}" class="reviewer_entry">'+
'<input type="hidden" name="__start__" value="reviewer:mapping">'+
'<div class="reviewer_status">'+
'<div class="flag_status not_reviewed pull-left reviewer_member_status"></div>'+
'</div>'+
'<img alt="gravatar" class="gravatar" src="{0}"/>'+
'<span class="reviewer_name user">{1}</span>'+
reasons_html +
'<input type="hidden" name="user_id" value="{2}">'+
'<input type="hidden" name="__start__" value="reasons:sequence">'+
'{3}'+
'<input type="hidden" name="__end__" value="reasons:sequence">';
if (mandatory) {
tmpl += ''+
'<div class="reviewer_member_mandatory_remove">' +
'<i class="icon-remove-sign"></i>'+
'</div>' +
'<input type="hidden" name="mandatory" value="true">'+
'<div class="reviewer_member_mandatory">' +
'<i class="icon-lock" title="Mandatory reviewer"></i>'+
'</div>';
} else {
tmpl += ''+
'<input type="hidden" name="mandatory" value="false">'+
'<div class="reviewer_member_remove action_button" onclick="reviewersController.removeReviewMember({2})">' +
'<i class="icon-remove-sign"></i>'+
'</div>';
}
// continue template
tmpl += ''+
'<input type="hidden" name="__end__" value="reviewer:mapping">'+
'</li>' ;
var displayname = "{0} ({1} {2})".format(
nname, escapeHtml(fname), escapeHtml(lname));
var element = tmpl.format(gravatar_link,displayname,id,reasons_inputs);
// check if we don't have this ID already in
var ids = [];
// register IDS to check if we don't have this ID already in
var currentIds = [];
var _els = self.$reviewMembers.find('li').toArray();
for (el in _els){
ids.push(_els[el].id)
currentIds.push(_els[el].id)
}
var userAllowedReview = function(userId) {
@ -333,19 +297,29 @@ ReviewersController = function () {
var userAllowed = userAllowedReview(id);
if (!userAllowed){
alert(_gettext('User `{0}` not allowed to be a reviewer').format(nname));
}
var shouldAdd = userAllowed && ids.indexOf('reviewer_'+id) == -1;
if(shouldAdd) {
alert(_gettext('User `{0}` not allowed to be a reviewer').format(username));
} else {
// only add if it's not there
members.innerHTML += element;
var alreadyReviewer = currentIds.indexOf('reviewer_'+id) != -1;
if (alreadyReviewer) {
alert(_gettext('User `{0}` already in reviewers').format(username));
} else {
members.innerHTML += renderTemplate('reviewMemberEntry', {
'member': reviewer_obj,
'mandatory': mandatory,
'allowed_to_update': true,
'review_status': 'not_reviewed',
'review_status_label': _gettext('Not Reviewed'),
'reasons': reasons
});
}
}
};
this.updateReviewers = function(repo_name, pull_request_id){
var postData = '_method=put&' + $('#reviewers input').serialize();
var postData = $('#reviewers input').serialize();
_updatePullRequest(repo_name, pull_request_id, postData);
};
@ -457,21 +431,30 @@ var ReviewerAutoComplete = function(inputId) {
formatResult: autocompleteFormatResult,
lookupFilter: autocompleteFilterResult,
onSelect: function(element, data) {
var mandatory = false;
var reasons = [_gettext('added manually by "{0}"').format(templateContext.rhodecode_user.username)];
// add whole user groups
if (data.value_type == 'user_group') {
reasons.push(_gettext('member of "{0}"').format(data.value_display));
$.each(data.members, function(index, member_data) {
reviewersController.addReviewMember(
member_data.id, member_data.first_name, member_data.last_name,
member_data.username, member_data.icon_link, reasons);
var reviewer = member_data;
reviewer['user_id'] = member_data['id'];
reviewer['gravatar_link'] = member_data['icon_link'];
reviewer['user_link'] = member_data['profile_link'];
reviewer['rules'] = [];
reviewersController.addReviewMember(reviewer, reasons, mandatory);
})
} else {
reviewersController.addReviewMember(
data.id, data.first_name, data.last_name,
data.username, data.icon_link, reasons);
}
// add single user
else {
var reviewer = data;
reviewer['user_id'] = data['id'];
reviewer['gravatar_link'] = data['icon_link'];
reviewer['user_link'] = data['profile_link'];
reviewer['rules'] = [];
reviewersController.addReviewMember(reviewer, reasons, mandatory);
}
$(inputId).val('');

View file

@ -1,6 +1,8 @@
## -*- coding: utf-8 -*-
<%inherit file="root.mako"/>
<%include file="/ejs_templates/templates.html"/>
<div class="outerwrapper">
<!-- HEADER -->
<div class="header">

View file

@ -0,0 +1,109 @@
<%text>
<div style="display: none">
<script id="ejs_gravatarWithUser" type="text/template" class="ejsTemplate">
<%
if (size > 16) {
var gravatar_class = 'gravatar gravatar-large';
} else {
var gravatar_class = 'gravatar';
}
%>
<%
if (show_disabled) {
var user_cls = 'user user-disabled';
} else {
var user_cls = 'user';
}
%>
<div class="rc-user">
<img class="<%= gravatar_class %>" src="<%- gravatar_url -%>" height="<%= size %>" width="<%= size %>">
<span class="<%= user_cls %>"> <%- user_link -%> </span>
</div>
</script>
<script id="ejs_reviewMemberEntry" type="text/template" class="ejsTemplate">
<li id="reviewer_<%= member.user_id %>" class="reviewer_entry">
<div class="reviewers_member">
<div class="reviewer_status tooltip" title="<%= review_status_label %>">
<div class="flag_status <%= review_status %> pull-left reviewer_member_status"></div>
</div>
<div id="reviewer_<%= member.user_id %>_name" class="reviewer_name">
<% if (mandatory) { %>
<div class="reviewer_member_mandatory tooltip" title="Mandatory reviewer">
<i class="icon-lock"></i>
</div>
<% } %>
<%if (member.user_group && member.user_group.vote_rule) {%>
<div style="float:right">
<%if (member.user_group.vote_rule == -1) {%>
Min votes: ALL
<%} else {%>
Min votes: <%= member.user_group.vote_rule %>
<%}%>
</div>
<%}%>
<%-
renderTemplate('gravatarWithUser', {
'size': 16,
'show_disabled': false,
'user_link': member.user_link,
'gravatar_url': member.gravatar_link
})
%>
</div>
<input type="hidden" name="__start__" value="reviewer:mapping">
<input type="hidden" name="__start__" value="reasons:sequence">
<% for (var i = 0; i < reasons.length; i++) { %>
<% var reason = reasons[i] %>
<div class="reviewer_reason">- <%= reason %></div>
<input type="hidden" name="reason" value="<%= reason %>">
<% } %>
<input type="hidden" name="__end__" value="reasons:sequence">
<input type="hidden" name="__start__" value="rules:sequence">
<% for (var i = 0; i < member.rules.length; i++) { %>
<% var rule = member.rules[i] %>
<input type="hidden" name="rule_id" value="<%= rule %>">
<% } %>
<input type="hidden" name="__end__" value="rules:sequence">
<input id="reviewer_<%= member.user_id %>_input" type="hidden" value="<%= member.user_id %>" name="user_id" />
<input type="hidden" name="mandatory" value="<%= mandatory %>"/>
<input type="hidden" name="__end__" value="reviewer:mapping">
<% if (mandatory) { %>
<div class="reviewer_member_mandatory_remove" style="visibility: hidden;">
<i class="icon-remove-sign"></i>
</div>
<% } else { %>
<% if (allowed_to_update) { %>
<div class="reviewer_member_remove action_button" onclick="reviewersController.removeReviewMember(<%= member.user_id %>, true)" style="visibility: hidden;">
<i class="icon-remove-sign" ></i>
</div>
<% } %>
<% } %>
</div>
</li>
</script>
</div>
<script>
// registers the templates into global cache
registerTemplates();
</script>
</%text>

View file

@ -432,7 +432,7 @@ $(function(){
// generate new DESC of target repo displayed next to select
var prLink = pyroutes.url('pullrequest_new', {'repo_name': repoData['name']});
$('#target_repo_desc').html(
"<strong>${_('Target repository')}</strong>: {0}. <a href=\"{1}\">Use as source</a>".format(repoData['description'], prLink)
"<strong>${_('Target repository')}</strong>: {0}. <a href=\"{1}\">Switch base, and use as source.</a>".format(repoData['description'], prLink)
);
// generate dynamic select2 for refs.

View file

@ -340,58 +340,42 @@
## REVIEWERS
<div class="reviewers-title block-right">
<div class="pr-details-title">
${_('Pull request reviewers')} / <a href="#toggleReasons" onclick="$('.reviewer_reason').toggle(); return false">${_('show reasons')}</a>
${_('Pull request reviewers')}
%if c.allowed_to_update:
<span id="open_edit_reviewers" class="block-right action_button last-item">${_('Edit')}</span>
%endif
</div>
</div>
<div id="reviewers" class="block-right pr-details-content reviewers">
## members goes here !
## members redering block
<input type="hidden" name="__start__" value="review_members:sequence">
<ul id="review_members" class="group_members">
%for member,reasons,mandatory,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.commit_status_lbl(status[0][1].status if status else 'not_reviewed'))}">
<div class="${'flag_status %s' % (status[0][1].status if status else 'not_reviewed')} pull-left reviewer_member_status"></div>
</div>
<div id="reviewer_${member.user_id}_name" class="reviewer_name">
${self.gravatar_with_user(member.email, 16)}
</div>
<input type="hidden" name="__start__" value="reviewer:mapping">
<input type="hidden" name="__start__" value="reasons:sequence">
% if reasons:
<div class="reviewer_reason_container">
%for reason in reasons:
<div class="reviewer_reason" style="display: none">- ${reason}</div>
<input type="hidden" name="reason" value="${reason}">
%endfor
</div>
% endif
<input type="hidden" name="__end__" value="reasons:sequence">
<input id="reviewer_${member.user_id}_input" type="hidden" value="${member.user_id}" name="user_id" />
<input type="hidden" name="mandatory" value="${mandatory}"/>
<input type="hidden" name="__end__" value="reviewer:mapping">
% if mandatory:
<div class="reviewer_member_mandatory_remove">
<i class="icon-remove-sign"></i>
</div>
<div class="reviewer_member_mandatory">
<i class="icon-lock" title="${h.tooltip(_('Mandatory reviewer'))}"></i>
</div>
% else:
%if c.allowed_to_update:
<div class="reviewer_member_remove action_button" onclick="reviewersController.removeReviewMember(${member.user_id}, true)" style="visibility: hidden;">
<i class="icon-remove-sign" ></i>
</div>
%endif
% endif
</div>
</li>
%endfor
% for review_obj, member, reasons, mandatory, status in c.pull_request_reviewers:
<script>
var member = ${h.json.dumps(h.reviewer_as_json(member, reasons=reasons, mandatory=mandatory, user_group=review_obj.rule_user_group_data()))|n};
var status = "${(status[0][1].status if status else 'not_reviewed')}";
var status_lbl = "${h.commit_status_lbl(status[0][1].status if status else 'not_reviewed')}";
var allowed_to_update = ${h.json.dumps(c.allowed_to_update)};
var entry = renderTemplate('reviewMemberEntry', {
'member': member,
'mandatory': member.mandatory,
'reasons': member.reasons,
'allowed_to_update': allowed_to_update,
'review_status': status,
'review_status_label': status_lbl,
'user_group': member.user_group
});
$('#review_members').append(entry)
</script>
% endfor
</ul>
<input type="hidden" name="__end__" value="review_members:sequence">
## end members redering block
%if not c.pull_request.is_closed():
<div id="add_reviewer" class="ac" style="display: none;">
@ -693,7 +677,7 @@
editButton: $('#open_edit_reviewers'),
closeButton: $('#close_edit_reviewers'),
addButton: $('#add_reviewer'),
removeButtons: $('.reviewer_member_remove,.reviewer_member_mandatory_remove,.reviewer_member_mandatory'),
removeButtons: $('.reviewer_member_remove,.reviewer_member_mandatory_remove'),
init: function() {
var self = this;

View file

@ -0,0 +1,127 @@
# -*- coding: utf-8 -*-
# Copyright (C) 2010-2017 RhodeCode GmbH
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License, version 3
# (only), as published by the Free Software Foundation.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
# This program is dual-licensed. If you wish to learn more about the
# RhodeCode Enterprise Edition, including its added features, Support services,
# and proprietary license terms, please see https://rhodecode.com/licenses/
import pytest
import random
from mock import Mock, MagicMock
from rhodecode.model import db
from rhodecode.model.changeset_status import ChangesetStatusModel
status_approved = db.ChangesetStatus.STATUS_APPROVED
status_rejected = db.ChangesetStatus.STATUS_REJECTED
status_under_review = db.ChangesetStatus.STATUS_UNDER_REVIEW
pytestmark = [
pytest.mark.backends("git", "hg"),
]
class ReviewerMock(object):
def __init__(self, reviewer_def):
self.reviewer_def = reviewer_def
def rule_user_group_data(self):
return {'vote_rule': self.reviewer_def['vote_rule']}
class MemberMock(object):
def __init__(self, reviewer_def):
self.reviewer_def = reviewer_def
self.user_id = random.randint(1, 1024)
class Statuses(object):
def __init__(self, member_status):
self.member_status = member_status
def get_statuses(self):
if not self.member_status:
return []
ver = 1
latest = MagicMock(status=self.member_status)
return [
[ver, latest]
]
@pytest.mark.parametrize("reviewers_def, expected_votes", [
# empty values
({},
[]),
# 3 members, 1 votes approved, 2 approvals required
({'members': [status_approved, None, None], 'vote_rule':2},
[status_approved, status_under_review, status_under_review]),
# 3 members, 2 approvals required
({'members': [status_approved, status_approved, None], 'vote_rule': 2},
[status_approved, status_approved, status_approved]),
# 3 members, 3 approvals required
({'members': [status_approved, status_approved, None], 'vote_rule': 3},
[status_approved, status_approved, status_under_review]),
# 3 members, 1 votes approved, 2 approvals required
({'members': [status_approved, status_approved, status_rejected], 'vote_rule': 2},
[status_approved, status_approved, status_approved]),
# 2 members, 1 votes approved, ALL approvals required
({'members': [status_approved, None,], 'vote_rule': -1},
[status_approved, status_under_review]),
# 4 members, 2 votes approved, 2 rejected, 3 approvals required
({'members': [status_approved, status_rejected, status_approved, status_rejected], 'vote_rule': 3},
[status_approved, status_rejected, status_approved, status_rejected]),
# 2 members, ALL approvals required
({'members': [status_approved, status_approved], 'vote_rule': -1},
[status_approved, status_approved]),
# 3 members, 4 approvals required
({'members': [status_approved, None, None], 'vote_rule': 4},
[status_approved, status_under_review, status_under_review]),
# 4 members, 3 approvals required
({'members': [status_approved, status_approved, status_rejected, status_approved], 'vote_rule': 3},
[status_approved, status_approved, status_approved, status_approved]),
# 4 members, 3 approvals required
({'members': [status_rejected, status_rejected, status_approved, status_approved], 'vote_rule': 3},
[status_rejected, status_rejected, status_approved, status_approved]),
])
def test_calculate_group_vote(reviewers_def, expected_votes):
reviewers_data = []
for member_status in reviewers_def.get('members', []):
mandatory_flag = True
reviewers_data.append((
ReviewerMock(reviewers_def),
MemberMock(reviewers_def),
'Test Reason',
mandatory_flag,
Statuses(member_status).get_statuses()
))
votes = ChangesetStatusModel().calculate_group_vote(123, reviewers_data)
assert votes == expected_votes

View file

@ -119,7 +119,7 @@ class TestPullRequestModel(object):
def test_get_awaiting_my_review(self, pull_request):
PullRequestModel().update_reviewers(
pull_request, [(pull_request.author, ['author'], False)],
pull_request, [(pull_request.author, ['author'], False, [])],
pull_request.author)
prs = PullRequestModel().get_awaiting_my_review(
pull_request.target_repo, user_id=pull_request.author.user_id)
@ -128,7 +128,7 @@ class TestPullRequestModel(object):
def test_count_awaiting_my_review(self, pull_request):
PullRequestModel().update_reviewers(
pull_request, [(pull_request.author, ['author'], False)],
pull_request, [(pull_request.author, ['author'], False, [])],
pull_request.author)
pr_count = PullRequestModel().count_awaiting_my_review(
pull_request.target_repo, user_id=pull_request.author.user_id)

View file

@ -41,7 +41,8 @@ class TestGetUsers(object):
user_util.create_user(active=is_active, lastname='Fake user')
with mock.patch('rhodecode.lib.helpers.gravatar_url'):
users = UserModel().get_users()
with mock.patch('rhodecode.lib.helpers.link_to_user'):
users = UserModel().get_users()
fake_users = [u for u in users if u['last_name'] == 'Fake user']
assert len(fake_users) == 2
@ -61,8 +62,9 @@ class TestGetUsers(object):
active=True, lastname=u'Fake {} user'.format(keyword))
with mock.patch('rhodecode.lib.helpers.gravatar_url'):
keyword = keywords[1].lower()
users = UserModel().get_users(name_contains=keyword)
with mock.patch('rhodecode.lib.helpers.link_to_user'):
keyword = keywords[1].lower()
users = UserModel().get_users(name_contains=keyword)
fake_users = [u for u in users if u['last_name'].startswith('Fake')]
assert len(fake_users) == 2
@ -80,7 +82,8 @@ class TestGetUsers(object):
keyword = keywords[1].lower()
with mock.patch('rhodecode.lib.helpers.gravatar_url'):
users = UserModel().get_users(name_contains=keyword)
with mock.patch('rhodecode.lib.helpers.link_to_user'):
users = UserModel().get_users(name_contains=keyword)
fake_users = [u for u in users if u['last_name'].startswith('Fake')]
assert len(fake_users) == 2
@ -95,7 +98,8 @@ class TestGetUsers(object):
user_filter = created_users[-1].username[-2:]
with mock.patch('rhodecode.lib.helpers.gravatar_url'):
users = UserModel().get_users(name_contains=user_filter)
with mock.patch('rhodecode.lib.helpers.link_to_user'):
users = UserModel().get_users(name_contains=user_filter)
fake_users = [u for u in users if u['last_name'].startswith('Fake')]
assert len(fake_users) == 1
@ -108,7 +112,8 @@ class TestGetUsers(object):
active=True, lastname='Fake user'))
with mock.patch('rhodecode.lib.helpers.gravatar_url'):
users = UserModel().get_users(name_contains='Fake', limit=3)
with mock.patch('rhodecode.lib.helpers.link_to_user'):
users = UserModel().get_users(name_contains='Fake', limit=3)
fake_users = [u for u in users if u['last_name'].startswith('Fake')]
assert len(fake_users) == 3

View file

@ -995,8 +995,8 @@ class PRTestUtility(object):
def _get_reviewers(self):
return [
(TEST_USER_REGULAR_LOGIN, ['default1'], False),
(TEST_USER_REGULAR2_LOGIN, ['default2'], False),
(TEST_USER_REGULAR_LOGIN, ['default1'], False, []),
(TEST_USER_REGULAR2_LOGIN, ['default2'], False, []),
]
def update_source_repository(self, head=None):