Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 643 #770

Merged
merged 6 commits into from
Oct 18, 2015
Merged

Issue 643 #770

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7cf7602
Show who flagged what in the UI
Oct 15, 2015
96d9d99
Fixes and compaction/distribution for who flagged what in UI
Oct 17, 2015
58602e3
Distribution changes
Oct 18, 2015
01cb33e
Change the identifier for the function name
Oct 18, 2015
ad2edb1
Create/fill in the missing lib for vote action
Oct 18, 2015
15c7c54
Fill out to a basic template for the newest controllers
Oct 18, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions controllers/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ var execQueryTask = require('../libs/tasks').execQueryTask;
var removeSession = require('../libs/modifySessions').remove;
var pageMetadata = require('../libs/templateHelpers').pageMetadata;
var orderDir = require('../libs/templateHelpers').orderDir;
var flagLib = require('../libs/flag');

// The home page has scripts and groups in a sidebar
exports.home = function (aReq, aRes) {
Expand Down Expand Up @@ -147,8 +148,25 @@ exports.home = function (aReq, aRes) {
}
}
}
function render() { aRes.render('pages/scriptListPage', options); }
function asyncComplete() { preRender(); render(); }
function render() {
aRes.render('pages/scriptListPage', options);
}
function asyncComplete() {

async.parallel([
function (aCallback) {
if (!options.isFlagged || !options.isAdmin) { // NOTE: Watchpoint
aCallback();
return;
}
flagLib.setFlaggedListInModel('Script', options, aCallback);
}
], function (aErr) {
preRender();
render();
});

}
async.parallel(tasks, asyncComplete);
};

Expand Down
21 changes: 19 additions & 2 deletions controllers/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,25 @@ exports.view = function (aReq, aRes, aNext) {
script.description, _.pluck(script.groups, 'name'));
}
}
function render() { aRes.render('pages/scriptPage', options); }
function asyncComplete() { preRender(); render(); }
function render() {
aRes.render('pages/scriptPage', options);
}
function asyncComplete() {

async.parallel([
function (aCallback) {
if (!options.isAdmin) { // NOTE: Watchpoint
aCallback();
return;
}
flagLib.setFlaggedListInModel('Script', options, aCallback);
}
], function (aErr) {
preRender();
render();
});

}

//---
if (aErr || !aScriptData) {
Expand Down
75 changes: 66 additions & 9 deletions controllers/user.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,29 @@ exports.userListPage = function (aReq, aRes, aNext) {
pageMetadata(options, ['Flagged Users', 'Moderation']);
}
}
function render() { aRes.render('pages/userListPage', options); }
function asyncComplete(err) { if (err) { return aNext(); } else { preRender(); render(); } }
function render() {
aRes.render('pages/userListPage', options);
}
function asyncComplete(aErr) {
if (aErr) {
aNext();
return;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

MITIGATION: Test why this error fall through is the only one (for express)... I am guessing if we (ever) have an error in the changed views ("elsewhere") we're going to 500 and not fail gracefully.

Copy link
Member

Choose a reason for hiding this comment

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

asyncComplete should always test for an error and handle it (at the very least a 500 page). If we can catch an error earlier on then great, but this is really the last line of defense. It would be nice if we could find a way to abstract this function and put it in libs/helpers.js since I hate seeing the same logic repeated, but I realize the difficulty of doing this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Kind of figured that with the missing aNext() and it raised an eyebrow yesterday in my background thinking... I think I'll do that in a single PR with some cleanup just on ./controllers/user.js... I read elsewhere that ES6 is going to need undefined for certain functions instead of null (or worse some other huge object in a bug) so that is why I have been splitting out the aNext() and return statements... also it makes more sense not to return something that isn't there and could change down the line.

Just a misc note too node 4.x went LTS (Long Term Support) according to their site e.g. 5.x is around some corner.


async.parallel([
function (aCallback) {
if (!options.isFlagged || !options.isAdmin) { // NOTE: Watchpoint
aCallback();
return;
}
flagLib.setFlaggedListInModel('User', options, aCallback);
}
], function (aErr) {
preRender();
render();
});

}
async.parallel(tasks, asyncComplete);
};

