From 2a4c0d8bbe29178ae90e776bb9453f86e6d23bd9 Mon Sep 17 00:00:00 2001 From: Wicklow <123956049+wickloww@users.noreply.github.com> Date: Wed, 12 Apr 2023 07:32:20 +0000 Subject: Feature/filter already watched videos (#5739) * filter already watched videos * Updated code based on review comments --- server/helpers/query.ts | 6 ++-- server/middlewares/validators/videos/videos.ts | 11 ++++++++ .../sql/video/videos-id-list-query-builder.ts | 22 +++++++++++++++ server/models/video/video.ts | 10 +++++-- .../api/check-params/videos-common-filters.ts | 19 +++++++++++-- server/tests/api/videos/videos-common-filters.ts | 32 +++++++++++++++++++++- 6 files changed, 93 insertions(+), 7 deletions(-) (limited to 'server') diff --git a/server/helpers/query.ts b/server/helpers/query.ts index 1142d02e4..10efae41c 100644 --- a/server/helpers/query.ts +++ b/server/helpers/query.ts @@ -24,7 +24,8 @@ function pickCommonVideoQuery (query: VideosCommonQueryAfterSanitize) { 'skipCount', 'hasHLSFiles', 'hasWebtorrentFiles', - 'search' + 'search', + 'excludeAlreadyWatched' ]) } @@ -41,7 +42,8 @@ function pickSearchVideoQuery (query: VideosSearchQueryAfterSanitize) { 'originallyPublishedEndDate', 'durationMin', 'durationMax', - 'uuids' + 'uuids', + 'excludeAlreadyWatched' ]) } } diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts index e29eb4a32..ea6bd0721 100644 --- a/server/middlewares/validators/videos/videos.ts +++ b/server/middlewares/validators/videos/videos.ts @@ -489,6 +489,10 @@ const commonVideosFiltersValidator = [ query('search') .optional() .custom(exists), + query('excludeAlreadyWatched') + .optional() + .customSanitizer(toBooleanOrNull) + .isBoolean().withMessage('Should be a valid excludeAlreadyWatched boolean'), (req: express.Request, res: express.Response, next: express.NextFunction) => { if (areValidationErrors(req, res)) return @@ -520,6 +524,13 @@ const commonVideosFiltersValidator = [ } } + if (!user && exists(req.query.excludeAlreadyWatched)) { + res.fail({ + status: HttpStatusCode.BAD_REQUEST_400, + message: 'Cannot use excludeAlreadyWatched parameter when auth token is not provided' + }) + return false + } return next() } ] diff --git a/server/models/video/sql/video/videos-id-list-query-builder.ts b/server/models/video/sql/video/videos-id-list-query-builder.ts index 62f1855c7..cba77c1d1 100644 --- a/server/models/video/sql/video/videos-id-list-query-builder.ts +++ b/server/models/video/sql/video/videos-id-list-query-builder.ts @@ -78,6 +78,8 @@ export type BuildVideosListQueryOptions = { transaction?: Transaction logging?: boolean + + excludeAlreadyWatched?: boolean } export class VideosIdListQueryBuilder extends AbstractRunQuery { @@ -260,6 +262,14 @@ export class VideosIdListQueryBuilder extends AbstractRunQuery { this.whereDurationMax(options.durationMax) } + if (options.excludeAlreadyWatched) { + if (exists(options.user.id)) { + this.whereExcludeAlreadyWatched(options.user.id) + } else { + throw new Error('Cannot use excludeAlreadyWatched parameter when auth token is not provided') + } + } + this.whereSearch(options.search) if (options.isCount === true) { @@ -598,6 +608,18 @@ export class VideosIdListQueryBuilder extends AbstractRunQuery { this.replacements.durationMax = durationMax } + private whereExcludeAlreadyWatched (userId: number) { + this.and.push( + 'NOT EXISTS (' + + ' SELECT 1' + + ' FROM "userVideoHistory"' + + ' WHERE "video"."id" = "userVideoHistory"."videoId"' + + ' AND "userVideoHistory"."userId" = :excludeAlreadyWatchedUserId' + + ')' + ) + this.replacements.excludeAlreadyWatchedUserId = userId + } + private groupForTrending (trendingDays: number) { const viewsGteDate = new Date(new Date().getTime() - (24 * 3600 * 1000) * trendingDays) diff --git a/server/models/video/video.ts b/server/models/video/video.ts index 0c5ed64ec..f817c4a33 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -1086,6 +1086,8 @@ export class VideoModel extends Model>> { countVideos?: boolean search?: string + + excludeAlreadyWatched?: boolean }) { VideoModel.throwIfPrivateIncludeWithoutUser(options.include, options.user) VideoModel.throwIfPrivacyOneOfWithoutUser(options.privacyOneOf, options.user) @@ -1124,7 +1126,8 @@ export class VideoModel extends Model>> { 'historyOfUser', 'hasHLSFiles', 'hasWebtorrentFiles', - 'search' + 'search', + 'excludeAlreadyWatched' ]), serverAccountIdForBlock: serverActor.Account.id, @@ -1170,6 +1173,8 @@ export class VideoModel extends Model>> { durationMin?: number // seconds durationMax?: number // seconds uuids?: string[] + + excludeAlreadyWatched?: boolean }) { VideoModel.throwIfPrivateIncludeWithoutUser(options.include, options.user) VideoModel.throwIfPrivacyOneOfWithoutUser(options.privacyOneOf, options.user) @@ -1203,7 +1208,8 @@ export class VideoModel extends Model>> { 'hasWebtorrentFiles', 'uuids', 'search', - 'displayOnlyForFollower' + 'displayOnlyForFollower', + 'excludeAlreadyWatched' ]), serverAccountIdForBlock: serverActor.Account.id } diff --git a/server/tests/api/check-params/videos-common-filters.ts b/server/tests/api/check-params/videos-common-filters.ts index 11d9fd95b..3e44e2f67 100644 --- a/server/tests/api/check-params/videos-common-filters.ts +++ b/server/tests/api/check-params/videos-common-filters.ts @@ -122,6 +122,8 @@ describe('Test video filters validators', function () { include?: VideoInclude privacyOneOf?: VideoPrivacy[] expectedStatus: HttpStatusCode + excludeAlreadyWatched?: boolean + unauthenticatedUser?: boolean }) { const paths = [ '/api/v1/video-channels/root_channel/videos', @@ -131,14 +133,19 @@ describe('Test video filters validators', function () { ] for (const path of paths) { + const token = options.unauthenticatedUser + ? undefined + : options.token || server.accessToken + await makeGetRequest({ url: server.url, path, - token: options.token || server.accessToken, + token, query: { isLocal: options.isLocal, privacyOneOf: options.privacyOneOf, - include: options.include + include: options.include, + excludeAlreadyWatched: options.excludeAlreadyWatched }, expectedStatus: options.expectedStatus }) @@ -213,6 +220,14 @@ describe('Test video filters validators', function () { } }) }) + + it('Should fail when trying to exclude already watched videos for an unlogged user', async function () { + await testEndpoints({ excludeAlreadyWatched: true, unauthenticatedUser: true, expectedStatus: HttpStatusCode.BAD_REQUEST_400 }) + }) + + it('Should succeed when trying to exclude already watched videos for a logged user', async function () { + await testEndpoints({ token: userAccessToken, excludeAlreadyWatched: true, expectedStatus: HttpStatusCode.OK_200 }) + }) }) after(async function () { diff --git a/server/tests/api/videos/videos-common-filters.ts b/server/tests/api/videos/videos-common-filters.ts index 1ab78ac49..30251706b 100644 --- a/server/tests/api/videos/videos-common-filters.ts +++ b/server/tests/api/videos/videos-common-filters.ts @@ -162,13 +162,23 @@ describe('Test videos filter', function () { tagsAllOf?: string[] token?: string expectedStatus?: HttpStatusCode + excludeAlreadyWatched?: boolean }) { const res = await makeGetRequest({ url: options.server.url, path: options.path, token: options.token ?? options.server.accessToken, query: { - ...pick(options, [ 'isLocal', 'include', 'category', 'tagsAllOf', 'hasWebtorrentFiles', 'hasHLSFiles', 'privacyOneOf' ]), + ...pick(options, [ + 'isLocal', + 'include', + 'category', + 'tagsAllOf', + 'hasWebtorrentFiles', + 'hasHLSFiles', + 'privacyOneOf', + 'excludeAlreadyWatched' + ]), sort: 'createdAt' }, @@ -187,6 +197,7 @@ describe('Test videos filter', function () { token?: string expectedStatus?: HttpStatusCode skipSubscription?: boolean + excludeAlreadyWatched?: boolean } ) { const { skipSubscription = false } = options @@ -525,6 +536,25 @@ describe('Test videos filter', function () { } } }) + + it('Should filter already watched videos by the user', async function () { + const { id } = await servers[0].videos.upload({ attributes: { name: 'video for history' } }) + + for (const path of paths) { + const videos = await listVideos({ server: servers[0], path, isLocal: true, excludeAlreadyWatched: true }) + const foundVideo = videos.find(video => video.id === id) + + expect(foundVideo).to.not.be.undefined + } + await servers[0].views.view({ id, token: servers[0].accessToken }) + + for (const path of paths) { + const videos = await listVideos({ server: servers[0], path, excludeAlreadyWatched: true }) + const foundVideo = videos.find(video => video.id === id) + + expect(foundVideo).to.be.undefined + } + }) }) after(async function () { -- cgit v1.2.3