Soft delete video comments instead of detroy

client/src/app/videos/+video-watch/comment/video-comment.component.html

@@ -1,7 +1,18 @@
 <div class="root-comment">
-  <img [src]="comment.accountAvatarUrl" alt="Avatar" />
+  <img
+    *ngIf="!comment.isDeleted"
+    class="comment-avatar"
+    [src]="comment.accountAvatarUrl"
+    alt="Avatar"
+  />
+
+  <span
+    *ngIf="comment.isDeleted"
+    class="comment-avatar"
+  ></span>
 
   <div class="comment">
+    <ng-container *ngIf="!comment.isDeleted">
       <div *ngIf="highlightedComment === true" class="highlighted-comment" i18n>Highlighted comment</div>
 
       <div class="comment-account-date">
@@ -14,9 +25,21 @@
         <div *ngIf="isUserLoggedIn()" (click)="onWantToReply()" class="comment-action-reply" i18n>Reply</div>
         <div *ngIf="isRemovableByUser()" (click)="onWantToDelete()" class="comment-action-delete" i18n>Delete</div>
       </div>
+    </ng-container>
+
+    <ng-container *ngIf="comment.isDeleted">
+      <div class="comment-account-date">
+        <span class="comment-account" i18n>Deleted</span>
+        <a [routerLink]="['/videos/watch', video.uuid, { 'threadId': comment.threadId }]" class="comment-date">{{ comment.createdAt | myFromNow }}</a>
+      </div>
+
+      <div *ngIf="comment.isDeleted" class="comment-html comment-html-deleted">
+        <i i18n>This comment has been deleted</i>
+      </div>
+    </ng-container>
 
     <my-video-comment-add
-      *ngIf="isUserLoggedIn() && inReplyToCommentId === comment.id"
+      *ngIf="!comment.isDeleted && isUserLoggedIn() && inReplyToCommentId === comment.id"
       [user]="user"
       [video]="video"
       [parentComment]="comment"

client/src/app/videos/+video-watch/comment/video-comment.component.scss

@@ -5,7 +5,7 @@
   font-size: 15px;
   display: flex;
 
-  img {
+  .comment-avatar {
     @include avatar(36px);
 
     margin-top: 5px;
@@ -48,6 +48,7 @@
 
     .comment-html {
       @include peertube-word-wrap;
+      margin-bottom: 10px;
 
       // Mentions
       ::ng-deep a {
@@ -61,10 +62,14 @@
         }
 
       }
+
+      &.comment-html-deleted {
+        color: $grey-foreground-color;
+      }
     }
 
     .comment-actions {
-      margin: 10px 0;
+      margin-bottom: 10px;
       display: flex;
 
       .comment-action-reply,
@@ -100,7 +105,7 @@
   }
 
   .root-comment {
-    img { margin-right: 10px; }
+    .comment-avatar { margin-right: 10px; }
   }
 }
 

client/src/app/videos/+video-watch/comment/video-comment.component.ts