Expand Down Expand Up @@ -292,9 +313,27 @@ exports.view = function (aReq, aRes, aNext) {
tasks = tasks.concat(stats.getSummaryTasks(options));

//---
function preRender() { }
function render() { aRes.render('pages/userPage', options); }
function asyncComplete() { preRender(); render(); }
function preRender() {
}
function render() {
aRes.render('pages/userPage', options);
}
function asyncComplete() {

Copy link
Member Author

Choose a reason for hiding this comment

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

"Elsewhere" after

async.parallel([
function (aCallback) {
if (!options.isAdmin) { // NOTE: Watchpoint
aCallback();
return;
}
flagLib.setFlaggedListInModel('User', options, aCallback);
}
], function (aErr) {
preRender();
render();
});

}
async.parallel(tasks, asyncComplete);
});
};
Expand Down Expand Up @@ -410,13 +449,14 @@ exports.userCommentListPage = function (aReq, aRes, aNext) {

exports.userScriptListPage = function (aReq, aRes, aNext) {
var authedUser = aReq.session.user;

var username = aReq.params.username;

User.findOne({
name: caseInsensitive(username)
}, function (aErr, aUserData) {
if (aErr || !aUserData) { return aNext(); }
if (aErr || !aUserData) {
return aNext();
}

//
var options = {};
Expand Down Expand Up @@ -497,8 +537,25 @@ exports.userScriptListPage = function (aReq, aRes, aNext) {
options.scriptListIsEmptyMessage = 'This user hasn\'t added any scripts yet.';
}
}
function render() { aRes.render('pages/userScriptListPage', options); }
function asyncComplete() { preRender(); render(); }
function render() {
aRes.render('pages/userScriptListPage', options);
}
function asyncComplete() {

async.parallel([
function (aCallback) {
if (!options.isFlagged || !options.isAdmin) { // NOTE: Watchpoint
aCallback();
return;
}
flagLib.setFlaggedListInModel('Script', options, aCallback);
}
], function (aErr) {
preRender();
render();
});

}
async.parallel(tasks, asyncComplete);
});
};
Expand Down
50 changes: 50 additions & 0 deletions libs/flag.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ var isDev = require('../libs/debug').isDev;
var isDbg = require('../libs/debug').isDbg;

//
var async = require('async');

var Flag = require('../models/flag').Flag;
var User = require('../models/user').User;
var getKarma = require('./collectiveRating').getKarma;
Expand Down Expand Up @@ -148,3 +150,51 @@ exports.unflag = function (aModel, aContent, aUser, aCallback) {
}
});
};

exports.setFlaggedListInModel = function (aModelName, aOptions, aCallback) {
Copy link
Member

Choose a reason for hiding this comment

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

This is definitely the wrong module for this function. This module is supposed to be completely devoid of any front-end view logic (notice how "aOptions" or "options" are mentioned nowhere else in this file). This module defines the logic for how flagging is handled.

Copy link
Member Author

Choose a reason for hiding this comment

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

So where should it go?... I don't have a clue since this function is a "one of a kind new" thing AFAIK

Copy link
Member

Choose a reason for hiding this comment

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

Answered in my comment below.


var content = aModelName.toLowerCase();
var contentList = aOptions[content + 'List'] || [aOptions[content]];

async.forEachOf(contentList, function (aContent, aContentKey, aEachOuterCallback) {

// NOTE: Directly use indexed parent identifier allowing set of the dynamic, virtual, field
// So basically do not use `aContent` anywhere in this function

// Always ensure a snapshot copy!
if (contentList[aContentKey].toObject) {
contentList[aContentKey] = contentList[aContentKey].toObject({
virtuals: true
});
}

// Ensure reset
contentList[aContentKey].flaggedList = [];

// Find any flags
Flag.find({
model: aModelName,
_contentId: contentList[aContentKey]._id

}, function (aErr, aFlagList) {
if (aErr || !aFlagList || aFlagList.length === 0) {
aEachOuterCallback();
return;
}

aOptions.hasFlagged = true;

async.forEachOfSeries(aFlagList, function (aFlag, aFlagKey, aEachInnerCallback) {
User.findOne({ _id: aFlag._userId }, function (aErr, aUser) {
contentList[aContentKey].flaggedList.push({
name: aUser.name,
reason: aFlagList[aFlagKey].reason,
since: aFlagList[aFlagKey]._since
});
aEachInnerCallback();
});
}, aEachOuterCallback);
});

}, aCallback);
}
13 changes: 7 additions & 6 deletions libs/modelParser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,12 @@ moment.locale('en-tiny', {
});

