From bb32819f12ecd5c425c35564eff2719bbafc1de9 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Tue, 23 Apr 2024 13:11:55 +0200 Subject: [PATCH] refactor: simplify logic in get-challenges (#54413) --- curriculum/get-challenges.js | 84 +++++++++++++------------------ curriculum/get-challenges.test.js | 28 +++-------- 2 files changed, 41 insertions(+), 71 deletions(-) diff --git a/curriculum/get-challenges.js b/curriculum/get-challenges.js index f369118cced..a0b2229b311 100644 --- a/curriculum/get-challenges.js +++ b/curriculum/get-challenges.js @@ -133,6 +133,10 @@ const walk = (root, target, options, cb) => { }; exports.getChallengesForLang = async function getChallengesForLang(lang) { + const invalidLang = !curriculumLangs.includes(lang); + if (invalidLang) + throw Error(`${lang} is not a accepted language. +Accepted languages are ${curriculumLangs.join(', ')}`); // english determines the shape of the curriculum, all other languages mirror // it. const root = getChallengesDirForLang('english'); @@ -217,48 +221,26 @@ async function buildChallenges({ path: filePath }, curriculum, lang) { } const { meta } = challengeBlock; const isCert = path.extname(filePath) === '.yml'; - const createChallenge = generateChallengeCreator(CHALLENGES_DIR, lang); + const englishPath = path.resolve( + __dirname, + CHALLENGES_DIR, + 'english', + filePath + ); + const i18nPath = path.resolve(__dirname, CHALLENGES_DIR, lang, filePath); + const createChallenge = generateChallengeCreator(lang, englishPath, i18nPath); + + await assertHasEnglishSource(filePath, lang, englishPath); const challenge = isCert - ? await createCertification(CHALLENGES_DIR, filePath, lang) + ? await parseCert(englishPath) : await createChallenge(filePath, meta); challengeBlock.challenges = [...challengeBlock.challenges, challenge]; } -async function createCertification(basePath, filePath) { - function getFullPath(pathLang) { - return path.resolve(__dirname, basePath, pathLang, filePath); - } - // TODO: restart using isAudited() once we can determine a) the superBlocks - // (plural) a certification belongs to and b) get that info from the parsed - // certification, rather than the path. ASSUMING that this is used by the - // client. If not, delete this comment and the lang param. - return parseCert(getFullPath('english')); -} - // This is a slightly weird abstraction, but it lets us define helper functions // without passing around a ton of arguments. -function generateChallengeCreator(basePath, lang) { - function getFullPath(pathLang, filePath) { - return path.resolve(__dirname, basePath, pathLang, filePath); - } - - async function validate(filePath) { - const invalidLang = !curriculumLangs.includes(lang); - if (invalidLang) - throw Error(`${lang} is not a accepted language. -Trying to parse ${filePath}`); - - const missingEnglish = - lang !== 'english' && !(await hasEnglishSource(basePath, filePath)); - if (missingEnglish) - throw Error(`Missing English challenge for -${filePath} -It should be in -${getFullPath('english', filePath)} -`); - } - +function generateChallengeCreator(lang, englishPath, i18nPath) { function addMetaToChallenge(challenge, meta) { const challengeOrder = findIndex( meta.challengeOrder, @@ -300,12 +282,6 @@ ${getFullPath('english', filePath)} challenge.template = meta.template; challenge.time = meta.time; challenge.helpCategory = challenge.helpCategory || meta.helpCategory; - challenge.translationPending = - lang !== 'english' && - !isAuditedSuperBlock(lang, meta.superBlock, { - showNewCurriculum: process.env.SHOW_NEW_CURRICULUM === 'true', - showUpcomingChanges: process.env.SHOW_UPCOMING_CHANGES === 'true' - }); challenge.usesMultifileEditor = !!meta.usesMultifileEditor; challenge.disableLoopProtectTests = !!meta.disableLoopProtectTests; challenge.disableLoopProtectPreview = !!meta.disableLoopProtectPreview; @@ -336,23 +312,20 @@ ${getFullPath('english', filePath)} path.resolve(META_DIR, `${getBlockNameFromPath(filePath)}/meta.json`) ); - await validate(filePath, meta.superBlock); + const isAudited = isAuditedSuperBlock(lang, meta.superBlock, { + showNewCurriculum: process.env.SHOW_NEW_CURRICULUM, + showUpcomingChanges: process.env.SHOW_UPCOMING_CHANGES + }); // If we can use the language, do so. Otherwise, default to english. - const langUsed = - isAuditedSuperBlock(lang, meta.superBlock, { - showNewCurriculum: process.env.SHOW_NEW_CURRICULUM, - showUpcomingChanges: process.env.SHOW_UPCOMING_CHANGES - }) && fs.existsSync(getFullPath(lang, filePath)) - ? lang - : 'english'; + const langUsed = isAudited && fs.existsSync(i18nPath) ? lang : 'english'; const challenge = translateCommentsInChallenge( - await parseMD(getFullPath(langUsed, filePath)), + await parseMD(langUsed === 'english' ? englishPath : i18nPath), langUsed, COMMENT_TRANSLATIONS ); - + challenge.translationPending = lang !== 'english' && !isAudited; addMetaToChallenge(challenge, meta); fixChallengeProperties(challenge); @@ -373,6 +346,17 @@ function challengeFilesToPolys(files) { }, []); } +async function assertHasEnglishSource(filePath, lang, englishPath) { + const missingEnglish = + lang !== 'english' && !(await hasEnglishSource(CHALLENGES_DIR, filePath)); + if (missingEnglish) + throw Error(`Missing English challenge for +${filePath} +It should be in +${englishPath} +`); +} + async function hasEnglishSource(basePath, translationPath) { const englishRoot = path.resolve(__dirname, basePath, 'english'); return await access( diff --git a/curriculum/get-challenges.test.js b/curriculum/get-challenges.test.js index 94db24b56f0..9066fdddfbb 100644 --- a/curriculum/get-challenges.test.js +++ b/curriculum/get-challenges.test.js @@ -1,9 +1,9 @@ const path = require('path'); const { - generateChallengeCreator, hasEnglishSource, - createCommentMap + createCommentMap, + getChallengesForLang } = require('./get-challenges'); const EXISTING_CHALLENGE_PATH = 'challenge.md'; @@ -12,25 +12,11 @@ const MISSING_CHALLENGE_PATH = 'no/challenge.md'; const basePath = '__fixtures__'; describe('create non-English challenge', () => { - describe('generateChallengeCreator', () => { - describe('createChallenge', () => { - it('throws if lang is an invalid language', async () => { - const createChallenge = generateChallengeCreator(basePath, 'notlang'); - await expect( - createChallenge(EXISTING_CHALLENGE_PATH, {}) - ).rejects.toThrow('notlang is not a accepted language'); - }); - it('throws an error if the source challenge is missing', async () => { - const createChallenge = generateChallengeCreator(basePath, 'chinese'); - await expect( - createChallenge(MISSING_CHALLENGE_PATH, {}) - ).rejects.toThrow( - `Missing English challenge for -${MISSING_CHALLENGE_PATH} -It should be in -` - ); - }); + describe('getChallengesForLang', () => { + it('throws if lang is an invalid language', async () => { + await expect(() => getChallengesForLang('notlang')).rejects.toThrow( + 'notlang is not a accepted language' + ); }); }); describe('hasEnglishSource', () => {