refactor: remove confusing abstraction

This commit is contained in:
Oliver Eyton-Williams
2020-12-17 15:02:56 +01:00
committed by Mrugesh Mohapatra
parent 7c4e0ec41e
commit 24d9c94fe5
3 changed files with 99 additions and 103 deletions

View File

@ -2,7 +2,7 @@ const _ = require('lodash');
const { const {
getChallengesForLang, getChallengesForLang,
createChallengeCreator, createChallenge,
challengesDir, challengesDir,
getChallengesDirForLang getChallengesDirForLang
} = require('../../curriculum/getChallenges'); } = require('../../curriculum/getChallenges');
@ -10,11 +10,9 @@ const { curriculumLocale } = require('../config/env.json');
exports.localeChallengesRootDir = getChallengesDirForLang(curriculumLocale); exports.localeChallengesRootDir = getChallengesDirForLang(curriculumLocale);
const createChallenge = createChallengeCreator(challengesDir, curriculumLocale);
exports.replaceChallengeNode = () => { exports.replaceChallengeNode = () => {
return async function replaceChallengeNode(filePath) { return async function replaceChallengeNode(filePath) {
return await createChallenge(filePath); return await createChallenge(challengesDir, filePath, curriculumLocale);
}; };
}; };

View File

@ -191,9 +191,8 @@ async function buildSuperBlocks({ path, fullPath }, curriculum) {
return walk(fullPath, curriculum, { depth: 1, type: 'directories' }, cb); return walk(fullPath, curriculum, { depth: 1, type: 'directories' }, cb);
} }
async function buildChallenges({ path, filePath }, curriculum, lang) { async function buildChallenges({ path }, curriculum, lang) {
// path is relative to getChallengesDirForLang(lang) // path is relative to getChallengesDirForLang(lang)
const createChallenge = createChallengeCreator(challengesDir, lang);
const block = getBlockNameFromPath(path); const block = getBlockNameFromPath(path);
const { name: superBlock } = superBlockInfoFromPath(path); const { name: superBlock } = superBlockInfoFromPath(path);
let challengeBlock; let challengeBlock;
@ -213,7 +212,7 @@ async function buildChallenges({ path, filePath }, curriculum, lang) {
} }
const { meta } = challengeBlock; const { meta } = challengeBlock;
const challenge = await createChallenge(filePath, meta); const challenge = await createChallenge(challengesDir, path, lang, meta);
challengeBlock.challenges = [...challengeBlock.challenges, challenge]; challengeBlock.challenges = [...challengeBlock.challenges, challenge];
} }
@ -228,83 +227,80 @@ async function parseTranslation(transPath, dict, lang, parse = parseMD) {
: translatedChal; : translatedChal;
} }
function createChallengeCreator(basePath, lang) { async function createChallenge(basePath, filePath, lang, maybeMeta) {
const hasEnglishSource = hasEnglishSourceCreator(basePath); function getFullPath(pathLang) {
return async function createChallenge(filePath, maybeMeta) { return path.resolve(__dirname, basePath, pathLang, filePath);
function getFullPath(pathLang) { }
return path.resolve(__dirname, basePath, pathLang, filePath); let meta;
} if (maybeMeta) {
let meta; meta = maybeMeta;
if (maybeMeta) { } else {
meta = maybeMeta; const metaPath = path.resolve(
} else { metaDir,
const metaPath = path.resolve( `./${getBlockNameFromPath(filePath)}/meta.json`
metaDir, );
`./${getBlockNameFromPath(filePath)}/meta.json` meta = require(metaPath);
); }
meta = require(metaPath); const { name: superBlock } = superBlockInfoFromPath(filePath);
} if (!curriculumLangs.includes(lang))
const { name: superBlock } = superBlockInfoFromPath(filePath); throw Error(`${lang} is not a accepted language.
if (!curriculumLangs.includes(lang))
throw Error(`${lang} is not a accepted language.
Trying to parse ${filePath}`); Trying to parse ${filePath}`);
if (lang !== 'english' && !(await hasEnglishSource(filePath))) if (lang !== 'english' && !(await hasEnglishSource(basePath, filePath)))
throw Error(`Missing English challenge for throw Error(`Missing English challenge for
${filePath} ${filePath}
It should be in It should be in
${getFullPath('english')} ${getFullPath('english')}
`); `);
// assumes superblock names are unique // assumes superblock names are unique
// while the auditing is ongoing, we default to English for un-audited certs // while the auditing is ongoing, we default to English for un-audited certs
// once that's complete, we can revert to using isEnglishChallenge(fullPath) // once that's complete, we can revert to using isEnglishChallenge(fullPath)
const useEnglish = lang === 'english' || !isAuditedCert(lang, superBlock); const useEnglish = lang === 'english' || !isAuditedCert(lang, superBlock);
const isCert = path.extname(filePath) === '.markdown'; const isCert = path.extname(filePath) === '.markdown';
let challenge; let challenge;
if (isCert) { if (isCert) {
// TODO: this uses the old parser to handle certifcates, but Markdown is a // TODO: this uses the old parser to handle certifcates, but Markdown is a
// clunky way to store data, consider converting to YAML and removing the // clunky way to store data, consider converting to YAML and removing the
// old parser. // old parser.
challenge = await (useEnglish challenge = await (useEnglish
? parseMarkdown(getFullPath('english')) ? parseMarkdown(getFullPath('english'))
: parseTranslation( : parseTranslation(
getFullPath(lang), getFullPath(lang),
COMMENT_TRANSLATIONS, COMMENT_TRANSLATIONS,
lang, lang,
parseMarkdown parseMarkdown
)); ));
} else { } else {
challenge = await (useEnglish challenge = await (useEnglish
? parseMD(getFullPath('english')) ? parseMD(getFullPath('english'))
: parseTranslation(getFullPath(lang), COMMENT_TRANSLATIONS, lang)); : parseTranslation(getFullPath(lang), COMMENT_TRANSLATIONS, lang));
} }
const challengeOrder = findIndex( const challengeOrder = findIndex(
meta.challengeOrder, meta.challengeOrder,
([id]) => id === challenge.id ([id]) => id === challenge.id
); );
const { const {
name: blockName, name: blockName,
order, order,
superOrder, superOrder,
isPrivate, isPrivate,
required = [], required = [],
template, template,
time time
} = meta; } = meta;
challenge.block = blockName; challenge.block = blockName;
challenge.order = order; challenge.order = order;
challenge.superOrder = superOrder; challenge.superOrder = superOrder;
challenge.superBlock = superBlock; challenge.superBlock = superBlock;
challenge.challengeOrder = challengeOrder; challenge.challengeOrder = challengeOrder;
challenge.isPrivate = challenge.isPrivate || isPrivate; challenge.isPrivate = challenge.isPrivate || isPrivate;
challenge.required = required.concat(challenge.required || []); challenge.required = required.concat(challenge.required || []);
challenge.template = template; challenge.template = template;
challenge.time = time; challenge.time = time;
challenge.helpCategory = challenge.helpCategory =
challenge.helpCategory || helpCategoryMap[dasherize(blockName)]; challenge.helpCategory || helpCategoryMap[dasherize(blockName)];
return prepareChallenge(challenge); return prepareChallenge(challenge);
};
} }
// TODO: tests and more descriptive name. // TODO: tests and more descriptive name.
@ -351,16 +347,14 @@ function prepareChallenge(challenge) {
return challenge; return challenge;
} }
function hasEnglishSourceCreator(basePath) { async function hasEnglishSource(basePath, translationPath) {
const englishRoot = path.resolve(__dirname, basePath, 'english'); const englishRoot = path.resolve(__dirname, basePath, 'english');
return async function(translationPath) { return await access(
return await access( path.join(englishRoot, translationPath),
path.join(englishRoot, translationPath), fs.constants.F_OK
fs.constants.F_OK )
) .then(() => true)
.then(() => true) .catch(() => false);
.catch(() => false);
};
} }
function superBlockInfoFromPath(filePath) { function superBlockInfoFromPath(filePath) {
@ -390,6 +384,6 @@ function arrToString(arr) {
return Array.isArray(arr) ? arr.join('\n') : toString(arr); return Array.isArray(arr) ? arr.join('\n') : toString(arr);
} }
exports.hasEnglishSourceCreator = hasEnglishSourceCreator; exports.hasEnglishSource = hasEnglishSource;
exports.parseTranslation = parseTranslation; exports.parseTranslation = parseTranslation;
exports.createChallengeCreator = createChallengeCreator; exports.createChallenge = createChallenge;

View File

@ -1,30 +1,28 @@
/* global expect beforeAll */ /* global expect */
const path = require('path'); const path = require('path');
const { const {
createChallengeCreator, createChallenge,
hasEnglishSourceCreator, hasEnglishSource,
createCommentMap createCommentMap
} = require('./getChallenges'); } = require('./getChallenges');
const EXISTING_CHALLENGE_PATH = 'challenge.md'; const EXISTING_CHALLENGE_PATH = 'challenge.md';
const MISSING_CHALLENGE_PATH = 'no/challenge.md'; const MISSING_CHALLENGE_PATH = 'no/challenge.md';
let hasEnglishSource;
let createChallenge;
const basePath = '__fixtures__'; const basePath = '__fixtures__';
describe('create non-English challenge', () => { describe('create non-English challenge', () => {
describe('createChallenge', () => { describe('createChallenge', () => {
it('throws if lang is an invalid language', async () => { it('throws if lang is an invalid language', async () => {
createChallenge = createChallengeCreator(basePath, 'notlang');
await expect( await expect(
createChallenge(EXISTING_CHALLENGE_PATH, {}) createChallenge(basePath, EXISTING_CHALLENGE_PATH, 'notlang', {})
).rejects.toThrow('notlang is not a accepted language'); ).rejects.toThrow('notlang is not a accepted language');
}); });
it('throws an error if the source challenge is missing', async () => { it('throws an error if the source challenge is missing', async () => {
createChallenge = createChallengeCreator(basePath, 'chinese'); await expect(
await expect(createChallenge(MISSING_CHALLENGE_PATH, {})).rejects.toThrow( createChallenge(basePath, MISSING_CHALLENGE_PATH, 'chinese', {})
).rejects.toThrow(
`Missing English challenge for `Missing English challenge for
${MISSING_CHALLENGE_PATH} ${MISSING_CHALLENGE_PATH}
It should be in It should be in
@ -33,19 +31,25 @@ It should be in
}); });
}); });
describe('hasEnglishSource', () => { describe('hasEnglishSource', () => {
beforeAll(() => {
hasEnglishSource = hasEnglishSourceCreator(basePath);
});
it('returns a boolean', async () => { it('returns a boolean', async () => {
const sourceExists = await hasEnglishSource(EXISTING_CHALLENGE_PATH); const sourceExists = await hasEnglishSource(
basePath,
EXISTING_CHALLENGE_PATH
);
expect(typeof sourceExists).toBe('boolean'); expect(typeof sourceExists).toBe('boolean');
}); });
it('returns true if the English challenge exists', async () => { it('returns true if the English challenge exists', async () => {
const sourceExists = await hasEnglishSource(EXISTING_CHALLENGE_PATH); const sourceExists = await hasEnglishSource(
basePath,
EXISTING_CHALLENGE_PATH
);
expect(sourceExists).toBe(true); expect(sourceExists).toBe(true);
}); });
it('returns false if the English challenge is missing', async () => { it('returns false if the English challenge is missing', async () => {
const sourceExists = await hasEnglishSource(MISSING_CHALLENGE_PATH); const sourceExists = await hasEnglishSource(
basePath,
MISSING_CHALLENGE_PATH
);
expect(sourceExists).toBe(false); expect(sourceExists).toBe(false);
}); });
}); });