Skip to content

Commit f06de0b

Browse files
tjmehtatjmehta
committed
Merge pull request #490 from CodeNow/notifications-cleanups
Notifications cleanups
2 parents aeeabfc + da7bbc5 commit f06de0b

File tree

6 files changed

+130
-23
lines changed

6 files changed

+130
-23
lines changed

‎configs/.env‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ SLACK_BOT_USERNAME="runnabot"
3939
HIPCHAT_BOT_USERNAME="runnabot"
4040
ENABLE_BUILDS_ON_GIT_PUSH=false
4141
ENABLE_NOTIFICATIONS_ON_GIT_PUSH=false
42+
ENABLE_NEW_BRANCH_BUILDS_ON_GIT_PUSH=false
4243
POLL_MONGO_TIMEOUT="30 minutes"
4344
GITHUB_SCOPE="user:email,repo,repo_deployment,read:repo_hook"
4445
GITHUB_HOOK_SECRET="3V3RYTHINGisAW3S0ME!"

‎configs/.env.production‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,5 @@ CAYLEY="http://cayley.runnable.io"
3030
DOCKER_IMAGE_BUILDER_CACHE="/git-cache"
3131
ENABLE_BUILDS_ON_GIT_PUSH=true
3232
ENABLE_NOTIFICATIONS_ON_GIT_PUSH=true
33+
ENABLE_NEW_BRANCH_BUILDS_ON_GIT_PUSH=false
3334
GITHUB_DEPLOY_KEYS_POOL_SIZE=100

