Better avatar order in AP representation

Remote federated software usually use the first item, and so 48x48 is
too small. Prefer 120x120 instead

client/e2e/src/suites-all/videos.e2e-spec.ts

@@ -2,8 +2,8 @@ import { LoginPage } from '../po/login.po'
 import { MyAccountPage } from '../po/my-account.po'
 import { PlayerPage } from '../po/player.po'
 import { VideoListPage } from '../po/video-list.po'
-import { VideoUpdatePage } from '../po/video-update.po'
 import { VideoPublishPage } from '../po/video-publish.po'
+import { VideoUpdatePage } from '../po/video-update.po'
 import { VideoWatchPage } from '../po/video-watch.po'
 import { FIXTURE_URLS, go, isIOS, isMobileDevice, isSafari, waitServerUp } from '../utils'
 

packages/tests/src/api/activitypub/client.ts

@@ -1,5 +1,6 @@
 /* eslint-disable @typescript-eslint/no-unused-expressions,@typescript-eslint/require-await */
 
+import { arrayify } from '@peertube/peertube-core-utils'
 import {
   ActivityPubActor,
   HttpStatusCode,
@@ -15,6 +16,8 @@ import {
   makeActivityPubGetRequest,
   PeerTubeServer,
   setAccessTokensToServers,
+  setDefaultAccountAvatar,
+  setDefaultChannelAvatar,
   setDefaultVideoChannel
 } from '@peertube/peertube-server-commands'
 import { processViewersStats } from '@tests/shared/views.js'
@@ -26,7 +29,7 @@ describe('Test ActivityPub', function () {
   let playlist: { id: number, uuid: string, shortUUID: string }
   let comment: VideoComment
 
-  async function testAccount (path: string) {
+  async function testAccount (path: string, hasIcon: boolean) {
     const res = await makeActivityPubGetRequest(servers[0].url, path)
     const object = res.body as ActivityPubActor
 
@@ -35,6 +38,12 @@ describe('Test ActivityPub', function () {
     expect(object.name).to.equal('root')
     expect(object.preferredUsername).to.equal('root')
 
+    if (hasIcon) {
+      expect(arrayify(object.icon).map(i => i.width)).to.deep.equal([ 120, 48, 600, 1500 ])
+    } else {
+      expect(object.icon).to.not.exist
+    }
+
     // TODO: enable in v8
     // const htmlURLs = [
     //   servers[0].url + '/accounts/root',
@@ -47,7 +56,7 @@ describe('Test ActivityPub', function () {
     // }
   }
 
-  async function testChannel (path: string) {
+  async function testChannel (path: string, hasIcon: boolean) {
     const res = await makeActivityPubGetRequest(servers[0].url, path)
     const object = res.body as ActivityPubActor
 
@@ -56,6 +65,12 @@ describe('Test ActivityPub', function () {
     expect(object.name).to.equal('Main root channel')
     expect(object.preferredUsername).to.equal('root_channel')
 
+    if (hasIcon) {
+      expect(arrayify(object.icon).map(i => i.width)).to.deep.equal([ 120, 48, 600, 1500 ])
+    } else {
+      expect(object.icon).to.not.exist
+    }
+
     // TODO: enable in v8
     // const htmlURLs = [
     //   servers[0].url + '/video-channels/root_channel',
@@ -130,13 +145,21 @@ describe('Test ActivityPub', function () {
   })
 
   it('Should return the account object', async function () {
-    await testAccount('/accounts/root')
-    await testAccount('/a/root')
+    await testAccount('/accounts/root', false)
+    await testAccount('/a/root', false)
   })
 
   it('Should return the channel object', async function () {
-    await testChannel('/video-channels/root_channel')
-    await testChannel('/c/root_channel')
+    await testChannel('/video-channels/root_channel', false)
+    await testChannel('/c/root_channel', false)
+  })
+
+  it('Should return account & channels with icons', async function () {
+    await setDefaultAccountAvatar(servers)
+    await setDefaultChannelAvatar(servers)
+
+    await testAccount('/a/root', true)
+    await testChannel('/c/root_channel', true)
   })
 
   it('Should return the video comment object', async function () {

server/core/models/actor/actor.ts

@@ -1,10 +1,5 @@
 import { forceNumber, maxBy } from '@peertube/peertube-core-utils'
-import {
-  ActivityIconObject,
-  ActorImageType,
-  ActorImageType_Type,
-  type ActivityPubActorType
-} from '@peertube/peertube-models'
+import { ActivityIconObject, ActorImageType, ActorImageType_Type, type ActivityPubActorType } from '@peertube/peertube-models'
 import { AttributesOnly } from '@peertube/peertube-typescript-utils'
 import { activityPubContextify } from '@server/helpers/activity-pub-utils.js'
 import { getContextFilter } from '@server/lib/activitypub/context.js'
@@ -34,13 +29,7 @@ import {
   isActorPublicKeyValid
 } from '../../helpers/custom-validators/activitypub/actor.js'
 import { isActivityPubUrlValid } from '../../helpers/custom-validators/activitypub/misc.js'
-import {
-  ACTIVITY_PUB,
-  ACTIVITY_PUB_ACTOR_TYPES,
-  CONSTRAINTS_FIELDS,
-  SERVER_ACTOR_NAME,
-  WEBSERVER
-} from '../../initializers/constants.js'
+import { ACTIVITY_PUB, ACTIVITY_PUB_ACTOR_TYPES, CONSTRAINTS_FIELDS, SERVER_ACTOR_NAME, WEBSERVER } from '../../initializers/constants.js'
 import {
   MActor,
   MActorAPAccount,
@@ -574,7 +563,13 @@ export class ActorModel extends SequelizeModel<ActorModel> {
     let image: ActivityIconObject[] // Banners
 
     if (this.hasImage(ActorImageType.AVATAR)) {
-      icon = this.Avatars.map(a => a.toActivityPubObject())
+      let avatars = this.Avatars
+
+      // Use 120px avatar as first position if possible, so that remote servers use it in priority (instead of using 48x48px)
+      const avatar120Px = avatars.find(a => a.width === 120)
+      if (avatar120Px) avatars = [ avatar120Px, ...avatars.filter(a => a.width !== 120) ]
+
+      icon = avatars.map(a => a.toActivityPubObject())
     }
 
     if (this.hasImage(ActorImageType.BANNER)) {