From 9d6b9d10ef8cbef39e89bc709285abffb0d8caa1 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 19 Feb 2021 09:50:13 +0100 Subject: [PATCH] Fix video comments display with deleted comments --- .../comment/video-comment.component.html | 2 +- .../comment/video-comment.component.ts | 9 ++- .../comment/video-comments.component.html | 8 +-- .../comment/video-comments.component.ts | 10 +++- .../video-comment-thread-tree.model.ts | 1 + .../video-comment.service.ts | 25 +++++--- server/controllers/api/videos/comment.ts | 13 +++- server/models/utils.ts | 2 +- server/models/video/video-comment.ts | 60 ++++++++++++------- server/tests/api/videos/video-comments.ts | 48 +++++++++------ shared/models/result-list.model.ts | 4 ++ 11 files changed, 120 insertions(+), 62 deletions(-) diff --git a/client/src/app/+videos/+video-watch/comment/video-comment.component.html b/client/src/app/+videos/+video-watch/comment/video-comment.component.html index 8847753a5..ba41b6f48 100644 --- a/client/src/app/+videos/+video-watch/comment/video-comment.component.html +++ b/client/src/app/+videos/+video-watch/comment/video-comment.component.html @@ -1,4 +1,4 @@ -
+

- + - 1 Comment - {{ componentPagination.totalItems }} Comments + 1 Comment + {{ totalNotDeletedComments }} Comments Comments

