From b620bea43d33d6fd4e9449aebbbeb445b8081c37 Mon Sep 17 00:00:00 2001 From: kontrollanten <6680299+kontrollanten@users.noreply.github.com> Date: Sat, 21 Sep 2024 22:41:32 +0200 Subject: [PATCH] feat: add canonical tag to pages with pagination --- packages/tests/src/client/index-html.ts | 40 +++++++++- server/core/controllers/client.ts | 14 ++++ server/core/controllers/index.html | 90 ++++++++++++++++++++++ server/core/lib/html/client-html.ts | 16 ++++ server/core/lib/html/shared/actor-html.ts | 20 ++++- server/core/lib/html/shared/video-html.ts | 1 - server/core/lib/html/shared/videos-html.ts | 52 +++++++++++++ server/core/models/account/account.ts | 4 +- server/core/models/video/video-channel.ts | 4 +- 9 files changed, 231 insertions(+), 10 deletions(-) create mode 100644 server/core/controllers/index.html create mode 100644 server/core/lib/html/shared/videos-html.ts diff --git a/packages/tests/src/client/index-html.ts b/packages/tests/src/client/index-html.ts index 5f33955dc2e..30484e209d4 100644 --- a/packages/tests/src/client/index-html.ts +++ b/packages/tests/src/client/index-html.ts @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/no-unused-expressions,@typescript-eslint/require-await */ import { expect } from 'chai' -import { HttpStatusCode, VideoPlaylistCreateResult } from '@peertube/peertube-models' +import { HttpStatusCode, ServerConfig, VideoPlaylistCreateResult } from '@peertube/peertube-models' import { cleanupTests, makeGetRequest, makeHTMLRequest, PeerTubeServer } from '@peertube/peertube-server-commands' import { checkIndexTags, getWatchPlaylistBasePaths, getWatchVideoBasePaths, prepareClientTests } from '@tests/shared/client.js' @@ -22,6 +22,8 @@ describe('Test index HTML generation', function () { let instanceDescription: string + const getTitleWithSuffix = (title: string, config: ServerConfig) => `${title} - ${config.instance.name}` + before(async function () { this.timeout(120000); @@ -46,7 +48,7 @@ describe('Test index HTML generation', function () { const config = await servers[0].config.getConfig() const res = await makeHTMLRequest(servers[0].url, '/videos/trending') - checkIndexTags(res.text, 'PeerTube', instanceDescription, '', config) + checkIndexTags(res.text, getTitleWithSuffix('Trending', config), instanceDescription, '', config) }) it('Should update the customized configuration and have the correct index html tags', async function () { @@ -70,20 +72,25 @@ describe('Test index HTML generation', function () { const config = await servers[0].config.getConfig() const res = await makeHTMLRequest(servers[0].url, '/videos/trending') - checkIndexTags(res.text, 'PeerTube updated', 'my short description', 'body { background-color: red; }', config) + checkIndexTags(res.text, getTitleWithSuffix('Trending', config), 'my short description', 'body { background-color: red; }', config) }) it('Should have valid index html updated tags (title, description...)', async function () { const config = await servers[0].config.getConfig() const res = await makeHTMLRequest(servers[0].url, '/videos/trending') - checkIndexTags(res.text, 'PeerTube updated', 'my short description', 'body { background-color: red; }', config) + checkIndexTags(res.text, getTitleWithSuffix('Trending', config), 'my short description', 'body { background-color: red; }', config) }) }) describe('Canonical tags', function () { it('Should use the original video URL for the canonical tag', async function () { + const res = await makeHTMLRequest(servers[0].url, '/videos/trending?page=2') + expect(res.text).to.contain(``) + }) + + it('Should use pagination in video URL for the canonical tag', async function () { for (const basePath of getWatchVideoBasePaths()) { for (const id of videoIds) { const res = await makeHTMLRequest(servers[0].url, basePath + id) @@ -111,6 +118,18 @@ describe('Test index HTML generation', function () { accountURLtest(await makeHTMLRequest(servers[0].url, '/@root@' + servers[0].host)) }) + it('Should use pagination in account video channels URL for the canonical tag', async function () { + const res = await makeHTMLRequest(servers[0].url, '/a/root/video-channels?page=2') + + expect(res.text).to.contain(``) + }) + + it('Should use pagination in account videos URL for the canonical tag', async function () { + const res = await makeHTMLRequest(servers[0].url, '/a/root/videos?page=2') + + expect(res.text).to.contain(``) + }) + it('Should use the original channel URL for the canonical tag', async function () { const channelURLtests = res => { expect(res.text).to.contain(``) @@ -120,6 +139,19 @@ describe('Test index HTML generation', function () { channelURLtests(await makeHTMLRequest(servers[0].url, '/c/root_channel@' + servers[0].host)) channelURLtests(await makeHTMLRequest(servers[0].url, '/@root_channel@' + servers[0].host)) }) + + it('Should use pagination in channel videos URL for the canonical tag', async function () { + const res = await makeHTMLRequest(servers[0].url, '/c/root_channel/videos?page=2') + + expect(res.text).to.contain(``) + }) + + it('Should use pagination in channel playlists URL for the canonical tag', async function () { + const res = await makeHTMLRequest(servers[0].url, '/c/root_channel/video-playlists?page=2') + console.log(res.text) + + expect(res.text).to.contain(``) + }) }) describe('Indexation tags', function () { diff --git a/server/core/controllers/client.ts b/server/core/controllers/client.ts index 403b8b141a6..f977106a60f 100644 --- a/server/core/controllers/client.ts +++ b/server/core/controllers/client.ts @@ -11,6 +11,7 @@ import { currentDir, root } from '@peertube/peertube-node-utils' import { STATIC_MAX_AGE } from '../initializers/constants.js' import { ClientHtml, sendHTML, serveIndexHTML } from '../lib/html/client-html.js' import { asyncMiddleware, buildRateLimiter, embedCSP } from '../middlewares/index.js' +import { VideosOrderType } from '../lib/html/shared/videos-html.js' const clientsRouter = express.Router() @@ -29,6 +30,11 @@ clientsRouter.use([ '/w/p/:id', '/videos/watch/playlist/:id' ], asyncMiddleware(generateWatchPlaylistHtmlPage) ) +clientsRouter.get([ '/videos/:type(overview|trending|recently-added|local)', '/' ], + clientsRateLimiter, + asyncMiddleware(generateVideosHtmlPage) +) + clientsRouter.use([ '/w/:id', '/videos/watch/:id' ], clientsRateLimiter, asyncMiddleware(generateWatchHtmlPage) @@ -186,6 +192,14 @@ async function generateVideoPlaylistEmbedHtmlPage (req: express.Request, res: ex return sendHTML(html, res) } +async function generateVideosHtmlPage (req: express.Request, res: express.Response) { + const { type } = req.params as { type: VideosOrderType } + + const html = await ClientHtml.getVideosHTMLPage(type, req, res, req.params.language) + + return sendHTML(html, res, true) +} + async function generateWatchHtmlPage (req: express.Request, res: express.Response) { // Thread link is '/w/:videoId;threadId=:threadId' // So to get the videoId we need to remove the last part diff --git a/server/core/controllers/index.html b/server/core/controllers/index.html new file mode 100644 index 00000000000..ce50f7be83b --- /dev/null +++ b/server/core/controllers/index.html @@ -0,0 +1,90 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/server/core/lib/html/client-html.ts b/server/core/lib/html/client-html.ts index c5e765ec8eb..c0561a00980 100644 --- a/server/core/lib/html/client-html.ts +++ b/server/core/lib/html/client-html.ts @@ -6,6 +6,8 @@ import { VideoHtml } from './shared/video-html.js' import { PlaylistHtml } from './shared/playlist-html.js' import { ActorHtml } from './shared/actor-html.js' import { PageHtml } from './shared/page-html.js' +import { VideosHtml, VideosOrderType } from './shared/videos-html.js' +import { CONFIG } from '@server/initializers/config.js' class ClientHtml { @@ -19,6 +21,20 @@ class ClientHtml { // --------------------------------------------------------------------------- + static getVideosHTMLPage (type: VideosOrderType, req: express.Request, res: express.Response, paramLang?: string) { + if (type) { + return VideosHtml.getVideosHTML(type, req, res) + } + + const [ , eventualType ] = CONFIG.INSTANCE.DEFAULT_CLIENT_ROUTE.split('/videos/') as VideosOrderType[] + + if (eventualType) { + return VideosHtml.getVideosHTML(eventualType, req, res) + } + + return PageHtml.getDefaultHTML(req, res, paramLang) + } + static getWatchHTMLPage (videoIdArg: string, req: express.Request, res: express.Response) { return VideoHtml.getWatchVideoHTML(videoIdArg, req, res) } diff --git a/server/core/lib/html/shared/actor-html.ts b/server/core/lib/html/shared/actor-html.ts index 2c6990afa5b..223537cb70f 100644 --- a/server/core/lib/html/shared/actor-html.ts +++ b/server/core/lib/html/shared/actor-html.ts @@ -55,7 +55,25 @@ export class ActorHtml { let customHTML = TagsHtml.addTitleTag(html, entity.getDisplayName()) customHTML = TagsHtml.addDescriptionTag(customHTML, escapedTruncatedDescription) - const url = entity.getClientUrl() + const eventualPage = req.path.split('/').pop() + let url + + if (entity instanceof AccountModel) { + const page = [ 'video-channels', 'videos' ].includes(eventualPage) + ? eventualPage + : undefined + url = entity.getClientUrl(page as 'video-channels' | 'videos') + } else if (entity instanceof VideoChannelModel) { + const page = [ 'video-playlists', 'videos' ].includes(eventualPage) + ? eventualPage + : undefined + url = entity.getClientUrl(page as 'video-playlists' | 'videos') + } + + if (req.query.page) { + url += `?page=${req.query.page}` + } + const siteName = CONFIG.INSTANCE.NAME const title = entity.getDisplayName() diff --git a/server/core/lib/html/shared/video-html.ts b/server/core/lib/html/shared/video-html.ts index e1b285a9c2a..e050851f7be 100644 --- a/server/core/lib/html/shared/video-html.ts +++ b/server/core/lib/html/shared/video-html.ts @@ -15,7 +15,6 @@ import { PageHtml } from './page-html.js' import { TagsHtml } from './tags-html.js' export class VideoHtml { - static async getWatchVideoHTML (videoIdArg: string, req: express.Request, res: express.Response) { const videoId = toCompleteUUID(videoIdArg) diff --git a/server/core/lib/html/shared/videos-html.ts b/server/core/lib/html/shared/videos-html.ts new file mode 100644 index 00000000000..31e3fd32e4e --- /dev/null +++ b/server/core/lib/html/shared/videos-html.ts @@ -0,0 +1,52 @@ +import { escapeHTML } from '@peertube/peertube-core-utils' +import express from 'express' +import { CONFIG } from '../../../initializers/config.js' +import { WEBSERVER } from '../../../initializers/constants.js' +import { PageHtml } from './page-html.js' +import { TagsHtml } from './tags-html.js' + +export type VideosOrderType = 'local' | 'trending' | 'overview' | 'recently-added' + +export class VideosHtml { + + static async getVideosHTML (type: VideosOrderType, req: express.Request, res: express.Response) { + const html = await PageHtml.getIndexHTML(req, res) + + return this.buildVideosHTML({ + html, + type, + currentPage: req.query.page + }) + } + + // --------------------------------------------------------------------------- + // Private + // --------------------------------------------------------------------------- + + private static buildVideosHTML (options: { + html: string + type: VideosOrderType + currentPage: string + }) { + const { html, currentPage, type } = options + + const title = type === 'recently-added' ? 'Recently added' : type.slice(0, 1).toUpperCase() + type.slice(1) + let customHTML = TagsHtml.addTitleTag(html, title) + customHTML = TagsHtml.addDescriptionTag(customHTML) + + let url = WEBSERVER.URL + '/videos/' + type + + if (currentPage) { + url += `?page=${currentPage}` + } + + return TagsHtml.addTags(customHTML, { + url, + + escapedSiteName: escapeHTML(CONFIG.INSTANCE.NAME), + escapedTitle: title, + + indexationPolicy: 'always' + }, {}) + } +} diff --git a/server/core/models/account/account.ts b/server/core/models/account/account.ts index f86a290623e..d9b42f5e203 100644 --- a/server/core/models/account/account.ts +++ b/server/core/models/account/account.ts @@ -476,8 +476,8 @@ export class AccountModel extends SequelizeModel { } // Avoid error when running this method on MAccount... | MChannel... - getClientUrl (this: MAccountHost | MChannelHost) { - return WEBSERVER.URL + '/a/' + this.Actor.getIdentifier() + '/video-channels' + getClientUrl (this: MAccountHost | MChannelHost, page: 'video-channels' | 'videos' = 'video-channels') { + return WEBSERVER.URL + '/a/' + this.Actor.getIdentifier() + '/' + page } isBlocked () { diff --git a/server/core/models/video/video-channel.ts b/server/core/models/video/video-channel.ts index 3b8795d07d2..e434d8c43e1 100644 --- a/server/core/models/video/video-channel.ts +++ b/server/core/models/video/video-channel.ts @@ -841,8 +841,8 @@ export class VideoChannelModel extends SequelizeModel { } // Avoid error when running this method on MAccount... | MChannel... - getClientUrl (this: MAccountHost | MChannelHost) { - return WEBSERVER.URL + '/c/' + this.Actor.getIdentifier() + '/videos' + getClientUrl (this: MAccountHost | MChannelHost, page: 'video-playlists' | 'videos' = 'videos') { + return WEBSERVER.URL + '/c/' + this.Actor.getIdentifier() + '/' + page } getDisplayName () {