diff --git a/src/graphql/mutations/CreateOrUpdateArticleReplyFeedback.js b/src/graphql/mutations/CreateOrUpdateArticleReplyFeedback.js index 7ba6c161..ca6c3709 100644 --- a/src/graphql/mutations/CreateOrUpdateArticleReplyFeedback.js +++ b/src/graphql/mutations/CreateOrUpdateArticleReplyFeedback.js @@ -15,6 +15,79 @@ export function getArticleReplyFeedbackId({ return `${articleId}__${replyId}__${userId}__${appId}`; } +/** + * Updates the positive and negative feedback count of the article reply with + * specified `articleId` and `replyId`. + * + * @param {string} articleId + * @param {string} replyId + * @param {object[]} feedbacks + * @returns {object} The updated article reply + */ +export async function updateArticleReplyByFeedbacks( + articleId, + replyId, + feedbacks +) { + const [positiveFeedbackCount, negativeFeedbackCount] = feedbacks + .filter(({ status }) => status === 'NORMAL') + .reduce( + (agg, { score }) => { + if (score === 1) { + agg[0] += 1; + } else if (score === -1) { + agg[1] += 1; + } + return agg; + }, + [0, 0] + ); + + const { body: articleReplyUpdateResult } = await client.update({ + index: 'articles', + type: 'doc', + id: articleId, + body: { + script: { + source: ` + int idx = 0; + int replyCount = ctx._source.articleReplies.size(); + for(; idx < replyCount; idx += 1) { + HashMap articleReply = ctx._source.articleReplies.get(idx); + if( articleReply.get('replyId').equals(params.replyId) ) { + break; + } + } + + if( idx === replyCount ) { + ctx.op = 'none'; + } else { + ctx._source.articleReplies.get(idx).put( + 'positiveFeedbackCount', params.positiveFeedbackCount); + ctx._source.articleReplies.get(idx).put( + 'negativeFeedbackCount', params.negativeFeedbackCount); + } + `, + params: { + replyId, + positiveFeedbackCount, + negativeFeedbackCount, + }, + }, + }, + _source: true, + }); + + /* istanbul ignore if */ + if (articleReplyUpdateResult.result !== 'updated') { + throw new Error(`Cannot article ${articleId}'s feedback count`); + } + + return articleReplyUpdateResult.get._source.articleReplies.find( + articleReply => articleReply.replyId === replyId + ); +} + export default { description: 'Create or update a feedback on an article-reply connection', type: ArticleReply, @@ -72,64 +145,10 @@ export default { replyId, }); - const [positiveFeedbackCount, negativeFeedbackCount] = feedbacks - .filter(({ status }) => status === 'NORMAL') - .reduce( - (agg, { score }) => { - if (score === 1) { - agg[0] += 1; - } else if (score === -1) { - agg[1] += 1; - } - return agg; - }, - [0, 0] - ); - - const { body: articleReplyUpdateResult } = await client.update({ - index: 'articles', - type: 'doc', - id: articleId, - body: { - script: { - source: ` - int idx = 0; - int replyCount = ctx._source.articleReplies.size(); - for(; idx < replyCount; idx += 1) { - HashMap articleReply = ctx._source.articleReplies.get(idx); - if( articleReply.get('replyId').equals(params.replyId) ) { - break; - } - } - - if( idx === replyCount ) { - ctx.op = 'none'; - } else { - ctx._source.articleReplies.get(idx).put( - 'positiveFeedbackCount', params.positiveFeedbackCount); - ctx._source.articleReplies.get(idx).put( - 'negativeFeedbackCount', params.negativeFeedbackCount); - } - `, - params: { - replyId, - positiveFeedbackCount, - negativeFeedbackCount, - }, - }, - }, - _source: true, - }); - - /* istanbul ignore if */ - if (articleReplyUpdateResult.result !== 'updated') { - throw new Error( - `Cannot article ${articleId}'s feedback count for feedback ID = ${id}` - ); - } - - const updatedArticleReply = articleReplyUpdateResult.get._source.articleReplies.find( - articleReply => articleReply.replyId === replyId + const updatedArticleReply = await updateArticleReplyByFeedbacks( + articleId, + replyId, + feedbacks ); /* istanbul ignore if */ diff --git a/src/scripts/__fixtures__/blockUser.js b/src/scripts/__fixtures__/blockUser.js index ac467e80..63adc888 100644 --- a/src/scripts/__fixtures__/blockUser.js +++ b/src/scripts/__fixtures__/blockUser.js @@ -15,9 +15,20 @@ export default { createdAt: '2021-11-11T00:00:00.000Z', }, + // Article with already-blocked contents '/articles/doc/some-article': { replyRequestCount: 1, + normalArticleReplyCount: 0, lastRequestedAt: '2021-01-01T00:00.000Z', + articleReplies: [ + { + replyId: 'some-reply', + userId: 'already-blocked', + appId: 'APP_ID', + status: 'BLOCKED', + updatedAt: '2021-11-11T00:00:00.000Z', + }, + ], }, '/replyrequests/doc/replyrequest-to-block': { @@ -34,8 +45,60 @@ export default { createdAt: '2021-10-10T00:00:00.000Z', }, + // Article with normal contents to block '/articles/doc/modified-article': { replyRequestCount: 2, + normalArticleReplyCount: 2, lastRequestedAt: '2021-01-01T00:00:01.000Z', + + articleReplies: [ + { + replyId: 'valid-reply', + userId: 'valid-user', + appId: 'APP_ID', + status: 'NORMAL', + updatedAt: '2021-11-11T00:00:00.000Z', + positiveFeedbackCount: 1, + negativeFeedbackCount: 1, + }, + { + replyId: 'some-reply', + userId: 'user-to-block', + appId: 'APP_ID', + status: 'NORMAL', + updatedAt: '2021-11-11T00:00:00.000Z', + positiveFeedbackCount: 0, + negativeFeedbackCount: 1, + }, + ], + }, + + '/articlereplyfeedbacks/doc/f-normal': { + userId: 'valid-user', + articleId: 'modified-article', + replyId: 'valid-reply', + score: 1, + status: 'NORMAL', + }, + '/articlereplyfeedbacks/doc/f-normal-2': { + userId: 'valid-user', + articleId: 'modified-article', + replyId: 'some-reply', + score: -1, + status: 'NORMAL', + }, + '/articlereplyfeedbacks/doc/f-spam': { + userId: 'user-to-block', + articleId: 'modified-article', + replyId: 'valid-reply', + score: -1, + status: 'NORMAL', + }, + '/articlereplyfeedbacks/doc/f-already-blocked': { + userId: 'already-blocked', + articleId: 'some-article', + replyId: 'some-reply', + score: 1, + status: 'BLOCKED', }, }; diff --git a/src/scripts/__tests__/blockUser.js b/src/scripts/__tests__/blockUser.js index aa7f829d..a326052b 100644 --- a/src/scripts/__tests__/blockUser.js +++ b/src/scripts/__tests__/blockUser.js @@ -14,6 +14,20 @@ it('fails if userId is not valid', async () => { ); }); +/** + * Asserts the document in database is the same as in the fixture, + * i.e. the document is not modified + * + * @param {string} fixtureKey + * @param {{index: string; id: string;}} clientGetArgs - Arguments for client.get() + */ +async function expectSameAsFixture(fixtureKey, clientGetArgs) { + const { + body: { _source: docInDb }, + } = await client.get({ ...clientGetArgs, type: 'doc' }); + expect(docInDb).toMatchObject(fixtures[fixtureKey]); +} + it('correctly sets the block reason and updates status of their works', async () => { await blockUser({ userId: 'user-to-block', @@ -36,24 +50,29 @@ it('correctly sets the block reason and updates status of their works', async () } `); + // Assert valid contents remain as-is // - // Check reply requests - // + await expectSameAsFixture('/replyrequests/doc/valid-reply-request', { + index: 'replyrequests', + id: 'valid-reply-request', + }); + await expectSameAsFixture('/articlereplyfeedbacks/doc/f-normal', { + index: 'articlereplyfeedbacks', + id: 'f-normal', + }); - // Assert reply requests that is already blocked are not selected to update + // Assert contents that is already blocked are not selected to update // - const { - body: { _source: someArticleWithAlreadyBlockedReplyRequest }, - } = await client.get({ + await expectSameAsFixture('/articles/doc/some-article', { index: 'articles', - type: 'doc', id: 'some-article', }); - expect(someArticleWithAlreadyBlockedReplyRequest).toMatchObject( - fixtures['/articles/doc/some-article'] - ); + await expectSameAsFixture('/articlereplyfeedbacks/doc/f-already-blocked', { + index: 'articlereplyfeedbacks', + id: 'f-already-blocked', + }); - // Assert normal reply requests being blocked and article being updated + // Assert normal contents being blocked and article being updated // const { body: { _source: replyRequestToBlock }, @@ -63,6 +82,7 @@ it('correctly sets the block reason and updates status of their works', async () id: 'replyrequest-to-block', }); expect(replyRequestToBlock.status).toEqual('BLOCKED'); + const { body: { _source: modifiedArticle }, } = await client.get({ @@ -71,11 +91,50 @@ it('correctly sets the block reason and updates status of their works', async () id: 'modified-article', }); expect(modifiedArticle).toMatchObject({ + // Only the article reply by valid-user + normalArticleReplyCount: 1, + // Only replyrequests/doc/valid-reply-request replyRequestCount: 1, lastRequestedAt: fixtures['/replyrequests/doc/valid-reply-request'].createdAt, }); + + // Assert article reply being blocked + expect(modifiedArticle.articleReplies[1]).toMatchObject({ + userId: 'user-to-block', + status: 'BLOCKED', + + // the negative feedback given by valid user is still there + positiveFeedbackCount: + fixtures['/articles/doc/modified-article'].articleReplies[1] + .positiveFeedbackCount, + negativeFeedbackCount: + fixtures['/articles/doc/modified-article'].articleReplies[1] + .negativeFeedbackCount, + }); + + // Assert article reply feedback is being blocked + // + const { + body: { _source: articleReplyFeedbackToBlock }, + } = await client.get({ + index: 'articlereplyfeedbacks', + type: 'doc', + id: 'f-spam', + }); + expect(articleReplyFeedbackToBlock.status).toEqual('BLOCKED'); + + // Assert malicious negative feedback is removed from normal article reply + // + expect(modifiedArticle.articleReplies[0]).toMatchObject({ + userId: 'valid-user', + status: 'NORMAL', + positiveFeedbackCount: + fixtures['/articles/doc/modified-article'].articleReplies[0] + .positiveFeedbackCount, + negativeFeedbackCount: 0, // Originally 1 + }); }); // it('still updates statuses of blocked user even if they are blocked previously') diff --git a/src/scripts/blockUser.js b/src/scripts/blockUser.js index d1b359a6..059653a8 100644 --- a/src/scripts/blockUser.js +++ b/src/scripts/blockUser.js @@ -5,8 +5,11 @@ import 'dotenv/config'; import yargs from 'yargs'; +import { SingleBar } from 'cli-progress'; import client from 'util/client'; import getAllDocs from 'util/getAllDocs'; +import { updateArticleReplyStatus } from 'graphql/mutations/UpdateArticleReplyStatus'; +import { updateArticleReplyByFeedbacks } from 'graphql/mutations/CreateOrUpdateArticleReplyFeedback'; /** * Update user to write blockedReason. Throws if user does not exist. @@ -84,9 +87,7 @@ async function processReplyRequests(userId) { `Updating ${articleIdsWithNormalReplyRequests.length} articles...` ); - for (let i = 0; i < articleIdsWithNormalReplyRequests.length; i += 1) { - const articleId = articleIdsWithNormalReplyRequests[i]; - + for (const [i, articleId] of articleIdsWithNormalReplyRequests.entries()) { const { body: { hits: { total }, @@ -134,7 +135,143 @@ async function processReplyRequests(userId) { * * @param {userId} userId */ -// async function processArticleReplies(/* userId */) {} +async function processArticleReplies(userId) { + const NORMAL_ARTICLE_REPLY_QUERY = { + nested: { + path: 'articleReplies', + query: { + bool: { + must: [ + { + term: { + 'articleReplies.status': 'NORMAL', + }, + }, + { + term: { + 'articleReplies.userId': userId, + }, + }, + ], + }, + }, + }, + }; + + const articleRepliesToProcess = []; + for await (const { + _id, + _source: { articleReplies }, + } of getAllDocs('articles', NORMAL_ARTICLE_REPLY_QUERY)) { + articleRepliesToProcess.push( + ...articleReplies + .filter(ar => { + // All articleReplies of the matching article is included, + // here we should only process the article replies to block. + return ar.userId === userId; + }) + .map(ar => ({ + ...ar, + + // Insert articleId for updateArticleReplyStatus() in the following for-loop + articleId: _id, + })) + ); + } + + console.log('Updating article replies...'); + const bar = new SingleBar({ stopOnComplete: true }); + bar.start(articleRepliesToProcess.length, 0); + for (const { articleId, replyId, userId, appId } of articleRepliesToProcess) { + await updateArticleReplyStatus({ + articleId, + replyId, + userId, + appId, + status: 'BLOCKED', + }); + bar.increment(); + } + bar.stop(); +} + +/** + * Convert all article reply feedbacks with NORMAL status by the user to BLOCKED status. + * + * @param {userId} userId + */ +async function processArticleReplyFeedbacks(userId) { + const NORMAL_FEEDBACK_QUERY = { + bool: { + must: [{ term: { status: 'NORMAL' } }, { term: { userId } }], + }, + }; + + /* Array of {articleId, replyId} */ + const articleReplyIdsWithNormalFeedbacks = []; + + for await (const { + _source: { articleId, replyId }, + } of getAllDocs('articlereplyfeedbacks', NORMAL_FEEDBACK_QUERY)) { + articleReplyIdsWithNormalFeedbacks.push({ articleId, replyId }); + } + + /* Bulk update feedback status */ + const { body: updateByQueryResult } = await client.updateByQuery({ + index: 'articlereplyfeedbacks', + type: 'doc', + body: { + query: NORMAL_FEEDBACK_QUERY, + script: { + lang: 'painless', + source: `ctx._source.status = 'BLOCKED';`, + }, + }, + refresh: true, + }); + + console.log( + 'Article reply feedback status update result', + updateByQueryResult + ); + + /* Bulk update articleReply's feedback counts */ + console.log( + `Updating ${articleReplyIdsWithNormalFeedbacks.length} article-replies...` + ); + + for (const [ + i, + { articleId, replyId }, + ] of articleReplyIdsWithNormalFeedbacks.entries()) { + const feedbacks = []; + for await (const { _source: feedback } of getAllDocs( + 'articlereplyfeedbacks', + { + bool: { + must: [ + { term: { status: 'NORMAL' } }, + { term: { articleId } }, + { term: { replyId } }, + ], + }, + } + )) { + feedbacks.push(feedback); + } + + const { + positiveFeedbackCount, + negativeFeedbackCount, + } = await updateArticleReplyByFeedbacks(articleId, replyId, feedbacks); + + console.log( + `[${i + 1}/${ + articleReplyIdsWithNormalFeedbacks.length + }] article=${articleId} reply=${replyId}: score changed to +${positiveFeedbackCount}, -${negativeFeedbackCount}` + ); + } +} /** * @param {object} args @@ -142,7 +279,8 @@ async function processReplyRequests(userId) { async function main({ userId, blockedReason } = {}) { await writeBlockedReasonToUser(userId, blockedReason); await processReplyRequests(userId); - // await processArticleReplies(userId); + await processArticleReplies(userId); + await processArticleReplyFeedbacks(userId); } export default main;