fix: fallback to english challenges (#45635)

* fix: fallback to english challenges

All challenges will use the english version if a translated file is not
available.  SHOW_NEW_CURRICULUM still gates what's shown in the client.

* refactor: use closures to simplify createChallenge

* refactor: remove messy destructure

* refactor: add meta via helper

* fix: fallback to [] for meta.required

* fix: repair challenge.block

* refactor: use CONST_CASE for meta + challenge dirs

* fix: catch empty superblocks immediately

* fix: clean up path.resolves

* fix: invalid syntax in JS project steps

* fix: default to english comments and relax tests

Instead of always throwing errors when a comment is not translated, the
tests now warn while SHOW_UPCOMING_CHANGES is true, so that tests will
pass while we're developing and allow translators time to work.

They still throw when SHOW_UPCOMING_CHANGES is false to catch issues
in production

* test: update createCommentMap test

* refactor: delete stale comment

* refactor: clarify validate with explanatory consts

* feat: throw if audited cert falls back to english

* fix: stop testing upcoming localized curriculum
This commit is contained in:
Oliver Eyton-Williams
2022-04-15 16:17:49 +02:00
committed by GitHub
parent e0a5fcdb8e
commit 4cc20172c5
10 changed files with 171 additions and 134 deletions

View File

@ -141,7 +141,6 @@ jobs:
- name: Set Environment variables - name: Set Environment variables
run: | run: |
cp sample.env .env cp sample.env .env
echo 'SHOW_UPCOMING_CHANGES=true' >> .env
cat .env cat .env
- name: Install Dependencies - name: Install Dependencies

View File

@ -5,8 +5,9 @@ const _ = require('lodash');
const envData = require('../../config/env.json'); const envData = require('../../config/env.json');
const { const {
getChallengesForLang, getChallengesForLang,
createChallenge, generateChallengeCreator,
challengesDir, CHALLENGES_DIR,
META_DIR,
getChallengesDirForLang getChallengesDirForLang
} = require('../../curriculum/getChallenges'); } = require('../../curriculum/getChallenges');
@ -20,22 +21,19 @@ exports.replaceChallengeNode = () => {
const blockNameRe = /\d\d-[-\w]+\/([^/]+)\//; const blockNameRe = /\d\d-[-\w]+\/([^/]+)\//;
const posix = path.normalize(filePath).split(path.sep).join(path.posix.sep); const posix = path.normalize(filePath).split(path.sep).join(path.posix.sep);
const blockName = posix.match(blockNameRe)[1]; const blockName = posix.match(blockNameRe)[1];
const metaPath = path.resolve( const metaPath = path.resolve(META_DIR, `/${blockName}/meta.json`);
__dirname,
`../../curriculum/challenges/_meta/${blockName}/meta.json`
);
delete require.cache[require.resolve(metaPath)]; delete require.cache[require.resolve(metaPath)];
const meta = require(metaPath); const meta = require(metaPath);
// TODO: reimplement hot-reloading of certifications // TODO: reimplement hot-reloading of certifications
return await createChallenge( return await createChallenge(filePath, meta);
challengesDir,
filePath,
curriculumLocale,
meta
);
}; };
}; };
const createChallenge = generateChallengeCreator(
CHALLENGES_DIR,
curriculumLocale
);
exports.buildChallenges = async function buildChallenges() { exports.buildChallenges = async function buildChallenges() {
const curriculum = await getChallengesForLang(curriculumLocale); const curriculum = await getChallengesForLang(curriculumLocale);
const superBlocks = Object.keys(curriculum); const superBlocks = Object.keys(curriculum);

View File

@ -199,7 +199,7 @@ const locations = [
"button text": ["REPLAY?", "REPLAY?", "REPLAY?"], "button text": ["REPLAY?", "REPLAY?", "REPLAY?"],
"button functions": [restart, restart, restart], "button functions": [restart, restart, restart],
text: "You die. ☠️" text: "You die. ☠️"
} },
{ {
name: "win", name: "win",
"button text": ["Fight slime", "Fight fanged beast", "Go to town square"], "button text": ["Fight slime", "Fight fanged beast", "Go to town square"],

View File

@ -111,7 +111,7 @@ const charRange = (start, end) =>
const evalFormula = x => { const evalFormula = x => {
const rangeRegex = /([A-J])([1-9][0-9]?):([A-J])([1-9][0-9]?)/gi; const rangeRegex = /([A-J])([1-9][0-9]?):([A-J])([1-9][0-9]?)/gi;
const rangeFromString = (n1, n2) => range(parseInt(n1), parseInt(n2)); const rangeFromString = (n1, n2) => range(parseInt(n1), parseInt(n2));
const elemValue = n => (c => document.getElementById(c + n).value)); const elemValue = n => c => document.getElementById(c + n).value;
const fn = elemValue("1"); const fn = elemValue("1");
return fn("A"); return fn("A");
}; };

View File

@ -90,7 +90,7 @@ const spreadsheetFunctions = {
lasttwo: arr => arr.slice(-2), lasttwo: arr => arr.slice(-2),
even: nums => nums.filter(isEven), even: nums => nums.filter(isEven),
sum: nums => nums.reduce((a, x) => a + x), sum: nums => nums.reduce((a, x) => a + x),
has2: arr => arr.includes(2) has2: arr => arr.includes(2),
nodups: arr => arr.reduce((a, x) => a.includes(x), []) nodups: arr => arr.reduce((a, x) => a.includes(x), [])
}; };

View File

@ -106,5 +106,6 @@
"es69h6": "When you join two windows into one window", "es69h6": "When you join two windows into one window",
"fho5t5": "When you open a new tab at the end", "fho5t5": "When you open a new tab at the end",
"00kcrm": "yields true", "00kcrm": "yields true",
"sxpg2a": "Your mailbox, drive, and other work sites" "sxpg2a": "Your mailbox, drive, and other work sites",
"4143lf": "initialize buttons"
} }

