review members are dynamically changed based on selected other_repo owner

- implemented pyroutes for smarter JS url handling
- fixed some old style global var routing

rhodecode/public/js/pyroutes_map.js

@@ -0,0 +1,11
+//Format is key == name
+//    "mark_error_fixed": [ # key
+//        "/mark_error_fixed/%(error_id)s", #url template 
+//        [ 
+//            "error_id" # list of args
+//        ]
+//    ], 
+//
+var PROUTES_MAP = {
+
+}
  No newline at end of file

rhodecode/controllers/pullrequests.py

@@ -129,16 +129,25 class PullrequestsController(BaseRepoCon
         c.other_repos.extend(c.org_repos)
         c.default_other_repo = org_repo.repo_name
         c.default_other_refs, c.default_other_ref = self._get_repo_refs(org_repo.scm_instance)
+        usr_data = lambda usr: dict(user_id=usr.user_id,
+                                    username=usr.username,
+                                    firstname=usr.firstname,
+                                    lastname=usr.lastname,
+                                    gravatar_link=h.gravatar_url(usr.email, 14))
         other_repos_info[org_repo.repo_name] = {
+            'user': usr_data(org_repo.user),
             'description': org_repo.description,
-            'revs': h.select('other_ref', c.default_other_ref, c.default_other_refs, class_='refs')
+            'revs': h.select('other_ref', c.default_other_ref,
+                             c.default_other_refs, class_='refs')
         }
 
-        # gather forks and add to this list ... even though it is rare to request forks to pull their parent
+        # gather forks and add to this list ... even though it is rare to 
+        # request forks to pull their parent
         for fork in org_repo.forks:
             c.other_repos.append((fork.repo_name, fork.repo_name))
             refs, default_ref = self._get_repo_refs(fork.scm_instance)
             other_repos_info[fork.repo_name] = {
+                'user': usr_data(fork.user),
                 'description': fork.description,
                 'revs': h.select('other_ref', default_ref, refs, class_='refs')
             }
@@ -149,12 +158,15 class PullrequestsController(BaseRepoCon
             c.default_other_refs, c.default_other_ref = self._get_repo_refs(org_repo.parent.scm_instance)
             c.other_repos.append((org_repo.parent.repo_name, org_repo.parent.repo_name))
             other_repos_info[org_repo.parent.repo_name] = {
+                'user': usr_data(org_repo.parent.user),
                 'description': org_repo.parent.description,
-                'revs': h.select('other_ref', c.default_other_ref, c.default_other_refs, class_='refs')
+                'revs': h.select('other_ref', c.default_other_ref,
+                                 c.default_other_refs, class_='refs')
             }
 
         c.other_repos_info = json.dumps(other_repos_info)
-        c.review_members = [org_repo.user]
+        # other repo owner
+        c.review_members = []
         return render('/pullrequests/pullrequest.html')
 
     @NotAnonymous()

rhodecode/public/js/rhodecode.js

@@ -163,7 +163,175 ColorGenerator.prototype = {
     }
 }
 
+/**
+ * PyRoutesJS
+ * 
+ * Usage pyroutes.url('mark_error_fixed',{"error_id":error_id}) // /mark_error_fixed/<error_id>
+ */
+var pyroutes = (function() {
+	// access global map defined in special file pyroutes
+    var matchlist = PROUTES_MAP;
+    var sprintf = (function() {
+        function get_type(variable) {
+            return Object.prototype.toString.call(variable).slice(8, -1).toLowerCase();
+        }
+        function str_repeat(input, multiplier) {
+            for (var output = []; multiplier > 0; output[--multiplier] = input) {/* do nothing */}
+            return output.join('');
+        }
 
+        var str_format = function() {
+            if (!str_format.cache.hasOwnProperty(arguments[0])) {
+                str_format.cache[arguments[0]] = str_format.parse(arguments[0]);
+            }
+            return str_format.format.call(null, str_format.cache[arguments[0]], arguments);
+        };
+
+        str_format.format = function(parse_tree, argv) {
+            var cursor = 1, tree_length = parse_tree.length, node_type = '', arg, output = [], i, k, match, pad, pad_character, pad_length;
+            for (i = 0; i < tree_length; i++) {
+                node_type = get_type(parse_tree[i]);
+                if (node_type === 'string') {
+                    output.push(parse_tree[i]);
+                }
+                else if (node_type === 'array') {
+                    match = parse_tree[i]; // convenience purposes only
+                    if (match[2]) { // keyword argument
+                        arg = argv[cursor];
+                        for (k = 0; k < match[2].length; k++) {
+                            if (!arg.hasOwnProperty(match[2][k])) {
+                                throw(sprintf('[sprintf] property "%s" does not exist', match[2][k]));
+                            }
+                            arg = arg[match[2][k]];
+                        }
+                    }
+                    else if (match[1]) { // positional argument (explicit)
+                        arg = argv[match[1]];
+                    }
+                    else { // positional argument (implicit)
+                        arg = argv[cursor++];
+                    }
+
+                    if (/[^s]/.test(match[8]) && (get_type(arg) != 'number')) {
+                        throw(sprintf('[sprintf] expecting number but found %s', get_type(arg)));
+                    }
+                    switch (match[8]) {
+                        case 'b': arg = arg.toString(2); break;
+                        case 'c': arg = String.fromCharCode(arg); break;
+                        case 'd': arg = parseInt(arg, 10); break;
+                        case 'e': arg = match[7] ? arg.toExponential(match[7]) : arg.toExponential(); break;
+                        case 'f': arg = match[7] ? parseFloat(arg).toFixed(match[7]) : parseFloat(arg); break;
+                        case 'o': arg = arg.toString(8); break;
+                        case 's': arg = ((arg = String(arg)) && match[7] ? arg.substring(0, match[7]) : arg); break;
+                        case 'u': arg = Math.abs(arg); break;
+                        case 'x': arg = arg.toString(16); break;
+                        case 'X': arg = arg.toString(16).toUpperCase(); break;
+                    }
+                    arg = (/[def]/.test(match[8]) && match[3] && arg >= 0 ? '+'+ arg : arg);
+                    pad_character = match[4] ? match[4] == '0' ? '0' : match[4].charAt(1) : ' ';
+                    pad_length = match[6] - String(arg).length;
+                    pad = match[6] ? str_repeat(pad_character, pad_length) : '';
+                    output.push(match[5] ? arg + pad : pad + arg);
+                }
+            }
+            return output.join('');
+        };
+
+        str_format.cache = {};
+
+        str_format.parse = function(fmt) {
+            var _fmt = fmt, match = [], parse_tree = [], arg_names = 0;
+            while (_fmt) {
+                if ((match = /^[^\x25]+/.exec(_fmt)) !== null) {
+                    parse_tree.push(match[0]);
+                }
+                else if ((match = /^\x25{2}/.exec(_fmt)) !== null) {
+                    parse_tree.push('%');
+                }
+                else if ((match = /^\x25(?:([1-9]\d*)\$|\(([^\)]+)\))?(\+)?(0|'[^$])?(-)?(\d+)?(?:\.(\d+))?([b-fosuxX])/.exec(_fmt)) !== null) {
+                    if (match[2]) {
+                        arg_names |= 1;
+                        var field_list = [], replacement_field = match[2], field_match = [];
+                        if ((field_match = /^([a-z_][a-z_\d]*)/i.exec(replacement_field)) !== null) {
+                            field_list.push(field_match[1]);
+                            while ((replacement_field = replacement_field.substring(field_match[0].length)) !== '') {
+                                if ((field_match = /^\.([a-z_][a-z_\d]*)/i.exec(replacement_field)) !== null) {
+                                    field_list.push(field_match[1]);
+                                }
+                                else if ((field_match = /^\[(\d+)\]/.exec(replacement_field)) !== null) {
+                                    field_list.push(field_match[1]);
+                                }
+                                else {
+                                    throw('[sprintf] huh?');
+                                }
+                            }
+                        }
+                        else {
+                            throw('[sprintf] huh?');
+                        }
+                        match[2] = field_list;
+                    }
+                    else {
+                        arg_names |= 2;
+                    }
+                    if (arg_names === 3) {
+                        throw('[sprintf] mixing positional and named placeholders is not (yet) supported');
+                    }
+                    parse_tree.push(match);
+                }
+                else {
+                    throw('[sprintf] huh?');
+                }
+                _fmt = _fmt.substring(match[0].length);
+            }
+            return parse_tree;
+        };
+
+        return str_format;
+    })();
+
+    var vsprintf = function(fmt, argv) {
+        argv.unshift(fmt);
+        return sprintf.apply(null, argv);
+    };
+    return {
+        'url': function(route_name, params) {
+            var result = route_name;
+            if (typeof(params) != 'object'){
+            	params = {};
+            }
+            if (matchlist.hasOwnProperty(route_name)) {
+                var route = matchlist[route_name];
+                for(var i=0; i < route[1].length; i++) {
+
+                   if (!params.hasOwnProperty(route[1][i]))
+                        throw new Error(route[1][i] + ' missing in "' + route_name + '" route generation');
+                }
+                result = sprintf(route[0], params);
+            }
+
+            return result;
+        },
+    	'register': function(route_name, route_tmpl, req_params) {
+    		if (typeof(req_params) != 'object') {
+    			req_params = [];
+    		}
+    		//fix escape
+    		route_tmpl = unescape(route_tmpl);
+    		keys = [];
+    		for (o in req_params){
+    			keys.push(req_params[o])
+    		}
+    		matchlist[route_name] = [
+    		    route_tmpl,
+    		    keys
+    		]
+    	},
+    	'_routes': function(){
+    		return matchlist;
+    	}
+    }
+})();
 
 
 
@@ -333,8 +501,9 var show_changeset_tooltip = function(){
 			YUD.setAttribute(target,'id',ttid);
 			YUD.setAttribute(target, 'title',_TM['loading...']);
 			YAHOO.yuitip.main.set_listeners(target);
-			YAHOO.yuitip.main.show_yuitip(e, target);			
-			ajaxGET(LAZY_CS_URL.replace('__NAME__',repo_name).replace('__REV__', rid), success)
+			YAHOO.yuitip.main.show_yuitip(e, target);
+			var url = pyroutes.url('changeset_info', {"repo_name":repo_name, "revision": rid});
+			ajaxGET(url, success)
 		}
 	});
 };
