pull-request: extended default reviewers functionality.

- added mandatory reviewers JS logic Fixes:  #4219
- added option to forbid author to be a reviewer via JS/Frontend checks. Fixes  #5190
- added option to forbid adding new reviewers except those defined. Fixes  #5208

rhodecode/apps/repository/tests/test_pull_requests_list.py

@@ -0,0 +1,83 b''
+# -*- 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
+from rhodecode.model.db import Repository
+
+
+def route_path(name, params=None, **kwargs):
+    import urllib
+
+    base_url = {
+        'pullrequest_show_all': '/{repo_name}/pull-request',
+        'pullrequest_show_all_data': '/{repo_name}/pull-request-data',
+    }[name].format(**kwargs)
+
+    if params:
+        base_url = '{}?{}'.format(base_url, urllib.urlencode(params))
+    return base_url
+
+
[e-mail]("git", "hg")
[e-mail]('autologin_user', 'app')
+class TestPullRequestList(object):
+
+    @pytest.mark.parametrize('params, expected_title', [
+        ({'source': 0, 'closed': 1}, 'Closed Pull Requests'),
+        ({'source': 0, 'my': 1}, 'opened by me'),
+        ({'source': 0, 'awaiting_review': 1}, 'awaiting review'),
+        ({'source': 0, 'awaiting_my_review': 1}, 'awaiting my review'),
+        ({'source': 1}, 'Pull Requests from'),
+    ])
+    def test_showing_list_page(self, backend, pr_util, params, expected_title):
+        pull_request = pr_util.create_pull_request()
+
+        response = self.app.get(
+            route_path('pullrequest_show_all',
+                       repo_name=pull_request.target_repo.repo_name,
+                       params=params))
+
+        assert_response = response.assert_response()
+        assert_response.element_equals_to('.panel-title', expected_title)
+        element = assert_response.get_element('.panel-title')
+        element_text = assert_response._element_to_string(element)
+
+    def test_showing_list_page_data(self, backend, pr_util, xhr_header):
+        pull_request = pr_util.create_pull_request()
+        response = self.app.get(
+            route_path('pullrequest_show_all_data',
+                       repo_name=pull_request.target_repo.repo_name),
+            extra_environ=xhr_header)
+
+        assert response.json['recordsTotal'] == 1
+        assert response.json['data'][0]['description'] == 'Description'
+
+    def test_description_is_escaped_on_index_page(self, backend, pr_util, xhr_header):
+        xss_description = "<script>alert('Hi!')</script>"
+        pull_request = pr_util.create_pull_request(description=xss_description)
+
+        response = self.app.get(
+            route_path('pullrequest_show_all_data',
+                       repo_name=pull_request.target_repo.repo_name),
+            extra_environ=xhr_header)
+
+        assert response.json['recordsTotal'] == 1
+        assert response.json['data'][0]['description'] == \
+               "&lt;script&gt;alert(&#39;Hi!&#39;)&lt;/script&gt;"

rhodecode/apps/repository/utils.py

@@ -0,0 +1,76 b''
+# -*- coding: utf-8 -*-
+
+# Copyright (C) 2016-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/
+
+from rhodecode.lib import helpers as h
+from rhodecode.lib.utils2 import safe_int
+
+
+def reviewer_as_json(user, reasons, mandatory):
+    """
+    Returns json struct of a reviewer for frontend
+
+    :param user: the reviewer
+    :param reasons: list of strings of why they are reviewers
+    :param mandatory: bool, to set user as mandatory
+    """
+
+    return {
+        'user_id': user.user_id,
+        'reasons': reasons,
+        'mandatory': mandatory,
+        'username': user.username,
+        'firstname': user.firstname,
+        'lastname': user.lastname,
+        'gravatar_link': h.gravatar_url(user.email, 14),
+    }
+
+
+def get_default_reviewers_data(
+        current_user, source_repo, source_commit, target_repo, target_commit):
+
+    """ Return json for default reviewers of a repository """
+
+    reasons = ['Default reviewer', 'Repository owner']
+    default = reviewer_as_json(
+        user=current_user, reasons=reasons, mandatory=False)
+
+    return {
+        'api_ver': 'v1',  # define version for later possible schema upgrade
+        'reviewers': [default],
+        'rules': {},
+        'rules_data': {},
+    }
+
+
+def validate_default_reviewers(review_members, reviewer_rules):
+    """
+    Function to validate submitted reviewers against the saved rules
+
+    """
+    reviewers = []
+    reviewer_by_id = {}
+    for r in review_members:
+        reviewer_user_id = safe_int(r['user_id'])
+        entry = (reviewer_user_id, r['reasons'], r['mandatory'])
+
+        reviewer_by_id[reviewer_user_id] = entry
+        reviewers.append(entry)
+
+    return reviewers

rhodecode/lib/dbmigrate/versions/074_version_4_8_0.py

@@ -0,0 +1,37 b''
+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_7_0_1 as db
+
+    repo_review_rule_table = db.RepoReviewRule.__table__
+
+    forbid_author_to_review = Column(
+        "forbid_author_to_review", Boolean(), nullable=True, default=False)
+    forbid_author_to_review.create(table=repo_review_rule_table)
+
+    forbid_adding_reviewers = Column(
+        "forbid_adding_reviewers", Boolean(), nullable=True, default=False)
+    forbid_adding_reviewers.create(table=repo_review_rule_table)
+
+    fixups(db, meta.Session)
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+
+def fixups(models, _SESSION):
+    pass

rhodecode/lib/dbmigrate/versions/075_version_4_8_0.py

@@ -0,0 +1,38 b''
+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_7_0_1 as db
+
+    repo_review_rule_user_table = db.RepoReviewRuleUser.__table__
+    repo_review_rule_user_group_table = db.RepoReviewRuleUserGroup.__table__
+
+    mandatory_user = Column(
+        "mandatory", Boolean(), nullable=True, default=False)
+    mandatory_user.create(table=repo_review_rule_user_table)
+
+    mandatory_user_group = Column(
+        "mandatory", Boolean(), nullable=True, default=False)
+    mandatory_user_group.create(table=repo_review_rule_user_group_table)
+
+    fixups(db, meta.Session)
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+
+def fixups(models, _SESSION):
+    pass

rhodecode/lib/dbmigrate/versions/076_version_4_8_0.py

@@ -0,0 +1,33 b''
+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_7_0_1 as db
+
+    pull_request_reviewers = db.PullRequestReviewers.__table__
+
+    mandatory = Column(
+        "mandatory", Boolean(), nullable=True, default=False)
+    mandatory.create(table=pull_request_reviewers)
+
+    fixups(db, meta.Session)
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+
+def fixups(models, _SESSION):
+    pass

rhodecode/lib/dbmigrate/versions/077_version_4_8_0.py

@@ -0,0 +1,40 b''
+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_7_0_1 as db
+
+    pull_request = db.PullRequest.__table__
+    pull_request_version = db.PullRequestVersion.__table__
+
+    reviewer_data_1 = Column(
+        'reviewer_data_json',
+        db.JsonType(dialect_map=dict(mysql=UnicodeText(16384))))
+    reviewer_data_1.create(table=pull_request)
+
+    reviewer_data_2 = Column(
+        'reviewer_data_json',
+        db.JsonType(dialect_map=dict(mysql=UnicodeText(16384))))
+    reviewer_data_2.create(table=pull_request_version)
+
+    fixups(db, meta.Session)
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+
+def fixups(models, _SESSION):
+    pass

rhodecode/model/validation_schema/schemas/reviewer_schema.py

@@ -0,0 +1,34 b''
+# -*- coding: utf-8 -*-
+
+# Copyright (C) 2016-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 colander
+from rhodecode.model.validation_schema import validators, preparers, types
+
+
+class ReviewerSchema(colander.MappingSchema):
+    username = colander.SchemaNode(types.StrOrIntType())
+    reasons = colander.SchemaNode(colander.List(), missing=[])
+    mandatory = colander.SchemaNode(colander.Boolean(), missing=False)
+
+
+class ReviewerListSchema(colander.SequenceSchema):
+    reviewers = ReviewerSchema()
+
+

rhodecode/__init__.py

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

rhodecode/api/tests/test_create_pull_request.py

@@ -98,7 +98,12 b' class TestCreatePullRequestApi(object):'
     def test_create_with_reviewers_specified_by_names(
             self, backend, no_notifications):
         data = self._prepare_data(backend)
-        reviewers = [TEST_USER_REGULAR_LOGIN, TEST_USER_ADMIN_LOGIN]
+        reviewers = [
+            {'username': TEST_USER_REGULAR_LOGIN,
+             'reasons': ['added manually']},
+            {'username': TEST_USER_ADMIN_LOGIN,
+             'reasons': ['added manually']},
+        ]
         data['reviewers'] = reviewers
         id_, params = build_data(
             self.apikey_regular, 'create_pull_request', **data)
@@ -110,16 +115,26 b' class TestCreatePullRequestApi(object):'
         assert result['result']['msg'] == expected_message
         pull_request_id = result['result']['pull_request_id']
         pull_request = PullRequestModel().get(pull_request_id)
-        actual_reviewers = [r.user.username for r in pull_request.reviewers]
+        actual_reviewers = [
+            {'username': r.user.username,
+             'reasons': ['added manually'],
+             } for r in pull_request.reviewers
+        ]
         assert sorted(actual_reviewers) == sorted(reviewers)
 
     @pytest.mark.backends("git", "hg")
     def test_create_with_reviewers_specified_by_ids(
             self, backend, no_notifications):
         data = self._prepare_data(backend)
-        reviewer_names = [TEST_USER_REGULAR_LOGIN, TEST_USER_ADMIN_LOGIN]
         reviewers = [
-            UserModel().get_by_username(n).user_id for n in reviewer_names]
+            {'username': UserModel().get_by_username(
+                TEST_USER_REGULAR_LOGIN).user_id,
+             'reasons': ['added manually']},
+            {'username': UserModel().get_by_username(
+                TEST_USER_ADMIN_LOGIN).user_id,
+             'reasons': ['added manually']},
+        ]
+
         data['reviewers'] = reviewers
         id_, params = build_data(
             self.apikey_regular, 'create_pull_request', **data)
@@ -131,14 +146,17 b' class TestCreatePullRequestApi(object):'
         assert result['result']['msg'] == expected_message
         pull_request_id = result['result']['pull_request_id']
         pull_request = PullRequestModel().get(pull_request_id)
-        actual_reviewers = [r.user.username for r in pull_request.reviewers]
-        assert sorted(actual_reviewers) == sorted(reviewer_names)
+        actual_reviewers = [
+            {'username': r.user.user_id,
+             'reasons': ['added manually'],
+             } for r in pull_request.reviewers
+        ]
+        assert sorted(actual_reviewers) == sorted(reviewers)
 
     @pytest.mark.backends("git", "hg")
     def test_create_fails_when_the_reviewer_is_not_found(self, backend):
         data = self._prepare_data(backend)
-        reviewers = ['somebody']
-        data['reviewers'] = reviewers
+        data['reviewers'] = [{'username': 'somebody'}]
         id_, params = build_data(
             self.apikey_regular, 'create_pull_request', **data)
         response = api_call(self.app, params)