@@ -30,7 +30,7 @@ [textValue]="commentThreadRedraftValue" > -
No comments.
+
No comments.
{ this.comments = this.comments.concat(res.data) - // Client does not display removed comments - this.componentPagination.totalItems = res.total - this.comments.filter(c => c.isDeleted).length + this.componentPagination.totalItems = res.total + this.totalNotDeletedComments = res.totalNotDeletedComments this.onDataSubject.next(res.data) this.hooks.runAction('action:video-watch.video-threads.loaded', 'video-watch', { data: this.componentPagination }) @@ -241,6 +246,7 @@ export class VideoCommentsComponent implements OnInit, OnChanges, OnDestroy { this.inReplyToCommentId = undefined this.componentPagination.currentPage = 1 this.componentPagination.totalItems = null + this.totalNotDeletedComments = null this.syndicationItems = this.videoCommentService.getVideoCommentsFeeds(this.video.uuid) this.loadMoreThreads() diff --git a/client/src/app/shared/shared-video-comment/video-comment-thread-tree.model.ts b/client/src/app/shared/shared-video-comment/video-comment-thread-tree.model.ts index 7c2aaeadd..9956c88a6 100644 --- a/client/src/app/shared/shared-video-comment/video-comment-thread-tree.model.ts +++ b/client/src/app/shared/shared-video-comment/video-comment-thread-tree.model.ts @@ -3,5 +3,6 @@ import { VideoComment } from './video-comment.model' export class VideoCommentThreadTree implements VideoCommentThreadTreeServerModel { comment: VideoComment + hasDisplayedChildren: boolean children: VideoCommentThreadTree[] } diff --git a/client/src/app/shared/shared-video-comment/video-comment.service.ts b/client/src/app/shared/shared-video-comment/video-comment.service.ts index c107a33ab..0f09778df 100644 --- a/client/src/app/shared/shared-video-comment/video-comment.service.ts +++ b/client/src/app/shared/shared-video-comment/video-comment.service.ts @@ -8,6 +8,7 @@ import { objectLineFeedToHtml } from '@app/helpers' import { FeedFormat, ResultList, + ThreadsResultList, VideoComment as VideoCommentServerModel, VideoCommentAdmin, VideoCommentCreate, @@ -76,7 +77,7 @@ export class VideoCommentService { videoId: number | string, componentPagination: ComponentPaginationLight, sort: string - }): Observable> { + }): Observable> { const { videoId, componentPagination, sort } = parameters const pagination = this.restService.componentPaginationToRestPagination(componentPagination) @@ -85,7 +86,7 @@ export class VideoCommentService { params = this.restService.addRestGetParams(params, pagination, sort) const url = VideoCommentService.BASE_VIDEO_URL + videoId + '/comment-threads' - return this.authHttp.get>(url, { params }) + return this.authHttp.get>(url, { params }) .pipe( map(result => this.extractVideoComments(result)), catchError(err => this.restExtractor.handleError(err)) @@ -158,7 +159,7 @@ export class VideoCommentService { return new VideoComment(videoComment) } - private extractVideoComments (result: ResultList) { + private extractVideoComments (result: ThreadsResultList) { const videoCommentsJson = result.data const totalComments = result.total const comments: VideoComment[] = [] @@ -167,16 +168,22 @@ export class VideoCommentService { comments.push(new VideoComment(videoCommentJson)) } - return { data: comments, total: totalComments } + return { data: comments, total: totalComments, totalNotDeletedComments: result.totalNotDeletedComments } } - private extractVideoCommentTree (tree: VideoCommentThreadTreeServerModel) { - if (!tree) return tree as VideoCommentThreadTree + private extractVideoCommentTree (serverTree: VideoCommentThreadTreeServerModel): VideoCommentThreadTree { + if (!serverTree) return null - tree.comment = new VideoComment(tree.comment) - tree.children.forEach(c => this.extractVideoCommentTree(c)) + const tree = { + comment: new VideoComment(serverTree.comment), + children: serverTree.children.map(c => this.extractVideoCommentTree(c)) + } + + const hasDisplayedChildren = tree.children.length === 0 + ? !tree.comment.isDeleted + : tree.children.some(c => c.hasDisplayedChildren) - return tree as VideoCommentThreadTree + return Object.assign(tree, { hasDisplayedChildren }) } private buildParamsFromSearch (search: string, params: HttpParams) { diff --git a/server/controllers/api/videos/comment.ts b/server/controllers/api/videos/comment.ts index 752a33596..b21698525 100644 --- a/server/controllers/api/videos/comment.ts +++ b/server/controllers/api/videos/comment.ts @@ -1,5 +1,5 @@ import * as express from 'express' -import { ResultList, UserRight } from '../../../../shared/models' +import { ResultList, ThreadsResultList, UserRight } from '../../../../shared/models' import { VideoCommentCreate } from '../../../../shared/models/videos/video-comment.model' import { auditLoggerFactory, CommentAuditView, getAuditIdFromRes } from '../../../helpers/audit-logger' import { getFormattedObjects } from '../../../helpers/utils' @@ -30,6 +30,7 @@ import { import { AccountModel } from '../../../models/account/account' import { VideoCommentModel } from '../../../models/video/video-comment' import { HttpStatusCode } from '../../../../shared/core-utils/miscs/http-error-codes' +import { logger } from '@server/helpers/logger' const auditLogger = auditLoggerFactory('comments') const videoCommentRouter = express.Router() @@ -108,7 +109,7 @@ async function listVideoThreads (req: express.Request, res: express.Response) { const video = res.locals.onlyVideo const user = res.locals.oauth ? res.locals.oauth.token.User : undefined - let resultList: ResultList + let resultList: ThreadsResultList if (video.commentsEnabled === true) { const apiOptions = await Hooks.wrapObject({ @@ -128,11 +129,15 @@ async function listVideoThreads (req: express.Request, res: express.Response) { } else { resultList = { total: 0, + totalNotDeletedComments: 0, data: [] } } - return res.json(getFormattedObjects(resultList.data, resultList.total)) + return res.json({ + ...getFormattedObjects(resultList.data, resultList.total), + totalNotDeletedComments: resultList.totalNotDeletedComments + }) } async function listVideoThreadComments (req: express.Request, res: express.Response) { @@ -161,6 +166,8 @@ async function listVideoThreadComments (req: express.Request, res: express.Respo } } + logger.info('coucou', { resultList }) + if (resultList.data.length === 0) { return res.sendStatus(HttpStatusCode.NOT_FOUND_404) } diff --git a/server/models/utils.ts b/server/models/utils.ts index 143c1a23c..5337ae75d 100644 --- a/server/models/utils.ts +++ b/server/models/utils.ts @@ -134,7 +134,7 @@ function buildBlockedAccountSQL (blockerIds: number[]) { const blockerIdsString = blockerIds.join(', ') return 'SELECT "targetAccountId" AS "id" FROM "accountBlocklist" WHERE "accountId" IN (' + blockerIdsString + ')' + - ' UNION ALL ' + + ' UNION ' + 'SELECT "account"."id" AS "id" FROM account INNER JOIN "actor" ON account."actorId" = actor.id ' + 'INNER JOIN "serverBlocklist" ON "actor"."serverId" = "serverBlocklist"."targetServerId" ' + 'WHERE "serverBlocklist"."accountId" IN (' + blockerIdsString + ')' diff --git a/server/models/video/video-comment.ts b/server/models/video/video-comment.ts index 8d1c38826..cfd1d5b7a 100644 --- a/server/models/video/video-comment.ts +++ b/server/models/video/video-comment.ts @@ -414,7 +414,15 @@ export class VideoCommentModel extends Model { const blockerAccountIds = await VideoCommentModel.buildBlockerAccountIds({ videoId, user, isVideoOwned }) - const query = { + const accountBlockedWhere = { + accountId: { + [Op.notIn]: Sequelize.literal( + '(' + buildBlockedAccountSQL(blockerAccountIds) + ')' + ) + } + } + + const queryList = { offset: start, limit: count, order: getCommentSort(sort), @@ -428,13 +436,7 @@ export class VideoCommentModel extends Model { }, { [Op.or]: [ - { - accountId: { - [Op.notIn]: Sequelize.literal( - '(' + buildBlockedAccountSQL(blockerAccountIds) + ')' - ) - } - }, + accountBlockedWhere, { accountId: null } @@ -444,19 +446,27 @@ export class VideoCommentModel extends Model { } } - const scopes: (string | ScopeOptions)[] = [ + const scopesList: (string | ScopeOptions)[] = [ ScopeNames.WITH_ACCOUNT_FOR_API, { method: [ ScopeNames.ATTRIBUTES_FOR_API, blockerAccountIds ] } ] - return VideoCommentModel - .scope(scopes) - .findAndCountAll(query) - .then(({ rows, count }) => { - return { total: count, data: rows } - }) + const queryCount = { + where: { + videoId, + deletedAt: null, + ...accountBlockedWhere + } + } + + return Promise.all([ + VideoCommentModel.scope(scopesList).findAndCountAll(queryList), + VideoCommentModel.count(queryCount) + ]).then(([ { rows, count }, totalNotDeletedComments ]) => { + return { total: count, data: rows, totalNotDeletedComments } + }) } static async listThreadCommentsForApi (parameters: { @@ -477,11 +487,18 @@ export class VideoCommentModel extends Model { { id: threadId }, { originCommentId: threadId } ], - accountId: { - [Op.notIn]: Sequelize.literal( - '(' + buildBlockedAccountSQL(blockerAccountIds) + ')' - ) - } + [Op.or]: [ + { + accountId: { + [Op.notIn]: Sequelize.literal( + '(' + buildBlockedAccountSQL(blockerAccountIds) + ')' + ) + } + }, + { + accountId: null + } + ] } } @@ -492,8 +509,7 @@ export class VideoCommentModel extends Model { } ] - return VideoCommentModel - .scope(scopes) + return VideoCommentModel.scope(scopes) .findAndCountAll(query) .then(({ rows, count }) => { return { total: count, data: rows } diff --git a/server/tests/api/videos/video-comments.ts b/server/tests/api/videos/video-comments.ts index 141a80690..615e0ea45 100644 --- a/server/tests/api/videos/video-comments.ts +++ b/server/tests/api/videos/video-comments.ts @@ -67,6 +67,7 @@ describe('Test video comments', function () { const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5) expect(res.body.total).to.equal(0) + expect(res.body.totalNotDeletedComments).to.equal(0) expect(res.body.data).to.be.an('array') expect(res.body.data).to.have.lengthOf(0) }) @@ -94,6 +95,7 @@ describe('Test video comments', function () { const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5) expect(res.body.total).to.equal(1) + expect(res.body.totalNotDeletedComments).to.equal(1) expect(res.body.data).to.be.an('array') expect(res.body.data).to.have.lengthOf(1) @@ -172,6 +174,7 @@ describe('Test video comments', function () { const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5, 'createdAt') expect(res.body.total).to.equal(3) + expect(res.body.totalNotDeletedComments).to.equal(6) expect(res.body.data).to.be.an('array') expect(res.body.data).to.have.lengthOf(3) @@ -186,26 +189,35 @@ describe('Test video comments', function () { it('Should delete a reply', async function () { await deleteVideoComment(server.url, server.accessToken, videoId, replyToDeleteId) - const res = await getVideoThreadComments(server.url, videoUUID, threadId) - - const tree: VideoCommentThreadTree = res.body - expect(tree.comment.text).equal('my super first comment') - expect(tree.children).to.have.lengthOf(2) - - const firstChild = tree.children[0] - expect(firstChild.comment.text).to.equal('my super answer to thread 1') - expect(firstChild.children).to.have.lengthOf(1) + { + const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5, 'createdAt') - 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) + expect(res.body.total).to.equal(3) + expect(res.body.totalNotDeletedComments).to.equal(5) + } - 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) + { + const res = await getVideoThreadComments(server.url, videoUUID, threadId) + + const tree: VideoCommentThreadTree = res.body + expect(tree.comment.text).equal('my super first comment') + expect(tree.children).to.have.lengthOf(2) + + const firstChild = tree.children[0] + expect(firstChild.comment.text).to.equal('my super answer to thread 1') + expect(firstChild.children).to.have.lengthOf(1) + + 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 () { diff --git a/shared/models/result-list.model.ts b/shared/models/result-list.model.ts index 2d5147a86..fcafcfb2f 100644 --- a/shared/models/result-list.model.ts +++ b/shared/models/result-list.model.ts @@ -2,3 +2,7 @@ export interface ResultList { total: number data: T[] } + +export interface ThreadsResultList extends ResultList { + totalNotDeletedComments: number +} -- 2.41.0