Skip to content

Commit

Permalink
feat: add canonical tag to pages with pagination
Browse files Browse the repository at this point in the history
  • Loading branch information
kontrollanten committed Oct 23, 2024
1 parent a91fd0c commit c7a1037
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 9 deletions.
40 changes: 36 additions & 4 deletions packages/tests/src/client/index-html.ts
Original file line number Diff line number Diff line change
@@ -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'

Expand All @@ -25,6 +25,8 @@ describe('Test index HTML generation', function () {
shortDescription: string
}

const getTitleWithSuffix = (title: string, config: ServerConfig) => `${title} - ${config.instance.name}`

before(async function () {
this.timeout(120000);

Expand All @@ -49,7 +51,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, instanceConfig.name, instanceConfig.shortDescription, '', config)
checkIndexTags(res.text, getTitleWithSuffix('Trending', config), instanceConfig.shortDescription, '', config)
})

it('Should update the customized configuration and have the correct index html tags', async function () {
Expand All @@ -73,20 +75,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(`<link rel="canonical" href="${servers[0].url}/videos/trending?page=2" />`)
})

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)
Expand Down Expand Up @@ -114,6 +121,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(`<link rel="canonical" href="${servers[0].url}/a/root/video-channels?page=2" />`)
})

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(`<link rel="canonical" href="${servers[0].url}/a/root/videos?page=2" />`)
})

it('Should use the original channel URL for the canonical tag', async function () {
const channelURLtests = res => {
expect(res.text).to.contain(`<link rel="canonical" href="${servers[0].url}/c/root_channel/videos" />`)
Expand All @@ -123,6 +142,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(`<link rel="canonical" href="${servers[0].url}/c/root_channel/videos?page=2" />`)
})

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(`<link rel="canonical" href="${servers[0].url}/c/root_channel/video-playlists?page=2" />`)
})
})

describe('Indexation tags', function () {
Expand Down
14 changes: 14 additions & 0 deletions server/core/controllers/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions server/core/lib/html/client-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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)
}
Expand Down
20 changes: 19 additions & 1 deletion server/core/lib/html/shared/actor-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
52 changes: 52 additions & 0 deletions server/core/lib/html/shared/videos-html.ts
Original file line number Diff line number Diff line change
@@ -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,

forbidIndexation: false
}, {})
}
}
4 changes: 2 additions & 2 deletions server/core/models/account/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -495,8 +495,8 @@ export class AccountModel extends SequelizeModel<AccountModel> {
}

// 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 () {
Expand Down
4 changes: 2 additions & 2 deletions server/core/models/video/video-channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -859,8 +859,8 @@ export class VideoChannelModel extends SequelizeModel<VideoChannelModel> {
}

// 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 () {
Expand Down

0 comments on commit c7a1037

Please sign in to comment.