From 6dc3c3b10fce4bd745396dce89ef122b09fd3bc4 Mon Sep 17 00:00:00 2001 From: NGPixel Date: Mon, 28 Sep 2026 13:07:52 -0400 Subject: [PATCH] feat: colorize broken links + update links on page move --- backend/api/pages.ts | 76 +++- backend/api/schemas/site.ts | 5 + backend/api/sites.ts | 5 + backend/helpers/linkRewrite.ts | 202 ++++++++++ backend/helpers/pageLinks.ts | 174 +++++++++ backend/locales/en.json | 13 +- backend/models/pageLinks.ts | 182 ++++++++- backend/models/pages.ts | 355 +++++++++++++++++- backend/models/sites.ts | 2 + backend/models/tree.ts | 15 + backend/tasks/workers/rebuild-page-links.ts | 13 +- .../_assets/icons/ultraviolet-broken-link.svg | 1 + frontend/src/components/FileManager.vue | 7 +- frontend/src/components/PageActionsCol.vue | 7 +- frontend/src/components/TreeBrowserDialog.vue | 22 +- frontend/src/css/_page-contents.scss | 16 + frontend/src/helpers/pageRelink.js | 37 ++ frontend/src/pages/AdminGeneral.vue | 15 + frontend/src/pages/Index.vue | 5 +- frontend/src/stores/page.js | 12 +- frontend/src/stores/site.js | 7 + 21 files changed, 1121 insertions(+), 50 deletions(-) create mode 100644 backend/helpers/linkRewrite.ts create mode 100644 frontend/public/_assets/icons/ultraviolet-broken-link.svg create mode 100644 frontend/src/helpers/pageRelink.js diff --git a/backend/api/pages.ts b/backend/api/pages.ts index d01e7baaa..37ff8393a 100644 --- a/backend/api/pages.ts +++ b/backend/api/pages.ts @@ -1155,7 +1155,7 @@ async function routes(app: FastifyInstance) { */ app.put<{ Params: { siteId: string; pageId: string } - Body: { path: string; locale?: string; title?: string } + Body: { path: string; locale?: string; title?: string; updateLinks?: boolean } }>( '/sites/:siteId/pages/:pageId/path', { @@ -1167,7 +1167,7 @@ async function routes(app: FastifyInstance) { schema: { summary: 'Move a page to another path', description: - 'Also renames it when a title is given, and moves it to another locale when one is given. The tree entry moves with it, any folder the new path needs is created, and the copy on every storage target follows.\n\nMoving between locales needs `manage:pages` at the destination as well as at the source, since page rules are granted per locale.', + 'Also renames it when a title is given, and moves it to another locale when one is given. The tree entry moves with it, any folder the new path needs is created, and the copy on every storage target follows.\n\nMoving between locales needs `manage:pages` at the destination as well as at the source, since page rules are granted per locale.\n\nWith `updateLinks`, every page linking to the old address by path is edited to point at the new one — its source and its stored render, as a new version with the move as its reason. Each needs `write:pages` of its own; a page the caller may not edit, or whose link could not be found in its source, is left alone and reported under `relinked.skipped`.', tags: ['Pages'], params: pageIdParam, body: { @@ -1188,6 +1188,12 @@ async function routes(app: FastifyInstance) { type: 'string', minLength: 1, maxLength: 255 + }, + updateLinks: { + type: 'boolean', + default: false, + description: + 'Rewrite the links of every page pointing at the old address so they point at the new one.' } } }, @@ -1198,7 +1204,38 @@ async function routes(app: FastifyInstance) { properties: { ok: { type: 'boolean' }, message: { type: 'string' }, - page: { $ref: 'Page#' } + page: { $ref: 'Page#' }, + relinked: { + type: 'object', + description: + 'Present when `updateLinks` was asked for and the page changed address.', + properties: { + updated: { + type: 'integer', + description: 'How many linking pages were edited.' + }, + skippedCount: { + type: 'integer', + description: + 'How many linking pages were left as they were, including any the caller may not read.' + }, + skipped: { + type: 'array', + description: + 'The skipped pages the caller may read. `forbidden`: the caller may not edit it. `notInSource`: the link could not be found in its source. `failed`: the save was refused.', + items: { + type: 'object', + properties: { + id: { type: 'string', format: 'uuid' }, + locale: { type: 'string' }, + path: { type: 'string' }, + title: { type: 'string' }, + reason: { type: 'string', enum: ['forbidden', 'notInSource', 'failed'] } + } + } + } + } + } } } } @@ -1247,6 +1284,22 @@ async function routes(app: FastifyInstance) { } const { page, versionId } = change + /* + After the move rather than inside it: the move is complete on its own, and what follows is a + set of ordinary edits to other people's pages, each asked about separately -- `write:pages` on + that page, which is what saving it from the editor would have taken. + */ + const relink = + req.body.updateLinks && (page.path !== target.path || page.locale !== target.locale) + ? await WIKI.models.pages.relinkMovedPage( + req.params.siteId, + page.id, + { locale: target.locale, path: target.path }, + actor, + (linking) => mayOnPage(req, 'write:pages', linking) + ) + : null + await audit(req, 'page', 'movePage', { pageId: page.id, siteId: req.params.siteId, @@ -1254,13 +1307,26 @@ async function routes(app: FastifyInstance) { path: page.path, previousLocale: target.locale, previousPath: target.path, - versionId + versionId, + // -> Each edited page by id and the version its edit produced, which is the record of it + ...(relink ? { relinked: relink.updated } : {}) }) return { ok: true, message: 'Page moved successfully.', - page + page, + ...(relink + ? { + relinked: { + updated: relink.updated.length, + skippedCount: relink.skipped.length, + // -> Named only where the caller may read the page: a page they may not edit may well + // be one whose title they are not allowed to know either + skipped: relink.skipped.filter((skip) => mayOnPage(req, 'read:pages', skip)) + } + } + : {}) } } ) diff --git a/backend/api/schemas/site.ts b/backend/api/schemas/site.ts index 0bbf3d028..c1f7e1b51 100644 --- a/backend/api/schemas/site.ts +++ b/backend/api/schemas/site.ts @@ -58,6 +58,11 @@ export async function registerSchemas(app: FastifyInstance): Promise { type: 'string' } }, + colorizeBrokenLinks: { + type: 'boolean', + description: + 'Whether links to pages that do not exist are drawn in red. Display only: every saved render marks those links with the `is-broken-link` class either way, and this decides whether the page view colours them.' + }, discoverable: { type: 'boolean' }, diff --git a/backend/api/sites.ts b/backend/api/sites.ts index 848a00c47..8f2ccd03f 100644 --- a/backend/api/sites.ts +++ b/backend/api/sites.ts @@ -21,6 +21,7 @@ const SITE_CONFIG_KEYS = [ 'footerExtra', 'banner', 'pageExtensions', + 'colorizeBrokenLinks', 'logoText', 'sitemap', 'discoverable', @@ -283,6 +284,7 @@ async function routes(app: FastifyInstance) { footerExtra?: string banner?: { isEnabled?: boolean; title?: string; content?: string } pageExtensions?: string[] + colorizeBrokenLinks?: boolean logoText?: boolean sitemap?: boolean discoverable?: boolean @@ -360,6 +362,9 @@ async function routes(app: FastifyInstance) { pattern: '^[a-z0-9]+$' } }, + colorizeBrokenLinks: { + $ref: 'Site#/properties/colorizeBrokenLinks' + }, logoText: { type: 'boolean' }, diff --git a/backend/helpers/linkRewrite.ts b/backend/helpers/linkRewrite.ts new file mode 100644 index 000000000..7fbde6616 --- /dev/null +++ b/backend/helpers/linkRewrite.ts @@ -0,0 +1,202 @@ +/** + * Rewriting a link in a page's SOURCE, for a page whose target moved. + * + * The render half is `rewriteRenderLinks` in `helpers/pageLinks.ts`, which has a parsed tree to work + * on. This half does not: there is no markdown or asciidoc parser on this side (a page's HTML is + * produced in the editor's browser), so a link is found by the syntax around it instead. What that + * buys is an edit that touches exactly the characters of the href and nothing else — no reformatted + * paragraph, no normalized list — which is what an author opening the page afterwards expects to see + * in its history. + * + * The href is looked for exactly as the `pageLinks` row recorded it, which is exactly as the render + * carried it. For both syntaxes that is the destination as written in the source, so the two agree; + * where they do not (an entity-encoded query, say) the link is simply not found, and the caller + * leaves that page alone rather than rewriting its render out from under its source. + * + * Code is left alone — a fenced block and an inline code span in markdown, a listing, literal or + * comment block in asciidoc — since a link there is an example of a link and not one. + */ + +/** What a page's content is, as far as finding a link in it goes. */ +export type SourceSyntax = 'markdown' | 'adoc' + +export interface SourceRewrite { + /** The source with every link that was found rewritten. */ + content: string + /** The old hrefs that were found at least once, and so rewritten. */ + replaced: Set +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +} + +/** Whether an href is a full URL, which is the only kind that can stand in prose on its own. */ +function isAbsoluteUrl(href: string): boolean { + return /^https?:\/\//i.test(href) +} + +/** + * Every place `href` can stand as a link destination, as a pattern whose first group is whatever + * comes before it — kept, and written back unchanged. + */ +function patternsFor(href: string, syntax: SourceSyntax): RegExp[] { + const h = escapeRegExp(href) + // -> Raw HTML is allowed in both, and an author reaching for it writes an ordinary anchor + const patterns = [new RegExp(`(\\bhref\\s*=\\s*["'])${h}(?=["'])`, 'g')] + + if (syntax === 'markdown') { + patterns.push( + // -> `[text](href)`, `[text]()`, `[text](href "title")` -- and the image form, which + // shares it + new RegExp(`(\\]\\(\\s*?(?:\\)|\\s))`, 'g'), + // -> A reference definition, `[label]: href`, at the start of its line + new RegExp(`(^[ \\t]{0,3}\\[[^\\]\\n]+\\]:[ \\t]*?(?:[ \\t]|$))`, 'gm') + ) + if (isAbsoluteUrl(href)) { + patterns.push( + // -> ``. Only for a full URL: `` is a closing tag, not a link to `/b` + new RegExp(`(<)${h}(?=>)`, 'g'), + // -> A bare URL, which the renderer's linkify turns into a link + new RegExp(`(^|[\\s(])${h}(?=[\\s).,;:!?]|$)`, 'gm') + ) + } + } else { + patterns.push( + // -> `link:href[text]`, and the passthrough form a target with odd characters needs + new RegExp(`(\\blink:)${h}(?=\\[)`, 'g'), + new RegExp(`(\\blink:\\+\\+)${h}(?=\\+\\+\\[)`, 'g') + ) + if (isAbsoluteUrl(href)) { + // -> `https://…[text]`, or a bare URL, which asciidoctor links on its own + patterns.push(new RegExp(`(^|[\\s(<])${h}(?=\\[|[\\s>).,;:!?]|$)`, 'gm')) + } + } + return patterns +} + +/** Whether a markdown line opens or closes a fence, and with what. */ +const FENCE = /^[ \t]{0,3}(`{3,}|~{3,})/ + +/** Delimiters of the asciidoc blocks whose content is not markup. */ +const ADOC_VERBATIM = /^(-{4,}|\.{4,}|\/{4,})[ \t]*$/ + +/** + * Split a source into the stretches a link may be rewritten in and the ones it may not. + * + * Line by line for the blocks, and within a markdown line for inline code spans — a backtick run + * closes only on a run of the same length, which is the rule markdown itself uses. + */ +function segments(source: string, syntax: SourceSyntax): { text: string; code: boolean }[] { + const out: { text: string; code: boolean }[] = [] + const push = (text: string, code: boolean) => { + const last = out.at(-1) + if (last && last.code === code) { + last.text += text + } else if (text) { + out.push({ text, code }) + } + } + + let fence: string | null = null + for (const line of source.split(/(?<=\n)/)) { + const bare = line.replace(/\r?\n$/, '') + if (syntax === 'markdown') { + const marker = FENCE.exec(bare)?.[1] + if (fence) { + push(line, true) + if ( + marker && + marker[0] === fence[0] && + marker.length >= fence.length && + !bare.trim().slice(marker.length).trim() + ) { + fence = null + } + continue + } + if (marker) { + fence = marker + push(line, true) + continue + } + // -> Inline code spans within an ordinary line + let rest = line + for (;;) { + const open = /`+/.exec(rest) + if (!open) { + push(rest, false) + break + } + const close = rest.indexOf(open[0], open.index + open[0].length) + if (close < 0) { + push(rest, false) + break + } + push(rest.slice(0, open.index), false) + push(rest.slice(open.index, close + open[0].length), true) + rest = rest.slice(close + open[0].length) + } + } else { + const marker = ADOC_VERBATIM.exec(bare)?.[1] + if (fence) { + push(line, true) + if (marker === fence) { + fence = null + } + continue + } + if (marker) { + fence = marker + push(line, true) + continue + } + push(line, false) + } + } + return out +} + +/** + * Rewrite every link destination in `source` that is one of `replacements`' keys. + * + * All of them in one pass over the segments, so that an href rewritten to something that is itself + * another key — a page moved onto the old path of another — is not rewritten twice. + */ +export function rewriteSourceLinks( + source: string, + syntax: SourceSyntax, + replacements: ReadonlyMap +): SourceRewrite { + const replaced = new Set() + if (!source || replacements.size < 1) { + return { content: source, replaced } + } + + // -> Longest first, so `/guides/setup` is tried before `/guides` wherever both could match + const hrefs = [...replacements.keys()].sort((a, b) => b.length - a.length) + const compiled = hrefs.map((href) => ({ href, patterns: patternsFor(href, syntax) })) + + const content = segments(source, syntax) + .map((segment) => { + if (segment.code) { + return segment.text + } + // -> Placeholders first, then the real values, so one replacement cannot feed the next + const placed: string[] = [] + let text = segment.text + for (const { href, patterns } of compiled) { + for (const pattern of patterns) { + text = text.replace(pattern, (_match, before: string) => { + replaced.add(href) + placed.push(replacements.get(href)!) + return `${before}\uE000${placed.length - 1}\uE000` + }) + } + } + return text.replace(/\uE000(\d+)\uE000/g, (_match, index: string) => placed[Number(index)]) + }) + .join('') + + return { content, replaced } +} diff --git a/backend/helpers/pageLinks.ts b/backend/helpers/pageLinks.ts index 08d9890d6..70ec62ddc 100644 --- a/backend/helpers/pageLinks.ts +++ b/backend/helpers/pageLinks.ts @@ -1,3 +1,4 @@ +import path from 'node:path' import * as cheerio from 'cheerio' import { isPageUrl, normalizePagePath, splitLocalePath, stripPageExtension } from './common.ts' @@ -62,6 +63,13 @@ export interface ResolvedLink { targetRef: string | null } +/** Where a page sits, which is what a link to it has to say. */ +export interface LinkTarget { + siteId: string + locale: string + path: string +} + /** The page a link is written on, which is what a relative href resolves against. */ export interface LinkSource { siteId: string @@ -270,3 +278,169 @@ function resolveFile(href: string, urlPath: string, targetSiteId: string): Resol targetRef: null } } + +/** + * The class a stored render puts on a link to a page that is not there — a red link. + * + * Owned by the server, not the author: it is set and cleared on every anchor each time a render is + * checked, so one written by hand is taken off again wherever the page it points at exists. Whether it + * is DRAWN red is a separate, per-site question the page view answers with a class on the contents + * container (`colorizeBrokenLinks`), which is what lets the setting change without a single page + * being rewritten. `frontend/src/css/_page-contents.scss` is the other half of this name. + */ +export const BROKEN_LINK_CLASS = 'is-broken-link' + +/** + * Put the broken-link class on exactly the anchors whose href is in `broken`, and take it off every + * other one. + * + * Matched on the href as written (trimmed, as `resolveLink` keys it), which is what the `pageLinks` + * rows hold — so the same href written twice on a page is marked twice, and two spellings of one + * missing page are both marked. + * + * @returns The render with the classes corrected, or null when not one anchor had to change — the + * common case, and one that must not cost a write. + */ +export function markBrokenLinks(html: string, broken: ReadonlySet): string | null { + // -> Nothing to add, and nothing there to take off: skip the parse. The substring test can only + // give a false positive (an author's own text mentioning the name), never a false negative + if (broken.size < 1 && !html.includes(BROKEN_LINK_CLASS)) { + return null + } + + const $ = cheerio.load(html, null, false) + let changed = false + for (const el of $('a')) { + const anchor = $(el) + const isBroken = broken.has((anchor.attr('href') ?? '').trim()) + if (isBroken === anchor.hasClass(BROKEN_LINK_CLASS)) { + continue + } + changed = true + if (isBroken) { + anchor.addClass(BROKEN_LINK_CLASS) + } else { + anchor.removeClass(BROKEN_LINK_CLASS) + // -> A link that only ever had this class goes back to having none, rather than `class=""` + if (!(anchor.attr('class') ?? '').trim()) { + anchor.removeAttr('class') + } + } + } + return changed ? $.html() : null +} + +/** + * The same link, rewritten to point at where its target now is — for a page that moved. + * + * Written the way the author wrote it rather than normalized, because the href is going back into + * their source: a relative link stays relative (from where the page holding it now sits), an absolute + * path stays absolute, a full URL keeps its host, and whatever the old one carried beyond the page — + * a locale prefix it did not strictly need, a page extension, a query, a fragment — is carried over. + * The one thing deliberately not kept is a trailing slash, which the resolver ignores anyway. + * + * Checked against `resolveLink` before it is handed back, so a spelling that would land somewhere + * else — a relative climb that no longer fits, a prefix the site stopped using — falls back to the + * plain absolute path, which always lands. + * + * @param source Where the page holding the link sits NOW, which is what a relative href resolves + * against — for a page linking to itself, that is its new address. + * @returns The new href, or null when `href` does not address a page at all. + */ +export function relinkHref(href: string, source: LinkSource, target: LinkTarget): string | null { + const raw = (href ?? '').trim() + const resolved = resolveLink(raw, source) + if (resolved?.kind !== 'page') { + return null + } + const url = new URL( + raw, + `${LOCAL_ORIGIN}${WIKI.models.pages.urlFor(source.siteId, source.locale, source.path)}` + ) + + // -> The query and fragment exactly as written, which is not always how `URL` re-serializes them + const suffixAt = raw.search(/[?#]/) + const suffix = suffixAt < 0 ? '' : raw.slice(suffixAt) + + const site = WIKI.sites?.[target.siteId] + const trimmed = + url.pathname.length > 1 && url.pathname.endsWith('/') ? url.pathname.slice(0, -1) : url.pathname + const withoutExtension = stripPageExtension(trimmed, site?.config?.pageExtensions) + const extension = withoutExtension === null ? '' : trimmed.slice(withoutExtension.length) + const hadPrefix = Boolean( + splitLocalePath( + withoutExtension ?? trimmed, + WIKI.models.locales.urlPrefixesFor(site?.config?.locales?.active) + ) + ) + + const plainPath = WIKI.models.pages.urlFor(target.siteId, target.locale, target.path) + let urlPath = hadPrefix + ? `/${WIKI.models.locales.shortCodeFor(target.locale)}/${target.path}` + : plainPath + // -> `/.md` addresses nothing, so the site root never takes an extension + if (extension && target.path) { + urlPath += extension + } + // -> A prefixed root is `/fr/`, which reads as `/fr` either way; written without the slash + if (urlPath.length > 1 && urlPath.endsWith('/')) { + urlPath = urlPath.slice(0, -1) + } + + const hasScheme = /^[a-z][a-z\d+.-]*:/i.test(raw) + let rewritten: string + if (hasScheme) { + rewritten = `${url.origin}${urlPath}${suffix}` + } else if (raw.startsWith('//')) { + rewritten = `//${url.host}${urlPath}${suffix}` + } else if (raw.startsWith('/')) { + rewritten = `${urlPath}${suffix}` + } else { + const from = path.posix.dirname( + WIKI.models.pages.urlFor(source.siteId, source.locale, source.path) + ) + const relative = path.posix.relative(from, urlPath) + rewritten = relative ? `${relative}${suffix}` : `${urlPath}${suffix}` + } + + const check = resolveLink(rewritten, source) + const lands = + check?.kind === 'page' && + check.targetSiteId === target.siteId && + check.targetLocale === target.locale && + check.targetPath === target.path + if (lands) { + return rewritten + } + return hasScheme ? `${url.origin}${plainPath}${suffix}` : `${plainPath}${suffix}` +} + +/** + * Replace hrefs in a render, anchor by anchor. + * + * The render half of relinking a moved page: the source is rewritten by `helpers/linkRewrite.ts`, and + * the stored HTML has to say the same thing without waiting for somebody to re-render it — the + * render is what a reader is served. + * + * @param replacements Old href (trimmed, as the `pageLinks` rows hold it) to new href. + * @returns The rewritten render, or null when no anchor carried any of them. + */ +export function rewriteRenderLinks( + html: string, + replacements: ReadonlyMap +): string | null { + if (replacements.size < 1 || !html) { + return null + } + const $ = cheerio.load(html, null, false) + let changed = false + for (const el of $('a[href]')) { + const anchor = $(el) + const replacement = replacements.get((anchor.attr('href') ?? '').trim()) + if (replacement !== undefined) { + anchor.attr('href', replacement) + changed = true + } + } + return changed ? $.html() : null +} diff --git a/backend/locales/en.json b/backend/locales/en.json index 62b3d1b52..827e43d30 100644 --- a/backend/locales/en.json +++ b/backend/locales/en.json @@ -504,6 +504,8 @@ "admin.general.bannerEnabledHint": "Display a notice at the top of every page of this site.", "admin.general.bannerTitle": "Banner Title", "admin.general.bannerTitleHint": "Heading shown above the banner contents. Leave empty to hide.", + "admin.general.colorizeBrokenLinks": "Colorize Broken Links", + "admin.general.colorizeBrokenLinksHint": "Show links to pages that do not exist yet in red, so it is easy to spot which pages still need to be written.", "admin.general.companyName": "Company / Organization Name", "admin.general.companyNameHint": "Name to use when displaying copyright notice in the footer. Leave empty to hide.", "admin.general.contentLicense": "Content License", @@ -1582,10 +1584,10 @@ "admin.utilities.purgeSampleHint": "Delete every page on this site tagged \"test\", which is what Generate Sample Content tags the pages it writes.", "admin.utilities.purgeSampleSuccess": "There was no sample content to delete. | Deleted 1 page. | Deleted {count} pages.", "admin.utilities.rebuildPageLinks": "Rebuild Page Links", - "admin.utilities.rebuildPageLinksConfirm": "Every page on every site will be read again to work out what it links to.", - "admin.utilities.rebuildPageLinksConfirmWarn": "Nothing about your content is changed — only the record of which page points at which. This normally keeps itself up to date, so it is worth running after importing content written elsewhere, or after changing a site's locale prefixes or page extensions.", + "admin.utilities.rebuildPageLinksConfirm": "Every page on every site will be read again to work out what it links to, and its links to pages that do not exist will be marked as broken.", + "admin.utilities.rebuildPageLinksConfirmWarn": "Nothing you wrote is changed — only the record of which page points at which, and the marking of broken links in each page's stored HTML. Both normally keep themselves up to date, so this is worth running on a wiki with pages saved before broken links were marked, after importing content written elsewhere, or after changing a site's locale prefixes or page extensions.", "admin.utilities.rebuildPageLinksFailed": "Failed to queue a page link rebuild.", - "admin.utilities.rebuildPageLinksHint": "Work out again what every page links to, from the content already stored. Runs in the background.", + "admin.utilities.rebuildPageLinksHint": "Work out again what every page links to, from the content already stored, and mark the links that point to pages that do not exist. Runs in the background.", "admin.utilities.rebuildPageLinksSuccess": "A page link rebuild has been initiated and will start shortly.", "admin.utilities.rebuildPageRatings": "Rebuild Page Ratings", "admin.utilities.rebuildPageRatingsConfirm": "The ratings shown on every page, on every site, will be counted again from the individual ratings readers gave.", @@ -2972,6 +2974,11 @@ "pageDeleteDialog.title": "Confirm Page Deletion", "pageDuplicateDialog.title": "Duplicate and Save As...", "pageRenameDialog.title": "Rename / Move to...", + "pageRenameDialog.updateLinks": "Update pages linking to this page with the new location", + "pageRenameDialog.updateLinksDone": "Updated the links on {count} page(s).", + "pageRenameDialog.updateLinksHint": "Rewrites the links in their source and in their rendered content. Pages you are not allowed to edit are left as they are.", + "pageRenameDialog.updateLinksReason": "Updated links to a page that moved from {from} to {to}", + "pageRenameDialog.updateLinksSkipped": "{count} page(s) still link to the old location and have to be updated by hand: {pages}", "pageSaveDialog.displayModePath": "Browse Using Paths", "pageSaveDialog.displayModeTitle": "Browse Using Titles", "pageSaveDialog.loadFailed": "Failed to load folder tree.", diff --git a/backend/models/pageLinks.ts b/backend/models/pageLinks.ts index bbdf1a9ba..bf664b783 100644 --- a/backend/models/pageLinks.ts +++ b/backend/models/pageLinks.ts @@ -1,7 +1,7 @@ -import { and, eq, ne, or, sql } from 'drizzle-orm' +import { and, eq, inArray, ne, or, sql } from 'drizzle-orm' import { pageLinks as pageLinksTable, pages as pagesTable } from '../db/schema.ts' -import { linksFromRender, resolveLink } from '../helpers/pageLinks.ts' -import type { LinkSource, PageLinkKind, ResolvedLink } from '../helpers/pageLinks.ts' +import { linksFromRender, markBrokenLinks, resolveLink } from '../helpers/pageLinks.ts' +import type { LinkSource, LinkTarget, PageLinkKind, ResolvedLink } from '../helpers/pageLinks.ts' /** * Page links model @@ -93,13 +93,26 @@ export interface OutboundLink extends PageLinkRow { targetPageTags: string[] | null } -/** Where a page sits, which is how a link addresses it. */ -export interface LinkTarget { - siteId: string - locale: string - path: string +/** + * Every way a link can address the pages that just appeared or went, on one site. + * + * All three, because a page is reachable by all three: a link written as `/i/` to a page that was + * deleted is exactly as broken as one written as its path. + */ +export interface ChangedTargets { + paths?: { locale: string; path: string }[] + pageIds?: string[] + aliases?: (string | null | undefined)[] } +/** + * How many targets one referrer lookup asks about at once. + * + * A folder rename hands over every page beneath it, and each path target is two bind parameters — + * postgres stops at 65535 of them per statement, well short of a large folder. + */ +const TARGETS_PER_QUERY = 1000 + class PageLinks { /** * Whether this site shows what links to a page. @@ -129,12 +142,19 @@ class PageLinks { * - **The page's relations**, the sidebar links its properties dialog collects. Stored as their own * column, written by the same link picker that writes one into content, and just as breakable. * + * And then brings the render's red links into line with what was just stored — see + * `syncBrokenLinks`. Here rather than beside each caller, because every caller is somewhere a link + * could have started or stopped resolving: a save changed the links, a move changed what the + * relative ones address, and the rebuild utility is asked to put all of it right at once. + * * @param renderHrefs What `postProcess` already read out of the render it just produced, for a save * that is storing one. Absent, the stored render is parsed instead — which is the * case for a page that MOVED: nothing about it changed, but every relative link * on it now resolves somewhere else. + * @returns The render as it now stands when marking changed it, or null when the stored one was + * already right — so a caller holding a copy of the page can keep it current. */ - async refreshForPage(page: PageLinkSource, renderHrefs?: string[]): Promise { + async refreshForPage(page: PageLinkSource, renderHrefs?: string[]): Promise { const hrefs = renderHrefs ?? linksFromRender(page.render) if (page.editor === REDIRECT_EDITOR && page.content) { @@ -163,6 +183,8 @@ class PageLinks { { pageId: page.id, siteId: page.siteId, locale: page.locale, path: page.path }, hrefs ) + + return this.syncBrokenLinks(page.id, page.render) } /** @@ -171,8 +193,14 @@ class PageLinks { * What everything but a create reaches for: a save, a move, a folder rename and the rebuild utility * all need the page as it stands AFTER their write, so reading it back is the point rather than an * overhead. + * + * @returns As `refreshForPage`: the corrected render, or null when it did not change. */ - async refreshById(siteId: string, pageId: string, renderHrefs?: string[]): Promise { + async refreshById( + siteId: string, + pageId: string, + renderHrefs?: string[] + ): Promise { const rows = await WIKI.db .select({ id: pagesTable.id, @@ -189,8 +217,138 @@ class PageLinks { .limit(1) // -> Gone while the save that asked for this was in flight. Its rows went with it - if (rows[0]) { - await this.refreshForPage(rows[0] as PageLinkSource, renderHrefs) + return rows[0] ? this.refreshForPage(rows[0] as PageLinkSource, renderHrefs) : null + } + + /** + * Put the broken-link class on the links of a page's render that point at nothing, and take it off + * the ones that do. + * + * **Always done, whatever the site's `colorizeBrokenLinks` says.** That setting only decides whether + * the class is drawn red; the class itself is kept true regardless, so that switching it on shows + * every red link at once instead of only those on pages saved since. + * + * "Points at nothing" is `outboundFor`'s answer — the join against `pages` that the Links tab draws + * its red links from — so the two cannot disagree about a link. Only the three page kinds count: a + * file link is recorded but never resolved (see `outboundFor`), and an href with no row at all is + * not a link into the wiki. + * + * Written without touching `updatedAt`, for the reason `storeRender` gives: nothing about the page + * changed, a page it points at did. And written only over the render that was read, so a save + * landing in between wins rather than being overwritten with a marked copy of what it replaced — + * that save checks its own links anyway. + * + * @param render The render as stored, for a caller that already has it in hand. Absent, it is read. + * @returns The corrected render, or null when nothing on it had to change. + */ + async syncBrokenLinks(pageId: string, render?: string | null): Promise { + if (render === undefined) { + const rows = await WIKI.db + .select({ render: pagesTable.render }) + .from(pagesTable) + .where(eq(pagesTable.id, pageId)) + .limit(1) + render = rows[0]?.render + } + if (!render) { + return null + } + + const broken = new Set( + (await this.outboundFor(pageId)) + .filter((link) => link.kind !== 'asset' && !link.targetPageId) + .map((link) => link.href) + ) + const marked = markBrokenLinks(render, broken) + if (marked === null) { + return null + } + + const updated = await WIKI.db + .update(pagesTable) + .set({ render: marked }) + .where(and(eq(pagesTable.id, pageId), eq(pagesTable.render, render))) + .returning({ id: pagesTable.id }) + return updated.length > 0 ? marked : null + } + + /** + * Re-check the red links of every page pointing at pages that just appeared or went. + * + * The other direction from `syncBrokenLinks` on the page being saved: a page created at a path + * somebody had already linked to turns their link good, and a page deleted turns every link to it + * red, without either of those pages being touched by anybody. Only the renders are rewritten — the + * `pageLinks` rows of those pages say where they point, and that has not changed. + * + * Nothing here asks whether a page exists; the callers pass what changed and `syncBrokenLinks` + * works the answer out from the table as it now stands, so a move can simply hand over both the + * path it left and the one it arrived at. + * + * @param siteId The site the changed pages are on. Referrers may be on any site of the instance. + * @param exceptPageId A page to leave out — the one being saved, which has just checked itself. + */ + async refreshLinksTo( + siteId: string, + targets: ChangedTargets, + exceptPageId?: string + ): Promise { + const paths = targets.paths ?? [] + const pageIds = targets.pageIds ?? [] + const aliases = (targets.aliases ?? []).filter((alias): alias is string => Boolean(alias)) + + const conditions: any[] = [] + for (let offset = 0; offset < paths.length; offset += TARGETS_PER_QUERY) { + const batch = paths.slice(offset, offset + TARGETS_PER_QUERY) + conditions.push( + and( + eq(pageLinksTable.kind, 'page'), + or( + ...batch.map((target) => + and( + eq(pageLinksTable.targetLocale, target.locale), + eq(pageLinksTable.targetPath, target.path) + ) + ) + ) + ) + ) + } + for (const [kind, refs] of [ + ['pageId', pageIds], + ['alias', aliases] + ] as const) { + for (let offset = 0; offset < refs.length; offset += TARGETS_PER_QUERY) { + conditions.push( + and( + eq(pageLinksTable.kind, kind), + inArray(pageLinksTable.targetRef, refs.slice(offset, offset + TARGETS_PER_QUERY)) + ) + ) + } + } + + const referrers = new Set() + for (const condition of conditions) { + const rows = await WIKI.db + .selectDistinct({ pageId: pageLinksTable.pageId }) + .from(pageLinksTable) + .where(and(eq(pageLinksTable.targetSiteId, siteId), condition)) + for (const row of rows) { + referrers.add(row.pageId) + } + } + if (exceptPageId) { + referrers.delete(exceptPageId) + } + + for (const pageId of referrers) { + // -> One page's render failing to parse is not a reason for the save that caused this to fail, + // nor for the rest of them to keep a stale colour + try { + await this.syncBrokenLinks(pageId) + } catch (err: any) { + WIKI.logger.warn(`Could not update the broken links of page ${pageId}: ${err.message}`) + } } } diff --git a/backend/models/pages.ts b/backend/models/pages.ts index aa5903b10..f65b3e36e 100644 --- a/backend/models/pages.ts +++ b/backend/models/pages.ts @@ -1,5 +1,10 @@ import { and, desc, eq, inArray, isNull, ne, notInArray, sql } from 'drizzle-orm' -import { pages as pagesTable, tree as treeTable, users as usersTable } from '../db/schema.ts' +import { + pageLinks as pageLinksTable, + pages as pagesTable, + tree as treeTable, + users as usersTable +} from '../db/schema.ts' import { CustomError, generatePathHash, @@ -7,6 +12,9 @@ import { timingSafeCompare } from '../helpers/common.ts' import { invalidateAppShellCache } from '../helpers/appShell.ts' +import { rewriteSourceLinks } from '../helpers/linkRewrite.ts' +import type { SourceSyntax } from '../helpers/linkRewrite.ts' +import { relinkHref, rewriteRenderLinks } from '../helpers/pageLinks.ts' import type { AccessActor } from './groups.ts' import type { RulePageRef } from '../helpers/pageRules.ts' import type { FastifyRequest } from 'fastify' @@ -473,6 +481,37 @@ export interface PageActor { permissions: string[] } +/** A page that links to a moved page's old address and was left alone, and why. */ +export interface RelinkSkip { + id: string + siteId: string + locale: string + path: string + title: string + /** What a page rule may address it by, so the caller can ask whether this page may be named. */ + tags: string[] + /** + * `forbidden` — the mover may not edit it. `notInSource` — the link is in its render but could not + * be found in its source in a form that can be rewritten, so rewriting the render alone would only + * last until the next render from that source. `failed` — the save was refused, and the page left + * as it was. + */ + reason: 'forbidden' | 'notInSource' | 'failed' +} + +export interface RelinkResult { + /** Each page whose links were rewritten, with the version recording it. */ + updated: { id: string; versionId: string | null }[] + skipped: RelinkSkip[] +} + +/** Which source syntax a link is looked for in, per editor. The rest keep links in JSON, or none. */ +const SOURCE_SYNTAX: Record = { + markdown: 'markdown', + visual: 'markdown', + asciidoc: 'adoc' +} + /** * A page as it stands after a change, together with the history version that change produced. * @@ -1847,6 +1886,12 @@ class Pages { // -> What this page points at, which is only knowable once it has an id and an address of its // own: a relative link resolves against the page holding it await WIKI.models.pageLinks.refreshForPage(page, links) + // -> And every link somebody had already written to this path or alias stops being a red one + await WIKI.models.pageLinks.refreshLinksTo( + siteId, + { paths: [{ locale, path }], aliases: [alias] }, + page.id + ) const versionId = await WIKI.models.pageHistory.record({ siteId, @@ -2020,7 +2065,19 @@ class Pages { those changes what the page links to without the render moving at all. Read back rather than assembled from the patch, for the same reason `changedFields` is worked out against the row. */ - await WIKI.models.pageLinks.refreshById(siteId, id, renderHrefs) + const markedRender = await WIKI.models.pageLinks.refreshById(siteId, id, renderHrefs) + // -> Read back before its red links were marked, and it is what the editor is handed to draw + if (markedRender !== null) { + updated.render = markedRender + } + // -> `/a/` links follow the alias: the old one now points at nothing, the new one at this + if (values.alias !== undefined && values.alias !== existing.alias) { + await WIKI.models.pageLinks.refreshLinksTo( + siteId, + { aliases: [existing.alias, values.alias] }, + id + ) + } const versionId = await WIKI.models.pageHistory.record({ siteId, @@ -2257,7 +2314,27 @@ class Pages { moved page itself carries resolved against the folder it used to sit in, and now resolves against a different one. Nothing about its content changed and every one of its links may have. */ - await WIKI.models.pageLinks.refreshById(siteId, id) + const markedRender = await WIKI.models.pageLinks.refreshById(siteId, id) + if (markedRender !== null) { + moved.render = markedRender + } + /* + And the pages pointing at it, whose rows still say where it was -- which is exactly what turns + their links red. The path it arrived at is handed over too, since a link somebody wrote to where + the page now is has just started resolving. + */ + if (isRelocated) { + await WIKI.models.pageLinks.refreshLinksTo( + siteId, + { + paths: [ + { locale: page.locale, path: page.path }, + { locale: newLocale, path: newPath } + ] + }, + id + ) + } // -> Moved and then rewritten, rather than deleted and written afresh: the move is what keeps a // versioned target's history of the file attached to it, and the rewrite is because a move may @@ -2278,6 +2355,231 @@ class Pages { return { page: moved, versionId } } + /** + * Point every page linking to where a page USED to be at where it is now. + * + * The optional half of a move, asked for from the rename dialog. A move on its own leaves those links + * saying the old address on purpose — that is what makes them findable as the ones it broke (see + * `movePage`) — and this is the repair. Only links written as a PATH: `/a/` and `/i/` + * survive a move by design, and have nothing to repair. + * + * Each page is edited the way an author would edit it, through `updatePage`: its source is + * rewritten (and the stored render with it, so readers see the fix without waiting on a re-render), + * which gives it a version in its history naming the move as the reason, a fresh copy on every + * storage target, re-derived links and red links, and a webhook. The source is rewritten in place, + * href by href, and nothing else in it is touched — see `helpers/linkRewrite.ts`. + * + * The render is never rewritten on its own. A link that is in the render but cannot be found in the + * source in a form that can be rewritten is left as it is and reported, because a fixed render over + * an unfixed source would break again the next time the page is rendered. + * + * @param previous Where the page was before the move. + * @param mayEdit Whether the mover may edit a given page. Every page here is somebody else's, so this + * is asked of each one, and a page they may not edit is reported rather than changed — + * moving a page is not a way to write to pages the mover could not have saved. + */ + async relinkMovedPage( + siteId: string, + id: string, + previous: { locale: string; path: string }, + actor: PageActor, + mayEdit: (page: RulePageRef) => boolean + ): Promise { + const result: RelinkResult = { updated: [], skipped: [] } + const [moved] = await WIKI.db + .select({ locale: pagesTable.locale, path: pagesTable.path }) + .from(pagesTable) + .where(and(eq(pagesTable.id, id), eq(pagesTable.siteId, siteId))) + .limit(1) + if (!moved || (moved.locale === previous.locale && moved.path === previous.path)) { + return result + } + const target = { siteId, locale: moved.locale, path: moved.path } + + const rows = await WIKI.db + .select({ pageId: pageLinksTable.pageId, href: pageLinksTable.href }) + .from(pageLinksTable) + .where( + and( + eq(pageLinksTable.targetSiteId, siteId), + eq(pageLinksTable.kind, 'page'), + eq(pageLinksTable.targetLocale, previous.locale), + eq(pageLinksTable.targetPath, previous.path) + ) + ) + const hrefsByPage = new Map() + for (const row of rows) { + hrefsByPage.set(row.pageId, [...(hrefsByPage.get(row.pageId) ?? []), row.href]) + } + if (hrefsByPage.size < 1) { + return result + } + + // -> In the wiki's own language, since it is read in every linking page's history -- which may be + // on several sites, and is not the mover's to choose + const { t } = await WIKI.models.locales.translator(WIKI.sites[siteId]?.config?.locales?.primary) + const reasonForChange = t('pageRenameDialog.updateLinksReason', { + from: this.urlFor(siteId, previous.locale, previous.path), + to: this.urlFor(siteId, moved.locale, moved.path) + }) + + for (const [pageId, hrefs] of hrefsByPage) { + const [page] = await WIKI.db + .select({ + id: pagesTable.id, + siteId: pagesTable.siteId, + locale: pagesTable.locale, + path: pagesTable.path, + title: pagesTable.title, + tags: pagesTable.tags, + editor: pagesTable.editor, + content: pagesTable.content, + render: pagesTable.render, + relations: pagesTable.relations + }) + .from(pagesTable) + .where(eq(pagesTable.id, pageId)) + .limit(1) + if (!page) { + continue + } + const skip = (reason: RelinkSkip['reason']) => + result.skipped.push({ + id: page.id, + siteId: page.siteId, + locale: page.locale, + path: page.path, + title: page.title, + tags: page.tags ?? [], + reason + }) + if ( + !mayEdit({ + siteId: page.siteId, + locale: page.locale, + path: page.path, + tags: page.tags ?? [] + }) + ) { + skip('forbidden') + continue + } + + // -> Resolved from where the linking page sits NOW, which for the moved page itself is its new + // address: a relative link is rewritten relative to it + const replacements = new Map() + for (const href of hrefs) { + const next = relinkHref(href, page, target) + if (next !== null && next !== href) { + replacements.set(href, next) + } + } + if (replacements.size < 1) { + continue + } + + const patch: Partial = { reasonForChange } + const found = new Set() + const content = page.content ?? '' + const syntax = SOURCE_SYNTAX[page.editor] + if (syntax) { + const rewrite = rewriteSourceLinks(content, syntax, replacements) + if (rewrite.replaced.size > 0) { + patch.content = rewrite.content + rewrite.replaced.forEach((href) => found.add(href)) + } + } else if (page.editor === REDIRECT_EDITOR || page.editor === 'excalidraw') { + try { + const parsed = JSON.parse(content) + if (page.editor === REDIRECT_EDITOR) { + const next = replacements.get(String(parsed?.target ?? '').trim()) + if (next !== undefined) { + found.add(parsed.target.trim()) + parsed.target = next + patch.content = JSON.stringify(parsed) + } + } else { + // -> An element's own link, which the export wraps the element in an anchor for. Written + // back the way `serializeAsJSON` writes a scene + for (const element of Array.isArray(parsed?.elements) ? parsed.elements : []) { + const next = + typeof element?.link === 'string' + ? replacements.get(element.link.trim()) + : undefined + if (next !== undefined) { + found.add(element.link.trim()) + element.link = next + } + } + if (found.size > 0) { + patch.content = JSON.stringify(parsed, null, 2) + } + } + } catch { + // -> Content that will not parse holds no link this can find + } + } + + // -> The sidebar relations, which are links too and are stored as a column of their own + if (Array.isArray(page.relations)) { + let relationsChanged = false + const relations = (page.relations as any[]).map((relation) => { + const next = + typeof relation?.target === 'string' + ? replacements.get(relation.target.trim()) + : undefined + if (next === undefined) { + return relation + } + found.add(relation.target.trim()) + relationsChanged = true + return { ...relation, target: next } + }) + if (relationsChanged) { + patch.relations = relations + } + } + + if (found.size < 1) { + skip('notInSource') + continue + } + + // -> Only the hrefs that were fixed in the source, so the render never says something the source + // does not. Written ahead of the save, which re-derives the page's links from it + const render = rewriteRenderLinks( + page.render ?? '', + new Map([...replacements].filter(([href]) => found.has(href))) + ) + if (render !== null) { + await WIKI.db.update(pagesTable).set({ render }).where(eq(pagesTable.id, page.id)) + } + + /* + One page at a time, and one failing does not stop the rest: the move itself has already + happened, and the other pages are no less worth fixing. The render goes back to what it was, so + a page whose save was refused is left exactly as it stood. + */ + try { + const change = await this.updatePage(page.siteId, page.id, patch, actor) + if (change) { + result.updated.push({ id: page.id, versionId: change.versionId }) + } + } catch (err: any) { + WIKI.logger.warn(`Could not update the links of page ${page.id}: ${err.message}`) + if (render !== null) { + await WIKI.db + .update(pagesTable) + .set({ render: page.render }) + .where(and(eq(pagesTable.id, page.id), eq(pagesTable.render, render))) + } + skip('failed') + } + } + + return result + } + /** * Delete a page and its tree entry. * @@ -2302,6 +2604,12 @@ class Pages { await this.detachFromLocaleGroup(siteId, id) await WIKI.db.delete(pagesTable).where(eq(pagesTable.id, id)) await WIKI.models.tree.deleteEntry(id) + // -> Every link to it, by any of the three ways a link can name it, now points at nothing + await WIKI.models.pageLinks.refreshLinksTo(siteId, { + paths: [{ locale: page.locale, path: page.path }], + pageIds: [id], + aliases: [page.alias] + }) // -> A page that overrode the sidebar owns a menu keyed by its own id, which nothing could reach // once the page is gone await WIKI.models.navigation.deleteNavForEntries([id]) @@ -2499,6 +2807,12 @@ class Pages { } await WIKI.models.pageLinks.refreshForPage(page, links) + // -> Under its own id, so a `/i/` link written before it was deleted resolves again as well + await WIKI.models.pageLinks.refreshLinksTo( + siteId, + { paths: [{ locale, path }], pageIds: [page.id], aliases: [page.alias] }, + page.id + ) const versionId = await WIKI.models.pageHistory.record({ siteId, @@ -2581,20 +2895,18 @@ class Pages { }) } // -> Read before the rows go: what a target filed each page under is decided by its content type, - // and guessing would mean reaching for names that may belong to the assets beside them - const contentTypes = new Map( - ( - await WIKI.db - .select({ id: pagesTable.id, contentType: pagesTable.contentType }) - .from(pagesTable) - .where( - inArray( - pagesTable.id, - entries.map((entry) => entry.id) - ) - ) - ).map((row) => [row.id, row.contentType]) - ) + // and guessing would mean reaching for names that may belong to the assets beside them. The + // aliases are for the links that name a page by one + const doomed = await WIKI.db + .select({ id: pagesTable.id, contentType: pagesTable.contentType, alias: pagesTable.alias }) + .from(pagesTable) + .where( + inArray( + pagesTable.id, + entries.map((entry) => entry.id) + ) + ) + const contentTypes = new Map(doomed.map((row) => [row.id, row.contentType])) // -> Out of their sets of translations first, for the same reason `deletePage` does it await this.detachFromLocaleGroups( siteId, @@ -2606,6 +2918,15 @@ class Pages { entries.map((entry) => entry.id) ) ) + // -> As for a single page: whatever linked to any of them is now a red link + await WIKI.models.pageLinks.refreshLinksTo(siteId, { + paths: entries.map((entry) => ({ + locale: entry.locale, + path: entry.folderPath ? `${entry.folderPath}/${entry.fileName}` : entry.fileName + })), + pageIds: entries.map((entry) => entry.id), + aliases: doomed.map((row) => row.alias) + }) // -> One per page, as deleting them one at a time would have sent: a subscriber mirroring the // wiki has to hear about each page, not about the folder it happened to sit in diff --git a/backend/models/sites.ts b/backend/models/sites.ts index df951444d..3113de0fb 100644 --- a/backend/models/sites.ts +++ b/backend/models/sites.ts @@ -118,6 +118,7 @@ class Sites { content: '' }, pageExtensions: ['md', 'html', 'txt'], + colorizeBrokenLinks: true, discoverable: false, defaults: { tocDepth: { @@ -447,6 +448,7 @@ class Sites { content: '' }, pageExtensions: ['md', 'html', 'txt'], + colorizeBrokenLinks: true, discoverable: false, defaults: { tocDepth: { diff --git a/backend/models/tree.ts b/backend/models/tree.ts index 533ed9806..b5fd24d10 100644 --- a/backend/models/tree.ts +++ b/backend/models/tree.ts @@ -1009,6 +1009,14 @@ class Tree { */ await WIKI.models.pageLinks.refreshById(folder.siteId, page.id) } + // -> Those links are left pointing where the pages were, so they turn red -- and anything already + // linking to where the pages are now turns good + await WIKI.models.pageLinks.refreshLinksTo(folder.siteId, { + paths: movedPages.flatMap((page) => [ + { locale: page.locale, path: page.previousPath }, + { locale: page.locale, path: page.path } + ]) + }) // -> A storage target that lays its content out by path has every one of those files to move. // Asked for after the rows are correct, so that where each file belongs is read off the tree @@ -1483,6 +1491,13 @@ class Tree { // were. This move can cross locales as well, which changes what a link's prefix means too await WIKI.models.pageLinks.refreshById(siteId, page.id) } + // -> As in `renameFolder`, for both ends of the move + await WIKI.models.pageLinks.refreshLinksTo(siteId, { + paths: movedPages.flatMap((page) => [ + { locale: folder.locale, path: page.previousPath }, + { locale: destinationLocale, path: page.path } + ]) + }) // -> A storage target that lays its content out by path has every one of those files to move. // Asked for after the rows are correct, so that where each file belongs is read off the tree diff --git a/backend/tasks/workers/rebuild-page-links.ts b/backend/tasks/workers/rebuild-page-links.ts index 9d0c3404a..57d8cc2b0 100644 --- a/backend/tasks/workers/rebuild-page-links.ts +++ b/backend/tasks/workers/rebuild-page-links.ts @@ -8,13 +8,18 @@ import { settings } from '../../models/settings.ts' import { sites } from '../../models/sites.ts' /** - * Work out what every page on the wiki links to, from scratch. + * Work out what every page on the wiki links to, from scratch — and mark its red links while at it. * * The links of a page are derived from its stored render and rewritten whenever that render is, so * ordinary editing keeps them current on its own. This is for the cases where there was nothing to * derive them from at the time: pages that predate the table, and a wiki whose locale prefixes or * page extensions changed, which silently changes what a link that was already written ADDRESSES. * + * The marking is `refreshForPage`'s own (see `pageLinks.syncBrokenLinks`): each render has the + * broken-link class put on the links that point at nothing and taken off the rest. Saving keeps that + * current too, and so does creating or deleting a page that others link to — so this is what brings + * the renders of a wiki that predates the class into line, in one pass rather than a save per page. + * * Offered under Admin → Utilities and never run on its own. It is not a migration and not a boot step: * a wiki with no links recorded works, it simply has no backlinks to show yet, and a rebuild that ran * itself on every start would re-read every render in the wiki to produce what is usually the same @@ -64,8 +69,10 @@ export async function task(): Promise { for (;;) { /* Walked by id rather than by offset: this reads every page in the wiki one batch at a time, and a - paged read that re-counts its way to each batch gets slower as it goes. Nothing is being written - to `pages` here, so the set is stable underneath it. + paged read that re-counts its way to each batch gets slower as it goes. The only thing written + to `pages` here is a render's link classes, which moves no id, so the set is stable underneath it + -- and whether a link resolves is read off `pages` rather than off the rows this is rewriting, + so the order the pages are visited in does not change the answer. */ const rows = await WIKI.db .select({ diff --git a/frontend/public/_assets/icons/ultraviolet-broken-link.svg b/frontend/public/_assets/icons/ultraviolet-broken-link.svg new file mode 100644 index 000000000..eb5c73f84 --- /dev/null +++ b/frontend/public/_assets/icons/ultraviolet-broken-link.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/frontend/src/components/FileManager.vue b/frontend/src/components/FileManager.vue index fee7fd6de..8ba2106ed 100644 --- a/frontend/src/components/FileManager.vue +++ b/frontend/src/components/FileManager.vue @@ -822,6 +822,7 @@ import { apiErrorMessage } from '@/helpers/apiError' import { assetUrl } from '@/helpers/assets' import fileTypes from '@/helpers/fileTypes' import { folderIconStyle } from '@/helpers/folderColors' +import { notifyRelinked } from '@/helpers/pageRelink' import { saveVersionSource } from '@/helpers/pageVersions' import FolderCreateDialog from '@/components/FolderCreateDialog.vue' import FolderDeleteDialog from '@/components/FolderDeleteDialog.vue' @@ -1863,16 +1864,18 @@ function renameMovePage(item) { message: 'Page renamed successfully.' }) } else { - await pageStore.pageMove({ + const relinked = await pageStore.pageMove({ id: item.id, path: opts.path, title: opts.title, - locale: opts.locale + locale: opts.locale, + updateLinks: opts.updateLinks }) notify({ type: 'positive', message: 'Page moved successfully.' }) + notifyRelinked(relinked, t) } // -> Reload current view await loadTree({ parentId: state.currentFolderId }) diff --git a/frontend/src/components/PageActionsCol.vue b/frontend/src/components/PageActionsCol.vue index fdc43f858..e60a7a58a 100644 --- a/frontend/src/components/PageActionsCol.vue +++ b/frontend/src/components/PageActionsCol.vue @@ -253,6 +253,7 @@ import { useI18n } from 'vue-i18n' import { dialog } from '@/composables/dialog' import { notify } from '@/composables/notify' +import { notifyRelinked } from '@/helpers/pageRelink' import { useEditorStore } from '@/stores/editor' import { useFlagsStore } from '@/stores/flags' @@ -457,16 +458,18 @@ function renamePage() { message: 'Page renamed successfully.' }) } else { - await pageStore.pageMove({ + const relinked = await pageStore.pageMove({ id: pageStore.id, path: renamedPageOpts.path, title: renamedPageOpts.title, - locale: renamedPageOpts.locale + locale: renamedPageOpts.locale, + updateLinks: renamedPageOpts.updateLinks }) notify({ type: 'positive', message: 'Page moved successfully.' }) + notifyRelinked(relinked, t) } } catch (err) { notify({ diff --git a/frontend/src/components/TreeBrowserDialog.vue b/frontend/src/components/TreeBrowserDialog.vue index 806278b67..d9874cadb 100644 --- a/frontend/src/components/TreeBrowserDialog.vue +++ b/frontend/src/components/TreeBrowserDialog.vue @@ -132,6 +132,21 @@ @keyup:enter="save" /> + + + + + {{ t(`pageRenameDialog.updateLinks`) }} + {{ t(`pageRenameDialog.updateLinksHint`) }} + + + + + @@ -276,7 +291,8 @@ const state = reactive({ title: '', path: '', typesToFetch: [], - pathDirty: false + pathDirty: false, + updateLinks: true }) // REFS @@ -477,7 +493,9 @@ async function save() { path: currentFolderPath.value.length > 1 ? `${currentFolderPath.value.substring(1)}${state.path}` - : state.path + : state.path, + // -> Only meaningful to a move, and only offered by the rename mode + ...(props.mode === 'renamePage' ? { updateLinks: state.updateLinks } : {}) }) } diff --git a/frontend/src/css/_page-contents.scss b/frontend/src/css/_page-contents.scss index a1fe01ec9..d19995223 100644 --- a/frontend/src/css/_page-contents.scss +++ b/frontend/src/css/_page-contents.scss @@ -51,6 +51,8 @@ --content-ink-faint: #8a9099; --content-link: var(--color-primary); + /* A link to a page that does not exist yet -- see `.is-broken-link` under LINKS */ + --content-link-broken: var(--color-red-8); /* The page's own title, in the site's colour -- see the gradient rule under `h1` */ --content-h1: var(--color-primary); /* @@ -295,6 +297,7 @@ /* -> The mid-tone brand blue is too dim on a dark surface; see `--color-primary-light` */ --content-link: var(--color-primary-light); + --content-link-broken: var(--color-red-3); --content-h1: var(--color-primary-light); --content-rule: rgba(255, 255, 255, 0.16); @@ -763,6 +766,19 @@ vertical-align: baseline; } + /* + A link to a page that does not exist -- a red link, which is how a wiki shows what still needs + writing. + + The class is on the anchor in every stored render, put there by the server whenever the page is + saved or the page it points at comes or goes (`pageLinks.syncBrokenLinks`). Whether it is drawn + is the site's Colorize Broken Links setting, which is `has-red-links` on the container: kept + apart so the setting can change without a single render being rewritten. + */ + @at-root .page-contents.has-red-links a.is-broken-link { + color: var(--content-link-broken); + } + /* -> A link wrapping code keeps the link's colour, not the code span's */ code { color: inherit; diff --git a/frontend/src/helpers/pageRelink.js b/frontend/src/helpers/pageRelink.js new file mode 100644 index 000000000..4770497c3 --- /dev/null +++ b/frontend/src/helpers/pageRelink.js @@ -0,0 +1,37 @@ +import { notify } from '@/composables/notify' + +/** + * Tell the mover what became of the pages linking to a page they just moved. + * + * Shared by the two places a page is moved from (the page view's action rail and the file manager), + * which open the same dialog and get the same `relinked` back from `pageStore.pageMove`. + * + * A move that fixed everything says so in one line; one that could not fix every page says which, + * and stays on screen until dismissed, because those pages are now links to nowhere that somebody + * has to go and repair by hand. + * + * @param {{ updated: number, skippedCount: number, skipped: { title: string }[] } | null} relinked + * @param {Function} t The caller's `t` from `useI18n` + */ +export function notifyRelinked(relinked, t) { + if (!relinked) { + return + } + if (relinked.updated > 0) { + notify({ + type: 'positive', + message: t('pageRenameDialog.updateLinksDone', { count: relinked.updated }) + }) + } + if (relinked.skippedCount > 0) { + notify({ + type: 'warning', + message: t('pageRenameDialog.updateLinksSkipped', { + count: relinked.skippedCount, + // -> Only the pages the server would name to this reader; the count covers the rest + pages: relinked.skipped.map((page) => page.title).join(', ') || '—' + }), + timeout: 0 + }) + } +} diff --git a/frontend/src/pages/AdminGeneral.vue b/frontend/src/pages/AdminGeneral.vue index 39102f7a1..a6d6c77e2 100644 --- a/frontend/src/pages/AdminGeneral.vue +++ b/frontend/src/pages/AdminGeneral.vue @@ -568,6 +568,19 @@ :aria-label="t(`admin.general.pageExtensions`)" /> + + + + + {{ t(`admin.general.colorizeBrokenLinks`) }} + {{ t(`admin.general.colorizeBrokenLinksHint`) }} + + + + + @@ -673,6 +686,7 @@ function defaultConfig() { content: '' }, pageExtensions: '', + colorizeBrokenLinks: true, logoText: false, features: { backlinks: true, @@ -808,6 +822,7 @@ async function save() { content: state.config.banner?.content ?? '' }, pageExtensions: parsePageExtensions(state.config.pageExtensions), + colorizeBrokenLinks: state.config.colorizeBrokenLinks ?? true, logoText: state.config.logoText ?? false, sitemap: state.config.sitemap ?? false, uploads: { diff --git a/frontend/src/pages/Index.vue b/frontend/src/pages/Index.vue index 701d0a87e..4ae8bdc8d 100644 --- a/frontend/src/pages/Index.vue +++ b/frontend/src/pages/Index.vue @@ -216,7 +216,10 @@
A move may cross locales, which is the same page translated rather than a new one - ...(locale ? { locale } : {}) + ...(locale ? { locale } : {}), + updateLinks } }).json() ) @@ -930,6 +935,7 @@ export const usePageStore = defineStore('page', { this.$patch({ path, ...(locale ? { locale } : {}) }) this.router.replace(this.editorExitPath) } + return resp?.relinked ?? null }, /** * PAGE - Rename diff --git a/frontend/src/stores/site.js b/frontend/src/stores/site.js index 48ac82a2e..293209507 100644 --- a/frontend/src/stores/site.js +++ b/frontend/src/stores/site.js @@ -94,6 +94,12 @@ export const useSiteStore = defineStore('site', { * router acts on for links inside pages and the server acts on for requests that reach it. */ pageExtensions: [], + /** + * Whether a link to a page that does not exist is drawn red. The render marks those links whatever + * this says (`is-broken-link`, set on the server); this only decides whether the page view adds + * the class to its contents that colours them. + */ + colorizeBrokenLinks: true, search: '', searchLastQuery: '', searchIsLoading: false, @@ -431,6 +437,7 @@ export const useSiteStore = defineStore('site', { description: siteInfo.description, logoText: siteInfo.logoText, pageExtensions: siteInfo.pageExtensions ?? [], + colorizeBrokenLinks: siteInfo.colorizeBrokenLinks ?? true, company: siteInfo.company, contentLicense: siteInfo.contentLicense, footerExtra: siteInfo.footerExtra,