From 69222afac8f8c41d90295b33f0695bbff352851e Mon Sep 17 00:00:00 2001 From: Julien Maulny Date: Fri, 15 Nov 2019 19:05:08 +0100 Subject: Soft delete video comments instead of detroy --- server/controllers/activitypub/client.ts | 13 +++-- server/controllers/api/videos/comment.ts | 12 +++-- .../migrations/0450-soft-delete-video-comments.ts | 36 ++++++++++++++ server/lib/activitypub/process/process-delete.ts | 7 ++- server/lib/video-comment.ts | 9 +++- server/models/account/account.ts | 2 +- server/models/video/video-comment.ts | 33 +++++++++++-- server/tests/api/videos/multiple-servers.ts | 56 ++++++++++++++++++---- server/tests/api/videos/video-comments.ts | 24 +++++++--- 9 files changed, 162 insertions(+), 30 deletions(-) create mode 100644 server/initializers/migrations/0450-soft-delete-video-comments.ts (limited to 'server') diff --git a/server/controllers/activitypub/client.ts b/server/controllers/activitypub/client.ts index 453ced8bf..5ed0435ff 100644 --- a/server/controllers/activitypub/client.ts +++ b/server/controllers/activitypub/client.ts @@ -308,13 +308,16 @@ async function videoCommentController (req: express.Request, res: express.Respon const threadParentComments = await VideoCommentModel.listThreadParentComments(videoComment, undefined) const isPublic = true // Comments are always public - const audience = getAudience(videoComment.Account.Actor, isPublic) + let videoCommentObject = videoComment.toActivityPubObject(threadParentComments) - const videoCommentObject = audiencify(videoComment.toActivityPubObject(threadParentComments), audience) + if (videoComment.Account) { + const audience = getAudience(videoComment.Account.Actor, isPublic) + videoCommentObject = audiencify(videoCommentObject, audience) - if (req.path.endsWith('/activity')) { - const data = buildCreateActivity(videoComment.url, videoComment.Account.Actor, videoCommentObject, audience) - return activityPubResponse(activityPubContextify(data), res) + if (req.path.endsWith('/activity')) { + const data = buildCreateActivity(videoComment.url, videoComment.Account.Actor, videoCommentObject, audience) + return activityPubResponse(activityPubContextify(data), res) + } } return activityPubResponse(activityPubContextify(videoCommentObject), res) diff --git a/server/controllers/api/videos/comment.ts b/server/controllers/api/videos/comment.ts index b2b06b170..5f3fed5c0 100644 --- a/server/controllers/api/videos/comment.ts +++ b/server/controllers/api/videos/comment.ts @@ -1,10 +1,11 @@ import * as express from 'express' +import { cloneDeep } from 'lodash' import { ResultList } from '../../../../shared/models' import { VideoCommentCreate } from '../../../../shared/models/videos/video-comment.model' import { logger } from '../../../helpers/logger' import { getFormattedObjects } from '../../../helpers/utils' import { sequelizeTypescript } from '../../../initializers' -import { buildFormattedCommentTree, createVideoComment } from '../../../lib/video-comment' +import { buildFormattedCommentTree, createVideoComment, markCommentAsDeleted } from '../../../lib/video-comment' import { asyncMiddleware, asyncRetryTransactionMiddleware, @@ -177,19 +178,22 @@ async function addVideoCommentReply (req: express.Request, res: express.Response async function removeVideoComment (req: express.Request, res: express.Response) { const videoCommentInstance = res.locals.videoCommentFull + const videoCommentInstanceBefore = cloneDeep(videoCommentInstance) await sequelizeTypescript.transaction(async t => { - await videoCommentInstance.destroy({ transaction: t }) - if (videoCommentInstance.isOwned() || videoCommentInstance.Video.isOwned()) { await sendDeleteVideoComment(videoCommentInstance, t) } + + markCommentAsDeleted(videoCommentInstance) + + await videoCommentInstance.save() }) auditLogger.delete(getAuditIdFromRes(res), new CommentAuditView(videoCommentInstance.toFormattedJSON())) logger.info('Video comment %d deleted.', videoCommentInstance.id) - Hooks.runAction('action:api.video-comment.deleted', { comment: videoCommentInstance }) + Hooks.runAction('action:api.video-comment.deleted', { comment: videoCommentInstanceBefore }) return res.type('json').status(204).end() } diff --git a/server/initializers/migrations/0450-soft-delete-video-comments.ts b/server/initializers/migrations/0450-soft-delete-video-comments.ts new file mode 100644 index 000000000..bcfb97b56 --- /dev/null +++ b/server/initializers/migrations/0450-soft-delete-video-comments.ts @@ -0,0 +1,36 @@ +import * as Sequelize from 'sequelize' + +async function up (utils: { + transaction: Sequelize.Transaction, + queryInterface: Sequelize.QueryInterface, + sequelize: Sequelize.Sequelize, + db: any +}): Promise { + { + const data = { + type: Sequelize.INTEGER, + allowNull: true, + defaultValue: null + } + + await utils.queryInterface.changeColumn('videoComment', 'accountId', data) + } + + { + const data = { + type: Sequelize.DATE, + allowNull: true, + defaultValue: null + } + await utils.queryInterface.addColumn('videoComment', 'deletedAt', data) + } +} + +function down (options) { + throw new Error('Not implemented.') +} + +export { + up, + down +} diff --git a/server/lib/activitypub/process/process-delete.ts b/server/lib/activitypub/process/process-delete.ts index 79d0e0d79..e76132f91 100644 --- a/server/lib/activitypub/process/process-delete.ts +++ b/server/lib/activitypub/process/process-delete.ts @@ -5,6 +5,7 @@ import { sequelizeTypescript } from '../../../initializers' import { ActorModel } from '../../../models/activitypub/actor' import { VideoModel } from '../../../models/video/video' import { VideoCommentModel } from '../../../models/video/video-comment' +import { markCommentAsDeleted } from '../../video-comment' import { forwardVideoRelatedActivity } from '../send/utils' import { VideoPlaylistModel } from '../../../models/video/video-playlist' import { APProcessorOptions } from '../../../typings/activitypub-processor.model' @@ -128,7 +129,11 @@ function processDeleteVideoComment (byActor: MActorSignature, videoComment: Vide throw new Error(`Account ${byActor.url} does not own video comment ${videoComment.url} or video ${videoComment.Video.url}`) } - await videoComment.destroy({ transaction: t }) + await sequelizeTypescript.transaction(async t => { + markCommentAsDeleted(videoComment) + + await videoComment.save() + }) if (videoComment.Video.isOwned()) { // Don't resend the activity to the sender diff --git a/server/lib/video-comment.ts b/server/lib/video-comment.ts index bb811bd2c..b8074e6d2 100644 --- a/server/lib/video-comment.ts +++ b/server/lib/video-comment.ts @@ -73,9 +73,16 @@ function buildFormattedCommentTree (resultList: ResultList): return thread } +function markCommentAsDeleted (comment: MCommentOwnerVideoReply): void { + comment.text = '' + comment.deletedAt = new Date() + comment.accountId = null +} + // --------------------------------------------------------------------------- export { createVideoComment, - buildFormattedCommentTree + buildFormattedCommentTree, + markCommentAsDeleted } diff --git a/server/models/account/account.ts b/server/models/account/account.ts index ba1094536..a818a5a4d 100644 --- a/server/models/account/account.ts +++ b/server/models/account/account.ts @@ -201,7 +201,7 @@ export class AccountModel extends Model { @HasMany(() => VideoCommentModel, { foreignKey: { - allowNull: false + allowNull: true }, onDelete: 'cascade', hooks: true diff --git a/server/models/video/video-comment.ts b/server/models/video/video-comment.ts index 2e4220434..b44d65138 100644 --- a/server/models/video/video-comment.ts +++ b/server/models/video/video-comment.ts @@ -1,5 +1,5 @@ import { AllowNull, BelongsTo, Column, CreatedAt, DataType, ForeignKey, Is, Model, Scopes, Table, UpdatedAt } from 'sequelize-typescript' -import { ActivityTagObject } from '../../../shared/models/activitypub/objects/common-objects' +import { ActivityTagObject, ActivityTombstoneObject } from '../../../shared/models/activitypub/objects/common-objects' import { VideoCommentObject } from '../../../shared/models/activitypub/objects/video-comment-object' import { VideoComment } from '../../../shared/models/videos/video-comment.model' import { isActivityPubUrlValid } from '../../helpers/custom-validators/activitypub/misc' @@ -122,6 +122,10 @@ export class VideoCommentModel extends Model { @UpdatedAt updatedAt: Date + @AllowNull(true) + @Column(DataType.DATE) + deletedAt: Date + @AllowNull(false) @Is('VideoCommentUrl', value => throwIfNotValid(value, isActivityPubUrlValid, 'url')) @Column(DataType.STRING(CONSTRAINTS_FIELDS.VIDEOS.URL.max)) @@ -177,7 +181,7 @@ export class VideoCommentModel extends Model { @BelongsTo(() => AccountModel, { foreignKey: { - allowNull: false + allowNull: true }, onDelete: 'CASCADE' }) @@ -436,9 +440,17 @@ export class VideoCommentModel extends Model { } isOwned () { + if (!this.Account) { + return false + } + return this.Account.isOwned() } + isDeleted () { + return null !== this.deletedAt + } + extractMentions () { let result: string[] = [] @@ -487,12 +499,25 @@ export class VideoCommentModel extends Model { videoId: this.videoId, createdAt: this.createdAt, updatedAt: this.updatedAt, + deletedAt: this.deletedAt, + isDeleted: this.isDeleted(), totalReplies: this.get('totalReplies') || 0, - account: this.Account.toFormattedJSON() + account: this.Account ? this.Account.toFormattedJSON() : null } as VideoComment } - toActivityPubObject (this: MCommentAP, threadParentComments: MCommentOwner[]): VideoCommentObject { + toActivityPubObject (this: MCommentAP, threadParentComments: MCommentOwner[]): VideoCommentObject | ActivityTombstoneObject { + if (this.isDeleted()) { + return { + id: this.url, + type: 'Tombstone', + formerType: 'Note', + published: this.createdAt.toISOString(), + updated: this.updatedAt.toISOString(), + deleted: this.deletedAt.toISOString() + } + } + let inReplyTo: string // New thread, so in AS we reply to the video if (this.inReplyToCommentId === null) { diff --git a/server/tests/api/videos/multiple-servers.ts b/server/tests/api/videos/multiple-servers.ts index aeda188c2..e7b57ba1f 100644 --- a/server/tests/api/videos/multiple-servers.ts +++ b/server/tests/api/videos/multiple-servers.ts @@ -868,7 +868,7 @@ describe('Test multiple servers', function () { await waitJobs(servers) }) - it('Should not have this comment anymore', async function () { + it('Should have this comment marked as deleted', async function () { for (const server of servers) { const res1 = await getVideoCommentThreads(server.url, videoUUID, 0, 5) const threadId = res1.body.data.find(c => c.text === 'my super first comment').id @@ -880,7 +880,13 @@ describe('Test multiple servers', function () { const firstChild = tree.children[0] expect(firstChild.comment.text).to.equal('my super answer to thread 1') - expect(firstChild.children).to.have.lengthOf(0) + expect(firstChild.children).to.have.lengthOf(1) + + const deletedComment = firstChild.children[0].comment + expect(deletedComment.isDeleted).to.be.true + expect(deletedComment.deletedAt).to.not.be.null + expect(deletedComment.account).to.be.null + expect(deletedComment.text).to.equal('') const secondChild = tree.children[1] expect(secondChild.comment.text).to.equal('my second answer to thread 1') @@ -897,13 +903,13 @@ describe('Test multiple servers', function () { await waitJobs(servers) }) - it('Should have the threads deleted on other servers too', async function () { + it('Should have the threads marked as deleted on other servers too', async function () { for (const server of servers) { const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5) - expect(res.body.total).to.equal(1) + expect(res.body.total).to.equal(2) expect(res.body.data).to.be.an('array') - expect(res.body.data).to.have.lengthOf(1) + expect(res.body.data).to.have.lengthOf(2) { const comment: VideoComment = res.body.data[0] @@ -915,6 +921,20 @@ describe('Test multiple servers', function () { expect(dateIsValid(comment.createdAt as string)).to.be.true expect(dateIsValid(comment.updatedAt as string)).to.be.true } + + { + const deletedComment: VideoComment = res.body.data[1] + expect(deletedComment).to.not.be.undefined + expect(deletedComment.isDeleted).to.be.true + expect(deletedComment.deletedAt).to.not.be.null + expect(deletedComment.text).to.equal('') + expect(deletedComment.inReplyToCommentId).to.be.null + expect(deletedComment.account).to.be.null + expect(deletedComment.totalReplies).to.equal(3) + expect(dateIsValid(deletedComment.createdAt as string)).to.be.true + expect(dateIsValid(deletedComment.updatedAt as string)).to.be.true + expect(dateIsValid(deletedComment.deletedAt as string)).to.be.true + } } }) @@ -926,12 +946,32 @@ describe('Test multiple servers', function () { await waitJobs(servers) }) - it('Should have the threads deleted on other servers too', async function () { + it('Should have the threads marked as deleted on other servers too', async function () { for (const server of servers) { const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5) - expect(res.body.total).to.equal(0) - expect(res.body.data).to.have.lengthOf(0) + expect(res.body.total).to.equal(2) + expect(res.body.data).to.have.lengthOf(2) + + { + const comment: VideoComment = res.body.data[0] + expect(comment.text).to.equal('') + expect(comment.isDeleted).to.be.true + expect(comment.createdAt).to.not.be.null + expect(comment.deletedAt).to.not.be.null + expect(comment.account).to.be.null + expect(comment.totalReplies).to.equal(0) + } + + { + const comment: VideoComment = res.body.data[1] + expect(comment.text).to.equal('') + expect(comment.isDeleted).to.be.true + expect(comment.createdAt).to.not.be.null + expect(comment.deletedAt).to.not.be.null + expect(comment.account).to.be.null + expect(comment.totalReplies).to.equal(3) + } } }) diff --git a/server/tests/api/videos/video-comments.ts b/server/tests/api/videos/video-comments.ts index 82182cc7c..95be14c0e 100644 --- a/server/tests/api/videos/video-comments.ts +++ b/server/tests/api/videos/video-comments.ts @@ -172,7 +172,7 @@ describe('Test video comments', function () { const tree: VideoCommentThreadTree = res.body expect(tree.comment.text).equal('my super first comment') - expect(tree.children).to.have.lengthOf(1) + expect(tree.children).to.have.lengthOf(2) const firstChild = tree.children[0] expect(firstChild.comment.text).to.equal('my super answer to thread 1') @@ -181,20 +181,32 @@ describe('Test video comments', function () { const childOfFirstChild = firstChild.children[0] expect(childOfFirstChild.comment.text).to.equal('my super answer to answer of thread 1') expect(childOfFirstChild.children).to.have.lengthOf(0) + + const deletedChildOfFirstChild = tree.children[1] + expect(deletedChildOfFirstChild.comment.text).to.equal('') + expect(deletedChildOfFirstChild.comment.isDeleted).to.be.true + expect(deletedChildOfFirstChild.comment.deletedAt).to.not.be.null + expect(deletedChildOfFirstChild.comment.account).to.be.null + expect(deletedChildOfFirstChild.children).to.have.lengthOf(0) }) it('Should delete a complete thread', async function () { await deleteVideoComment(server.url, server.accessToken, videoId, threadId) const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5, 'createdAt') - expect(res.body.total).to.equal(2) + expect(res.body.total).to.equal(3) expect(res.body.data).to.be.an('array') - expect(res.body.data).to.have.lengthOf(2) + expect(res.body.data).to.have.lengthOf(3) - expect(res.body.data[0].text).to.equal('super thread 2') - expect(res.body.data[0].totalReplies).to.equal(0) - expect(res.body.data[1].text).to.equal('super thread 3') + expect(res.body.data[0].text).to.equal('') + expect(res.body.data[0].isDeleted).to.be.true + expect(res.body.data[0].deletedAt).to.not.be.null + expect(res.body.data[0].account).to.be.null + expect(res.body.data[0].totalReplies).to.equal(3) + expect(res.body.data[1].text).to.equal('super thread 2') expect(res.body.data[1].totalReplies).to.equal(0) + expect(res.body.data[2].text).to.equal('super thread 3') + expect(res.body.data[2].totalReplies).to.equal(0) }) after(async function () { -- cgit v1.2.3