Add banner tests

server/controllers/api/users/me.ts

@@ -25,7 +25,7 @@ import {
   usersVideoRatingValidator
 } from '../../../middlewares'
 import { deleteMeValidator, videoImportsSortValidator, videosSortValidator } from '../../../middlewares/validators'
-import { updateAvatarValidator } from '../../../middlewares/validators/avatar'
+import { updateAvatarValidator } from '../../../middlewares/validators/actor-image'
 import { AccountModel } from '../../../models/account/account'
 import { AccountVideoRateModel } from '../../../models/account/account-video-rate'
 import { UserModel } from '../../../models/account/user'

server/controllers/api/video-channel.ts

@@ -33,7 +33,7 @@ import {
   videoPlaylistsSortValidator
 } from '../../middlewares'
 import { videoChannelsNameWithHostValidator, videoChannelsOwnSearchValidator, videosSortValidator } from '../../middlewares/validators'
-import { updateAvatarValidator, updateBannerValidator } from '../../middlewares/validators/avatar'
+import { updateAvatarValidator, updateBannerValidator } from '../../middlewares/validators/actor-image'
 import { commonVideoPlaylistFiltersValidator } from '../../middlewares/validators/videos/video-playlists'
 import { AccountModel } from '../../models/account/account'
 import { VideoModel } from '../../models/video/video'

server/helpers/custom-validators/actor-images.ts

@@ -0,0 +1,17 @@
+
+import { CONSTRAINTS_FIELDS } from '../../initializers/constants'
+import { isFileValid } from './misc'
+
+const imageMimeTypes = CONSTRAINTS_FIELDS.ACTORS.IMAGE.EXTNAME
+  .map(v => v.replace('.', ''))
+  .join('|')
+const imageMimeTypesRegex = `image/(${imageMimeTypes})`
+function isActorImageFile (files: { [ fieldname: string ]: Express.Multer.File[] } | Express.Multer.File[], fieldname: string) {
+  return isFileValid(files, imageMimeTypesRegex, fieldname, CONSTRAINTS_FIELDS.ACTORS.IMAGE.FILE_SIZE.max)
+}
+
+// ---------------------------------------------------------------------------
+
+export {
+  isActorImageFile
+}

server/helpers/custom-validators/users.ts

@@ -3,7 +3,7 @@ import validator from 'validator'
 import { UserRole } from '../../../shared'
 import { isEmailEnabled } from '../../initializers/config'
 import { CONSTRAINTS_FIELDS, NSFW_POLICY_TYPES } from '../../initializers/constants'
-import { exists, isArray, isBooleanValid, isFileValid } from './misc'
+import { exists, isArray, isBooleanValid } from './misc'
 
 const USERS_CONSTRAINTS_FIELDS = CONSTRAINTS_FIELDS.USERS
 
@@ -97,14 +97,6 @@ function isUserRoleValid (value: any) {
   return exists(value) && validator.isInt('' + value) && UserRole[value] !== undefined
 }
 
-const avatarMimeTypes = CONSTRAINTS_FIELDS.ACTORS.IMAGE.EXTNAME
-  .map(v => v.replace('.', ''))
-  .join('|')
-const avatarMimeTypesRegex = `image/(${avatarMimeTypes})`
-function isAvatarFile (files: { [ fieldname: string ]: Express.Multer.File[] } | Express.Multer.File[]) {
-  return isFileValid(files, avatarMimeTypesRegex, 'avatarfile', CONSTRAINTS_FIELDS.ACTORS.IMAGE.FILE_SIZE.max)
-}
-
 // ---------------------------------------------------------------------------
 
 export {
@@ -128,6 +120,5 @@ export {
   isUserDisplayNameValid,
   isUserDescriptionValid,
   isNoInstanceConfigWarningModal,
-  isNoWelcomeModal,
-  isAvatarFile
+  isNoWelcomeModal
 }

server/lib/activitypub/actor.ts

@@ -34,6 +34,7 @@ import {
   MActorFull,
   MActorFullActor,
   MActorId,
+  MActorImage,
   MActorImages,
   MChannel
 } from '../../types/models'
@@ -169,38 +170,34 @@ async function updateActorInstance (actorInstance: ActorModel, attributes: Activ
   }
 }
 
