Skip to content
This repository has been archived by the owner on Apr 30, 2020. It is now read-only.

Add backend infrastructure for Extras #14

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.DS_Store
node_modules/
npm-debug.log
config/github.json
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a note in the README on how to setup this config file? I think it's fine for everyone to create their own credentials.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea.

4 changes: 3 additions & 1 deletion app.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ app.locals({
{id: 'grids', url: '/grids/', label: 'Grids'},
{id: 'forms', url: '/forms/', label: 'Forms'},
{id: 'tables', url: '/tables/', label: 'Tables'},
{id: 'lists', url: '/lists/', label: 'Navigation'}
{id: 'lists', url: '/lists/', label: 'Navigation'},
{id: 'extras', url: '/extras/', label: 'Extras'}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alignment nit-pick :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hehe, good catch!

],

yui : config.yui,
Expand Down Expand Up @@ -68,6 +69,7 @@ app.get('/grids/', routes.render('grids'));
app.get('/forms/', routes.render('forms'));
app.get('/tables/', routes.render('tables'));
app.get('/lists/', routes.render('lists'));
app.get('/extras/', routes.extras);

// -- Exports ------------------------------------------------------------------

Expand Down
8 changes: 8 additions & 0 deletions config/extras.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module.exports = Object.freeze({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this freeze here. I'm planning to use the deep-freeze npm package to recursively freeze the main config object before it is exported.

modules: [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can just export this array. If you want, we can make this a JSON file :-/ what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I didn't keep it as just an array is in case we needed to add properties later on that applied to all modules. Having said that, it makes sense to change it to a JSON file.

{
username: 'tilomitra',
repo : 'cssextras'
}
]
});
4 changes: 3 additions & 1 deletion config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,7 @@ module.exports = Object.freeze({
}),

typekit: 'ajf8ggy',
yui : require('./yui')
yui : require('./yui'),
extras : require('./extras'),
github : require('./github.json')
});
56 changes: 56 additions & 0 deletions lib/githubApi.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
var request = require('request'),
config = require('../config'),
async = require('async');

module.exports = {

data: {
lastFetched: undefined,
results: undefined
},

timeThreshold: 3600000, //60 minutes in milliseconds

//Get information for an individual Github repo and execute a callback
getRepo: function (username, repo, callback) {
request({
url: 'https://api.github.com/repos/' + username + '/' + repo,
qs: {
client_id: config.github.clientId,
client_secret: config.github.clientSecret
}
}, function (error, response, body) {
callback(error, response, JSON.parse(body));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would wrap the parse in a try/catch, but IIRC, request might have built in support for JSON requests.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add json: true to the request config and it should return response.body already parsed.

https://github.com/mikeal/request#requestoptions-callback

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davglass thx!

});
},

getRepos: function (userArray, callback) {
var parallelTasks = [],
self = this,
now = new Date();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use Date.now() in node.


//If results exist and those results are less than 60 minutes old (this.timeThreshold)
if (this.data.results && (now.getTime() - this.data.lastFetched) < this.timeThreshold) {
//return with those results
callback(null, this.data.results);
}

else {
userArray.forEach(function (elem, i, arr) {
parallelTasks.push(function (callback) {
self.getRepo(elem.username, elem.repo, function (error, response, body) {
callback(error, body);
});
});
});

async.parallel(parallelTasks, function (error, response) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use the async.forEach() method, that is if it runs in parallel. It might help simplify this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right - looks like I can use async.each() https://github.com/caolan/async#each

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah async is awesome!

if (!error) {
self.data.results = response;
self.data.lastFetched = new Date().getTime();
}
callback(error, self.data.results);
});
}
}
}
33 changes: 33 additions & 0 deletions lib/routes/extras.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
var config = require('../../config'),
github = require('../githubApi'),
inProgress = false;

module.exports = function (req, res, next) {

//If an API call is in progress, render the last collected data.
if (inProgress) {
res.render('extras', {
results: github.data.results
});
}

//If an API call isn't in progress, set the inProgress flag to true and make the call to getRepos()
else {
inProgress = true;
github.getRepos(config.extras.modules, function (error, results) {
inProgress = false;
if (!error) {
res.render('extras', {
results: results
});
}
else {
console.log(error);
//We can probably serve the older results if we get an error for the newer results.
res.render('extras', {
results: github.data.results
});
}
});
}
};
3 changes: 2 additions & 1 deletion lib/routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ function render(viewName) {
}

module.exports = {
render: render
render: render,
extras: require('./extras')
};
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
"dependencies": {
"express" : "3.x",
"express3-handlebars": "0.3.x",
"handlebars" : "1.x"
"handlebars" : "1.x",
"async" : "0.2.x",
"request" : "2.x"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need me to do a npm shrinkwrap?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah.

},

"main": "server",
Expand Down
20 changes: 20 additions & 0 deletions views/extras.handlebars
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{{setTitle "Extras"}}
{{setSubTitle "Add-ons built on YUI CSS"}}
{{setActiveNav "extras"}}

{{addCdnCSS "3.9.0pr3" "cssnormalize"}}
{{addGithubCSS "tilomitra" "csstables" "master/css/tables.css"}}

{{> header}}

<div class="content">

<div class="yui3-g-r">
{{#each results}}
<div class="extras-module yui3-u-1-3">
<img src="{{this.owner.avatar_url}}"/>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a placeholder for now to make sure stuff is getting outputted correctly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this here, it's assumed to be the context inside the block.

</div>
{{/each}}
</div>

</div>