From ad633aad4583546bad9302ca1cbef9ad052c131e Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Mon, 19 Dec 2016 11:16:03 -0500 Subject: [PATCH] additional tests and proper handling for purged flag targets, #5232 --- public/language/en-GB/flags.json | 1 + src/controllers/mods.js | 9 +++-- src/flags.js | 60 +++++++++++++++++++------------- src/posts/delete.js | 3 -- src/upgrade.js | 47 ------------------------- src/user/admin.js | 10 ------ test/flags.js | 44 +++++++++++++++++++++++ 7 files changed, 87 insertions(+), 87 deletions(-) diff --git a/public/language/en-GB/flags.json b/public/language/en-GB/flags.json index 2a1bf919f8..66b9acc92a 100644 --- a/public/language/en-GB/flags.json +++ b/public/language/en-GB/flags.json @@ -7,6 +7,7 @@ "assignee": "Assignee", "update": "Update", "updated": "Updated", + "target-purged": "The content this flag referred to has been purged and is no longer available.", "quick-filters": "Quick Filters", "filter-active": "There are one or more filters active in this list of flags", diff --git a/src/controllers/mods.js b/src/controllers/mods.js index cae9ade1ed..242d68d708 100644 --- a/src/controllers/mods.js +++ b/src/controllers/mods.js @@ -78,8 +78,13 @@ modsController.flags.detail = function (req, res, next) { res.render('flags/detail', Object.assign(results.flagData, { assignees: results.assignees, - type_bool: ['post', 'user'].reduce(function (memo, cur) { - memo[cur] = results.flagData.type === cur; + type_bool: ['post', 'user', 'empty'].reduce(function (memo, cur) { + if (cur !== 'empty') { + memo[cur] = results.flagData.type === cur && !!Object.keys(results.flagData.target).length; + } else { + memo[cur] = !Object.keys(results.flagData.target).length; + } + return memo; }, {}), title: '[[pages:flag-details, ' + req.params.flagId + ']]' diff --git a/src/flags.js b/src/flags.js index 81b97ee408..61b7a1746d 100644 --- a/src/flags.js +++ b/src/flags.js @@ -202,31 +202,6 @@ Flags.validate = function (payload, callback) { }); }; -Flags.getTarget = function (type, id, uid, callback) { - switch (type) { - case 'post': - async.waterfall([ - async.apply(posts.getPostsByPids, [id], uid), - function (posts, next) { - topics.addPostData(posts, uid, next); - } - ], function (err, posts) { - callback(err, posts[0]); - }); - break; - - case 'user': - user.getUsersData([id], function (err, users) { - callback(err, users ? users[0] : undefined); - }); - break; - - default: - callback(new Error('[[error:invalid-data]]')); - break; - } -}; - Flags.getNotes = function (flagId, callback) { async.waterfall([ async.apply(db.getSortedSetRevRangeWithScores.bind(db), 'flag:' + flagId + ':notes', 0, -1), @@ -348,6 +323,41 @@ Flags.exists = function (type, id, uid, callback) { db.isSortedSetMember('flags:hash', [type, id, uid].join(':'), callback); }; +Flags.getTarget = function (type, id, uid, callback) { + async.waterfall([ + async.apply(Flags.targetExists, type, id), + function (exists, next) { + if (exists) { + switch (type) { + case 'post': + async.waterfall([ + async.apply(posts.getPostsByPids, [id], uid), + function (posts, next) { + topics.addPostData(posts, uid, next); + } + ], function (err, posts) { + next(err, posts[0]); + }); + break; + + case 'user': + user.getUsersData([id], function (err, users) { + next(err, users ? users[0] : undefined); + }); + break; + + default: + next(new Error('[[error:invalid-data]]')); + break; + } + } else { + // Target used to exist (otherwise flag creation'd fail), but no longer + next(null, {}); + } + } + ], callback); +}; + Flags.targetExists = function (type, id, callback) { switch (type) { case 'post': diff --git a/src/posts/delete.js b/src/posts/delete.js index ebf902aef2..32ee6b6f41 100644 --- a/src/posts/delete.js +++ b/src/posts/delete.js @@ -144,9 +144,6 @@ module.exports = function (Posts) { }, function (next) { db.sortedSetsRemove(['posts:pid', 'posts:flagged'], pid, next); - }, - function (next) { - flags.dismiss(pid, next); } ], function (err) { if (err) { diff --git a/src/upgrade.js b/src/upgrade.js index 29b806d4a7..1815d1e560 100644 --- a/src/upgrade.js +++ b/src/upgrade.js @@ -455,53 +455,6 @@ Upgrade.upgrade = function (callback) { next(); } }, - function (next) { - thisSchemaDate = Date.UTC(2016, 3, 29); - - if (schemaDate < thisSchemaDate) { - updatesMade = true; - winston.info('[2016/04/29] Dismiss flags from deleted topics'); - - var posts = require('./posts'); - var topics = require('./topics'); - var flags = require('./flags'); - - var pids, tids; - - async.waterfall([ - async.apply(db.getSortedSetRange, 'posts:flagged', 0, -1), - function (_pids, next) { - pids = _pids; - posts.getPostsFields(pids, ['tid'], next); - }, - function (_tids, next) { - tids = _tids.map(function (a) { - return a.tid; - }); - - topics.getTopicsFields(tids, ['deleted'], next); - }, - function (state, next) { - var toDismiss = state.map(function (a, idx) { - return parseInt(a.deleted, 10) === 1 ? pids[idx] : null; - }).filter(Boolean); - - winston.info('[2016/04/29] ' + toDismiss.length + ' dismissable flags found'); - async.each(toDismiss, flags.dismiss, next); - } - ], function (err) { - if (err) { - return next(err); - } - - winston.info('[2016/04/29] Dismiss flags from deleted topics done'); - Upgrade.update(thisSchemaDate, next); - }); - } else { - winston.info('[2016/04/29] Dismiss flags from deleted topics skipped!'); - next(); - } - }, function (next) { thisSchemaDate = Date.UTC(2016, 4, 28); diff --git a/src/user/admin.js b/src/user/admin.js index 5d2215980c..4f7ecf66fb 100644 --- a/src/user/admin.js +++ b/src/user/admin.js @@ -56,14 +56,4 @@ module.exports = function (User) { } ], callback); }; - - User.resetFlags = function (uids, callback) { - if (!Array.isArray(uids) || !uids.length) { - return callback(); - } - - async.eachSeries(uids, function (uid, next) { - flags.dismissByUid(uid, next); - }, callback); - }; }; diff --git a/test/flags.js b/test/flags.js index cc8a0425e3..58250fb27f 100644 --- a/test/flags.js +++ b/test/flags.js @@ -82,6 +82,42 @@ describe('Flags', function () { }); }); + describe('.exists()', function () { + it('should return Boolean True if a flag matching the flag hash already exists', function (done) { + Flags.exists('post', 1, 1, function (err, exists) { + assert.ifError(err); + assert.strictEqual(true, exists); + done(); + }); + }); + + it('should return Boolean False if a flag matching the flag hash does not already exists', function (done) { + Flags.exists('post', 1, 2, function (err, exists) { + assert.ifError(err); + assert.strictEqual(false, exists); + done(); + }); + }); + }); + + describe('.targetExists()', function () { + it('should return Boolean True if the targeted element exists', function (done) { + Flags.targetExists('post', 1, function (err, exists) { + assert.ifError(err); + assert.strictEqual(true, exists); + done(); + }); + }); + + it('should return Boolean False if the targeted element does not exist', function (done) { + Flags.targetExists('post', 15, function (err, exists) { + assert.ifError(err); + assert.strictEqual(false, exists); + done(); + }); + }); + }); + describe('.get()', function () { it('should retrieve and display a flag\'s data', function (done) { Flags.get(1, function (err, flagData) { @@ -252,6 +288,14 @@ describe('Flags', function () { done(); }); }); + + it('should return a plain object with no properties if the target no longer exists', function (done) { + Flags.getTarget('user', 15, 1, function (err, data) { + assert.ifError(err); + assert.strictEqual(0, Object.keys(data).length); + done(); + }); + }); }); describe('.validate()', function () {