diff --git a/.github/workflows/crowdin-translations.yml b/.github/workflows/crowdin-translations.yml index c69eab09f..92851f154 100644 --- a/.github/workflows/crowdin-translations.yml +++ b/.github/workflows/crowdin-translations.yml @@ -59,6 +59,7 @@ jobs: env: CROWDIN_ALLOWED_TRANSLATION_LOSSES: ${{ inputs.allowed_translation_losses }} CROWDIN_PROJECT_ID: ${{ vars.CROWDIN_PROJECT_ID }} + CROWDIN_REJECTION_REPORT: ${{ runner.temp }}/crowdin-rejections.md CROWDIN_TOKEN: ${{ secrets.CROWDIN_TOKEN }} run: pnpm --filter client sync:crowdin @@ -72,6 +73,23 @@ jobs: scripts/sync-crowdin-translations.spec.mjs src/i18n/icu.spec.ts + - name: Compose pull request body + env: + PR_BODY: ${{ runner.temp }}/crowdin-pr-body.md + REJECTION_REPORT: ${{ runner.temp }}/crowdin-rejections.md + run: | + { + echo "Automated sparse translation export from Crowdin." + echo + echo "Untranslated entries are omitted so Vue I18n falls back to the English catalog." + } > "$PR_BODY" + if [ -s "$REJECTION_REPORT" ]; then + echo >> "$PR_BODY" + cat "$REJECTION_REPORT" >> "$PR_BODY" + cat "$REJECTION_REPORT" >> "$GITHUB_STEP_SUMMARY" + echo "::warning::Crowdin returned translations that failed catalog validation; they were omitted and are listed in the pull request body." + fi + - name: Create translation pull request uses: peter-evans/create-pull-request@5f6978faf089d4d20b00c7766989d076bb2fc7f1 # v8.1.1 with: @@ -111,7 +129,4 @@ jobs: delete-branch: true base: main title: "i18n(client): sync translations from Crowdin" - body: |- - Automated sparse translation export from Crowdin. - - Untranslated entries are omitted so Vue I18n falls back to the English catalog. + body-path: ${{ runner.temp }}/crowdin-pr-body.md diff --git a/client/scripts/locale-catalog-validation.mjs b/client/scripts/locale-catalog-validation.mjs index dbb841249..d36bfb88a 100644 --- a/client/scripts/locale-catalog-validation.mjs +++ b/client/scripts/locale-catalog-validation.mjs @@ -15,6 +15,18 @@ export function flattenCatalog(value, prefix = '', output = new Map()) { return output } +function messageErrors({ key, locale, message, referenceMessage, slotCountKeys }) { + if (referenceMessage === undefined) return [`${locale}: unexpected key ${key}`] + + const errors = [] + if (message.length === 0) errors.push(`${locale}: empty message ${key}`) + if (message.includes('\u2014')) errors.push(`${locale}: Unicode em dash is not allowed in ${key}`) + if (/<[^>]+>/.test(message)) errors.push(`${locale}: HTML is not allowed in ${key}`) + errors.push(...validateLocaleMessage({ key, locale, message, referenceMessage })) + if (slotCountKeys.has(key)) errors.push(...validateSlotCountMessage({ key, locale, message })) + return errors +} + export function validateCatalogs({ catalogs, referencedKeys = new Set(), slotCountKeys = new Set() }) { const reference = catalogs.get('en') if (!reference) throw new Error('English reference catalog is required') @@ -26,18 +38,25 @@ export function validateCatalogs({ catalogs, referencedKeys = new Set(), slotCou for (const [locale, catalog] of catalogs) { for (const [key, message] of catalog) { - const referenceMessage = reference.get(key) - if (referenceMessage === undefined) { - errors.push(`${locale}: unexpected key ${key}`) - continue - } - if (message.length === 0) errors.push(`${locale}: empty message ${key}`) - if (message.includes('\u2014')) errors.push(`${locale}: Unicode em dash is not allowed in ${key}`) - if (/<[^>]+>/.test(message)) errors.push(`${locale}: HTML is not allowed in ${key}`) - errors.push(...validateLocaleMessage({ key, locale, message, referenceMessage })) - if (slotCountKeys.has(key)) errors.push(...validateSlotCountMessage({ key, locale, message })) + errors.push(...messageErrors({ key, locale, message, referenceMessage: reference.get(key), slotCountKeys })) } } return errors } + +export function findInvalidTargetMessages({ catalogs, slotCountKeys = new Set() }) { + const reference = catalogs.get('en') + if (!reference) throw new Error('English reference catalog is required') + + const invalid = [] + for (const [locale, catalog] of catalogs) { + if (locale === 'en') continue + for (const [key, message] of catalog) { + const errors = messageErrors({ key, locale, message, referenceMessage: reference.get(key), slotCountKeys }) + if (errors.length > 0) invalid.push({ locale, key, errors }) + } + } + + return invalid +} diff --git a/client/scripts/locale-catalog-validation.spec.mjs b/client/scripts/locale-catalog-validation.spec.mjs index 25f34938a..ccc6b2d71 100644 --- a/client/scripts/locale-catalog-validation.spec.mjs +++ b/client/scripts/locale-catalog-validation.spec.mjs @@ -36,9 +36,6 @@ describe('locale catalog validation', () => { books: { count: '{count, plural, one {# kniha} other {# knih}}' }, } - expect(validateCatalogs({ catalogs: catalogs(target) })).toEqual([ - 'cs: ICU plural category few missing for books.count', - 'cs: ICU plural category many missing for books.count', - ]) + expect(validateCatalogs({ catalogs: catalogs(target) })).toEqual(['cs: ICU plural category few missing for books.count']) }) }) diff --git a/client/scripts/locale-message-validation.mjs b/client/scripts/locale-message-validation.mjs index 41627b768..f7c7bb6d5 100644 --- a/client/scripts/locale-message-validation.mjs +++ b/client/scripts/locale-message-validation.mjs @@ -101,8 +101,17 @@ export function analyzeIcuMessage(message) { } } +// Every plural in the catalogs counts whole items, and a library tops out well below a million. +// CLDR still lists categories that only fractional counts (Czech and Slovak `many`) or counts of a +// million and over (Spanish, French, Italian and Portuguese `many`) can ever select, so demanding a +// translation for them rejects otherwise correct Crowdin exports for a branch that cannot render. +// Sampling 0..1000 selects exactly the categories every integer below a million selects. +const REACHABLE_COUNT_SAMPLE = Array.from({ length: 1001 }, (_, index) => index) + function pluralCategories(locale, type) { - return new Intl.PluralRules(locale, { type }).resolvedOptions().pluralCategories + const rules = new Intl.PluralRules(locale, { type }) + const reachable = new Set(REACHABLE_COUNT_SAMPLE.map((count) => rules.select(count))) + return rules.resolvedOptions().pluralCategories.filter((category) => reachable.has(category)) } function exactSelectors(selectors) { diff --git a/client/scripts/locale-message-validation.spec.mjs b/client/scripts/locale-message-validation.spec.mjs index 334cf7d41..f4fee6f2d 100644 --- a/client/scripts/locale-message-validation.spec.mjs +++ b/client/scripts/locale-message-validation.spec.mjs @@ -86,6 +86,27 @@ describe('locale message validation', () => { ]) }) + it('accepts cardinal categories that only fractional or million-scale counts select', () => { + const reference = '{count, plural, one {# book} other {# books}}' + + expect(validate('es', reference, '{count, plural, one {# libro} other {# libros}}')).toEqual([]) + expect(validate('fr', reference, '{count, plural, one {# livre} other {# livres}}')).toEqual([]) + expect(validate('cs', reference, '{count, plural, one {# kniha} few {# knihy} other {# knih}}')).toEqual([]) + expect(validate('es', reference, '{count, plural, one {# libro} many {# de libros} other {# libros}}')).toEqual([]) + }) + + it('still requires categories that everyday counts select', () => { + const reference = '{count, plural, one {# book} other {# books}}' + + expect(validate('ru', reference, '{count, plural, one {# книга} other {# книги}}')).toEqual([ + 'ru: ICU plural category few missing for books.count', + 'ru: ICU plural category many missing for books.count', + ]) + expect(validate('cs', reference, '{count, plural, one {# kniha} other {# knih}}')).toEqual([ + 'cs: ICU plural category few missing for books.count', + ]) + }) + it('reports invalid ICU and rejects legacy branches for migrated keys', () => { const reference = '{count, plural, one {One book} other {# books}}' diff --git a/client/scripts/sync-crowdin-translations.mjs b/client/scripts/sync-crowdin-translations.mjs index 2e7e4b65e..fa7119f77 100644 --- a/client/scripts/sync-crowdin-translations.mjs +++ b/client/scripts/sync-crowdin-translations.mjs @@ -4,7 +4,7 @@ import { isIP } from 'node:net' import { pathToFileURL } from 'node:url' import path from 'node:path' import { TARGET_CATALOGS, assertCrowdinTargetConfiguration } from './locale-configuration.mjs' -import { flattenCatalog, validateCatalogs } from './locale-catalog-validation.mjs' +import { findInvalidTargetMessages, flattenCatalog, validateCatalogs } from './locale-catalog-validation.mjs' const API = 'https://api.crowdin.com/api/v2' const SOURCE_PATH_SUFFIX = '/client/src/locales/en.json' @@ -260,7 +260,7 @@ export function parseAllowedTranslationLosses(value = '') { return allowed } -export function findTranslationLosses({ locale, reference, current, exported }) { +export function findTranslationLosses({ locale, reference, current, exported, rejected = new Map() }) { const legacyComplete = current.size === reference.size && [...reference.keys()].every((key) => current.has(key)) const losses = [] @@ -270,7 +270,8 @@ export function findTranslationLosses({ locale, reference, current, exported }) const exportedMessage = exported.get(key) if (exportedMessage === undefined) { - losses.push({ locale, key, reason: 'missing from Crowdin export' }) + const rejection = rejected.get(key) + losses.push({ locale, key, reason: rejection ? `rejected by catalog validation - ${rejection[0]}` : 'missing from Crowdin export' }) } else if (currentMessage !== referenceMessage && exportedMessage === referenceMessage) { losses.push({ locale, key, reason: 'replaced by English source text' }) } @@ -285,13 +286,20 @@ export function assertTranslationRetention({ exportedCatalogs, allowedLosses = new Set(), targetCatalogs = TARGET_CATALOGS, + rejections = [], }) { + const rejectedByLocale = new Map() + for (const { locale, key, errors } of rejections) { + if (!rejectedByLocale.has(locale)) rejectedByLocale.set(locale, new Map()) + rejectedByLocale.get(locale).set(key, errors) + } + const losses = [] for (const { locale } of targetCatalogs) { const current = currentCatalogs.get(locale) const exported = exportedCatalogs.get(locale) if (!current || !exported) throw new Error(`Translation retention comparison is missing the ${locale} catalog`) - losses.push(...findTranslationLosses({ locale, reference, current, exported })) + losses.push(...findTranslationLosses({ locale, reference, current, exported, rejected: rejectedByLocale.get(locale) })) } const detected = new Set(losses.map(({ locale, key }) => `${locale}:${key}`)) @@ -308,6 +316,35 @@ export function assertTranslationRetention({ throw new Error(`Crowdin export would lose existing translations:\n${details.join('\n')}`) } +const MAX_REPORTED_REJECTIONS = 50 + +export function formatRejectionReport(rejections) { + if (rejections.length === 0) return '' + + const listed = rejections.slice(0, MAX_REPORTED_REJECTIONS) + const lines = [ + `### Rejected Crowdin messages (${rejections.length})`, + '', + 'These translations did not pass catalog validation and were omitted, so the English source renders instead. Fix them in Crowdin.', + '', + ...listed.map(({ errors }) => `- ${errors[0]}`), + ] + if (rejections.length > listed.length) lines.push(`- ...and ${rejections.length - listed.length} more`) + return `${lines.join('\n')}\n` +} + +async function reportRejections(rejections, reportPath) { + if (rejections.length === 0) { + if (reportPath) await writeFile(reportPath, '') + return + } + + console.log(`Rejected ${rejections.length} invalid Crowdin messages; the English source renders instead:`) + for (const { errors } of rejections.slice(0, MAX_REPORTED_REJECTIONS)) console.log(` ${errors[0]}`) + if (rejections.length > MAX_REPORTED_REJECTIONS) console.log(` ...and ${rejections.length - MAX_REPORTED_REJECTIONS} more`) + if (reportPath) await writeFile(reportPath, formatRejectionReport(rejections)) +} + export async function syncCrowdinTranslations({ token, projectId = '912891', @@ -317,6 +354,7 @@ export async function syncCrowdinTranslations({ allowedLosses = new Set(), targetCatalogs = TARGET_CATALOGS, assertTargetConfiguration = assertCrowdinTargetConfiguration, + reportPath = process.env.CROWDIN_REJECTION_REPORT || '', }) { if (!token) throw new Error('CROWDIN_TOKEN is required') await assertTargetConfiguration() @@ -350,6 +388,13 @@ export async function syncCrowdinTranslations({ const catalogs = new Map([['en', referenceMessages]]) for (const { locale, catalog } of downloaded) catalogs.set(locale, flattenCatalog(catalog)) + const rejections = findInvalidTargetMessages({ catalogs }) + const rejectedLocales = new Set(rejections.map(({ locale }) => locale)) + for (const { locale, key } of rejections) catalogs.get(locale).delete(key) + for (const entry of downloaded) { + if (rejectedLocales.has(entry.locale)) entry.catalog = orderedSparseCatalog(reference, catalogs.get(entry.locale)) + } + const errors = validateCatalogs({ catalogs }) if (errors.length > 0) throw new Error(`Crowdin export validation failed:\n${errors.join('\n')}`) assertTranslationRetention({ @@ -358,13 +403,16 @@ export async function syncCrowdinTranslations({ exportedCatalogs: catalogs, allowedLosses, targetCatalogs, + rejections, }) await mkdir(outputDirectory, { recursive: true }) await Promise.all( downloaded.map(({ locale, catalog }) => writeFile(path.join(outputDirectory, `${locale}.json`), `${JSON.stringify(catalog, null, 2)}\n`)), ) + await reportRejections(rejections, reportPath) console.log(`Synchronized ${downloaded.length} sparse translation catalogs from Crowdin`) + return { rejections } } if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { diff --git a/client/scripts/sync-crowdin-translations.spec.mjs b/client/scripts/sync-crowdin-translations.spec.mjs index 0cfb367a2..27de91d60 100644 --- a/client/scripts/sync-crowdin-translations.spec.mjs +++ b/client/scripts/sync-crowdin-translations.spec.mjs @@ -284,10 +284,42 @@ describe('Crowdin translation synchronization', () => { await expect(access(outputDirectory)).rejects.toThrow() }) - it('validates every downloaded catalog before writing any files', async () => { + it('omits an invalid downloaded message instead of failing the whole export', async () => { const targetCatalogs = TARGET_CATALOGS.slice(0, 2) const catalogDirectory = await createCatalogFixture(targetCatalogs) const outputDirectory = path.join(catalogDirectory, 'output') + const reportPath = path.join(catalogDirectory, 'rejections.md') + const catalogs = new Map([[targetCatalogs[0].languageId, { common: { save: 'Bad \u2014 value' } }]]) + + const { rejections } = await syncCrowdinTranslations({ + token: 'secret', + fetchImpl: createSynchronizationFetch({ catalogs }), + catalogDirectory, + outputDirectory, + targetCatalogs, + reportPath, + assertTargetConfiguration: async () => {}, + }) + + expect(rejections).toEqual([ + { + locale: targetCatalogs[0].locale, + key: 'common.save', + errors: [`${targetCatalogs[0].locale}: Unicode em dash is not allowed in common.save`], + }, + ]) + expect(JSON.parse(await readFile(path.join(outputDirectory, `${targetCatalogs[0].locale}.json`), 'utf8'))).toEqual({}) + expect(JSON.parse(await readFile(path.join(outputDirectory, `${targetCatalogs[1].locale}.json`), 'utf8'))).toEqual({ + common: { save: `Translated ${targetCatalogs[1].languageId}` }, + }) + expect(await readFile(reportPath, 'utf8')).toContain('Unicode em dash is not allowed in common.save') + }) + + it('fails when a rejected message would drop an existing translation', async () => { + const targetCatalogs = TARGET_CATALOGS.slice(0, 2) + const currentCatalogs = new Map([[targetCatalogs[0].locale, { common: { save: 'Ulozit' } }]]) + const catalogDirectory = await createCatalogFixture(targetCatalogs, currentCatalogs) + const outputDirectory = path.join(catalogDirectory, 'output') const catalogs = new Map([[targetCatalogs[0].languageId, { common: { save: 'Bad \u2014 value' } }]]) await expect( @@ -299,7 +331,7 @@ describe('Crowdin translation synchronization', () => { targetCatalogs, assertTargetConfiguration: async () => {}, }), - ).rejects.toThrow('Unicode em dash is not allowed') + ).rejects.toThrow('rejected by catalog validation') await expect(access(outputDirectory)).rejects.toThrow() })