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: [PATCH] Feature/filter already watched videos (#5739) * filter already watched videos * Updated code based on review comments --- .../recent-videos-recommendation.service.ts | 3 ++ .../shared-search/advanced-search.model.ts | 12 +++++-- server/helpers/query.ts | 6 ++-- .../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 +++++++++-- .../tests/api/videos/videos-common-filters.ts | 32 ++++++++++++++++++- .../search/videos-common-query.model.ts | 2 ++ support/doc/api/openapi.yaml | 11 +++++++ 10 files changed, 119 insertions(+), 9 deletions(-) diff --git a/client/src/app/+videos/+video-watch/shared/recommendations/recent-videos-recommendation.service.ts b/client/src/app/+videos/+video-watch/shared/recommendations/recent-videos-recommendation.service.ts index 4654da847..b0ae910ac 100644 --- a/client/src/app/+videos/+video-watch/shared/recommendations/recent-videos-recommendation.service.ts +++ b/client/src/app/+videos/+video-watch/shared/recommendations/recent-videos-recommendation.service.ts @@ -63,6 +63,9 @@ export class RecentVideosRecommendationService implements RecommendationService searchTarget: 'local', nsfw: user.nsfwPolicy ? this.videos.nsfwPolicyToParam(user.nsfwPolicy) + : undefined, + excludeAlreadyWatched: user.id + ? true : undefined }) } diff --git a/client/src/app/shared/shared-search/advanced-search.model.ts b/client/src/app/shared/shared-search/advanced-search.model.ts index e8bb00fd3..29fe3e8dc 100644 --- a/client/src/app/shared/shared-search/advanced-search.model.ts +++ b/client/src/app/shared/shared-search/advanced-search.model.ts @@ -40,6 +40,8 @@ export class AdvancedSearch { searchTarget: SearchTargetType resultType: AdvancedSearchResultType + excludeAlreadyWatched?: boolean + constructor (options?: { startDate?: string endDate?: string @@ -62,6 +64,8 @@ export class AdvancedSearch { sort?: string searchTarget?: SearchTargetType resultType?: AdvancedSearchResultType + + excludeAlreadyWatched?: boolean }) { if (!options) return @@ -87,6 +91,8 @@ export class AdvancedSearch { this.resultType = options.resultType || undefined + this.excludeAlreadyWatched = options.excludeAlreadyWatched || undefined + if (!this.resultType && this.hasVideoFilter()) { this.resultType = 'videos' } @@ -138,7 +144,8 @@ export class AdvancedSearch { host: this.host, sort: this.sort, searchTarget: this.searchTarget, - resultType: this.resultType + resultType: this.resultType, + excludeAlreadyWatched: this.excludeAlreadyWatched } } @@ -162,7 +169,8 @@ export class AdvancedSearch { host: this.host, isLive, sort: this.sort, - searchTarget: this.searchTarget + searchTarget: this.searchTarget, + excludeAlreadyWatched: this.excludeAlreadyWatched } } 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 () { diff --git a/shared/models/search/videos-common-query.model.ts b/shared/models/search/videos-common-query.model.ts index 2cbf7b014..da479c928 100644 --- a/shared/models/search/videos-common-query.model.ts +++ b/shared/models/search/videos-common-query.model.ts @@ -35,6 +35,8 @@ export interface VideosCommonQuery { skipCount?: boolean search?: string + + excludeAlreadyWatched?: boolean } export interface VideosCommonQueryAfterSanitize extends VideosCommonQuery { diff --git a/support/doc/api/openapi.yaml b/support/doc/api/openapi.yaml index 046eec544..a36ae0c7e 100644 --- a/support/doc/api/openapi.yaml +++ b/support/doc/api/openapi.yaml @@ -717,6 +717,7 @@ paths: - $ref: '#/components/parameters/start' - $ref: '#/components/parameters/count' - $ref: '#/components/parameters/videosSort' + - $ref: '#/components/parameters/excludeAlreadyWatched' responses: '200': description: successful operation @@ -1835,6 +1836,7 @@ paths: - $ref: '#/components/parameters/start' - $ref: '#/components/parameters/count' - $ref: '#/components/parameters/videosSort' + - $ref: '#/components/parameters/excludeAlreadyWatched' responses: '200': description: successful operation @@ -2378,6 +2380,7 @@ paths: - $ref: '#/components/parameters/start' - $ref: '#/components/parameters/count' - $ref: '#/components/parameters/videosSort' + - $ref: '#/components/parameters/excludeAlreadyWatched' responses: '200': description: successful operation @@ -3799,6 +3802,7 @@ paths: - $ref: '#/components/parameters/start' - $ref: '#/components/parameters/count' - $ref: '#/components/parameters/videosSort' + - $ref: '#/components/parameters/excludeAlreadyWatched' responses: '200': description: successful operation @@ -4742,6 +4746,7 @@ paths: - $ref: '#/components/parameters/count' - $ref: '#/components/parameters/searchTarget' - $ref: '#/components/parameters/videosSearchSort' + - $ref: '#/components/parameters/excludeAlreadyWatched' - name: startDate in: query description: Get videos that are published after this date @@ -5872,6 +5877,12 @@ components: schema: $ref: '#/components/schemas/VideoPrivacySet' description: '**PeerTube >= 4.0** Display only videos in this specific privacy/privacies' + excludeAlreadyWatched: + name: excludeAlreadyWatched + in: query + description: Whether or not to exclude videos that are in the user's video history + schema: + type: boolean uuids: name: uuids in: query