From 371906639ee9b6ea4daae504bc7c2b15856c3f38 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Mon, 17 Aug 2020 16:39:32 +0200 Subject: Add ability to a video multiple times in a playlist --- server/controllers/activitypub/client.ts | 2 +- server/controllers/api/video-playlist.ts | 4 ++- server/lib/activitypub/url.ts | 7 +++-- .../validators/videos/video-playlists.ts | 19 ++++-------- server/models/video/video-playlist-element.ts | 21 ++++++------- server/tests/api/check-params/video-playlists.ts | 5 ---- server/tests/api/videos/video-playlists.ts | 35 ++++++++++++++++++---- 7 files changed, 53 insertions(+), 40 deletions(-) (limited to 'server') 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 @@ -43,10 +43,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 @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 return VideoPlaylistElementModel.findByPk(playlistElementId) } - static loadByPlaylistAndVideoForAP ( + static loadByPlaylistAndElementIdForAP ( playlistId: number | string, - videoId: number | string + playlistElementId: number ): Bluebird { 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 }, { 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) } }) -- cgit v1.2.3