Skip to content

Commit d7365d5

Browse files
author
anandkumarpatel
committed
Merge pull request #571 from CodeNow/SAN-1048-feature-add-github-username-gravatar-createdby
gravatar and username for createdBy and owner
2 parents 6ccfff2 + 15eb221 commit d7365d5

File tree

11 files changed

+116
-51
lines changed

11 files changed

+116
-51
lines changed

‎lib/models/mongo/instance.js‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -49,40 +49,57 @@ InstanceSchema.statics.findByBuild = function (build /*, args*/) {
4949
Instance.find.apply(Instance, args);
5050
};
5151

52-
InstanceSchema.methods.getGithubUsername = function (sessionUser, cb) {
52+
InstanceSchema.methods.populateOwnerAndCreatedBy = function (sessionUser, cb) {
5353
var instance = this;
54-
sessionUser.findGithubUserByGithubId(this.owner.github, function (err, user) {
54+
async.parallel({
55+
owner: sessionUser.findGithubUserByGithubId.bind(sessionUser, this.owner.github),
56+
createdBy: sessionUser.findGithubUserByGithubId.bind(sessionUser, this.createdBy.github),
57+
}, function (err, data) {
5558
if (err) { return cb(err); }
56-
instance.owner.username = user.login;
57-
cb(err, instance);
59+
instance.owner.username = keypather.get(data, 'owner.login');
60+
instance.owner.gravatar = keypather.get(data, 'owner.avatar_url');
61+
instance.createdBy.username = keypather.get(data, 'createdBy.login');
62+
instance.createdBy.gravatar = keypather.get(data, 'createdBy.avatar_url');
63+
cb(null, instance);
5864
});
5965
};
6066

61-
InstanceSchema.statics.getGithubUsernamesForInstances = function (sessionUser, instances, cb) {
67+
InstanceSchema.statics.populateOwnerAndCreatedByForInstances =
68+
function (sessionUser, instances, cb) {
6269
if (instances.length === 0) {
6370
done();
6471
}
6572
else {
66-
var instancesByGithubId = groupBy(instances, 'owner.github');
67-
var githubIds = Object.keys(instancesByGithubId);
68-
var count = createCount(githubIds.length, done);
69-
githubIds.map(toInt).forEach(function (githubId) {
73+
var instancesByOwnerGithubId = groupBy(instances, 'owner.github');
74+
var ownerGithubIds = Object.keys(instancesByOwnerGithubId);
75+
var instancesByCreatedByGithubId = groupBy(instances, 'createdBy.github');
76+
var createdByGithubIds = Object.keys(instancesByCreatedByGithubId);
77+
async.parallel([
78+
populateField.bind(null, ownerGithubIds, instancesByOwnerGithubId, 'owner'),
79+
populateField.bind(null, createdByGithubIds, instancesByCreatedByGithubId, 'createdBy')
80+
], done);
81+
}
82+
83+
function populateField (keyIds, mapToUpdateList, fieldPath, cb) {
84+
async.each(keyIds.map(toInt), function (githubId, asyncCb) {
7085
sessionUser.findGithubUserByGithubId(githubId, function (err, user) {
71-
var username;
86+
var username = null;
87+
var gravatar = null;
7288
if (err) {
7389
// log error, and continue
7490
error.logIfErr(err);
75-
username = null;
7691
}
7792
else {
7893
username = user.login;
94+
gravatar = user.avatar_url;
7995
}
80-
instancesByGithubId[githubId].forEach(function (instance) {
81-
instance.owner.username = username;
96+
mapToUpdateList[githubId].forEach(function (instance) {
97+
keypather.set(instance, fieldPath + '.username', username);
98+
keypather.set(instance, fieldPath + '.gravatar', gravatar);
8299
});
83-
count.next(); // don't pass error
100+
asyncCb(); // don't pass error
84101
});
85-
});
102+
}, cb);
86103
}
87104
function done (err) {
88105
cb(err, instances);
@@ -96,7 +113,7 @@ InstanceSchema.methods.populateDeps = function (seenNodes, cb) {
96113
}
97114
var self = this;
98115
var graph = new Graph();
99-
var fields = { _id: 1, shortHash: 1, owner: 1 , lowerName: 1, name: 1 };
116+
var fields = { _id: 1, shortHash: 1, owner: 1 , lowerName: 1, name: 1, createdBy: 1 };
100117
var fieldKeys = Object.keys(fields).concat('dependencies');
101118
seenNodes = seenNodes || [];
102119
seenNodes.push(this._id.toString());

‎lib/models/mongo/schemas/instance.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ var InstanceSchema = module.exports = new Schema({
5353
// FIXME: nested validations dont work, validated on pre save
5454
// validate: validators.githubId({ model: 'Instance', literal: 'Github Owner' })
5555
},
56-
username: String // dynamic field for filling in
56+
username: String, // dynamic field for filling in
57+
gravatar: String
5758
}
5859
},
5960
/** @type Object */
@@ -64,7 +65,10 @@ var InstanceSchema = module.exports = new Schema({
6465
type: Number,
6566
// FIXME: nested validations dont work, validated on pre save
6667
// validate: validators.githubId({ model: 'Instance', literal: 'Github CreatedBy' })
67-
}
68+
},
69+
// fields used to pass things around back to the frontend
70+
username: String,
71+
gravatar: String
6872
}
6973
},
7074
/** Instance that this instance was forked from
@@ -217,4 +221,4 @@ InstanceSchema.pre('save', function (next) {
217221
} else {
218222
next();
219223
}
220-
});
224+
});

‎lib/routes/actions/github.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ function followBranch (instancesKey) {
400400
next();
401401
},
402402
timers.model.stopTimer('github_push_event'),
403-
instances.getGithubUsernamesForInstances(
403+
instances.populateOwnerAndCreatedByForInstances(
404404
'instanceCreator', 'deployedInstances'),
405405
heap.create(),
406406
heap.model.track('githubPushInfo.user.id', 'github_hook_autodeploy', {
@@ -453,4 +453,4 @@ function waitForContextVersionBuildCompleted (contextVersionIdsKey) {
453453
next();
454454
}
455455
);
456-
}
456+
}

‎lib/routes/instances/index.js‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,9 @@ app.get('/instances/',
269269
ownerIsHelloRunnable('query')
270270
),
271271
instances.find('query'),
272-
timers.model.startTimer('getGithubUsername'),
273-
instances.getGithubUsernamesForInstances('sessionUser', 'instances'),
274-
timers.model.stopTimer('getGithubUsername'),
272+
timers.model.startTimer('populateOwnerAndCreatedByForInstances'),
273+
instances.populateOwnerAndCreatedByForInstances('sessionUser', 'instances'),
274+
timers.model.stopTimer('populateOwnerAndCreatedByForInstances'),
275275
timers.model.startTimer('instance-route.populateModels'),
276276
instances.models.populateModels(),
277277
timers.model.stopTimer('instance-route.populateModels'),
@@ -348,7 +348,7 @@ app.post('/instances/',
348348
error.logIfErrMw),
349349
mw.next('err') // next error
350350
),
351-
instances.model.getGithubUsername('sessionUser'),
351+
instances.model.populateOwnerAndCreatedBy('sessionUser'),
352352
instances.model.populateModels(),
353353
messenger.emitInstanceUpdate('instance', 'post'),
354354
mw.res.send(201, 'instance'));
@@ -362,7 +362,7 @@ app.get('/instances/:id',
362362
me.isOwnerOf('instance'),
363363
instances.model.isPublic(),
364364
me.isModerator),
365-
instances.model.getGithubUsername('sessionUser'),
365+
instances.model.populateOwnerAndCreatedBy('sessionUser'),
366366
instances.model.populateModels(),
367367
mw.res.json('instance'));
368368

@@ -528,7 +528,7 @@ app.patch('/instances/:id',
528528
instances.model.update({ $set: 'body' }),
529529
instances.findById('instanceId')),
530530
instances.findById('instanceId'),
531-
instances.model.getGithubUsername('sessionUser'),
531+
instances.model.populateOwnerAndCreatedBy('sessionUser'),
532532
mw.body({ or: ['name', 'env'] }).require()
533533
.then(
534534
// acquire lock
@@ -695,7 +695,7 @@ app.post('/instances/:id/actions/deploy',
695695
// create container
696696
createSaveAndNetworkContainer,
697697
graph.create(),
698-
instances.model.getGithubUsername('sessionUser'),
698+
instances.model.populateOwnerAndCreatedBy('sessionUser'),
699699
graph.model.graphInstanceDeps('instance'))
700700
.catch(
701701
mw.req().setToErr('err'),
@@ -755,7 +755,7 @@ app.post('/instances/:id/actions/redeploy',
755755
// create container
756756
createSaveAndNetworkContainer,
757757
graph.create(),
758-
instances.model.getGithubUsername('sessionUser'),
758+
instances.model.populateOwnerAndCreatedBy('sessionUser'),
759759
graph.model.graphInstanceDeps('instance'))
760760
.catch(
761761
mw.req().setToErr('err'),
@@ -937,7 +937,7 @@ app.post('/instances/:id/actions/regraph',
937937
checkFound('user', 'Owner not found'),
938938
mw.req().set('ownerUsername', 'user.login'),
939939
graph.create(),
940-
instances.model.getGithubUsername('sessionUser'),
940+
instances.model.populateOwnerAndCreatedBy('sessionUser'),
941941
graph.model.graphInstanceDeps('instance'),
942942
instances.model.populateModels(),
943943
mw.req().set('instance.owner.username', 'ownerUsername'),

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@
8585
"form-data": "^0.1.4",
8686
"function-proxy": "^0.5.2",
8787
"jsdoc": "^3.2.2",
88-
"jshint": "^2.5.5",
88+
"jshint": "2.6.0",
8989
"krain": "git+ssh://[email protected]:CodeNow/krain.git#v0.0.9",
9090
"lab": "^3.2.1",
9191
"mavis": "git+ssh://[email protected]:CodeNow/mavis#v0.0.3",

‎test/bdd-instance-dependencies.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ describe('BDD - Instance Dependencies', function () {
108108
expect(instance.dependencies[apiId]).to.be.okay;
109109
expect(instance.dependencies[apiId].shortHash).to.equal(ctx.apiInstance.attrs.shortHash);
110110
expect(instance.dependencies[apiId].lowerName).to.equal(ctx.apiInstance.attrs.lowerName);
111+
expect(instance.dependencies[apiId].owner.github).to.equal(ctx.user.attrs.accounts.github.id);
112+
expect(instance.dependencies[apiId].createdBy.github).to.equal(ctx.user.attrs.accounts.github.id);
111113
expect(instance.dependencies[apiId].dependencies).to.equal(undefined);
112114
done();
113115
});

‎test/instances-id-actions-copy/post/index.js‎

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,12 @@ describe('POST /instances/:id/actions/copy', { timeout: 500 }, function () {
4747
shortHash: exists,
4848
name: exists,
4949
public: exists,
50-
createdBy: { github: ctx.user.json().accounts.github.id },
50+
createdBy: { github: ctx.user.json().accounts.github.id,
51+
username: ctx.user.json().accounts.github.username,
52+
gravatar: ctx.user.json().accounts.github.avatar_url },
5153
owner: { github: ctx.user.json().accounts.github.id,
52-
username: ctx.user.json().accounts.github.username },
54+
username: ctx.user.json().accounts.github.username,
55+
gravatar: ctx.user.json().accounts.github.avatar_url },
5356
parent: ctx.instance.id(),
5457
'build._id': ctx.build.id(),
5558
containers: exists
@@ -62,9 +65,12 @@ describe('POST /instances/:id/actions/copy', { timeout: 500 }, function () {
6265
shortHash: exists,
6366
name: 'new-name-fo-shizzle',
6467
public: exists,
65-
createdBy: { github: ctx.user.json().accounts.github.id },
66-
'owner.github': ctx.user.json().accounts.github.id,
67-
'owner.username': ctx.user.json().accounts.github.username,
68+
createdBy: { github: ctx.user.json().accounts.github.id,
69+
username: ctx.user.json().accounts.github.username,
70+
gravatar: ctx.user.json().accounts.github.avatar_url },
71+
owner: { github: ctx.user.json().accounts.github.id,
72+
username: ctx.user.json().accounts.github.username,
73+
gravatar: ctx.user.json().accounts.github.avatar_url },
6874
parent: ctx.instance.id(),
6975
'build._id': ctx.build.id(),
7076
containers: exists
@@ -83,9 +89,12 @@ describe('POST /instances/:id/actions/copy', { timeout: 500 }, function () {
8389
shortHash: exists,
8490
name: exists,
8591
public: exists,
86-
createdBy: { github: ctx.user.json().accounts.github.id },
92+
createdBy: { github: ctx.user.json().accounts.github.id,
93+
username: ctx.user.json().accounts.github.username,
94+
gravatar: ctx.user.json().accounts.github.avatar_url },
8795
owner: { github: ctx.user.json().accounts.github.id,
88-
username: ctx.user.json().accounts.github.username },
96+
username: ctx.user.json().accounts.github.username,
97+
gravatar: ctx.user.json().accounts.github.avatar_url },
8998
parent: ctx.instance.id(),
9099
'build._id': ctx.build.id(),
91100
containers: exists,
@@ -99,9 +108,12 @@ describe('POST /instances/:id/actions/copy', { timeout: 500 }, function () {
99108
shortHash: exists,
100109
name: exists,
101110
public: exists,
102-
createdBy: { github: ctx.user.json().accounts.github.id },
111+
createdBy: { github: ctx.user.json().accounts.github.id,
112+
username: ctx.user.json().accounts.github.username,
113+
gravatar: ctx.user.json().accounts.github.avatar_url },
103114
owner: { github: ctx.user.json().accounts.github.id,
104-
username: ctx.user.json().accounts.github.username },
115+
username: ctx.user.json().accounts.github.username,
116+
gravatar: ctx.user.json().accounts.github.avatar_url },
105117
parent: ctx.instance.id(),
106118
'build._id': ctx.build.id(),
107119
containers: exists,
@@ -131,7 +143,9 @@ describe('POST /instances/:id/actions/copy', { timeout: 500 }, function () {
131143
shortHash: exists,
132144
name: exists,
133145
public: exists,
134-
createdBy: { github: ctx.user.json().accounts.github.id },
146+
createdBy: { github: ctx.user.json().accounts.github.id,
147+
username: ctx.user.json().accounts.github.username,
148+
gravatar: ctx.user.json().accounts.github.avatar_url },
135149
'owner.github': ctx.orgId,
136150
parent: ctx.instance.id(),
137151
'build._id': ctx.build.id(),
@@ -157,14 +171,17 @@ describe('POST /instances/:id/actions/copy', { timeout: 500 }, function () {
157171
require('../../fixtures/mocks/github/user-orgs')(ctx.orgId, 'Runnable');
158172
require('../../fixtures/mocks/github/user-orgs')(ctx.orgId, 'Runnable');
159173
require('../../fixtures/mocks/github/user-orgs')(ctx.orgId, 'Runnable');
174+
require('../../fixtures/mocks/github/user')(ctx.nonOwner);
160175
ctx.otherInstance = ctx.user.copyInstance(ctx.instance.id(), done);
161176
});
162177
it('should copy the instance when part of the same org as the owner', function (done) {
163178
var expected = {
164179
shortHash: exists,
165180
name: exists,
166181
public: exists,
167-
createdBy: { github: ctx.nonOwner.json().accounts.github.id },
182+
createdBy: { github: ctx.nonOwner.json().accounts.github.id,
183+
username: ctx.nonOwner.json().accounts.github.username,
184+
gravatar: ctx.nonOwner.json().accounts.github.avatar_url },
168185
'owner.github': ctx.orgId,
169186
parent: ctx.otherInstance.id(),
170187
'build._id': ctx.build.id(),
@@ -198,9 +215,12 @@ describe('POST /instances/:id/actions/copy', { timeout: 500 }, function () {
198215
shortHash: exists,
199216
name: exists,
200217
public: exists,
201-
createdBy: { github: ctx.user.json().accounts.github.id },
218+
createdBy: { github: ctx.user.json().accounts.github.id,
219+
username: ctx.user.json().accounts.github.username,
220+
gravatar: ctx.user.json().accounts.github.avatar_url },
202221
owner: { github: ctx.user.json().accounts.github.id,
203-
username: ctx.user.json().accounts.github.username },
222+
username: ctx.user.json().accounts.github.username,
223+
gravatar: ctx.user.json().accounts.github.avatar_url },
204224
parent: ctx.instance.id(),
205225
'build._id': ctx.build.id(),
206226
containers: exists

‎test/instances-id/patch/200.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,14 @@ describe('200 PATCH /instances/:id', {timeout:1000}, function () {
9292
_id: exists,
9393
shortHash: exists,
9494
'createdBy.github': ctx.user.attrs.accounts.github.id,
95+
'createdBy.username': ctx.user.attrs.accounts.github.username,
96+
'createdBy.gravatar': ctx.user.attrs.accounts.github.avatar_url,
9597
name: exists,
9698
env: [],
9799
owner: {
98100
username: ctx.user.json().accounts.github.login,
99-
github: ctx.user.json().accounts.github.id
101+
github: ctx.user.json().accounts.github.id,
102+
gravatar: ctx.user.json().accounts.github.avatar_url
100103
},
101104
contextVersions: exists,
102105
'network.networkIp': exists,

‎test/instances/get/index.js‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,11 @@ describe('GET /instances', function () {
5151
var expected = [{
5252
_id: ctx.instance.json()._id,
5353
shortHash: ctx.instance.json().shortHash,
54-
'containers[0].inspect.State.Running': true
54+
'containers[0].inspect.State.Running': true,
55+
'owner.github': ctx.user.json().accounts.github.id,
56+
'owner.username': ctx.user.json().accounts.github.login,
57+
'createdBy.username': ctx.user.json().accounts.github.login,
58+
'createdBy.gravatar': ctx.user.json().accounts.github.avatar_url
5559
}];
5660
ctx.user.fetchInstances(query, expects.success(200, expected, count.next));
5761
var query2 = {
@@ -63,7 +67,11 @@ describe('GET /instances', function () {
6367
var expected2 = [{
6468
_id: ctx.instance2.json()._id,
6569
shortHash: ctx.instance2.json().shortHash,
66-
'containers[0].inspect.State.Running': true
70+
'containers[0].inspect.State.Running': true,
71+
'owner.github': ctx.user2.json().accounts.github.id,
72+
'owner.username': ctx.user2.json().accounts.github.login,
73+
'createdBy.username': ctx.user2.json().accounts.github.login,
74+
'createdBy.gravatar': ctx.user2.json().accounts.github.avatar_url
6775
}];
6876
ctx.user2.fetchInstances(query2, expects.success(200, expected2, count.next));
6977
});
@@ -81,7 +89,8 @@ describe('GET /instances', function () {
8189
'containers[0].inspect.State.Running': true
8290
}
8391
];
84-
require('../../fixtures/mocks/github/users-username')(ctx.user.json().accounts.github.id, ctx.user.username);
92+
require('../../fixtures/mocks/github/users-username')
93+
(ctx.user.json().accounts.github.id, ctx.user.json().accounts.github.username);
8594
ctx.user.fetchInstances(query, expects.success(200, expected, count.next));
8695
var query2 = {
8796
githubUsername: ctx.user2.json().accounts.github.username
@@ -93,7 +102,8 @@ describe('GET /instances', function () {
93102
'containers[0].inspect.State.Running': true
94103
}
95104
];
96-
require('../../fixtures/mocks/github/users-username')(ctx.user2.json().accounts.github.id, ctx.user2.username);
105+
require('../../fixtures/mocks/github/users-username')
106+
(ctx.user2.json().accounts.github.id, ctx.user2.json().accounts.github.username);
97107
ctx.user2.fetchInstances(query2, expects.success(200, expected2, count.next));
98108
});
99109
it('should get instances by ["contextVersion.appCodeVersions.repo"]', {timeout:500}, function (done) {
@@ -114,7 +124,8 @@ describe('GET /instances', function () {
114124
'containers[0].inspect.State.Running': true
115125
}
116126
];
117-
require('../../fixtures/mocks/github/users-username')(ctx.user.json().accounts.github.id, ctx.user.username);
127+
require('../../fixtures/mocks/github/users-username')
128+
(ctx.user.json().accounts.github.id, ctx.user.attrs.accounts.github.username);
118129
ctx.user.fetchInstances(query, expects.success(200, expected, count.next));
119130
var query2 = {
120131
'contextVersion.appCodeVersions.repo': ctx.instance2.attrs.contextVersion.appCodeVersions[0].repo,
@@ -129,7 +140,8 @@ describe('GET /instances', function () {
129140
'containers[0].inspect.State.Running': true
130141
}
131142
];
132-
require('../../fixtures/mocks/github/users-username')(ctx.user2.json().accounts.github.id, ctx.user2.username);
143+
require('../../fixtures/mocks/github/users-username')
144+
(ctx.user2.json().accounts.github.id, ctx.user2.attrs.accounts.github.username);
133145
ctx.user2.fetchInstances(query2, expects.success(200, expected2, count.next));
134146
});
135147
it('should list instances by owner.github', function (done) {

0 commit comments

Comments
 (0)