Notifications cleanups

configs/.env

@@ -39,6 +39,7 @@ SLACK_BOT_USERNAME="runnabot"
 HIPCHAT_BOT_USERNAME="runnabot"
 ENABLE_BUILDS_ON_GIT_PUSH=false
 ENABLE_NOTIFICATIONS_ON_GIT_PUSH=false
+ENABLE_NEW_BRANCH_BUILDS_ON_GIT_PUSH=false
 POLL_MONGO_TIMEOUT="30 minutes"
 GITHUB_SCOPE="user:email,repo,repo_deployment,read:repo_hook"
 GITHUB_HOOK_SECRET="3V3RYTHINGisAW3S0ME!"

configs/.env.production

@@ -30,4 +30,5 @@ CAYLEY="http://cayley.runnable.io"
 DOCKER_IMAGE_BUILDER_CACHE="/git-cache"
 ENABLE_BUILDS_ON_GIT_PUSH=true
 ENABLE_NOTIFICATIONS_ON_GIT_PUSH=true
+ENABLE_NEW_BRANCH_BUILDS_ON_GIT_PUSH=false
 GITHUB_DEPLOY_KEYS_POOL_SIZE=100

lib/models/notifications/notifier.js

@@ -24,9 +24,12 @@ Handlebars.registerHelper('moreChangesSlack', function(repo, commitLog) {
 
 
 Handlebars.registerHelper('encode', function (str) {
-  return encodeURIComponent(str);
+  // we do double encoding here for angular because
+  // browser would automatically replace `%2F` to `/` and angular router will fail
+  return encodeURIComponent(encodeURIComponent(str));
 });
 
+
 /**
  * Slack requires light escaping with just 3 rules:
  * & replaced with &

lib/routes/actions/github.js

@@ -60,17 +60,22 @@ app.post('/actions/github/',
         mw.res.status(202),
         mw.res.send('Deleted the branch; no work to be done.')),
     parseGitHubPushData,
-    instances.findInstancesLinkedToBranch('githubPushInfo.repo', 'githubPushInfo.branch'),
-    // check if there are instances that follow specific branch
-    mw.req('instances.length').validate(validations.equals(0))
+    mw.req('githubPushInfo.commitLog.length').validate(validations.equals(0))
       .then(
-        // no instances found. This can be push to the new branch
-        newBranch()
-      )
+        mw.res.status(202),
+        mw.res.send('No commits pushed; no work to be done.'))
       .else(
-        // instances following particular branch were found. Redeploy them with the new code
-        followBranch('instances')
-      )
+        instances.findInstancesLinkedToBranch('githubPushInfo.repo', 'githubPushInfo.branch'),
+        // check if there are instances that follow specific branch
+        mw.req('instances.length').validate(validations.equals(0))
+          .then(
+            // no instances found. This can be push to the new branch
+            newBranch()
+          )
+          .else(
+            // instances following particular branch were found. Redeploy them with the new code
+            followBranch('instances')
+          ))
   ),
   mw.res.status(501),
   mw.res.send('No action set up for that payload.'));
@@ -88,14 +93,24 @@ function parseGitHubPushData (req, res, next) {
     branch  : req.body.ref.replace('refs/heads/', ''),
     commit  : req.body.head_commit.id,
     headCommit: req.body.head_commit,
-    commitLog : req.body.commits,
+    commitLog : req.body.commits || [],
     user: req.body.sender
   };
   next();
 }
 
 function newBranch () {
   return flow.series(
+    function (req, res, next) {
+      // our env parsing cannot parse boolean correctly atm
+      if (process.env.ENABLE_NEW_BRANCH_BUILDS_ON_GIT_PUSH !== 'true') {
+        res.status(202);
+        res.send('New branch builds are disabled for now');
+      } else {
+        next();
+      }
+    },
+
     // TODO: ask praful again about creating builds for all branches
     instances.findContextVersionsForRepo('githubPushInfo.repo'),
     mw.req().set('contextVersionIds', 'instances'),

test/actions-github.js

@@ -1,3 +1,4 @@
+'use strict';
 var Lab = require('lab');
 var describe = Lab.experiment;
 var it = Lab.test;
@@ -76,10 +77,10 @@ describe('Github - /actions/github', function () {
       process.env.ENABLE_BUILDS_ON_GIT_PUSH = ctx.originalBuildsOnPushSetting;
       done();
     });
-    it('should just say hi if hooks are disabled', function (done) {
+    it('should send response immediately if hooks are disabled', function (done) {
       var options = hooks(ctx.contextVersion.json()).push;
       options.json.ref = 'refs/heads/someotherbranch';
-      // require('./fixtures/mocks/github/users-username')(101, 'bkendall');
+      require('./fixtures/mocks/github/users-username')(101, 'bkendall');
       request.post(options, function (err, res) {
         if (err) {
           done(err);
@@ -93,6 +94,55 @@ describe('Github - /actions/github', function () {
     });
   });
 
+  describe('ignore hooks without commits data', function () {
+    var ctx = {};
+    beforeEach(function (done) {
+      process.env.ENABLE_BUILDS_ON_GIT_PUSH = 'true';
+      multi.createInstance(function (err, instance, build, user, modelsArr) {
+        ctx.contextVersion = modelsArr[0];
+        ctx.context = modelsArr[1];
+        ctx.build = build;
+        ctx.user = user;
+        done(err);
+      });
+    });
+
+    it('should send response immediately there are no commits data ([]) in the payload ', function (done) {
+      var options = hooks(ctx.contextVersion.json()).push;
+      options.json.ref = 'refs/heads/someotherbranch';
+      options.json.commits = [];
+      require('./fixtures/mocks/github/users-username')(101, 'bkendall');
+      request.post(options, function (err, res) {
+        if (err) {
+          done(err);
+        }
+        else {
+          expect(res.statusCode).to.equal(202);
+          expect(res.body).to.equal('No commits pushed; no work to be done.');
+          done();
+        }
+      });
+    });
+
+    it('should send response immediately there are no commits data (null) in the payload ', function (done) {
+      var options = hooks(ctx.contextVersion.json()).push;
+      options.json.ref = 'refs/heads/someotherbranch';
+      options.json.commits = null;
+      require('./fixtures/mocks/github/users-username')(101, 'bkendall');
+      request.post(options, function (err, res) {
+        if (err) {
+          done(err);
+        }
+        else {
+          expect(res.statusCode).to.equal(202);
+          expect(res.body).to.equal('No commits pushed; no work to be done.');
+          done();
+        }
+      });
+    });
+  });
+
+
   // describe('push', function () {
   //   var ctx = {};
   //   beforeEach(function (done) {

unit/notifier.js

@@ -47,7 +47,7 @@ describe('Notifier',  function () {
       message += '<' + headCommit.url + '|changes>';
       message += ' (init &amp; commit &amp; push) to CodeNow/api (develop) are ready.\n';
       message += '<http://runnable3.net/';
-      message += 'podviaznikov/boxSelection/api/develop/init%20%26%20commit%20%26%20push';
+      message += 'podviaznikov/boxSelection/api/develop/init%2520%2526%2520commit%2520%2526%2520push';
       message += '/a240edf982d467201845b3bf10ccbe16f6049ea9';
       message += '|Choose a server to run develop>.';
       expect(text).to.equal(message);
@@ -77,14 +77,51 @@ describe('Notifier',  function () {
     slack.notifyOnBuild(githubPushInfo, done);
   });
 
+  it('should render proper escaped branch name and commit message on slack.notifyOnBuild call', function (done) {
+    var slack = new Slack({});
+    slack.send = function (text, cb) {
+      var message = 'podviaznikov\'s ';
+      message += '<' + headCommit.url + '|changes>';
+      message += ' (init &amp; commit &amp; push) to CodeNow/api (feature-1/fix) are ready.\n';
+      message += '<http://runnable3.net/';
+      message += 'podviaznikov/boxSelection/api/feature-1%252Ffix/init%2520%2526%2520commit%2520%2526%2520push';
+      message += '/a240edf982d467201845b3bf10ccbe16f6049ea9';
+      message += '|Choose a server to run feature-1/fix>.';
+      expect(text).to.equal(message);
+      cb();
+    };
+
+    var headCommit = {
+      id: 'a240edf982d467201845b3bf10ccbe16f6049ea9',
+      message: 'init & commit & push',
+      url: 'https://github.com/CodeNow/api/commit/a240edf982d467201845b3bf10ccbe16f6049ea9'
+    };
+    var githubPushInfo = {
+      commitLog: [headCommit],
+      repo: 'CodeNow/api',
+      repoName: 'api',
+      branch: 'feature-1/fix',
+      commit: 'a240edf982d467201845b3bf10ccbe16f6049ea9',
+      headCommit: headCommit,
+      user: {
+        login: 'podviaznikov'
+      },
+      owner: {
+        login: 'podviaznikov'
+      }
+    };
+
+    slack.notifyOnBuild(githubPushInfo, done);
+  });
+
   it('should render proper text on slack.notifyOnInstances call', function (done) {
     var slack = new Slack({});
     slack.send = function (message, cb) {
       var text = 'tjmehta\'s ';
       text += '<' + headCommit.url + '|changes>';
       text += ' (init &amp; commit &lt;p&gt;Hello&lt;/p&gt; and  ';
       text += '<https://github.com/CodeNow/api/compare/b240edf982d4...a240edf982d4|1 more>)';
-      text += ' to CodeNow/api (develop) are deployed on servers:';
+      text += ' to CodeNow/api (feature-1/fix) are deployed on servers:';
       expect(text).to.equal(message.text);
       expect(message.attachments.length).to.equal(1);
       var attachment = message.attachments[0];
@@ -116,7 +153,7 @@ describe('Notifier',  function () {
         }],
       repo: 'CodeNow/api',
       repoName: 'api',
-      branch: 'develop',
+      branch: 'feature-1/fix',
       commit: 'a240edf982d46720,1845b3bf10ccbe16f6049ea9',
       headCommit: headCommit,
       user: {
@@ -131,9 +168,9 @@ describe('Notifier',  function () {
     hipchat.send = function (text, cb) {
       var message = 'podviaznikov\'s ';
       message += '<a href="' + headCommit.url + '">changes</a>';
-      message += ' (hey there) to Runnable/api (develop) are ready.\n';
-      message += '<a href="http://runnable3.net/podviaznikov/boxSelection/api/develop';
-      message += '/hey%20there/a240edf982d467201845b3bf10ccbe16f6049ea9">Choose a server to run develop</a>.';
+      message += ' (hey there) to Runnable/api (feature-1/fix) are ready.\n';
+      message += '<a href="http://runnable3.net/podviaznikov/boxSelection/api/feature-1%252Ffix';
+      message += '/hey%2520there/a240edf982d467201845b3bf10ccbe16f6049ea9">Choose a server to run feature-1/fix</a>.';
       expect(text).to.equal(message);
       cb();
     };
@@ -146,7 +183,7 @@ describe('Notifier',  function () {
       commitLog: [headCommit],
       repo: 'Runnable/api',
       repoName: 'api',
-      branch: 'develop',
+      branch: 'feature-1/fix',
       commit: 'a240edf982d467201845b3bf10ccbe16f6049ea9',
       headCommit: headCommit,
       user: {
@@ -164,7 +201,7 @@ describe('Notifier',  function () {
     hipchat.send = function (text, cb) {
       var message = 'podviaznikov\'s ';
       message += '<a href="' + headCommit.url + '">changes</a>';
-      message += ' (init) to CodeNow/api (develop) are deployed on servers:<br/>\n ';
+      message += ' (init) to CodeNow/api (feature-1/fix) are deployed on servers:<br/>\n ';
       message += '<a href="http://runnable3.net/podviaznikov/instance1">instance1</a><br/>\n ';
       message += '<a href="http://runnable3.net/podviaznikov/instance2">instance2</a><br/>\n';
 
@@ -180,7 +217,7 @@ describe('Notifier',  function () {
       commitLog: [headCommit],
       repo: 'CodeNow/api',
       repoName: 'api',
-      branch: 'develop',
+      branch: 'feature-1/fix',
       commit: 'a240edf982d467201845b3bf10ccbe16f6049ea9',
       headCommit: headCommit,
       user: {
@@ -252,7 +289,7 @@ describe('Notifier',  function () {
           });
           done();
         });
-      }, 2200);
+      }, 2500);
     });
   });
 });