Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(flaws): don't report link to missing translation as broken if en-US fallback exists #9408

Merged
merged 11 commits into from
Mar 15, 2024
40 changes: 26 additions & 14 deletions build/flaws/broken-links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,16 @@ function mutateLink(
) {
if (isSelfLink) {
$element.attr("aria-current", "page");
} else if (suggestion) {
$element.attr("href", suggestion);
} else if (enUSFallback) {
// If we have an English (US) fallback, then we use this first.
// As we still suggest the translated version even if we only
// have an English (US) version.
$element.attr("href", enUSFallback);
$element.append(` <small>(${DEFAULT_LOCALE})<small>`);
$element.addClass("only-in-en-us");
$element.attr("title", "Currently only available in English (US)");
} else if (suggestion) {
$element.attr("href", suggestion);
} else {
$element.addClass("page-not-created");
$element.attr("title", "This is a link to an unwritten page");
Expand Down Expand Up @@ -299,7 +302,6 @@ export function getBrokenLinksFlaws(
resolved + absoluteURL.search + absoluteURL.hash.toLowerCase()
);
} else {
let enUSFallbackURL = null;
// Test if the document is a translated document and the link isn't
// to an en-US URL. We know the link is broken (in this locale!)
// but it might be "salvageable" if we link the en-US equivalent.
Expand All @@ -316,28 +318,38 @@ export function getBrokenLinksFlaws(
`/${DEFAULT_LOCALE}/`
);
const enUSFound = Document.findByURL(enUSHrefNormalized);
// Note, we still recommend that contributors use localized links,
// even if the target document is still not localized.
if (enUSFound) {
enUSFallbackURL = enUSFound.url;
// Found the en-US version of the document. Just link to that.
mutateLink(a, null, enUSFound.url);
} else {
const enUSResolved = Redirect.resolve(enUSHrefNormalized);
let suggestion = null;
let enUSFallbackURL = null;
if (enUSResolved !== enUSHrefNormalized) {
enUSFallbackURL =
enUSResolved +
absoluteURL.search +
absoluteURL.hash.toLowerCase();
suggestion = enUSFallbackURL.replace(
`/${DEFAULT_LOCALE}/`,
`/${doc.locale}/`
);
}
addBrokenLink(
a,
checked.get(href),
href,
suggestion,
null,
enUSFallbackURL
);
}
} else {
// The link is broken and we don't have a suggestion.
addBrokenLink(a, checked.get(href), href);
}
addBrokenLink(
a,
checked.get(href),
href,
null,
enUSFallbackURL
? "Can use the English (en-US) link as a fallback"
: null,
enUSFallbackURL
);
}
}
// But does it have the correct case?!
Expand Down
16 changes: 10 additions & 6 deletions testing/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1613,19 +1613,23 @@ test("translated content broken links can fall back to en-us", () => {
const { doc } = JSON.parse(fs.readFileSync(jsonFile, "utf-8")) as {
doc: Doc;
};

const map = new Map(doc.flaws.broken_links.map((x) => [x.href, x]));
expect(map.get("/fr/docs/Web/CSS/dumber").explanation).toBe(
"Can use the English (en-US) link as a fallback"
);
expect(map.get("/fr/docs/Web/CSS/number").explanation).toBe(
"Can use the English (en-US) link as a fallback"
);
expect(map.get("/fr/docs/Web/CSS/dumber")).toMatchObject({
explanation: "Can't resolve /fr/docs/Web/CSS/dumber",
suggestion: "/fr/docs/Web/CSS/number",
fixable: true,
line: 19,
column: 16,
});
expect(map.get("/fr/docs/Web/CSS/number")).toBeUndefined();

const htmlFile = path.join(builtFolder, "index.html");
const html = fs.readFileSync(htmlFile, "utf-8");
const $ = cheerio.load(html);
expect($('article a[href="/fr/docs/Web/CSS/dumber"]')).toHaveLength(0);
expect($('article a[href="/fr/docs/Web/CSS/number"]')).toHaveLength(0);
// Localized docs don't exist, but fallback to en-US is possible
expect($('article a[href="/en-US/docs/Web/CSS/number"]')).toHaveLength(2);
expect($("article a.only-in-en-us")).toHaveLength(2);
expect($("article a.only-in-en-us").attr("title")).toBe(
Expand Down
Loading