Skip to content

Commit

Permalink
Refactor the homepage view. Move search/sort/pagination to querystring
Browse files Browse the repository at this point in the history
This and the following commits should be PR ready, and should not
break existing routes. This means refactored code that affect more
than one route will be duplicated and renamed. We can easily cleanup
extra code after implementing the entire refactor.
  • Loading branch information
Zren committed May 24, 2014
1 parent 2251943 commit 1d36664
Show file tree
Hide file tree
Showing 16 changed files with 638 additions and 11 deletions.
99 changes: 89 additions & 10 deletions controllers/index.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,113 @@
var async = require('async');
var _ = require('underscore');
var url = require("url");

var Group = require('../models/group').Group;
var Script = require('../models/script').Script;
var Strategy = require('../models/strategy.js').Strategy;
var User = require('../models/user').User;
var Script = require('../models/script').Script;

var strategies = require('./strategies.json');
var modelsList = require('../libs/modelsList');
var userRoles = require('../models/userRoles.json');
var modelParser = require('../libs/modelParser');
var modelQuery = require('../libs/modelQuery');
var helpers = require('../libs/helpers');
var paginateTemplate = require('../libs/templateHelpers').paginateTemplate;

// The home page has scripts and groups in a sidebar
exports.home = function (req, res) {
var options = {};
options.title = 'OpenUserJS.org';

// Session
var user = req.session.user;
var options = { title: 'Home Page', username: user ? user.name : null };
options.user = user ? modelParser.parseUser(user) : null;

// Scripts: Query
var scriptsQuery = Script.find();

// Scripts: Query: Search
if (req.query.q)
modelQuery.parseScriptSearchQuery(scriptsQuery, req.query.q);

// Scripts: Query: Sort
if (req.query.orderBy) {
// parseScriptsSortQuery(scriptsQuery, req.query, function(scriptsQuery) {
// scriptsQuery.sort('-updated');
// });
} else {
scriptsQuery.sort('-rating -installs -updated');
}


// Scripts: Pagination
options.scriptsPage = req.query.p ? helpers.limitMin(1, req.query.p) : 1;
options.scriptsLimit = req.query.limit ? helpers.limitRange(0, req.query.limit, 100) : 10;
var scriptsSkipFrom = (options.scriptsPage * options.scriptsLimit) - options.scriptsLimit;
scriptsQuery
.skip(scriptsSkipFrom)
.limit(options.scriptsLimit);

// Groups: Query
var groupsQuery = Group.find();
groupsQuery
.limit(25);

async.parallel([
function (callback) {
modelsList.listScripts({}, req.route.params, '',
function (scriptsList) {
options.scriptsList = scriptsList;
scriptsQuery.exec(function(err, scriptDataList){
if (err) {
callback();
} else {
options.scriptList = _.map(scriptDataList, modelParser.parseScript);
callback();
}
});
},
function (callback) {
var params = [null, null, null, null, null];
modelsList.listGroups({}, params, '',
function (groupsList) {
options.groupsList = groupsList;
Script.count(scriptsQuery._conditions, function(err, totalScripts){
if (err) {
callback();
} else {
options.totalScripts = totalScripts;
options.numScriptPages = Math.ceil(options.totalScripts / options.scriptsLimit) || 1;
callback();
}
});
}, function (callback) {
groupsQuery.exec(function(err, groupDataList){
if (err) {
callback();
} else {
options.groupList = _.map(groupDataList, modelParser.parseGroup);
callback();
}
});
}, function (callback) {
Group.count(groupsQuery._conditions, function(err, totalGroups){
if (err) {
callback();
} else {
options.totalGroups = totalGroups;
callback();
}
});
}
], function () {
res.render('index', options);
options.pagination = paginateTemplate({
currentPage: options.scriptsPage,
lastPage: options.numScriptPages,
urlFn: function(p) {
var parseQueryString = true;
var u = url.parse(req.url, parseQueryString);
u.query.p = p;
delete u.search; // http://stackoverflow.com/a/7517673/947742
return url.format(u);
}
});

res.render('pages/scriptListPage', options);
});
};

Expand Down
8 changes: 8 additions & 0 deletions libs/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,11 @@ exports.cleanFilename = function (filename, defaultName) {

return cleanName || defaultName;
};

exports.limitRange = function(min, x, max) {
return Math.max(Math.min(x, max), min);
};

exports.limitMin = function(min, x) {
return Math.max(x, min);
};
105 changes: 105 additions & 0 deletions libs/modelParser.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
var moment = require('moment');

/**
* Parse persisted model data and return a new object with additional generated fields used in view templates.
*/

var Script = require('../models/script').Script;

/**
* Script
*/

// Urls
var getScriptPageUrl = function(script) {
var isLib = script.isLib || false;
var scriptPath = script.installName
.replace(isLib ? /\.js$/ : /\.user\.js$/, '');
return (isLib ? '/libs/' : '/scripts/') + scriptPath;

This comment has been minimized.

Copy link
@sizzlemctwizzle

sizzlemctwizzle Jun 24, 2014

Member

Should be: return (isLib ? '/libs/' : '/scripts/') + encodeURI(scriptPath); #200

};

var getScriptViewSourcePageUrl = function(script) {
return getScriptPageUrl(script) + '/source';
};

var getScriptEditAboutPageUrl = function(script) {
var isLib = script.isLib || false;
var scriptPath = script.installName
.replace(isLib ? /\.js$/ : /\.user\.js$/, '');
var editUrl = scriptPath.split('/');
editUrl.shift();
return (isLib ? '/lib/' : '/script/') + editUrl.join('/') + '/edit';
};

var getScriptEditSourcePageUrl = function(script) {
return getScriptViewSourcePageUrl(script);
};

var getScriptInstallPageUrl = function(script) {
var isLib = script.isLib || false;
return (isLib ? '/libs/src/' : '/install/') + script.installName;
};

//
exports.parseScript = function(scriptData) {
var script = scriptData.toObject ? scriptData.toObject() : scriptData;

// Script Good/Bad bar.
// script.votes = upvotes
// script.flags = downvotes + flags?
var sumVotesAndFlags = script.votes + script.flags;
var votesRatio = sumVotesAndFlags > 0 ? script.votes / sumVotesAndFlags : 0;
var flagsRatio = sumVotesAndFlags > 0 ? script.flags / sumVotesAndFlags : 0;
script.votesPercent = votesRatio * 100;
script.flagsPercent = flagsRatio * 100;

// Urls: Public
script.scriptPageUrl = getScriptPageUrl(script);
script.scriptInstallPageUrl = getScriptInstallPageUrl(script);
script.scriptViewSourcePageUrl = getScriptViewSourcePageUrl(script);

// Urls: Author
script.scriptEditMetadataPageUrl = getScriptEditAboutPageUrl(script);
script.scriptEditSourcePageUrl = getScriptEditSourcePageUrl(script);

// Dates
script.updatedISOFormat = script.updated.toISOString();
script.updatedHumanized = moment(script.updated).fromNow();

return script;
};

/**
* User
*/

var getUserPageUrl = function(script) {
var isLib = script.isLib || false;
return (isLib ? '/libs/src/' : '/install/') + script.installName;
};

//
exports.parseUser = function(userData) {
var user = userData.toObject ? userData.toObject() : userData;

// Urls: Public
user.userPageUrl = '/users/' + user.name;

return user;
};

/**
* Group
*/

//
exports.parseGroup = function(groupData) {
var group = groupData.toObject ? groupData.toObject() : groupData;

group.size = group._scriptIds.length;
group.multiple = group._scriptIds.length > 1;

group.groupPageUrl = '/group/' + group.name.replace(/\s+/g, '_');

return group;
};
59 changes: 59 additions & 0 deletions libs/modelQuery.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
var orderDirs = ['asc', 'desc'];
function parseScriptsSortQuery(scriptsQuery, query, defaultFn) {
if (query.orderBy) {
var orderBy = query.orderBy;
var orderDir = query.orderDir;
if (_.isUndefined(query.orderDir) || !_.contains(orderDirs, orderDir))
orderDir = 'asc';

if (_.has(Script.schema.paths, query.orderBy)) {
var sortBy = {};
sortBy[orderBy] = orderDir;
scriptsQuery.sort(sortBy);
return;
}
}
defaultFn(scriptsQuery);
}

var parseSearchConditions = function(q, prefixSearchFields, fullSearchFields) {
var conditions = [];
var query = null;
var prefixStr = '';
var fullStr = '';
var prefixRegex = null;
var fullRegex = null;
var terms = q.replace(/([.*+?^=!:${}()|\[\]\/\\])/g, '\\$1').split(/\s+/);

// Match all the terms but in any order
terms.forEach(function (term) {
prefixStr += '(?=.*?\\b' + term + ')';
fullStr += '(?=.*?' + term + ')';
});
prefixRegex = new RegExp(prefixStr, 'i');
fullRegex = new RegExp(fullStr, 'i');

// One of the searchable fields must match the conditions
prefixSearchFields.forEach(function (prop) {
var condition = {};
condition[prop] = prefixRegex;
conditions.push(condition);
});

fullSearchFields.forEach(function (prop) {
var condition = {};
condition[prop] = fullRegex;
conditions.push(condition);
});
return conditions;
};
exports.parseSearchConditions = parseSearchConditions;

exports.parseScriptSearchQuery = function(scriptsQuery, query) {
var q = unescape(query);

This comment has been minimized.

Copy link
@Martii

Martii Jun 24, 2014

Member

Finally found the first commit... at least at this path anyways. :)


escape and unescape is deprecated... See here

See also:

  • May apply to #133
  • escape and unescape is known to use ISO 8859-1 (ASCII related) and not UTF-8

So a question is ... "Why are we unescapeing here?"

This comment has been minimized.

Copy link
@sizzlemctwizzle

sizzlemctwizzle Jun 24, 2014

Member

@Martii

This should be changed to decodeURI, right?

This comment has been minimized.

Copy link
@sizzlemctwizzle

sizzlemctwizzle Jun 24, 2014

Member

This comment has been minimized.

Copy link
@Martii

Martii Jun 24, 2014

Member

This should be changed to decodeURI, right?

I'm not entirely sure. V8 is server side and may not be deprecating it especially since it's ES5 but it is not consistent with pushing everything to UTF-8. EDIT: We might not need it at all here when #133 gets a fix. IDKY. Since I'm not as search knowledgeable in the context of OUJS as I would like to be just yet I'll just present what I do know. In #133 I didn't see this section of code which could possibly have changed how I searched for things... e.g. convert the target data to ISO-8859-1 first (memory hog probably)... but there are several inconsistencies like this throughout the code like I alluded to in #198 and before digging around in #133 again.

var partialWordMatchFields = ['name', 'author', 'about', 'meta.description'];
var fullWordMatchFields = ['meta.include', 'meta.match'];
scriptsQuery.find({
'$or': parseSearchConditions(q, partialWordMatchFields, fullWordMatchFields)
});
}
42 changes: 42 additions & 0 deletions libs/templateHelpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@

exports.paginateTemplate = function(opts) {
// Required
var currentPage = opts.currentPage;
var lastPage = opts.lastPage;
var urlFn = opts.urlFn;

// Optional
var distVisible = opts.distVisible || 4;
var firstVisible = opts.firstVisible || true;
var lastVisible = opts.firstVisible || true;

var linkedPages = [];

for (var i = Math.max(1, currentPage - distVisible); i <= Math.min(currentPage + distVisible, lastPage); i++)
linkedPages.push(i);

if (firstVisible && linkedPages.length > 0 && linkedPages[0] != 1)
linkedPages.splice(0, 0, 1); // insert the value 1 at index 0

if (lastVisible && linkedPages.length > 0 && linkedPages[linkedPages.length-1] != lastPage)
linkedPages.push(lastPage);


var html = '';
html += '<ul class="pagination">';
for (var i = 0; i < linkedPages.length; i++) {
var linkedPage = linkedPages[i];
html += '<li';
if (linkedPage == currentPage)
html += ' class="active"';
html += '>';
html += '<a href="';
html += urlFn(linkedPage);
html += '">';
html += linkedPage;
html += '</a>';
html += '</li>';
}
html += '</ul>';
return html;
}
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@
"passport-foursquare": "*",
"passport-instagram": "*",
"passport-steam": "*",
"passport-tumblr": "*"
"passport-tumblr": "*",
"underscore": "*",
"moment": "*",
"url": "*"
},
"repository": {
"type": "git",
Expand Down
Loading

0 comments on commit 1d36664

Please sign in to comment.