@@ -396,7 +565,7 var showRepoSize = function(target, repo
     
     if(!YUD.hasClass(target, 'loaded')){
         YUD.get(target).innerHTML = _TM['loading...'];
-        var url = REPO_SIZE_URL.replace('__NAME__', repo_name);    	
+        var url = pyroutes.url('repo_size', {"repo_name":repo_name});
         YUC.asyncRequest('POST',url,{
             success:function(o){
             	YUD.get(target).innerHTML = JSON.parse(o.responseText);
@@ -725,24 +894,6 var deleteComment = function(comment_id)
     ajaxPOST(url,postData,success);
 }
 
-var updateReviewers = function(reviewers_ids){
-	if (reviewers_ids === undefined){
-  	  var reviewers_ids = [];
-	  var ids = YUQ('#review_members input');
-	  for(var i=0; i<ids.length;i++){
-		  var id = ids[i].value
-		  reviewers_ids.push(id);
-	  }		
-	}
-	var url = AJAX_UPDATE_PULLREQUEST;
-	var postData = {'_method':'put',
-			        'reviewers_ids': reviewers_ids};
-	var success = function(o){
-		window.location.reload();
-	}
-	ajaxPOST(url,postData,success);
-}
-
 var createInlineAddButton = function(tr){
 
 	var label = TRANSLATION_MAP['add another comment'];
@@ -869,14 +1020,6 var renderInlineComments = function(file
     }	
 }
 
-var removeReviewer = function(reviewer_id){
-	var el = YUD.get('reviewer_{0}'.format(reviewer_id));
-	if (el.parentNode !== undefined){
-		el.parentNode.removeChild(el);
-	}
-	updateReviewers();
-}
-
 var fileBrowserListeners = function(current_url, node_list_url, url_base){
 	var current_url_branch = +"?branch=__BRANCH__";
 
@@ -1488,6 +1631,56 var MentionsAutoComplete = function (div
     };
 }
 
+var addReviewMember = function(id,fname,lname,nname,gravatar_link){
+	var members  = YUD.get('review_members');
+	var tmpl = '<li id="reviewer_{2}">'+
+    '<div class="reviewers_member">'+
+      '<div class="gravatar"><img alt="gravatar" src="{0}"/> </div>'+
+      '<div style="float:left">{1}</div>'+
+      '<input type="hidden" value="{2}" name="review_members" />'+
+      '<span class="delete_icon action_button" onclick="removeReviewMember({2})"></span>'+
+    '</div>'+
+    '</li>'	;
+    var displayname = "{0} {1} ({2})".format(fname,lname,nname);
+	var element = tmpl.format(gravatar_link,displayname,id);
+	// check if we don't have this ID already in
+	var ids = [];
+	var _els = YUQ('#review_members li');
+	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;
+	}
+	    
+}
+
+var removeReviewMember = function(reviewer_id, repo_name, pull_request_id){
+	var el = YUD.get('reviewer_{0}'.format(reviewer_id));
+	if (el.parentNode !== undefined){
+		el.parentNode.removeChild(el);
+	}
+}
+
+var updateReviewers = function(reviewers_ids, repo_name, pull_request_id){
+	if (reviewers_ids === undefined){
+  	  var reviewers_ids = [];
+	  var ids = YUQ('#review_members input');
+	  for(var i=0; i<ids.length;i++){
+		  var id = ids[i].value
+		  reviewers_ids.push(id);
+	  }		
+	}
+	var url = pyroutes.url('pullrequest_update', {"repo_name":repo_name,
+												  "pull_request_id": pull_request_id});
+	var postData = {'_method':'put',
+			        'reviewers_ids': reviewers_ids};
+	var success = function(o){
+		window.location.reload();
+	}
+	ajaxPOST(url,postData,success);
+}
 
 var PullRequestAutoComplete = function (divid, cont, users_list, groups_list) {
     var myUsers = users_list;
@@ -1620,26 +1813,12 var PullRequestAutoComplete = function (
             var myAC = aArgs[0]; // reference back to the AC instance
             var elLI = aArgs[1]; // reference to the selected LI element
             var oData = aArgs[2]; // object literal of selected item's result data
-            var members  = YUD.get('review_members');
+            
             //fill the autocomplete with value
 
             if (oData.nname != undefined) {
-            	if (myAC.dataSource.cache.indexOf(oData.id) != -1){
-            		return
-            	}
-
-            	var tmpl = '<li id="reviewer_{2}">'+
-		                      '<div class="reviewers_member">'+
-		                        '<div class="gravatar"><img alt="gravatar" src="{0}"/> </div>'+
-		                        '<div style="float:left">{1}</div>'+
-		                        '<input type="hidden" value="{2}" name="review_members" />'+
-		                        '<span class="delete_icon action_button" onclick="removeReviewer({2})"></span>'+
-		                      '</div>'+
-		                   '</li>'
-
-		        var displayname = "{0} {1} ({2})".format(oData.fname,oData.lname,oData.nname);
-            	var element = tmpl.format(oData.gravatar_lnk,displayname,oData.id);
-            	members.innerHTML += element;
+            	addReviewMember(oData.id, oData.fname, oData.lname, oData.nname,
+            					oData.gravatar_lnk);
             	myAC.dataSource.cache.push(oData.id);
             	YUD.get('user').value = '' 
             }
@@ -1651,7 +1830,6 var PullRequestAutoComplete = function (
     };
 }
 
-
 /**
  * QUICK REPO MENU
  */

rhodecode/templates/base/root.html

@@ -53,9 +53,8
                 'Selection link': "${_('Selection link')}",
             };
             var _TM = TRANSLATION_MAP;
+
             var TOGGLE_FOLLOW_URL  = "${h.url('toggle_following')}";
-            var LAZY_CS_URL = "${h.url('changeset_info', repo_name='__NAME__', revision='__REV__')}";
-            var REPO_SIZE_URL = "${h.url('repo_size', repo_name='__NAME__')}";
 
             </script>
             <script type="text/javascript" src="${h.url('/js/yui.2.9.js', ver=c.rhodecode_version)}"></script>
@@ -64,6 +63,7
             <![endif]-->
             <script type="text/javascript" src="${h.url('/js/yui.flot.js', ver=c.rhodecode_version)}"></script>
             <script type="text/javascript" src="${h.url('/js/native.history.js', ver=c.rhodecode_version)}"></script>
+            <script type="text/javascript" src="${h.url('/js/pyroutes_map.js', ver=c.rhodecode_version)}"></script>
             <script type="text/javascript" src="${h.url('/js/rhodecode.js', ver=c.rhodecode_version)}"></script>
            ## EXTRA FOR JS
            ${self.js_extra()}
@@ -82,6 +82,10
               tooltip_activate();
               show_more_event();
               show_changeset_tooltip();
+              // routes registration
+              pyroutes.register('toggle_following', "${h.url('toggle_following')}");
+              pyroutes.register('changeset_info', "${h.url('changeset_info', repo_name='%(repo_name)s', revision='%(revision)s')}", ['repo_name', 'revision']);
+              pyroutes.register('repo_size', "${h.url('repo_size', repo_name='%(repo_name)s')}", ['repo_name']);              
            })
             </script>
         </%def>

rhodecode/templates/pullrequests/pullrequest.html

@@ -67,7 +67,7
                   <div class="gravatar"><img alt="gravatar" src="${h.gravatar_url(member.email,14)}"/> </div>
                   <div style="float:left">${member.full_name} (${_('owner')})</div>
                   <input type="hidden" value="${member.user_id}" name="review_members" />
-                  <span class="delete_icon action_button" onclick="removeReviewer(${member.user_id})"></span>
+                  <span class="delete_icon action_button" onclick="removeReviewMember(${member.user_id})"></span>
                 </div>
               </li>
             %endfor
@@ -161,12 +161,18
       ypjax(url,'pull_request_overview', function(data){
           var sel_box = YUQ('#pull_request_form #other_repo')[0];
           var repo_name = sel_box.options[sel_box.selectedIndex].value;
+          var _data = other_repos_info[repo_name];
           YUD.get('pull_request_overview_url').href = url;
           YUD.setStyle(YUD.get('pull_request_overview_url').parentElement,'display','');
           YUD.get('other_repo_desc').innerHTML = other_repos_info[repo_name]['description'];
           YUD.get('other_ref').innerHTML = other_repos_info[repo_name]['revs'];
           // select back the revision that was just compared
           setSelectValue(YUD.get('other_ref'), rev_data['other_ref']);
+          // reset && add the reviewer based on selected repo
+          YUD.get('review_members').innerHTML = '';
+          addReviewMember(_data.user.user_id, _data.user.firstname,
+        		          _data.user.lastname, _data.user.username,
+        		          _data.user.gravatar_link);
       })
   }
 