@@ -153,7 +171,7 b' class TestCreatePullRequestApi(object):'
         id_, params = build_data(
             self.apikey_regular, 'create_pull_request', **data)
         response = api_call(self.app, params)
-        expected_message = 'reviewers should be specified as a list'
+        expected_message = {u'': '"test_regular,test_admin" is not iterable'}
         assert_error(id_, expected_message, given=response.body)
 
     @pytest.mark.backends("git", "hg")

rhodecode/api/tests/test_get_pull_request.py

@@ -109,7 +109,8 b' class TestGetPullRequest(object):'
                     'reasons': reasons,
                     'review_status': st[0][1].status if st else 'not_reviewed',
                 }
-                for reviewer, reasons, st in pull_request.reviewers_statuses()
+                for reviewer, reasons, mandatory, st in
+                pull_request.reviewers_statuses()
             ]
         }
         assert_ok(id_, expected, response.body)

rhodecode/api/tests/test_update_pull_request.py

@@ -119,20 +119,28 b' class TestUpdatePullRequest(object):'
 
     @pytest.mark.backends("git", "hg")
     def test_api_update_change_reviewers(
-            self, pr_util, silence_action_logger, no_notifications):
+            self, user_util, pr_util, silence_action_logger, no_notifications):
+        a = user_util.create_user()
+        b = user_util.create_user()
+        c = user_util.create_user()
+        new_reviewers = [
+            {'username': b.username,'reasons': ['updated via API'],
+             'mandatory':False},
+            {'username': c.username, 'reasons': ['updated via API'],
+             'mandatory':False},
+        ]
 
-        users = [x.username for x in User.get_all()]
-        new = [users.pop(0)]
-        removed = sorted(new)
-        added = sorted(users)
+        added = [b.username, c.username]
+        removed = [a.username]
 
-        pull_request = pr_util.create_pull_request(reviewers=new)
+        pull_request = pr_util.create_pull_request(
+            reviewers=[(a.username, ['added via API'], False)])
 
         id_, params = build_data(
             self.apikey, 'update_pull_request',
             repoid=pull_request.target_repo.repo_name,
             pullrequestid=pull_request.pull_request_id,
-            reviewers=added)
+            reviewers=new_reviewers)
         response = api_call(self.app, params)
         expected = {
             "msg": "Updated pull request `{}`".format(
@@ -152,7 +160,7 b' class TestUpdatePullRequest(object):'
             self.apikey, 'update_pull_request',
             repoid=pull_request.target_repo.repo_name,
             pullrequestid=pull_request.pull_request_id,
-            reviewers=['bad_name'])
+            reviewers=[{'username': 'bad_name'}])
         response = api_call(self.app, params)
 
         expected = 'user `bad_name` does not exist'
@@ -165,7 +173,7 b' class TestUpdatePullRequest(object):'
             self.apikey, 'update_pull_request',
             repoid='fake',
             pullrequestid='fake',
-            reviewers=['bad_name'])
+            reviewers=[{'username': 'bad_name'}])
         response = api_call(self.app, params)
 
         expected = 'repository `fake` does not exist'
@@ -181,7 +189,7 b' class TestUpdatePullRequest(object):'
             self.apikey, 'update_pull_request',
             repoid=pull_request.target_repo.repo_name,
             pullrequestid=999999,
-            reviewers=['bad_name'])
+            reviewers=[{'username': 'bad_name'}])
         response = api_call(self.app, params)
 
         expected = 'pull request `999999` does not exist'

rhodecode/api/views/pull_request_api.py

@@ -21,7 +21,7 b''
 
 import logging
 
-from rhodecode.api import jsonrpc_method, JSONRPCError
+from rhodecode.api import jsonrpc_method, JSONRPCError, JSONRPCValidationError
 from rhodecode.api.utils import (
     has_superadmin_permission, Optional, OAttr, get_repo_or_error,
     get_pull_request_or_error, get_commit_or_error, get_user_or_error,
@@ -34,6 +34,9 b' from rhodecode.model.comment import Comm'
 from rhodecode.model.db import Session, ChangesetStatus, ChangesetComment
 from rhodecode.model.pull_request import PullRequestModel, MergeCheck
 from rhodecode.model.settings import SettingsModel
+from rhodecode.model.validation_schema import Invalid
+from rhodecode.model.validation_schema.schemas.reviewer_schema import \
+    ReviewerListSchema
 
 log = logging.getLogger(__name__)
 
@@ -537,7 +540,7 b' def create_pull_request('
     :type reviewers: Optional(list)
         Accepts username strings or objects of the format:
 
-            {'username': 'nick', 'reasons': ['original author']}
+            {'username': 'nick', 'reasons': ['original author'], 'mandatory': <bool>}
     """
 
     source = get_repo_or_error(source_repo)
@@ -567,20 +570,19 b' def create_pull_request('
         raise JSONRPCError('no common ancestor found')
 
     reviewer_objects = Optional.extract(reviewers) or []
-    if not isinstance(reviewer_objects, list):
-        raise JSONRPCError('reviewers should be specified as a list')
+    if reviewer_objects:
+        schema = ReviewerListSchema()
+        try:
+            reviewer_objects = schema.deserialize(reviewer_objects)
+        except Invalid as err:
+            raise JSONRPCValidationError(colander_exc=err)
 
-    reviewers_reasons = []
+    reviewers = []
     for reviewer_object in reviewer_objects:
-        reviewer_reasons = []
-        if isinstance(reviewer_object, (basestring, int)):
-            reviewer_username = reviewer_object
-        else:
-            reviewer_username = reviewer_object['username']
-            reviewer_reasons = reviewer_object.get('reasons', [])
-
-        user = get_user_or_error(reviewer_username)
-        reviewers_reasons.append((user.user_id, reviewer_reasons))
+        user = get_user_or_error(reviewer_object['username'])
+        reasons = reviewer_object['reasons']
+        mandatory = reviewer_object['mandatory']
+        reviewers.append((user.user_id, reasons, mandatory))
 
     pull_request_model = PullRequestModel()
     pull_request = pull_request_model.create(
@@ -591,7 +593,7 b' def create_pull_request('
         target_ref=full_target_ref,
         revisions=reversed(
             [commit.raw_id for commit in reversed(commit_ranges)]),
-        reviewers=reviewers_reasons,
+        reviewers=reviewers,
         title=title,
         description=Optional.extract(description)
     )
@@ -624,6 +626,10 b' def update_pull_request('
     :type description: Optional(str)
     :param reviewers: Update pull request reviewers list with new value.
     :type reviewers: Optional(list)
+        Accepts username strings or objects of the format:
+
+            {'username': 'nick', 'reasons': ['original author'], 'mandatory': <bool>}
+
     :param update_commits: Trigger update of commits for this pull request
     :type: update_commits: Optional(bool)
     :param close_pull_request: Close this pull request with rejected state
@@ -669,23 +675,21 b' def update_pull_request('
             'pull request `%s` update failed, pull request is closed' % (
                 pullrequestid,))
 
+
     reviewer_objects = Optional.extract(reviewers) or []
-    if not isinstance(reviewer_objects, list):
-        raise JSONRPCError('reviewers should be specified as a list')
+    if reviewer_objects:
+        schema = ReviewerListSchema()
+        try:
+            reviewer_objects = schema.deserialize(reviewer_objects)
+        except Invalid as err:
+            raise JSONRPCValidationError(colander_exc=err)
 
-    reviewers_reasons = []
-    reviewer_ids = set()
+    reviewers = []
     for reviewer_object in reviewer_objects:
-        reviewer_reasons = []
-        if isinstance(reviewer_object, (int, basestring)):
-            reviewer_username = reviewer_object
-        else:
-            reviewer_username = reviewer_object['username']
-            reviewer_reasons = reviewer_object.get('reasons', [])
-
-        user = get_user_or_error(reviewer_username)
-        reviewer_ids.add(user.user_id)
-        reviewers_reasons.append((user.user_id, reviewer_reasons))
+        user = get_user_or_error(reviewer_object['username'])
+        reasons = reviewer_object['reasons']
+        mandatory = reviewer_object['mandatory']
+        reviewers.append((user.user_id, reasons, mandatory))
 
     title = Optional.extract(title)
     description = Optional.extract(description)
@@ -704,9 +708,9 b' def update_pull_request('
         Session().commit()
 
     reviewers_changes = {"added": [], "removed": []}
-    if reviewer_ids:
+    if reviewers:
         added_reviewers, removed_reviewers = \
-            PullRequestModel().update_reviewers(pull_request, reviewers_reasons)
+            PullRequestModel().update_reviewers(pull_request, reviewers)
 
         reviewers_changes['added'] = sorted(
             [get_user_or_error(n).username for n in added_reviewers])

rhodecode/apps/_base/__init__.py

@@ -290,6 +290,13 b' class RepoTypeRoutePredicate(object):'
         else:
             log.warning('Current view is not supported for repo type:%s',
                         rhodecode_db_repo.repo_type)
+            #
+            # h.flash(h.literal(
+            #     _('Action not supported for %s.' % rhodecode_repo.alias)),
+            #     category='warning')
+            # return redirect(
+            #     url('summary_home', repo_name=cls.rhodecode_db_repo.repo_name))
+
             return False
 
 

rhodecode/apps/repository/views/repo_review_rules.py

@@ -23,7 +23,7 b' import logging'
 from pyramid.view import view_config
 
 from rhodecode.apps._base import RepoAppView
-from rhodecode.controllers import utils
+from rhodecode.apps.repository.utils import get_default_reviewers_data
 from rhodecode.lib.auth import LoginRequired, HasRepoPermissionAnyDecorator
 
 log = logging.getLogger(__name__)
@@ -57,13 +57,8 b' class RepoReviewRulesView(RepoAppView):'
         route_name='repo_default_reviewers_data', request_method='GET',
         renderer='json_ext')
     def repo_default_reviewers_data(self):
-        reasons = ['Default reviewer', 'Repository owner']
-        default = utils.reviewer_as_json(
-            user=self.db_repo.user, reasons=reasons, mandatory=False)
+        review_data = get_default_reviewers_data(
+            self.db_repo.user, None, None, None, None)
+        return review_data
 
-        return {
-            'api_ver': 'v1',  # define version for later possible schema upgrade
-            'reviewers': [default],
-            'rules': {},
-            'rules_data': {},
-        }
+

rhodecode/controllers/pullrequests.py

@@ -228,8 +228,6 b' class PullrequestsController(BaseRepoCon'
         target_repo = _form['target_repo']
         target_ref = _form['target_ref']
         commit_ids = _form['revisions'][::-1]
-        reviewers = [
-            (r['user_id'], r['reasons']) for r in _form['review_members']]
 
         # find the ancestor for this pr
         source_db_repo = Repository.get_by_repo_name(_form['source_repo'])
@@ -257,17 +255,29 b' class PullrequestsController(BaseRepoCon'
             )
 
         description = _form['pullrequest_desc']
+
+        get_default_reviewers_data, validate_default_reviewers = \
+            PullRequestModel().get_reviewer_functions()
+
+        # recalculate reviewers logic, to make sure we can validate this
+        reviewer_rules = get_default_reviewers_data(
+            c.rhodecode_user, source_db_repo, source_commit, target_db_repo,
+            target_commit)
+
+        reviewers = validate_default_reviewers(
+            _form['review_members'], reviewer_rules)
+
         try:
             pull_request = PullRequestModel().create(
                 c.rhodecode_user.user_id, source_repo, source_ref, target_repo,
                 target_ref, commit_ids, reviewers, pullrequest_title,
-                description
+                description, reviewer_rules
             )
             Session().commit()
             h.flash(_('Successfully opened new pull request'),
                     category='success')
         except Exception as e:
-            msg = _('Error occurred during sending pull request')
+            msg = _('Error occurred during creation of this pull request.')
             log.exception(msg)
             h.flash(msg, category='error')
             return redirect(url('pullrequest_home', repo_name=repo_name))
@@ -292,7 +302,8 b' class PullrequestsController(BaseRepoCon'
 
             if 'review_members' in controls:
                 self._update_reviewers(
-                    pull_request_id, controls['review_members'])
+                    pull_request_id, controls['review_members'],
+                    pull_request.reviewer_data)
             elif str2bool(request.POST.get('update_commits', 'false')):
                 self._update_commits(pull_request)
             elif str2bool(request.POST.get('close_pull_request', 'false')):
@@ -435,10 +446,20 b' class PullrequestsController(BaseRepoCon'
                 merge_resp.failure_reason)
             h.flash(msg, category='error')
 
-    def _update_reviewers(self, pull_request_id, review_members):
-        reviewers = [
-            (int(r['user_id']), r['reasons']) for r in review_members]
+    def _update_reviewers(self, pull_request_id, review_members, reviewer_rules):
+
+        get_default_reviewers_data, validate_default_reviewers = \
+            PullRequestModel().get_reviewer_functions()
+
+        try:
+            reviewers = validate_default_reviewers(review_members, reviewer_rules)
+        except ValueError as e:
+            log.error('Reviewers Validation:{}'.format(e))
+            h.flash(e, category='error')
+            return
+
         PullRequestModel().update_reviewers(pull_request_id, reviewers)
+        h.flash(_('Pull request reviewers updated.'), category='success')
         Session().commit()
 
     def _reject_close(self, pull_request):
@@ -490,7 +511,8 b' class PullrequestsController(BaseRepoCon'
             _org_pull_request_obj = pull_request_ver.pull_request
             at_version = pull_request_ver.pull_request_version_id
         else:
-            _org_pull_request_obj = pull_request_obj = PullRequest.get_or_404(pull_request_id)
+            _org_pull_request_obj = pull_request_obj = PullRequest.get_or_404(
+                pull_request_id)
 
         pull_request_display_obj = PullRequest.get_pr_display_object(
             pull_request_obj, _org_pull_request_obj)
@@ -597,9 +619,9 b' class PullrequestsController(BaseRepoCon'
             c.allowed_to_comment = False
             c.allowed_to_close = False
         else:
-            c.allowed_to_change_status = PullRequestModel(). \
-                check_user_change_status(pull_request_at_ver, c.rhodecode_user) \
-                                         and not pr_closed
+            can_change_status = PullRequestModel().check_user_change_status(
+                pull_request_at_ver, c.rhodecode_user)
+            c.allowed_to_change_status = can_change_status and not pr_closed
 
             c.allowed_to_update = PullRequestModel().check_user_update(
                 pull_request_latest, c.rhodecode_user) and not pr_closed
@@ -610,6 +632,18 b' class PullrequestsController(BaseRepoCon'
             c.allowed_to_comment = not pr_closed
             c.allowed_to_close = c.allowed_to_merge and not pr_closed
 
+        c.forbid_adding_reviewers = False
+        c.forbid_author_to_review = False
+
+        if pull_request_latest.reviewer_data and \
+                        'rules' in pull_request_latest.reviewer_data:
+            rules = pull_request_latest.reviewer_data['rules'] or {}
+            try:
+                c.forbid_adding_reviewers = rules.get('forbid_adding_reviewers')
+                c.forbid_author_to_review = rules.get('forbid_author_to_review')
+            except Exception:
+                pass
+
         # check merge capabilities
         _merge_check = MergeCheck.validate(
             pull_request_latest, user=c.rhodecode_user)
@@ -724,12 +758,18 b' class PullrequestsController(BaseRepoCon'
                 c.commit_ranges.append(comm)
                 commit_cache[comm.raw_id] = comm
 
+            # Order here matters, we first need to get target, and then
+            # the source
             target_commit = commits_source_repo.get_commit(
                 commit_id=safe_str(target_ref_id))
+
             source_commit = commits_source_repo.get_commit(
                 commit_id=safe_str(source_ref_id))
+
         except CommitDoesNotExistError:
-            pass
+            log.warning(
+                'Failed to get commit from `{}` repo'.format(
+                    commits_source_repo), exc_info=True)
         except RepositoryRequirementError:
             log.warning(
                 'Failed to get all required data from repo', exc_info=True)

rhodecode/controllers/utils.py

@@ -87,23 +87,3 b' def get_commit_from_ref_name(repo, ref_n'
                     '%s "%s" does not exist' % (ref_type, ref_name))
 
     return repo_scm.get_commit(commit_id)
-
-
-def reviewer_as_json(user, reasons, mandatory):
-    """
-    Returns json struct of a reviewer for frontend
-
-    :param user: the reviewer
-    :param reasons: list of strings of why they are reviewers
-    :param mandatory: bool, to set user as mandatory
-    """
-
-    return {
-        'user_id': user.user_id,
-        'reasons': reasons,
-        'mandatory': mandatory,
-        'username': user.username,
-        'firstname': user.firstname,
-        'lastname': user.lastname,
-        'gravatar_link': h.gravatar_url(user.email, 14),
-    }

rhodecode/model/changeset_status.py

@@ -80,7 +80,7 b' class ChangesetStatusModel(BaseModel):'
         """
         votes = defaultdict(int)
         reviewers_number = len(statuses_by_reviewers)
-        for user, reasons, statuses in statuses_by_reviewers:
+        for user, reasons, mandatory, statuses in statuses_by_reviewers:
             if statuses:
                 ver, latest = statuses[0]
                 votes[latest.status] += 1
@@ -248,13 +248,14 b' class ChangesetStatusModel(BaseModel):'
         for o in pull_request.reviewers:
             if not o.user:
                 continue
-            st = commit_statuses.get(o.user.username, None)
-            if st:
-                st = [(x, list(y)[0])
-                      for x, y in (itertools.groupby(sorted(st, key=version),
-                                                     version))]
+            statuses = commit_statuses.get(o.user.username, None)
+            if statuses:
+                statuses = [(x, list(y)[0])
+                      for x, y in (itertools.groupby(
+                        sorted(statuses, key=version),version))]
 
-            pull_request_reviewers.append((o.user, o.reasons, st))
+            pull_request_reviewers.append(
+                (o.user, o.reasons, o.mandatory, statuses))
         return pull_request_reviewers
 
     def calculated_review_status(self, pull_request, reviewers_statuses=None):

rhodecode/model/db.py

@@ -3229,6 +3229,14 b' class _PullRequestBase(BaseModel):'
     _last_merge_status = Column('merge_status', Integer(), nullable=True)
     merge_rev = Column('merge_rev', String(40), nullable=True)
 
+    reviewer_data = Column(
+        'reviewer_data_json', MutationObj.as_mutable(
+            JsonType(dialect_map=dict(mysql=UnicodeText(16384)))))
+
+    @property
+    def reviewer_data_json(self):
+        return json.dumps(self.reviewer_data)
+
     @hybrid_property
     def revisions(self):
         return self._revisions.split(':') if self._revisions else []
@@ -3348,7 +3356,8 b' class _PullRequestBase(BaseModel):'
                     'reasons': reasons,
                     'review_status': st[0][1].status if st else 'not_reviewed',
                 }
-                for reviewer, reasons, st in pull_request.reviewers_statuses()
+                for reviewer, reasons, mandatory, st in
+                pull_request.reviewers_statuses()
             ]
         }
 
@@ -3438,6 +3447,8 b' class PullRequest(Base, _PullRequestBase'
         attrs.revisions = pull_request_obj.revisions
 
         attrs.shadow_merge_ref = org_pull_request_obj.shadow_merge_ref
+        attrs.reviewer_data = org_pull_request_obj.reviewer_data
+        attrs.reviewer_data_json = org_pull_request_obj.reviewer_data_json
 
         return PullRequestDisplay(attrs, internal=internal_methods)
 
@@ -3516,11 +3527,6 b' class PullRequestReviewers(Base, BaseMod'
          'mysql_charset': 'utf8', 'sqlite_autoincrement': True},
     )
 
-    def __init__(self, user=None, pull_request=None, reasons=None):
-        self.user = user
-        self.pull_request = pull_request
-        self.reasons = reasons or []
-
     @hybrid_property
     def reasons(self):
         if not self._reasons:
@@ -3545,7 +3551,7 b' class PullRequestReviewers(Base, BaseMod'
     _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')
 
@@ -3849,14 +3855,17 b' 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)
+    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)
+    mandatory = Column("mandatory", Boolean(), nullable=False, default=False)
     user = relationship('User')
 
+    def rule_data(self):
+        return {
+            'mandatory': self.mandatory
+        }
+
 
 class RepoReviewRuleUserGroup(Base, BaseModel):
     __tablename__ = 'repo_review_rules_users_groups'
@@ -3864,14 +3873,17 b' class RepoReviewRuleUserGroup(Base, Base'
         {'extend_existing': True, 'mysql_engine': 'InnoDB',
          'mysql_charset': 'utf8', 'sqlite_autoincrement': True,}
     )
-    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)
+    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)
     users_group = relationship('UserGroup')
 
+    def rule_data(self):
+        return {
+            'mandatory': self.mandatory
+        }
+
 
 class RepoReviewRule(Base, BaseModel):
     __tablename__ = 'repo_review_rules'
@@ -3886,13 +3898,13 b' class RepoReviewRule(Base, BaseModel):'
         "repo_id", Integer(), ForeignKey('repositories.repo_id'))
     repo = relationship('Repository', backref='review_rules')
 
-    _branch_pattern = Column("branch_pattern", UnicodeText().with_variant(UnicodeText(255), 'mysql'),
-        default=u'*') # glob
-    _file_pattern = Column("file_pattern", UnicodeText().with_variant(UnicodeText(255), 'mysql'),
-        default=u'*') # glob
-
-    use_authors_for_review = Column("use_authors_for_review", Boolean(),
-        nullable=False, default=False)
+    _branch_pattern = Column("branch_pattern", UnicodeText().with_variant(UnicodeText(255), 'mysql'), default=u'*')  # glob
+    _file_pattern = Column("file_pattern", UnicodeText().with_variant(UnicodeText(255), 'mysql'), default=u'*')  # glob
+
+    use_authors_for_review = Column("use_authors_for_review", Boolean(), nullable=False, default=False)
+    forbid_author_to_review = Column("forbid_author_to_review", Boolean(), nullable=False, default=False)
+    forbid_adding_reviewers = Column("forbid_adding_reviewers", Boolean(), nullable=False, default=False)
+
     rule_users = relationship('RepoReviewRuleUser')
     rule_user_groups = relationship('RepoReviewRuleUserGroup')
 
@@ -3948,16 +3960,26 b' class RepoReviewRule(Base, BaseModel):'
     def review_users(self):
         """ Returns the users which this rule applies to """
 
-        users = set()
-        users |= set([
-            rule_user.user for rule_user in self.rule_users
-            if rule_user.user.active])
-        users |= set(
-            member.user
-            for rule_user_group in self.rule_user_groups
-            for member in rule_user_group.users_group.members
-            if member.user.active
-        )
+        users = collections.OrderedDict()
+
+        for rule_user in self.rule_users:
+            if rule_user.user.active:
+                if rule_user.user not in users:
+                    users[rule_user.user.username] = {
+                        'user': rule_user.user,
+                        'source': 'user',
+                        'data': rule_user.rule_data()
+                    }
+
+        for rule_user_group in self.rule_user_groups:
+            for member in rule_user_group.users_group.members:
+                if member.user.active:
+                    users[member.user.username] = {
+                        'user': member.user,
+                        'source': 'user_group',
+                        'data': rule_user_group.rule_data()
+                    }
+
         return users
 
     def __repr__(self):

rhodecode/model/forms.py

@@ -535,12 +535,13 b' def PullRequestForm(repo_id):'
     class ReviewerForm(formencode.Schema):
         user_id = v.Int(not_empty=True)
         reasons = All()
+        mandatory = v.StringBoolean()
 
     class _PullRequestForm(formencode.Schema):
         allow_extra_fields = True
         filter_extra_fields = True
 
-        user = v.UnicodeString(strip=True, required=True)
+        common_ancestor = v.UnicodeString(strip=True, required=True)
         source_repo = v.UnicodeString(strip=True, required=True)
         source_ref = v.UnicodeString(strip=True, required=True)
         target_repo = v.UnicodeString(strip=True, required=True)

rhodecode/model/pull_request.py

@@ -415,7 +415,9 b' class PullRequestModel(BaseModel):'
             .all()
 
     def create(self, created_by, source_repo, source_ref, target_repo,
-        target_ref, revisions, reviewers, title, description=None):
+               target_ref, revisions, reviewers, title, description=None,
+               reviewer_data=None):
+
         created_by_user = self._get_user(created_by)
         source_repo = self._get_repo(source_repo)
         target_repo = self._get_repo(target_repo)
@@ -429,6 +431,7 b' class PullRequestModel(BaseModel):'
         pull_request.title = title
         pull_request.description = description
         pull_request.author = created_by_user
+        pull_request.reviewer_data = reviewer_data
 
         Session().add(pull_request)
         Session().flush()
@@ -436,15 +439,16 b' class PullRequestModel(BaseModel):'
         reviewer_ids = set()
         # members / reviewers
         for reviewer_object in reviewers:
-            if isinstance(reviewer_object, tuple):
-                user_id, reasons = reviewer_object
-            else:
-                user_id, reasons = reviewer_object, []
+            user_id, reasons, mandatory = reviewer_object
 
             user = self._get_user(user_id)
             reviewer_ids.add(user.user_id)
 
-            reviewer = PullRequestReviewers(user, pull_request, reasons)
+            reviewer = PullRequestReviewers()
+            reviewer.user = user
+            reviewer.pull_request = pull_request
+            reviewer.reasons = reasons
+            reviewer.mandatory = mandatory
             Session().add(reviewer)
 
         # Set approval status to "Under Review" for all commits which are
@@ -763,6 +767,7 b' class PullRequestModel(BaseModel):'
         version._last_merge_status = pull_request._last_merge_status
         version.shadow_merge_ref = pull_request.shadow_merge_ref
         version.merge_rev = pull_request.merge_rev
+        version.reviewer_data = pull_request.reviewer_data
 
         version.revisions = pull_request.revisions
         version.pull_request = pull_request
@@ -903,16 +908,18 b' class PullRequestModel(BaseModel):'
         Update the reviewers in the pull request
 
         :param pull_request: the pr to update
-        :param reviewer_data: list of tuples [(user, ['reason1', 'reason2'])]
+        :param reviewer_data: list of tuples
+            [(user, ['reason1', 'reason2'], mandatory_flag)]
         """
 
-        reviewers_reasons = {}
-        for user_id, reasons in reviewer_data:
+        reviewers = {}
+        for user_id, reasons, mandatory in reviewer_data:
             if isinstance(user_id, (int, basestring)):
                 user_id = self._get_user(user_id).user_id
-            reviewers_reasons[user_id] = reasons
+            reviewers[user_id] = {
+                'reasons': reasons, 'mandatory': mandatory}
 
-        reviewers_ids = set(reviewers_reasons.keys())
+        reviewers_ids = set(reviewers.keys())
         pull_request = self.__get_pull_request(pull_request)
         current_reviewers = PullRequestReviewers.query()\
             .filter(PullRequestReviewers.pull_request ==
@@ -928,8 +935,12 b' class PullRequestModel(BaseModel):'
         for uid in ids_to_add:
             changed = True
             _usr = self._get_user(uid)
-            reasons = reviewers_reasons[uid]
-            reviewer = PullRequestReviewers(_usr, pull_request, reasons)
+            reviewer = PullRequestReviewers()
+            reviewer.user = _usr
+            reviewer.pull_request = pull_request
+            reviewer.reasons = reviewers[uid]['reasons']
+            # NOTE(marcink): mandatory shouldn't be changed now
+            #reviewer.mandatory = reviewers[uid]['reasons']
             Session().add(reviewer)
 
         for uid in ids_to_remove:
@@ -1369,6 +1380,23 b' class PullRequestModel(BaseModel):'
                 action=action, pr_id=pull_request.pull_request_id),
             pull_request.target_repo)
 
+    def get_reviewer_functions(self):
+        """
+        Fetches functions for validation and fetching default reviewers.
+        If available we use the EE package, else we fallback to CE
+        package functions
+        """
+        try:
+            from rc_reviewers.utils import get_default_reviewers_data
+            from rc_reviewers.utils import validate_default_reviewers
+        except ImportError:
+            from rhodecode.apps.repository.utils import \
+                get_default_reviewers_data
+            from rhodecode.apps.repository.utils import \
+                validate_default_reviewers
+
+        return get_default_reviewers_data, validate_default_reviewers
+
 
 class MergeCheck(object):
     """

rhodecode/model/validation_schema/types.py

@@ -190,7 +190,7 b' class UserGroupType(UserOrUserGroupType)'
 
 class StrOrIntType(colander.String):
     def deserialize(self, node, cstruct):
-        if isinstance(node, basestring):
+        if isinstance(cstruct, basestring):
             return super(StrOrIntType, self).deserialize(node, cstruct)
         else:
             return colander.Integer().deserialize(node, cstruct)

rhodecode/public/css/buttons.less

@@ -62,6 +62,10 b' input[type="button"] {'
         text-shadow: none;
     }
 
+    &.no-margin {
+        margin: 0 0 0 0;
+    }
+
 }
 
 
@@ -270,11 +274,17 b' input[type="button"] {'
     font-size: inherit;
     color: @rcblue;
     border: none;
-    .border-radius (0);
+    border-radius: 0;
     background-color: transparent;
 
+    &.last-item {
+        border: none;
+        padding: 0 0 0 0;
+    }
+
     &:last-child {
         border: none;
+        padding: 0 0 0 0;
     }
 
     &:hover {

rhodecode/public/css/deform.less

@@ -12,7 +12,7 b''
 
   .control-label {
     width: 200px;
-    padding: 10px;
+    padding: 10px 0px;
     float: left;
   }
   .control-inputs {

rhodecode/public/css/main.less

@@ -359,9 +359,26 b' ul.auth_plugins {'
     }
 }
 
+.pr-mergeinfo {
+    clear: both;
+    margin: .5em 0;
+
+    input {
+      min-width: 100% !important;
+      padding: 0 !important;
+      border: 0;
+    }
+}
+
 .pr-pullinfo {
     clear: both;
     margin: .5em 0;
+
+    input {
+      min-width: 100% !important;
+      padding: 0 !important;
+      border: 0;
+    }
 }
 
 #pr-title-input {
@@ -1298,6 +1315,11 b' table.integrations {'
     width: 100%;
     margin-bottom: 8px;
 }
+
+.reviewer_entry {
+  min-height: 55px;
+}
+
 .reviewers_member {
     width: 100%;
     overflow: auto;
@@ -1332,6 +1354,8 b' table.integrations {'
     }
 }
 
+.reviewer_member_mandatory,
+.reviewer_member_mandatory_remove,
 .reviewer_member_remove {
     position: absolute;
     right: 0;
@@ -1341,6 +1365,15 b' table.integrations {'
     padding: 0;
     color: black;
 }
+
+.reviewer_member_mandatory_remove {
+  color: @grey4;
+}
+
+.reviewer_member_mandatory {
+  padding-top:20px;
+}
+
 .reviewer_member_status {
     margin-top: 5px;
 }
@@ -1370,6 +1403,11 b' table.integrations {'
 .pr-description {
     white-space:pre-wrap;
 }
+
+.pr-reviewer-rules {
+   padding: 10px 0px 20px 0px;
+}
+
 .group_members {
     margin-top: 0;
     padding: 0;

rhodecode/public/js/src/rhodecode/pullrequests.js

@@ -16,77 +16,327 b''
 // # RhodeCode Enterprise Edition, including its added features, Support services,
 // # and proprietary license terms, please see https://rhodecode.com/licenses/
 
+
+var prButtonLockChecks = {
+  'compare': false,
+  'reviewers': false
+};
+
 /**
- * Pull request reviewers
+ * lock button until all checks and loads are made. E.g reviewer calculation
+ * should prevent from submitting a PR
+ * @param lockEnabled
+ * @param msg
+ * @param scope
  */
-var removeReviewMember = function(reviewer_id, mark_delete){
-    var reviewer = $('#reviewer_{0}'.format(reviewer_id));
+var prButtonLock = function(lockEnabled, msg, scope) {
+      scope = scope || 'all';
+      if (scope == 'all'){
+          prButtonLockChecks['compare'] = !lockEnabled;
+          prButtonLockChecks['reviewers'] = !lockEnabled;
+      } else if (scope == 'compare') {
+          prButtonLockChecks['compare'] = !lockEnabled;
+      } else if (scope == 'reviewers'){
+          prButtonLockChecks['reviewers'] = !lockEnabled;
+      }
+      var checksMeet = prButtonLockChecks.compare && prButtonLockChecks.reviewers;
+      if (lockEnabled) {
+          $('#save').attr('disabled', 'disabled');
+      }
+      else if (checksMeet) {
+          $('#save').removeAttr('disabled');
+      }
 
-    if(typeof(mark_delete) === undefined){
-        mark_delete = false;
-    }
+      if (msg) {
+          $('#pr_open_message').html(msg);
+      }
+};
 
-    if(mark_delete === true){
-        if (reviewer){
-            // now delete the input
-            $('#reviewer_{0} input'.format(reviewer_id)).remove();
-            // mark as to-delete
-            var obj = $('#reviewer_{0}_name'.format(reviewer_id));
-            obj.addClass('to-delete');
-            obj.css({"text-decoration":"line-through", "opacity": 0.5});
-        }
-    }
-    else{
-        $('#reviewer_{0}'.format(reviewer_id)).remove();
-    }
+
+/**
+Generate Title and Description for a PullRequest.
+In case of 1 commits, the title and description is that one commit
+in case of multiple commits, we iterate on them with max N number of commits,
+and build description in a form
+- commitN
+- commitN+1
+...
+
+Title is then constructed from branch names, or other references,
+replacing '-' and '_' into spaces
+
+* @param sourceRef
+* @param elements
+* @param limit
+* @returns {*[]}
+*/
+var getTitleAndDescription = function(sourceRef, elements, limit) {
+  var title = '';
+  var desc = '';
+
+  $.each($(elements).get().reverse().slice(0, limit), function(idx, value) {
+      var rawMessage = $(value).find('td.td-description .message').data('messageRaw');
+      desc += '- ' + rawMessage.split('\n')[0].replace(/\n+$/, "") + '\n';
+  });
+  // only 1 commit, use commit message as title
+  if (elements.length === 1) {
+      title = $(elements[0]).find('td.td-description .message').data('messageRaw').split('\n')[0];
+  }
+  else {
+      // use reference name
+      title = sourceRef.replace(/-/g, ' ').replace(/_/g, ' ').capitalizeFirstLetter();
+  }
+
+  return [title, desc]
 };
 
-var addReviewMember = function(id, fname, lname, nname, gravatar_link, reasons) {
-    var members = $('#review_members').get(0);
-    var reasons_html = '';
-    var reasons_inputs = '';
-    var reasons = reasons || [];
-    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]) + '">';
+
+
+ReviewersController = function () {
+    var self = this;
+    this.$reviewRulesContainer = $('#review_rules');
+    this.$rulesList = this.$reviewRulesContainer.find('.pr-reviewer-rules');
+    this.forbidReviewUsers = undefined;
+    this.$reviewMembers = $('#review_members');
+    this.currentRequest = null;
+
+    this.defaultForbidReviewUsers = function() {
+        return [
+            {'username': 'default',
+             'user_id': templateContext.default_user.user_id}
+        ];
+    };
+
+    this.hideReviewRules = function() {
+        self.$reviewRulesContainer.hide();
+    };
+
+    this.showReviewRules = function() {
+        self.$reviewRulesContainer.show();
+    };
+
+    this.addRule = function(ruleText) {
+        self.showReviewRules();
+        return '<div>- {0}</div>'.format(ruleText)
+    };
+
+    this.loadReviewRules = function(data) {
+        // reset forbidden Users
+        this.forbidReviewUsers = self.defaultForbidReviewUsers();
+
+        // reset state of review rules
+        self.$rulesList.html('');
+
+        if (!data || data.rules === undefined || $.isEmptyObject(data.rules)) {
+            // default rule, case for older repo that don't have any rules stored
+            self.$rulesList.append(
+                self.addRule(
+                    _gettext('All reviewers must vote.'))
+            );
+            return self.forbidReviewUsers
+        }
+
+        if (data.rules.voting !== undefined) {
+            if (data.rules.voting < 0){
+                self.$rulesList.append(
+                    self.addRule(
+                        _gettext('All reviewers must vote.'))
+                )
+            } else if (data.rules.voting === 1) {
+                self.$rulesList.append(
+                    self.addRule(
+                        _gettext('At least {0} reviewer must vote.').format(data.rules.voting))
+                )
+
+            } else {
+                self.$rulesList.append(
+                    self.addRule(
+                        _gettext('At least {0} reviewers must vote.').format(data.rules.voting))
+                )
+            }
+        }
+        if (data.rules.use_code_authors_for_review) {
+            self.$rulesList.append(
+                self.addRule(
+                    _gettext('Reviewers picked from source code changes.'))
+            )
+        }
+        if (data.rules.forbid_adding_reviewers) {
+            $('#add_reviewer_input').remove();
+            self.$rulesList.append(
+                self.addRule(
+                    _gettext('Adding new reviewers is forbidden.'))
+            )
+        }
+        if (data.rules.forbid_author_to_review) {
+            self.forbidReviewUsers.push(data.rules_data.pr_author);
+            self.$rulesList.append(
+                self.addRule(
+                    _gettext('Author is not allowed to be a reviewer.'))
+            )
+        }
+        return self.forbidReviewUsers
+    };
+
+    this.loadDefaultReviewers = function(sourceRepo, sourceRef, targetRepo, targetRef) {
+
+        if (self.currentRequest) {
+            // make sure we cleanup old running requests before triggering this
+            // again
+            self.currentRequest.abort();
         }
-    }
-    var tmpl = '<li id="reviewer_{2}">'+
-       '<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">'+
-      '<div class="reviewer_member_remove action_button" onclick="removeReviewMember({2})">' +
-      '<i class="icon-remove-sign"></i>'+
-        '</div>'+
-    '</div>'+
-    '<input type="hidden" name="__end__" value="reviewer:mapping">'+
-    '</li>' ;
+
+        $('.calculate-reviewers').show();
+        // reset reviewer members
+        self.$reviewMembers.empty();
+
+        prButtonLock(true, null, 'reviewers');
+        $('#user').hide(); // hide user autocomplete before load
+
+        var url = pyroutes.url('repo_default_reviewers_data',
+                {
+                    'repo_name': templateContext.repo_name,
+                    'source_repo': sourceRepo,
+                    'source_ref': sourceRef[2],
+                    'target_repo': targetRepo,
+                    'target_ref': targetRef[2]
+                });
+
+        self.currentRequest = $.get(url)
+            .done(function(data) {
+                self.currentRequest = null;
+
+                // review rules
+                self.loadReviewRules(data);
+
+                for (var i = 0; i < data.reviewers.length; i++) {
+                  var reviewer = data.reviewers[i];
+                  self.addReviewMember(
+                      reviewer.user_id, reviewer.firstname,
+                      reviewer.lastname, reviewer.username,
+                      reviewer.gravatar_link, reviewer.reasons,
+                      reviewer.mandatory);
+                }
+                $('.calculate-reviewers').hide();
+                prButtonLock(false, null, 'reviewers');
+                $('#user').show(); // show user autocomplete after load
+            });
+    };
+
+    // check those, refactor
+    this.removeReviewMember = function(reviewer_id, mark_delete) {
+        var reviewer = $('#reviewer_{0}'.format(reviewer_id));
+
+        if(typeof(mark_delete) === undefined){
+            mark_delete = false;
+        }
+
+        if(mark_delete === true){
+            if (reviewer){
+                // now delete the input
+                $('#reviewer_{0} input'.format(reviewer_id)).remove();
+                // mark as to-delete
+                var obj = $('#reviewer_{0}_name'.format(reviewer_id));
+                obj.addClass('to-delete');
+                obj.css({"text-decoration":"line-through", "opacity": 0.5});
+            }
+        }
+        else{
+            $('#reviewer_{0}'.format(reviewer_id)).remove();
+        }
+    };
+
+    this.addReviewMember = function(id, fname, lname, nname, gravatar_link, reasons, mandatory) {
+        var members = self.$reviewMembers.get(0);
+        var reasons_html = '';
+        var reasons_inputs = '';
+        var reasons = reasons || [];
+        var mandatory = mandatory || false;
 
-    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 = [];
-    var _els = $('#review_members li').toArray();
-    for (el in _els){
-        ids.push(_els[el].id)
-    }
-    if(ids.indexOf('reviewer_'+id) == -1){
-        // only add if it's not there
-        members.innerHTML += element;
-    }
+        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 = [];
+        var _els = self.$reviewMembers.find('li').toArray();
+        for (el in _els){
+            ids.push(_els[el].id)
+        }
+
+        var userAllowedReview = function(userId) {
+            var allowed = true;
+            $.each(self.forbidReviewUsers, function(index, member_data) {
+                if (parseInt(userId) === member_data['user_id']) {
+                    allowed = false;
+                    return false // breaks the loop
+                }
+            });
+            return allowed
+        };
+
+        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) {
+            // only add if it's not there
+            members.innerHTML += element;
+        }
+
+    };
+
+    this.updateReviewers = function(repo_name, pull_request_id){
+        var postData = '_method=put&' + $('#reviewers input').serialize();
+        _updatePullRequest(repo_name, pull_request_id, postData);
+    };
 
 };
 
+
 var _updatePullRequest = function(repo_name, pull_request_id, postData) {
     var url = pyroutes.url(
         'pullrequest_update',
@@ -102,13 +352,6 b' var _updatePullRequest = function(repo_n'
     ajaxPOST(url, postData, success);
 };
 
-var updateReviewers = function(reviewers_ids, repo_name, pull_request_id){
-    if (reviewers_ids === undefined){
-      var postData = '_method=put&' + $('#reviewers input').serialize();
-      _updatePullRequest(repo_name, pull_request_id, postData);
-    }
-};
-
 /**
  * PULL REQUEST reject & close
  */
@@ -207,7 +450,7 b' var ReviewerAutoComplete = function(inpu'
     showNoSuggestionNotice: true,
     tabDisabled: true,
     autoSelectFirst: true,
-    params: { user_id: templateContext.rhodecode_user.user_id, user_groups:true, user_groups_expand:true },
+    params: { user_id: templateContext.rhodecode_user.user_id, user_groups:true, user_groups_expand:true, skip_default_user:true },
     formatResult: autocompleteFormatResult,
     lookupFilter: autocompleteFilterResult,
     onSelect: function(element, data) {
@@ -217,13 +460,15 b' var ReviewerAutoComplete = function(inpu'
             reasons.push(_gettext('member of "{0}"').format(data.value_display));
 
             $.each(data.members, function(index, member_data) {
-                addReviewMember(member_data.id, member_data.first_name, member_data.last_name,
-                                member_data.username, member_data.icon_link, reasons);
+                reviewersController.addReviewMember(
+                    member_data.id, member_data.first_name, member_data.last_name,
+                    member_data.username, member_data.icon_link, reasons);
             })
 
         } else {
-          addReviewMember(data.id, data.first_name, data.last_name,
-                          data.username, data.icon_link, reasons);
+          reviewersController.addReviewMember(
+              data.id, data.first_name, data.last_name,
+              data.username, data.icon_link, reasons);
         }
 
       $(inputId).val('');

rhodecode/templates/admin/repos/repo_edit.mako

@@ -24,7 +24,11 b''
 </%def>
 
 <%def name="main_content()">
-  <%include file="/admin/repos/repo_edit_${c.active}.mako"/>
+  % if hasattr(c, 'repo_edit_template'):
+    <%include file="${c.repo_edit_template}"/>
+  % else:
+    <%include file="/admin/repos/repo_edit_${c.active}.mako"/>
+  % endif
 </%def>
 
 

rhodecode/templates/compare/compare_commits.mako

@@ -6,6 +6,7 b''
     <a href="${h.url('changeset_home', repo_name=c.repo_name, revision=c.ancestor)}">
         ${h.short_id(c.ancestor)}
     </a>. ${_('Compare was calculated based on this shared commit.')}
+    <input id="common_ancestor" type="hidden" name="common_ancestor" value="${c.ancestor}">
 </div>
 %endif
 

rhodecode/templates/forms/sequence.pt

@@ -32,21 +32,21 b''
          tal:attributes="prototype prototype"/>
 
   <div class="panel panel-default">
-    <div class="panel-heading">${title}</div>
     <div class="panel-body">
 
       <div class="deform-seq-container"
            id="${oid}-orderable">
         <div tal:define="subfields [ x[1] for x in subfields ]"
              tal:repeat="subfield subfields"
-             tal:replace="structure subfield.render_template(item_tmpl,
-                                                          parent=field)" />
+             tal:replace="structure subfield.render_template(item_tmpl,parent=field)" />
         <span class="deform-insert-before"
               tal:attributes="
                  min_len min_len;
                  max_len max_len;
                  now_len now_len;
-                 orderable orderable;"></span>
+                 orderable orderable;">
+
+        </span>
       </div>
 
       <div style="clear: both"></div>
@@ -78,12 +78,12 b''
                placeholder: '<span class="glyphicon glyphicon-arrow-right placeholder"></span>',
                onDragStart: function ($item, container, _super) {
                  var offset = $item.offset(),
-                 pointer = container.rootGroup.pointer
+                 pointer = container.rootGroup.pointer;
 
                  adjustment = {
                    left: pointer.left - offset.left,
                    top: pointer.top - offset.top
-                 }
+                 };
 
                  _super($item, container)
                },

rhodecode/templates/forms/sequence_item.pt

@@ -6,6 +6,7 b''
                  oid oid|field.oid"
      class="form-group row deform-seq-item ${field.error and error_class or ''} ${field.widget.item_css_class or ''}"
      i18n:domain="deform">
+
   <div class="deform-seq-item-group">
     <span tal:replace="structure field.serialize(cstruct)"/>
     <tal:errors condition="field.error and not hidden"
@@ -17,13 +18,8 b''
          i18n:translate="">${msg}</p>
     </tal:errors>
   </div>
+
   <div class="deform-seq-item-handle" style="padding:0">
-    <!-- sequence_item -->
-    <span class="deform-order-button close glyphicon glyphicon-resize-vertical"
-          id="${oid}-order"
-          tal:condition="not hidden"
-          title="Reorder (via drag and drop)"
-          i18n:attributes="title"></span>
     <a class="deform-close-button close"
        id="${oid}-close"
        tal:condition="not field.widget.hidden"
@@ -31,5 +27,5 b''
        i18n:attributes="title"
        onclick="javascript:deform.removeSequenceItem(this);">&times;</a>
   </div>
-  <!-- /sequence_item -->
+
 </div>

rhodecode/templates/pullrequests/pullrequest.mako

@@ -62,7 +62,7 b''
 
                             ##ORG
                             <div class="content">
-                                <strong>${_('Origin repository')}:</strong>
+                                <strong>${_('Source repository')}:</strong>
                                 ${c.rhodecode_db_repo.description}
                             </div>
                             <div class="content">
@@ -102,6 +102,17 b''
                 </div>
             </div>
             <div>
+                ## REIEW RULES
+                <div id="review_rules" style="display: none" class="reviewers-title block-right">
+                    <div class="pr-details-title">
+                        ${_('Reviewer rules')}
+                    </div>
+                    <div class="pr-reviewer-rules">
+                        ## review rules will be appended here, by default reviewers logic
+                    </div>
+                </div>
+
+                ## REVIEWERS
                 <div class="reviewers-title block-right">
                     <div class="pr-details-title">
                         ${_('Pull request reviewers')}
@@ -137,7 +148,6 b''
   var defaultSourceRepoData = ${c.default_repo_data['source_refs_json']|n};
   var defaultTargetRepo = '${c.default_repo_data['target_repo_name']}';
   var defaultTargetRepoData = ${c.default_repo_data['target_refs_json']|n};
-  var targetRepoName = '${c.repo_name}';
 
   var $pullRequestForm = $('#pull_request_form');
   var $sourceRepo = $('#source_repo', $pullRequestForm);
@@ -145,6 +155,12 b''
   var $sourceRef = $('#source_ref', $pullRequestForm);
   var $targetRef = $('#target_ref', $pullRequestForm);
 
+  var sourceRepo = function() { return $sourceRepo.eq(0).val() };
+  var sourceRef = function() { return $sourceRef.eq(0).val().split(':') };
+
+  var targetRepo = function() { return $targetRepo.eq(0).val() };
+  var targetRef = function() { return $targetRef.eq(0).val().split(':') };
+
   var calculateContainerWidth = function() {
       var maxWidth = 0;
       var repoSelect2Containers = ['#source_repo', '#target_repo'];
@@ -204,6 +220,8 b''
   // custom code mirror
   var codeMirrorInstance = initPullRequestsCodeMirror('#pullrequest_desc');
 
+  reviewersController = new ReviewersController();
+
   var queryTargetRepo = function(self, query) {
       // cache ALL results if query is empty
       var cacheKey = query.term || '__';
@@ -213,7 +231,7 b''
           query.callback({results: cachedData.results});
       } else {
           $.ajax({
-              url: pyroutes.url('pullrequest_repo_destinations', {'repo_name': targetRepoName}),
+              url: pyroutes.url('pullrequest_repo_destinations', {'repo_name': templateContext.repo_name}),
               data: {query: query.term},
               dataType: 'json',
               type: 'GET',
@@ -246,55 +264,21 b''
       query.callback({results: data.results});
   };
 
-
-  var prButtonLockChecks = {
-      'compare': false,
-      'reviewers': false
-  };
-
-  var prButtonLock = function(lockEnabled, msg, scope) {
-      scope = scope || 'all';
-      if (scope == 'all'){
-          prButtonLockChecks['compare'] = !lockEnabled;
-          prButtonLockChecks['reviewers'] = !lockEnabled;
-      } else if (scope == 'compare') {
-          prButtonLockChecks['compare'] = !lockEnabled;
-      } else if (scope == 'reviewers'){
-          prButtonLockChecks['reviewers'] = !lockEnabled;
-      }
-      var checksMeet = prButtonLockChecks.compare && prButtonLockChecks.reviewers;
-      if (lockEnabled) {
-          $('#save').attr('disabled', 'disabled');
-      }
-      else if (checksMeet) {
-          $('#save').removeAttr('disabled');
-      }
-
-      if (msg) {
-          $('#pr_open_message').html(msg);
-      }
-  };
-
   var loadRepoRefDiffPreview = function() {
-      var sourceRepo = $sourceRepo.eq(0).val();
-      var sourceRef = $sourceRef.eq(0).val().split(':');
-
-      var targetRepo = $targetRepo.eq(0).val();
-      var targetRef = $targetRef.eq(0).val().split(':');
 
       var url_data = {
-          'repo_name': targetRepo,
-          'target_repo': sourceRepo,
-          'source_ref': targetRef[2],
+          'repo_name': targetRepo(),
+          'target_repo': sourceRepo(),
+          'source_ref': targetRef()[2],
           'source_ref_type': 'rev',
-          'target_ref': sourceRef[2],
+          'target_ref': sourceRef()[2],
           'target_ref_type': 'rev',
           'merge': true,
           '_': Date.now() // bypass browser caching
       }; // gather the source/target ref and repo here
 
-      if (sourceRef.length !== 3 || targetRef.length !== 3) {
-          prButtonLock(true, "${_('Please select origin and destination')}");
+      if (sourceRef().length !== 3 || targetRef().length !== 3) {
+          prButtonLock(true, "${_('Please select source and target')}");
           return;
       }
       var url = pyroutes.url('compare_url', url_data);
@@ -315,10 +299,11 b''
           .done(function(data) {
               loadRepoRefDiffPreview._currentRequest = null;
               $('#pull_request_overview').html(data);
+
               var commitElements = $(data).find('tr[commit_id]');
 
-              var prTitleAndDesc = getTitleAndDescription(sourceRef[1],
-                                                          commitElements, 5);
+              var prTitleAndDesc = getTitleAndDescription(
+                  sourceRef()[1], commitElements, 5);
 
               var title = prTitleAndDesc[0];
               var proposedDescription = prTitleAndDesc[1];
@@ -366,43 +351,6 b''
           });
   };
 
-  /**
-   Generate Title and Description for a PullRequest.
-   In case of 1 commits, the title and description is that one commit
-   in case of multiple commits, we iterate on them with max N number of commits,
-   and build description in a form
-   - commitN
-   - commitN+1
-   ...
-
-   Title is then constructed from branch names, or other references,
-   replacing '-' and '_' into spaces
-
-   * @param sourceRef
-   * @param elements
-   * @param limit
-   * @returns {*[]}
-   */
-  var getTitleAndDescription = function(sourceRef, elements, limit) {
-      var title = '';
-      var desc = '';
-
-      $.each($(elements).get().reverse().slice(0, limit), function(idx, value) {
-          var rawMessage = $(value).find('td.td-description .message').data('messageRaw');
-          desc += '- ' + rawMessage.split('\n')[0].replace(/\n+$/, "") + '\n';
-      });
-      // only 1 commit, use commit message as title
-      if (elements.length == 1) {
-          title = $(elements[0]).find('td.td-description .message').data('messageRaw').split('\n')[0];
-      }
-      else {
-          // use reference name
-          title = sourceRef.replace(/-/g, ' ').replace(/_/g, ' ').capitalizeFirstLetter();
-      }
-
-      return [title, desc]
-  };
-
   var Select2Box = function(element, overrides) {
     var globalDefaults = {
         dropdownAutoWidth: true,
@@ -459,7 +407,7 b''
   var targetRepoChanged = function(repoData) {
       // generate new DESC of target repo displayed next to select
       $('#target_repo_desc').html(
-          "<strong>${_('Destination repository')}</strong>: {0}".format(repoData['description'])
+          "<strong>${_('Target repository')}</strong>: {0}".format(repoData['description'])
       );
 
       // generate dynamic select2 for refs.
@@ -468,8 +416,7 b''
 
   };
 
-  var sourceRefSelect2 = Select2Box(
-    $sourceRef, {
+  var sourceRefSelect2 = Select2Box($sourceRef, {
       placeholder: "${_('Select commit reference')}",
       query: function(query) {
         var initialData = defaultSourceRepoData['refs']['select2_refs'];
@@ -499,12 +446,14 b''
 
   $sourceRef.on('change', function(e){
     loadRepoRefDiffPreview();
-    loadDefaultReviewers();
+    reviewersController.loadDefaultReviewers(
+        sourceRepo(), sourceRef(), targetRepo(), targetRef());
   });
 
   $targetRef.on('change', function(e){
     loadRepoRefDiffPreview();
-    loadDefaultReviewers();
+    reviewersController.loadDefaultReviewers(
+        sourceRepo(), sourceRef(), targetRepo(), targetRef());
   });
 
   $targetRepo.on('change', function(e){
@@ -515,7 +464,7 b''
 
       $.ajax({
           url: pyroutes.url('pullrequest_repo_refs',
-            {'repo_name': targetRepoName, 'target_repo_name':repoName}),
+            {'repo_name': templateContext.repo_name, 'target_repo_name':repoName}),
           data: {},
           dataType: 'json',
           type: 'GET',
@@ -531,43 +480,7 b''
 
   });
 
-  var loadDefaultReviewers = function() {
-    if (loadDefaultReviewers._currentRequest) {
-        loadDefaultReviewers._currentRequest.abort();
-    }
-    $('.calculate-reviewers').show();
-    prButtonLock(true, null, 'reviewers');
-
-    var url = pyroutes.url('repo_default_reviewers_data', {'repo_name': targetRepoName});
-
-    var sourceRepo = $sourceRepo.eq(0).val();
-    var sourceRef = $sourceRef.eq(0).val().split(':');
-    var targetRepo = $targetRepo.eq(0).val();
-    var targetRef = $targetRef.eq(0).val().split(':');
-    url += '?source_repo=' + sourceRepo;
-    url += '&source_ref=' + sourceRef[2];
-    url += '&target_repo=' + targetRepo;
-    url += '&target_ref=' + targetRef[2];
-
-    loadDefaultReviewers._currentRequest = $.get(url)
-        .done(function(data) {
-            loadDefaultReviewers._currentRequest = null;
-
-            // reset && add the reviewer based on selected repo
-            $('#review_members').html('');
-            for (var i = 0; i < data.reviewers.length; i++) {
-              var reviewer = data.reviewers[i];
-              addReviewMember(
-                  reviewer.user_id, reviewer.firstname,
-                  reviewer.lastname, reviewer.username,
-                  reviewer.gravatar_link, reviewer.reasons);
-            }
-            $('.calculate-reviewers').hide();
-            prButtonLock(false, null, 'reviewers');
-        });
-  };
-
-  prButtonLock(true, "${_('Please select origin and destination')}", 'all');
+  prButtonLock(true, "${_('Please select source and target')}", 'all');
 
   // auto-load on init, the target refs select2
   calculateContainerWidth();
@@ -578,10 +491,11 b''
   });
 
   % if c.default_source_ref:
-      // in case we have a pre-selected value, use it now
-      $sourceRef.select2('val', '${c.default_source_ref}');
-      loadRepoRefDiffPreview();
-      loadDefaultReviewers();
+  // in case we have a pre-selected value, use it now
+  $sourceRef.select2('val', '${c.default_source_ref}');
+  loadRepoRefDiffPreview();
+  reviewersController.loadDefaultReviewers(
+      sourceRepo(), sourceRef(), targetRepo(), targetRef());
   % endif
 
   ReviewerAutoComplete('#user');

rhodecode/templates/pullrequests/pullrequest_show.mako

@@ -48,13 +48,13 b''
       <div class="summary-details block-left">
         <% summary = lambda n:{False:'summary-short'}.get(n) %>
         <div class="pr-details-title">
-            <a href="${h.url('pull_requests_global', pull_request_id=c.pull_request.pull_request_id)}">${_('Pull request #%s') % c.pull_request.pull_request_id}</a> ${_('From')} ${h.format_date(c.pull_request.created_on)}
+            <a href="${h.route_path('pull_requests_global', pull_request_id=c.pull_request.pull_request_id)}">${_('Pull request #%s') % c.pull_request.pull_request_id}</a> ${_('From')} ${h.format_date(c.pull_request.created_on)}
             %if c.allowed_to_update:
               <div id="delete_pullrequest" class="pull-right action_button ${'' if c.allowed_to_delete else 'disabled' }" style="clear:inherit;padding: 0">
                   % if c.allowed_to_delete:
                       ${h.secure_form(url('pullrequest_delete', repo_name=c.pull_request.target_repo.repo_name, pull_request_id=c.pull_request.pull_request_id),method='delete')}
                           ${h.submit('remove_%s' % c.pull_request.pull_request_id, _('Delete'),
-                        class_="btn btn-link btn-danger",onclick="return confirm('"+_('Confirm to delete this pull request')+"');")}
+                        class_="btn btn-link btn-danger no-margin",onclick="return confirm('"+_('Confirm to delete this pull request')+"');")}
                       ${h.end_form()}
                   % else:
                     ${_('Delete')}
@@ -68,7 +68,7 b''
         <div id="summary" class="fields pr-details-content">
            <div class="field">
             <div class="label-summary">
-                <label>${_('Origin')}:</label>
+                <label>${_('Source')}:</label>
             </div>
             <div class="input">
                 <div class="pr-origininfo">
@@ -297,7 +297,7 b''
            <div id="pr-save" class="field" style="display: none;">
             <div class="label-summary"></div>
             <div class="input">
-              <span id="edit_pull_request" class="btn btn-small">${_('Save Changes')}</span>
+              <span id="edit_pull_request" class="btn btn-small no-margin">${_('Save Changes')}</span>
             </div>
            </div>
         </div>
@@ -316,13 +316,27 b''
               </li>
             </ul>
         </div>
+
+        ## REVIEW RULES
+        <div id="review_rules" style="display: none" class="reviewers-title block-right">
+            <div class="pr-details-title">
+                ${_('Reviewer rules')}
+              %if c.allowed_to_update:
+                <span id="close_edit_reviewers" class="block-right action_button last-item" style="display: none;">${_('Close')}</span>
+              %endif
+            </div>
+            <div class="pr-reviewer-rules">
+                ## review rules will be appended here, by default reviewers logic
+            </div>
+            <input id="review_data" type="hidden" name="review_data" value="">
+        </div>
+
         ## REVIEWERS
         <div class="reviewers-title block-right">
           <div class="pr-details-title">
               ${_('Pull request reviewers')}
               %if c.allowed_to_update:
-                <span id="open_edit_reviewers" class="block-right action_button">${_('Edit')}</span>
-                <span id="close_edit_reviewers" class="block-right action_button" style="display: none;">${_('Close')}</span>
+                <span id="open_edit_reviewers" class="block-right action_button last-item">${_('Edit')}</span>
               %endif
           </div>
         </div>
@@ -330,8 +344,8 b''
           ## members goes here !
             <input type="hidden" name="__start__" value="review_members:sequence">
             <ul id="review_members" class="group_members">
-            %for member,reasons,status in c.pull_request_reviewers:
-              <li id="reviewer_${member.user_id}">
+            %for member,reasons,mandatory,status in c.pull_request_reviewers:
+              <li id="reviewer_${member.user_id}" class="reviewer_entry">
                 <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>
@@ -348,30 +362,43 b''
                   %endfor
                   <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 c.allowed_to_update:
-                  <div class="reviewer_member_remove action_button" onclick="removeReviewMember(${member.user_id}, true)" style="visibility: hidden;">
-                      <i class="icon-remove-sign" ></i>
-                  </div>
-                  %endif
+                  % 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="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
             </ul>
             <input type="hidden" name="__end__" value="review_members:sequence">
-          %if not c.pull_request.is_closed():
-          <div id="add_reviewer_input" class='ac' style="display: none;">
-            %if c.allowed_to_update:
-            <div class="reviewer_ac">
-               ${h.text('user', class_='ac-input', placeholder=_('Add reviewer or reviewer group'))}
-               <div id="reviewers_container"></div>
-            </div>
-            <div>
-             <button id="update_pull_request" class="btn btn-small">${_('Save Changes')}</button>
-            </div>
+
+            %if not c.pull_request.is_closed():
+                <div id="add_reviewer" class="ac" style="display: none;">
+                %if c.allowed_to_update:
+                    % if not c.forbid_adding_reviewers:
+                        <div id="add_reviewer_input" class="reviewer_ac">
+                           ${h.text('user', class_='ac-input', placeholder=_('Add reviewer or reviewer group'))}
+                           <div id="reviewers_container"></div>
+                        </div>
+                    % endif
+                    <div class="pull-right">
+                        <button id="update_pull_request" class="btn btn-small no-margin">${_('Save Changes')}</button>
+                    </div>
+                %endif
+                </div>
             %endif
-          </div>
-          %endif
         </div>
       </div>
   </div>
@@ -429,7 +456,7 b''
 
                     <div class="pull-right">
                         % if c.allowed_to_update and not c.pull_request.is_closed():
-                          <a id="update_commits" class="btn btn-primary pull-right">${_('Update commits')}</a>
+                          <a id="update_commits" class="btn btn-primary no-margin pull-right">${_('Update commits')}</a>
                         % else:
                           <a class="tooltip btn disabled pull-right" disabled="disabled" title="${_('Update is disabled for current view')}">${_('Update commits')}</a>
                         % endif
@@ -615,9 +642,10 b''
         versionController = new VersionController();
         versionController.init();
 
+        reviewersController = new ReviewersController();
 
         $(function(){
-            ReviewerAutoComplete('#user');
+
             // custom code mirror
             var codeMirrorInstance = initPullRequestsCodeMirror('#pr-description-input');
 
@@ -655,13 +683,13 b''
             var ReviewersPanel = {
               editButton: $('#open_edit_reviewers'),
               closeButton: $('#close_edit_reviewers'),
-              addButton: $('#add_reviewer_input'),
-              removeButtons: $('.reviewer_member_remove'),
+              addButton: $('#add_reviewer'),
+              removeButtons: $('.reviewer_member_remove,.reviewer_member_mandatory_remove,.reviewer_member_mandatory'),
 
               init: function() {
-                var that = this;
-                this.editButton.on('click', function(e) { that.edit(); });
-                this.closeButton.on('click', function(e) { that.close(); });
+                var self = this;
+                this.editButton.on('click', function(e) { self.edit(); });
+                this.closeButton.on('click', function(e) { self.close(); });
               },
 
               edit: function(event) {
@@ -669,6 +697,9 b''
                 this.closeButton.show();
                 this.addButton.show();
                 this.removeButtons.css('visibility', 'visible');
+                // review rules
+                reviewersController.loadReviewRules(
+                    ${c.pull_request.reviewer_data_json | n});
               },
 
               close: function(event) {
@@ -676,6 +707,8 b''
                 this.closeButton.hide();
                 this.addButton.hide();
                 this.removeButtons.css('visibility', 'hidden');
+                // hide review rules
+                reviewersController.hideReviewRules()
               }
             };
 
@@ -774,7 +807,8 b''
                 $(this).attr('disabled', 'disabled');
                 $(this).addClass('disabled');
                 $(this).html(_gettext('Saving...'));
-                updateReviewers(undefined, "${c.repo_name}", "${c.pull_request.pull_request_id}");
+                reviewersController.updateReviewers(
+                    "${c.repo_name}", "${c.pull_request.pull_request_id}");
             });
 
             $('#update_commits').on('click', function(e){
@@ -784,14 +818,16 b''
                 $(e.currentTarget).removeClass('btn-primary');
                 $(e.currentTarget).text(_gettext('Updating...'));
                 if(isDisabled){
-                    updateCommits("${c.repo_name}", "${c.pull_request.pull_request_id}");
+                    updateCommits(
+                        "${c.repo_name}", "${c.pull_request.pull_request_id}");
                 }
             });
             // fixing issue with caches on firefox
             $('#update_commits').removeAttr("disabled");
 
             $('#close_pull_request').on('click', function(e){
-                closePullRequest("${c.repo_name}", "${c.pull_request.pull_request_id}");
+                closePullRequest(
+                    "${c.repo_name}", "${c.pull_request.pull_request_id}");
             });
 
             $('.show-inline-comments').on('click', function(e){
@@ -818,6 +854,8 b''
             // initial injection
             injectCloseAction();
 
+            ReviewerAutoComplete('#user');
+
         })
       </script>
 

rhodecode/tests/functional/test_login.py

@@ -25,7 +25,8 b' import pytest'
 
 from rhodecode.config.routing import ADMIN_PREFIX
 from rhodecode.tests import (
-    assert_session_flash, url, HG_REPO, TEST_USER_ADMIN_LOGIN)
+    assert_session_flash, url, HG_REPO, TEST_USER_ADMIN_LOGIN,
+    no_newline_id_generator)
 from rhodecode.tests.fixture import Fixture
 from rhodecode.tests.utils import AssertResponse, get_session_from_response
 from rhodecode.lib.auth import check_password
@@ -122,7 +123,7 b' class TestLoginController(object):'
         'ftp://some.ftp.server',
         'http://other.domain',
         '/\r\nX-Forwarded-Host: http://example.org',
-    ])
+    ], ids=no_newline_id_generator)
     def test_login_bad_came_froms(self, url_came_from):
         _url = '{}?came_from={}'.format(login_url, url_came_from)
         response = self.app.post(

rhodecode/tests/functional/test_pullrequests.py

@@ -295,8 +295,8 b' class TestPullrequestsController:'
     def test_comment_force_close_pull_request(self, pr_util, csrf_token):
         pull_request = pr_util.create_pull_request()
         pull_request_id = pull_request.pull_request_id
-        reviewers_data = [(1, ['reason']), (2, ['reason2'])]
-        PullRequestModel().update_reviewers(pull_request_id, reviewers_data)
+        PullRequestModel().update_reviewers(
+            pull_request_id, [(1, ['reason'], False), (2, ['reason2'], False)])
         author = pull_request.user_id
         repo = pull_request.target_repo.repo_id
         self.app.post(
@@ -345,6 +345,7 b' class TestPullrequestsController:'
                 ('source_ref', 'branch:default:' + commit_ids['change2']),
                 ('target_repo', target.repo_name),
                 ('target_ref',  'branch:default:' + commit_ids['ancestor']),
+                ('common_ancestor', commit_ids['ancestor']),
                 ('pullrequest_desc', 'Description'),
                 ('pullrequest_title', 'Title'),
                 ('__start__', 'review_members:sequence'),
@@ -353,6 +354,7 b' class TestPullrequestsController:'
                         ('__start__', 'reasons:sequence'),
                             ('reason', 'Some reason'),
                         ('__end__', 'reasons:sequence'),
+                        ('mandatory', 'False'),
                     ('__end__', 'reviewer:mapping'),
                 ('__end__', 'review_members:sequence'),
                 ('__start__', 'revisions:sequence'),
@@ -365,8 +367,9 b' class TestPullrequestsController:'
             status=302)
 
         location = response.headers['Location']
-        pull_request_id = int(location.rsplit('/', 1)[1])
-        pull_request = PullRequest.get(pull_request_id)
+        pull_request_id = location.rsplit('/', 1)[1]
+        assert pull_request_id != 'new'
+        pull_request = PullRequest.get(int(pull_request_id))
 
         # check that we have now both revisions
         assert pull_request.revisions == [commit_ids['change2'], commit_ids['change']]
@@ -403,6 +406,7 b' class TestPullrequestsController:'
                 ('source_ref', 'branch:default:' + commit_ids['change']),
                 ('target_repo', target.repo_name),
                 ('target_ref',  'branch:default:' + commit_ids['ancestor-child']),
+                ('common_ancestor', commit_ids['ancestor']),
                 ('pullrequest_desc', 'Description'),
                 ('pullrequest_title', 'Title'),
                 ('__start__', 'review_members:sequence'),
@@ -411,6 +415,7 b' class TestPullrequestsController:'
                         ('__start__', 'reasons:sequence'),
                             ('reason', 'Some reason'),
                         ('__end__', 'reasons:sequence'),
+                        ('mandatory', 'False'),
                     ('__end__', 'reviewer:mapping'),
                 ('__end__', 'review_members:sequence'),
                 ('__start__', 'revisions:sequence'),
@@ -422,21 +427,22 b' class TestPullrequestsController:'
             status=302)
 
         location = response.headers['Location']
-        pull_request_id = int(location.rsplit('/', 1)[1])
-        pull_request = PullRequest.get(pull_request_id)
+
+        pull_request_id = location.rsplit('/', 1)[1]
+        assert pull_request_id != 'new'
+        pull_request = PullRequest.get(int(pull_request_id))
 
         # Check that a notification was made
         notifications = Notification.query()\
             .filter(Notification.created_by == pull_request.author.user_id,
                     Notification.type_ == Notification.TYPE_PULL_REQUEST,
-                    Notification.subject.contains("wants you to review "
-                                                  "pull request #%d"
-                                                  % pull_request_id))
+                    Notification.subject.contains(
+                        "wants you to review pull request #%s" % pull_request_id))
         assert len(notifications.all()) == 1
 
         # Change reviewers and check that a notification was made
         PullRequestModel().update_reviewers(
-            pull_request.pull_request_id, [(1, [])])
+            pull_request.pull_request_id, [(1, [], False)])
         assert len(notifications.all()) == 2
 
     def test_create_pull_request_stores_ancestor_commit_id(self, backend,
@@ -467,6 +473,7 b' class TestPullrequestsController:'
                 ('source_ref', 'branch:default:' + commit_ids['change']),
                 ('target_repo', target.repo_name),
                 ('target_ref',  'branch:default:' + commit_ids['ancestor-child']),
+                ('common_ancestor', commit_ids['ancestor']),
                 ('pullrequest_desc', 'Description'),
                 ('pullrequest_title', 'Title'),
                 ('__start__', 'review_members:sequence'),
@@ -475,6 +482,7 b' class TestPullrequestsController:'
                         ('__start__', 'reasons:sequence'),
                             ('reason', 'Some reason'),
                         ('__end__', 'reasons:sequence'),
+                        ('mandatory', 'False'),
                     ('__end__', 'reviewer:mapping'),
                 ('__end__', 'review_members:sequence'),
                 ('__start__', 'revisions:sequence'),
@@ -486,8 +494,10 b' class TestPullrequestsController:'
             status=302)
 
         location = response.headers['Location']
-        pull_request_id = int(location.rsplit('/', 1)[1])
-        pull_request = PullRequest.get(pull_request_id)
+
+        pull_request_id = location.rsplit('/', 1)[1]
+        assert pull_request_id != 'new'
+        pull_request = PullRequest.get(int(pull_request_id))
 
         # target_ref has to point to the ancestor's commit_id in order to
         # show the correct diff
@@ -954,14 +964,7 b' class TestPullrequestsController:'
         assert target.text.strip() == 'tag: target'
         assert target.getchildren() == []
 
-    def test_description_is_escaped_on_index_page(self, backend, pr_util):
-        xss_description = "<script>alert('Hi!')</script>"
-        pull_request = pr_util.create_pull_request(description=xss_description)
-        response = self.app.get(url(
-            controller='pullrequests', action='show_all',
-            repo_name=pull_request.target_repo.repo_name))
-        response.mustcontain(
-            "&lt;script&gt;alert(&#39;Hi!&#39;)&lt;/script&gt;")
+
 
     @pytest.mark.parametrize('mergeable', [True, False])
     def test_shadow_repository_link(
@@ -1061,23 +1064,20 b' def assert_pull_request_status(pull_requ'
     assert status == expected_status
 
 
[e-mail]('action', ['show_all', 'index', 'create'])
[e-mail]('action', ['index', 'create'])
 @pytest.mark.usefixtures("autologin_user")
-def test_redirects_to_repo_summary_for_svn_repositories(
-        backend_svn, app, action):
-    denied_actions = ['show_all', 'index', 'create']
-    for action in denied_actions:
-        response = app.get(url(
-            controller='pullrequests', action=action,
-            repo_name=backend_svn.repo_name))
-        assert response.status_int == 302
+def test_redirects_to_repo_summary_for_svn_repositories(backend_svn, app, action):
+    response = app.get(url(
+        controller='pullrequests', action=action,
+        repo_name=backend_svn.repo_name))
+    assert response.status_int == 302
 
-        # Not allowed, redirect to the summary
-        redirected = response.follow()
-        summary_url = url('summary_home', repo_name=backend_svn.repo_name)
+    # Not allowed, redirect to the summary
+    redirected = response.follow()
+    summary_url = url('summary_home', repo_name=backend_svn.repo_name)
 
-        # URL adds leading slash and path doesn't have it
-        assert redirected.req.path == summary_url
+    # URL adds leading slash and path doesn't have it
+    assert redirected.req.path == summary_url
 
 
 def test_delete_comment_returns_404_if_comment_does_not_exist(pylonsapp):

rhodecode/tests/models/test_pullrequest.py

@@ -116,7 +116,7 b' class TestPullRequestModel:'
 
     def test_get_awaiting_my_review(self, pull_request):
         PullRequestModel().update_reviewers(
-            pull_request, [(pull_request.author, ['author'])])
+            pull_request, [(pull_request.author, ['author'], False)])
         prs = PullRequestModel().get_awaiting_my_review(
             pull_request.target_repo, user_id=pull_request.author.user_id)
         assert isinstance(prs, list)
@@ -124,7 +124,7 b' class TestPullRequestModel:'
 
     def test_count_awaiting_my_review(self, pull_request):
         PullRequestModel().update_reviewers(
-            pull_request, [(pull_request.author, ['author'])])
+            pull_request, [(pull_request.author, ['author'], False)])
         pr_count = PullRequestModel().count_awaiting_my_review(
             pull_request.target_repo, user_id=pull_request.author.user_id)
         assert pr_count == 1

rhodecode/tests/plugin.py

@@ -986,10 +986,9 b' class PRTestUtility(object):'
         return reference
 
     def _get_reviewers(self):
-        model = UserModel()
         return [
-            model.get_by_username(TEST_USER_REGULAR_LOGIN),
-            model.get_by_username(TEST_USER_REGULAR2_LOGIN),
+            (TEST_USER_REGULAR_LOGIN, ['default1'], False),
+            (TEST_USER_REGULAR2_LOGIN, ['default2'], False),
         ]
 
     def update_source_repository(self, head=None):