Automatically add owners as read-only collaborators.

Author: tobie

lib/comment.js

@@ -1,12 +1,7 @@
 "use strict";
 var github = require('./github');
 var q = require('q');
-var funk = require("./funk");
 var labelModel = require("./label-model");
-
-var NO_COLAB = "None of the owners of this pull request are repo collaborators. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing ([web client](http://irc.w3.org/?channels=testing)) to get someone to merge this or get added as a collaborator. Thank you!";
-    
-var NO_OWNERS = "There are no owners for this pull request. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing ([web client](http://irc.w3.org/?channels=testing)) to get help with this. Thank you!";
 
 function promise(value) {
     var deferred = q.defer();
@@ -15,66 +10,32 @@ function promise(value) {
 }
 
 module.exports = function(number, metadata) {
-    if (metadata.reviewers.length || metadata.reviewedUpstream) { // If there are reviewers we abord early. They know best.
+    if (metadata.reviewers.length || metadata.reviewedUpstream) {
+        // If there are reviewers we abord early. They know best.
         return promise();
-    } else {
-        // if there are collaborators, we ask them for a review
-        if (metadata.collaborators.length > 0) {
-            return requestReview(number, metadata.collaborators).then(function() {
-                // And we ping the non-collaborators in a comment.
-                if (metadata.nonCollaborators.length > 0) {
-                    var msg = msgToNonCollaborators(metadata);
-                    return comment(number, msg);
-                }
-                return;
-            });
-        } else if (metadata.authorIsCollaborator) {
-            // There are no collaborators outside of the author.
-            if (metadata.nonCollaborators.length > 0) {
-                var msg = msgToNonCollaborators(metadata);
-                return comment(number, msg);
+    }
+
+    // if there are collaborators, we ask them for a review
+    if (metadata.ownersExcludingAuthor.length > 0) {
+        var reviewers = metadata.ownersExcludingAuthor.map(function(owner) { return owner.login });
+        return  github.post('/repos/:owner/:repo/pulls/:number/requested_reviewers', {
+            reviewers: reviewers
+        }, { number: number }).then(function() {
+            if (!metadata.isMergeable) {
+                return labelAndComment("status:needs-collaborator", "No owner on this pull request has write-access to the repository.");
             }
-            return promise();
-        } else if (metadata.nonCollaborators.length > 0) {
-            // We've no collaborators but some owners.
-            var msg = NO_COLAB + "\n\n" + msgToNonCollaborators(metadata);
-            return labelModel.post(number, ["status:needs-collaborator"]).then(function() {
-                return comment(number, msg);
-            });
-        } else {
-            return labelModel.post(number, ["status:needs-owners"]).then(function() {
-                return comment(number, NO_OWNERS);
-            });
+        });
+    } else {
+        if (metadata.author.isOwner) {
+            return labelAndComment("status:needs-owners", "There are no owners for this pull request besides its author.");
         }
+        return labelAndComment("status:needs-owners", "There are no owners for this pull request.");
     }
 };
 
-function comment(number, body) {
-    return github.post('/repos/:owner/:repo/issues/:number/comments', { body: body }, { number: number });
-}
-
-function requestReview(number, collaborators) {
-    return github.post('/repos/:owner/:repo/pulls/:number/requested_reviewers', {
-        reviewers: collaborators
-    }, { number: number })
-}
-
-function msgToNonCollaborators(metadata) {
-    var handles = metadata.nonCollaborators.map(function(nonCollab) { return "@" + nonCollab; });
-    return "Notifying owners who are not repo collaborators: " + join(handles) + ".";
-}
-
-module.exports.join = join;
-function join(owners) {
-	if (owners.length == 0) {
-		return "";
-	}
-	if (owners.length == 1) {
-		return owners[0];
-	}
-	if (owners.length == 2) {
-		return owners[0] + " and " + owners[1];
-	}
-	var last = owners.pop();
-	return owners.join(", ") + ", and " + last;
-}
+function labelAndComment(label, msg) {
+    msg += " Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing ([web client](http://irc.w3.org/?channels=testing)) to get help with this. Thank you!";
+    return labelModel.post(number, [label]).then(function() {
+        return github.post('/repos/:owner/:repo/issues/:number/comments', { body: msg }, { number: number });
+    });
+}

lib/github.js

@@ -21,19 +21,27 @@ function getHeaders() {
 }
 
 function post(url, body, options) {
+    return _post("POST", url, body, options);
+}
+
+function put(url, body, options) {
+    return _post("PUT", url, body, options);
+}
+
+function _post(method, url, body, options) {
     options = mergeOptions(options || {});
     url = replaceURL(baseURL(url), options);
     var deferred = q.defer();
     body = typeof body == "string" ? body : JSON.stringify(body)
-    console.log("POST " + url + "\n" + body);
-    request.post({
+    console.log(method + " " + url + "\n" + body);
+    request[method.toLowerCase()]({
         url: url,
         headers: getHeaders(),
         body: body
     }, function(err, response, body) {
         if (err) {
             deferred.reject(err);
-        } else if (response.statusCode == 200) {
+        } else if (response.statusCode >= 200 && response.statusCode < 300) {
             deferred.resolve(JSON.parse(body));
         } else {
             deferred.reject(errFrom(body));
@@ -51,7 +59,7 @@ function get(url, options) {
     function onResponse(err, response, body) {
         if (err) {
             deferred.reject(err);
-        } else if (response.statusCode == 200) {
+        } else if (response.statusCode >= 200 && response.statusCode < 300) {
             var link = response.headers.link;
 	    	body = JSON.parse(body);
             if (link) {
@@ -116,4 +124,5 @@ function replaceURL(url, options) {
 }
 
 exports.get = get;
-exports.post = post;
+exports.post = post;
+exports.put = put;

lib/metadata/check-author-status.js

@@ -3,20 +3,17 @@
 var filenames = require('./filenames'),
     paths = require('./paths'),
     rawLabels = require('./raw-labels'),
-//    isSafe = require('./is-safe'),
-//    testUrls = require('./test-urls'),
     findSpecs = require('./find-specs'),
     findOwners = require('./find-owners'),
     wg = require('./wg'),
-    checkAuthorStatus = require('./check-author-status'),
+    status = require('./status'),
     labels = require('./labels'),
     funk = require('../funk'),
     github = require('../github');
 
 module.exports = function getMetadada(number, author, content) {
     var metadata = {
-        issue: number,
-        author: author
+        issue: number
     };
 
     return filenames(number)
@@ -25,46 +22,45 @@ module.exports = function getMetadada(number, author, content) {
             metadata.filenamesIgnoreRemoved = filenames.ignoreRemoved;
             metadata.paths = paths(metadata.filenames);
             metadata.rawLabels = rawLabels(metadata.filenames);
-//            metadata.isSafe = isSafe(filenames.ignoreRemoved);
-//            metadata.testUrls = testUrls(number, filenames.ignoreRemoved, metadata.paths);
             return findSpecs(metadata.rawLabels);
         }).then(function(specs) {
             metadata.specs = specs;
             metadata.workingGroups = wg(specs);
             metadata.labels = labels(metadata);
         }).then(function() {
-            return findOwners(metadata.paths);
+            return findOwners(metadata.paths)
         }).then(function(owners) {
-            metadata.owners = owners; 
+            return status(owners);
+        }).then(function(owners) {
+            metadata.owners = owners;
         }).then(function() {
-            return github.get("/repos/:owner/:repo/collaborators").then(function(collaborators) {
-                collaborators = collaborators.map(funk.prop("login"));
-                
-                metadata.authorIsCollaborator = collaborators.some(function(login) {
-                    return login == metadata.author;
-                });
-                
-                var _owners = metadata.owners.filter(function(owner) {
-                    return owner != metadata.author;
-                })
-                
-                // owners which have commit rights to the repo but are not the author
-                metadata.collaborators = _owners.filter(function(owner) {
-                    return collaborators.indexOf(owner) > -1;
-                });
-                
-                // owners which do not have commit rights to the repo and are not the author
-                metadata.nonCollaborators = _owners.filter(function(owner) {
-                    return collaborators.indexOf(owner) < 0;
+            return status(author);
+        }).then(function(status) {
+            metadata.author = {
+                login: author,
+                permission: permission
+            };
+        }).then(function() {
+            metadata.isMergeable = metadata.author.permission == "admin" ||
+                metadata.author.permission == "write" ||
+                metadata.owners.some(function(owner) {
+                    return owner.permission == "admin" || owner.permission == "write";
                 });
+
+            metadata.ownersExcludingAuthor = metadata.owners.filter(function(owner) {
+                return owner.login != metadata.author.login;
+            });
+
+            metadata.author.isOwner = metadata.owners.some(function(owner) {
+                return owner.login == metadata.author.login;
             });
         }).then(function() {
             return github.get('/repos/:owner/:repo/pulls/:number/requested_reviewers', { number: number }).then(function(reviewers) {
                 metadata.reviewers = reviewers.map(function(r) { return r.login }).sort();
             });
         }).then(function() {
-            metadata.reviewedUpstream = (metadata.author == "chromium-wpt-export-bot") ||
-                (metadata.author == "jgraham" && content.indexOf("MozReview-Commit-ID") > -1);
+            metadata.reviewedUpstream = (metadata.author.login == "chromium-wpt-export-bot") ||
+                (metadata.author.login == "jgraham" && content.indexOf("MozReview-Commit-ID") > -1);
             return metadata;
         });        
 };

lib/metadata/index.js

@@ -0,0 +1,47 @@
+"use strict";
+
+var github = require("../github");
+var q = require('q');
+    
+function promise(value) {
+    var deferred = q.defer();
+    deferred.resolve(value);
+    return deferred.promise;
+}
+
+module.exports = function(arg) {
+    return typeof arg == "string" ? _status(arg) : _statuses(arg);
+}
+
+function _statuses(handles) {
+    handles = handles.slice(0);
+    var output = [];
+    var deferred = q.defer();
+    
+    function next() {
+        if (handles.length > 0) {
+            var handle = handles.pop();
+            _status(handle).then(function(permission) {
+                output.push({ login: handle, permission: permission });
+            }, function(err) { deferred.reject(err) });
+        } else {
+            deferred.resolve(output);
+        }
+    }
+    next();
+    return deferred.promise;
+}
+
+
+function _status(handle) {
+    return github.get("/repos/:owner/:repo/collaborators/:username", { username: handle }).then(function() {
+        return github.get("/repos/:owner/:repo/collaborators/:username/permission", { username: handle }).then(function(data) {
+            return data.permission;
+        });
+    }, function() {
+        return github.put('/repos/:owner/:repo/collaborators/:username', { permission: "pull" }, { username: username }).then(function() {
+            return "read";
+        });
+    });
+}
+