@@ -78,7 +78,7 @@ export class VideoCommentComponent implements OnInit, OnChanges {
   }
 
   isRemovableByUser () {
-    return this.isUserLoggedIn() &&
+    return this.comment.account && this.isUserLoggedIn() &&
       (
         this.user.account.id === this.comment.account.id ||
         this.user.hasRight(UserRight.REMOVE_ANY_VIDEO_COMMENT)

client/src/app/videos/+video-watch/comment/video-comment.model.ts

@@ -12,6 +12,8 @@ export class VideoComment implements VideoCommentServerModel {
   videoId: number
   createdAt: Date | string
   updatedAt: Date | string
+  deletedAt: Date | string
+  isDeleted: boolean
   account: AccountInterface
   totalReplies: number
   by: string
@@ -28,9 +30,12 @@ export class VideoComment implements VideoCommentServerModel {
     this.videoId = hash.videoId
     this.createdAt = new Date(hash.createdAt.toString())
     this.updatedAt = new Date(hash.updatedAt.toString())
+    this.deletedAt = hash.deletedAt ? new Date(hash.deletedAt.toString()) : null
+    this.isDeleted = hash.isDeleted
     this.account = hash.account
     this.totalReplies = hash.totalReplies
 
+    if (this.account) {
       this.by = Actor.CREATE_BY_STRING(this.account.name, this.account.host)
       this.accountAvatarUrl = Actor.GET_ACTOR_AVATAR_URL(this.account)
 
@@ -39,3 +44,4 @@ export class VideoComment implements VideoCommentServerModel {
       this.isLocal = this.account.host.trim() === thisHost
     }
   }
+}

client/src/app/videos/+video-watch/comment/video-comments.component.ts

@@ -153,10 +153,6 @@ export class VideoCommentsComponent implements OnInit, OnChanges, OnDestroy {
   async onWantedToDelete (commentToDelete: VideoComment) {
     let message = 'Do you really want to delete this comment?'
 
-    if (commentToDelete.totalReplies !== 0) {
-      message += this.i18n(' {{totalReplies}} replies will be deleted too.', { totalReplies: commentToDelete.totalReplies })
-    }
-
     if (commentToDelete.isLocal) {
       message += this.i18n(' The deletion will be sent to remote instances, so they remove the comment too.')
     } else {
@@ -169,21 +165,8 @@ export class VideoCommentsComponent implements OnInit, OnChanges, OnDestroy {
     this.videoCommentService.deleteVideoComment(commentToDelete.videoId, commentToDelete.id)
       .subscribe(
         () => {
-          // Delete the comment in the tree
-          if (commentToDelete.inReplyToCommentId) {
-            const thread = this.threadComments[commentToDelete.threadId]
-            if (!thread) {
-              console.error(`Cannot find thread ${commentToDelete.threadId} of the comment to delete ${commentToDelete.id}`)
-              return
-            }
-
-            this.deleteLocalCommentThread(thread, commentToDelete)
-            return
-          }
-
-          // Delete the thread
-          this.comments = this.comments.filter(c => c.id !== commentToDelete.id)
-          this.componentPagination.totalItems--
+          // Mark the comment as deleted
+          this.softDeleteComment(commentToDelete)
 
           if (this.highlightedThread.id === commentToDelete.id) this.highlightedThread = undefined
         },
@@ -204,15 +187,11 @@ export class VideoCommentsComponent implements OnInit, OnChanges, OnDestroy {
     }
   }
 
-  private deleteLocalCommentThread (parentComment: VideoCommentThreadTree, commentToDelete: VideoComment) {
-    for (const commentChild of parentComment.children) {
-      if (commentChild.comment.id === commentToDelete.id) {
-        parentComment.children = parentComment.children.filter(c => c.comment.id !== commentToDelete.id)
-        return
-      }
-
-      this.deleteLocalCommentThread(commentChild, commentToDelete)
-    }
+  private softDeleteComment (comment: VideoComment) {
+    comment.isDeleted = true
+    comment.deletedAt = new Date()
+    comment.text = ''
+    comment.account = null
   }
 
   private resetVideo () {

server/controllers/activitypub/client.ts

@@ -308,14 +308,17 @@ 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)
     }
+  }
 
   return activityPubResponse(activityPubContextify(videoCommentObject), res)
 }

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()
 }

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<void> {
+  {
+    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
+}

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

server/lib/video-comment.ts

@@ -73,9 +73,16 @@ function buildFormattedCommentTree (resultList: ResultList<VideoCommentModel>):
   return thread
 }
 
+function markCommentAsDeleted (comment: MCommentOwnerVideoReply): void {
+  comment.text = ''
+  comment.deletedAt = new Date()
+  comment.accountId = null
+}
+
 // ---------------------------------------------------------------------------
 
 export {
   createVideoComment,
-  buildFormattedCommentTree
+  buildFormattedCommentTree,
+  markCommentAsDeleted
 }

server/models/account/account.ts

@@ -201,7 +201,7 @@ export class AccountModel extends Model<AccountModel> {
 
   @HasMany(() => VideoCommentModel, {
     foreignKey: {
-      allowNull: false
+      allowNull: true
     },
     onDelete: 'cascade',
     hooks: true

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<VideoCommentModel> {
   @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<VideoCommentModel> {
 
   @BelongsTo(() => AccountModel, {
     foreignKey: {
-      allowNull: false
+      allowNull: true
     },
     onDelete: 'CASCADE'
   })
@@ -436,9 +440,17 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
   }
 
   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<VideoCommentModel> {
       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) {

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)
+        }
       }
     })
 

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 () {

shared/models/activitypub/objects/common-objects.ts

@@ -89,3 +89,14 @@ export interface ActivityPubAttributedTo {
   type: 'Group' | 'Person'
   id: string
 }
+
+export interface ActivityTombstoneObject {
+  '@context'?: any
+  id: string
+  type: 'Tombstone'
+  name?: string
+  formerType?: string
+  published: string
+  updated: string
+  deleted: string
+}

shared/models/videos/video-comment.model.ts

@@ -9,6 +9,8 @@ export interface VideoComment {
   videoId: number
   createdAt: Date | string
   updatedAt: Date | string
+  deletedAt: Date | string
+  isDeleted: boolean
   totalReplies: number
   account: Account
 }