rhodecode/templates/pullrequests/pullrequest_show.html

@@ -28,7 +28,7
       <div id="summary" class="fields">
          <div class="field">
           <div class="label-summary">
-              <label>${_('Status')}:</label>
+              <label>${_('Review status')}:</label>
           </div>
           <div class="input">
             <div class="changeset-status-container" style="float:none;clear:both">
@@ -140,7 +140,7
                   <div style="float:left">${member.full_name} (${_('owner') if c.pull_request.user_id == member.user_id else _('reviewer')})</div>
                   <input type="hidden" value="${member.user_id}" name="review_members" />
                   %if not c.pull_request.is_closed() and (h.HasPermissionAny('hg.admin', 'repository.admin')() or c.pull_request.user_id == c.rhodecode_user.user_id):
-                  <span class="delete_icon action_button" onclick="removeReviewer(${member.user_id})"></span>
+                  <span class="delete_icon action_button" onclick="removeReviewMember(${member.user_id})"></span>
                   %endif
                 </div>
               </li>
@@ -152,11 +152,11
             %if h.HasPermissionAny('hg.admin', 'repository.admin')() or c.pull_request.author.user_id == c.rhodecode_user.user_id:
             <div class="reviewer_ac">
                ${h.text('user', class_='yui-ac-input')}