‎lib/models/notifications/notifier.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,12 @@ Handlebars.registerHelper('moreChangesSlack', function(repo, commitLog) {
2424

2525

2626
Handlebars.registerHelper('encode', function (str) {
27-
return encodeURIComponent(str);
27+
// we do double encoding here for angular because
28+
// browser would automatically replace `%2F` to `/` and angular router will fail
29+
return encodeURIComponent(encodeURIComponent(str));
2830
});
2931

32+
3033
/**
3134
* Slack requires light escaping with just 3 rules:
3235
* & replaced with &

‎lib/routes/actions/github.js‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,22 @@ app.post('/actions/github/',
6060
mw.res.status(202),
6161
mw.res.send('Deleted the branch; no work to be done.')),
6262
parseGitHubPushData,
63-
instances.findInstancesLinkedToBranch('githubPushInfo.repo', 'githubPushInfo.branch'),
64-
// check if there are instances that follow specific branch
65-
mw.req('instances.length').validate(validations.equals(0))
63+
mw.req('githubPushInfo.commitLog.length').validate(validations.equals(0))
6664
.then(
67-
// no instances found. This can be push to the new branch
68-
newBranch()
69-
)
65+
mw.res.status(202),
66+
mw.res.send('No commits pushed; no work to be done.'))
7067
.else(
71-
// instances following particular branch were found. Redeploy them with the new code
72-
followBranch('instances')
73-
)
68+
instances.findInstancesLinkedToBranch('githubPushInfo.repo', 'githubPushInfo.branch'),
69+
// check if there are instances that follow specific branch
70+
mw.req('instances.length').validate(validations.equals(0))
71+
.then(
72+
// no instances found. This can be push to the new branch
73+
newBranch()
74+
)
75+
.else(
76+
// instances following particular branch were found. Redeploy them with the new code
77+
followBranch('instances')
78+
))
7479
),
7580
mw.res.status(501),
7681
mw.res.send('No action set up for that payload.'));
@@ -88,14 +93,24 @@ function parseGitHubPushData (req, res, next) {
8893
branch : req.body.ref.replace('refs/heads/', ''),
8994
commit : req.body.head_commit.id,
9095
headCommit: req.body.head_commit,
91-
commitLog : req.body.commits,
96+
commitLog : req.body.commits || [],
9297
user: req.body.sender
9398
};
9499
next();
95100
}
96101

97102
function newBranch () {
98103
return flow.series(
104+
function (req, res, next) {
105+
// our env parsing cannot parse boolean correctly atm
106+
if (process.env.ENABLE_NEW_BRANCH_BUILDS_ON_GIT_PUSH !== 'true') {
107+
res.status(202);
108+
res.send('New branch builds are disabled for now');
109+
} else {
110+
next();
111+
}
112+
},
113+
99114
// TODO: ask praful again about creating builds for all branches
100115
instances.findContextVersionsForRepo('githubPushInfo.repo'),
101116
mw.req().set('contextVersionIds', 'instances'),

‎test/actions-github.js‎

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
'use strict';
12
var Lab = require('lab');
23
var describe = Lab.experiment;
34
var it = Lab.test;
@@ -76,10 +77,10 @@ describe('Github - /actions/github', function () {
7677
process.env.ENABLE_BUILDS_ON_GIT_PUSH = ctx.originalBuildsOnPushSetting;
7778
done();
7879
});
79-
it('should just say hi if hooks are disabled', function (done) {
80+
it('should send response immediately if hooks are disabled', function (done) {
8081
var options = hooks(ctx.contextVersion.json()).push;
8182
options.json.ref = 'refs/heads/someotherbranch';
82-
// require('./fixtures/mocks/github/users-username')(101, 'bkendall');
83+
require('./fixtures/mocks/github/users-username')(101, 'bkendall');
8384
request.post(options, function (err, res) {
8485
if (err) {
8586
done(err);
@@ -93,6 +94,55 @@ describe('Github - /actions/github', function () {
9394
});
9495
});
9596

97+
describe('ignore hooks without commits data', function () {
98+
var ctx = {};
99+
beforeEach(function (done) {
100+
process.env.ENABLE_BUILDS_ON_GIT_PUSH = 'true';
101+
multi.createInstance(function (err, instance, build, user, modelsArr) {
102+
ctx.contextVersion = modelsArr[0];
103+
ctx.context = modelsArr[1];
104+
ctx.build = build;
105+
ctx.user = user;
106+
done(err);
107+
});
108+
});
109+
110+
it('should send response immediately there are no commits data ([]) in the payload ', function (done) {
111+
var options = hooks(ctx.contextVersion.json()).push;
112+
options.json.ref = 'refs/heads/someotherbranch';
113+
options.json.commits = [];
114+
require('./fixtures/mocks/github/users-username')(101, 'bkendall');
115+
request.post(options, function (err, res) {
116+
if (err) {
117+
done(err);
118+
}
119+
else {
120+
expect(res.statusCode).to.equal(202);
121+
expect(res.body).to.equal('No commits pushed; no work to be done.');
122+
done();
123+
}
124+
});
125+
});
126+
127+
it('should send response immediately there are no commits data (null) in the payload ', function (done) {
128+
var options = hooks(ctx.contextVersion.json()).push;
129+
options.json.ref = 'refs/heads/someotherbranch';
130+
options.json.commits = null;
131+
require('./fixtures/mocks/github/users-username')(101, 'bkendall');
132+
request.post(options, function (err, res) {
133+
if (err) {
134+
done(err);
135+
}
136+
else {
137+
expect(res.statusCode).to.equal(202);
138+
expect(res.body).to.equal('No commits pushed; no work to be done.');
139+
done();
140+
}
141+
});
142+
});
143+
});
144+
145+
96146
// describe('push', function () {
97147
// var ctx = {};
98148
// beforeEach(function (done) {

‎unit/notifier.js‎

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ describe('Notifier', function () {
4747
message += '<' + headCommit.url + '|changes>';
4848
message += ' (init &amp; commit &amp; push) to CodeNow/api (develop) are ready.\n';
4949
message += '<http://runnable3.net/';
50-
message += 'podviaznikov/boxSelection/api/develop/init%20%26%20commit%20%26%20push';
50+
message += 'podviaznikov/boxSelection/api/develop/init%2520%2526%2520commit%2520%2526%2520push';
5151
message += '/a240edf982d467201845b3bf10ccbe16f6049ea9';
5252
message += '|Choose a server to run develop>.';
5353
expect(text).to.equal(message);
@@ -77,14 +77,51 @@ describe('Notifier', function () {
7777
slack.notifyOnBuild(githubPushInfo, done);
7878
});
7979

80+
it('should render proper escaped branch name and commit message on slack.notifyOnBuild call', function (done) {
81+
var slack = new Slack({});
82+
slack.send = function (text, cb) {
83+
var message = 'podviaznikov\'s ';
84+
message += '<' + headCommit.url + '|changes>';
85+
message += ' (init &amp; commit &amp; push) to CodeNow/api (feature-1/fix) are ready.\n';
86+
message += '<http://runnable3.net/';
87+
message += 'podviaznikov/boxSelection/api/feature-1%252Ffix/init%2520%2526%2520commit%2520%2526%2520push';
88+
message += '/a240edf982d467201845b3bf10ccbe16f6049ea9';
89+
message += '|Choose a server to run feature-1/fix>.';
90+
expect(text).to.equal(message);
91+
cb();
92+
};
93+
94+
var headCommit = {
95+
id: 'a240edf982d467201845b3bf10ccbe16f6049ea9',
96+
message: 'init & commit & push',
97+
url: 'https://github.com/CodeNow/api/commit/a240edf982d467201845b3bf10ccbe16f6049ea9'
98+
};
99+
var githubPushInfo = {
100+
commitLog: [headCommit],
101+
repo: 'CodeNow/api',
102+
repoName: 'api',
103+
branch: 'feature-1/fix',
104+
commit: 'a240edf982d467201845b3bf10ccbe16f6049ea9',
105+
headCommit: headCommit,
106+
user: {
107+
login: 'podviaznikov'
108+
},
109+
owner: {
110+
login: 'podviaznikov'
111+
}
112+
};
113+
114+
slack.notifyOnBuild(githubPushInfo, done);
115+
});
116+
80117
it('should render proper text on slack.notifyOnInstances call', function (done) {
81118
var slack = new Slack({});
82119
slack.send = function (message, cb) {
83120
var text = 'tjmehta\'s ';
84121
text += '<' + headCommit.url + '|changes>';
85122
text += ' (init &amp; commit &lt;p&gt;Hello&lt;/p&gt; and ';
86123
text += '<https://github.com/CodeNow/api/compare/b240edf982d4...a240edf982d4|1 more>)';
87-
text += ' to CodeNow/api (develop) are deployed on servers:';
124+
text += ' to CodeNow/api (feature-1/fix) are deployed on servers:';
88125
expect(text).to.equal(message.text);
89126
expect(message.attachments.length).to.equal(1);
90127
var attachment = message.attachments[0];
@@ -116,7 +153,7 @@ describe('Notifier', function () {
116153
}],
117154
repo: 'CodeNow/api',
118155
repoName: 'api',
119-
branch: 'develop',
156+
branch: 'feature-1/fix',
120157
commit: 'a240edf982d46720,1845b3bf10ccbe16f6049ea9',
121158
headCommit: headCommit,
122159
user: {
@@ -131,9 +168,9 @@ describe('Notifier', function () {
131168
hipchat.send = function (text, cb) {
132169
var message = 'podviaznikov\'s ';
133170
message += '<a href="' + headCommit.url + '">changes</a>';
134-
message += ' (hey there) to Runnable/api (develop) are ready.\n';
135-
message += '<a href="http://runnable3.net/podviaznikov/boxSelection/api/develop';
136-
message += '/hey%20there/a240edf982d467201845b3bf10ccbe16f6049ea9">Choose a server to run develop</a>.';
171+
message += ' (hey there) to Runnable/api (feature-1/fix) are ready.\n';
172+
message += '<a href="http://runnable3.net/podviaznikov/boxSelection/api/feature-1%252Ffix';
173+
message += '/hey%2520there/a240edf982d467201845b3bf10ccbe16f6049ea9">Choose a server to run feature-1/fix</a>.';
137174
expect(text).to.equal(message);
138175
cb();
139176
};
@@ -146,7 +183,7 @@ describe('Notifier', function () {
146183
commitLog: [headCommit],
147184
repo: 'Runnable/api',
148185
repoName: 'api',
149-
branch: 'develop',
186+
branch: 'feature-1/fix',
150187
commit: 'a240edf982d467201845b3bf10ccbe16f6049ea9',
151188
headCommit: headCommit,
152189
user: {
@@ -164,7 +201,7 @@ describe('Notifier', function () {
164201
hipchat.send = function (text, cb) {
165202
var message = 'podviaznikov\'s ';
166203
message += '<a href="' + headCommit.url + '">changes</a>';
167-
message += ' (init) to CodeNow/api (develop) are deployed on servers:<br/>\n ';
204+
message += ' (init) to CodeNow/api (feature-1/fix) are deployed on servers:<br/>\n ';
168205
message += '<a href="http://runnable3.net/podviaznikov/instance1">instance1</a><br/>\n ';
169206
message += '<a href="http://runnable3.net/podviaznikov/instance2">instance2</a><br/>\n';
170207

@@ -180,7 +217,7 @@ describe('Notifier', function () {
180217
commitLog: [headCommit],
181218
repo: 'CodeNow/api',
182219
repoName: 'api',
183-
branch: 'develop',
220+
branch: 'feature-1/fix',
184221
commit: 'a240edf982d467201845b3bf10ccbe16f6049ea9',
185222
headCommit: headCommit,
186223
user: {
@@ -252,7 +289,7 @@ describe('Notifier', function () {
252289
});
253290
done();
254291
});
255-
}, 2200);
292+
}, 2500);
256293
});
257294
});
258295
});

0 commit comments

Comments
 (0)