openvibers/openvibe.play Commits

Leaderboards: all time and this week are two pages, each its own canonical (the week board named the all-time one while the sitemap listed both) (#22)

OP OpenVibers committed 24 days ago · 77cffc0a8f · 5 files, +20 −6

Changes

ddiff --git a/server/http/discovery.js b/server/http/discovery.js
iindex 03e38db..bf63a84 100644
---- a/server/http/discovery.js
++++ b/server/http/discovery.js
@@@ -53,7 +53,8 @@ function createDiscoveryRoutes(ctx) {
const site = String(config.baseUrl).replace(/\/+$/, '');
const abs = (p) => `${site}${p}`;
const TEXT = cache.htmlHeaders({ maxAge: 3600 });
-- const boardPath = (t, b, period) => `/t/${t}/b/${b}?period=${period}`;
++ // All time is the board's plain address, this week ?period=week (each page's own canonical).
++ const boardPath = (t, b, period) => `/t/${t}/b/${b}${period === 'week' ? '?period=week' : ''}`;
r.get('/robots.txt', (_req, res) => {
res.type('text/plain').set('Cache-Control', TEXT).send(
ddiff --git a/server/http/pages.js b/server/http/pages.js
iindex 7f769b6..7ec358a 100644
---- a/server/http/pages.js
++++ b/server/http/pages.js
@@@ -103,7 +103,7 @@ ${raw(showcase.steps({
<h1>${t.name}</h1>
${t.summary ? html`<p class="pl-lede">${t.summary}</p>` : ''}
${t.url ? html`<p><a class="pl-button" href="${t.url}">Play ${t.name}</a></p>` : ''}
--${boards.length ? html`<section aria-labelledby="pl-boards-h"><h2 id="pl-boards-h">Leaderboards this week</h2>${boards.map((b) => html`<h3>${b.name}</h3>${boardTable({ ...b, entries: b.entries.slice(0, 10) })}<p><a href="/t/${t.id}/b/${b.board}?period=week">This week</a> · <a href="/t/${t.id}/b/${b.board}?period=all">All time</a></p>`)}</section>` : ''}
++${boards.length ? html`<section aria-labelledby="pl-boards-h"><h2 id="pl-boards-h">Leaderboards this week</h2>${boards.map((b) => html`<h3>${b.name}</h3>${boardTable({ ...b, entries: b.entries.slice(0, 10) })}<p><a href="/t/${t.id}/b/${b.board}?period=week">This week</a> · <a href="/t/${t.id}/b/${b.board}">All time</a></p>`)}</section>` : ''}
<section aria-labelledby="pl-ach-h"><h2 id="pl-ach-h">Achievements</h2><p class="pl-muted">${plural(t.players, 'player has', 'players have')} unlocked at least one.</p>${achievementList(t.achievements, { players: t.players })}</section>`,
});
});
@@@ -117,8 +117,14 @@ ${boards.length ? html`<section aria-labelledby="pl-boards-h"><h2 id="pl-boards-
if (err instanceof PlayError) return notFound(req, res, 'No board here.');
throw err;
}
-- const link = (o) => `/t/${b.title}/b/${b.board}?period=${o.period || period}${(o.page || 1) > 1 ? `&page=${o.page}` : ''}`;
++ // All time is the board's plain address and this week is ?period=week: two pages, each its own canonical (the
++ // week board named the all-time one while the sitemap listed both).
++ const link = (o) => {
++ const q = [(o.period || period) === 'week' ? 'period=week' : null, (o.page || 1) > 1 ? `page=${o.page}` : null].filter(Boolean);
++ return `/t/${b.title}/b/${b.board}${q.length ? `?${q.join('&')}` : ''}`;
++ };
page(req, res, {
++ canonicalPath: link({ page: pageNo }),
index: true, cache: PUBLIC_CACHE, title: `${b.name}, ${period === 'week' ? 'this week' : 'all time'} · ${b.title_name}`,
description: `The ${b.name} leaderboard of ${b.title_name}, ${period === 'week' ? 'this week' : 'all time'}: ${b.kind === 'sum' ? 'each player\'s total' : 'each player\'s best score'}.`,
body: html`<p class="pl-crumbs"><a href="/">Games</a> / <a href="/t/${b.title}">${b.title_name}</a> /</p>
ddiff --git a/server/render/layout.js b/server/render/layout.js
iindex a225423..640ed3b 100644
---- a/server/render/layout.js
++++ b/server/render/layout.js
@@@ -46,7 +46,8 @@ let RELEASE = 'dev';
function setRelease(id) { if (id) RELEASE = String(id); }
/**
-- * o: title, description, body (html), viewer, config, path, index (default false), scripts [rel],
++ * o: title, description, body (html), viewer, config, path, canonicalPath (default: the path without its query),
++ * index (default false), scripts [rel],
* crumbs [{ label, href }], jsonLd [nodes] (serialised by openvibe-shared/seo), styles [openvibe-shared stylesheet names]
*/
function renderPage(o) {
@@@ -54,7 +55,8 @@ function renderPage(o) {
const signedIn = viewer.kind === 'user';
const pathNow = o.path || '/';
const loginNext = encodeURIComponent(pathNow);
-- const canonical = `${o.config.baseUrl}${pathNow.split('?')[0]}`;
++ // The path without its query, unless the page names its own (`canonicalPath`: a query that is a different page).
++ const canonical = `${o.config.baseUrl}${o.canonicalPath || pathNow.split('?')[0]}`;
const nav = {
service: 'play',
apiBase: NETWORK_URL,
ddiff --git a/test/discovery.test.js b/test/discovery.test.js
iindex a2d7c6d..adb1b22 100644
---- a/test/discovery.test.js
++++ b/test/discovery.test.js
@@@ -26,7 +26,7 @@ const { boot, check, done } = require('./helpers/boot');
await check('the sitemap lists the pages, the game and both periods of its board', async () => {
const r = await t.get('/sitemap.xml');
assert.match(r.headers.get('content-type'), /xml/);
-- for (const loc of ['/', '/docs', '/updates', '/t/scraplandia', '/t/scraplandia/b/haul?period=week', '/t/scraplandia/b/haul?period=all']) {
++ for (const loc of ['/', '/docs', '/updates', '/t/scraplandia', '/t/scraplandia/b/haul?period=week', '/t/scraplandia/b/haul']) {
assert.ok(r.text.includes(`<loc>https://play.openvibe.network${loc.replace('&', '&amp;')}</loc>`), loc);
}
});
ddiff --git a/test/play-api.test.js b/test/play-api.test.js
iindex e3468e9..94f15d6 100644
---- a/test/play-api.test.js
++++ b/test/play-api.test.js
@@@ -139,6 +139,11 @@ const DAY = 86_400_000;
assert.doesNotMatch(title.text, /Nobody knows/, 'a secret achievement\'s text never shows');
const board = await t.get('/t/salvage-rush/b/best?period=week');
assert.match(board.text, /aria-current="page">This week</);
++ // Each period is its own page: the week board names itself, all time is the plain address.
++ assert.match(board.text, /<link rel="canonical" href="[^"]*\/t\/salvage-rush\/b\/best\?period=week">/);
++ const allTime = await t.get('/t/salvage-rush/b/best?period=all');
++ assert.match(allTime.text, /<link rel="canonical" href="[^"]*\/t\/salvage-rush\/b\/best">/);
++ assert.match(allTime.text, /<a href="\/t\/salvage-rush\/b\/best\?period=week">This week<\/a>/);
assert.doesNotMatch(board.text, /usr_|guest:/);
assert.strictEqual((await t.get('/t/nope')).status, 404);
assert.strictEqual((await t.get('/t/salvage-rush/b/nope')).status, 404);