From 7930478f16e9c6b16ea092658ec9764448381db4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Smyrek?= Date: Wed, 16 Oct 2024 11:21:49 +0200 Subject: [PATCH 1/5] Introduced script for moving translations between packages. --- .../ckeditor5-dev-translations/lib/index.js | 1 + .../lib/movetranslations.js | 101 ++++++ .../lib/synchronizetranslations.js | 23 +- .../lib/utils/getpackagecontext.js | 32 ++ .../lib/utils/getpackagecontexts.js | 15 +- .../lib/utils/getsourcemessages.js | 11 + .../utils/movetranslationsbetweenpackages.js | 74 ++++ ... synchronizetranslationsbasedoncontext.js} | 2 +- .../tests/movetranslations.js | 209 +++++++++++ .../tests/synchronizetranslations.js | 14 +- .../tests/utils/getpackagecontext.js | 65 ++++ .../tests/utils/getpackagecontexts.js | 60 ++-- .../utils/movetranslationsbetweenpackages.js | 332 ++++++++++++++++++ ... synchronizetranslationsbasedoncontext.js} | 28 +- 14 files changed, 875 insertions(+), 92 deletions(-) create mode 100644 packages/ckeditor5-dev-translations/lib/movetranslations.js create mode 100644 packages/ckeditor5-dev-translations/lib/utils/getpackagecontext.js create mode 100644 packages/ckeditor5-dev-translations/lib/utils/movetranslationsbetweenpackages.js rename packages/ckeditor5-dev-translations/lib/utils/{updatepackagetranslations.js => synchronizetranslationsbasedoncontext.js} (96%) create mode 100644 packages/ckeditor5-dev-translations/tests/movetranslations.js create mode 100644 packages/ckeditor5-dev-translations/tests/utils/getpackagecontext.js create mode 100644 packages/ckeditor5-dev-translations/tests/utils/movetranslationsbetweenpackages.js rename packages/ckeditor5-dev-translations/tests/utils/{updatepackagetranslations.js => synchronizetranslationsbasedoncontext.js} (85%) diff --git a/packages/ckeditor5-dev-translations/lib/index.js b/packages/ckeditor5-dev-translations/lib/index.js index 3e24e5ea2..d43793755 100644 --- a/packages/ckeditor5-dev-translations/lib/index.js +++ b/packages/ckeditor5-dev-translations/lib/index.js @@ -7,3 +7,4 @@ export { default as findMessages } from './findmessages.js'; export { default as MultipleLanguageTranslationService } from './multiplelanguagetranslationservice.js'; export { default as CKEditorTranslationsPlugin } from './ckeditortranslationsplugin.js'; export { default as synchronizeTranslations } from './synchronizetranslations.js'; +export { default as moveTranslations } from './movetranslations.js'; diff --git a/packages/ckeditor5-dev-translations/lib/movetranslations.js b/packages/ckeditor5-dev-translations/lib/movetranslations.js new file mode 100644 index 000000000..956a0d825 --- /dev/null +++ b/packages/ckeditor5-dev-translations/lib/movetranslations.js @@ -0,0 +1,101 @@ +/** + * @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import fs from 'fs-extra'; +import { logger } from '@ckeditor/ckeditor5-dev-utils'; +import getPackageContext from './utils/getpackagecontext.js'; +import moveTranslationsBetweenPackages from './utils/movetranslationsbetweenpackages.js'; + +/** + * Moves the requested translations (context and messages) between packages by performing the following steps: + * * Detect if both source and destination packages exist. + * * Detect if translation context to move exists in the source package. Message may not exist in the target package, + * but if it does, it will be overwritten. + * * If there are no validation errors, move the requested translations between packages: the context and the translation + * messages for each language found in the source package. + * + * @param {object} options + * @param {Array.} options.config Configuration that defines the messages to move. + */ +export default function moveTranslations( options ) { + const { config } = options; + const log = logger(); + + log.info( '📍 Loading translations contexts...' ); + const packageContexts = config.flatMap( entry => [ + getPackageContext( { packagePath: entry.source } ), + getPackageContext( { packagePath: entry.destination } ) + ] ); + + const errors = []; + + log.info( '📍 Checking provided configuration...' ); + errors.push( + ...assertPackagesExist( { config } ), + ...assertContextsExist( { packageContexts, config } ) + ); + + if ( errors.length ) { + log.error( '🔥 The following errors have been found:' ); + + for ( const error of errors ) { + log.error( ` - ${ error }` ); + } + + process.exit( 1 ); + } + + log.info( '📍 Moving translations between packages...' ); + moveTranslationsBetweenPackages( { packageContexts, config } ); + + log.info( '✨ Done.' ); +} + +/** + * @param {object} options + * @param {Array.} options.config Configuration that defines the messages to move. + * @returns {Array.} + */ +function assertPackagesExist( { config } ) { + return config + .flatMap( entry => { + const missingPackages = []; + + if ( !fs.existsSync( entry.source ) ) { + missingPackages.push( entry.source ); + } + + if ( !fs.existsSync( entry.destination ) ) { + missingPackages.push( entry.destination ); + } + + return missingPackages; + } ) + .map( packagePath => `Missing package: the "${ packagePath }" package does not exist.` ); +} + +/** + * @param {object} options + * @param {Array.} options.packageContexts An array of language contexts. + * @param {Array.} options.config Configuration that defines the messages to move. + * @returns {Array.} + */ +function assertContextsExist( { packageContexts, config } ) { + return config + .filter( entry => { + const packageContext = packageContexts.find( context => context.packagePath === entry.source ); + + return !packageContext.contextContent[ entry.messageId ]; + } ) + .map( entry => `Missing context: the "${ entry.messageId }" message does not exist in "${ entry.source }" package.` ); +} + +/** + * @typedef {object} TranslationMoveEntry + * + * @property {string} source Relative path to the source package from which the `messageId` should be moved. + * @property {string} destination Relative path to the destination package to which the `messageId` should be moved. + * @property {string} messageId The message identifier to move. + */ diff --git a/packages/ckeditor5-dev-translations/lib/synchronizetranslations.js b/packages/ckeditor5-dev-translations/lib/synchronizetranslations.js index f0283148c..0b75f04d9 100644 --- a/packages/ckeditor5-dev-translations/lib/synchronizetranslations.js +++ b/packages/ckeditor5-dev-translations/lib/synchronizetranslations.js @@ -8,7 +8,7 @@ import { logger } from '@ckeditor/ckeditor5-dev-utils'; import getPackageContexts from './utils/getpackagecontexts.js'; import { CONTEXT_FILE_PATH } from './utils/constants.js'; import getSourceMessages from './utils/getsourcemessages.js'; -import updatePackageTranslations from './utils/updatepackagetranslations.js'; +import synchronizeTranslationsBasedOnContext from './utils/synchronizetranslationsbasedoncontext.js'; /** * Synchronizes translations in provided packages by performing the following steps: @@ -72,7 +72,7 @@ export default function synchronizeTranslations( options ) { } log.info( '📍 Synchronizing translations files...' ); - updatePackageTranslations( { packageContexts, sourceMessages, skipLicenseHeader } ); + synchronizeTranslationsBasedOnContext( { packageContexts, sourceMessages, skipLicenseHeader } ); log.info( '✨ Done.' ); } @@ -176,22 +176,3 @@ function assertNoRepeatedContext( { packageContexts } ) { return `Duplicated context "${ messageId }" in "${ contextFilePaths.join( '", "' ) }".`; } ); } - -/** - * @typedef {object} Message - * - * @property {string} id - * @property {string} string - * @property {string} filePath - * @property {string} packagePath - * @property {string} context - * @property {string} [plural] - */ - -/** - * @typedef {object} Context - * - * @property {string} contextFilePath - * @property {object} contextContent - * @property {string} packagePath - */ diff --git a/packages/ckeditor5-dev-translations/lib/utils/getpackagecontext.js b/packages/ckeditor5-dev-translations/lib/utils/getpackagecontext.js new file mode 100644 index 000000000..57932cbdc --- /dev/null +++ b/packages/ckeditor5-dev-translations/lib/utils/getpackagecontext.js @@ -0,0 +1,32 @@ +/** + * @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import upath from 'upath'; +import fs from 'fs-extra'; +import { CONTEXT_FILE_PATH } from './constants.js'; + +/** + * @param {object} options + * @param {string} options.packagePath Path to the package containing the context. + * @returns {Context} + */ +export default function getPackageContext( { packagePath } ) { + const contextFilePath = upath.join( packagePath, CONTEXT_FILE_PATH ); + const contextContent = fs.readJsonSync( contextFilePath, { throws: false } ) || {}; + + return { + contextContent, + contextFilePath, + packagePath + }; +} + +/** + * @typedef {object} Context + * + * @property {string} contextFilePath + * @property {object} contextContent + * @property {string} packagePath + */ diff --git a/packages/ckeditor5-dev-translations/lib/utils/getpackagecontexts.js b/packages/ckeditor5-dev-translations/lib/utils/getpackagecontexts.js index fca663fc0..fa5c2277b 100644 --- a/packages/ckeditor5-dev-translations/lib/utils/getpackagecontexts.js +++ b/packages/ckeditor5-dev-translations/lib/utils/getpackagecontexts.js @@ -3,9 +3,7 @@ * For licensing, see LICENSE.md. */ -import upath from 'upath'; -import fs from 'fs-extra'; -import { CONTEXT_FILE_PATH } from './constants.js'; +import getPackageContext from './getpackagecontext.js'; /** * @param {object} options @@ -20,14 +18,5 @@ export default function getPackageContexts( { packagePaths, corePackagePath } ) packagePaths.push( corePackagePath ); } - return packagePaths.map( packagePath => { - const contextFilePath = upath.join( packagePath, CONTEXT_FILE_PATH ); - const contextContent = fs.existsSync( contextFilePath ) ? fs.readJsonSync( contextFilePath ) : {}; - - return { - contextContent, - contextFilePath, - packagePath - }; - } ); + return packagePaths.map( packagePath => getPackageContext( { packagePath } ) ); } diff --git a/packages/ckeditor5-dev-translations/lib/utils/getsourcemessages.js b/packages/ckeditor5-dev-translations/lib/utils/getsourcemessages.js index 1b92d9f2b..94ca1774b 100644 --- a/packages/ckeditor5-dev-translations/lib/utils/getsourcemessages.js +++ b/packages/ckeditor5-dev-translations/lib/utils/getsourcemessages.js @@ -30,3 +30,14 @@ export default function getSourceMessages( { packagePaths, sourceFiles, onErrorC return sourceMessages; } ); } + +/** + * @typedef {object} Message + * + * @property {string} id + * @property {string} string + * @property {string} filePath + * @property {string} packagePath + * @property {string} context + * @property {string} [plural] + */ diff --git a/packages/ckeditor5-dev-translations/lib/utils/movetranslationsbetweenpackages.js b/packages/ckeditor5-dev-translations/lib/utils/movetranslationsbetweenpackages.js new file mode 100644 index 000000000..1225a03f0 --- /dev/null +++ b/packages/ckeditor5-dev-translations/lib/utils/movetranslationsbetweenpackages.js @@ -0,0 +1,74 @@ +/** + * @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import upath from 'upath'; +import fs from 'fs-extra'; +import PO from 'pofile'; +import { glob } from 'glob'; +import { TRANSLATION_FILES_PATH } from './constants.js'; +import cleanTranslationFileContent from './cleantranslationfilecontent.js'; + +/** + * @param {object} options + * @param {Array.} options.packageContexts An array of language contexts. + * @param {Array.} options.config Configuration that defines the messages to move. + */ +export default function moveTranslationsBetweenPackages( { packageContexts, config } ) { + // For each message to move: + for ( const { source, destination, messageId } of config ) { + // (1) Skip the message if its source and destination package is the same. + if ( source === destination ) { + continue; + } + + // (2) Move translation context from source package to destination package. + const sourcePackageContext = packageContexts.find( context => context.packagePath === source ); + const destinationPackageContext = packageContexts.find( context => context.packagePath === destination ); + + destinationPackageContext.contextContent[ messageId ] = sourcePackageContext.contextContent[ messageId ]; + delete sourcePackageContext.contextContent[ messageId ]; + + // (3) Prepare the list of paths to translation files ("*.po" files) in source and destination packages. + // The source package defines the list of files for both packages. + const translationFilesPattern = upath.join( source, TRANSLATION_FILES_PATH, '*.po' ); + const translationFilePaths = glob.sync( translationFilesPattern ) + .map( filePath => upath.basename( filePath ) ) + .map( fileName => ( { + sourceTranslationFilePath: upath.join( source, TRANSLATION_FILES_PATH, fileName ), + destinationTranslationFilePath: upath.join( destination, TRANSLATION_FILES_PATH, fileName ) + } ) ); + + // Then, for each translation file: + for ( const { sourceTranslationFilePath, destinationTranslationFilePath } of translationFilePaths ) { + // (3.1) Read the source translation file. + const sourceTranslationFile = fs.readFileSync( sourceTranslationFilePath, 'utf-8' ); + const sourceTranslations = PO.parse( sourceTranslationFile ); + + // (3.2) Read the destination translation file. + // If the destination file does not exist, use the source file as a base and remove all translations. + const destinationTranslationFile = fs.existsSync( destinationTranslationFilePath ) ? + fs.readFileSync( destinationTranslationFilePath, 'utf-8' ) : + null; + const destinationTranslations = PO.parse( destinationTranslationFile || sourceTranslationFile ); + + if ( !destinationTranslationFile ) { + destinationTranslations.items = []; + } + + // (3.3) Move the translation from source file to destination file. + const sourceMessage = sourceTranslations.items.find( item => item.msgid === messageId ); + sourceTranslations.items = sourceTranslations.items.filter( item => item.msgid !== messageId ); + + destinationTranslations.items = destinationTranslations.items.filter( item => item.msgid !== messageId ); + destinationTranslations.items.push( sourceMessage ); + + fs.outputFileSync( sourceTranslationFilePath, cleanTranslationFileContent( sourceTranslations ).toString(), 'utf-8' ); + fs.outputFileSync( destinationTranslationFilePath, cleanTranslationFileContent( destinationTranslations ).toString(), 'utf-8' ); + } + + fs.outputJsonSync( sourcePackageContext.contextFilePath, sourcePackageContext.contextContent, { spaces: '\t' } ); + fs.outputJsonSync( destinationPackageContext.contextFilePath, destinationPackageContext.contextContent, { spaces: '\t' } ); + } +} diff --git a/packages/ckeditor5-dev-translations/lib/utils/updatepackagetranslations.js b/packages/ckeditor5-dev-translations/lib/utils/synchronizetranslationsbasedoncontext.js similarity index 96% rename from packages/ckeditor5-dev-translations/lib/utils/updatepackagetranslations.js rename to packages/ckeditor5-dev-translations/lib/utils/synchronizetranslationsbasedoncontext.js index 8e4293c57..486c488cb 100644 --- a/packages/ckeditor5-dev-translations/lib/utils/updatepackagetranslations.js +++ b/packages/ckeditor5-dev-translations/lib/utils/synchronizetranslationsbasedoncontext.js @@ -17,7 +17,7 @@ import cleanTranslationFileContent from './cleantranslationfilecontent.js'; * @param {Array.} options.sourceMessages An array of i18n source messages. * @param {boolean} options.skipLicenseHeader Whether to skip adding the license header to newly created translation files. */ -export default function updatePackageTranslations( { packageContexts, sourceMessages, skipLicenseHeader } ) { +export default function synchronizeTranslationsBasedOnContext( { packageContexts, sourceMessages, skipLicenseHeader } ) { // For each package: for ( const { packagePath, contextContent } of packageContexts ) { // (1) Skip packages that do not contain language context. diff --git a/packages/ckeditor5-dev-translations/tests/movetranslations.js b/packages/ckeditor5-dev-translations/tests/movetranslations.js new file mode 100644 index 000000000..0b5817351 --- /dev/null +++ b/packages/ckeditor5-dev-translations/tests/movetranslations.js @@ -0,0 +1,209 @@ +/** + * @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'fs-extra'; +import getPackageContext from '../lib/utils/getpackagecontext.js'; +import moveTranslationsBetweenPackages from '../lib/utils/movetranslationsbetweenpackages.js'; +import moveTranslations from '../lib/movetranslations.js'; + +const stubs = vi.hoisted( () => { + return { + logger: { + info: vi.fn(), + error: vi.fn() + } + }; +} ); + +vi.mock( 'fs-extra' ); +vi.mock( '@ckeditor/ckeditor5-dev-utils', () => ( { + logger: vi.fn( () => stubs.logger ) +} ) ); +vi.mock( '../lib/utils/getpackagecontext.js' ); +vi.mock( '../lib/utils/movetranslationsbetweenpackages.js' ); + +describe( 'moveTranslations()', () => { + let defaultOptions; + + beforeEach( () => { + defaultOptions = { + config: [ + { + source: 'packages/ckeditor5-foo', + destination: 'packages/ckeditor5-bar', + messageId: 'id1' + } + ] + }; + + vi.mocked( fs.existsSync ).mockReturnValue( true ); + + vi.mocked( getPackageContext ).mockImplementation( ( { packagePath } ) => { + const contextContent = {}; + + if ( packagePath === 'packages/ckeditor5-foo' ) { + contextContent.id1 = 'Context for message id1 from "ckeditor5-foo".'; + } + + if ( packagePath === 'packages/ckeditor5-bar' ) { + contextContent.id2 = 'Context for message id2 from "ckeditor5-bar".'; + } + + return { + contextContent, + contextFilePath: packagePath + '/lang/contexts.json', + packagePath + }; + } ); + + vi.spyOn( process, 'exit' ).mockImplementation( () => {} ); + } ); + + it( 'should be a function', () => { + expect( moveTranslations ).toBeInstanceOf( Function ); + } ); + + it( 'should load translations contexts', () => { + moveTranslations( defaultOptions ); + + expect( getPackageContext ).toHaveBeenCalledTimes( 2 ); + expect( getPackageContext ).toHaveBeenCalledWith( { packagePath: 'packages/ckeditor5-foo' } ); + expect( getPackageContext ).toHaveBeenCalledWith( { packagePath: 'packages/ckeditor5-bar' } ); + + expect( stubs.logger.info ).toHaveBeenCalledWith( '📍 Loading translations contexts...' ); + } ); + + it( 'should move translations between packages', () => { + moveTranslations( defaultOptions ); + + expect( moveTranslationsBetweenPackages ).toHaveBeenCalledTimes( 1 ); + expect( moveTranslationsBetweenPackages ).toHaveBeenCalledWith( { + packageContexts: [ + { + contextContent: { + id1: 'Context for message id1 from "ckeditor5-foo".' + }, + contextFilePath: 'packages/ckeditor5-foo/lang/contexts.json', + packagePath: 'packages/ckeditor5-foo' + }, + { + contextContent: { + id2: 'Context for message id2 from "ckeditor5-bar".' + }, + contextFilePath: 'packages/ckeditor5-bar/lang/contexts.json', + packagePath: 'packages/ckeditor5-bar' + } + ], + config: [ + { + source: 'packages/ckeditor5-foo', + destination: 'packages/ckeditor5-bar', + messageId: 'id1' + } + ] + } ); + + expect( stubs.logger.info ).toHaveBeenCalledWith( '📍 Moving translations between packages...' ); + } ); + + describe( 'validation', () => { + describe( 'packages exist', () => { + it( 'should return no error if there is no missing package', () => { + moveTranslations( defaultOptions ); + + expect( stubs.logger.error ).not.toHaveBeenCalledWith( expect.stringContaining( 'Missing package' ) ); + } ); + + it( 'should return error if there is missing package (missing source package)', () => { + vi.mocked( fs.existsSync ).mockImplementation( path => { + return path !== 'packages/ckeditor5-foo'; + } ); + + moveTranslations( defaultOptions ); + + expect( stubs.logger.error ).toHaveBeenCalledWith( + ' - Missing package: the "packages/ckeditor5-foo" package does not exist.' + ); + + expect( process.exit ).toHaveBeenCalledWith( 1 ); + } ); + + it( 'should return error if there is missing package (missing destination package)', () => { + vi.mocked( fs.existsSync ).mockImplementation( path => { + return path !== 'packages/ckeditor5-bar'; + } ); + + moveTranslations( defaultOptions ); + + expect( stubs.logger.error ).toHaveBeenCalledWith( + ' - Missing package: the "packages/ckeditor5-bar" package does not exist.' + ); + + expect( process.exit ).toHaveBeenCalledWith( 1 ); + } ); + + it( 'should return error if there is missing package (missing source and destination packages)', () => { + vi.mocked( fs.existsSync ).mockReturnValue( false ); + + moveTranslations( defaultOptions ); + + expect( stubs.logger.error ).toHaveBeenCalledWith( + ' - Missing package: the "packages/ckeditor5-foo" package does not exist.' + ); + + expect( stubs.logger.error ).toHaveBeenCalledWith( + ' - Missing package: the "packages/ckeditor5-bar" package does not exist.' + ); + + expect( process.exit ).toHaveBeenCalledWith( 1 ); + } ); + } ); + + describe( 'context exists', () => { + it( 'should return no error if there is no missing context', () => { + moveTranslations( defaultOptions ); + + expect( stubs.logger.error ).not.toHaveBeenCalledWith( expect.stringContaining( 'Missing context' ) ); + } ); + + it( 'should return error if there is missing context (message id does not exist)', () => { + defaultOptions.config = [ + { + source: 'packages/ckeditor5-foo', + destination: 'packages/ckeditor5-bar', + messageId: 'id100' + } + ]; + + moveTranslations( defaultOptions ); + + expect( stubs.logger.error ).toHaveBeenCalledWith( + ' - Missing context: the "id100" message does not exist in "packages/ckeditor5-foo" package.' + ); + + expect( process.exit ).toHaveBeenCalledWith( 1 ); + } ); + + it( 'should return error if there is missing context (message id exists only in destination package)', () => { + defaultOptions.config = [ + { + source: 'packages/ckeditor5-foo', + destination: 'packages/ckeditor5-bar', + messageId: 'id2' + } + ]; + + moveTranslations( defaultOptions ); + + expect( stubs.logger.error ).toHaveBeenCalledWith( + ' - Missing context: the "id2" message does not exist in "packages/ckeditor5-foo" package.' + ); + + expect( process.exit ).toHaveBeenCalledWith( 1 ); + } ); + } ); + } ); +} ); diff --git a/packages/ckeditor5-dev-translations/tests/synchronizetranslations.js b/packages/ckeditor5-dev-translations/tests/synchronizetranslations.js index a2693310d..86e8e3c60 100644 --- a/packages/ckeditor5-dev-translations/tests/synchronizetranslations.js +++ b/packages/ckeditor5-dev-translations/tests/synchronizetranslations.js @@ -6,7 +6,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import getPackageContexts from '../lib/utils/getpackagecontexts.js'; import getSourceMessages from '../lib/utils/getsourcemessages.js'; -import updatePackageTranslations from '../lib/utils/updatepackagetranslations.js'; +import synchronizeTranslationsBasedOnContext from '../lib/utils/synchronizetranslationsbasedoncontext.js'; import synchronizeTranslations from '../lib/synchronizetranslations.js'; const stubs = vi.hoisted( () => { @@ -23,7 +23,7 @@ vi.mock( '@ckeditor/ckeditor5-dev-utils', () => ( { } ) ); vi.mock( '../lib/utils/getpackagecontexts.js' ); vi.mock( '../lib/utils/getsourcemessages.js' ); -vi.mock( '../lib/utils/updatepackagetranslations.js' ); +vi.mock( '../lib/utils/synchronizetranslationsbasedoncontext.js' ); describe( 'synchronizeTranslations()', () => { let defaultOptions; @@ -105,8 +105,8 @@ describe( 'synchronizeTranslations()', () => { it( 'should synchronize translations files', () => { synchronizeTranslations( defaultOptions ); - expect( updatePackageTranslations ).toHaveBeenCalledTimes( 1 ); - expect( updatePackageTranslations ).toHaveBeenCalledWith( { + expect( synchronizeTranslationsBasedOnContext ).toHaveBeenCalledTimes( 1 ); + expect( synchronizeTranslationsBasedOnContext ).toHaveBeenCalledWith( { packageContexts: [], sourceMessages: [], skipLicenseHeader: false @@ -120,8 +120,8 @@ describe( 'synchronizeTranslations()', () => { synchronizeTranslations( defaultOptions ); - expect( updatePackageTranslations ).toHaveBeenCalledTimes( 1 ); - expect( updatePackageTranslations ).toHaveBeenCalledWith( { + expect( synchronizeTranslationsBasedOnContext ).toHaveBeenCalledTimes( 1 ); + expect( synchronizeTranslationsBasedOnContext ).toHaveBeenCalledWith( { packageContexts: [], sourceMessages: [], skipLicenseHeader: true @@ -134,7 +134,7 @@ describe( 'synchronizeTranslations()', () => { defaultOptions.validateOnly = true; synchronizeTranslations( defaultOptions ); - expect( updatePackageTranslations ).not.toHaveBeenCalled(); + expect( synchronizeTranslationsBasedOnContext ).not.toHaveBeenCalled(); expect( stubs.logger.info ).toHaveBeenCalledWith( '✨ No errors found.' ); } ); diff --git a/packages/ckeditor5-dev-translations/tests/utils/getpackagecontext.js b/packages/ckeditor5-dev-translations/tests/utils/getpackagecontext.js new file mode 100644 index 000000000..1e0465ae9 --- /dev/null +++ b/packages/ckeditor5-dev-translations/tests/utils/getpackagecontext.js @@ -0,0 +1,65 @@ +/** + * @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'fs-extra'; +import getPackageContext from '../../lib/utils/getpackagecontext.js'; + +vi.mock( 'fs-extra' ); + +describe( 'getPackageContext()', () => { + let defaultOptions; + + beforeEach( () => { + defaultOptions = { + packagePath: 'packages/ckeditor5-foo' + }; + + vi.mocked( fs.readJsonSync ).mockImplementation( path => { + if ( path === 'packages/ckeditor5-foo/lang/contexts.json' ) { + return { + id1: 'Context for message id1 from "ckeditor5-foo".' + }; + } + + return null; + } ); + } ); + + it( 'should be a function', () => { + expect( getPackageContext ).toBeInstanceOf( Function ); + } ); + + it( 'should read context file from package', () => { + getPackageContext( defaultOptions ); + + expect( fs.readJsonSync ).toHaveBeenCalledTimes( 1 ); + expect( fs.readJsonSync ).toHaveBeenCalledWith( 'packages/ckeditor5-foo/lang/contexts.json', { throws: false } ); + } ); + + it( 'should return package contexts', () => { + const result = getPackageContext( defaultOptions ); + + expect( result ).toEqual( expect.objectContaining( { + contextContent: { + id1: 'Context for message id1 from "ckeditor5-foo".' + }, + contextFilePath: 'packages/ckeditor5-foo/lang/contexts.json', + packagePath: 'packages/ckeditor5-foo' + } ) ); + } ); + + it( 'should return empty context if package does not have context file', () => { + defaultOptions.packagePath = 'packages/ckeditor5-bar'; + + const result = getPackageContext( defaultOptions ); + + expect( result ).toEqual( expect.objectContaining( { + contextContent: {}, + contextFilePath: 'packages/ckeditor5-bar/lang/contexts.json', + packagePath: 'packages/ckeditor5-bar' + } ) ); + } ); +} ); diff --git a/packages/ckeditor5-dev-translations/tests/utils/getpackagecontexts.js b/packages/ckeditor5-dev-translations/tests/utils/getpackagecontexts.js index 341340371..471860d78 100644 --- a/packages/ckeditor5-dev-translations/tests/utils/getpackagecontexts.js +++ b/packages/ckeditor5-dev-translations/tests/utils/getpackagecontexts.js @@ -4,10 +4,10 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import fs from 'fs-extra'; +import getPackageContext from '../../lib/utils/getpackagecontext.js'; import getPackageContexts from '../../lib/utils/getpackagecontexts.js'; -vi.mock( 'fs-extra' ); +vi.mock( '../../lib/utils/getpackagecontext.js' ); describe( 'getPackageContexts()', () => { let defaultOptions; @@ -18,32 +18,22 @@ describe( 'getPackageContexts()', () => { corePackagePath: 'packages/ckeditor5-core' }; - vi.mocked( fs.existsSync ).mockImplementation( path => { - if ( path === 'packages/ckeditor5-foo/lang/contexts.json' ) { - return true; - } + vi.mocked( getPackageContext ).mockImplementation( ( { packagePath } ) => { + const contextContent = {}; - if ( path === 'packages/ckeditor5-core/lang/contexts.json' ) { - return true; + if ( packagePath === 'packages/ckeditor5-foo' ) { + contextContent.id1 = 'Context for message id1 from "ckeditor5-foo".'; } - return false; - } ); - - vi.mocked( fs.readJsonSync ).mockImplementation( path => { - if ( path === 'packages/ckeditor5-foo/lang/contexts.json' ) { - return { - 'Text ID in "ckeditor5-foo"': 'Example context for text in "ckeditor5-foo".' - }; + if ( packagePath === 'packages/ckeditor5-core' ) { + contextContent.id2 = 'Context for message id2 from "ckeditor5-core".'; } - if ( path === 'packages/ckeditor5-core/lang/contexts.json' ) { - return { - 'Text ID in "ckeditor5-core"': 'Example context for text in "ckeditor5-core".' - }; - } - - throw new Error( `ENOENT: no such file or directory, open ${ path }` ); + return { + contextContent, + contextFilePath: packagePath + '/lang/contexts.json', + packagePath + }; } ); } ); @@ -51,18 +41,13 @@ describe( 'getPackageContexts()', () => { expect( getPackageContexts ).toBeInstanceOf( Function ); } ); - it( 'should read existing context files from packages (including core package)', () => { + it( 'should add core package if it is not included in the packages', () => { getPackageContexts( defaultOptions ); - expect( defaultOptions.packagePaths ).toEqual( expect.arrayContaining( [ 'packages/ckeditor5-core' ] ) ); - - expect( fs.existsSync ).toHaveBeenCalledTimes( 2 ); - expect( fs.existsSync ).toHaveBeenCalledWith( 'packages/ckeditor5-foo/lang/contexts.json' ); - expect( fs.existsSync ).toHaveBeenCalledWith( 'packages/ckeditor5-core/lang/contexts.json' ); - - expect( fs.readJsonSync ).toHaveBeenCalledTimes( 2 ); - expect( fs.readJsonSync ).toHaveBeenCalledWith( 'packages/ckeditor5-foo/lang/contexts.json' ); - expect( fs.readJsonSync ).toHaveBeenCalledWith( 'packages/ckeditor5-core/lang/contexts.json' ); + expect( defaultOptions.packagePaths ).toEqual( [ + 'packages/ckeditor5-foo', + 'packages/ckeditor5-core' + ] ); } ); it( 'should not duplicate core package if it is already included in the packages', () => { @@ -70,7 +55,10 @@ describe( 'getPackageContexts()', () => { getPackageContexts( defaultOptions ); - expect( defaultOptions.packagePaths ).toHaveLength( 2 ); + expect( defaultOptions.packagePaths ).toEqual( [ + 'packages/ckeditor5-foo', + 'packages/ckeditor5-core' + ] ); } ); it( 'should return package contexts', () => { @@ -81,7 +69,7 @@ describe( 'getPackageContexts()', () => { expect( result ).toEqual( expect.arrayContaining( [ { contextContent: { - 'Text ID in "ckeditor5-foo"': 'Example context for text in "ckeditor5-foo".' + id1: 'Context for message id1 from "ckeditor5-foo".' }, contextFilePath: 'packages/ckeditor5-foo/lang/contexts.json', packagePath: 'packages/ckeditor5-foo' @@ -90,7 +78,7 @@ describe( 'getPackageContexts()', () => { expect( result ).toEqual( expect.arrayContaining( [ { contextContent: { - 'Text ID in "ckeditor5-core"': 'Example context for text in "ckeditor5-core".' + id2: 'Context for message id2 from "ckeditor5-core".' }, contextFilePath: 'packages/ckeditor5-core/lang/contexts.json', packagePath: 'packages/ckeditor5-core' diff --git a/packages/ckeditor5-dev-translations/tests/utils/movetranslationsbetweenpackages.js b/packages/ckeditor5-dev-translations/tests/utils/movetranslationsbetweenpackages.js new file mode 100644 index 000000000..4f40ebde9 --- /dev/null +++ b/packages/ckeditor5-dev-translations/tests/utils/movetranslationsbetweenpackages.js @@ -0,0 +1,332 @@ +/** + * @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'fs-extra'; +import PO from 'pofile'; +import { glob } from 'glob'; +import cleanTranslationFileContent from '../../lib/utils/cleantranslationfilecontent.js'; +import moveTranslationsBetweenPackages from '../../lib/utils/movetranslationsbetweenpackages.js'; + +vi.mock( 'fs-extra' ); +vi.mock( 'pofile' ); +vi.mock( 'glob' ); +vi.mock( '../../lib/utils/cleantranslationfilecontent.js' ); + +describe( 'moveTranslationsBetweenPackages()', () => { + let defaultOptions, packageTranslationsFoo, packageTranslationsBar, packageContextFoo, packageContextBar; + + beforeEach( () => { + packageTranslationsFoo = [ + [ 'id1', 'Context for message id1 from "ckeditor5-foo".' ] + ]; + + packageTranslationsBar = [ + [ 'id2', 'Context for message id2 from "ckeditor5-bar".' ] + ]; + + packageContextFoo = { + packagePath: 'packages/ckeditor5-foo', + contextFilePath: 'packages/ckeditor5-foo/lang/contexts.json', + contextContent: Object.fromEntries( packageTranslationsFoo ) + }; + + packageContextBar = { + packagePath: 'packages/ckeditor5-bar', + contextFilePath: 'packages/ckeditor5-bar/lang/contexts.json', + contextContent: Object.fromEntries( packageTranslationsBar ) + }; + + defaultOptions = { + packageContexts: [ packageContextFoo, packageContextBar ], + config: [ + { + source: 'packages/ckeditor5-foo', + destination: 'packages/ckeditor5-bar', + messageId: 'id1' + } + ] + }; + + vi.mocked( fs.existsSync ).mockReturnValue( true ); + + vi.mocked( fs.readFileSync ).mockImplementation( path => { + if ( path.startsWith( 'packages/ckeditor5-foo/lang/translations/' ) ) { + return JSON.stringify( { + items: packageTranslationsFoo.map( ( [ msgid, msgctxt ] ) => ( { msgid, msgctxt } ) ) + } ); + } + + if ( path.startsWith( 'packages/ckeditor5-bar/lang/translations/' ) ) { + return JSON.stringify( { + items: packageTranslationsBar.map( ( [ msgid, msgctxt ] ) => ( { msgid, msgctxt } ) ) + } ); + } + + return JSON.stringify( {} ); + } ); + + vi.mocked( PO.parse ).mockImplementation( data => JSON.parse( data ) ); + + vi.mocked( glob.sync ).mockImplementation( pattern => [ + pattern.replace( '*', 'en' ), + pattern.replace( '*', 'pl' ) + ] ); + + vi.mocked( cleanTranslationFileContent ).mockReturnValue( { + toString: () => 'Clean PO file content.' + } ); + } ); + + it( 'should be a function', () => { + expect( moveTranslationsBetweenPackages ).toBeInstanceOf( Function ); + } ); + + it( 'should not move translations between packages if source and destination are the same', () => { + defaultOptions.config = [ { + source: 'packages/ckeditor5-foo', + destination: 'packages/ckeditor5-foo', + messageId: 'id1' + } ]; + + moveTranslationsBetweenPackages( defaultOptions ); + + expect( packageContextFoo ).toEqual( { + packagePath: 'packages/ckeditor5-foo', + contextFilePath: 'packages/ckeditor5-foo/lang/contexts.json', + contextContent: { + id1: 'Context for message id1 from "ckeditor5-foo".' + } + } ); + + expect( packageContextBar ).toEqual( { + packagePath: 'packages/ckeditor5-bar', + contextFilePath: 'packages/ckeditor5-bar/lang/contexts.json', + contextContent: { + id2: 'Context for message id2 from "ckeditor5-bar".' + } + } ); + + expect( fs.outputJsonSync ).not.toHaveBeenCalled(); + expect( fs.outputFileSync ).not.toHaveBeenCalled(); + } ); + + it( 'should move translation context between packages', () => { + moveTranslationsBetweenPackages( defaultOptions ); + + expect( packageContextFoo ).toEqual( { + packagePath: 'packages/ckeditor5-foo', + contextFilePath: 'packages/ckeditor5-foo/lang/contexts.json', + contextContent: {} + } ); + + expect( packageContextBar ).toEqual( { + packagePath: 'packages/ckeditor5-bar', + contextFilePath: 'packages/ckeditor5-bar/lang/contexts.json', + contextContent: { + id1: 'Context for message id1 from "ckeditor5-foo".', + id2: 'Context for message id2 from "ckeditor5-bar".' + } + } ); + } ); + + it( 'should overwrite existing translation context in destination package', () => { + packageContextBar.contextContent.id1 = 'Context for message id1 from "ckeditor5-bar".'; + + moveTranslationsBetweenPackages( defaultOptions ); + + expect( packageContextFoo ).toEqual( { + packagePath: 'packages/ckeditor5-foo', + contextFilePath: 'packages/ckeditor5-foo/lang/contexts.json', + contextContent: {} + } ); + + expect( packageContextBar ).toEqual( { + packagePath: 'packages/ckeditor5-bar', + contextFilePath: 'packages/ckeditor5-bar/lang/contexts.json', + contextContent: { + id1: 'Context for message id1 from "ckeditor5-foo".', + id2: 'Context for message id2 from "ckeditor5-bar".' + } + } ); + } ); + + it( 'should save translation contexts on filesystem', () => { + moveTranslationsBetweenPackages( defaultOptions ); + + expect( fs.outputJsonSync ).toHaveBeenCalledTimes( 2 ); + expect( fs.outputJsonSync ).toHaveBeenCalledWith( + 'packages/ckeditor5-foo/lang/contexts.json', + {}, + { spaces: '\t' } + ); + + expect( fs.outputJsonSync ).toHaveBeenCalledWith( + 'packages/ckeditor5-bar/lang/contexts.json', + { + id1: 'Context for message id1 from "ckeditor5-foo".', + id2: 'Context for message id2 from "ckeditor5-bar".' + }, + { spaces: '\t' } + ); + } ); + + it( 'should search for source translation files', () => { + moveTranslationsBetweenPackages( defaultOptions ); + + expect( glob.sync ).toHaveBeenCalledTimes( 1 ); + expect( glob.sync ).toHaveBeenCalledWith( 'packages/ckeditor5-foo/lang/translations/*.po' ); + } ); + + it( 'should parse each translation file', () => { + moveTranslationsBetweenPackages( defaultOptions ); + + expect( fs.readFileSync ).toHaveBeenCalledTimes( 4 ); + expect( fs.readFileSync ).toHaveBeenCalledWith( 'packages/ckeditor5-foo/lang/translations/en.po', 'utf-8' ); + expect( fs.readFileSync ).toHaveBeenCalledWith( 'packages/ckeditor5-foo/lang/translations/pl.po', 'utf-8' ); + expect( fs.readFileSync ).toHaveBeenCalledWith( 'packages/ckeditor5-bar/lang/translations/en.po', 'utf-8' ); + expect( fs.readFileSync ).toHaveBeenCalledWith( 'packages/ckeditor5-bar/lang/translations/pl.po', 'utf-8' ); + + expect( PO.parse ).toHaveBeenCalledTimes( 4 ); + expect( PO.parse ).toHaveBeenNthCalledWith( + 1, + '{"items":[{"msgid":"id1","msgctxt":"Context for message id1 from \\"ckeditor5-foo\\"."}]}' + ); + expect( PO.parse ).toHaveBeenNthCalledWith( + 2, + '{"items":[{"msgid":"id2","msgctxt":"Context for message id2 from \\"ckeditor5-bar\\"."}]}' + ); + expect( PO.parse ).toHaveBeenNthCalledWith( + 3, + '{"items":[{"msgid":"id1","msgctxt":"Context for message id1 from \\"ckeditor5-foo\\"."}]}' + ); + expect( PO.parse ).toHaveBeenNthCalledWith( + 4, + '{"items":[{"msgid":"id2","msgctxt":"Context for message id2 from \\"ckeditor5-bar\\"."}]}' + ); + } ); + + it( 'should move translations between packages for each language', () => { + moveTranslationsBetweenPackages( defaultOptions ); + + const [ + sourceTranslationsFooEn, + sourceTranslationsBarEn, + sourceTranslationsFooPl, + sourceTranslationsBarPl + ] = PO.parse.mock.results.map( entry => entry.value ); + + expect( sourceTranslationsFooEn.items ).toEqual( [] ); + expect( sourceTranslationsBarEn.items ).toEqual( [ + { msgid: 'id2', msgctxt: 'Context for message id2 from "ckeditor5-bar".' }, + { msgid: 'id1', msgctxt: 'Context for message id1 from "ckeditor5-foo".' } + ] ); + + expect( sourceTranslationsFooPl.items ).toEqual( [] ); + expect( sourceTranslationsBarPl.items ).toEqual( [ + { msgid: 'id2', msgctxt: 'Context for message id2 from "ckeditor5-bar".' }, + { msgid: 'id1', msgctxt: 'Context for message id1 from "ckeditor5-foo".' } + ] ); + } ); + + it( 'should overwrite existing translations in destination package', () => { + packageTranslationsBar.push( + [ 'id1', 'Context for message id1 from "ckeditor5-bar".' ] + ); + + moveTranslationsBetweenPackages( defaultOptions ); + + const [ + sourceTranslationsFooEn, + sourceTranslationsBarEn, + sourceTranslationsFooPl, + sourceTranslationsBarPl + ] = PO.parse.mock.results.map( entry => entry.value ); + + expect( sourceTranslationsFooEn.items ).toEqual( [] ); + expect( sourceTranslationsBarEn.items ).toEqual( [ + { msgid: 'id2', msgctxt: 'Context for message id2 from "ckeditor5-bar".' }, + { msgid: 'id1', msgctxt: 'Context for message id1 from "ckeditor5-foo".' } + ] ); + + expect( sourceTranslationsFooPl.items ).toEqual( [] ); + expect( sourceTranslationsBarPl.items ).toEqual( [ + { msgid: 'id2', msgctxt: 'Context for message id2 from "ckeditor5-bar".' }, + { msgid: 'id1', msgctxt: 'Context for message id1 from "ckeditor5-foo".' } + ] ); + } ); + + it( 'should use the source translation file as a base if the destination file does not exist', () => { + vi.mocked( fs.existsSync ).mockImplementation( path => { + return path !== 'packages/ckeditor5-bar/lang/translations/pl.po'; + } ); + + moveTranslationsBetweenPackages( defaultOptions ); + + expect( PO.parse ).toHaveBeenCalledTimes( 4 ); + expect( PO.parse ).toHaveBeenNthCalledWith( + 1, + '{"items":[{"msgid":"id1","msgctxt":"Context for message id1 from \\"ckeditor5-foo\\"."}]}' + ); + expect( PO.parse ).toHaveBeenNthCalledWith( + 2, + '{"items":[{"msgid":"id2","msgctxt":"Context for message id2 from \\"ckeditor5-bar\\"."}]}' + ); + expect( PO.parse ).toHaveBeenNthCalledWith( + 3, + '{"items":[{"msgid":"id1","msgctxt":"Context for message id1 from \\"ckeditor5-foo\\"."}]}' + ); + expect( PO.parse ).toHaveBeenNthCalledWith( + 4, + '{"items":[{"msgid":"id1","msgctxt":"Context for message id1 from \\"ckeditor5-foo\\"."}]}' + ); + + const [ + sourceTranslationsFooEn, + sourceTranslationsBarEn, + sourceTranslationsFooPl, + sourceTranslationsBarPl + ] = PO.parse.mock.results.map( entry => entry.value ); + + expect( sourceTranslationsFooEn.items ).toEqual( [] ); + expect( sourceTranslationsBarEn.items ).toEqual( [ + { msgid: 'id2', msgctxt: 'Context for message id2 from "ckeditor5-bar".' }, + { msgid: 'id1', msgctxt: 'Context for message id1 from "ckeditor5-foo".' } + ] ); + + expect( sourceTranslationsFooPl.items ).toEqual( [] ); + expect( sourceTranslationsBarPl.items ).toEqual( [ + { msgid: 'id1', msgctxt: 'Context for message id1 from "ckeditor5-foo".' } + ] ); + } ); + + it( 'should save updated translation files on filesystem after cleaning the content', () => { + moveTranslationsBetweenPackages( defaultOptions ); + + expect( cleanTranslationFileContent ).toHaveBeenCalledTimes( 4 ); + + expect( fs.outputFileSync ).toHaveBeenCalledTimes( 4 ); + expect( fs.outputFileSync ).toHaveBeenCalledWith( + 'packages/ckeditor5-foo/lang/translations/en.po', + 'Clean PO file content.', + 'utf-8' + ); + expect( fs.outputFileSync ).toHaveBeenCalledWith( + 'packages/ckeditor5-foo/lang/translations/pl.po', + 'Clean PO file content.', + 'utf-8' + ); + expect( fs.outputFileSync ).toHaveBeenCalledWith( + 'packages/ckeditor5-bar/lang/translations/en.po', + 'Clean PO file content.', + 'utf-8' + ); + expect( fs.outputFileSync ).toHaveBeenCalledWith( + 'packages/ckeditor5-bar/lang/translations/pl.po', + 'Clean PO file content.', + 'utf-8' + ); + } ); +} ); diff --git a/packages/ckeditor5-dev-translations/tests/utils/updatepackagetranslations.js b/packages/ckeditor5-dev-translations/tests/utils/synchronizetranslationsbasedoncontext.js similarity index 85% rename from packages/ckeditor5-dev-translations/tests/utils/updatepackagetranslations.js rename to packages/ckeditor5-dev-translations/tests/utils/synchronizetranslationsbasedoncontext.js index 6c9ff8882..39ebda590 100644 --- a/packages/ckeditor5-dev-translations/tests/utils/updatepackagetranslations.js +++ b/packages/ckeditor5-dev-translations/tests/utils/synchronizetranslationsbasedoncontext.js @@ -9,7 +9,7 @@ import PO from 'pofile'; import { glob } from 'glob'; import cleanTranslationFileContent from '../../lib/utils/cleantranslationfilecontent.js'; import createMissingPackageTranslations from '../../lib/utils/createmissingpackagetranslations.js'; -import updatePackageTranslations from '../../lib/utils/updatepackagetranslations.js'; +import synchronizeTranslationsBasedOnContext from '../../lib/utils/synchronizetranslationsbasedoncontext.js'; vi.mock( 'fs-extra' ); vi.mock( 'pofile' ); @@ -17,7 +17,7 @@ vi.mock( 'glob' ); vi.mock( '../../lib/utils/createmissingpackagetranslations.js' ); vi.mock( '../../lib/utils/cleantranslationfilecontent.js' ); -describe( 'updatePackageTranslations()', () => { +describe( 'synchronizeTranslationsBasedOnContext()', () => { let defaultOptions, translations, stubs; beforeEach( () => { @@ -26,8 +26,8 @@ describe( 'updatePackageTranslations()', () => { { packagePath: 'packages/ckeditor5-foo', contextContent: { - 'id1': 'Context for example message 1', - 'id2': 'Context for example message 2' + id1: 'Context for example message 1', + id2: 'Context for example message 2' } } ], @@ -83,11 +83,11 @@ describe( 'updatePackageTranslations()', () => { } ); it( 'should be a function', () => { - expect( updatePackageTranslations ).toBeInstanceOf( Function ); + expect( synchronizeTranslationsBasedOnContext ).toBeInstanceOf( Function ); } ); it( 'should create missing translations', () => { - updatePackageTranslations( defaultOptions ); + synchronizeTranslationsBasedOnContext( defaultOptions ); expect( createMissingPackageTranslations ).toHaveBeenCalledTimes( 1 ); expect( createMissingPackageTranslations ).toHaveBeenCalledWith( { @@ -99,7 +99,7 @@ describe( 'updatePackageTranslations()', () => { it( 'should create missing translations with skipping the license header', () => { defaultOptions.skipLicenseHeader = true; - updatePackageTranslations( defaultOptions ); + synchronizeTranslationsBasedOnContext( defaultOptions ); expect( createMissingPackageTranslations ).toHaveBeenCalledTimes( 1 ); expect( createMissingPackageTranslations ).toHaveBeenCalledWith( { @@ -116,14 +116,14 @@ describe( 'updatePackageTranslations()', () => { } ]; - updatePackageTranslations( defaultOptions ); + synchronizeTranslationsBasedOnContext( defaultOptions ); expect( createMissingPackageTranslations ).not.toHaveBeenCalled(); expect( fs.writeFileSync ).not.toHaveBeenCalled(); } ); it( 'should search for translation files', () => { - updatePackageTranslations( defaultOptions ); + synchronizeTranslationsBasedOnContext( defaultOptions ); expect( glob.sync ).toHaveBeenCalledTimes( 1 ); expect( glob.sync ).toHaveBeenCalledWith( 'packages/ckeditor5-foo/lang/translations/*.po' ); @@ -134,7 +134,7 @@ describe( 'updatePackageTranslations()', () => { return [ 'en', 'pl' ].map( language => pattern.replace( '*', language ) ); } ); - updatePackageTranslations( defaultOptions ); + synchronizeTranslationsBasedOnContext( defaultOptions ); expect( fs.readFileSync ).toHaveBeenCalledTimes( 2 ); expect( fs.readFileSync ).toHaveBeenCalledWith( 'packages/ckeditor5-foo/lang/translations/en.po', 'utf-8' ); @@ -147,7 +147,7 @@ describe( 'updatePackageTranslations()', () => { { msgid: 'id4' } ); - updatePackageTranslations( defaultOptions ); + synchronizeTranslationsBasedOnContext( defaultOptions ); expect( translations.items ).toEqual( [ { msgid: 'id1' }, @@ -158,7 +158,7 @@ describe( 'updatePackageTranslations()', () => { it( 'should add missing translations', () => { translations.items = []; - updatePackageTranslations( defaultOptions ); + synchronizeTranslationsBasedOnContext( defaultOptions ); expect( translations.items ).toEqual( [ { @@ -177,7 +177,7 @@ describe( 'updatePackageTranslations()', () => { } ); it( 'should save updated translation files on filesystem after cleaning the content', () => { - updatePackageTranslations( defaultOptions ); + synchronizeTranslationsBasedOnContext( defaultOptions ); expect( cleanTranslationFileContent ).toHaveBeenCalledTimes( 1 ); @@ -192,7 +192,7 @@ describe( 'updatePackageTranslations()', () => { it( 'should not save translation files on filesystem if their content is not updated', () => { vi.mocked( cleanTranslationFileContent ).mockImplementation( input => input ); - updatePackageTranslations( defaultOptions ); + synchronizeTranslationsBasedOnContext( defaultOptions ); expect( fs.writeFileSync ).not.toHaveBeenCalled(); } ); From 203d6fc0515b40b00fa8231cc087807c10297bc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Smyrek?= Date: Fri, 18 Oct 2024 07:52:24 +0200 Subject: [PATCH 2/5] When adding missing translation, use its id instead of the translated text. --- .../lib/utils/synchronizetranslationsbasedoncontext.js | 2 +- .../tests/utils/synchronizetranslationsbasedoncontext.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-dev-translations/lib/utils/synchronizetranslationsbasedoncontext.js b/packages/ckeditor5-dev-translations/lib/utils/synchronizetranslationsbasedoncontext.js index 486c488cb..75d9d5af5 100644 --- a/packages/ckeditor5-dev-translations/lib/utils/synchronizetranslationsbasedoncontext.js +++ b/packages/ckeditor5-dev-translations/lib/utils/synchronizetranslationsbasedoncontext.js @@ -55,7 +55,7 @@ export default function synchronizeTranslationsBasedOnContext( { packageContexts const item = new PO.Item( { nplurals: numberOfPluralForms } ); item.msgctxt = contextContent[ message.id ]; - item.msgid = message.string; + item.msgid = message.id; item.msgstr.push( '' ); if ( message.plural ) { diff --git a/packages/ckeditor5-dev-translations/tests/utils/synchronizetranslationsbasedoncontext.js b/packages/ckeditor5-dev-translations/tests/utils/synchronizetranslationsbasedoncontext.js index 39ebda590..58d4e1fd5 100644 --- a/packages/ckeditor5-dev-translations/tests/utils/synchronizetranslationsbasedoncontext.js +++ b/packages/ckeditor5-dev-translations/tests/utils/synchronizetranslationsbasedoncontext.js @@ -162,13 +162,13 @@ describe( 'synchronizeTranslationsBasedOnContext()', () => { expect( translations.items ).toEqual( [ { - msgid: 'Example message 1', + msgid: 'id1', msgctxt: 'Context for example message 1', msgid_plural: '', msgstr: [ '' ] }, { - msgid: 'Example message 2', + msgid: 'id2', msgctxt: 'Context for example message 2', msgid_plural: 'Example message 2 - plural form', msgstr: [ '', '', '', '' ] From 2c456b019f1837f24499ab8787db6f8e76c51a5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Smyrek?= Date: Fri, 18 Oct 2024 09:45:23 +0200 Subject: [PATCH 3/5] Detect if translations to move are not duplicated. --- .../lib/movetranslations.js | 20 ++++ .../tests/movetranslations.js | 92 +++++++++++++++++++ 2 files changed, 112 insertions(+) diff --git a/packages/ckeditor5-dev-translations/lib/movetranslations.js b/packages/ckeditor5-dev-translations/lib/movetranslations.js index 956a0d825..1dc65d131 100644 --- a/packages/ckeditor5-dev-translations/lib/movetranslations.js +++ b/packages/ckeditor5-dev-translations/lib/movetranslations.js @@ -10,6 +10,7 @@ import moveTranslationsBetweenPackages from './utils/movetranslationsbetweenpack /** * Moves the requested translations (context and messages) between packages by performing the following steps: + * * Detect if translations to move are not duplicated. * * Detect if both source and destination packages exist. * * Detect if translation context to move exists in the source package. Message may not exist in the target package, * but if it does, it will be overwritten. @@ -33,6 +34,7 @@ export default function moveTranslations( options ) { log.info( '📍 Checking provided configuration...' ); errors.push( + ...assertTranslationMoveEntriesUnique( { config } ), ...assertPackagesExist( { config } ), ...assertContextsExist( { packageContexts, config } ) ); @@ -53,6 +55,24 @@ export default function moveTranslations( options ) { log.info( '✨ Done.' ); } +/** + * @param {object} options + * @param {Array.} options.config Configuration that defines the messages to move. + * @returns {Array.} + */ +function assertTranslationMoveEntriesUnique( { config } ) { + const moveEntriesGroupedByMessageId = config.reduce( ( result, entry ) => { + result[ entry.messageId ] = result[ entry.messageId ] || 0; + result[ entry.messageId ]++; + + return result; + }, {} ); + + return Object.keys( moveEntriesGroupedByMessageId ) + .filter( messageId => moveEntriesGroupedByMessageId[ messageId ] > 1 ) + .map( messageId => `Duplicated entry: the "${ messageId }" message is configured to be moved multiple times.` ); +} + /** * @param {object} options * @param {Array.} options.config Configuration that defines the messages to move. diff --git a/packages/ckeditor5-dev-translations/tests/movetranslations.js b/packages/ckeditor5-dev-translations/tests/movetranslations.js index 0b5817351..b5f381bd2 100644 --- a/packages/ckeditor5-dev-translations/tests/movetranslations.js +++ b/packages/ckeditor5-dev-translations/tests/movetranslations.js @@ -110,6 +110,98 @@ describe( 'moveTranslations()', () => { } ); describe( 'validation', () => { + describe( 'unique move entries', () => { + it( 'should return no error if there are unique entries (one entry, no duplicates)', () => { + moveTranslations( defaultOptions ); + + expect( stubs.logger.error ).not.toHaveBeenCalledWith( expect.stringContaining( 'Duplicated entry' ) ); + } ); + + it( 'should return no error if there are unique entries (many entries, no duplicates)', () => { + defaultOptions = { + config: [ + { + source: 'packages/ckeditor5-foo', + destination: 'packages/ckeditor5-bar', + messageId: 'id1' + }, + { + source: 'packages/ckeditor5-bar', + destination: 'packages/ckeditor5-foo', + messageId: 'id2' + } + ] + }; + + moveTranslations( defaultOptions ); + + expect( stubs.logger.error ).not.toHaveBeenCalledWith( expect.stringContaining( 'Duplicated entry' ) ); + } ); + + it( 'should return error if there are duplicated entries (many entries, one duplicated entry)', () => { + defaultOptions = { + config: [ + { + source: 'packages/ckeditor5-foo', + destination: 'packages/ckeditor5-bar', + messageId: 'id1' + }, + { + source: 'packages/ckeditor5-foo', + destination: 'packages/ckeditor5-bar', + messageId: 'id1' + } + ] + }; + + moveTranslations( defaultOptions ); + + expect( stubs.logger.error ).toHaveBeenCalledWith( + ' - Duplicated entry: the "id1" message is configured to be moved multiple times.' + ); + + expect( process.exit ).toHaveBeenCalledWith( 1 ); + } ); + + it( 'should return error once for each duplicated entry (many entries, many repeated duplicated entries)', () => { + defaultOptions = { + config: [ + { + source: 'packages/ckeditor5-foo', + destination: 'packages/ckeditor5-bar', + messageId: 'id1' + }, + { + source: 'packages/ckeditor5-foo', + destination: 'packages/ckeditor5-bar', + messageId: 'id1' + }, + { + source: 'packages/ckeditor5-foo', + destination: 'packages/ckeditor5-bar', + messageId: 'id1' + } + ] + }; + + moveTranslations( defaultOptions ); + + expect( stubs.logger.error ).toHaveBeenCalledWith( + ' - Duplicated entry: the "id1" message is configured to be moved multiple times.' + ); + + const callsWithDuplicatedEntryLog = stubs.logger.error.mock.calls.filter( call => { + const [ arg ] = call; + + return arg.includes( 'Duplicated entry' ); + } ); + + expect( callsWithDuplicatedEntryLog.length ).toEqual( 1 ); + + expect( process.exit ).toHaveBeenCalledWith( 1 ); + } ); + } ); + describe( 'packages exist', () => { it( 'should return no error if there is no missing package', () => { moveTranslations( defaultOptions ); From bd6320bf55dd23b40c41e05b3f1c02370d48a31f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Smyrek?= Date: Fri, 18 Oct 2024 09:57:30 +0200 Subject: [PATCH 4/5] Fixed filename for `sr@latin` language to be the same as previously: `sr-latn`. --- packages/ckeditor5-dev-translations/lib/utils/getlanguages.js | 1 + packages/ckeditor5-dev-translations/tests/utils/getlanguages.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-dev-translations/lib/utils/getlanguages.js b/packages/ckeditor5-dev-translations/lib/utils/getlanguages.js index dc6b27211..8259687fc 100644 --- a/packages/ckeditor5-dev-translations/lib/utils/getlanguages.js +++ b/packages/ckeditor5-dev-translations/lib/utils/getlanguages.js @@ -81,6 +81,7 @@ const SUPPORTED_LOCALES = [ const LOCALES_FILENAME_MAP = { 'ne_NP': 'ne', 'si_LK': 'si', + 'sr@latin': 'sr-latn', 'zh_TW': 'zh' }; diff --git a/packages/ckeditor5-dev-translations/tests/utils/getlanguages.js b/packages/ckeditor5-dev-translations/tests/utils/getlanguages.js index fbede17a3..2327f8206 100644 --- a/packages/ckeditor5-dev-translations/tests/utils/getlanguages.js +++ b/packages/ckeditor5-dev-translations/tests/utils/getlanguages.js @@ -40,7 +40,7 @@ describe( 'getLanguages()', () => { expect( languageSerbianLatin ).toEqual( { localeCode: 'sr@latin', languageCode: 'sr', - languageFileName: 'sr-latin' + languageFileName: 'sr-latn' } ); } ); From 5eb675f3874e4aefc83807afcfe12c182521eccd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Smyrek?= Date: Mon, 21 Oct 2024 08:27:06 +0200 Subject: [PATCH 5/5] Renamed typedefs. --- .../ckeditor5-dev-translations/lib/movetranslations.js | 2 +- .../lib/synchronizetranslations.js | 10 +++++----- .../lib/utils/getpackagecontext.js | 4 ++-- .../lib/utils/getpackagecontexts.js | 2 +- .../lib/utils/getsourcemessages.js | 4 ++-- .../lib/utils/movetranslationsbetweenpackages.js | 2 +- .../lib/utils/synchronizetranslationsbasedoncontext.js | 4 ++-- 7 files changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/ckeditor5-dev-translations/lib/movetranslations.js b/packages/ckeditor5-dev-translations/lib/movetranslations.js index 1dc65d131..6ea417e05 100644 --- a/packages/ckeditor5-dev-translations/lib/movetranslations.js +++ b/packages/ckeditor5-dev-translations/lib/movetranslations.js @@ -98,7 +98,7 @@ function assertPackagesExist( { config } ) { /** * @param {object} options - * @param {Array.} options.packageContexts An array of language contexts. + * @param {Array.} options.packageContexts An array of language contexts. * @param {Array.} options.config Configuration that defines the messages to move. * @returns {Array.} */ diff --git a/packages/ckeditor5-dev-translations/lib/synchronizetranslations.js b/packages/ckeditor5-dev-translations/lib/synchronizetranslations.js index 0b75f04d9..a940a1d64 100644 --- a/packages/ckeditor5-dev-translations/lib/synchronizetranslations.js +++ b/packages/ckeditor5-dev-translations/lib/synchronizetranslations.js @@ -79,8 +79,8 @@ export default function synchronizeTranslations( options ) { /** * @param {object} options - * @param {Array.} options.packageContexts An array of language contexts. - * @param {Array.} options.sourceMessages An array of i18n source messages. + * @param {Array.} options.packageContexts An array of language contexts. + * @param {Array.} options.sourceMessages An array of i18n source messages. * @param {string} options.corePackagePath A relative to `process.cwd()` path to the `@ckeditor/ckeditor5-core` package. * @returns {Array.} */ @@ -105,8 +105,8 @@ function assertNoMissingContext( { packageContexts, sourceMessages, corePackageP /** * @param {object} options - * @param {Array.} options.packageContexts An array of language contexts. - * @param {Array.} options.sourceMessages An array of i18n source messages. + * @param {Array.} options.packageContexts An array of language contexts. + * @param {Array.} options.sourceMessages An array of i18n source messages. * @param {string} options.corePackagePath A relative to `process.cwd()` path to the `@ckeditor/ckeditor5-core` package. * @param {boolean} options.ignoreUnusedCorePackageContexts Whether to skip unused context errors related to the `@ckeditor/ckeditor5-core` * package. @@ -152,7 +152,7 @@ function assertAllContextUsed( { packageContexts, sourceMessages, corePackagePat /** * @param {object} options - * @param {Array.} options.packageContexts An array of language contexts. + * @param {Array.} options.packageContexts An array of language contexts. * @returns {Array.} */ function assertNoRepeatedContext( { packageContexts } ) { diff --git a/packages/ckeditor5-dev-translations/lib/utils/getpackagecontext.js b/packages/ckeditor5-dev-translations/lib/utils/getpackagecontext.js index 57932cbdc..f97d745d3 100644 --- a/packages/ckeditor5-dev-translations/lib/utils/getpackagecontext.js +++ b/packages/ckeditor5-dev-translations/lib/utils/getpackagecontext.js @@ -10,7 +10,7 @@ import { CONTEXT_FILE_PATH } from './constants.js'; /** * @param {object} options * @param {string} options.packagePath Path to the package containing the context. - * @returns {Context} + * @returns {TranslationsContext} */ export default function getPackageContext( { packagePath } ) { const contextFilePath = upath.join( packagePath, CONTEXT_FILE_PATH ); @@ -24,7 +24,7 @@ export default function getPackageContext( { packagePath } ) { } /** - * @typedef {object} Context + * @typedef {object} TranslationsContext * * @property {string} contextFilePath * @property {object} contextContent diff --git a/packages/ckeditor5-dev-translations/lib/utils/getpackagecontexts.js b/packages/ckeditor5-dev-translations/lib/utils/getpackagecontexts.js index fa5c2277b..cce345a21 100644 --- a/packages/ckeditor5-dev-translations/lib/utils/getpackagecontexts.js +++ b/packages/ckeditor5-dev-translations/lib/utils/getpackagecontexts.js @@ -9,7 +9,7 @@ import getPackageContext from './getpackagecontext.js'; * @param {object} options * @param {Array.} options.packagePaths An array of paths to packages, which will be used to find message contexts. * @param {string} options.corePackagePath A relative to `process.cwd()` path to the `@ckeditor/ckeditor5-core` package. - * @returns {Array.} + * @returns {Array.} */ export default function getPackageContexts( { packagePaths, corePackagePath } ) { // Add path to the core package if not included in the package paths. diff --git a/packages/ckeditor5-dev-translations/lib/utils/getsourcemessages.js b/packages/ckeditor5-dev-translations/lib/utils/getsourcemessages.js index 94ca1774b..6336774e5 100644 --- a/packages/ckeditor5-dev-translations/lib/utils/getsourcemessages.js +++ b/packages/ckeditor5-dev-translations/lib/utils/getsourcemessages.js @@ -11,7 +11,7 @@ import findMessages from '../findmessages.js'; * @param {Array.} options.packagePaths An array of paths to packages that contain source files with messages to translate. * @param {Array.} options.sourceFiles An array of source files that contain messages to translate. * @param {Function} options.onErrorCallback Called when there is an error with parsing the source files. - * @returns {Array.} + * @returns {Array.} */ export default function getSourceMessages( { packagePaths, sourceFiles, onErrorCallback } ) { return sourceFiles @@ -32,7 +32,7 @@ export default function getSourceMessages( { packagePaths, sourceFiles, onErrorC } /** - * @typedef {object} Message + * @typedef {object} TranslatableEntry * * @property {string} id * @property {string} string diff --git a/packages/ckeditor5-dev-translations/lib/utils/movetranslationsbetweenpackages.js b/packages/ckeditor5-dev-translations/lib/utils/movetranslationsbetweenpackages.js index 1225a03f0..1e5fceddb 100644 --- a/packages/ckeditor5-dev-translations/lib/utils/movetranslationsbetweenpackages.js +++ b/packages/ckeditor5-dev-translations/lib/utils/movetranslationsbetweenpackages.js @@ -12,7 +12,7 @@ import cleanTranslationFileContent from './cleantranslationfilecontent.js'; /** * @param {object} options - * @param {Array.} options.packageContexts An array of language contexts. + * @param {Array.} options.packageContexts An array of language contexts. * @param {Array.} options.config Configuration that defines the messages to move. */ export default function moveTranslationsBetweenPackages( { packageContexts, config } ) { diff --git a/packages/ckeditor5-dev-translations/lib/utils/synchronizetranslationsbasedoncontext.js b/packages/ckeditor5-dev-translations/lib/utils/synchronizetranslationsbasedoncontext.js index 75d9d5af5..31d2df852 100644 --- a/packages/ckeditor5-dev-translations/lib/utils/synchronizetranslationsbasedoncontext.js +++ b/packages/ckeditor5-dev-translations/lib/utils/synchronizetranslationsbasedoncontext.js @@ -13,8 +13,8 @@ import cleanTranslationFileContent from './cleantranslationfilecontent.js'; /** * @param {object} options - * @param {Array.} options.packageContexts An array of language contexts. - * @param {Array.} options.sourceMessages An array of i18n source messages. + * @param {Array.} options.packageContexts An array of language contexts. + * @param {Array.} options.sourceMessages An array of i18n source messages. * @param {boolean} options.skipLicenseHeader Whether to skip adding the license header to newly created translation files. */ export default function synchronizeTranslationsBasedOnContext( { packageContexts, sourceMessages, skipLicenseHeader } ) {