Add banner tests

15 files changed, 181 insertions, 114 deletions

diff --git a/server/controllers/api/users/me.ts b/server/controllers/api/users/me.ts
index 25a18caa5..9f9d2d77f 100644
--- a/server/controllers/api/users/me.ts
+++ b/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'
diff --git a/server/controllers/api/video-channel.ts b/server/controllers/api/video-channel.ts
index 1c926722d..149d6cfb4 100644
--- a/server/controllers/api/video-channel.ts
+++ b/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'
diff --git a/server/helpers/custom-validators/actor-images.ts b/server/helpers/custom-validators/actor-images.ts
new file mode 100644
index 000000000..4fb0b7c70
--- /dev/null
+++ b/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
+}
diff --git a/server/helpers/custom-validators/users.ts b/server/helpers/custom-validators/users.ts
index 85f3634c8..5b21c3529 100644
--- a/server/helpers/custom-validators/users.ts
+++ b/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
 }
diff --git a/server/lib/activitypub/actor.ts b/server/lib/activitypub/actor.ts
index fe4796a3d..917fed6ec 100644
--- a/server/lib/activitypub/actor.ts
+++ b/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,
diff --git a/server/lib/activitypub/process/process-update.ts b/server/lib/activitypub/process/process-update.ts
index ad3bb392d..6df9b93b2 100644
--- a/server/lib/activitypub/process/process-update.ts
+++ b/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 })
 
diff --git a/server/lib/actor-image.ts b/server/lib/actor-image.ts
index 59afa93bd..fa1a2a18a 100644
--- a/server/lib/actor-image.ts
+++ b/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)
diff --git a/server/middlewares/validators/avatar.ts b/server/middlewares/validators/actor-image.ts
index f7eb367bd..961d7a7e5 100644
--- a/server/middlewares/validators/avatar.ts
+++ b/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(', ')
   ),
diff --git a/server/middlewares/validators/index.ts b/server/middlewares/validators/index.ts
index 4086d77aa..24faeea3e 100644
--- a/server/middlewares/validators/index.ts
+++ b/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'
diff --git a/server/models/video/video-channel.ts b/server/models/video/video-channel.ts
index 74885edfb..d2a055f5b 100644
--- a/server/models/video/video-channel.ts
+++ b/server/models/video/video-channel.ts
@@ -99,7 +99,14 @@ export type SummaryOptions = {
                 }
               }
             ]
-          }
+          },
+          include: [
+            {
+              model: ActorImageModel,
+              as: 'Banner',
+              required: false
+            }
+          ]
         },
         {
           model: AccountModel,
diff --git a/server/tests/api/check-params/video-channels.ts b/server/tests/api/check-params/video-channels.ts
index 0dd436426..bc2e6192e 100644
--- a/server/tests/api/check-params/video-channels.ts
+++ b/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
-      })
     })
   })
 
diff --git a/server/tests/api/videos/video-channels.ts b/server/tests/api/videos/video-channels.ts
index 367f99fdd..8033b9ba5 100644
--- a/server/tests/api/videos/video-channels.ts
+++ b/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)
+
+    for (const server of servers) {
+      const videoChannel = await findChannel(server, secondVideoChannelId)
+
+      await testImage(server.url, 'avatar-resized', videoChannel.avatar.path, '.png')
+    }
   })
 
-  it('Should have video channel avatar updated', async function () {
+  it('Should update video channel banner', async function () {
+    this.timeout(15000)
+
+    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 res = await getVideoChannelsList(server.url, 0, 1, '-name')
+      const videoChannel = await findChannel(server, secondVideoChannelId)
 
-      const videoChannel = res.body.data.find(c => c.id === secondVideoChannelId)
+      await testImage(server.url, 'banner-resized', videoChannel.banner.path)
+    }
+  })
 
-      await testImage(server.url, 'avatar-resized', videoChannel.avatar.path, '.png')
+  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 get video channel', async function () {
-    const res = await getVideoChannel(servers[0].url, 'second_video_channel')
+  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'
+    })
 
-    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')
+    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 () {
diff --git a/server/tests/fixtures/banner-resized.jpg b/server/tests/fixtures/banner-resized.jpg
new file mode 100644
index 000000000..13ea422cb
--- /dev/null
+++ b/server/tests/fixtures/banner-resized.jpg
diff --git a/server/tests/fixtures/banner.jpg b/server/tests/fixtures/banner.jpg
new file mode 100644
index 000000000..e5f284f59
--- /dev/null
+++ b/server/tests/fixtures/banner.jpg
diff --git a/server/typings/express/index.d.ts b/server/typings/express/index.d.ts
index ee4faa35d..cf3e7ae34 100644
--- a/server/typings/express/index.d.ts
+++ b/server/typings/express/index.d.ts
@@ -3,7 +3,6 @@ import {
   MAbuseMessage,
   MAbuseReporter,
   MAccountBlocklist,
-  MActorFollowActors,
   MActorFollowActorsDefault,
   MActorUrl,
   MChannelBannerAccountDefault,