var parseDateProperty = function (aObj, aKey) {
var date = aObj[aKey];
if (date) {
aObj[aKey + 'ISOFormat'] = date.toISOString();
aObj[aKey + 'Humanized'] = moment(date).locale('en-tiny').calendar();
if (aObj[aKey]) {
var date = new Date(aObj[aKey]);
if (date) {
aObj[aKey + 'ISOFormat'] = date.toISOString();
aObj[aKey + 'Humanized'] = moment(date).locale('en-tiny').calendar();
}
}
};

Expand Down Expand Up @@ -243,10 +245,9 @@ var parseUser = function (aUserData) {
if (!aUserData) {
return;
}
// var user = aUserData.toObject ? aUserData.toObject() : aUserData;

// Intermediates
var user = aUserData;
var user = aUserData.toObject ? aUserData.toObject({ virtuals: true }) : aUserData;

// Role
user.isMod = user.role < 4;
Expand Down
5 changes: 5 additions & 0 deletions models/flag.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,15 @@ var Schema = mongoose.Schema;

var flagSchema = new Schema({
model: String,
reason: String,
_contentId: Schema.Types.ObjectId,
_userId: Schema.Types.ObjectId
});

flagSchema.virtual('_since').get(function () {
return this._id.getTimestamp();
});

var Flag = mongoose.model('Flag', flagSchema);

exports.Flag = Flag;
5 changes: 5 additions & 0 deletions public/css/common.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,3 +295,8 @@ a.panel-heading {
.reason-automated {
opacity: 0.25;
}

ul.flaggedList {
list-style-type: none;
padding-left: 0;
}
17 changes: 17 additions & 0 deletions views/includes/scriptAdminToolsPanel.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,23 @@
</div>
</div>
<div class="panel-body">
{{#hasFlagged}}
<div class="form-horizontal">
<div class="form-group">
<label class="col-sm-4 control-label">Flagged</label>
<div class="col-sm-8">
<ul class="flaggedList">
{{#script.flaggedList}}
<li>
{{#name}}<span class="label label-info"><a href="/users/{{name}}">{{name}}</a></span>{{/name}}
<span title="{{since}}">{{#reason}}{{reason}}{{/reason}}{{^reason}}&hellip;{{/reason}}</span>
</li>
{{/script.flaggedList}}
</ul>
</div>
</div>
</div>
{{/hasFlagged}}
<a rel="nofollow" href="/admin/json?model=Script&id={{{script._id}}}" class="btn btn-link col-xs-12"><i class="fa fa-database"></i> Raw JSON Data</a>
</div>
</div>
Expand Down
15 changes: 15 additions & 0 deletions views/includes/scriptList.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
{{^librariesOnly}}<th class="text-center td-fit"><a href="?orderBy=installs&orderDir={{orderDir.install}}{{#searchBarValue}}&q={{searchBarValue}}{{/searchBarValue}}{{#isFlagged}}&flagged=true{{/isFlagged}}">Installs</a></th>{{/librariesOnly}}
<th class="text-center td-fit"><a href="?orderBy=rating&orderDir={{orderDir.rating}}{{#searchBarValue}}&q={{searchBarValue}}{{/searchBarValue}}{{#librariesOnly}}&library=true{{/librariesOnly}}{{#isFlagged}}&flagged=true{{/isFlagged}}">Rating</a></th>
<th class="text-center td-fit"><a href="?orderBy=updated&orderDir={{orderDir.updated}}{{#searchBarValue}}&q={{searchBarValue}}{{/searchBarValue}}{{#librariesOnly}}&library=true{{/librariesOnly}}{{#isFlagged}}&flagged=true{{/isFlagged}}">Last Updated</a></th>
{{#hasFlagged}}
<th>Flagged</th>
{{/hasFlagged}}
</tr>
</thead>
<tbody>
Expand Down Expand Up @@ -36,6 +39,18 @@
<td class="text-center td-fit">
<time datetime="{{updatedISOFormat}}" title="{{updated}}">{{updatedHumanized}}</time>
</td>
{{#hasFlagged}}
<td>
<ul class="flaggedList">
{{#flaggedList}}
<li>
{{#name}}<span class="label label-info"><a href="/users/{{name}}">{{name}}</a></span>{{/name}}
<span title="{{since}}">{{#reason}}{{reason}}{{/reason}}{{^reason}}&hellip;{{/reason}}</span>
</li>
{{/flaggedList}}
</ul>
</td>
{{/hasFlagged}}
</tr>
{{/scriptList}}
{{^scriptList}}
Expand Down
17 changes: 17 additions & 0 deletions views/includes/userAdminToolsPanel.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,23 @@
</div>
</div>
<div class="panel-body">
{{#hasFlagged}}
<div class="form-horizontal">
<div class="form-group">
<label class="col-sm-4 control-label">Flagged</label>
<div class="col-sm-8">
<ul class="flaggedList">
{{#user.flaggedList}}
<li>
{{#name}}<span class="label label-info"><a href="/users/{{name}}">{{name}}</a></span>{{/name}}
<span title="{{since}}">{{#reason}}{{reason}}{{/reason}}{{^reason}}&hellip;{{/reason}}</span>
</li>
{{/user.flaggedList}}
</ul>
</div>
</div>
</div>
{{/hasFlagged}}
<form class="form-horizontal" role="form" method="post" action="{{{user.userUpdatePageUrl}}}">
<div class="form-group container-fluid">
<label class="col-sm-2 control-label">Role</label>
Expand Down
Loading