diff options
author | Brad Johnson <bradsk88@gmail.com> | 2018-08-31 09:19:21 -0600 |
---|---|---|
committer | Chocobozzz <me@florianbigard.com> | 2018-08-31 17:19:21 +0200 |
commit | 7f5f4152a4cd4fc328d6ae177d281ebe7e792dd3 (patch) | |
tree | fd0401bd9c43e1adbbedbd2042c93bd4fef46632 /client/src/app/videos | |
parent | 1a4710914432b44115b185cec1883fdf409aef1d (diff) | |
download | PeerTube-7f5f4152a4cd4fc328d6ae177d281ebe7e792dd3.tar.gz PeerTube-7f5f4152a4cd4fc328d6ae177d281ebe7e792dd3.tar.zst PeerTube-7f5f4152a4cd4fc328d6ae177d281ebe7e792dd3.zip |
Refactor: Separated "Other Videos" section into a dedicated component/service (#969)
* Separated "Other Videos" section into a dedicated component/service
I'm currently working on some proof-of-concepts for recommendation
providers that could work with PeerTube to provide useful video
suggestions to the user.
As a first step, I want to have great clarity about how PeerTube,
itself, will surface these videos to the user.
With this branch, I'm refactoring the "recommendations" to make it
easier to swap out different recommender implementations quickly.
Stop recommender from including the video that's being watched.
Ensure always 5 recommendations
* Treat recommendations as a stream of values, rather than a single async value.
* Prioritize readability over HTTP response size early-optimization.
* Simplify pipe
Diffstat (limited to 'client/src/app/videos')
11 files changed, 240 insertions, 35 deletions
diff --git a/client/src/app/videos/+video-watch/video-watch.component.html b/client/src/app/videos/+video-watch/video-watch.component.html index 2c8305777..16d657a65 100644 --- a/client/src/app/videos/+video-watch/video-watch.component.html +++ b/client/src/app/videos/+video-watch/video-watch.component.html | |||
@@ -197,19 +197,10 @@ | |||
197 | </div> | 197 | </div> |
198 | </div> | 198 | </div> |
199 | 199 | ||
200 | <my-video-comments [video]="video" [user]="user"></my-video-comments> | 200 | <my-video-comments [video]="video" [user]="user"></my-video-comments> |
201 | </div> | ||
202 | |||
203 | <div class="ml-3 ml-sm-0 col-12 col-md-3 other-videos"> | ||
204 | <div i18n class="title-page title-page-single"> | ||
205 | Other videos | ||
206 | </div> | ||
207 | |||
208 | <div *ngFor="let video of otherVideosDisplayed"> | ||
209 | <my-video-miniature [video]="video" [user]="user"></my-video-miniature> | ||
210 | </div> | ||
211 | </div> | ||
212 | </div> | 201 | </div> |
202 | <my-recommended-videos class="ml-3 ml-sm-0 col-12 col-md-3" | ||
203 | [inputVideo]="video" [user]="user"></my-recommended-videos> | ||
213 | </div> | 204 | </div> |
214 | 205 | ||
215 | 206 | ||
diff --git a/client/src/app/videos/+video-watch/video-watch.component.ts b/client/src/app/videos/+video-watch/video-watch.component.ts index d838ebe79..25643cfde 100644 --- a/client/src/app/videos/+video-watch/video-watch.component.ts +++ b/client/src/app/videos/+video-watch/video-watch.component.ts | |||
@@ -15,7 +15,6 @@ import '../../../assets/player/peertube-videojs-plugin' | |||
15 | import { AuthService, ConfirmService } from '../../core' | 15 | import { AuthService, ConfirmService } from '../../core' |
16 | import { RestExtractor, VideoBlacklistService } from '../../shared' | 16 | import { RestExtractor, VideoBlacklistService } from '../../shared' |
17 | import { VideoDetails } from '../../shared/video/video-details.model' | 17 | import { VideoDetails } from '../../shared/video/video-details.model' |
18 | import { Video } from '../../shared/video/video.model' | ||
19 | import { VideoService } from '../../shared/video/video.service' | 18 | import { VideoService } from '../../shared/video/video.service' |
20 | import { MarkdownService } from '../shared' | 19 | import { MarkdownService } from '../shared' |
21 | import { VideoDownloadComponent } from './modal/video-download.component' | 20 | import { VideoDownloadComponent } from './modal/video-download.component' |
@@ -43,8 +42,6 @@ export class VideoWatchComponent implements OnInit, OnDestroy { | |||
43 | @ViewChild('videoSupportModal') videoSupportModal: VideoSupportComponent | 42 | @ViewChild('videoSupportModal') videoSupportModal: VideoSupportComponent |
44 | @ViewChild('videoBlacklistModal') videoBlacklistModal: VideoBlacklistComponent | 43 | @ViewChild('videoBlacklistModal') videoBlacklistModal: VideoBlacklistComponent |
45 | 44 | ||
46 | otherVideosDisplayed: Video[] = [] | ||
47 | |||
48 | player: videojs.Player | 45 | player: videojs.Player |
49 | playerElement: HTMLVideoElement | 46 | playerElement: HTMLVideoElement |
50 | userRating: UserVideoRateType = null | 47 | userRating: UserVideoRateType = null |
@@ -60,7 +57,6 @@ export class VideoWatchComponent implements OnInit, OnDestroy { | |||
60 | remoteServerDown = false | 57 | remoteServerDown = false |
61 | 58 | ||
62 | private videojsLocaleLoaded = false | 59 | private videojsLocaleLoaded = false |
63 | private otherVideos: Video[] = [] | ||
64 | private paramsSub: Subscription | 60 | private paramsSub: Subscription |
65 | 61 | ||
66 | constructor ( | 62 | constructor ( |
@@ -96,16 +92,6 @@ export class VideoWatchComponent implements OnInit, OnDestroy { | |||
96 | this.hasAlreadyAcceptedPrivacyConcern = true | 92 | this.hasAlreadyAcceptedPrivacyConcern = true |
97 | } | 93 | } |
98 | 94 | ||
99 | this.videoService.getVideos({ currentPage: 1, itemsPerPage: 5 }, '-createdAt') | ||
100 | .subscribe( | ||
101 | data => { | ||
102 | this.otherVideos = data.videos | ||
103 | this.updateOtherVideosDisplayed() | ||
104 | }, | ||
105 | |||
106 | err => console.error(err) | ||
107 | ) | ||
108 | |||
109 | this.paramsSub = this.route.params.subscribe(routeParams => { | 95 | this.paramsSub = this.route.params.subscribe(routeParams => { |
110 | const uuid = routeParams[ 'uuid' ] | 96 | const uuid = routeParams[ 'uuid' ] |
111 | 97 | ||
@@ -365,8 +351,6 @@ export class VideoWatchComponent implements OnInit, OnDestroy { | |||
365 | this.completeDescriptionShown = false | 351 | this.completeDescriptionShown = false |
366 | this.remoteServerDown = false | 352 | this.remoteServerDown = false |
367 | 353 | ||
368 | this.updateOtherVideosDisplayed() | ||
369 | |||
370 | if (this.video.isVideoNSFWForUser(this.user, this.serverService.getConfig())) { | 354 | if (this.video.isVideoNSFWForUser(this.user, this.serverService.getConfig())) { |
371 | const res = await this.confirmService.confirm( | 355 | const res = await this.confirmService.confirm( |
372 | this.i18n('This video contains mature or explicit content. Are you sure you want to watch it?'), | 356 | this.i18n('This video contains mature or explicit content. Are you sure you want to watch it?'), |
@@ -474,12 +458,6 @@ export class VideoWatchComponent implements OnInit, OnDestroy { | |||
474 | this.setVideoLikesBarTooltipText() | 458 | this.setVideoLikesBarTooltipText() |
475 | } | 459 | } |
476 | 460 | ||
477 | private updateOtherVideosDisplayed () { | ||
478 | if (this.video && this.otherVideos && this.otherVideos.length > 0) { | ||
479 | this.otherVideosDisplayed = this.otherVideos.filter(v => v.uuid !== this.video.uuid) | ||
480 | } | ||
481 | } | ||
482 | |||
483 | private setOpenGraphTags () { | 461 | private setOpenGraphTags () { |
484 | this.metaService.setTitle(this.video.name) | 462 | this.metaService.setTitle(this.video.name) |
485 | 463 | ||
diff --git a/client/src/app/videos/+video-watch/video-watch.module.ts b/client/src/app/videos/+video-watch/video-watch.module.ts index 7920147b2..5582ab40f 100644 --- a/client/src/app/videos/+video-watch/video-watch.module.ts +++ b/client/src/app/videos/+video-watch/video-watch.module.ts | |||
@@ -16,6 +16,7 @@ import { VideoWatchComponent } from './video-watch.component' | |||
16 | import { NgxQRCodeModule } from 'ngx-qrcode2' | 16 | import { NgxQRCodeModule } from 'ngx-qrcode2' |
17 | import { NgbTooltipModule } from '@ng-bootstrap/ng-bootstrap' | 17 | import { NgbTooltipModule } from '@ng-bootstrap/ng-bootstrap' |
18 | import { VideoBlacklistComponent } from '@app/videos/+video-watch/modal/video-blacklist.component' | 18 | import { VideoBlacklistComponent } from '@app/videos/+video-watch/modal/video-blacklist.component' |
19 | import { RecommendationsModule } from '@app/videos/recommendations/recommendations.module' | ||
19 | import { TextareaAutosizeModule } from 'ngx-textarea-autosize' | 20 | import { TextareaAutosizeModule } from 'ngx-textarea-autosize' |
20 | 21 | ||
21 | @NgModule({ | 22 | @NgModule({ |
@@ -25,7 +26,8 @@ import { TextareaAutosizeModule } from 'ngx-textarea-autosize' | |||
25 | ClipboardModule, | 26 | ClipboardModule, |
26 | NgbTooltipModule, | 27 | NgbTooltipModule, |
27 | NgxQRCodeModule, | 28 | NgxQRCodeModule, |
28 | TextareaAutosizeModule | 29 | TextareaAutosizeModule, |
30 | RecommendationsModule | ||
29 | ], | 31 | ], |
30 | 32 | ||
31 | declarations: [ | 33 | declarations: [ |
diff --git a/client/src/app/videos/recommendations/recent-videos-recommendation.service.spec.ts b/client/src/app/videos/recommendations/recent-videos-recommendation.service.spec.ts new file mode 100644 index 000000000..f9055b82c --- /dev/null +++ b/client/src/app/videos/recommendations/recent-videos-recommendation.service.spec.ts | |||
@@ -0,0 +1,66 @@ | |||
1 | import { RecentVideosRecommendationService } from '@app/videos/recommendations/recent-videos-recommendation.service' | ||
2 | import { VideosProvider } from '@app/shared/video/video.service' | ||
3 | import { EMPTY, of } from 'rxjs' | ||
4 | import Mock = jest.Mock | ||
5 | |||
6 | describe('"Recent Videos" Recommender', () => { | ||
7 | describe('getRecommendations', () => { | ||
8 | let videosService: VideosProvider | ||
9 | let service: RecentVideosRecommendationService | ||
10 | let getVideosMock: Mock<any> | ||
11 | beforeEach(() => { | ||
12 | getVideosMock = jest.fn(() => EMPTY) | ||
13 | videosService = { | ||
14 | getVideos: getVideosMock | ||
15 | } | ||
16 | service = new RecentVideosRecommendationService(videosService) | ||
17 | }) | ||
18 | it('should filter out the given UUID from the results', async (done) => { | ||
19 | const vids = [ | ||
20 | { uuid: 'uuid1' }, | ||
21 | { uuid: 'uuid2' } | ||
22 | ] | ||
23 | getVideosMock.mockReturnValueOnce(of({ videos: vids })) | ||
24 | const result = await service.getRecommendations('uuid1').toPromise() | ||
25 | const uuids = result.map(v => v.uuid) | ||
26 | expect(uuids).toEqual(['uuid2']) | ||
27 | done() | ||
28 | }) | ||
29 | it('should return 5 results when the given UUID is NOT in the first 5 results', async (done) => { | ||
30 | const vids = [ | ||
31 | { uuid: 'uuid2' }, | ||
32 | { uuid: 'uuid3' }, | ||
33 | { uuid: 'uuid4' }, | ||
34 | { uuid: 'uuid5' }, | ||
35 | { uuid: 'uuid6' }, | ||
36 | { uuid: 'uuid7' } | ||
37 | ] | ||
38 | getVideosMock.mockReturnValueOnce(of({ videos: vids })) | ||
39 | const result = await service.getRecommendations('uuid1').toPromise() | ||
40 | expect(result.length).toEqual(5) | ||
41 | done() | ||
42 | }) | ||
43 | it('should return 5 results when the given UUID IS PRESENT in the first 5 results', async (done) => { | ||
44 | const vids = [ | ||
45 | { uuid: 'uuid1' }, | ||
46 | { uuid: 'uuid2' }, | ||
47 | { uuid: 'uuid3' }, | ||
48 | { uuid: 'uuid4' }, | ||
49 | { uuid: 'uuid5' }, | ||
50 | { uuid: 'uuid6' } | ||
51 | ] | ||
52 | getVideosMock | ||
53 | .mockReturnValueOnce(of({ videos: vids })) | ||
54 | const result = await service.getRecommendations('uuid1').toPromise() | ||
55 | expect(result.length).toEqual(5) | ||
56 | done() | ||
57 | }) | ||
58 | it('should fetch an extra result in case the given UUID is in the list', async (done) => { | ||
59 | await service.getRecommendations('uuid1').toPromise() | ||
60 | let expectedSize = service.pageSize + 1 | ||
61 | let params = { currentPage: jasmine.anything(), itemsPerPage: expectedSize } | ||
62 | expect(getVideosMock).toHaveBeenCalledWith(params, jasmine.anything()) | ||
63 | done() | ||
64 | }) | ||
65 | }) | ||
66 | }) | ||
diff --git a/client/src/app/videos/recommendations/recent-videos-recommendation.service.ts b/client/src/app/videos/recommendations/recent-videos-recommendation.service.ts new file mode 100644 index 000000000..708d67699 --- /dev/null +++ b/client/src/app/videos/recommendations/recent-videos-recommendation.service.ts | |||
@@ -0,0 +1,39 @@ | |||
1 | import { Inject, Injectable } from '@angular/core' | ||
2 | import { RecommendationService } from '@app/videos/recommendations/recommendations.service' | ||
3 | import { Video } from '@app/shared/video/video.model' | ||
4 | import { VideoService, VideosProvider } from '@app/shared/video/video.service' | ||
5 | import { map } from 'rxjs/operators' | ||
6 | import { Observable } from 'rxjs' | ||
7 | |||
8 | /** | ||
9 | * Provides "recommendations" by providing the most recently uploaded videos. | ||
10 | */ | ||
11 | @Injectable() | ||
12 | export class RecentVideosRecommendationService implements RecommendationService { | ||
13 | |||
14 | readonly pageSize = 5 | ||
15 | |||
16 | constructor ( | ||
17 | @Inject(VideoService) private videos: VideosProvider | ||
18 | ) { | ||
19 | } | ||
20 | |||
21 | getRecommendations (uuid: string): Observable<Video[]> { | ||
22 | return this.fetchPage(1) | ||
23 | .pipe( | ||
24 | map(vids => { | ||
25 | const otherVideos = vids.filter(v => v.uuid !== uuid) | ||
26 | return otherVideos.slice(0, this.pageSize) | ||
27 | }) | ||
28 | ) | ||
29 | } | ||
30 | |||
31 | private fetchPage (page: number): Observable<Video[]> { | ||
32 | let pagination = { currentPage: page, itemsPerPage: this.pageSize + 1 } | ||
33 | return this.videos.getVideos(pagination, '-createdAt') | ||
34 | .pipe( | ||
35 | map(v => v.videos) | ||
36 | ) | ||
37 | } | ||
38 | |||
39 | } | ||
diff --git a/client/src/app/videos/recommendations/recommendations.module.ts b/client/src/app/videos/recommendations/recommendations.module.ts new file mode 100644 index 000000000..5a46ea739 --- /dev/null +++ b/client/src/app/videos/recommendations/recommendations.module.ts | |||
@@ -0,0 +1,25 @@ | |||
1 | import { NgModule } from '@angular/core' | ||
2 | import { RecommendedVideosComponent } from '@app/videos/recommendations/recommended-videos.component' | ||
3 | import { RecommendedVideosStore } from '@app/videos/recommendations/recommended-videos.store' | ||
4 | import { CommonModule } from '@angular/common' | ||
5 | import { SharedModule } from '@app/shared' | ||
6 | import { RecentVideosRecommendationService } from '@app/videos/recommendations/recent-videos-recommendation.service' | ||
7 | |||
8 | @NgModule({ | ||
9 | imports: [ | ||
10 | SharedModule, | ||
11 | CommonModule | ||
12 | ], | ||
13 | declarations: [ | ||
14 | RecommendedVideosComponent | ||
15 | ], | ||
16 | exports: [ | ||
17 | RecommendedVideosComponent | ||
18 | ], | ||
19 | providers: [ | ||
20 | RecommendedVideosStore, | ||
21 | RecentVideosRecommendationService | ||
22 | ] | ||
23 | }) | ||
24 | export class RecommendationsModule { | ||
25 | } | ||
diff --git a/client/src/app/videos/recommendations/recommendations.service.ts b/client/src/app/videos/recommendations/recommendations.service.ts new file mode 100644 index 000000000..44cbda9b7 --- /dev/null +++ b/client/src/app/videos/recommendations/recommendations.service.ts | |||
@@ -0,0 +1,8 @@ | |||
1 | import { Video } from '@app/shared/video/video.model' | ||
2 | import { Observable } from 'rxjs' | ||
3 | |||
4 | export type UUID = string | ||
5 | |||
6 | export interface RecommendationService { | ||
7 | getRecommendations (uuid: UUID): Observable<Video[]> | ||
8 | } | ||
diff --git a/client/src/app/videos/recommendations/recommended-videos.component.html b/client/src/app/videos/recommendations/recommended-videos.component.html new file mode 100644 index 000000000..7cfaffec2 --- /dev/null +++ b/client/src/app/videos/recommendations/recommended-videos.component.html | |||
@@ -0,0 +1,11 @@ | |||
1 | <div class="other-videos"> | ||
2 | <div i18n class="title-page title-page-single"> | ||
3 | Other videos | ||
4 | </div> | ||
5 | |||
6 | <ng-container *ngIf="hasVideos$ | async"> | ||
7 | <div *ngFor="let video of (videos$ | async)"> | ||
8 | <my-video-miniature [video]="video" [user]="user"></my-video-miniature> | ||
9 | </div> | ||
10 | </ng-container> | ||
11 | </div> | ||
diff --git a/client/src/app/videos/recommendations/recommended-videos.component.ts b/client/src/app/videos/recommendations/recommended-videos.component.ts new file mode 100644 index 000000000..aa4dd0ee2 --- /dev/null +++ b/client/src/app/videos/recommendations/recommended-videos.component.ts | |||
@@ -0,0 +1,31 @@ | |||
1 | import { Component, Input, OnChanges } from '@angular/core' | ||
2 | import { Observable } from 'rxjs' | ||
3 | import { Video } from '@app/shared/video/video.model' | ||
4 | import { RecommendedVideosStore } from '@app/videos/recommendations/recommended-videos.store' | ||
5 | import { User } from '@app/shared' | ||
6 | |||
7 | @Component({ | ||
8 | selector: 'my-recommended-videos', | ||
9 | templateUrl: './recommended-videos.component.html' | ||
10 | }) | ||
11 | export class RecommendedVideosComponent implements OnChanges { | ||
12 | @Input() inputVideo: Video | ||
13 | @Input() user: User | ||
14 | |||
15 | readonly hasVideos$: Observable<boolean> | ||
16 | readonly videos$: Observable<Video[]> | ||
17 | |||
18 | constructor ( | ||
19 | private store: RecommendedVideosStore | ||
20 | ) { | ||
21 | this.videos$ = this.store.recommendations$ | ||
22 | this.hasVideos$ = this.store.hasRecommendations$ | ||
23 | } | ||
24 | |||
25 | public ngOnChanges (): void { | ||
26 | if (this.inputVideo) { | ||
27 | this.store.requestNewRecommendations(this.inputVideo.uuid) | ||
28 | } | ||
29 | } | ||
30 | |||
31 | } | ||
diff --git a/client/src/app/videos/recommendations/recommended-videos.store.spec.ts b/client/src/app/videos/recommendations/recommended-videos.store.spec.ts new file mode 100644 index 000000000..e12a3f520 --- /dev/null +++ b/client/src/app/videos/recommendations/recommended-videos.store.spec.ts | |||
@@ -0,0 +1,22 @@ | |||
1 | import { RecommendedVideosStore } from '@app/videos/recommendations/recommended-videos.store' | ||
2 | import { RecommendationService } from '@app/videos/recommendations/recommendations.service' | ||
3 | |||
4 | describe('RecommendedVideosStore', () => { | ||
5 | describe('requestNewRecommendations', () => { | ||
6 | let store: RecommendedVideosStore | ||
7 | let service: RecommendationService | ||
8 | beforeEach(() => { | ||
9 | service = { | ||
10 | getRecommendations: jest.fn(() => new Promise((r) => r())) | ||
11 | } | ||
12 | store = new RecommendedVideosStore(service) | ||
13 | }) | ||
14 | it('should pull new videos from the service one time when given the same UUID twice', () => { | ||
15 | store.requestNewRecommendations('some-uuid') | ||
16 | store.requestNewRecommendations('some-uuid') | ||
17 | // Requests aren't fulfilled until someone asks for them (ie: subscribes) | ||
18 | store.recommendations$.subscribe() | ||
19 | expect(service.getRecommendations).toHaveBeenCalledTimes(1) | ||
20 | }) | ||
21 | }) | ||
22 | }) | ||
diff --git a/client/src/app/videos/recommendations/recommended-videos.store.ts b/client/src/app/videos/recommendations/recommended-videos.store.ts new file mode 100644 index 000000000..689adeb1f --- /dev/null +++ b/client/src/app/videos/recommendations/recommended-videos.store.ts | |||
@@ -0,0 +1,32 @@ | |||
1 | import { Inject, Injectable } from '@angular/core' | ||
2 | import { Observable, ReplaySubject } from 'rxjs' | ||
3 | import { Video } from '@app/shared/video/video.model' | ||
4 | import { RecentVideosRecommendationService } from '@app/videos/recommendations/recent-videos-recommendation.service' | ||
5 | import { RecommendationService, UUID } from '@app/videos/recommendations/recommendations.service' | ||
6 | import { map, switchMap, take } from 'rxjs/operators' | ||
7 | |||
8 | /** | ||
9 | * This store is intended to provide data for the RecommendedVideosComponent. | ||
10 | */ | ||
11 | @Injectable() | ||
12 | export class RecommendedVideosStore { | ||
13 | public readonly recommendations$: Observable<Video[]> | ||
14 | public readonly hasRecommendations$: Observable<boolean> | ||
15 | private readonly requestsForLoad$$ = new ReplaySubject<UUID>(1) | ||
16 | |||
17 | constructor ( | ||
18 | @Inject(RecentVideosRecommendationService) private recommendations: RecommendationService | ||
19 | ) { | ||
20 | this.recommendations$ = this.requestsForLoad$$.pipe( | ||
21 | switchMap(requestedUUID => recommendations.getRecommendations(requestedUUID) | ||
22 | .pipe(take(1)) | ||
23 | )) | ||
24 | this.hasRecommendations$ = this.recommendations$.pipe( | ||
25 | map(otherVideos => otherVideos.length > 0) | ||
26 | ) | ||
27 | } | ||
28 | |||
29 | requestNewRecommendations (videoUUID: string) { | ||
30 | this.requestsForLoad$$.next(videoUUID) | ||
31 | } | ||
32 | } | ||