Fix playlist elements merge on import

packages/server-commands/src/videos/playlists-command.ts

@@ -14,6 +14,7 @@ import {
   VideoPlaylistPrivacy,
   VideoPlaylistPrivacyType,
   VideoPlaylistReorder,
+  VideoPlaylistType,
   VideoPlaylistType_Type,
   VideoPlaylistUpdate
 } from '@peertube/peertube-models'
@@ -82,6 +83,8 @@ export class PlaylistsCommand extends AbstractCommand {
     })
   }
 
+  // ---------------------------------------------------------------------------
+
   get (options: OverrideCommandOptions & {
     playlistId: number | string
   }) {
@@ -97,6 +100,20 @@ export class PlaylistsCommand extends AbstractCommand {
     })
   }
 
+  async getWatchLater (options: OverrideCommandOptions & {
+    handle: string
+  }) {
+    const { data: playlists } = await this.listByAccount({
+      ...options,
+
+      playlistType: VideoPlaylistType.WATCH_LATER
+    })
+
+    return playlists[0]
+  }
+
+  // ---------------------------------------------------------------------------
+
   listVideos (options: OverrideCommandOptions & {
     playlistId: number | string
     start?: number

packages/tests/src/api/users/user-import.ts

@@ -97,17 +97,16 @@ function runTest (withObjectStorage: boolean) {
     })
 
     // Add a video in watch later playlist
-    const { data: playlists } = await server.playlists.listByAccount({
-      token: noahToken,
-      handle: 'noah',
-      playlistType: VideoPlaylistType.WATCH_LATER
-    })
-
     await server.playlists.addElement({
-      playlistId: playlists[0].id,
+      playlistId: (await server.playlists.getWatchLater({ token: noahToken, handle: 'noah' })).id,
       attributes: { videoId: noahVideo.uuid }
     })
 
+    await remoteServer.playlists.addElement({
+      playlistId: (await remoteServer.playlists.getWatchLater({ token: remoteNoahToken, handle: 'noah_remote' })).id,
+      attributes: { videoId: mouskaVideo.uuid }
+    })
+
     await waitJobs([ server, remoteServer, blockedServer ])
 
     // ---------------------------------------------------------------------------
@@ -285,11 +284,15 @@ function runTest (withObjectStorage: boolean) {
       expect(watchLater.privacy.id).to.equal(VideoPlaylistPrivacy.PRIVATE)
 
       // Playlists were merged
-      expect(watchLater.videosLength).to.equal(1)
+      expect(watchLater.videosLength).to.equal(2)
 
       const { data: videos } = await remoteServer.playlists.listVideos({ playlistId: watchLater.id, token: remoteNoahToken })
+
       expect(videos[0].position).to.equal(1)
-      expect(videos[0].video.uuid).to.equal(noahVideo.uuid)
+      // Mouska is muted
+      expect(videos[0].video).to.not.exist
+      expect(videos[1].position).to.equal(2)
+      expect(videos[1].video.uuid).to.equal(noahVideo.uuid)
 
       // Not federated
       await server.playlists.get({ playlistId: watchLater.uuid, expectedStatus: HttpStatusCode.NOT_FOUND_404 })

server/core/controllers/api/video-playlist.ts

@@ -13,10 +13,9 @@ import {
   VideoPlaylistUpdate
 } from '@peertube/peertube-models'
 import { scheduleRefreshIfNeeded } from '@server/lib/activitypub/playlists/index.js'
-import { VideoMiniaturePermanentFileCache } from '@server/lib/files-cache/index.js'
 import { Hooks } from '@server/lib/plugins/hooks.js'
 import { getServerActor } from '@server/models/application/application.js'
-import { MVideoPlaylistFull, MVideoPlaylistThumbnail, MVideoThumbnail } from '@server/types/models/index.js'
+import { MVideoPlaylistFull, MVideoPlaylistThumbnail } from '@server/types/models/index.js'
 import { uuidToShort } from '@peertube/peertube-node-utils'
 import { resetSequelizeInstance } from '../../helpers/database-utils.js'
 import { createReqFiles } from '../../helpers/express-utils.js'
@@ -51,6 +50,7 @@ import {
 import { AccountModel } from '../../models/account/account.js'
 import { VideoPlaylistElementModel } from '../../models/video/video-playlist-element.js'
 import { VideoPlaylistModel } from '../../models/video/video-playlist.js'
+import { generateThumbnailForPlaylist } from '@server/lib/video-playlist.js'
 
 const reqThumbnailFile = createReqFiles([ 'thumbnailfile' ], MIMETYPES.IMAGE.MIMETYPE_EXT)
 
@@ -329,7 +329,7 @@ async function addVideoInPlaylist (req: express.Request, res: express.Response)
   })
 
   // If the user did not set a thumbnail, automatically take the video thumbnail
-  if (videoPlaylist.hasThumbnail() === false || (videoPlaylist.hasGeneratedThumbnail() && playlistElement.position === 1)) {
+  if (videoPlaylist.shouldGenerateThumbnailWithNewElement(playlistElement)) {
     await generateThumbnailForPlaylist(videoPlaylist, video)
   }
 
@@ -488,30 +488,3 @@ async function regeneratePlaylistThumbnail (videoPlaylist: MVideoPlaylistThumbna
   const firstElement = await VideoPlaylistElementModel.loadFirstElementWithVideoThumbnail(videoPlaylist.id)
   if (firstElement) await generateThumbnailForPlaylist(videoPlaylist, firstElement.Video)
 }
-
-async function generateThumbnailForPlaylist (videoPlaylist: MVideoPlaylistThumbnail, video: MVideoThumbnail) {
-  logger.info('Generating default thumbnail to playlist %s.', videoPlaylist.url)
-
-  const videoMiniature = video.getMiniature()
-  if (!videoMiniature) {
-    logger.info('Cannot generate thumbnail for playlist %s because video %s does not have any.', videoPlaylist.url, video.url)
-    return
-  }
-
-  // Ensure the file is on disk
-  const videoMiniaturePermanentFileCache = new VideoMiniaturePermanentFileCache()
-  const inputPath = videoMiniature.isOwned()
-    ? videoMiniature.getPath()
-    : await videoMiniaturePermanentFileCache.downloadRemoteFile(videoMiniature)
-
-  const thumbnailModel = await updateLocalPlaylistMiniatureFromExisting({
-    inputPath,
-    playlist: videoPlaylist,
-    automaticallyGenerated: true,
-    keepOriginal: true
-  })
-
-  thumbnailModel.videoPlaylistId = videoPlaylist.id
-
-  videoPlaylist.Thumbnail = await thumbnailModel.save()
-}

server/core/lib/user-import-export/importers/video-playlists-importer.ts

@@ -2,8 +2,7 @@ import { VideoPlaylistPrivacy, VideoPlaylistType, VideoPlaylistsExportJSON } fro
 import { logger, loggerTagsFactory } from '@server/helpers/logger.js'
 import { buildUUID } from '@peertube/peertube-node-utils'
 import {
-  MChannelBannerAccountDefault, MVideoPlaylist,
-  MVideoPlaylistFull,
+  MChannelBannerAccountDefault, MVideoPlaylistFull,
   MVideoPlaylistThumbnail
 } from '@server/types/models/index.js'
 import { getLocalVideoPlaylistActivityPubUrl, getLocalVideoPlaylistElementActivityPubUrl } from '@server/lib/activitypub/url.js'
@@ -28,6 +27,8 @@ import { saveInTransactionWithRetries } from '@server/helpers/database-utils.js'
 import { isArray } from '@server/helpers/custom-validators/misc.js'
 import { isUrlValid } from '@server/helpers/custom-validators/activitypub/misc.js'
 import { pick } from '@peertube/peertube-core-utils'
+import { VideoModel } from '@server/models/video/video.js'
+import { generateThumbnailForPlaylist } from '@server/lib/video-playlist.js'
 
 const lTags = loggerTagsFactory('user-import')
 
@@ -135,7 +136,7 @@ export class VideoPlaylistsImporter extends AbstractUserImporter <VideoPlaylists
     await playlist.setAndSaveThumbnail(thumbnail, undefined)
   }
 
-  private async createElements (playlist: MVideoPlaylist, playlistImportData: SanitizedObject) {
+  private async createElements (playlist: MVideoPlaylistThumbnail, playlistImportData: SanitizedObject) {
     const elementsToCreate: { videoId: number, startTimestamp: number, stopTimestamp: number }[] = []
 
     for (const element of playlistImportData.elements.slice(0, USER_IMPORT.MAX_PLAYLIST_ELEMENTS)) {
@@ -154,9 +155,9 @@ export class VideoPlaylistsImporter extends AbstractUserImporter <VideoPlaylists
     }
 
     await sequelizeTypescript.transaction(async t => {
-      for (let position = 1; position <= elementsToCreate.length; position++) {
-        const elementToCreate = elementsToCreate[position - 1]
+      let position = await VideoPlaylistElementModel.getNextPositionOf(playlist.id, t)
 
+      for (const elementToCreate of elementsToCreate) {
         const playlistElement = new VideoPlaylistElementModel({
           position,
           startTimestamp: elementToCreate.startTimestamp,
@@ -168,6 +169,15 @@ export class VideoPlaylistsImporter extends AbstractUserImporter <VideoPlaylists
 
         playlistElement.url = getLocalVideoPlaylistElementActivityPubUrl(playlist, playlistElement)
         await playlistElement.save({ transaction: t })
+
+        if (playlist.shouldGenerateThumbnailWithNewElement(playlistElement)) {
+          const video = await VideoModel.loadFull(elementToCreate.videoId)
+
+          generateThumbnailForPlaylist(playlist, video)
+            .catch(err => logger.error('Cannot generate thumbnail from playlist', { err }))
+        }
+
+        position++
       }
     })
   }

server/core/lib/video-playlist.ts

@@ -1,11 +1,14 @@
 import * as Sequelize from 'sequelize'
 import { VideoPlaylistPrivacy, VideoPlaylistType } from '@peertube/peertube-models'
 import { VideoPlaylistModel } from '../models/video/video-playlist.js'
-import { MAccount } from '../types/models/index.js'
-import { MVideoPlaylistOwner } from '../types/models/video/video-playlist.js'
+import { MAccount, MVideoThumbnail } from '../types/models/index.js'
+import { MVideoPlaylistOwner, MVideoPlaylistThumbnail } from '../types/models/video/video-playlist.js'
 import { getLocalVideoPlaylistActivityPubUrl } from './activitypub/url.js'
+import { VideoMiniaturePermanentFileCache } from './files-cache/video-miniature-permanent-file-cache.js'
+import { updateLocalPlaylistMiniatureFromExisting } from './thumbnail.js'
+import { logger } from '@server/helpers/logger.js'
 
-async function createWatchLaterPlaylist (account: MAccount, t: Sequelize.Transaction) {
+export async function createWatchLaterPlaylist (account: MAccount, t: Sequelize.Transaction) {
   const videoPlaylist: MVideoPlaylistOwner = new VideoPlaylistModel({
     name: 'Watch later',
     privacy: VideoPlaylistPrivacy.PRIVATE,
@@ -22,8 +25,29 @@ async function createWatchLaterPlaylist (account: MAccount, t: Sequelize.Transac
   return videoPlaylist
 }
 
-// ---------------------------------------------------------------------------
+export async function generateThumbnailForPlaylist (videoPlaylist: MVideoPlaylistThumbnail, video: MVideoThumbnail) {
+  logger.info('Generating default thumbnail to playlist %s.', videoPlaylist.url)
 
-export {
-  createWatchLaterPlaylist
+  const videoMiniature = video.getMiniature()
+  if (!videoMiniature) {
+    logger.info('Cannot generate thumbnail for playlist %s because video %s does not have any.', videoPlaylist.url, video.url)
+    return
+  }
+
+  // Ensure the file is on disk
+  const videoMiniaturePermanentFileCache = new VideoMiniaturePermanentFileCache()
+  const inputPath = videoMiniature.isOwned()
+    ? videoMiniature.getPath()
+    : await videoMiniaturePermanentFileCache.downloadRemoteFile(videoMiniature)
+
+  const thumbnailModel = await updateLocalPlaylistMiniatureFromExisting({
+    inputPath,
+    playlist: videoPlaylist,
+    automaticallyGenerated: true,
+    keepOriginal: true
+  })
+
+  thumbnailModel.videoPlaylistId = videoPlaylist.id
+
+  videoPlaylist.Thumbnail = await thumbnailModel.save()
 }

server/core/models/video/video-playlist.ts

@@ -10,7 +10,7 @@ import {
 } from '@peertube/peertube-models'
 import { buildUUID, uuidToShort } from '@peertube/peertube-node-utils'
 import { activityPubCollectionPagination } from '@server/lib/activitypub/collection.js'
-import { MAccountId, MChannelId } from '@server/types/models/index.js'
+import { MAccountId, MChannelId, MVideoPlaylistElement } from '@server/types/models/index.js'
 import { join } from 'path'
 import { FindOptions, Includeable, literal, Op, ScopeOptions, Sequelize, Transaction, WhereOptions } from 'sequelize'
 import {
@@ -629,6 +629,13 @@ export class VideoPlaylistModel extends SequelizeModel<VideoPlaylistModel> {
     return this.hasThumbnail() && this.Thumbnail.automaticallyGenerated === true
   }
 
+  shouldGenerateThumbnailWithNewElement (newElement: MVideoPlaylistElement) {
+    if (this.hasThumbnail() === false) return true
+    if (newElement.position === 1 && this.hasGeneratedThumbnail()) return true
+
+    return false
+  }
+
   generateThumbnailName () {
     const extension = '.jpg'