From 8518404e226994994073feb5ad806bcb93b7ae1b Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Fri, 13 Nov 2020 14:15:37 -0500 Subject: [PATCH] feat: allow groups to specify which cids to show member posts from (#8875) * feat: allow groups to specify which cids to show member posts from * docs: fix tests for openapi * fix: test breakage caused by improper conditional * feat: server-side checking of memberPostCids for validity * feat: admin panel template update to select categories to include * refactor: privilege helpers.isUserAllowedTo ... to helpers.isAllowedTo, allowing group names to be passed in --- public/language/en-GB/groups.json | 2 + .../components/schemas/GroupObject.yaml | 38 +++++++++++- public/openapi/read/admin/manage/groups.yaml | 5 ++ public/openapi/read/groups.yaml | 5 ++ public/openapi/write/groups.yaml | 5 ++ public/src/admin/manage/group.js | 1 + src/groups/data.js | 1 + src/groups/index.js | 8 ++- src/groups/posts.js | 5 ++ src/groups/update.js | 12 +++- src/plugins/hooks.js | 2 +- src/privileges/admin.js | 4 +- src/privileges/categories.js | 8 +-- src/privileges/global.js | 4 +- src/privileges/helpers.js | 62 +++++++++++++++---- src/privileges/posts.js | 10 +-- src/privileges/topics.js | 4 +- src/privileges/users.js | 2 +- src/views/admin/manage/group.tpl | 16 +++++ 19 files changed, 159 insertions(+), 35 deletions(-) diff --git a/public/language/en-GB/groups.json b/public/language/en-GB/groups.json index d165c4a71b..0256067933 100644 --- a/public/language/en-GB/groups.json +++ b/public/language/en-GB/groups.json @@ -40,6 +40,8 @@ "details.member_count": "Member Count", "details.creation_date": "Creation Date", "details.description": "Description", + "details.member-post-cids": "Categories to display posts from", + "details.member-post-cids-help": "Note: Selecting no categories will assume all categories are included. Use ctrl and shift to select multiple options.", "details.badge_preview": "Badge Preview", "details.change_icon": "Change Icon", "details.change_label_colour": "Change Label Colour", diff --git a/public/openapi/components/schemas/GroupObject.yaml b/public/openapi/components/schemas/GroupObject.yaml index e9802cdf7e..dbef43abc3 100644 --- a/public/openapi/components/schemas/GroupObject.yaml +++ b/public/openapi/components/schemas/GroupObject.yaml @@ -42,6 +42,11 @@ GroupFullObject: textColor: type: string description: A six-character hexadecimal colour code + memberPostCids: + type: array + items: + type: number + example: [1, 2, 3] icon: type: string description: A FontAwesome icon string @@ -56,6 +61,32 @@ GroupFullObject: type: string descriptionParsed: type: string + categories: + type: array + items: + type: object + properties: + cid: + type: number + description: A category identifier + name: + type: string + level: + type: string + icon: + type: string + parentCid: + type: number + description: The category identifier for the category that is the immediate + ancestor of the current category + color: + type: string + bgColor: + type: string + selected: + type: boolean + imageClass: + type: string members: type: array items: @@ -130,4 +161,9 @@ GroupDataObject: type: string description: "`createtime` rendered as an ISO 8601 format" cover:position: - type: string \ No newline at end of file + type: string + memberPostCids: + type: array + items: + type: number + example: [1, 2, 3] \ No newline at end of file diff --git a/public/openapi/read/admin/manage/groups.yaml b/public/openapi/read/admin/manage/groups.yaml index 649147348c..7b7ae1db46 100644 --- a/public/openapi/read/admin/manage/groups.yaml +++ b/public/openapi/read/admin/manage/groups.yaml @@ -64,6 +64,11 @@ get: type: string ownerUid: type: number + memberPostCids: + type: array + items: + type: number + example: [1, 2, 3] required: - name - description diff --git a/public/openapi/read/groups.yaml b/public/openapi/read/groups.yaml index 90a8c31d1d..a73b29864f 100644 --- a/public/openapi/read/groups.yaml +++ b/public/openapi/read/groups.yaml @@ -58,6 +58,11 @@ get: type: string cover:position: type: string + memberPostCids: + type: array + items: + type: number + example: [1, 2, 3] members: type: array items: diff --git a/public/openapi/write/groups.yaml b/public/openapi/write/groups.yaml index 8ae4d703f8..8d325c758e 100644 --- a/public/openapi/write/groups.yaml +++ b/public/openapi/write/groups.yaml @@ -37,6 +37,11 @@ post: enum: [0, 1] createtime: type: number + memberPostCids: + type: array + items: + type: number + example: [1, 2, 3] required: - name responses: diff --git a/public/src/admin/manage/group.js b/public/src/admin/manage/group.js index 0d2ff00403..a64a8d88c5 100644 --- a/public/src/admin/manage/group.js +++ b/public/src/admin/manage/group.js @@ -80,6 +80,7 @@ define('admin/manage/group', [ userTitleEnabled: $('#group-userTitleEnabled').is(':checked'), private: $('#group-private').is(':checked'), hidden: $('#group-hidden').is(':checked'), + memberPostCids: $('#memberPostCids').val(), disableJoinRequests: $('#group-disableJoinRequests').is(':checked'), disableLeave: $('#group-disableLeave').is(':checked'), }, diff --git a/src/groups/data.js b/src/groups/data.js index c866902b4c..6747fc0078 100644 --- a/src/groups/data.js +++ b/src/groups/data.js @@ -75,6 +75,7 @@ function modifyGroup(group, fields) { group.icon = validator.escape(String(group.icon || '')); group.createtimeISO = utils.toISOString(group.createtime); group.private = ([null, undefined].includes(group.private)) ? 1 : group.private; + group.memberPostCids = (group.memberPostCids || '').split(',').map(cid => parseInt(cid, 10)).filter(Boolean); group['cover:thumb:url'] = group['cover:thumb:url'] || group['cover:url']; diff --git a/src/groups/index.js b/src/groups/index.js index 4140e558f9..732f415e6b 100644 --- a/src/groups/index.js +++ b/src/groups/index.js @@ -1,6 +1,7 @@ 'use strict'; const user = require('../user'); +const categories = require('../categories'); const db = require('../database'); const plugins = require('../plugins'); const slugify = require('../slugify'); @@ -119,9 +120,10 @@ Groups.get = async function (groupName, options) { stop = (parseInt(options.userListCount, 10) || 4) - 1; } - const [groupData, members, pending, invited, isMember, isPending, isInvited, isOwner] = await Promise.all([ + const [groupData, members, selectCategories, pending, invited, isMember, isPending, isInvited, isOwner] = await Promise.all([ Groups.getGroupData(groupName), Groups.getOwnersAndMembers(groupName, options.uid, 0, stop), + categories.buildForSelect(groupName, 'topics:read', []), Groups.getUsersFromSet('group:' + groupName + ':pending', ['username', 'userslug', 'picture']), Groups.getUsersFromSet('group:' + groupName + ':invited', ['username', 'userslug', 'picture']), Groups.isMember(options.uid, groupName), @@ -135,6 +137,10 @@ Groups.get = async function (groupName, options) { } const descriptionParsed = await plugins.fireHook('filter:parse.raw', groupData.description); groupData.descriptionParsed = descriptionParsed; + groupData.categories = selectCategories.map((category) => { + category.selected = groupData.memberPostCids.includes(category.cid); + return category; + }); groupData.members = members; groupData.membersNextStart = stop + 1; groupData.pending = pending.filter(Boolean); diff --git a/src/groups/posts.js b/src/groups/posts.js index a96cd9e217..465f060b4b 100644 --- a/src/groups/posts.js +++ b/src/groups/posts.js @@ -1,6 +1,7 @@ 'use strict'; const db = require('../database'); +const groups = require('.'); const privileges = require('../privileges'); const posts = require('../posts'); @@ -13,6 +14,10 @@ module.exports = function (Groups) { let groupNames = await Groups.getUserGroupMembership('groups:visible:createtime', [postData.uid]); groupNames = groupNames[0]; + // Only process those groups that have the cid in its memberPostCids setting (or no setting at all) + const groupsCids = await groups.getGroupsFields(groupNames, ['memberPostCids']); + groupNames = groupNames.filter((groupName, idx) => !groupsCids[idx].memberPostCids.length || groupsCids[idx].memberPostCids.includes(postData.cid)); + const keys = groupNames.map(groupName => 'group:' + groupName + ':member:pids'); await db.sortedSetsAdd(keys, postData.timestamp, postData.pid); await Promise.all(groupNames.map(name => truncateMemberPosts(name))); diff --git a/src/groups/update.js b/src/groups/update.js index eb477944c5..50027fc9bd 100644 --- a/src/groups/update.js +++ b/src/groups/update.js @@ -2,6 +2,7 @@ const winston = require('winston'); +const categories = require('../categories'); const plugins = require('../plugins'); const slugify = require('../slugify'); const db = require('../database'); @@ -18,11 +19,10 @@ module.exports = function (Groups) { throw new Error('[[error:no-group]]'); } - const result = await plugins.fireHook('filter:group.update', { + ({ values } = await plugins.fireHook('filter:group.update', { groupName: groupName, values: values, - }); - values = result.values; + })); const payload = { description: values.description || '', @@ -66,6 +66,12 @@ module.exports = function (Groups) { if (values.hasOwnProperty('hidden')) { await updateVisibility(groupName, values.hidden); } + + if (values.hasOwnProperty('memberPostCids')) { + const validCids = await categories.getCidsByPrivilege('categories:cid', groupName, 'topics:read'); + payload.memberPostCids = values.memberPostCids.filter(cid => validCids.includes(cid)).join(',') || ''; + } + await db.setObject('group:' + groupName, payload); await Groups.renameGroup(groupName, values.name); diff --git a/src/plugins/hooks.js b/src/plugins/hooks.js index 529cc8059d..6ce56fd988 100644 --- a/src/plugins/hooks.js +++ b/src/plugins/hooks.js @@ -6,7 +6,7 @@ const utils = require('../utils'); module.exports = function (Plugins) { Plugins.deprecatedHooks = { - + 'filter:privileges:isUserAllowedTo': 'filter:privileges:isAllowedTo', }; Plugins.internals = { diff --git a/src/privileges/admin.js b/src/privileges/admin.js index dc8ad7e343..305ea98fde 100644 --- a/src/privileges/admin.js +++ b/src/privileges/admin.js @@ -144,7 +144,7 @@ module.exports = function (privileges) { privileges.admin.get = async function (uid) { const [userPrivileges, isAdministrator] = await Promise.all([ - helpers.isUserAllowedTo(privileges.admin.userPrivilegeList, uid, 0), + helpers.isAllowedTo(privileges.admin.userPrivilegeList, uid, 0), user.isAdministrator(uid), ]); @@ -157,7 +157,7 @@ module.exports = function (privileges) { privileges.admin.can = async function (privilege, uid) { const [isUserAllowedTo, isAdministrator] = await Promise.all([ - helpers.isUserAllowedTo(privilege, uid, [0]), + helpers.isAllowedTo(privilege, uid, [0]), user.isAdministrator(uid), ]); return isAdministrator || isUserAllowedTo[0]; diff --git a/src/privileges/categories.js b/src/privileges/categories.js index d1c867e27e..b915f289da 100644 --- a/src/privileges/categories.js +++ b/src/privileges/categories.js @@ -46,7 +46,7 @@ module.exports = function (privileges) { const privs = ['topics:create', 'topics:read', 'topics:tag', 'read']; const [userPrivileges, isAdministrator, isModerator] = await Promise.all([ - helpers.isUserAllowedTo(privs, uid, cid), + helpers.isAllowedTo(privs, uid, cid), user.isAdministrator(uid), user.isModerator(uid, cid), ]); @@ -80,7 +80,7 @@ module.exports = function (privileges) { if (!cid) { return false; } - const results = await helpers.isUserAllowedTo(privilege, uid, Array.isArray(cid) ? cid : [cid]); + const results = await helpers.isAllowedTo(privilege, uid, Array.isArray(cid) ? cid : [cid]); if (Array.isArray(results) && results.length) { return Array.isArray(cid) ? results : results[0]; @@ -113,8 +113,8 @@ module.exports = function (privileges) { privileges.categories.getBase = async function (privilege, cids, uid) { return await utils.promiseParallel({ categories: categories.getCategoriesFields(cids, ['disabled']), - allowedTo: helpers.isUserAllowedTo(privilege, uid, cids), - view_deleted: helpers.isUserAllowedTo('posts:view_deleted', uid, cids), + allowedTo: helpers.isAllowedTo(privilege, uid, cids), + view_deleted: helpers.isAllowedTo('posts:view_deleted', uid, cids), isAdmin: user.isAdministrator(uid), }); }; diff --git a/src/privileges/global.js b/src/privileges/global.js index 7c1f3f7dcd..8a11e8a71a 100644 --- a/src/privileges/global.js +++ b/src/privileges/global.js @@ -75,7 +75,7 @@ module.exports = function (privileges) { privileges.global.get = async function (uid) { const [userPrivileges, isAdministrator] = await Promise.all([ - helpers.isUserAllowedTo(privileges.global.userPrivilegeList, uid, 0), + helpers.isAllowedTo(privileges.global.userPrivilegeList, uid, 0), user.isAdministrator(uid), ]); @@ -88,7 +88,7 @@ module.exports = function (privileges) { privileges.global.can = async function (privilege, uid) { const [isAdministrator, isUserAllowedTo] = await Promise.all([ user.isAdministrator(uid), - helpers.isUserAllowedTo(privilege, uid, [0]), + helpers.isAllowedTo(privilege, uid, [0]), ]); return isAdministrator || isUserAllowedTo[0]; }; diff --git a/src/privileges/helpers.js b/src/privileges/helpers.js index 9d1fb78e61..01bd609ece 100644 --- a/src/privileges/helpers.js +++ b/src/privileges/helpers.js @@ -26,26 +26,45 @@ helpers.isUsersAllowedTo = async function (privilege, uids, cid) { return result.allowed; }; -helpers.isUserAllowedTo = async function (privilege, uid, cid) { +helpers.isAllowedTo = async function (privilege, uidOrGroupName, cid) { let allowed; if (Array.isArray(privilege) && !Array.isArray(cid)) { - allowed = await isUserAllowedToPrivileges(privilege, uid, cid); + allowed = await isAllowedToPrivileges(privilege, uidOrGroupName, cid); } else if (Array.isArray(cid) && !Array.isArray(privilege)) { - allowed = await isUserAllowedToCids(privilege, uid, cid); + allowed = await isAllowedToCids(privilege, uidOrGroupName, cid); } if (allowed) { - const result = await plugins.fireHook('filter:privileges:isUserAllowedTo', { allowed: allowed, privilege: privilege, uid: uid, cid: cid }); - return result.allowed; + ({ allowed } = await plugins.fireHook('filter:privileges:isUserAllowedTo', { allowed: allowed, privilege: privilege, uid: uidOrGroupName, cid: cid })); + ({ allowed } = await plugins.fireHook('filter:privileges:isAllowedTo', { allowed: allowed, privilege: privilege, uid: uidOrGroupName, cid: cid })); + return allowed; } throw new Error('[[error:invalid-data]]'); }; -async function isUserAllowedToCids(privilege, uid, cids) { +async function isAllowedToCids(privilege, uidOrGroupName, cids) { if (!privilege) { return cids.map(() => false); } - if (parseInt(uid, 10) <= 0) { - return await isSystemGroupAllowedToCids(privilege, uid, cids); + + // Group handling + if (isNaN(parseInt(uidOrGroupName, 10)) && (uidOrGroupName || '').length) { + const groupKeys = []; + cids.forEach(function (cid) { + groupKeys.push('cid:' + cid + ':privileges:groups:' + privilege); + }); + const sets = await Promise.all([ + groups.isMemberOfGroups(uidOrGroupName, groupKeys), + groups.isMemberOfGroups('registered-users', groupKeys), + ]); + return sets[0].reduce((memo, cur, idx) => { + memo.push(cur || sets[1][idx]); + return memo; + }, []); + } + + // User handling + if (parseInt(uidOrGroupName, 10) <= 0) { + return await isSystemGroupAllowedToCids(privilege, uidOrGroupName, cids); } const userKeys = []; @@ -55,12 +74,29 @@ async function isUserAllowedToCids(privilege, uid, cids) { groupKeys.push('cid:' + cid + ':privileges:groups:' + privilege); }); - return await checkIfAllowed(uid, userKeys, groupKeys); + return await checkIfAllowed(uidOrGroupName, userKeys, groupKeys); } -async function isUserAllowedToPrivileges(privileges, uid, cid) { - if (parseInt(uid, 10) <= 0) { - return await isSystemGroupAllowedToPrivileges(privileges, uid, cid); +async function isAllowedToPrivileges(privileges, uidOrGroupName, cid) { + // Group handling + if (isNaN(parseInt(uidOrGroupName, 10)) && (uidOrGroupName || '').length) { + const groupKeys = []; + privileges.forEach(function (privilege) { + groupKeys.push('cid:' + cid + ':privileges:groups:' + privilege); + }); + const sets = await Promise.all([ + groups.isMemberOfGroups(uidOrGroupName, groupKeys), + groups.isMemberOfGroups('registered-users', groupKeys), + ]); + return sets[0].reduce((memo, cur, idx) => { + memo.push(cur || sets[1][idx]); + return memo; + }, []); + } + + // User handling + if (parseInt(uidOrGroupName, 10) <= 0) { + return await isSystemGroupAllowedToPrivileges(privileges, uidOrGroupName, cid); } const userKeys = []; @@ -70,7 +106,7 @@ async function isUserAllowedToPrivileges(privileges, uid, cid) { groupKeys.push('cid:' + cid + ':privileges:groups:' + privilege); }); - return await checkIfAllowed(uid, userKeys, groupKeys); + return await checkIfAllowed(uidOrGroupName, userKeys, groupKeys); } async function checkIfAllowed(uid, userKeys, groupKeys) { diff --git a/src/privileges/posts.js b/src/privileges/posts.js index ac595c0c31..f1274b892e 100644 --- a/src/privileges/posts.js +++ b/src/privileges/posts.js @@ -25,11 +25,11 @@ module.exports = function (privileges) { isAdmin: user.isAdministrator(uid), isModerator: user.isModerator(uid, uniqueCids), isOwner: posts.isOwner(pids, uid), - 'topics:read': helpers.isUserAllowedTo('topics:read', uid, uniqueCids), - read: helpers.isUserAllowedTo('read', uid, uniqueCids), - 'posts:edit': helpers.isUserAllowedTo('posts:edit', uid, uniqueCids), - 'posts:history': helpers.isUserAllowedTo('posts:history', uid, uniqueCids), - 'posts:view_deleted': helpers.isUserAllowedTo('posts:view_deleted', uid, uniqueCids), + 'topics:read': helpers.isAllowedTo('topics:read', uid, uniqueCids), + read: helpers.isAllowedTo('read', uid, uniqueCids), + 'posts:edit': helpers.isAllowedTo('posts:edit', uid, uniqueCids), + 'posts:history': helpers.isAllowedTo('posts:history', uid, uniqueCids), + 'posts:view_deleted': helpers.isAllowedTo('posts:view_deleted', uid, uniqueCids), }); const isModerator = _.zipObject(uniqueCids, results.isModerator); diff --git a/src/privileges/topics.js b/src/privileges/topics.js index d4b34194ff..75a3e01fb3 100644 --- a/src/privileges/topics.js +++ b/src/privileges/topics.js @@ -23,7 +23,7 @@ module.exports = function (privileges) { ]; const topicData = await topics.getTopicFields(tid, ['cid', 'uid', 'locked', 'deleted']); const [userPrivileges, isAdministrator, isModerator, disabled] = await Promise.all([ - helpers.isUserAllowedTo(privs, uid, topicData.cid), + helpers.isAllowedTo(privs, uid, topicData.cid), user.isAdministrator(uid), user.isModerator(uid, topicData.cid), categories.getCategoryField(topicData.cid, 'disabled'), @@ -121,7 +121,7 @@ module.exports = function (privileges) { user.isModerator(uid, topicData.cid), user.isAdministrator(uid), topics.isOwner(tid, uid), - helpers.isUserAllowedTo('topics:delete', uid, [topicData.cid]), + helpers.isAllowedTo('topics:delete', uid, [topicData.cid]), ]); if (isAdministrator) { diff --git a/src/privileges/users.js b/src/privileges/users.js index 1cf9441aee..2f43ceca13 100644 --- a/src/privileges/users.js +++ b/src/privileges/users.js @@ -41,7 +41,7 @@ module.exports = function (privileges) { return await filterIsModerator(cids, uid, cids.map(() => true)); } const uniqueCids = _.uniq(cids); - const isAllowed = await helpers.isUserAllowedTo('moderate', uid, uniqueCids); + const isAllowed = await helpers.isAllowedTo('moderate', uid, uniqueCids); const cidToIsAllowed = _.zipObject(uniqueCids, isAllowed); const isModerator = cids.map(cid => cidToIsAllowed[cid]); diff --git a/src/views/admin/manage/group.tpl b/src/views/admin/manage/group.tpl index 382a26cf8e..282d864b06 100644 --- a/src/views/admin/manage/group.tpl +++ b/src/views/admin/manage/group.tpl @@ -95,6 +95,22 @@ +
+ +
+ + +

[[groups:details.member-post-cids-help]]

+
+ +
+