From 1ebddadd0704812a4600c39cabe2268321e88331 Mon Sep 17 00:00:00 2001 From: Rigel Kent Date: Mon, 22 Jun 2020 13:00:39 +0200 Subject: predefined report reasons & improved reporter UI (#2842) - added `startAt` and `endAt` optional timestamps to help pin down reported sections of a video - added predefined report reasons - added video player with report modal --- server/controllers/api/videos/abuse.ts | 11 +++- server/helpers/custom-validators/video-abuses.ts | 29 ++++++++-- server/initializers/constants.ts | 2 +- .../0515-video-abuse-reason-timestamps.ts | 31 +++++++++++ server/lib/activitypub/process/process-flag.ts | 17 +++++- .../middlewares/validators/videos/video-abuses.ts | 39 +++++++++++-- server/models/video/video-abuse.ts | 64 +++++++++++++++++++++- server/tests/api/check-params/video-abuses.ts | 30 +++++++++- server/tests/api/videos/video-abuse.ts | 43 ++++++++++++--- 9 files changed, 239 insertions(+), 27 deletions(-) create mode 100644 server/initializers/migrations/0515-video-abuse-reason-timestamps.ts (limited to 'server') diff --git a/server/controllers/api/videos/abuse.ts b/server/controllers/api/videos/abuse.ts index 77843f149..ab2074459 100644 --- a/server/controllers/api/videos/abuse.ts +++ b/server/controllers/api/videos/abuse.ts @@ -1,5 +1,5 @@ import * as express from 'express' -import { UserRight, VideoAbuseCreate, VideoAbuseState, VideoAbuse } from '../../../../shared' +import { UserRight, VideoAbuseCreate, VideoAbuseState, VideoAbuse, videoAbusePredefinedReasonsMap } from '../../../../shared' import { logger } from '../../../helpers/logger' import { getFormattedObjects } from '../../../helpers/utils' import { sequelizeTypescript } from '../../../initializers/database' @@ -74,6 +74,7 @@ async function listVideoAbuses (req: express.Request, res: express.Response) { count: req.query.count, sort: req.query.sort, id: req.query.id, + predefinedReason: req.query.predefinedReason, search: req.query.search, state: req.query.state, videoIs: req.query.videoIs, @@ -123,12 +124,16 @@ async function reportVideoAbuse (req: express.Request, res: express.Response) { const videoAbuseInstance = await sequelizeTypescript.transaction(async t => { reporterAccount = await AccountModel.load(res.locals.oauth.token.User.Account.id, t) + const predefinedReasons = body.predefinedReasons?.map(r => videoAbusePredefinedReasonsMap[r]) const abuseToCreate = { reporterAccountId: reporterAccount.id, reason: body.reason, videoId: videoInstance.id, - state: VideoAbuseState.PENDING + state: VideoAbuseState.PENDING, + predefinedReasons, + startAt: body.startAt, + endAt: body.endAt } const videoAbuseInstance: MVideoAbuseAccountVideo = await VideoAbuseModel.create(abuseToCreate, { transaction: t }) @@ -152,7 +157,7 @@ async function reportVideoAbuse (req: express.Request, res: express.Response) { reporter: reporterAccount.Actor.getIdentifier() }) - logger.info('Abuse report for video %s created.', videoInstance.name) + logger.info('Abuse report for video "%s" created.', videoInstance.name) return res.json({ videoAbuse: videoAbuseJSON }).end() } diff --git a/server/helpers/custom-validators/video-abuses.ts b/server/helpers/custom-validators/video-abuses.ts index 05e11b1c6..0c2c34268 100644 --- a/server/helpers/custom-validators/video-abuses.ts +++ b/server/helpers/custom-validators/video-abuses.ts @@ -1,8 +1,9 @@ import validator from 'validator' import { CONSTRAINTS_FIELDS, VIDEO_ABUSE_STATES } from '../../initializers/constants' -import { exists } from './misc' +import { exists, isArray } from './misc' import { VideoAbuseVideoIs } from '@shared/models/videos/abuse/video-abuse-video-is.type' +import { VideoAbusePredefinedReasonsString, videoAbusePredefinedReasonsMap } from '@shared/models/videos/abuse/video-abuse-reason.model' const VIDEO_ABUSES_CONSTRAINTS_FIELDS = CONSTRAINTS_FIELDS.VIDEO_ABUSES @@ -10,6 +11,22 @@ function isVideoAbuseReasonValid (value: string) { return exists(value) && validator.isLength(value, VIDEO_ABUSES_CONSTRAINTS_FIELDS.REASON) } +function isVideoAbusePredefinedReasonValid (value: VideoAbusePredefinedReasonsString) { + return exists(value) && value in videoAbusePredefinedReasonsMap +} + +function isVideoAbusePredefinedReasonsValid (value: VideoAbusePredefinedReasonsString[]) { + return exists(value) && isArray(value) && value.every(v => v in videoAbusePredefinedReasonsMap) +} + +function isVideoAbuseTimestampValid (value: number) { + return value === null || (exists(value) && validator.isInt('' + value, { min: 0 })) +} + +function isVideoAbuseTimestampCoherent (endAt: number, { req }) { + return exists(req.body.startAt) && endAt > req.body.startAt +} + function isVideoAbuseModerationCommentValid (value: string) { return exists(value) && validator.isLength(value, VIDEO_ABUSES_CONSTRAINTS_FIELDS.MODERATION_COMMENT) } @@ -28,8 +45,12 @@ function isAbuseVideoIsValid (value: VideoAbuseVideoIs) { // --------------------------------------------------------------------------- export { - isVideoAbuseStateValid, isVideoAbuseReasonValid, - isAbuseVideoIsValid, - isVideoAbuseModerationCommentValid + isVideoAbusePredefinedReasonValid, + isVideoAbusePredefinedReasonsValid, + isVideoAbuseTimestampValid, + isVideoAbuseTimestampCoherent, + isVideoAbuseModerationCommentValid, + isVideoAbuseStateValid, + isAbuseVideoIsValid } diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts index 314f094b3..dd79c0e16 100644 --- a/server/initializers/constants.ts +++ b/server/initializers/constants.ts @@ -14,7 +14,7 @@ import { CONFIG, registerConfigChangedHandler } from './config' // --------------------------------------------------------------------------- -const LAST_MIGRATION_VERSION = 510 +const LAST_MIGRATION_VERSION = 515 // --------------------------------------------------------------------------- diff --git a/server/initializers/migrations/0515-video-abuse-reason-timestamps.ts b/server/initializers/migrations/0515-video-abuse-reason-timestamps.ts new file mode 100644 index 000000000..c58335617 --- /dev/null +++ b/server/initializers/migrations/0515-video-abuse-reason-timestamps.ts @@ -0,0 +1,31 @@ +import * as Sequelize from 'sequelize' + +async function up (utils: { + transaction: Sequelize.Transaction + queryInterface: Sequelize.QueryInterface + sequelize: Sequelize.Sequelize +}): Promise { + await utils.queryInterface.addColumn('videoAbuse', 'predefinedReasons', { + type: Sequelize.ARRAY(Sequelize.INTEGER), + allowNull: true + }) + + await utils.queryInterface.addColumn('videoAbuse', 'startAt', { + type: Sequelize.INTEGER, + allowNull: true + }) + + await utils.queryInterface.addColumn('videoAbuse', 'endAt', { + type: Sequelize.INTEGER, + allowNull: true + }) +} + +function down (options) { + throw new Error('Not implemented.') +} + +export { + up, + down +} diff --git a/server/lib/activitypub/process/process-flag.ts b/server/lib/activitypub/process/process-flag.ts index 8d1c9c869..1d7132a3a 100644 --- a/server/lib/activitypub/process/process-flag.ts +++ b/server/lib/activitypub/process/process-flag.ts @@ -1,4 +1,9 @@ -import { ActivityCreate, ActivityFlag, VideoAbuseState } from '../../../../shared' +import { + ActivityCreate, + ActivityFlag, + VideoAbuseState, + videoAbusePredefinedReasonsMap +} from '../../../../shared' import { VideoAbuseObject } from '../../../../shared/models/activitypub/objects' import { retryTransactionWrapper } from '../../../helpers/database-utils' import { logger } from '../../../helpers/logger' @@ -38,13 +43,21 @@ async function processCreateVideoAbuse (activity: ActivityCreate | ActivityFlag, const { video } = await getOrCreateVideoAndAccountAndChannel({ videoObject: object }) const reporterAccount = await sequelizeTypescript.transaction(async t => AccountModel.load(account.id, t)) + const tags = Array.isArray(flag.tag) ? flag.tag : [] + const predefinedReasons = tags.map(tag => videoAbusePredefinedReasonsMap[tag.name]) + .filter(v => !isNaN(v)) + const startAt = flag.startAt + const endAt = flag.endAt const videoAbuseInstance = await sequelizeTypescript.transaction(async t => { const videoAbuseData = { reporterAccountId: account.id, reason: flag.content, videoId: video.id, - state: VideoAbuseState.PENDING + state: VideoAbuseState.PENDING, + predefinedReasons, + startAt, + endAt } const videoAbuseInstance: MVideoAbuseAccountVideo = await VideoAbuseModel.create(videoAbuseData, { transaction: t }) diff --git a/server/middlewares/validators/videos/video-abuses.ts b/server/middlewares/validators/videos/video-abuses.ts index 901997bcb..5bbd1e3c6 100644 --- a/server/middlewares/validators/videos/video-abuses.ts +++ b/server/middlewares/validators/videos/video-abuses.ts @@ -1,19 +1,46 @@ import * as express from 'express' import { body, param, query } from 'express-validator' -import { exists, isIdOrUUIDValid, isIdValid } from '../../../helpers/custom-validators/misc' +import { exists, isIdOrUUIDValid, isIdValid, toIntOrNull } from '../../../helpers/custom-validators/misc' import { isAbuseVideoIsValid, isVideoAbuseModerationCommentValid, isVideoAbuseReasonValid, - isVideoAbuseStateValid + isVideoAbuseStateValid, + isVideoAbusePredefinedReasonsValid, + isVideoAbusePredefinedReasonValid, + isVideoAbuseTimestampValid, + isVideoAbuseTimestampCoherent } from '../../../helpers/custom-validators/video-abuses' import { logger } from '../../../helpers/logger' import { doesVideoAbuseExist, doesVideoExist } from '../../../helpers/middlewares' import { areValidationErrors } from '../utils' const videoAbuseReportValidator = [ - param('videoId').custom(isIdOrUUIDValid).not().isEmpty().withMessage('Should have a valid videoId'), - body('reason').custom(isVideoAbuseReasonValid).withMessage('Should have a valid reason'), + param('videoId') + .custom(isIdOrUUIDValid) + .not() + .isEmpty() + .withMessage('Should have a valid videoId'), + body('reason') + .custom(isVideoAbuseReasonValid) + .withMessage('Should have a valid reason'), + body('predefinedReasons') + .optional() + .custom(isVideoAbusePredefinedReasonsValid) + .withMessage('Should have a valid list of predefined reasons'), + body('startAt') + .optional() + .customSanitizer(toIntOrNull) + .custom(isVideoAbuseTimestampValid) + .withMessage('Should have valid starting time value'), + body('endAt') + .optional() + .customSanitizer(toIntOrNull) + .custom(isVideoAbuseTimestampValid) + .withMessage('Should have valid ending time value') + .bail() + .custom(isVideoAbuseTimestampCoherent) + .withMessage('Should have a startAt timestamp beginning before endAt'), async (req: express.Request, res: express.Response, next: express.NextFunction) => { logger.debug('Checking videoAbuseReport parameters', { parameters: req.body }) @@ -63,6 +90,10 @@ const videoAbuseListValidator = [ query('id') .optional() .custom(isIdValid).withMessage('Should have a valid id'), + query('predefinedReason') + .optional() + .custom(isVideoAbusePredefinedReasonValid) + .withMessage('Should have a valid predefinedReason'), query('search') .optional() .custom(exists).withMessage('Should have a valid search'), diff --git a/server/models/video/video-abuse.ts b/server/models/video/video-abuse.ts index b2f111337..1319332f0 100644 --- a/server/models/video/video-abuse.ts +++ b/server/models/video/video-abuse.ts @@ -15,7 +15,13 @@ import { UpdatedAt } from 'sequelize-typescript' import { VideoAbuseVideoIs } from '@shared/models/videos/abuse/video-abuse-video-is.type' -import { VideoAbuseState, VideoDetails } from '../../../shared' +import { + VideoAbuseState, + VideoDetails, + VideoAbusePredefinedReasons, + VideoAbusePredefinedReasonsString, + videoAbusePredefinedReasonsMap +} from '../../../shared' import { VideoAbuseObject } from '../../../shared/models/activitypub/objects' import { VideoAbuse } from '../../../shared/models/videos' import { @@ -31,6 +37,7 @@ import { ThumbnailModel } from './thumbnail' import { VideoModel } from './video' import { VideoBlacklistModel } from './video-blacklist' import { ScopeNames as VideoChannelScopeNames, SummaryOptions, VideoChannelModel } from './video-channel' +import { invert } from 'lodash' export enum ScopeNames { FOR_API = 'FOR_API' @@ -47,6 +54,7 @@ export enum ScopeNames { // filters id?: number + predefinedReasonId?: number state?: VideoAbuseState videoIs?: VideoAbuseVideoIs @@ -104,6 +112,14 @@ export enum ScopeNames { }) } + if (options.predefinedReasonId) { + Object.assign(where, { + predefinedReasons: { + [Op.contains]: [ options.predefinedReasonId ] + } + }) + } + const onlyBlacklisted = options.videoIs === 'blacklisted' return { @@ -258,6 +274,21 @@ export class VideoAbuseModel extends Model { @Column(DataType.JSONB) deletedVideo: VideoDetails + @AllowNull(true) + @Default(null) + @Column(DataType.ARRAY(DataType.INTEGER)) + predefinedReasons: VideoAbusePredefinedReasons[] + + @AllowNull(true) + @Default(null) + @Column + startAt: number + + @AllowNull(true) + @Default(null) + @Column + endAt: number + @CreatedAt createdAt: Date @@ -311,6 +342,7 @@ export class VideoAbuseModel extends Model { user?: MUserAccountId id?: number + predefinedReason?: VideoAbusePredefinedReasonsString state?: VideoAbuseState videoIs?: VideoAbuseVideoIs @@ -329,6 +361,7 @@ export class VideoAbuseModel extends Model { serverAccountId, state, videoIs, + predefinedReason, searchReportee, searchVideo, searchVideoChannel, @@ -337,6 +370,7 @@ export class VideoAbuseModel extends Model { } = parameters const userAccountId = user ? user.Account.id : undefined + const predefinedReasonId = predefinedReason ? videoAbusePredefinedReasonsMap[predefinedReason] : undefined const query = { offset: start, @@ -348,6 +382,7 @@ export class VideoAbuseModel extends Model { const filters = { id, + predefinedReasonId, search, state, videoIs, @@ -360,7 +395,9 @@ export class VideoAbuseModel extends Model { } return VideoAbuseModel - .scope({ method: [ ScopeNames.FOR_API, filters ] }) + .scope([ + { method: [ ScopeNames.FOR_API, filters ] } + ]) .findAndCountAll(query) .then(({ rows, count }) => { return { total: count, data: rows } @@ -368,6 +405,7 @@ export class VideoAbuseModel extends Model { } toFormattedJSON (this: MVideoAbuseFormattable): VideoAbuse { + const predefinedReasons = VideoAbuseModel.getPredefinedReasonsStrings(this.predefinedReasons) const countReportsForVideo = this.get('countReportsForVideo') as number const nthReportForVideo = this.get('nthReportForVideo') as number const countReportsForReporterVideo = this.get('countReportsForReporter__video') as number @@ -382,6 +420,7 @@ export class VideoAbuseModel extends Model { return { id: this.id, reason: this.reason, + predefinedReasons, reporterAccount: this.Account.toFormattedJSON(), state: { id: this.state, @@ -400,6 +439,8 @@ export class VideoAbuseModel extends Model { }, createdAt: this.createdAt, updatedAt: this.updatedAt, + startAt: this.startAt, + endAt: this.endAt, count: countReportsForVideo || 0, nth: nthReportForVideo || 0, countReportsForReporter: (countReportsForReporterVideo || 0) + (countReportsForReporterDeletedVideo || 0), @@ -408,14 +449,31 @@ export class VideoAbuseModel extends Model { } toActivityPubObject (this: MVideoAbuseVideo): VideoAbuseObject { + const predefinedReasons = VideoAbuseModel.getPredefinedReasonsStrings(this.predefinedReasons) + + const startAt = this.startAt + const endAt = this.endAt + return { type: 'Flag' as 'Flag', content: this.reason, - object: this.Video.url + object: this.Video.url, + tag: predefinedReasons.map(r => ({ + type: 'Hashtag' as 'Hashtag', + name: r + })), + startAt, + endAt } } private static getStateLabel (id: number) { return VIDEO_ABUSE_STATES[id] || 'Unknown' } + + private static getPredefinedReasonsStrings (predefinedReasons: VideoAbusePredefinedReasons[]): VideoAbusePredefinedReasonsString[] { + return (predefinedReasons || []) + .filter(r => r in VideoAbusePredefinedReasons) + .map(r => invert(videoAbusePredefinedReasonsMap)[r] as VideoAbusePredefinedReasonsString) + } } diff --git a/server/tests/api/check-params/video-abuses.ts b/server/tests/api/check-params/video-abuses.ts index a3fe00ffb..557bf20eb 100644 --- a/server/tests/api/check-params/video-abuses.ts +++ b/server/tests/api/check-params/video-abuses.ts @@ -20,7 +20,7 @@ import { checkBadSortPagination, checkBadStartPagination } from '../../../../shared/extra-utils/requests/check-api-params' -import { VideoAbuseState } from '../../../../shared/models/videos' +import { VideoAbuseState, VideoAbuseCreate } from '../../../../shared/models/videos' describe('Test video abuses API validators', function () { let server: ServerInfo @@ -132,12 +132,36 @@ describe('Test video abuses API validators', function () { await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) }) - it('Should succeed with the correct parameters', async function () { - const fields = { reason: 'super reason' } + it('Should succeed with the correct parameters (basic)', async function () { + const fields = { reason: 'my super reason' } const res = await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields, statusCodeExpected: 200 }) videoAbuseId = res.body.videoAbuse.id }) + + it('Should fail with a wrong predefined reason', async function () { + const fields = { reason: 'my super reason', predefinedReasons: [ 'wrongPredefinedReason' ] } + + await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) + }) + + it('Should fail with negative timestamps', async function () { + const fields = { reason: 'my super reason', startAt: -1 } + + await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) + }) + + it('Should fail mith misordered startAt/endAt', async function () { + const fields = { reason: 'my super reason', startAt: 5, endAt: 1 } + + await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields }) + }) + + it('Should succeed with the corret parameters (advanced)', async function () { + const fields: VideoAbuseCreate = { reason: 'my super reason', predefinedReasons: [ 'serverRules' ], startAt: 1, endAt: 5 } + + await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields, statusCodeExpected: 200 }) + }) }) describe('When updating a video abuse', function () { diff --git a/server/tests/api/videos/video-abuse.ts b/server/tests/api/videos/video-abuse.ts index a96be97f6..7383bd991 100644 --- a/server/tests/api/videos/video-abuse.ts +++ b/server/tests/api/videos/video-abuse.ts @@ -2,7 +2,7 @@ import * as chai from 'chai' import 'mocha' -import { VideoAbuse, VideoAbuseState } from '../../../../shared/models/videos' +import { VideoAbuse, VideoAbuseState, VideoAbusePredefinedReasonsString } from '../../../../shared/models/videos' import { cleanupTests, deleteVideoAbuse, @@ -291,6 +291,32 @@ describe('Test video abuses', function () { } }) + it('Should list predefined reasons as well as timestamps for the reported video', async function () { + this.timeout(10000) + + const reason5 = 'my super bad reason 5' + const predefinedReasons5: VideoAbusePredefinedReasonsString[] = [ 'violentOrRepulsive', 'captions' ] + const createdAbuse = (await reportVideoAbuse( + servers[0].url, + servers[0].accessToken, + servers[0].video.id, + reason5, + predefinedReasons5, + 1, + 5 + )).body.videoAbuse as VideoAbuse + + const res = await getVideoAbusesList({ url: servers[0].url, token: servers[0].accessToken }) + + { + const abuse = (res.body.data as VideoAbuse[]).find(a => a.id === createdAbuse.id) + expect(abuse.reason).to.equals(reason5) + expect(abuse.predefinedReasons).to.deep.equals(predefinedReasons5, "predefined reasons do not match the one reported") + expect(abuse.startAt).to.equal(1, "starting timestamp doesn't match the one reported") + expect(abuse.endAt).to.equal(5, "ending timestamp doesn't match the one reported") + } + }) + it('Should delete the video abuse', async function () { this.timeout(10000) @@ -307,7 +333,7 @@ describe('Test video abuses', function () { { const res = await getVideoAbusesList({ url: servers[0].url, token: servers[0].accessToken }) - expect(res.body.total).to.equal(5) + expect(res.body.total).to.equal(6) } }) @@ -328,25 +354,28 @@ describe('Test video abuses', function () { expect(await list({ id: 56 })).to.have.lengthOf(0) expect(await list({ id: 1 })).to.have.lengthOf(1) - expect(await list({ search: 'my super name for server 1' })).to.have.lengthOf(3) + expect(await list({ search: 'my super name for server 1' })).to.have.lengthOf(4) expect(await list({ search: 'aaaaaaaaaaaaaaaaaaaaaaaaaa' })).to.have.lengthOf(0) expect(await list({ searchVideo: 'my second super name for server 1' })).to.have.lengthOf(1) - expect(await list({ searchVideoChannel: 'root' })).to.have.lengthOf(3) + expect(await list({ searchVideoChannel: 'root' })).to.have.lengthOf(4) expect(await list({ searchVideoChannel: 'aaaa' })).to.have.lengthOf(0) expect(await list({ searchReporter: 'user2' })).to.have.lengthOf(1) - expect(await list({ searchReporter: 'root' })).to.have.lengthOf(4) + expect(await list({ searchReporter: 'root' })).to.have.lengthOf(5) - expect(await list({ searchReportee: 'root' })).to.have.lengthOf(3) + expect(await list({ searchReportee: 'root' })).to.have.lengthOf(4) expect(await list({ searchReportee: 'aaaa' })).to.have.lengthOf(0) expect(await list({ videoIs: 'deleted' })).to.have.lengthOf(1) expect(await list({ videoIs: 'blacklisted' })).to.have.lengthOf(0) expect(await list({ state: VideoAbuseState.ACCEPTED })).to.have.lengthOf(0) - expect(await list({ state: VideoAbuseState.PENDING })).to.have.lengthOf(5) + expect(await list({ state: VideoAbuseState.PENDING })).to.have.lengthOf(6) + + expect(await list({ predefinedReason: 'violentOrRepulsive' })).to.have.lengthOf(1) + expect(await list({ predefinedReason: 'serverRules' })).to.have.lengthOf(0) }) after(async function () { -- cgit v1.2.3