Add ability to a video multiple times in a playlist

7 files changed, 53 insertions, 40 deletions

diff --git a/server/controllers/activitypub/client.ts b/server/controllers/activitypub/client.ts
index acce53713..1da44d096 100644
--- a/server/controllers/activitypub/client.ts
+++ b/server/controllers/activitypub/client.ts
@@ -159,7 +159,7 @@ activityPubClientRouter.get('/video-playlists/:playlistId',
   asyncMiddleware(videoPlaylistsGetValidator('all')),
   asyncMiddleware(videoPlaylistController)
 )
-activityPubClientRouter.get('/video-playlists/:playlistId/:videoId',
+activityPubClientRouter.get('/video-playlists/:playlistId/videos/:playlistElementId',
   executeIfActivityPub,
   asyncMiddleware(videoPlaylistElementAPGetValidator),
   videoPlaylistElementController
diff --git a/server/controllers/api/video-playlist.ts b/server/controllers/api/video-playlist.ts
index 88a2314fb..41a0e07ff 100644
--- a/server/controllers/api/video-playlist.ts
+++ b/server/controllers/api/video-playlist.ts
@@ -297,7 +297,6 @@ async function addVideoInPlaylist (req: express.Request, res: express.Response)
     const position = await VideoPlaylistElementModel.getNextPositionOf(videoPlaylist.id, t)
 
     const playlistElement = await VideoPlaylistElementModel.create({
-      url: getVideoPlaylistElementActivityPubUrl(videoPlaylist, video),
       position,
       startTimestamp: body.startTimestamp || null,
       stopTimestamp: body.stopTimestamp || null,
@@ -305,6 +304,9 @@ async function addVideoInPlaylist (req: express.Request, res: express.Response)
       videoId: video.id
     }, { transaction: t })
 
+    playlistElement.url = getVideoPlaylistElementActivityPubUrl(videoPlaylist, playlistElement)
+    await playlistElement.save({ transaction: t })
+
     videoPlaylist.changed('updatedAt', true)
     await videoPlaylist.save({ transaction: t })
 
diff --git a/server/lib/activitypub/url.ts b/server/lib/activitypub/url.ts
index b54e038a4..58030be2c 100644
--- a/server/lib/activitypub/url.ts
+++ b/server/lib/activitypub/url.ts
@@ -8,7 +8,8 @@ import {
   MVideoId,
   MVideoUrl,
   MVideoUUID,
-  MAbuseId
+  MAbuseId,
+  MVideoPlaylistElement
 } from '../../types/models'
 import { MVideoPlaylist, MVideoPlaylistUUID } from '../../types/models/video/video-playlist'
 import { MVideoFileVideoUUID } from '../../types/models/video/video-file'
@@ -22,8 +23,8 @@ function getVideoPlaylistActivityPubUrl (videoPlaylist: MVideoPlaylist) {
   return WEBSERVER.URL + '/video-playlists/' + videoPlaylist.uuid
 }
 
-function getVideoPlaylistElementActivityPubUrl (videoPlaylist: MVideoPlaylistUUID, video: MVideoUUID) {
-  return WEBSERVER.URL + '/video-playlists/' + videoPlaylist.uuid + '/' + video.uuid
+function getVideoPlaylistElementActivityPubUrl (videoPlaylist: MVideoPlaylistUUID, videoPlaylistElement: MVideoPlaylistElement) {
+  return WEBSERVER.URL + '/video-playlists/' + videoPlaylist.uuid + '/videos/' + videoPlaylistElement.id
 }
 
 function getVideoCacheFileActivityPubUrl (videoFile: MVideoFileVideoUUID) {
diff --git a/server/middlewares/validators/videos/video-playlists.ts b/server/middlewares/validators/videos/video-playlists.ts
index 07fd8533c..4647eae44 100644
--- a/server/middlewares/validators/videos/video-playlists.ts
+++ b/server/middlewares/validators/videos/video-playlists.ts
@@ -199,16 +199,6 @@ const videoPlaylistsAddVideoValidator = [
     if (!await doesVideoExist(req.body.videoId, res, 'only-video')) return
 
     const videoPlaylist = getPlaylist(res)
-    const video = res.locals.onlyVideo
-
-    const videoPlaylistElement = await VideoPlaylistElementModel.loadByPlaylistAndVideo(videoPlaylist.id, video.id)
-    if (videoPlaylistElement) {
-      res.status(409)
-         .json({ error: 'This video in this playlist already exists' })
-         .end()
-
-      return
-    }
 
     if (!checkUserCanManageVideoPlaylist(res.locals.oauth.token.User, videoPlaylist, UserRight.UPDATE_ANY_VIDEO_PLAYLIST, res)) {
       return
@@ -258,15 +248,18 @@ const videoPlaylistsUpdateOrRemoveVideoValidator = [
 const videoPlaylistElementAPGetValidator = [
   param('playlistId')
     .custom(isIdOrUUIDValid).withMessage('Should have a valid playlist id/uuid'),
-  param('videoId')
-    .custom(isIdOrUUIDValid).withMessage('Should have an video id/uuid'),
+  param('playlistElementId')
+    .custom(isIdValid).withMessage('Should have an playlist element id'),
 
   async (req: express.Request, res: express.Response, next: express.NextFunction) => {
     logger.debug('Checking videoPlaylistElementAPGetValidator parameters', { parameters: req.params })
 
     if (areValidationErrors(req, res)) return
 
-    const videoPlaylistElement = await VideoPlaylistElementModel.loadByPlaylistAndVideoForAP(req.params.playlistId, req.params.videoId)
+    const playlistElementId = parseInt(req.params.playlistElementId + '', 10)
+    const playlistId = req.params.playlistId
+
+    const videoPlaylistElement = await VideoPlaylistElementModel.loadByPlaylistAndElementIdForAP(playlistId, playlistElementId)
     if (!videoPlaylistElement) {
       res.status(404)
          .json({ error: 'Video playlist element not found' })
diff --git a/server/models/video/video-playlist-element.ts b/server/models/video/video-playlist-element.ts
index ba92e129a..d357766e9 100644
--- a/server/models/video/video-playlist-element.ts
+++ b/server/models/video/video-playlist-element.ts
@@ -44,10 +44,6 @@ import { MUserAccountId } from '@server/types/models'
       fields: [ 'videoId' ]
     },
     {
-      fields: [ 'videoPlaylistId', 'videoId' ],
-      unique: true
-    },
-    {
       fields: [ 'url' ],
       unique: true
     }
@@ -60,8 +56,8 @@ export class VideoPlaylistElementModel extends Model<VideoPlaylistElementModel>
   @UpdatedAt
   updatedAt: Date
 
-  @AllowNull(false)
-  @Is('VideoPlaylistUrl', value => throwIfNotValid(value, isActivityPubUrlValid, 'url'))
+  @AllowNull(true)
+  @Is('VideoPlaylistUrl', value => throwIfNotValid(value, isActivityPubUrlValid, 'url', true))
   @Column(DataType.STRING(CONSTRAINTS_FIELDS.VIDEO_PLAYLISTS.URL.max))
   url: string
 
@@ -185,12 +181,11 @@ export class VideoPlaylistElementModel extends Model<VideoPlaylistElementModel>
     return VideoPlaylistElementModel.findByPk(playlistElementId)
   }
 
-  static loadByPlaylistAndVideoForAP (
+  static loadByPlaylistAndElementIdForAP (
     playlistId: number | string,
-    videoId: number | string
+    playlistElementId: number
   ): Bluebird<MVideoPlaylistElementVideoUrlPlaylistPrivacy> {
     const playlistWhere = validator.isUUID('' + playlistId) ? { uuid: playlistId } : { id: playlistId }
-    const videoWhere = validator.isUUID('' + videoId) ? { uuid: videoId } : { id: videoId }
 
     const query = {
       include: [
@@ -201,10 +196,12 @@ export class VideoPlaylistElementModel extends Model<VideoPlaylistElementModel>
         },
         {
           attributes: [ 'url' ],
-          model: VideoModel.unscoped(),
-          where: videoWhere
+          model: VideoModel.unscoped()
         }
-      ]
+      ],
+      where: {
+        id: playlistElementId
+      }
     }
 
     return VideoPlaylistElementModel.findOne(query)
diff --git a/server/tests/api/check-params/video-playlists.ts b/server/tests/api/check-params/video-playlists.ts
index 46ec00d46..179ae9201 100644
--- a/server/tests/api/check-params/video-playlists.ts
+++ b/server/tests/api/check-params/video-playlists.ts
@@ -346,11 +346,6 @@ describe('Test video playlists API validator', function () {
       const res = await addVideoInPlaylist(params)
       playlistElementId = res.body.videoPlaylistElement.id
     })
-
-    it('Should fail if the video was already added in the playlist', async function () {
-      const params = getBase({}, { expectedStatus: 409 })
-      await addVideoInPlaylist(params)
-    })
   })
 
   describe('When updating an element in a playlist', function () {
diff --git a/server/tests/api/videos/video-playlists.ts b/server/tests/api/videos/video-playlists.ts
index 52b32998d..0bfb5bcd4 100644
--- a/server/tests/api/videos/video-playlists.ts
+++ b/server/tests/api/videos/video-playlists.ts
@@ -552,6 +552,9 @@ describe('Test video playlists', function () {
       {
         const res = await addVideo({ videoId: nsfwVideoServer1, startTimestamp: 5 })
         playlistElementNSFW = res.body.videoPlaylistElement.id
+
+        await addVideo({ videoId: nsfwVideoServer1, startTimestamp: 4 })
+        await addVideo({ videoId: nsfwVideoServer1 })
       }
 
       await waitJobs(servers)
@@ -563,10 +566,10 @@ describe('Test video playlists', function () {
       for (const server of servers) {
         const res = await getPlaylistVideos(server.url, server.accessToken, playlistServer1UUID, 0, 10)
 
-        expect(res.body.total).to.equal(6)
+        expect(res.body.total).to.equal(8)
 
         const videoElements: VideoPlaylistElement[] = res.body.data
-        expect(videoElements).to.have.lengthOf(6)
+        expect(videoElements).to.have.lengthOf(8)
 
         expect(videoElements[0].video.name).to.equal('video 0 server 1')
         expect(videoElements[0].position).to.equal(1)
@@ -598,6 +601,16 @@ describe('Test video playlists', function () {
         expect(videoElements[5].startTimestamp).to.equal(5)
         expect(videoElements[5].stopTimestamp).to.be.null
 
+        expect(videoElements[6].video.name).to.equal('NSFW video')
+        expect(videoElements[6].position).to.equal(7)
+        expect(videoElements[6].startTimestamp).to.equal(4)
+        expect(videoElements[6].stopTimestamp).to.be.null
+
+        expect(videoElements[7].video.name).to.equal('NSFW video')
+        expect(videoElements[7].position).to.equal(8)
+        expect(videoElements[7].startTimestamp).to.be.null
+        expect(videoElements[7].stopTimestamp).to.be.null
+
         const res3 = await getPlaylistVideos(server.url, server.accessToken, playlistServer1UUID, 0, 2)
         expect(res3.body.data).to.have.lengthOf(2)
       }
@@ -807,6 +820,8 @@ describe('Test video playlists', function () {
             'video 1 server 3',
             'video 3 server 1',
             'video 4 server 1',
+            'NSFW video',
+            'NSFW video',
             'NSFW video'
           ])
         }
@@ -836,6 +851,8 @@ describe('Test video playlists', function () {
             'video 2 server 3',
             'video 1 server 3',
             'video 4 server 1',
+            'NSFW video',
+            'NSFW video',
             'NSFW video'
           ])
         }
@@ -865,7 +882,9 @@ describe('Test video playlists', function () {
             'video 2 server 3',
             'NSFW video',
             'video 1 server 3',
-            'video 4 server 1'
+            'video 4 server 1',
+            'NSFW video',
+            'NSFW video'
           ])
 
           for (let i = 1; i <= elements.length; i++) {
@@ -1023,10 +1042,10 @@ describe('Test video playlists', function () {
       for (const server of servers) {
         const res = await getPlaylistVideos(server.url, server.accessToken, playlistServer1UUID, 0, 10)
 
-        expect(res.body.total).to.equal(4)
+        expect(res.body.total).to.equal(6)
 
         const elements: VideoPlaylistElement[] = res.body.data
-        expect(elements).to.have.lengthOf(4)
+        expect(elements).to.have.lengthOf(6)
 
         expect(elements[0].video.name).to.equal('video 0 server 1')
         expect(elements[0].position).to.equal(1)
@@ -1039,6 +1058,12 @@ describe('Test video playlists', function () {
 
         expect(elements[3].video.name).to.equal('video 4 server 1')
         expect(elements[3].position).to.equal(4)
+
+        expect(elements[4].video.name).to.equal('NSFW video')
+        expect(elements[4].position).to.equal(5)
+
+        expect(elements[5].video.name).to.equal('NSFW video')
+        expect(elements[5].position).to.equal(6)
       }
     })