Skip to content

Commit

Permalink
fix video-abuse tests and check timestamp coherence
Browse files Browse the repository at this point in the history
  • Loading branch information
rigelk committed Jun 22, 2020
1 parent 17afa20 commit 2b06047
Show file tree
Hide file tree
Showing 10 changed files with 61 additions and 29 deletions.
4 changes: 2 additions & 2 deletions client/src/app/shared/video/modals/video-report.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { VideoAbuseService } from '@app/shared/video-abuse'
import { Video } from '@app/shared/video/video.model'
import { buildVideoEmbed, buildVideoLink } from 'src/assets/player/utils'
import { DomSanitizer, SafeHtml } from '@angular/platform-browser'
import { VideoAbusePredefinedReasonsString, VideoAbusePredefinedReasonsMap } from '@shared/models/videos/abuse/video-abuse-reason.model'
import { VideoAbusePredefinedReasonsString, videoAbusePredefinedReasonsMap } from '@shared/models/videos/abuse/video-abuse-reason.model'
import { mapValues, pickBy } from 'lodash-es'

@Component({
Expand Down Expand Up @@ -72,7 +72,7 @@ export class VideoReportComponent extends FormReactive implements OnInit {
ngOnInit () {
this.buildForm({
reason: this.videoAbuseValidatorsService.VIDEO_ABUSE_REASON,
predefinedReasons: mapValues(VideoAbusePredefinedReasonsMap, r => null),
predefinedReasons: mapValues(videoAbusePredefinedReasonsMap, r => null),
timestamp: {
hasStart: null,
startAt: null,
Expand Down
4 changes: 2 additions & 2 deletions server/controllers/api/videos/abuse.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as express from 'express'
import { UserRight, VideoAbuseCreate, VideoAbuseState, VideoAbuse, VideoAbusePredefinedReasonsMap } 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'
Expand Down Expand Up @@ -124,7 +124,7 @@ 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 predefinedReasons = body.predefinedReasons?.map(r => videoAbusePredefinedReasonsMap[r])

const abuseToCreate = {
reporterAccountId: reporterAccount.id,
Expand Down
11 changes: 8 additions & 3 deletions server/helpers/custom-validators/video-abuses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import validator from 'validator'
import { CONSTRAINTS_FIELDS, VIDEO_ABUSE_STATES } from '../../initializers/constants'
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'
import { VideoAbusePredefinedReasonsString, videoAbusePredefinedReasonsMap } from '@shared/models/videos/abuse/video-abuse-reason.model'

const VIDEO_ABUSES_CONSTRAINTS_FIELDS = CONSTRAINTS_FIELDS.VIDEO_ABUSES

Expand All @@ -12,17 +12,21 @@ function isVideoAbuseReasonValid (value: string) {
}

function isVideoAbusePredefinedReasonValid (value: VideoAbusePredefinedReasonsString) {
return exists(value) && value in VideoAbusePredefinedReasonsMap
return exists(value) && value in videoAbusePredefinedReasonsMap
}

function isVideoAbusePredefinedReasonsValid (value: VideoAbusePredefinedReasonsString[]) {
return exists(value) && isArray(value) && value.every(v => v in VideoAbusePredefinedReasonsMap)
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)
}
Expand All @@ -45,6 +49,7 @@ export {
isVideoAbusePredefinedReasonValid,
isVideoAbusePredefinedReasonsValid,
isVideoAbuseTimestampValid,
isVideoAbuseTimestampCoherent,
isVideoAbuseModerationCommentValid,
isVideoAbuseStateValid,
isAbuseVideoIsValid
Expand Down
4 changes: 2 additions & 2 deletions server/lib/activitypub/process/process-flag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {
ActivityCreate,
ActivityFlag,
VideoAbuseState,
VideoAbusePredefinedReasonsMap
videoAbusePredefinedReasonsMap
} from '../../../../shared'
import { VideoAbuseObject } from '../../../../shared/models/activitypub/objects'
import { retryTransactionWrapper } from '../../../helpers/database-utils'
Expand Down Expand Up @@ -44,7 +44,7 @@ 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])
const predefinedReasons = tags.map(tag => videoAbusePredefinedReasonsMap[tag.name])
.filter(v => !isNaN(v))
const startAt = flag.startAt
const endAt = flag.endAt
Expand Down
8 changes: 6 additions & 2 deletions server/middlewares/validators/videos/video-abuses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
isVideoAbuseStateValid,
isVideoAbusePredefinedReasonsValid,
isVideoAbusePredefinedReasonValid,
isVideoAbuseTimestampValid
isVideoAbuseTimestampValid,
isVideoAbuseTimestampCoherent
} from '../../../helpers/custom-validators/video-abuses'
import { logger } from '../../../helpers/logger'
import { doesVideoAbuseExist, doesVideoExist } from '../../../helpers/middlewares'
Expand Down Expand Up @@ -36,7 +37,10 @@ const videoAbuseReportValidator = [
.optional()
.customSanitizer(toIntOrNull)
.custom(isVideoAbuseTimestampValid)
.withMessage('Should have valid ending time value'),
.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 })
Expand Down
12 changes: 6 additions & 6 deletions server/models/video/video-abuse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
VideoDetails,
VideoAbusePredefinedReasons,
VideoAbusePredefinedReasonsString,
VideoAbusePredefinedReasonsMap
videoAbusePredefinedReasonsMap
} from '../../../shared'
import { VideoAbuseObject } from '../../../shared/models/activitypub/objects'
import { VideoAbuse } from '../../../shared/models/videos'
Expand Down Expand Up @@ -370,7 +370,7 @@ export class VideoAbuseModel extends Model<VideoAbuseModel> {
} = parameters

const userAccountId = user ? user.Account.id : undefined
const predefinedReasonId = predefinedReason ? VideoAbusePredefinedReasonsMap[predefinedReason] : undefined
const predefinedReasonId = predefinedReason ? videoAbusePredefinedReasonsMap[predefinedReason] : undefined

const query = {
offset: start,
Expand Down Expand Up @@ -405,7 +405,7 @@ export class VideoAbuseModel extends Model<VideoAbuseModel> {
}

toFormattedJSON (this: MVideoAbuseFormattable): VideoAbuse {
const predefinedReasons = VideoAbuseModel.getPredefinedReasonsStrings(this.predefinedReasons || [])
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
Expand Down Expand Up @@ -449,7 +449,7 @@ export class VideoAbuseModel extends Model<VideoAbuseModel> {
}

toActivityPubObject (this: MVideoAbuseVideo): VideoAbuseObject {
const predefinedReasons = VideoAbuseModel.getPredefinedReasonsStrings(this.predefinedReasons || [])
const predefinedReasons = VideoAbuseModel.getPredefinedReasonsStrings(this.predefinedReasons)

const startAt = this.startAt
const endAt = this.endAt
Expand All @@ -472,8 +472,8 @@ export class VideoAbuseModel extends Model<VideoAbuseModel> {
}

private static getPredefinedReasonsStrings (predefinedReasons: VideoAbusePredefinedReasons[]): VideoAbusePredefinedReasonsString[] {
return predefinedReasons
return (predefinedReasons || [])
.filter(r => r in VideoAbusePredefinedReasons)
.map(r => invert(VideoAbusePredefinedReasonsMap)[r] as VideoAbusePredefinedReasonsString)
.map(r => invert(videoAbusePredefinedReasonsMap)[r] as VideoAbusePredefinedReasonsString)
}
}
24 changes: 21 additions & 3 deletions server/tests/api/check-params/video-abuses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -132,12 +132,30 @@ 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 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 () {
Expand Down
15 changes: 7 additions & 8 deletions server/tests/api/videos/video-abuse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,17 +304,16 @@ describe('Test video abuses', function () {
predefinedReasons5,
1,
5
)).body as VideoAbuse
)).body.videoAbuse as VideoAbuse

const res = await getVideoAbusesList({ url: servers[0].url, token: servers[0].accessToken })

for (const abuse of res.body.data as VideoAbuse[]) {
if (abuse.id === createdAbuse.id) {
expect(abuse.reason).to.equals(reason5)
expect(abuse.predefinedReasons).to.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")
}
{
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")
}
})

Expand Down
2 changes: 1 addition & 1 deletion shared/models/videos/abuse/video-abuse-reason.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export type VideoAbusePredefinedReasonsString =
'thumbnails' |
'captions'

export const VideoAbusePredefinedReasonsMap: {
export const videoAbusePredefinedReasonsMap: {
[key in VideoAbusePredefinedReasonsString]: VideoAbusePredefinedReasons
} = {
violentOrRepulsive: VideoAbusePredefinedReasons.VIOLENT_OR_REPULSIVE,
Expand Down
6 changes: 6 additions & 0 deletions support/doc/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1268,6 +1268,12 @@ paths:
- serverRules
- thumbnails
- captions
startAt:
type: number
description: Timestamp in the video that marks the beginning of the report
endAt:
type: number
description: Timestamp in the video that marks the ending of the report
required:
- reason
responses:
Expand Down

0 comments on commit 2b06047

Please sign in to comment.