View File

@ -1,8 +1,9 @@
const fs = require('fs'); const fs = require('fs');
const path = require('path'); const path = require('path');
const util = require('util'); const util = require('util');
const assert = require('assert');
const yaml = require('js-yaml'); const yaml = require('js-yaml');
const { findIndex } = require('lodash'); const { findIndex, isEmpty } = require('lodash');
const readDirP = require('readdirp'); const readDirP = require('readdirp');
const { helpCategoryMap } = require('../client/utils/challenge-types'); const { helpCategoryMap } = require('../client/utils/challenge-types');
const { showUpcomingChanges } = require('../config/env.json'); const { showUpcomingChanges } = require('../config/env.json');
@ -22,13 +23,13 @@ const { getSuperOrder, getSuperBlockFromDir } = require('./utils');
const access = util.promisify(fs.access); const access = util.promisify(fs.access);
const challengesDir = path.resolve(__dirname, './challenges'); const CHALLENGES_DIR = path.resolve(__dirname, 'challenges');
const metaDir = path.resolve(challengesDir, '_meta'); const META_DIR = path.resolve(CHALLENGES_DIR, '_meta');
exports.challengesDir = challengesDir; exports.CHALLENGES_DIR = CHALLENGES_DIR;
exports.metaDir = metaDir; exports.META_DIR = META_DIR;
const COMMENT_TRANSLATIONS = createCommentMap( const COMMENT_TRANSLATIONS = createCommentMap(
path.resolve(__dirname, './dictionaries') path.resolve(__dirname, 'dictionaries')
); );
function getTranslatableComments(dictionariesDir) { function getTranslatableComments(dictionariesDir) {
@ -109,20 +110,19 @@ function getTranslationEntry(dicts, { engId, text }) {
if (entry) { if (entry) {
return { ...acc, [lang]: entry }; return { ...acc, [lang]: entry };
} else { } else {
throw Error(`Missing translation for comment // default to english
'${text}' return { ...acc, [lang]: text };
with id of ${engId}`);
} }
}, {}); }, {});
} }
function getChallengesDirForLang(lang) { function getChallengesDirForLang(lang) {
return path.resolve(challengesDir, `./${lang}`); return path.resolve(CHALLENGES_DIR, `${lang}`);
} }
function getMetaForBlock(block) { function getMetaForBlock(block) {
return JSON.parse( return JSON.parse(
fs.readFileSync(path.resolve(metaDir, `./${block}/meta.json`), 'utf8') fs.readFileSync(path.resolve(META_DIR, `${block}/meta.json`), 'utf8')
); );
} }
@ -153,7 +153,9 @@ const walk = (root, target, options, cb) => {
}; };
exports.getChallengesForLang = async function getChallengesForLang(lang) { exports.getChallengesForLang = async function getChallengesForLang(lang) {
const root = getChallengesDirForLang(lang); // english determines the shape of the curriculum, all other languages mirror
// it.
const root = getChallengesDirForLang('english');
// scaffold the curriculum, first set up the superblocks, then recurse into // scaffold the curriculum, first set up the superblocks, then recurse into
// the blocks // the blocks
const curriculum = await walk( const curriculum = await walk(
@ -162,6 +164,9 @@ exports.getChallengesForLang = async function getChallengesForLang(lang) {
{ type: 'directories', depth: 0 }, { type: 'directories', depth: 0 },
buildSuperBlocks buildSuperBlocks
); );
Object.entries(curriculum).forEach(([name, superBlock]) => {
assert(!isEmpty(superBlock.blocks), `superblock ${name} has no blocks`);
});
const cb = (file, curriculum) => buildChallenges(file, curriculum, lang); const cb = (file, curriculum) => buildChallenges(file, curriculum, lang);
// fill the scaffold with the challenges // fill the scaffold with the challenges
return walk( return walk(
@ -173,10 +178,7 @@ exports.getChallengesForLang = async function getChallengesForLang(lang) {
}; };
async function buildBlocks({ basename: blockName }, curriculum, superBlock) { async function buildBlocks({ basename: blockName }, curriculum, superBlock) {
const metaPath = path.resolve( const metaPath = path.resolve(META_DIR, `${blockName}/meta.json`);
__dirname,
`./challenges/_meta/${blockName}/meta.json`
);
if (fs.existsSync(metaPath)) { if (fs.existsSync(metaPath)) {
// try to read the file, if the meta path does not exist it should be a certification. // try to read the file, if the meta path does not exist it should be a certification.
@ -240,9 +242,10 @@ async function buildChallenges({ path: filePath }, curriculum, lang) {
) { ) {
return; return;
} }
const createChallenge = generateChallengeCreator(CHALLENGES_DIR, lang);
const challenge = isCert const challenge = isCert
? await createCertification(challengesDir, filePath, lang) ? await createCertification(CHALLENGES_DIR, filePath, lang)
: await createChallenge(challengesDir, filePath, lang, meta); : await createChallenge(filePath, meta);
challengeBlock.challenges = [...challengeBlock.challenges, challenge]; challengeBlock.challenges = [...challengeBlock.challenges, challenge];
} }
@ -258,8 +261,7 @@ async function parseTranslation(transPath, dict, lang, parse = parseMD) {
: translatedChal; : translatedChal;
} }
// eslint-disable-next-line no-unused-vars async function createCertification(basePath, filePath) {
async function createCertification(basePath, filePath, lang) {
function getFullPath(pathLang) { function getFullPath(pathLang) {
return path.resolve(__dirname, basePath, pathLang, filePath); return path.resolve(__dirname, basePath, pathLang, filePath);
} }
@ -270,90 +272,111 @@ async function createCertification(basePath, filePath, lang) {
return parseCert(getFullPath('english')); return parseCert(getFullPath('english'));
} }
async function createChallenge(basePath, filePath, lang, maybeMeta) { // This is a slightly weird abstraction, but it lets us define helper functions
function getFullPath(pathLang) { // without passing around a ton of arguments.
function generateChallengeCreator(basePath, lang) {
function getFullPath(pathLang, filePath) {
return path.resolve(__dirname, basePath, pathLang, filePath); return path.resolve(__dirname, basePath, pathLang, filePath);
} }
let meta;
if (maybeMeta) { async function validate(filePath, superBlock) {
meta = maybeMeta; const invalidLang = !curriculumLangs.includes(lang);
} else { if (invalidLang)
const metaPath = path.resolve( throw Error(`${lang} is not a accepted language.
metaDir, Trying to parse ${filePath}`);
`./${getBlockNameFromPath(filePath)}/meta.json`
); const missingEnglish =
meta = require(metaPath); lang !== 'english' && !(await hasEnglishSource(basePath, filePath));
} if (missingEnglish)
const { superBlock } = meta; throw Error(`Missing English challenge for
if (!curriculumLangs.includes(lang))
throw Error(`${lang} is not a accepted language.
Trying to parse ${filePath}`);
if (lang !== 'english' && !(await hasEnglishSource(basePath, filePath)))
throw Error(`Missing English challenge for
${filePath} ${filePath}
It should be in It should be in
${getFullPath('english')} ${getFullPath('english', filePath)}
`); `);
// assumes superblock names are unique
// while the auditing is ongoing, we default to English for un-audited certs
// once that's complete, we can revert to using isEnglishChallenge(fullPath)
const useEnglish = lang === 'english' || !isAuditedCert(lang, superBlock);
const challenge = await (useEnglish const missingAuditedChallenge =
? parseMD(getFullPath('english')) isAuditedCert(lang, superBlock) &&
: parseTranslation(getFullPath(lang), COMMENT_TRANSLATIONS, lang)); !fs.existsSync(getFullPath(lang, filePath));
if (missingAuditedChallenge)
throw Error(`Missing ${lang} audited challenge for
${filePath}
No audited challenges should fallback to English.
`);
}
const challengeOrder = findIndex( function addMetaToChallenge(challenge, meta) {
meta.challengeOrder, const challengeOrder = findIndex(
([id]) => id === challenge.id meta.challengeOrder,
); ([id]) => id === challenge.id
const { );
name: blockName,
hasEditableBoundaries, challenge.block = meta.name ? dasherize(meta.name) : null;
order, challenge.hasEditableBoundaries = !!meta.hasEditableBoundaries;
isPrivate, challenge.order = meta.order;
required = [], const superOrder = getSuperOrder(meta.superBlock, {
template, showNewCurriculum: process.env.SHOW_NEW_CURRICULUM === 'true'
time, });
usesMultifileEditor if (superOrder !== null) challenge.superOrder = superOrder;
} = meta; /* Since there can be more than one way to complete a certification (using the
challenge.block = dasherize(blockName);
challenge.hasEditableBoundaries = !!hasEditableBoundaries;
challenge.order = order;
const superOrder = getSuperOrder(superBlock, {
showNewCurriculum: process.env.SHOW_NEW_CURRICULUM === 'true'
});
if (superOrder !== null) challenge.superOrder = superOrder;
/* Since there can be more than one way to complete a certification (using the
legacy curriculum or the new one, for instance), we need a certification legacy curriculum or the new one, for instance), we need a certification
field to track which certification this belongs to. */ field to track which certification this belongs to. */
// TODO: generalize this to all superBlocks // TODO: generalize this to all superBlocks
challenge.certification = challenge.certification =
superBlock === '2022/responsive-web-design' meta.superBlock === '2022/responsive-web-design'
? 'responsive-web-design' ? 'responsive-web-design'
: superBlock; : meta.superBlock;
challenge.superBlock = superBlock; challenge.superBlock = meta.superBlock;
challenge.challengeOrder = challengeOrder; challenge.challengeOrder = challengeOrder;
challenge.isPrivate = challenge.isPrivate || isPrivate; challenge.isPrivate = challenge.isPrivate || meta.isPrivate;
challenge.required = required.concat(challenge.required || []); challenge.required = (meta.required || []).concat(challenge.required || []);
challenge.template = template; challenge.template = meta.template;
challenge.time = time; challenge.time = meta.time;
challenge.helpCategory = challenge.helpCategory =
challenge.helpCategory || helpCategoryMap[challenge.block]; challenge.helpCategory || helpCategoryMap[challenge.block];
challenge.translationPending = challenge.translationPending =
lang !== 'english' && !isAuditedCert(lang, superBlock); lang !== 'english' && !isAuditedCert(lang, meta.superBlock);
challenge.usesMultifileEditor = !!usesMultifileEditor; challenge.usesMultifileEditor = !!meta.usesMultifileEditor;
if (challenge.challengeFiles) { if (challenge.challengeFiles) {
// The client expects the challengeFiles to be an array of polyvinyls // The client expects the challengeFiles to be an array of polyvinyls
challenge.challengeFiles = challengeFilesToPolys(challenge.challengeFiles); challenge.challengeFiles = challengeFilesToPolys(
} challenge.challengeFiles
if (challenge.solutions?.length) { );
// The test runner needs the solutions to be arrays of polyvinyls so it }
// can sort them correctly. if (challenge.solutions?.length) {
challenge.solutions = challenge.solutions.map(challengeFilesToPolys); // The test runner needs the solutions to be arrays of polyvinyls so it
// can sort them correctly.
challenge.solutions = challenge.solutions.map(challengeFilesToPolys);
}
} }
return challenge; async function createChallenge(filePath, maybeMeta) {
const meta = maybeMeta
? maybeMeta
: require(path.resolve(
META_DIR,
`${getBlockNameFromPath(filePath)}/meta.json`
));
await validate(filePath, meta.superBlock);
const useEnglish =
lang === 'english' ||
!isAuditedCert(lang, meta.superBlock) ||
!fs.existsSync(getFullPath(lang, filePath));
const challenge = await (useEnglish
? parseMD(getFullPath('english', filePath))
: parseTranslation(
getFullPath(lang, filePath),
COMMENT_TRANSLATIONS,
lang
));
addMetaToChallenge(challenge, meta);
return challenge;
}
return createChallenge;
} }
function challengeFilesToPolys(files) { function challengeFilesToPolys(files) {
@ -390,4 +413,4 @@ function getBlockNameFromPath(filePath) {
exports.hasEnglishSource = hasEnglishSource; exports.hasEnglishSource = hasEnglishSource;
exports.parseTranslation = parseTranslation; exports.parseTranslation = parseTranslation;
exports.createChallenge = createChallenge; exports.generateChallengeCreator = generateChallengeCreator;

View File

@ -1,7 +1,7 @@
const path = require('path'); const path = require('path');
const { const {
createChallenge, generateChallengeCreator,
hasEnglishSource, hasEnglishSource,
createCommentMap createCommentMap
} = require('./getChallenges'); } = require('./getChallenges');
@ -12,21 +12,25 @@ const MISSING_CHALLENGE_PATH = 'no/challenge.md';
const basePath = '__fixtures__'; const basePath = '__fixtures__';
describe('create non-English challenge', () => { describe('create non-English challenge', () => {
describe('createChallenge', () => { describe('generateChallengeCreator', () => {
it('throws if lang is an invalid language', async () => { describe('createChallenge', () => {
await expect( it('throws if lang is an invalid language', async () => {
createChallenge(basePath, EXISTING_CHALLENGE_PATH, 'notlang', {}) const createChallenge = generateChallengeCreator(basePath, 'notlang');
).rejects.toThrow('notlang is not a accepted language'); await expect(
}); createChallenge(EXISTING_CHALLENGE_PATH, {})
it('throws an error if the source challenge is missing', async () => { ).rejects.toThrow('notlang is not a accepted language');
await expect( });
createChallenge(basePath, MISSING_CHALLENGE_PATH, 'chinese', {}) it('throws an error if the source challenge is missing', async () => {
).rejects.toThrow( const createChallenge = generateChallengeCreator(basePath, 'chinese');
`Missing English challenge for await expect(
createChallenge(MISSING_CHALLENGE_PATH, {})
).rejects.toThrow(
`Missing English challenge for
${MISSING_CHALLENGE_PATH} ${MISSING_CHALLENGE_PATH}
It should be in It should be in
` `
); );
});
}); });
}); });
describe('hasEnglishSource', () => { describe('hasEnglishSource', () => {
@ -69,9 +73,15 @@ It should be in
expect(typeof createCommentMap(dictionaryDir)).toBe('object'); expect(typeof createCommentMap(dictionaryDir)).toBe('object');
}); });
it('throws if an entry is missing', () => { it('fallback to the untranslated string', () => {
expect.assertions(1); expect.assertions(2);
expect(() => createCommentMap(incompleteDictDir)).toThrow(); const commentMap = createCommentMap(incompleteDictDir);
expect(commentMap['To be translated one'].spanish).toEqual(
'Spanish translation one'
);
expect(commentMap['To be translated two'].spanish).toEqual(
'To be translated two'
);
}); });
it('returns an object with an expected form', () => { it('returns an object with an expected form', () => {

View File

@ -23,7 +23,7 @@ const fileJoi = Joi.object().keys({
const schema = Joi.object() const schema = Joi.object()
.keys({ .keys({
block: Joi.string().regex(slugRE), block: Joi.string().regex(slugRE).required(),
blockId: Joi.objectId(), blockId: Joi.objectId(),
challengeOrder: Joi.number(), challengeOrder: Joi.number(),
removeComments: Joi.bool(), removeComments: Joi.bool(),

View File

@ -317,13 +317,13 @@ function populateTestsForLang({ lang, challenges, meta }) {
// Note: the title in meta.json are purely for human readability and // Note: the title in meta.json are purely for human readability and
// do not include translations, so we do not validate against them. // do not include translations, so we do not validate against them.
it('Matches an ID in meta.json', function () { it('Matches an ID in meta.json', function () {
const index = meta[dashedBlockName].challengeOrder.findIndex( const index = meta[dashedBlockName]?.challengeOrder?.findIndex(
arr => arr[0] === challenge.id arr => arr[0] === challenge.id
); );
if (index < 0) { if (index < 0) {
throw new AssertionError( throw new AssertionError(
`Cannot find ID "${challenge.id}" in meta.json file` `Cannot find ID "${challenge.id}" in meta.json file for block "${dashedBlockName}"`
); );
} }
}); });
@ -370,11 +370,14 @@ function populateTestsForLang({ lang, challenges, meta }) {
// currently have the text of a comment elsewhere. If that happens // currently have the text of a comment elsewhere. If that happens
// we can handle that challenge separately. // we can handle that challenge separately.
TRANSLATABLE_COMMENTS.forEach(comment => { TRANSLATABLE_COMMENTS.forEach(comment => {
const errorText = `English comment '${comment}' should be replaced with its translation`;
challenge.challengeFiles.forEach(challengeFile => { challenge.challengeFiles.forEach(challengeFile => {
if (challengeFile.contents.includes(comment)) if (challengeFile.contents.includes(comment))
throw Error( if (process.env.SHOW_UPCOMING_CHANGES == 'true') {
`English comment '${comment}' should be replaced with its translation` console.warn(errorText);
); } else {
throw Error(errorText);
}
}); });
}); });
@ -414,7 +417,10 @@ function populateTestsForLang({ lang, challenges, meta }) {
if (isEmpty(challenge.__commentCounts) && isEmpty(commentMap)) if (isEmpty(challenge.__commentCounts) && isEmpty(commentMap))
return; return;
if (!isEqual(commentMap, challenge.__commentCounts)) if (
process.env.SHOW_NEW_CURRICULUM !== 'true' &&
!isEqual(commentMap, challenge.__commentCounts)
)
throw Error(`Mismatch in ${challenge.title}. Replaced comments: throw Error(`Mismatch in ${challenge.title}. Replaced comments:
${inspect(challenge.__commentCounts)} ${inspect(challenge.__commentCounts)}
Comments in translated text: Comments in translated text: