reviewers: store reviewer reasons to database, fixes #4238

rhodecode/api/tests/test_get_pull_request.py

@@ -95,9 +95,10 b' class TestGetPullRequest(object):'
                 {
                     'user': reviewer.get_api_data(include_secrets=False,
                                                   details='basic'),
+                    'reasons': reasons,
                     'review_status': st[0][1].status if st else 'not_reviewed',
                 }
-                for reviewer, st in pull_request.reviewers_statuses()
+                for reviewer, reasons, st in pull_request.reviewers_statuses()
             ]
         }
         assert_ok(id_, expected, response.body)

rhodecode/api/views/pull_request_api.py

@@ -463,7 +463,12 b' def create_pull_request('
     :type description: Optional(str)
     :param reviewers: Set the new pull request reviewers list.
     :type reviewers: Optional(list)
+        Accepts username strings or objects of the format:
+        {
+            'username': 'nick', 'reasons': ['original author']
+        }
     """
+
     source = get_repo_or_error(source_repo)
     target = get_repo_or_error(target_repo)
     if not has_superadmin_permission(apiuser):
@@ -490,12 +495,21 b' def create_pull_request('
     if not ancestor:
         raise JSONRPCError('no common ancestor found')
 
-    reviewer_names = Optional.extract(reviewers) or []
-    if not isinstance(reviewer_names, list):
+    reviewer_objects = Optional.extract(reviewers) or []
+    if not isinstance(reviewer_objects, list):
         raise JSONRPCError('reviewers should be specified as a list')
 
-    reviewer_users = [get_user_or_error(n) for n in reviewer_names]
-    reviewer_ids = [u.user_id for u in reviewer_users]
+    reviewers_reasons = []
+    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))
 
     pull_request_model = PullRequestModel()
     pull_request = pull_request_model.create(
@@ -506,7 +520,7 b' def create_pull_request('
         target_ref=full_target_ref,
         revisions=reversed(
             [commit.raw_id for commit in reversed(commit_ranges)]),
-        reviewers=reviewer_ids,
+        reviewers=reviewers_reasons,
         title=title,
         description=Optional.extract(description)
     )
@@ -585,12 +599,23 b' def update_pull_request('
             'pull request `%s` update failed, pull request is closed' % (
                 pullrequestid,))
 
-    reviewer_names = Optional.extract(reviewers) or []
-    if not isinstance(reviewer_names, list):
+    reviewer_objects = Optional.extract(reviewers) or []
+    if not isinstance(reviewer_objects, list):
         raise JSONRPCError('reviewers should be specified as a list')
 
-    reviewer_users = [get_user_or_error(n) for n in reviewer_names]
-    reviewer_ids = [u.user_id for u in reviewer_users]
+    reviewers_reasons = []
+    reviewer_ids = set()
+    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))
 
     title = Optional.extract(title)
     description = Optional.extract(description)
@@ -611,7 +636,7 b' def update_pull_request('
     reviewers_changes = {"added": [], "removed": []}
     if reviewer_ids:
         added_reviewers, removed_reviewers = \
-            PullRequestModel().update_reviewers(pull_request, reviewer_ids)
+            PullRequestModel().update_reviewers(pull_request, reviewers_reasons)
 
         reviewers_changes['added'] = sorted(
             [get_user_or_error(n).username for n in added_reviewers])

rhodecode/controllers/pullrequests.py

@@ -22,6 +22,7 b''
 pull requests controller for rhodecode for initializing pull requests
 """
 
+import peppercorn
 import formencode
 import logging
 
@@ -399,8 +400,10 b' class PullrequestsController(BaseRepoCon'
         if not repo:
             raise HTTPNotFound
 
+        controls = peppercorn.parse(request.POST.items())
+
         try:
-            _form = PullRequestForm(repo.repo_id)().to_python(request.POST)
+            _form = PullRequestForm(repo.repo_id)().to_python(controls)
         except formencode.Invalid as errors:
             if errors.error_dict.get('revisions'):
                 msg = 'Revisions: %s' % errors.error_dict['revisions']
@@ -419,7 +422,8 b' class PullrequestsController(BaseRepoCon'
         target_repo = _form['target_repo']
         target_ref = _form['target_ref']
         commit_ids = _form['revisions'][::-1]
-        reviewers = _form['review_members']
+        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'])
@@ -478,8 +482,11 b' class PullrequestsController(BaseRepoCon'
         allowed_to_update = PullRequestModel().check_user_update(
             pull_request, c.rhodecode_user)
         if allowed_to_update:
-            if 'reviewers_ids' in request.POST:
-                self._update_reviewers(pull_request_id)
+            controls = peppercorn.parse(request.POST.items())
+
+            if 'review_members' in controls:
+                self._update_reviewers(
+                    pull_request_id, controls['review_members'])
             elif str2bool(request.POST.get('update_commits', 'false')):
                 self._update_commits(pull_request)
             elif str2bool(request.POST.get('close_pull_request', 'false')):
@@ -631,11 +638,10 b' class PullrequestsController(BaseRepoCon'
                 merge_resp.failure_reason)
             h.flash(msg, category='error')
 
-    def _update_reviewers(self, pull_request_id):
-        reviewers_ids = map(int, filter(
-            lambda v: v not in [None, ''],
-            request.POST.get('reviewers_ids', '').split(',')))
-        PullRequestModel().update_reviewers(pull_request_id, reviewers_ids)
+    def _update_reviewers(self, pull_request_id, review_members):
+        reviewers = [
+            (int(r['user_id']), r['reasons']) for r in review_members]
+        PullRequestModel().update_reviewers(pull_request_id, reviewers)
         Session().commit()
 
     def _reject_close(self, pull_request):

rhodecode/lib/dbmigrate/schema/db_4_5_0_0.py

@@ -59,7 +59,7 b' from rhodecode.lib.utils2 import ('
     str2bool, safe_str, get_commit_safe, safe_unicode, remove_prefix, md5_safe,
     time_to_datetime, aslist, Optional, safe_int, get_clone_url, AttributeDict,
     glob2re)
-from rhodecode.lib.jsonalchemy import MutationObj, JsonType, JSONDict
+from rhodecode.lib.jsonalchemy import MutationObj, MutationList, JsonType, JSONDict
 from rhodecode.lib.ext_json import json
 from rhodecode.lib.caching_query import FromCache
 from rhodecode.lib.encrypt import AESCipher
@@ -3149,9 +3149,10 b' class PullRequest(Base, _PullRequestBase'
                 {
                     'user': reviewer.get_api_data(include_secrets=False,
                                                   details='basic'),
+                    'reasons': reasons,
                     'review_status': st[0][1].status if st else 'not_reviewed',
                 }
-                for reviewer, st in pull_request.reviewers_statuses()
+                for reviewer, reasons, st in pull_request.reviewers_statuses()
             ]
         }
 
@@ -3203,9 +3204,23 b' class PullRequestReviewers(Base, BaseMod'
          'mysql_charset': 'utf8', 'sqlite_autoincrement': True},
     )
 
-    def __init__(self, user=None, pull_request=None):
+    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:
+            return []
+        return self._reasons
+
+    @reasons.setter
+    def reasons(self, val):
+        val = val or []
+        if any(not isinstance(x, basestring) for x in val):
+            raise Exception('invalid reasons type, must be list of strings')
+        self._reasons = val
 
     pull_requests_reviewers_id = Column(
         'pull_requests_reviewers_id', Integer(), nullable=False,
@@ -3215,8 +3230,9 b' class PullRequestReviewers(Base, BaseMod'
         ForeignKey('pull_requests.pull_request_id'), nullable=False)
     user_id = Column(
         "user_id", Integer(), ForeignKey('users.user_id'), nullable=True)
-    reason = Column('reason',
-        UnicodeText().with_variant(UnicodeText(255), 'mysql'), nullable=True)
+    _reasons = Column(
+        'reason', MutationList.as_mutable(
+            JsonType('list', dialect_map=dict(mysql=UnicodeText(16384)))))
 
     user = relationship('User')
     pull_request = relationship('PullRequest')

rhodecode/lib/dbmigrate/versions/060_version_4_5_0.py

@@ -26,7 +26,7 b' def upgrade(migrate_engine):'
     _reset_base(migrate_engine)
     from rhodecode.lib.dbmigrate.schema import db_4_5_0_0
 
-    db_4_5_0_0.PullRequestReviewers.reason.create(
+    db_4_5_0_0.PullRequestReviewers.reasons.create(
         table=db_4_5_0_0.PullRequestReviewers.__table__)
 
 def downgrade(migrate_engine):

rhodecode/model/changeset_status.py

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

rhodecode/model/db.py

@@ -59,7 +59,7 b' from rhodecode.lib.utils2 import ('
     str2bool, safe_str, get_commit_safe, safe_unicode, remove_prefix, md5_safe,
     time_to_datetime, aslist, Optional, safe_int, get_clone_url, AttributeDict,
     glob2re)
-from rhodecode.lib.jsonalchemy import MutationObj, JsonType, JSONDict
+from rhodecode.lib.jsonalchemy import MutationObj, MutationList, JsonType, JSONDict
 from rhodecode.lib.ext_json import json
 from rhodecode.lib.caching_query import FromCache
 from rhodecode.lib.encrypt import AESCipher
@@ -3149,9 +3149,10 b' class PullRequest(Base, _PullRequestBase'
                 {
                     'user': reviewer.get_api_data(include_secrets=False,
                                                   details='basic'),
+                    'reasons': reasons,
                     'review_status': st[0][1].status if st else 'not_reviewed',
                 }
-                for reviewer, st in pull_request.reviewers_statuses()
+                for reviewer, reasons, st in pull_request.reviewers_statuses()
             ]
         }
 
@@ -3203,9 +3204,23 b' class PullRequestReviewers(Base, BaseMod'
          'mysql_charset': 'utf8', 'sqlite_autoincrement': True},
     )
 
-    def __init__(self, user=None, pull_request=None):
+    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:
+            return []
+        return self._reasons
+
+    @reasons.setter
+    def reasons(self, val):
+        val = val or []
+        if any(not isinstance(x, basestring) for x in val):
+            raise Exception('invalid reasons type, must be list of strings')
+        self._reasons = val
 
     pull_requests_reviewers_id = Column(
         'pull_requests_reviewers_id', Integer(), nullable=False,
@@ -3215,8 +3230,9 b' class PullRequestReviewers(Base, BaseMod'
         ForeignKey('pull_requests.pull_request_id'), nullable=False)
     user_id = Column(
         "user_id", Integer(), ForeignKey('users.user_id'), nullable=True)
-    reason = Column('reason',
-        UnicodeText().with_variant(UnicodeText(255), 'mysql'), nullable=True)
+    _reasons = Column(
+        'reason', MutationList.as_mutable(
+            JsonType('list', dialect_map=dict(mysql=UnicodeText(16384)))))
 
     user = relationship('User')
     pull_request = relationship('PullRequest')

rhodecode/model/forms.py

@@ -519,7 +519,12 b' def UserExtraIpForm():'
     return _UserExtraIpForm
 
 
+
 def PullRequestForm(repo_id):
+    class ReviewerForm(formencode.Schema):
+        user_id = v.Int(not_empty=True)
+        reasons = All()
+
     class _PullRequestForm(formencode.Schema):
         allow_extra_fields = True
         filter_extra_fields = True
@@ -531,8 +536,7 b' def PullRequestForm(repo_id):'
         target_ref = v.UnicodeString(strip=True, required=True)
         revisions = All(#v.NotReviewedRevisions(repo_id)(),
                         v.UniqueList()(not_empty=True))
-        review_members = v.UniqueList(convert=int)(not_empty=True)
-
+        review_members = formencode.ForEach(ReviewerForm())
         pullrequest_title = v.UnicodeString(strip=True, required=True)
         pullrequest_desc = v.UnicodeString(strip=True, required=False)
 

rhodecode/model/pull_request.py

@@ -333,10 +333,18 b' class PullRequestModel(BaseModel):'
         Session().add(pull_request)
         Session().flush()
 
+        reviewer_ids = set()
         # members / reviewers
-        for user_id in set(reviewers):
+        for reviewer_object in reviewers:
+            if isinstance(reviewer_object, tuple):
+                user_id, reasons = reviewer_object
+            else:
+                user_id, reasons = reviewer_object, []
+
             user = self._get_user(user_id)
-            reviewer = PullRequestReviewers(user, pull_request)
+            reviewer_ids.add(user.user_id)
+
+            reviewer = PullRequestReviewers(user, pull_request, reasons)
             Session().add(reviewer)
 
         # Set approval status to "Under Review" for all commits which are
@@ -348,7 +356,7 b' class PullRequestModel(BaseModel):'
             pull_request=pull_request
         )
 
-        self.notify_reviewers(pull_request, reviewers)
+        self.notify_reviewers(pull_request, reviewer_ids)
         self._trigger_pull_request_hook(
             pull_request, created_by_user, 'create')
 
@@ -570,6 +578,7 b' class PullRequestModel(BaseModel):'
         Session().commit()
         self._trigger_pull_request_hook(pull_request, pull_request.author,
                                         'update')
+
         return (pull_request_version, changes)
 
     def _create_version_from_snapshot(self, pull_request):
@@ -711,8 +720,21 b' class PullRequestModel(BaseModel):'
         pull_request.updated_on = datetime.datetime.now()
         Session().add(pull_request)
 
-    def update_reviewers(self, pull_request, reviewers_ids):
-        reviewers_ids = set(reviewers_ids)
+    def update_reviewers(self, pull_request, reviewer_data):
+        """
+        Update the reviewers in the pull request
+
+        :param pull_request: the pr to update
+        :param reviewer_data: list of tuples [(user, ['reason1', 'reason2'])]
+        """
+
+        reviewers_reasons = {}
+        for user_id, reasons in reviewer_data:
+            if isinstance(user_id, (int, basestring)):
+                user_id = self._get_user(user_id).user_id
+            reviewers_reasons[user_id] = reasons
+
+        reviewers_ids = set(reviewers_reasons.keys())
         pull_request = self.__get_pull_request(pull_request)
         current_reviewers = PullRequestReviewers.query()\
             .filter(PullRequestReviewers.pull_request ==
@@ -728,7 +750,8 b' class PullRequestModel(BaseModel):'
         for uid in ids_to_add:
             changed = True
             _usr = self._get_user(uid)
-            reviewer = PullRequestReviewers(_usr, pull_request)
+            reasons = reviewers_reasons[uid]
+            reviewer = PullRequestReviewers(_usr, pull_request, reasons)
             Session().add(reviewer)
 
         self.notify_reviewers(pull_request, ids_to_add)

rhodecode/public/css/main.less

@@ -145,7 +145,7 b' input.inline[type="file"] {'
     .alert {
         margin: @padding 0;
     }
-    
+
     .error-branding {
         font-family: @text-semibold;
         color: @grey4;
@@ -1282,6 +1282,9 b' table.integrations {'
     width: 100%;
     overflow: auto;
 }
+.reviewer_reason {
+    padding-left: 20px;
+}
 .reviewer_status {
     display: inline-block;
     vertical-align: top;

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

@@ -32,7 +32,7 b' var removeReviewMember = function(review'
             var obj = $('#reviewer_{0}_name'.format(reviewer_id));
             obj.addClass('to-delete');
             // now delete the input
-            $('#reviewer_{0}_input'.format(reviewer_id)).remove();
+            $('#reviewer_{0} input'.format(reviewer_id)).remove();
         }
     }
     else{
@@ -43,29 +43,36 b' var removeReviewMember = function(review'
 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_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}">'+
+       '<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" value="{2}" name="review_members" />'+
+      '<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>' ;
+
     var displayname = "{0} ({1} {2})".format(
             nname, escapeHtml(fname), escapeHtml(lname));
-    var element = tmpl.format(gravatar_link,displayname,id);
+    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();
@@ -83,7 +90,11 b' var _updatePullRequest = function(repo_n'
     var url = pyroutes.url(
         'pullrequest_update',
         {"repo_name": repo_name, "pull_request_id": pull_request_id});
-    postData.csrf_token = CSRF_TOKEN;
+    if (typeof postData === 'string' ) {
+        postData += '&csrf_token=' + CSRF_TOKEN;
+    } else {
+        postData.csrf_token = CSRF_TOKEN;
+    }
     var success = function(o) {
         window.location.reload();
     };
@@ -92,17 +103,9 b' var _updatePullRequest = function(repo_n'
 
 var updateReviewers = function(reviewers_ids, repo_name, pull_request_id){
     if (reviewers_ids === undefined){
-      var reviewers_ids = [];
-      var ids = $('#review_members input').toArray();
-      for(var i=0; i<ids.length;i++){
-          var id = ids[i].value
-          reviewers_ids.push(id);
-      }
+      var postData = '_method=put&' + $('#reviewers input').serialize();
+      _updatePullRequest(repo_name, pull_request_id, postData);
     }
-    var postData = {
-        '_method':'put',
-        'reviewers_ids': reviewers_ids};
-    _updatePullRequest(repo_name, pull_request_id, postData);
 };
 
 /**

rhodecode/public/js/src/rhodecode/utils/ajax.js

@@ -20,6 +20,9 b''
 * turns objects into GET query string
 */
 var toQueryString = function(o) {
+  if(typeof o === 'string') {
+    return o;
+  }
   if(typeof o !== 'object') {
     return false;
   }

rhodecode/templates/pullrequests/pullrequest.html

@@ -111,7 +111,9 b''
                 </div>
                 <div id="reviewers" class="block-right pr-details-content reviewers">
                     ## members goes here, filled via JS based on initial selection !
+                    <input type="hidden" name="__start__" value="review_members:sequence">
                     <ul id="review_members" class="group_members"></ul>
+                    <input type="hidden" name="__end__" value="review_members:sequence">
                     <div id="add_reviewer_input" class='ac'>
                         <div class="reviewer_ac">
                             ${h.text('user', class_='ac-input', placeholder=_('Add reviewer'))}

rhodecode/templates/pullrequests/pullrequest_show.html

@@ -188,17 +188,27 b''
         </div>
         <div id="reviewers" class="block-right pr-details-content reviewers">
           ## members goes here !
+            <input type="hidden" name="__start__" value="review_members:sequence">
             <ul id="review_members" class="group_members">
-            %for member,status in c.pull_request_reviewers:
+            %for member,reasons,status in c.pull_request_reviewers:
               <li id="reviewer_${member.user_id}">
                 <div class="reviewers_member">
                     <div class="reviewer_status tooltip" title="${h.tooltip(h.commit_status_lbl(status[0][1].status if status else 'not_reviewed'))}">
                       <div class="${'flag_status %s' % (status[0][1].status if status else 'not_reviewed')} pull-left reviewer_member_status"></div>
                     </div>
                   <div id="reviewer_${member.user_id}_name" class="reviewer_name">
-                    ${self.gravatar_with_user(member.email, 16)} <div class="reviewer">(${_('owner') if c.pull_request.user_id == member.user_id else _('reviewer')})</div>
+                    ${self.gravatar_with_user(member.email, 16)}
                   </div>
-                  <input id="reviewer_${member.user_id}_input" type="hidden" value="${member.user_id}" name="review_members" />
+                  <input type="hidden" name="__start__" value="reviewer:mapping">
+                  <input type="hidden" name="__start__" value="reasons:sequence">
+                  %for reason in reasons:
+                  <div class="reviewer_reason">- ${reason}</div>
+                  <input type="hidden" name="reason" value="${reason}">
+
+                  %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="__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>
@@ -208,6 +218,7 b''
               </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:
@@ -569,7 +580,7 b''
             $('.show-inline-comments').on('click', function(e){
                 var boxid = $(this).attr('data-comment-id');
                 var button = $(this);
-  
+
                 if(button.hasClass("comments-visible")) {
                   $('#{0} .inline-comments'.format(boxid)).each(function(index){
                     $(this).hide();

rhodecode/tests/functional/test_pullrequests.py

@@ -256,8 +256,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_ids = [1, 2]
-        PullRequestModel().update_reviewers(pull_request_id, reviewers_ids)
+        reviewers_data = [(1, ['reason']), (2, ['reason2'])]
+        PullRequestModel().update_reviewers(pull_request_id, reviewers_data)
         author = pull_request.user_id
         repo = pull_request.target_repo.repo_id
         self.app.post(
@@ -297,19 +297,27 b' class TestPullrequestsController:'
             url(
                 controller='pullrequests',
                 action='create',
-                repo_name=source.repo_name),
-            params={
-                'source_repo': source.repo_name,
-                'source_ref': 'branch:default:' + commit_ids['change'],
-                'target_repo': target.repo_name,
-                'target_ref': 'branch:default:' + commit_ids['ancestor'],
-                'pullrequest_desc': 'Description',
-                'pullrequest_title': 'Title',
-                'review_members': '1',
-                'revisions': commit_ids['change'],
-                'user': '',
-                'csrf_token': csrf_token,
-            },
+                repo_name=source.repo_name
+            ),
+            [
+                ('source_repo', source.repo_name),
+                ('source_ref', 'branch:default:' + commit_ids['change']),
+                ('target_repo', target.repo_name),
+                ('target_ref',  'branch:default:' + commit_ids['ancestor']),
+                ('pullrequest_desc', 'Description'),
+                ('pullrequest_title', 'Title'),
+                ('__start__', 'review_members:sequence'),
+                    ('__start__', 'reviewer:mapping'),
+                        ('user_id', '1'),
+                        ('__start__', 'reasons:sequence'),
+                            ('reason', 'Some reason'),
+                        ('__end__', 'reasons:sequence'),
+                    ('__end__', 'reviewer:mapping'),
+                ('__end__', 'review_members:sequence'),
+                ('revisions', commit_ids['change']),
+                ('user', ''),
+                ('csrf_token', csrf_token),
+            ],
             status=302)
 
         location = response.headers['Location']
@@ -344,19 +352,27 b' class TestPullrequestsController:'
             url(
                 controller='pullrequests',
                 action='create',
-                repo_name=source.repo_name),
-            params={
-                'source_repo': source.repo_name,
-                'source_ref': 'branch:default:' + commit_ids['change'],
-                'target_repo': target.repo_name,
-                'target_ref': 'branch:default:' + commit_ids['ancestor-child'],
-                'pullrequest_desc': 'Description',
-                'pullrequest_title': 'Title',
-                'review_members': '2',
-                'revisions': commit_ids['change'],
-                'user': '',
-                'csrf_token': csrf_token,
-            },
+                repo_name=source.repo_name
+            ),
+            [
+                ('source_repo', source.repo_name),
+                ('source_ref', 'branch:default:' + commit_ids['change']),
+                ('target_repo', target.repo_name),
+                ('target_ref',  'branch:default:' + commit_ids['ancestor-child']),
+                ('pullrequest_desc', 'Description'),
+                ('pullrequest_title', 'Title'),
+                ('__start__', 'review_members:sequence'),
+                    ('__start__', 'reviewer:mapping'),
+                        ('user_id', '2'),
+                        ('__start__', 'reasons:sequence'),
+                            ('reason', 'Some reason'),
+                        ('__end__', 'reasons:sequence'),
+                    ('__end__', 'reviewer:mapping'),
+                ('__end__', 'review_members:sequence'),
+                ('revisions', commit_ids['change']),
+                ('user', ''),
+                ('csrf_token', csrf_token),
+            ],
             status=302)
 
         location = response.headers['Location']
@@ -373,7 +389,8 b' class TestPullrequestsController:'
         assert len(notifications.all()) == 1
 
         # Change reviewers and check that a notification was made
-        PullRequestModel().update_reviewers(pull_request.pull_request_id, [1])
+        PullRequestModel().update_reviewers(
+            pull_request.pull_request_id, [(1, [])])
         assert len(notifications.all()) == 2
 
     def test_create_pull_request_stores_ancestor_commit_id(self, backend,
@@ -397,19 +414,27 b' class TestPullrequestsController:'
             url(
                 controller='pullrequests',
                 action='create',
-                repo_name=source.repo_name),
-            params={
-                'source_repo': source.repo_name,
-                'source_ref': 'branch:default:' + commit_ids['change'],
-                'target_repo': target.repo_name,
-                'target_ref': 'branch:default:' + commit_ids['ancestor-child'],
-                'pullrequest_desc': 'Description',
-                'pullrequest_title': 'Title',
-                'review_members': '1',
-                'revisions': commit_ids['change'],
-                'user': '',
-                'csrf_token': csrf_token,
-            },
+                repo_name=source.repo_name
+            ),
+            [
+                ('source_repo', source.repo_name),
+                ('source_ref', 'branch:default:' + commit_ids['change']),
+                ('target_repo', target.repo_name),
+                ('target_ref',  'branch:default:' + commit_ids['ancestor-child']),
+                ('pullrequest_desc', 'Description'),
+                ('pullrequest_title', 'Title'),
+                ('__start__', 'review_members:sequence'),
+                    ('__start__', 'reviewer:mapping'),
+                        ('user_id', '1'),
+                        ('__start__', 'reasons:sequence'),
+                            ('reason', 'Some reason'),
+                        ('__end__', 'reasons:sequence'),
+                    ('__end__', 'reviewer:mapping'),
+                ('__end__', 'review_members:sequence'),
+                ('revisions', commit_ids['change']),
+                ('user', ''),
+                ('csrf_token', csrf_token),
+            ],
             status=302)
 
         location = response.headers['Location']

rhodecode/tests/models/test_changeset_status.py

@@ -80,7 +80,7 b' def test_pull_request_stays_if_update_wi'
         voted_status, *pull_request.reviewers)
 
     # Update, without change
-    PullRequestModel().update_commits(pull_request)
+    version, changes = PullRequestModel().update_commits(pull_request)
 
     # Expect that review status is the voted_status
     expected_review_status = voted_status
@@ -99,7 +99,7 b' def test_pull_request_under_review_if_up'
 
     # Update, with change
     pr_util.update_source_repository()
-    PullRequestModel().update_commits(pull_request)
+    version, changes = PullRequestModel().update_commits(pull_request)
 
     # Expect that review status is the voted_status
     expected_review_status = db.ChangesetStatus.STATUS_UNDER_REVIEW

rhodecode/tests/models/test_pullrequest.py

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