-type AvatarInfo = { name: string, onDisk: boolean, fileUrl: string, type: ActorImageType }
-async function updateActorImageInstance (actor: MActorImages, info: AvatarInfo, t: Transaction) {
-  if (!info.name) return actor
-
-  const oldImageModel = info.type === ActorImageType.AVATAR
+type ImageInfo = { name: string, onDisk?: boolean, fileUrl: string }
+async function updateActorImageInstance (actor: MActorImages, type: ActorImageType, imageInfo: ImageInfo | null, t: Transaction) {
+  const oldImageModel = type === ActorImageType.AVATAR
     ? actor.Avatar
     : actor.Banner
 
   if (oldImageModel) {
     // Don't update the avatar if the file URL did not change
-    if (info.fileUrl && oldImageModel.fileUrl === info.fileUrl) return actor
+    if (imageInfo?.fileUrl && oldImageModel.fileUrl === imageInfo.fileUrl) return actor
 
     try {
       await oldImageModel.destroy({ transaction: t })
+
+      setActorImage(actor, type, null)
     } catch (err) {
       logger.error('Cannot remove old actor image of actor %s.', actor.url, { err })
     }
   }
 
-  const imageModel = await ActorImageModel.create({
-    filename: info.name,
-    onDisk: info.onDisk,
-    fileUrl: info.fileUrl,
-    type: info.type
-  }, { transaction: t })
+  if (imageInfo) {
+    const imageModel = await ActorImageModel.create({
+      filename: imageInfo.name,
+      onDisk: imageInfo.onDisk ?? false,
+      fileUrl: imageInfo.fileUrl,
+      type: type
+    }, { transaction: t })
 
-  if (info.type === ActorImageType.AVATAR) {
-    actor.avatarId = imageModel.id
-    actor.Avatar = imageModel
-  } else {
-    actor.bannerId = imageModel.id
-    actor.Banner = imageModel
+    setActorImage(actor, type, imageModel)
   }
 
   return actor
@@ -310,27 +307,8 @@ async function refreshActorIfNeeded <T extends MActorFull | MActorAccountChannel
     return sequelizeTypescript.transaction(async t => {
       updateInstanceWithAnother(actor, result.actor)
 
-      if (result.avatar !== undefined) {
-        const avatarInfo = {
-          name: result.avatar.name,
-          fileUrl: result.avatar.fileUrl,
-          onDisk: false,
-          type: ActorImageType.AVATAR
-        }
-
-        await updateActorImageInstance(actor, avatarInfo, t)
-      }
-
-      if (result.banner !== undefined) {
-        const bannerInfo = {
-          name: result.banner.name,
-          fileUrl: result.banner.fileUrl,
-          onDisk: false,
-          type: ActorImageType.BANNER
-        }
-
-        await updateActorImageInstance(actor, bannerInfo, t)
-      }
+      await updateActorImageInstance(actor, ActorImageType.AVATAR, result.avatar, t)
+      await updateActorImageInstance(actor, ActorImageType.BANNER, result.banner, t)
 
       // Force update
       actor.setDataValue('updatedAt', new Date())
@@ -381,6 +359,22 @@ export {
 
 // ---------------------------------------------------------------------------
 
+function setActorImage (actorModel: MActorImages, type: ActorImageType, imageModel: MActorImage) {
+  const id = imageModel
+    ? imageModel.id
+    : null
+
+  if (type === ActorImageType.AVATAR) {
+    actorModel.avatarId = id
+    actorModel.Avatar = imageModel
+  } else {
+    actorModel.bannerId = id
+    actorModel.Banner = imageModel
+  }
+
+  return actorModel
+}
+
 function saveActorAndServerAndModelIfNotExist (
   result: FetchRemoteActorResult,
   ownerActor?: MActorFullActor,

server/lib/activitypub/process/process-update.ts

@@ -134,13 +134,8 @@ async function processUpdateActor (actor: ActorModel, activity: ActivityUpdate)
 
       await updateActorInstance(actor, actorAttributesToUpdate)
 
-      for (const imageInfo of [ avatarInfo, bannerInfo ]) {
-        if (!imageInfo) continue
-
-        const imageOptions = Object.assign({}, imageInfo, { onDisk: false })
-
-        await updateActorImageInstance(actor, imageOptions, t)
-      }
+      await updateActorImageInstance(actor, ActorImageType.AVATAR, avatarInfo, t)
+      await updateActorImageInstance(actor, ActorImageType.BANNER, bannerInfo, t)
 
       await actor.save({ transaction: t })
 

server/lib/actor-image.ts

@@ -34,11 +34,10 @@ async function updateLocalActorImageFile (
       const actorImageInfo = {
         name: imageName,
         fileUrl: null,
-        type,
         onDisk: true
       }
 
-      const updatedActor = await updateActorImageInstance(accountOrChannel.Actor, actorImageInfo, t)
+      const updatedActor = await updateActorImageInstance(accountOrChannel.Actor, type, actorImageInfo, t)
       await updatedActor.save({ transaction: t })
 
       await sendUpdateActor(accountOrChannel, t)

server/middlewares/validators/actor-image.ts

@@ -1,13 +1,13 @@
 import * as express from 'express'
 import { body } from 'express-validator'
-import { isAvatarFile } from '../../helpers/custom-validators/users'
-import { areValidationErrors } from './utils'
-import { CONSTRAINTS_FIELDS } from '../../initializers/constants'
-import { logger } from '../../helpers/logger'
+import { isActorImageFile } from '@server/helpers/custom-validators/actor-images'
 import { cleanUpReqFiles } from '../../helpers/express-utils'
+import { logger } from '../../helpers/logger'
+import { CONSTRAINTS_FIELDS } from '../../initializers/constants'
+import { areValidationErrors } from './utils'
 
 const updateActorImageValidatorFactory = (fieldname: string) => ([
-  body(fieldname).custom((value, { req }) => isAvatarFile(req.files)).withMessage(
+  body(fieldname).custom((value, { req }) => isActorImageFile(req.files, fieldname)).withMessage(
     'This file is not supported or too large. Please, make sure it is of the following type : ' +
     CONSTRAINTS_FIELDS.ACTORS.IMAGE.EXTNAME.join(', ')
   ),

server/middlewares/validators/index.ts

@@ -1,5 +1,6 @@
 export * from './abuse'
 export * from './account'
+export * from './actor-image'
 export * from './blocklist'
 export * from './oembed'
 export * from './activitypub'

server/models/video/video-channel.ts

@@ -99,7 +99,14 @@ export type SummaryOptions = {
                 }
               }
             ]
-          }
+          },
+          include: [
+            {
+              model: ActorImageModel,
+              as: 'Banner',
+              required: false
+            }
+          ]
         },
         {
           model: AccountModel,

server/tests/api/check-params/video-channels.ts

@@ -234,7 +234,8 @@ describe('Test video channels API validator', function () {
     })
   })
 
-  describe('When updating video channel avatar', function () {
+  describe('When updating video channel avatar/banner', function () {
+    const types = [ 'avatar', 'banner' ]
     let path: string
 
     before(async function () {
@@ -242,48 +243,57 @@ describe('Test video channels API validator', function () {
     })
 
     it('Should fail with an incorrect input file', async function () {
-      const fields = {}
-      const attaches = {
-        avatarfile: join(__dirname, '..', '..', 'fixtures', 'video_short.mp4')
+      for (const type of types) {
+        const fields = {}
+        const attaches = {
+          [type + 'file']: join(__dirname, '..', '..', 'fixtures', 'video_short.mp4')
+        }
+
+        await makeUploadRequest({ url: server.url, path: `${path}/${type}/pick`, token: server.accessToken, fields, attaches })
       }
-      await makeUploadRequest({ url: server.url, path: path + '/avatar/pick', token: server.accessToken, fields, attaches })
     })
 
     it('Should fail with a big file', async function () {
-      const fields = {}
-      const attaches = {
-        avatarfile: join(__dirname, '..', '..', 'fixtures', 'avatar-big.png')
+      for (const type of types) {
+        const fields = {}
+        const attaches = {
+          [type + 'file']: join(__dirname, '..', '..', 'fixtures', 'avatar-big.png')
+        }
+        await makeUploadRequest({ url: server.url, path: `${path}/${type}/pick`, token: server.accessToken, fields, attaches })
       }
-      await makeUploadRequest({ url: server.url, path: path + '/avatar/pick', token: server.accessToken, fields, attaches })
     })
 
     it('Should fail with an unauthenticated user', async function () {
-      const fields = {}
-      const attaches = {
-        avatarfile: join(__dirname, '..', '..', 'fixtures', 'avatar.png')
+      for (const type of types) {
+        const fields = {}
+        const attaches = {
+          [type + 'file']: join(__dirname, '..', '..', 'fixtures', 'avatar.png')
+        }
+        await makeUploadRequest({
+          url: server.url,
+          path: `${path}/${type}/pick`,
+          fields,
+          attaches,
+          statusCodeExpected: HttpStatusCode.UNAUTHORIZED_401
+        })
       }
-      await makeUploadRequest({
-        url: server.url,
-        path: path + '/avatar/pick',
-        fields,
-        attaches,
-        statusCodeExpected: HttpStatusCode.UNAUTHORIZED_401
-      })
     })
 
     it('Should succeed with the correct params', async function () {
-      const fields = {}
-      const attaches = {
-        avatarfile: join(__dirname, '..', '..', 'fixtures', 'avatar.png')
+      for (const type of types) {
+        const fields = {}
+        const attaches = {
+          [type + 'file']: join(__dirname, '..', '..', 'fixtures', 'avatar.png')
+        }
+        await makeUploadRequest({
+          url: server.url,
+          path: `${path}/${type}/pick`,
+          token: server.accessToken,
+          fields,
+          attaches,
+          statusCodeExpected: HttpStatusCode.OK_200
+        })
       }
-      await makeUploadRequest({
-        url: server.url,
-        path: path + '/avatar/pick',
-        token: server.accessToken,
-        fields,
-        attaches,
-        statusCodeExpected: HttpStatusCode.OK_200
-      })
     })
   })
 

server/tests/api/videos/video-channels.ts

@@ -5,13 +5,14 @@ import * as chai from 'chai'
 import {
   cleanupTests,
   createUser,
+  deleteVideoChannelImage,
   doubleFollow,
   flushAndRunMultipleServers,
   getVideo,
   getVideoChannelVideos,
   testImage,
   updateVideo,
-  updateVideoChannelAvatar,
+  updateVideoChannelImage,
   uploadVideo,
   userLogin,
   wait
@@ -21,7 +22,6 @@ import {
   deleteVideoChannel,
   getAccountVideoChannelsList,
   getMyUserInformation,
-  getVideoChannel,
   getVideoChannelsList,
   ServerInfo,
   setAccessTokensToServers,
@@ -33,6 +33,13 @@ import { User, Video, VideoChannel, VideoDetails } from '../../../../shared/inde
 
 const expect = chai.expect
 
+async function findChannel (server: ServerInfo, channelId: number) {
+  const res = await getVideoChannelsList(server.url, 0, 5, '-name')
+  const videoChannel = res.body.data.find(c => c.id === channelId)
+
+  return videoChannel as VideoChannel
+}
+
 describe('Test video channels', function () {
   let servers: ServerInfo[]
   let userInfo: User
@@ -262,38 +269,85 @@ describe('Test video channels', function () {
   })
 
   it('Should update video channel avatar', async function () {
-    this.timeout(5000)
+    this.timeout(15000)
 
     const fixture = 'avatar.png'
 
-    await updateVideoChannelAvatar({
+    await updateVideoChannelImage({
       url: servers[0].url,
       accessToken: servers[0].accessToken,
       videoChannelName: 'second_video_channel',
-      fixture
+      fixture,
+      type: 'avatar'
     })
 
     await waitJobs(servers)
-  })
 
-  it('Should have video channel avatar updated', async function () {
     for (const server of servers) {
-      const res = await getVideoChannelsList(server.url, 0, 1, '-name')
-
-      const videoChannel = res.body.data.find(c => c.id === secondVideoChannelId)
+      const videoChannel = await findChannel(server, secondVideoChannelId)
 
       await testImage(server.url, 'avatar-resized', videoChannel.avatar.path, '.png')
     }
   })
 
-  it('Should get video channel', async function () {
-    const res = await getVideoChannel(servers[0].url, 'second_video_channel')
+  it('Should update video channel banner', async function () {
+    this.timeout(15000)
 
-    const videoChannel = res.body
-    expect(videoChannel.name).to.equal('second_video_channel')
-    expect(videoChannel.displayName).to.equal('video channel updated')
-    expect(videoChannel.description).to.equal('video channel description updated')
-    expect(videoChannel.support).to.equal('video channel support text updated')
+    const fixture = 'banner.jpg'
+
+    await updateVideoChannelImage({
+      url: servers[0].url,
+      accessToken: servers[0].accessToken,
+      videoChannelName: 'second_video_channel',
+      fixture,
+      type: 'banner'
+    })
+
+    await waitJobs(servers)
+
+    for (const server of servers) {
+      const videoChannel = await findChannel(server, secondVideoChannelId)
+
+      await testImage(server.url, 'banner-resized', videoChannel.banner.path)
+    }
+  })
+
+  it('Should delete the video channel avatar', async function () {
+    this.timeout(15000)
+
+    await deleteVideoChannelImage({
+      url: servers[0].url,
+      accessToken: servers[0].accessToken,
+      videoChannelName: 'second_video_channel',
+      type: 'avatar'
+    })
+
+    await waitJobs(servers)
+
+    for (const server of servers) {
+      const videoChannel = await findChannel(server, secondVideoChannelId)
+
+      expect(videoChannel.avatar).to.be.null
+    }
+  })
+
+  it('Should delete the video channel banner', async function () {
+    this.timeout(15000)
+
+    await deleteVideoChannelImage({
+      url: servers[0].url,
+      accessToken: servers[0].accessToken,
+      videoChannelName: 'second_video_channel',
+      type: 'banner'
+    })
+
+    await waitJobs(servers)
+
+    for (const server of servers) {
+      const videoChannel = await findChannel(server, secondVideoChannelId)
+
+      expect(videoChannel.banner).to.be.null
+    }
   })
 
   it('Should list the second video channel videos', async function () {

server/typings/express/index.d.ts

@@ -3,7 +3,6 @@ import {
   MAbuseMessage,
   MAbuseReporter,
   MAccountBlocklist,
-  MActorFollowActors,
   MActorFollowActorsDefault,
   MActorUrl,
   MChannelBannerAccountDefault,

shared/extra-utils/requests/requests.ts

@@ -152,11 +152,12 @@ function makeHTMLRequest (url: string, path: string) {
     .expect(HttpStatusCode.OK_200)
 }
 
-function updateAvatarRequest (options: {
+function updateImageRequest (options: {
   url: string
   path: string
   accessToken: string
   fixture: string
+  fieldname: string
 }) {
   let filePath = ''
   if (isAbsolute(options.fixture)) {
@@ -170,7 +171,7 @@ function updateAvatarRequest (options: {
     path: options.path,
     token: options.accessToken,
     fields: {},
-    attaches: { avatarfile: filePath },
+    attaches: { [options.fieldname]: filePath },
     statusCodeExpected: HttpStatusCode.OK_200
   })
 }
@@ -191,5 +192,5 @@ export {
   makePutBodyRequest,
   makeDeleteRequest,
   makeRawRequest,
-  updateAvatarRequest
+  updateImageRequest
 }

shared/extra-utils/users/users.ts

@@ -4,7 +4,7 @@ import { UserUpdateMe } from '../../models/users'
 import { UserAdminFlag } from '../../models/users/user-flag.model'
 import { UserRegister } from '../../models/users/user-register.model'
 import { UserRole } from '../../models/users/user-role'
-import { makeGetRequest, makePostBodyRequest, makePutBodyRequest, updateAvatarRequest } from '../requests/requests'
+import { makeGetRequest, makePostBodyRequest, makePutBodyRequest, updateImageRequest } from '../requests/requests'
 import { ServerInfo } from '../server/servers'
 import { userLogin } from './login'
 import { HttpStatusCode } from '../../../shared/core-utils/miscs/http-error-codes'
@@ -275,7 +275,7 @@ function updateMyAvatar (options: {
 }) {
   const path = '/api/v1/users/me/avatar/pick'
 
-  return updateAvatarRequest(Object.assign(options, { path }))
+  return updateImageRequest({ ...options, path, fieldname: 'avatarfile' })
 }
 
 function updateUser (options: {

shared/extra-utils/videos/video-channels.ts

@@ -3,7 +3,7 @@
 import * as request from 'supertest'
 import { VideoChannelUpdate } from '../../models/videos/channel/video-channel-update.model'
 import { VideoChannelCreate } from '../../models/videos/channel/video-channel-create.model'
-import { makeGetRequest, updateAvatarRequest } from '../requests/requests'
+import { makeDeleteRequest, makeGetRequest, updateImageRequest } from '../requests/requests'
 import { ServerInfo } from '../server/servers'
 import { User } from '../../models/users/user.model'
 import { getMyUserInformation } from '../users/users'
@@ -129,16 +129,32 @@ function getVideoChannel (url: string, channelName: string) {
     .expect('Content-Type', /json/)
 }
 
-function updateVideoChannelAvatar (options: {
+function updateVideoChannelImage (options: {
   url: string
   accessToken: string
   fixture: string
   videoChannelName: string | number
+  type: 'avatar' | 'banner'
 }) {
+  const path = `/api/v1/video-channels/${options.videoChannelName}/${options.type}/pick`
 
-  const path = '/api/v1/video-channels/' + options.videoChannelName + '/avatar/pick'
+  return updateImageRequest({ ...options, path, fieldname: options.type + 'file' })
+}
 
-  return updateAvatarRequest(Object.assign(options, { path }))
+function deleteVideoChannelImage (options: {
+  url: string
+  accessToken: string
+  videoChannelName: string | number
+  type: 'avatar' | 'banner'
+}) {
+  const path = `/api/v1/video-channels/${options.videoChannelName}/${options.type}`
+
+  return makeDeleteRequest({
+    url: options.url,
+    token: options.accessToken,
+    path,
+    statusCodeExpected: 204
+  })
 }
 
 function setDefaultVideoChannel (servers: ServerInfo[]) {
@@ -157,12 +173,13 @@ function setDefaultVideoChannel (servers: ServerInfo[]) {
 // ---------------------------------------------------------------------------
 
 export {
-  updateVideoChannelAvatar,
+  updateVideoChannelImage,
   getVideoChannelsList,
   getAccountVideoChannelsList,
   addVideoChannel,
   updateVideoChannel,
   deleteVideoChannel,
   getVideoChannel,
-  setDefaultVideoChannel
+  setDefaultVideoChannel,
+  deleteVideoChannelImage
 }