-               <span class="help-block">${_('Add reviewer to this pull request.')}</span>
+               <span class="help-block">${_('Add or remove reviewer to this pull request.')}</span>
                <div id="reviewers_container"></div>
             </div>
             <div style="padding:0px 10px">
-             <span id="update_pull_request" class="ui-btn xsmall">${_('save')}</span>
+             <span id="update_pull_request" class="ui-btn xsmall">${_('save changes')}</span>
             </div>
             %endif
           </div>
@@ -167,9 +167,14
     <script>
     var _USERS_AC_DATA = ${c.users_array|n};
     var _GROUPS_AC_DATA = ${c.users_groups_array|n};
+    // TODO: switch this to pyroutes
     AJAX_COMMENT_URL = "${url('pullrequest_comment',repo_name=c.repo_name,pull_request_id=c.pull_request.pull_request_id)}";
     AJAX_COMMENT_DELETE_URL = "${url('pullrequest_comment_delete',repo_name=c.repo_name,comment_id='__COMMENT_ID__')}";
-    AJAX_UPDATE_PULLREQUEST = "${url('pullrequest_update',repo_name=c.repo_name,pull_request_id=c.pull_request.pull_request_id)}"
+
+    pyroutes.register('pullrequest_comment', "${url('pullrequest_comment',repo_name='%(repo_name)s',pull_request_id='%(pull_request_id)s')}", ['repo_name', 'pull_request_id']);
+    pyroutes.register('pullrequest_comment_delete', "${url('pullrequest_comment_delete',repo_name='%(repo_name)s',comment_id='%(comment_id)s')}", ['repo_name', 'comment_id']);
+    pyroutes.register('pullrequest_update', "${url('pullrequest_update',repo_name='%(repo_name)s',pull_request_id='%(pull_request_id)s')}", ['repo_name', 'pull_request_id']);
+    
     </script>
 
     ## diff block
@@ -228,7 +233,7
           renderInlineComments(file_comments);
 
           YUE.on(YUD.get('update_pull_request'),'click',function(e){
-              updateReviewers();
+              updateReviewers(undefined, "${c.repo_name}", "${c.pull_request.pull_request_id}");
           })
       })
     </script>