From 53e1b349ae6f56af12a83b6073b578f3b5471864 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Fri, 12 Jul 2019 14:06:09 -0400 Subject: [PATCH] Change post owner (#7752) * feat: #7749, allow array of keys for setObject * feat: sortedSetRemoveBulk * feat: test for bulk remove * feat: #7083, ability to change post ownership * feat: #7083, fix tid::posters * feat: #7083, front end * fix: #7752, psql methods * fix: add missing await * fix: maybe psql --- public/language/en-GB/topic.json | 3 + public/src/client/topic/change-owner.js | 91 +++++++++++++++++++ public/src/client/topic/postTools.js | 7 ++ public/src/modules/postSelect.js | 8 +- src/database/mongo/hash.js | 9 +- src/database/mongo/sorted/remove.js | 9 ++ src/database/postgres/hash.js | 58 +++++++----- src/database/postgres/sorted/remove.js | 23 +++++ src/database/redis/hash.js | 18 +++- src/database/redis/sorted/remove.js | 9 ++ src/posts/user.js | 113 ++++++++++++++++++++++++ src/socket.io/posts/tools.js | 12 +++ test/database/hash.js | 16 ++++ test/database/sorted.js | 15 ++++ test/posts.js | 30 +++++++ 15 files changed, 395 insertions(+), 26 deletions(-) create mode 100644 public/src/client/topic/change-owner.js diff --git a/public/language/en-GB/topic.json b/public/language/en-GB/topic.json index 40d18227bb..27af72a9df 100644 --- a/public/language/en-GB/topic.json +++ b/public/language/en-GB/topic.json @@ -26,6 +26,7 @@ "purge": "Purge", "restore": "Restore", "move": "Move", + "change-owner": "Change Owner", "fork": "Fork", "link": "Link", "share": "Share", @@ -75,6 +76,7 @@ "thread_tools.move": "Move Topic", "thread_tools.move-posts": "Move Posts", "thread_tools.move_all": "Move All", + "thread_tools.change_owner": "Change Owner", "thread_tools.select_category": "Select Category", "thread_tools.fork": "Fork Topic", "thread_tools.delete": "Delete Topic", @@ -114,6 +116,7 @@ "delete_posts_instruction": "Click the posts you want to delete/purge", "merge_topics_instruction": "Click the topics you want to merge", "move_posts_instruction": "Click the posts you want to move", + "change_owner_instruction": "Click the posts you want to assign to another user", "composer.title_placeholder": "Enter your topic title here...", "composer.handle_placeholder": "Name", diff --git a/public/src/client/topic/change-owner.js b/public/src/client/topic/change-owner.js new file mode 100644 index 0000000000..094ef8c969 --- /dev/null +++ b/public/src/client/topic/change-owner.js @@ -0,0 +1,91 @@ +'use strict'; + + +define('forum/topic/change-owner', [ + 'components', + 'postSelect', + 'autocomplete', +], function (components, postSelect, autocomplete) { + var ChangeOwner = {}; + + var modal; + var commit; + var toUid = 0; + ChangeOwner.init = function (postEl) { + if (modal) { + return; + } + app.parseAndTranslate('partials/change_owner_modal', {}, function (html) { + modal = html; + + commit = modal.find('#change_owner_commit'); + + $('body').append(modal); + + modal.find('.close,#change_owner_cancel').on('click', closeModal); + modal.find('#username').on('keyup', checkButtonEnable); + postSelect.init(onPostToggled, { + allowMainPostSelect: true, + }); + showPostsSelected(); + + if (postEl) { + postSelect.togglePostSelection(postEl, onPostToggled); + } + + commit.on('click', function () { + changeOwner(); + }); + + autocomplete.user(modal.find('#username'), function (ev, ui) { + toUid = ui.item.user.uid; + checkButtonEnable(); + }); + }); + }; + + function showPostsSelected() { + if (postSelect.pids.length) { + modal.find('#pids').translateHtml('[[topic:fork_pid_count, ' + postSelect.pids.length + ']]'); + } else { + modal.find('#pids').translateHtml('[[topic:fork_no_pids]]'); + } + } + + function checkButtonEnable() { + if (toUid && modal.find('#username').length && modal.find('#username').val().length && postSelect.pids.length) { + commit.removeAttr('disabled'); + } else { + commit.attr('disabled', true); + } + } + + function onPostToggled() { + checkButtonEnable(); + showPostsSelected(); + } + + function changeOwner() { + if (!toUid) { + return; + } + socket.emit('posts.changeOwner', { pids: postSelect.pids, toUid: toUid }, function (err) { + if (err) { + return app.alertError(err.message); + } + ajaxify.refresh(); + + closeModal(); + }); + } + + function closeModal() { + if (modal) { + modal.remove(); + modal = null; + postSelect.disable(); + } + } + + return ChangeOwner; +}); diff --git a/public/src/client/topic/postTools.js b/public/src/client/topic/postTools.js index 4d29d966dd..6dea439ce7 100644 --- a/public/src/client/topic/postTools.js +++ b/public/src/client/topic/postTools.js @@ -207,6 +207,13 @@ define('forum/topic/postTools', [ }); }); + postContainer.on('click', '[component="post/change-owner"]', function () { + var btn = $(this); + require(['forum/topic/change-owner'], function (changeOwner) { + changeOwner.init(btn.parents('[data-pid]')); + }); + }); + postContainer.on('click', '[component="post/ban-ip"]', function () { var ip = $(this).attr('data-ip'); socket.emit('blacklist.addRule', ip, function (err) { diff --git a/public/src/modules/postSelect.js b/public/src/modules/postSelect.js index 8b70689f7c..a44420e2d3 100644 --- a/public/src/modules/postSelect.js +++ b/public/src/modules/postSelect.js @@ -7,9 +7,13 @@ define('postSelect', ['components'], function (components) { PostSelect.pids = []; - PostSelect.init = function (_onSelect) { + var allowMainPostSelect = false; + + PostSelect.init = function (_onSelect, options) { PostSelect.pids.length = 0; onSelect = _onSelect; + options = options || {}; + allowMainPostSelect = options.allowMainPostSelect || false; $('#content').on('click', '[component="topic"] [component="post"]', onPostClicked); disableClicksOnPosts(); }; @@ -30,7 +34,7 @@ define('postSelect', ['components'], function (components) { PostSelect.togglePostSelection = function (post) { var newPid = post.attr('data-pid'); - if (parseInt(post.attr('data-index'), 10) === 0) { + if (!allowMainPostSelect && parseInt(post.attr('data-index'), 10) === 0) { return; } diff --git a/src/database/mongo/hash.js b/src/database/mongo/hash.js index 8d7e1c8123..cacde06614 100644 --- a/src/database/mongo/hash.js +++ b/src/database/mongo/hash.js @@ -14,7 +14,14 @@ module.exports = function (db, module) { } const writeData = helpers.serializeData(data); - await db.collection('objects').updateOne({ _key: key }, { $set: writeData }, { upsert: true, w: 1 }); + if (Array.isArray(key)) { + var bulk = db.collection('objects').initializeUnorderedBulkOp(); + key.forEach(key => bulk.find({ _key: key }).upsert().updateOne({ $set: writeData })); + await bulk.execute(); + } else { + await db.collection('objects').updateOne({ _key: key }, { $set: writeData }, { upsert: true, w: 1 }); + } + cache.delObjectCache(key); }; diff --git a/src/database/mongo/sorted/remove.js b/src/database/mongo/sorted/remove.js index 2803f2fcd2..3fe486d433 100644 --- a/src/database/mongo/sorted/remove.js +++ b/src/database/mongo/sorted/remove.js @@ -49,4 +49,13 @@ module.exports = function (db, module) { await db.collection('objects').deleteMany(query); }; + + module.sortedSetRemoveBulk = async function (data) { + if (!Array.isArray(data) || !data.length) { + return; + } + var bulk = db.collection('objects').initializeUnorderedBulkOp(); + data.forEach(item => bulk.find({ _key: item[0], value: String(item[1]) }).remove()); + await bulk.execute(); + }; }; diff --git a/src/database/postgres/hash.js b/src/database/postgres/hash.js index 1a6c4aecaa..435f1fcee5 100644 --- a/src/database/postgres/hash.js +++ b/src/database/postgres/hash.js @@ -14,17 +14,24 @@ module.exports = function (db, module) { await module.transaction(async function (client) { var query = client.query.bind(client); - - await helpers.ensureLegacyObjectType(client, key, 'hash'); - await query({ - name: 'setObject', - text: ` -INSERT INTO "legacy_hash" ("_key", "data") -VALUES ($1::TEXT, $2::TEXT::JSONB) -ON CONFLICT ("_key") -DO UPDATE SET "data" = "legacy_hash"."data" || $2::TEXT::JSONB`, - values: [key, JSON.stringify(data)], - }); + const dataString = JSON.stringify(data); + async function setOne(key) { + await helpers.ensureLegacyObjectType(client, key, 'hash'); + await query({ + name: 'setObject', + text: ` + INSERT INTO "legacy_hash" ("_key", "data") + VALUES ($1::TEXT, $2::TEXT::JSONB) + ON CONFLICT ("_key") + DO UPDATE SET "data" = "legacy_hash"."data" || $2::TEXT::JSONB`, + values: [key, dataString], + }); + } + if (Array.isArray(key)) { + await Promise.all(key.map(k => setOne(k))); + } else { + await setOne(key); + } }); }; @@ -35,16 +42,25 @@ DO UPDATE SET "data" = "legacy_hash"."data" || $2::TEXT::JSONB`, await module.transaction(async function (client) { var query = client.query.bind(client); - await helpers.ensureLegacyObjectType(client, key, 'hash'); - await query({ - name: 'setObjectField', - text: ` -INSERT INTO "legacy_hash" ("_key", "data") -VALUES ($1::TEXT, jsonb_build_object($2::TEXT, $3::TEXT::JSONB)) -ON CONFLICT ("_key") -DO UPDATE SET "data" = jsonb_set("legacy_hash"."data", ARRAY[$2::TEXT], $3::TEXT::JSONB)`, - values: [key, field, JSON.stringify(value)], - }); + const valueString = JSON.stringify(value); + async function setOne(key) { + await helpers.ensureLegacyObjectType(client, key, 'hash'); + await query({ + name: 'setObjectField', + text: ` + INSERT INTO "legacy_hash" ("_key", "data") + VALUES ($1::TEXT, jsonb_build_object($2::TEXT, $3::TEXT::JSONB)) + ON CONFLICT ("_key") + DO UPDATE SET "data" = jsonb_set("legacy_hash"."data", ARRAY[$2::TEXT], $3::TEXT::JSONB)`, + values: [key, field, valueString], + }); + } + + if (Array.isArray(key)) { + await Promise.all(key.map(k => setOne(k))); + } else { + await setOne(key); + } }); }; diff --git a/src/database/postgres/sorted/remove.js b/src/database/postgres/sorted/remove.js index bf5e9fd69d..a985f597d1 100644 --- a/src/database/postgres/sorted/remove.js +++ b/src/database/postgres/sorted/remove.js @@ -69,4 +69,27 @@ DELETE FROM "legacy_zset" values: [keys, min, max], }); }; + + module.sortedSetRemoveBulk = async function (data) { + // const keys = []; + // const values = []; + + // data.forEach(function (item) { + // keys.push(item[0]); + // values.push(item[1]); + // }); + + const promises = data.map(item => module.sortedSetRemove(item[0], item[1])); + await Promise.all(promises); + + // TODO + // await query({ + // name: 'sortedSetRemoveBulk', + // text: ` + // DELETE FROM "legacy_zset" + // SELECT k, v + // FROM UNNEST($1::TEXT[], $2::TEXT[]) vs(k, v)`, + // values: [keys, values], + // }); + }; }; diff --git a/src/database/redis/hash.js b/src/database/redis/hash.js index 873547f80b..16114b2669 100644 --- a/src/database/redis/hash.js +++ b/src/database/redis/hash.js @@ -27,7 +27,14 @@ module.exports = function (redisClient, module) { if (!Object.keys(data).length) { return; } - await redisClient.async.hmset(key, data); + if (Array.isArray(key)) { + const batch = redisClient.batch(); + key.forEach(k => batch.hmset(k, data)); + await helpers.execBatch(batch); + } else { + await redisClient.async.hmset(key, data); + } + cache.delObjectCache(key); }; @@ -35,7 +42,14 @@ module.exports = function (redisClient, module) { if (!field) { return; } - await redisClient.async.hset(key, field, value); + if (Array.isArray(key)) { + const batch = redisClient.batch(); + key.forEach(k => batch.hset(k, field, value)); + await helpers.execBatch(batch); + } else { + await redisClient.async.hset(key, field, value); + } + cache.delObjectCache(key); }; diff --git a/src/database/redis/sorted/remove.js b/src/database/redis/sorted/remove.js index d8f5000964..5d973fee7a 100644 --- a/src/database/redis/sorted/remove.js +++ b/src/database/redis/sorted/remove.js @@ -34,4 +34,13 @@ module.exports = function (redisClient, module) { keys.forEach(k => batch.zremrangebyscore(k, min, max)); await helpers.execBatch(batch); }; + + module.sortedSetRemoveBulk = async function (data) { + if (!Array.isArray(data) || !data.length) { + return; + } + const batch = redisClient.batch(); + data.forEach(item => batch.zrem(item[0], item[1])); + await helpers.execBatch(batch); + }; }; diff --git a/src/posts/user.js b/src/posts/user.js index 922204745b..eb513e2157 100644 --- a/src/posts/user.js +++ b/src/posts/user.js @@ -4,7 +4,9 @@ var async = require('async'); var validator = require('validator'); var _ = require('lodash'); +const db = require('../database'); var user = require('../user'); +const topics = require('../topics'); var groups = require('../groups'); var meta = require('../meta'); var plugins = require('../plugins'); @@ -121,4 +123,115 @@ module.exports = function (Posts) { const cids = await Posts.getCidsByPids(pids); return await user.isModerator(uid, cids); }; + + Posts.changeOwner = async function (pids, toUid) { + const exists = user.exists(toUid); + if (!exists) { + throw new Error('[[error:no-user]]'); + } + const postData = await Posts.getPostsFields(pids, ['pid', 'tid', 'uid', 'timestamp', 'upvotes', 'downvotes']); + pids = postData.filter(p => p.pid && p.uid !== parseInt(toUid, 10)) + .map(p => p.pid); + + const cids = await Posts.getCidsByPids(pids); + + const bulkRemove = []; + const bulkAdd = []; + let repChange = 0; + const postsByUser = {}; + postData.forEach((post, i) => { + post.cid = cids[i]; + repChange += post.votes; + bulkRemove.push(['uid:' + post.uid + ':posts', post.pid]); + bulkRemove.push(['cid:' + post.cid + ':uid:' + post.uid + ':pids', post.pid]); + bulkRemove.push(['cid:' + post.cid + ':uid:' + post.uid + ':pids:votes', post.pid]); + + bulkAdd.push(['uid:' + toUid + ':posts', post.timestamp, post.pid]); + bulkAdd.push(['cid:' + post.cid + ':uid:' + toUid + ':pids', post.timestamp, post.pid]); + if (post.votes > 0) { + bulkAdd.push(['cid:' + post.cid + ':uid:' + toUid + ':pids:votes', post.votes, post.pid]); + } + postsByUser[post.uid] = postsByUser[post.uid] || []; + postsByUser[post.uid].push(post); + }); + + await Promise.all([ + db.setObjectField(pids.map(pid => 'post:' + pid), 'uid', toUid), + db.sortedSetRemoveBulk(bulkRemove), + db.sortedSetAddBulk(bulkAdd), + user.incrementUserPostCountBy(toUid, pids.length), + updateReputation(toUid, repChange), + handleMainPidOwnerChange(postData, toUid), + reduceCounters(postsByUser), + updateTopicPosters(postData, toUid), + ]); + }; + + async function reduceCounters(postsByUser) { + await async.eachOfSeries(postsByUser, async function (posts, uid) { + const repChange = posts.reduce((acc, val) => acc + val.votes, 0); + await Promise.all([ + user.incrementUserPostCountBy(uid, -posts.length), + updateReputation(uid, -repChange), + ]); + }); + } + + async function updateTopicPosters(postData, toUid) { + const postsByTopic = _.groupBy(postData, p => parseInt(p.tid, 10)); + await async.eachOf(postsByTopic, async function (posts, tid) { + const postsByUser = _.groupBy(posts, p => parseInt(p.uid, 10)); + await db.sortedSetIncrBy('tid:' + tid + ':posters', posts.length, toUid); + await async.eachOf(postsByUser, async function (posts, uid) { + await db.sortedSetIncrBy('tid:' + tid + ':posters', -posts.length, uid); + }); + }); + } + + async function updateReputation(uid, change) { + if (!change) { + return; + } + const newReputation = await user.incrementUserFieldBy(uid, 'reputation', change); + await db.sortedSetAdd('users:reputation', newReputation, uid); + } + + async function handleMainPidOwnerChange(postData, toUid) { + const tids = _.uniq(postData.map(p => p.tid)); + const topicData = await topics.getTopicsFields(tids, ['mainPid', 'timestamp']); + const tidToTopic = _.zipObject(tids, topicData); + + const mainPosts = postData.filter(p => p.pid === tidToTopic[p.tid].mainPid); + if (!mainPosts.length) { + return; + } + + const bulkAdd = []; + const bulkRemove = []; + const postsByUser = {}; + mainPosts.forEach((post) => { + bulkRemove.push(['cid:' + post.cid + ':uid:' + post.uid + ':tids', post.tid]); + bulkRemove.push(['uid:' + post.uid + ':topics', post.tid]); + + bulkAdd.push(['cid:' + post.cid + ':uid:' + toUid + ':tids', tidToTopic[post.tid].timestamp, post.tid]); + bulkAdd.push(['uid:' + toUid + ':topics', tidToTopic[post.tid].timestamp, post.tid]); + postsByUser[post.uid] = postsByUser[post.uid] || []; + postsByUser[post.uid].push(post); + }); + + await Promise.all([ + db.setObjectField(mainPosts.map(p => 'topic:' + p.tid), 'uid', toUid), + db.sortedSetRemoveBulk(bulkRemove), + db.sortedSetAddBulk(bulkAdd), + user.incrementUserFieldBy(toUid, 'topiccount', mainPosts.length), + reduceTopicCounts(postsByUser), + ]); + } + + async function reduceTopicCounts(postsByUser) { + await async.eachSeries(Object.keys(postsByUser), async function (uid) { + const posts = postsByUser[uid]; + await user.incrementUserFieldBy(uid, 'topiccount', -posts.length); + }); + } }; diff --git a/src/socket.io/posts/tools.js b/src/socket.io/posts/tools.js index fccfc9388e..5abf9b7f7d 100644 --- a/src/socket.io/posts/tools.js +++ b/src/socket.io/posts/tools.js @@ -248,4 +248,16 @@ module.exports = function (SocketPosts) { }, }, callback); } + + SocketPosts.changeOwner = async function (socket, data) { + if (!data || !Array.isArray(data.pids) || !data.toUid) { + throw new Error('[[error:invalid-data]]'); + } + const isAdminOrGlobalMod = user.isAdminOrGlobalMod(socket.uid); + if (!isAdminOrGlobalMod) { + throw new Error('[[error:no-privileges]]'); + } + + await posts.changeOwner(data.pids, data.toUid); + }; }; diff --git a/test/database/hash.js b/test/database/hash.js index f521b4238a..5c33ef91a3 100644 --- a/test/database/hash.js +++ b/test/database/hash.js @@ -25,6 +25,14 @@ describe('Hash methods', function () { }); }); + it('should set two objects to same data', async function () { + const data = { foo: 'baz', test: '1' }; + await db.setObject(['multiObject1', 'multiObject2'], data); + const result = await db.getObjects(['multiObject1', 'multiObject2']); + assert.deepStrictEqual(result[0], data); + assert.deepStrictEqual(result[1], data); + }); + it('should do nothing if key is falsy', function (done) { db.setObject('', { foo: 1, derp: 2 }, function (err) { assert.ifError(err); @@ -82,6 +90,14 @@ describe('Hash methods', function () { }); }); + it('should set two objects fields to same data', async function () { + const data = { foo: 'baz', test: '1' }; + await db.setObjectField(['multiObject1', 'multiObject2'], 'myField', '2'); + const result = await db.getObjects(['multiObject1', 'multiObject2']); + assert.deepStrictEqual(result[0].myField, '2'); + assert.deepStrictEqual(result[1].myField, '2'); + }); + it('should work for field names with "." in them', function (done) { db.setObjectField('dotObject2', 'my.dot.field', 'foo2', function (err) { assert.ifError(err); diff --git a/test/database/sorted.js b/test/database/sorted.js index 6da38340ae..1f0561d5fe 100644 --- a/test/database/sorted.js +++ b/test/database/sorted.js @@ -978,6 +978,21 @@ describe('Sorted Set methods', function () { }); }); }); + + it('should do a bulk remove', async function () { + await db.sortedSetAddBulk([ + ['bulkRemove1', 1, 'value1'], + ['bulkRemove1', 2, 'value2'], + ['bulkRemove2', 3, 'value2'], + ]); + await db.sortedSetRemoveBulk([ + ['bulkRemove1', 'value1'], + ['bulkRemove1', 'value2'], + ['bulkRemove2', 'value2'], + ]); + const members = await db.getSortedSetsMembers(['bulkRemove1', 'bulkRemove2']); + assert.deepStrictEqual(members, [[], []]); + }); }); describe('sortedSetsRemove()', function () { diff --git a/test/posts.js b/test/posts.js index a975bc771a..211cc0f9b0 100644 --- a/test/posts.js +++ b/test/posts.js @@ -73,6 +73,36 @@ describe('Post\'s', function () { }); }); + it('should change owner of post and topic properly', async function () { + const oldUid = await user.create({ username: 'olduser' }); + const newUid = await user.create({ username: 'newuser' }); + const postResult = await topics.post({ uid: oldUid, cid: cid, title: 'change owner', content: 'original post' }); + const postData = await topics.reply({ uid: oldUid, tid: postResult.topicData.tid, content: 'firstReply' }); + const pid1 = postResult.postData.pid; + const pid2 = postData.pid; + + assert.deepStrictEqual(await db.sortedSetScores('tid:' + postResult.topicData.tid + ':posters', [oldUid, newUid]), [2, null]); + + await posts.changeOwner([pid1, pid2], newUid); + + assert.deepStrictEqual(await db.sortedSetScores('tid:' + postResult.topicData.tid + ':posters', [oldUid, newUid]), [0, 2]); + + assert.deepStrictEqual(await posts.isOwner([pid1, pid2], oldUid), [false, false]); + assert.deepStrictEqual(await posts.isOwner([pid1, pid2], newUid), [true, true]); + + assert.strictEqual(await user.getUserField(oldUid, 'postcount'), 0); + assert.strictEqual(await user.getUserField(newUid, 'postcount'), 2); + + assert.strictEqual(await user.getUserField(oldUid, 'topiccount'), 0); + assert.strictEqual(await user.getUserField(newUid, 'topiccount'), 1); + + assert.strictEqual(await db.sortedSetScore('users:postcount', oldUid), 0); + assert.strictEqual(await db.sortedSetScore('users:postcount', newUid), 2); + + assert.strictEqual(await topics.isOwner(postResult.topicData.tid, oldUid), false); + assert.strictEqual(await topics.isOwner(postResult.topicData.tid, newUid), true); + }); + it('should return falsy if post does not exist', function (done) { posts.getPostData(9999, function (err, postData) { assert.ifError(err);