From a2edb86dfbe35c6e620b6aeb85b469e795e6df5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Tue, 13 Oct 2020 20:37:38 -0400 Subject: [PATCH] feat: change user search to use filters array --- public/src/admin/manage/users.js | 26 ++++++------ public/src/client/topic/change-owner.js | 2 +- public/src/client/users.js | 13 +++--- src/controllers/admin/users.js | 6 ++- src/controllers/users.js | 20 +++++---- src/socket.io/user/search.js | 17 +++++--- src/user/search.js | 56 ++++++++++++++----------- test/user.js | 8 ++-- 8 files changed, 86 insertions(+), 62 deletions(-) diff --git a/public/src/admin/manage/users.js b/public/src/admin/manage/users.js index 9d0fa707c3..4317a40753 100644 --- a/public/src/admin/manage/users.js +++ b/public/src/admin/manage/users.js @@ -493,8 +493,8 @@ define('admin/manage/users', [ params.query = $('#user-search').val(); params.searchBy = $('#user-search-by').val(); } else { - params.query = undefined; - params.searchBy = undefined; + delete params.query; + delete params.searchBy; } return decodeURIComponent($.param(params)); @@ -520,17 +520,17 @@ define('admin/manage/users', [ }); } - function handleFilter() { - function getFilters() { - var filters = []; - $('#filter-by').find('[data-filter-by]').each(function () { - if ($(this).find('.fa-check').length) { - filters.push($(this).attr('data-filter-by')); - } - }); - return filters; - } + function getFilters() { + var filters = []; + $('#filter-by').find('[data-filter-by]').each(function () { + if ($(this).find('.fa-check').length) { + filters.push($(this).attr('data-filter-by')); + } + }); + return filters; + } + function handleFilter() { var currentFilters = getFilters(); $('#filter-by').on('click', 'li', function () { var $this = $(this); @@ -551,7 +551,7 @@ define('admin/manage/users', [ currentFilters = getFilters(); if (changed) { var params = utils.params(); - params.filter = filters; + params.filters = filters; var qs = buildSearchQuery(params); ajaxify.go('admin/manage/users?' + qs); } diff --git a/public/src/client/topic/change-owner.js b/public/src/client/topic/change-owner.js index 771b3b6345..04fda698f5 100644 --- a/public/src/client/topic/change-owner.js +++ b/public/src/client/topic/change-owner.js @@ -37,7 +37,7 @@ define('forum/topic/change-owner', [ changeOwner(); }); - autocomplete.user(modal.find('#username'), { notBanned: true }, function (ev, ui) { + autocomplete.user(modal.find('#username'), { filters: ['notbanned'] }, function (ev, ui) { toUid = ui.item.user.uid; checkButtonEnable(); }); diff --git a/public/src/client/users.js b/public/src/client/users.js index 732208cd60..3ff1a62c68 100644 --- a/public/src/client/users.js +++ b/public/src/client/users.js @@ -61,17 +61,20 @@ define('forum/users', ['translator', 'benchpress'], function (translator, Benchp return loadPage(query); } - query.term = username; + query.query = username; query.sortBy = getSortBy(); - + var filters = []; if ($('.search .online-only').is(':checked') || (activeSection === 'online')) { - query.onlineOnly = true; + filters.push('online'); } if (activeSection === 'banned') { - query.bannedOnly = true; + filters.push('banned'); } if (activeSection === 'flagged') { - query.flaggedOnly = true; + filters.push('flagged'); + } + if (filters.length) { + query.filters = filters; } loadPage(query); diff --git a/src/controllers/admin/users.js b/src/controllers/admin/users.js index a908c445fb..59cb1f20be 100644 --- a/src/controllers/admin/users.js +++ b/src/controllers/admin/users.js @@ -36,7 +36,7 @@ async function getUsers(req, res) { resultsPerPage = 50; } let sortBy = validator.escape(req.query.sortBy || ''); - const filterBy = Array.isArray(req.query.filter) ? req.query.filter : [req.query.filter]; + const filterBy = Array.isArray(req.query.filters) ? req.query.filters : [req.query.filters]; const start = Math.max(0, page - 1) * resultsPerPage; const stop = start + resultsPerPage - 1; @@ -129,12 +129,14 @@ usersController.search = async function (req, res) { if (![50, 100, 250, 500].includes(resultsPerPage)) { resultsPerPage = 50; } + const searchData = await user.search({ uid: req.uid, query: req.query.query, searchBy: req.query.searchBy, sortBy: req.query.sortBy, sortDirection: sortDirection, + filters: req.query.filters, page: page, resultsPerPage: resultsPerPage, findUids: async function (query, searchBy, hardCap) { @@ -229,7 +231,7 @@ function render(req, res, data) { data.adminInviteOnly = registrationType === 'admin-invite-only'; data['sort_' + data.sortBy] = true; data['searchBy_' + validator.escape(String(req.query.searchBy))] = true; - const filterBy = Array.isArray(req.query.filter) ? req.query.filter : [req.query.filter]; + const filterBy = Array.isArray(req.query.filters) ? req.query.filters : [req.query.filters]; filterBy.forEach(function (filter) { data['filterBy_' + validator.escape(String(filter))] = true; }); diff --git a/src/controllers/users.js b/src/controllers/users.js index c485e8ecff..3ead286208 100644 --- a/src/controllers/users.js +++ b/src/controllers/users.js @@ -21,7 +21,7 @@ usersController.index = async function (req, res, next) { flagged: usersController.getFlaggedUsers, }; - if (req.query.term) { + if (req.query.query) { await usersController.search(req, res, next); } else if (sectionToController[section]) { await sectionToController[section](req, res, next); @@ -35,19 +35,25 @@ usersController.search = async function (req, res) { privileges.global.can('search:users', req.uid), user.isPrivileged(req.uid), ]); - - if (!allowed || ((req.query.searchBy === 'ip' || req.query.searchBy === 'email' || req.query.bannedOnly === 'true' || req.query.flaggedOnly === 'true') && !isPrivileged)) { + let filters = req.query.filters || []; + filters = Array.isArray(filters) ? filters : [filters]; + if (!allowed || + (( + req.query.searchBy === 'ip' || + req.query.searchBy === 'email' || + filters.includes('banned') || + filters.includes('flagged') + ) && !isPrivileged) + ) { throw new Error('[[error:no-privileges]]'); } const [searchData, isAdminOrGlobalMod] = await Promise.all([ user.search({ - query: req.query.term, + query: req.query.query, searchBy: req.query.searchBy || 'username', page: req.query.page || 1, sortBy: req.query.sortBy || 'joindate', - onlineOnly: req.query.onlineOnly === 'true', - bannedOnly: req.query.bannedOnly === 'true', - flaggedOnly: req.query.flaggedOnly === 'true', + filters: filters, }), user.isAdminOrGlobalMod(req.uid), ]); diff --git a/src/socket.io/user/search.js b/src/socket.io/user/search.js index b1fc2e1e3e..c181fb599d 100644 --- a/src/socket.io/user/search.js +++ b/src/socket.io/user/search.js @@ -6,6 +6,7 @@ const privileges = require('../../privileges'); module.exports = function (SocketUser) { SocketUser.search = async function (socket, data) { + // TODO: depracate and use usersController.search if (!data) { throw new Error('[[error:invalid-data]]'); } @@ -14,7 +15,16 @@ module.exports = function (SocketUser) { user.isPrivileged(socket.uid), ]); - if (!allowed || ((data.searchBy === 'ip' || data.searchBy === 'email' || data.bannedOnly || data.flaggedOnly) && !isPrivileged)) { + let filters = data.filters || []; + filters = Array.isArray(filters) ? filters : [filters]; + if (!allowed || + (( + data.searchBy === 'ip' || + data.searchBy === 'email' || + filters.includes('banned') || + filters.includes('flagged') + ) && !isPrivileged) + ) { throw new Error('[[error:no-privileges]]'); } const result = await user.search({ @@ -22,10 +32,7 @@ module.exports = function (SocketUser) { page: data.page, searchBy: data.searchBy, sortBy: data.sortBy, - onlineOnly: data.onlineOnly, - bannedOnly: data.bannedOnly, - notBanned: data.notBanned, - flaggedOnly: data.flaggedOnly, + filters: data.filters, paginate: data.paginate, uid: socket.uid, }); diff --git a/src/user/search.js b/src/user/search.js index 0934371932..32c9d6bd2f 100644 --- a/src/user/search.js +++ b/src/user/search.js @@ -10,6 +10,25 @@ const groups = require('../groups'); const utils = require('../utils'); module.exports = function (User) { + const filterFnMap = { + online: user => user.status !== 'offline' && (Date.now() - user.lastonline < 300000), + banned: user => user.banned, + notbanned: user => !user.banned, + flagged: user => parseInt(user.flags, 10) > 0, + verified: user => !!user['email:confirmed'], + unverified: user => !user['email:confirmed'], + }; + + const filterFieldMap = { + online: ['status', 'lastonline'], + banned: ['banned'], + notbanned: ['banned'], + flagged: ['flags'], + verified: ['email:confirmed'], + unverified: ['email:confirmed'], + }; + + User.search = async function (data) { const query = data.query || ''; const searchBy = data.searchBy || 'username'; @@ -69,21 +88,19 @@ module.exports = function (User) { async function filterAndSortUids(uids, data) { uids = uids.filter(uid => parseInt(uid, 10)); - + let filters = data.filters || []; + filters = Array.isArray(filters) ? filters : [data.filters]; const fields = []; if (data.sortBy) { fields.push(data.sortBy); } - if (data.onlineOnly) { - fields.push('status', 'lastonline'); - } - if (data.bannedOnly || data.notBanned) { - fields.push('banned'); - } - if (data.flaggedOnly) { - fields.push('flags'); - } + + filters.forEach(function (filter) { + if (filterFieldMap[filter]) { + fields.push(...filterFieldMap[filter]); + } + }); if (data.groupName) { const isMembers = await groups.isMembers(uids, data.groupName); @@ -96,21 +113,12 @@ module.exports = function (User) { fields.push('uid'); let userData = await User.getUsersFields(uids, fields); - if (data.onlineOnly) { - userData = userData.filter(user => user.status !== 'offline' && (Date.now() - user.lastonline < 300000)); - } - if (data.bannedOnly) { - userData = userData.filter(user => user.banned); - } - - if (data.notBanned) { - userData = userData.filter(user => !user.banned); - } - - if (data.flaggedOnly) { - userData = userData.filter(user => parseInt(user.flags, 10) > 0); - } + filters.forEach(function (filter) { + if (filterFnMap[filter]) { + userData = userData.filter(filterFnMap[filter]); + } + }); if (data.sortBy) { sortUsers(userData, data.sortBy, data.sortDirection); diff --git a/test/user.js b/test/user.js index ee8295f11a..060137e3d4 100644 --- a/test/user.js +++ b/test/user.js @@ -369,14 +369,14 @@ describe('User', function () { }); it('should error for unprivileged user', function (done) { - socketUser.search({ uid: testUid }, { bannedOnly: true, query: '123' }, function (err) { + socketUser.search({ uid: testUid }, { filters: ['banned'], query: '123' }, function (err) { assert.equal(err.message, '[[error:no-privileges]]'); done(); }); }); it('should error for unprivileged user', function (done) { - socketUser.search({ uid: testUid }, { flaggedOnly: true, query: '123' }, function (err) { + socketUser.search({ uid: testUid }, { filters: ['flagged'], query: '123' }, function (err) { assert.equal(err.message, '[[error:no-privileges]]'); done(); }); @@ -430,9 +430,7 @@ describe('User', function () { assert.ifError(err); socketUser.search({ uid: adminUid }, { query: 'ipsearch', - onlineOnly: true, - bannedOnly: true, - flaggedOnly: true, + filters: ['online', 'banned', 'flagged'], }, function (err, data) { assert.ifError(err); assert.equal(data.users[0].username, 'ipsearch_filter');