diff options
author | Chocobozzz <me@florianbigard.com> | 2021-02-19 09:50:13 +0100 |
---|---|---|
committer | Chocobozzz <me@florianbigard.com> | 2021-02-19 10:06:52 +0100 |
commit | 9d6b9d10ef8cbef39e89bc709285abffb0d8caa1 (patch) | |
tree | 3425b22556e00d1b15de15c72b2802cfc9374473 /server | |
parent | fae6e4da8f516a9d6c3bad9bf6f35811ccacbad8 (diff) | |
download | PeerTube-9d6b9d10ef8cbef39e89bc709285abffb0d8caa1.tar.gz PeerTube-9d6b9d10ef8cbef39e89bc709285abffb0d8caa1.tar.zst PeerTube-9d6b9d10ef8cbef39e89bc709285abffb0d8caa1.zip |
Fix video comments display with deleted comments
Diffstat (limited to 'server')
-rw-r--r-- | server/controllers/api/videos/comment.ts | 13 | ||||
-rw-r--r-- | server/models/utils.ts | 2 | ||||
-rw-r--r-- | server/models/video/video-comment.ts | 60 | ||||
-rw-r--r-- | server/tests/api/videos/video-comments.ts | 48 |
4 files changed, 79 insertions, 44 deletions
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 @@ | |||
1 | import * as express from 'express' | 1 | import * as express from 'express' |
2 | import { ResultList, UserRight } from '../../../../shared/models' | 2 | import { ResultList, ThreadsResultList, UserRight } from '../../../../shared/models' |
3 | import { VideoCommentCreate } from '../../../../shared/models/videos/video-comment.model' | 3 | import { VideoCommentCreate } from '../../../../shared/models/videos/video-comment.model' |
4 | import { auditLoggerFactory, CommentAuditView, getAuditIdFromRes } from '../../../helpers/audit-logger' | 4 | import { auditLoggerFactory, CommentAuditView, getAuditIdFromRes } from '../../../helpers/audit-logger' |
5 | import { getFormattedObjects } from '../../../helpers/utils' | 5 | import { getFormattedObjects } from '../../../helpers/utils' |
@@ -30,6 +30,7 @@ import { | |||
30 | import { AccountModel } from '../../../models/account/account' | 30 | import { AccountModel } from '../../../models/account/account' |
31 | import { VideoCommentModel } from '../../../models/video/video-comment' | 31 | import { VideoCommentModel } from '../../../models/video/video-comment' |
32 | import { HttpStatusCode } from '../../../../shared/core-utils/miscs/http-error-codes' | 32 | import { HttpStatusCode } from '../../../../shared/core-utils/miscs/http-error-codes' |
33 | import { logger } from '@server/helpers/logger' | ||
33 | 34 | ||
34 | const auditLogger = auditLoggerFactory('comments') | 35 | const auditLogger = auditLoggerFactory('comments') |
35 | const videoCommentRouter = express.Router() | 36 | const videoCommentRouter = express.Router() |
@@ -108,7 +109,7 @@ async function listVideoThreads (req: express.Request, res: express.Response) { | |||
108 | const video = res.locals.onlyVideo | 109 | const video = res.locals.onlyVideo |
109 | const user = res.locals.oauth ? res.locals.oauth.token.User : undefined | 110 | const user = res.locals.oauth ? res.locals.oauth.token.User : undefined |
110 | 111 | ||
111 | let resultList: ResultList<VideoCommentModel> | 112 | let resultList: ThreadsResultList<VideoCommentModel> |
112 | 113 | ||
113 | if (video.commentsEnabled === true) { | 114 | if (video.commentsEnabled === true) { |
114 | const apiOptions = await Hooks.wrapObject({ | 115 | const apiOptions = await Hooks.wrapObject({ |
@@ -128,11 +129,15 @@ async function listVideoThreads (req: express.Request, res: express.Response) { | |||
128 | } else { | 129 | } else { |
129 | resultList = { | 130 | resultList = { |
130 | total: 0, | 131 | total: 0, |
132 | totalNotDeletedComments: 0, | ||
131 | data: [] | 133 | data: [] |
132 | } | 134 | } |
133 | } | 135 | } |
134 | 136 | ||
135 | return res.json(getFormattedObjects(resultList.data, resultList.total)) | 137 | return res.json({ |
138 | ...getFormattedObjects(resultList.data, resultList.total), | ||
139 | totalNotDeletedComments: resultList.totalNotDeletedComments | ||
140 | }) | ||
136 | } | 141 | } |
137 | 142 | ||
138 | async function listVideoThreadComments (req: express.Request, res: express.Response) { | 143 | async function listVideoThreadComments (req: express.Request, res: express.Response) { |
@@ -161,6 +166,8 @@ async function listVideoThreadComments (req: express.Request, res: express.Respo | |||
161 | } | 166 | } |
162 | } | 167 | } |
163 | 168 | ||
169 | logger.info('coucou', { resultList }) | ||
170 | |||
164 | if (resultList.data.length === 0) { | 171 | if (resultList.data.length === 0) { |
165 | return res.sendStatus(HttpStatusCode.NOT_FOUND_404) | 172 | return res.sendStatus(HttpStatusCode.NOT_FOUND_404) |
166 | } | 173 | } |
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[]) { | |||
134 | const blockerIdsString = blockerIds.join(', ') | 134 | const blockerIdsString = blockerIds.join(', ') |
135 | 135 | ||
136 | return 'SELECT "targetAccountId" AS "id" FROM "accountBlocklist" WHERE "accountId" IN (' + blockerIdsString + ')' + | 136 | return 'SELECT "targetAccountId" AS "id" FROM "accountBlocklist" WHERE "accountId" IN (' + blockerIdsString + ')' + |
137 | ' UNION ALL ' + | 137 | ' UNION ' + |
138 | 'SELECT "account"."id" AS "id" FROM account INNER JOIN "actor" ON account."actorId" = actor.id ' + | 138 | 'SELECT "account"."id" AS "id" FROM account INNER JOIN "actor" ON account."actorId" = actor.id ' + |
139 | 'INNER JOIN "serverBlocklist" ON "actor"."serverId" = "serverBlocklist"."targetServerId" ' + | 139 | 'INNER JOIN "serverBlocklist" ON "actor"."serverId" = "serverBlocklist"."targetServerId" ' + |
140 | 'WHERE "serverBlocklist"."accountId" IN (' + blockerIdsString + ')' | 140 | '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 { | |||
414 | 414 | ||
415 | const blockerAccountIds = await VideoCommentModel.buildBlockerAccountIds({ videoId, user, isVideoOwned }) | 415 | const blockerAccountIds = await VideoCommentModel.buildBlockerAccountIds({ videoId, user, isVideoOwned }) |
416 | 416 | ||
417 | const query = { | 417 | const accountBlockedWhere = { |
418 | accountId: { | ||
419 | [Op.notIn]: Sequelize.literal( | ||
420 | '(' + buildBlockedAccountSQL(blockerAccountIds) + ')' | ||
421 | ) | ||
422 | } | ||
423 | } | ||
424 | |||
425 | const queryList = { | ||
418 | offset: start, | 426 | offset: start, |
419 | limit: count, | 427 | limit: count, |
420 | order: getCommentSort(sort), | 428 | order: getCommentSort(sort), |
@@ -428,13 +436,7 @@ export class VideoCommentModel extends Model { | |||
428 | }, | 436 | }, |
429 | { | 437 | { |
430 | [Op.or]: [ | 438 | [Op.or]: [ |
431 | { | 439 | accountBlockedWhere, |
432 | accountId: { | ||
433 | [Op.notIn]: Sequelize.literal( | ||
434 | '(' + buildBlockedAccountSQL(blockerAccountIds) + ')' | ||
435 | ) | ||
436 | } | ||
437 | }, | ||
438 | { | 440 | { |
439 | accountId: null | 441 | accountId: null |
440 | } | 442 | } |
@@ -444,19 +446,27 @@ export class VideoCommentModel extends Model { | |||
444 | } | 446 | } |
445 | } | 447 | } |
446 | 448 | ||
447 | const scopes: (string | ScopeOptions)[] = [ | 449 | const scopesList: (string | ScopeOptions)[] = [ |
448 | ScopeNames.WITH_ACCOUNT_FOR_API, | 450 | ScopeNames.WITH_ACCOUNT_FOR_API, |
449 | { | 451 | { |
450 | method: [ ScopeNames.ATTRIBUTES_FOR_API, blockerAccountIds ] | 452 | method: [ ScopeNames.ATTRIBUTES_FOR_API, blockerAccountIds ] |
451 | } | 453 | } |
452 | ] | 454 | ] |
453 | 455 | ||
454 | return VideoCommentModel | 456 | const queryCount = { |
455 | .scope(scopes) | 457 | where: { |
456 | .findAndCountAll(query) | 458 | videoId, |
457 | .then(({ rows, count }) => { | 459 | deletedAt: null, |
458 | return { total: count, data: rows } | 460 | ...accountBlockedWhere |
459 | }) | 461 | } |
462 | } | ||
463 | |||
464 | return Promise.all([ | ||
465 | VideoCommentModel.scope(scopesList).findAndCountAll(queryList), | ||
466 | VideoCommentModel.count(queryCount) | ||
467 | ]).then(([ { rows, count }, totalNotDeletedComments ]) => { | ||
468 | return { total: count, data: rows, totalNotDeletedComments } | ||
469 | }) | ||
460 | } | 470 | } |
461 | 471 | ||
462 | static async listThreadCommentsForApi (parameters: { | 472 | static async listThreadCommentsForApi (parameters: { |
@@ -477,11 +487,18 @@ export class VideoCommentModel extends Model { | |||
477 | { id: threadId }, | 487 | { id: threadId }, |
478 | { originCommentId: threadId } | 488 | { originCommentId: threadId } |
479 | ], | 489 | ], |
480 | accountId: { | 490 | [Op.or]: [ |
481 | [Op.notIn]: Sequelize.literal( | 491 | { |
482 | '(' + buildBlockedAccountSQL(blockerAccountIds) + ')' | 492 | accountId: { |
483 | ) | 493 | [Op.notIn]: Sequelize.literal( |
484 | } | 494 | '(' + buildBlockedAccountSQL(blockerAccountIds) + ')' |
495 | ) | ||
496 | } | ||
497 | }, | ||
498 | { | ||
499 | accountId: null | ||
500 | } | ||
501 | ] | ||
485 | } | 502 | } |
486 | } | 503 | } |
487 | 504 | ||
@@ -492,8 +509,7 @@ export class VideoCommentModel extends Model { | |||
492 | } | 509 | } |
493 | ] | 510 | ] |
494 | 511 | ||
495 | return VideoCommentModel | 512 | return VideoCommentModel.scope(scopes) |
496 | .scope(scopes) | ||
497 | .findAndCountAll(query) | 513 | .findAndCountAll(query) |
498 | .then(({ rows, count }) => { | 514 | .then(({ rows, count }) => { |
499 | return { total: count, data: rows } | 515 | 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 () { | |||
67 | const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5) | 67 | const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5) |
68 | 68 | ||
69 | expect(res.body.total).to.equal(0) | 69 | expect(res.body.total).to.equal(0) |
70 | expect(res.body.totalNotDeletedComments).to.equal(0) | ||
70 | expect(res.body.data).to.be.an('array') | 71 | expect(res.body.data).to.be.an('array') |
71 | expect(res.body.data).to.have.lengthOf(0) | 72 | expect(res.body.data).to.have.lengthOf(0) |
72 | }) | 73 | }) |
@@ -94,6 +95,7 @@ describe('Test video comments', function () { | |||
94 | const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5) | 95 | const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5) |
95 | 96 | ||
96 | expect(res.body.total).to.equal(1) | 97 | expect(res.body.total).to.equal(1) |
98 | expect(res.body.totalNotDeletedComments).to.equal(1) | ||
97 | expect(res.body.data).to.be.an('array') | 99 | expect(res.body.data).to.be.an('array') |
98 | expect(res.body.data).to.have.lengthOf(1) | 100 | expect(res.body.data).to.have.lengthOf(1) |
99 | 101 | ||
@@ -172,6 +174,7 @@ describe('Test video comments', function () { | |||
172 | const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5, 'createdAt') | 174 | const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5, 'createdAt') |
173 | 175 | ||
174 | expect(res.body.total).to.equal(3) | 176 | expect(res.body.total).to.equal(3) |
177 | expect(res.body.totalNotDeletedComments).to.equal(6) | ||
175 | expect(res.body.data).to.be.an('array') | 178 | expect(res.body.data).to.be.an('array') |
176 | expect(res.body.data).to.have.lengthOf(3) | 179 | expect(res.body.data).to.have.lengthOf(3) |
177 | 180 | ||
@@ -186,26 +189,35 @@ describe('Test video comments', function () { | |||
186 | it('Should delete a reply', async function () { | 189 | it('Should delete a reply', async function () { |
187 | await deleteVideoComment(server.url, server.accessToken, videoId, replyToDeleteId) | 190 | await deleteVideoComment(server.url, server.accessToken, videoId, replyToDeleteId) |
188 | 191 | ||
189 | const res = await getVideoThreadComments(server.url, videoUUID, threadId) | 192 | { |
190 | 193 | const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5, 'createdAt') | |
191 | const tree: VideoCommentThreadTree = res.body | ||
192 | expect(tree.comment.text).equal('my super first comment') | ||
193 | expect(tree.children).to.have.lengthOf(2) | ||
194 | |||
195 | const firstChild = tree.children[0] | ||
196 | expect(firstChild.comment.text).to.equal('my super answer to thread 1') | ||
197 | expect(firstChild.children).to.have.lengthOf(1) | ||
198 | 194 | ||
199 | const childOfFirstChild = firstChild.children[0] | 195 | expect(res.body.total).to.equal(3) |
200 | expect(childOfFirstChild.comment.text).to.equal('my super answer to answer of thread 1') | 196 | expect(res.body.totalNotDeletedComments).to.equal(5) |
201 | expect(childOfFirstChild.children).to.have.lengthOf(0) | 197 | } |
202 | 198 | ||
203 | const deletedChildOfFirstChild = tree.children[1] | 199 | { |
204 | expect(deletedChildOfFirstChild.comment.text).to.equal('') | 200 | const res = await getVideoThreadComments(server.url, videoUUID, threadId) |
205 | expect(deletedChildOfFirstChild.comment.isDeleted).to.be.true | 201 | |
206 | expect(deletedChildOfFirstChild.comment.deletedAt).to.not.be.null | 202 | const tree: VideoCommentThreadTree = res.body |
207 | expect(deletedChildOfFirstChild.comment.account).to.be.null | 203 | expect(tree.comment.text).equal('my super first comment') |
208 | expect(deletedChildOfFirstChild.children).to.have.lengthOf(0) | 204 | expect(tree.children).to.have.lengthOf(2) |
205 | |||
206 | const firstChild = tree.children[0] | ||
207 | expect(firstChild.comment.text).to.equal('my super answer to thread 1') | ||
208 | expect(firstChild.children).to.have.lengthOf(1) | ||
209 | |||
210 | const childOfFirstChild = firstChild.children[0] | ||
211 | expect(childOfFirstChild.comment.text).to.equal('my super answer to answer of thread 1') | ||
212 | expect(childOfFirstChild.children).to.have.lengthOf(0) | ||
213 | |||
214 | const deletedChildOfFirstChild = tree.children[1] | ||
215 | expect(deletedChildOfFirstChild.comment.text).to.equal('') | ||
216 | expect(deletedChildOfFirstChild.comment.isDeleted).to.be.true | ||
217 | expect(deletedChildOfFirstChild.comment.deletedAt).to.not.be.null | ||
218 | expect(deletedChildOfFirstChild.comment.account).to.be.null | ||
219 | expect(deletedChildOfFirstChild.children).to.have.lengthOf(0) | ||
220 | } | ||
209 | }) | 221 | }) |
210 | 222 | ||
211 | it('Should delete a complete thread', async function () { | 223 | it('Should delete a complete thread', async function () { |