From 1b895e7809a6ab964a56099aa51db6bda01c4722 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Thu, 1 Oct 2020 15:50:43 +0200 Subject: [PATCH] fix: translate title and dashName correctly --- .../chinese/challenge-stripped.md | 2 +- curriculum/__fixtures__/chinese/challenge.md | 3 +- .../__fixtures__/combined-html-comments.md | 4 +- .../__fixtures__/combined-js-comments.md | 4 +- .../__fixtures__/combined-jsx-comments.md | 4 +- curriculum/__fixtures__/combined.md | 4 +- curriculum/getChallenges.js | 6 +- curriculum/schema/challengeSchema.js | 166 ++++++++---------- curriculum/test/test-challenges.js | 16 +- .../__fixtures__/challenge-objects.js | 40 ++++- .../translation-parser/translation-parser.js | 11 +- .../translation-parser.test.js | 23 ++- 12 files changed, 156 insertions(+), 127 deletions(-) diff --git a/curriculum/__fixtures__/chinese/challenge-stripped.md b/curriculum/__fixtures__/chinese/challenge-stripped.md index f22e6497b0..f5629904f7 100644 --- a/curriculum/__fixtures__/chinese/challenge-stripped.md +++ b/curriculum/__fixtures__/chinese/challenge-stripped.md @@ -1,5 +1,5 @@ --- -localeTitle: 向HTML Elements说你好 +title: 向HTML Elements说你好 forumTopicId: 18276 --- diff --git a/curriculum/__fixtures__/chinese/challenge.md b/curriculum/__fixtures__/chinese/challenge.md index be9d42d84d..0e1d19b4fa 100644 --- a/curriculum/__fixtures__/chinese/challenge.md +++ b/curriculum/__fixtures__/chinese/challenge.md @@ -1,10 +1,9 @@ --- id: bd7123c8c441eddfaeb5bdef -title: Say Hello to HTML Elements +title: 向HTML Elements说你好 challengeType: 0 videoUrl: '' forumTopicId: 18276 -localeTitle: 向HTML Elements说你好 --- ## Description diff --git a/curriculum/__fixtures__/combined-html-comments.md b/curriculum/__fixtures__/combined-html-comments.md index 8a19eb8122..ade195b7f7 100644 --- a/curriculum/__fixtures__/combined-html-comments.md +++ b/curriculum/__fixtures__/combined-html-comments.md @@ -1,10 +1,10 @@ --- id: bd7123c8c441eddfaeb5bdef -title: Say Hello to HTML Elements +originalTitle: Say Hello to HTML Elements challengeType: 0 videoUrl: 'https://scrimba.com/p/pVMPUv/cE8Gpt2' forumTopicId: 18276 -localeTitle: 向HTML Elements说你好 +title: 向HTML Elements说你好 --- ## Description diff --git a/curriculum/__fixtures__/combined-js-comments.md b/curriculum/__fixtures__/combined-js-comments.md index 0486a1ae5d..95fb9b1702 100644 --- a/curriculum/__fixtures__/combined-js-comments.md +++ b/curriculum/__fixtures__/combined-js-comments.md @@ -1,10 +1,10 @@ --- id: bd7123c8c441eddfaeb5bdef -title: Say Hello to HTML Elements +originalTitle: Say Hello to HTML Elements challengeType: 0 videoUrl: 'https://scrimba.com/p/pVMPUv/cE8Gpt2' forumTopicId: 18276 -localeTitle: 向HTML Elements说你好 +title: 向HTML Elements说你好 --- ## Description diff --git a/curriculum/__fixtures__/combined-jsx-comments.md b/curriculum/__fixtures__/combined-jsx-comments.md index 71006fb14c..bab0ce2830 100644 --- a/curriculum/__fixtures__/combined-jsx-comments.md +++ b/curriculum/__fixtures__/combined-jsx-comments.md @@ -1,10 +1,10 @@ --- id: bd7123c8c441eddfaeb5bdef -title: Say Hello to HTML Elements +originalTitle: Say Hello to HTML Elements challengeType: 0 videoUrl: 'https://scrimba.com/p/pVMPUv/cE8Gpt2' forumTopicId: 18276 -localeTitle: 向HTML Elements说你好 +title: 向HTML Elements说你好 --- ## Description diff --git a/curriculum/__fixtures__/combined.md b/curriculum/__fixtures__/combined.md index 2dc99ca8fb..4bca7e7312 100644 --- a/curriculum/__fixtures__/combined.md +++ b/curriculum/__fixtures__/combined.md @@ -1,10 +1,10 @@ --- id: bd7123c8c441eddfaeb5bdef -title: Say Hello to HTML Elements +originalTitle: Say Hello to HTML Elements challengeType: 0 videoUrl: 'https://scrimba.com/p/pVMPUv/cE8Gpt2' forumTopicId: 18276 -localeTitle: 向HTML Elements说你好 +title: 向HTML Elements说你好 --- ## Description diff --git a/curriculum/getChallenges.js b/curriculum/getChallenges.js index 0c5d8e76be..254cebbbd2 100644 --- a/curriculum/getChallenges.js +++ b/curriculum/getChallenges.js @@ -180,7 +180,11 @@ ${getFullPath('english')} time } = meta; challenge.block = blockName; - challenge.dashedName = dasherize(challenge.title); + challenge.dashedName = + lang === 'english' + ? dasherize(challenge.title) + : dasherize(challenge.originalTitle); + delete challenge.originalTitle; challenge.order = order; challenge.superOrder = superOrder; challenge.superBlock = superBlock; diff --git a/curriculum/schema/challengeSchema.js b/curriculum/schema/challengeSchema.js index 7d05313c75..787f119923 100644 --- a/curriculum/schema/challengeSchema.js +++ b/curriculum/schema/challengeSchema.js @@ -17,100 +17,90 @@ const fileJoi = Joi.object().keys({ history: [Joi.array().items(Joi.string().allow('')), Joi.string().allow('')] }); -function getSchemaForLang(lang) { - let schema = Joi.object().keys({ - block: Joi.string(), - blockId: Joi.objectId(), - challengeOrder: Joi.number(), - challengeType: Joi.number() - .min(0) - .max(11) +const schema = Joi.object().keys({ + block: Joi.string(), + blockId: Joi.objectId(), + challengeOrder: Joi.number(), + challengeType: Joi.number() + .min(0) + .max(11) + .required(), + checksum: Joi.number(), + dashedName: Joi.string(), + description: Joi.when('challengeType', { + is: Joi.only([challengeTypes.step, challengeTypes.video]), + then: Joi.string().allow(''), + otherwise: Joi.string().required() + }), + fileName: Joi.string(), + files: Joi.object().keys({ + indexcss: fileJoi, + indexhtml: fileJoi, + indexjs: fileJoi, + indexjsx: fileJoi + }), + guideUrl: Joi.string().uri({ scheme: 'https' }), + videoUrl: Joi.string().allow(''), + forumTopicId: Joi.number(), + helpRoom: Joi.string(), + id: Joi.objectId().required(), + instructions: Joi.string().allow(''), + isComingSoon: Joi.bool(), + isLocked: Joi.bool(), + isPrivate: Joi.bool(), + name: Joi.string(), + order: Joi.number(), + // video challenges only: + videoId: Joi.when('challengeType', { + is: challengeTypes.video, + then: Joi.string().required() + }), + question: Joi.object().keys({ + text: Joi.string().required(), + answers: Joi.array() + .items(Joi.string()) .required(), - checksum: Joi.number(), - dashedName: Joi.string(), - description: Joi.when('challengeType', { - is: Joi.only([challengeTypes.step, challengeTypes.video]), - then: Joi.string().allow(''), - otherwise: Joi.string().required() - }), - fileName: Joi.string(), - files: Joi.object().keys({ + solution: Joi.number().required() + }), + required: Joi.array().items( + Joi.object().keys({ + link: Joi.string(), + raw: Joi.bool(), + src: Joi.string(), + crossDomain: Joi.bool() + }) + ), + solutions: Joi.array().items( + Joi.object().keys({ indexcss: fileJoi, indexhtml: fileJoi, indexjs: fileJoi, - indexjsx: fileJoi - }), - guideUrl: Joi.string().uri({ scheme: 'https' }), - videoUrl: Joi.string().allow(''), - forumTopicId: Joi.number(), - helpRoom: Joi.string(), - id: Joi.objectId().required(), - instructions: Joi.string().allow(''), - isComingSoon: Joi.bool(), - isLocked: Joi.bool(), - isPrivate: Joi.bool(), - name: Joi.string(), - order: Joi.number(), - // video challenges only: - videoId: Joi.when('challengeType', { - is: challengeTypes.video, - then: Joi.string().required() - }), - question: Joi.object().keys({ + indexjsx: fileJoi, + indexpy: fileJoi + }) + ), + superBlock: Joi.string(), + superOrder: Joi.number(), + suborder: Joi.number(), + tests: Joi.array().items( + // public challenges + Joi.object().keys({ text: Joi.string().required(), - answers: Joi.array() - .items(Joi.string()) - .required(), - solution: Joi.number().required() + testString: Joi.string() + .allow('') + .required() }), - required: Joi.array().items( - Joi.object().keys({ - link: Joi.string(), - raw: Joi.bool(), - src: Joi.string(), - crossDomain: Joi.bool() - }) - ), - solutions: Joi.array().items( - Joi.object().keys({ - indexcss: fileJoi, - indexhtml: fileJoi, - indexjs: fileJoi, - indexjsx: fileJoi, - indexpy: fileJoi - }) - ), - superBlock: Joi.string(), - superOrder: Joi.number(), - suborder: Joi.number(), - tests: Joi.array().items( - // public challenges - Joi.object().keys({ - text: Joi.string().required(), - testString: Joi.string() - .allow('') - .required() - }), - // our tests used in certification verification - Joi.object().keys({ - id: Joi.string().required(), - title: Joi.string().required() - }) - ), - template: Joi.string().allow(''), - time: Joi.string().allow(''), - title: Joi.string().required() - }); + // our tests used in certification verification + Joi.object().keys({ + id: Joi.string().required(), + title: Joi.string().required() + }) + ), + template: Joi.string().allow(''), + time: Joi.string().allow(''), + title: Joi.string().required() +}); - if (lang !== 'english') { - // TODO: make this required again once all current challenges have it. - schema = schema.append({ - localeTitle: Joi.string().allow('') - }); - } - return schema; -} -exports.challengeSchemaValidator = lang => { - const schema = getSchemaForLang(lang); +exports.challengeSchemaValidator = () => { return challenge => Joi.validate(challenge, schema); }; diff --git a/curriculum/test/test-challenges.js b/curriculum/test/test-challenges.js index 3d613169b8..fa95324ad1 100644 --- a/curriculum/test/test-challenges.js +++ b/curriculum/test/test-challenges.js @@ -245,7 +245,7 @@ function validateBlock(challenge) { function populateTestsForLang({ lang, challenges, meta }) { const mongoIds = new MongoIds(); const challengeTitles = new ChallengeTitles(); - const validateChallenge = challengeSchemaValidator(lang); + const validateChallenge = challengeSchemaValidator(); describe(`Check challenges (${lang})`, function() { this.timeout(5000); @@ -253,18 +253,8 @@ function populateTestsForLang({ lang, challenges, meta }) { const dashedBlockName = dasherize(challenge.block); describe(challenge.block || 'No block', function() { describe(challenge.title || 'No title', function() { - it('Matches a title in meta.json', function() { - const index = meta[dashedBlockName].findIndex( - arr => arr[1] === challenge.title - ); - - if (index < 0) { - throw new AssertionError( - `Cannot find title "${challenge.title}" in meta.json file` - ); - } - }); - + // Note: the title in meta.json are purely for human readability and + // do not include translations, so we do not validate against them. it('Matches an ID in meta.json', function() { const index = meta[dashedBlockName].findIndex( arr => arr[0] === challenge.id diff --git a/tools/challenge-md-parser/translation-parser/__fixtures__/challenge-objects.js b/tools/challenge-md-parser/translation-parser/__fixtures__/challenge-objects.js index fe2583994a..db06f99c43 100644 --- a/tools/challenge-md-parser/translation-parser/__fixtures__/challenge-objects.js +++ b/tools/challenge-md-parser/translation-parser/__fixtures__/challenge-objects.js @@ -122,11 +122,10 @@ const ENGLISH_VIDEO_CHALLENGE = { const TRANSLATED_CERTIFICATE = { id: '561add10cb82ac38a17513bc', - title: 'Responsive Web Design Certificate', + title: '响应式网页设计证书', challengeType: 7, isPrivate: true, videoUrl: '', - localeTitle: '响应式网页设计证书', tests: [ { id: 'bd7158d8c442eddfaeb5bd18', title: 'Build a Tribute Page' }, { id: '587d78af367417b2b2512b03', title: 'Build a Survey Form' }, @@ -151,11 +150,40 @@ const TRANSLATED_CERTIFICATE = { const TRANSLATED_CHALLENGE = { id: 'id', - title: 'Title', + title: 'Translated title', + challengeType: 0, + videoUrl: 'https://scrimba.com/', + forumTopicId: 9876, + tests: [ + { + text: 'Translated test text', + testString: 'Translated assertions, should be ignored' + }, + { + text: 'Translated test text2', + testString: 'Translated assertions, should be ignored2' + } + ], + solutions: ['Translated solution html string, should be ignored'], + description: 'Translated description html string', + instructions: 'Translated instructions html string', + files: [ + { + key: 'indexhtml', + ext: 'html', + name: 'index', + contents: 'Translated seed html string, should be ignored', + head: 'Translated head string, should be ignored', + tail: 'Translated tail string, should be ignored' + } + ] +}; + +const TRANSLATED_CHALLENGE_NO_TITLE = { + id: 'id', challengeType: 0, videoUrl: 'https://scrimba.com/', forumTopicId: 9876, - localeTitle: 'Translated title', tests: [ { text: 'Translated test text', @@ -194,11 +222,10 @@ const TRANSLATED_VIDEO_CHALLENGE = { const WRONG_NUM_TESTS_CHALLENGE = { id: 'id', - title: 'Title', + title: 'Translated title', challengeType: 0, videoUrl: 'https://scrimba.com/', forumTopicId: 12345, - localeTitle: 'Translated title', tests: [ { text: 'Translated test text', @@ -227,5 +254,6 @@ exports.ENGLISH_CHALLENGE_NO_FILES = ENGLISH_CHALLENGE_NO_FILES; exports.ENGLISH_VIDEO_CHALLENGE = ENGLISH_VIDEO_CHALLENGE; exports.TRANSLATED_CERTIFICATE = TRANSLATED_CERTIFICATE; exports.TRANSLATED_CHALLENGE = TRANSLATED_CHALLENGE; +exports.TRANSLATED_CHALLENGE_NO_TITLE = TRANSLATED_CHALLENGE_NO_TITLE; exports.TRANSLATED_VIDEO_CHALLENGE = TRANSLATED_VIDEO_CHALLENGE; exports.WRONG_NUM_TESTS_CHALLENGE = WRONG_NUM_TESTS_CHALLENGE; diff --git a/tools/challenge-md-parser/translation-parser/translation-parser.js b/tools/challenge-md-parser/translation-parser/translation-parser.js index 614f4e0fbf..82bee5d5a8 100644 --- a/tools/challenge-md-parser/translation-parser/translation-parser.js +++ b/tools/challenge-md-parser/translation-parser/translation-parser.js @@ -1,3 +1,4 @@ +const { isEmpty } = require('lodash'); const clone = require('lodash/cloneDeep'); exports.translateComments = (text, lang, dict, codeLang) => { @@ -40,14 +41,16 @@ exports.mergeChallenges = (engChal, transChal) => { ...engChal, description: transChal.description, instructions: transChal.instructions, - localeTitle: transChal.localeTitle, + originalTitle: engChal.title, + // TODO: throw in production? + title: isEmpty(transChal.title) ? engChal.title : transChal.title, forumTopicId: transChal.forumTopicId }; if (!hasTests) throw Error( `Both challenges must have tests or questions. title: ${engChal.title} - localeTitle: ${transChal.localeTitle}` + translated title: ${transChal.title}` ); // TODO: this should break the build when we go to production, but // not for testing. @@ -55,7 +58,7 @@ exports.mergeChallenges = (engChal, transChal) => { console.error( `Challenges in both languages must have the same number of tests. title: ${engChal.title} - localeTitle: ${transChal.localeTitle}` + translated title: ${transChal.title}` ); return challenge; } @@ -63,7 +66,7 @@ exports.mergeChallenges = (engChal, transChal) => { // throw Error( // `Challenges in both languages must have the same number of tests. // title: ${engChal.title} - // localeTitle: ${transChal.localeTitle}` + // translated title: ${transChal.title}` // ); if (transChal.tests) { diff --git a/tools/challenge-md-parser/translation-parser/translation-parser.test.js b/tools/challenge-md-parser/translation-parser/translation-parser.test.js index e17451cab8..46866db76d 100644 --- a/tools/challenge-md-parser/translation-parser/translation-parser.test.js +++ b/tools/challenge-md-parser/translation-parser/translation-parser.test.js @@ -12,6 +12,7 @@ const { ENGLISH_VIDEO_CHALLENGE, TRANSLATED_CERTIFICATE, TRANSLATED_CHALLENGE, + TRANSLATED_CHALLENGE_NO_TITLE, TRANSLATED_VIDEO_CHALLENGE // WRONG_NUM_TESTS_CHALLENGE } = require('./__fixtures__/challenge-objects'); @@ -22,6 +23,11 @@ const COMBINED_CHALLENGE = mergeChallenges( TRANSLATED_CHALLENGE ); +const COMBINED_CHALLENGE_NO_TITLE = mergeChallenges( + ENGLISH_CHALLENGE, + TRANSLATED_CHALLENGE_NO_TITLE +); + const COMBINED_CHALLENGE_TWO_SOLUTIONS = mergeChallenges( ENGLISH_CHALLENGE_TWO_SOLUTIONS, TRANSLATED_CHALLENGE @@ -100,10 +106,19 @@ describe('translation parser', () => { expect(actualStrings[i]).toBe(expectedStrings[i]); } }); - it('takes the localTitle from the second challenge', () => { - expect(COMBINED_CHALLENGE.localeTitle).toBe( - TRANSLATED_CHALLENGE.localeTitle - ); + it('takes the title from the second challenge', () => { + expect(COMBINED_CHALLENGE.title).toBe(TRANSLATED_CHALLENGE.title); + }); + + // TODO: throw in production? + it("takes the first challenge's title if the second is missing", () => { + expect(COMBINED_CHALLENGE_NO_TITLE.title).toBe(ENGLISH_CHALLENGE.title); + }); + + // 'originalTitle' is just used to create the dashedName (which must be + // the same in both challenges, but only gets added after parsing) + it('creates originalTitle from the first challenge', () => { + expect(COMBINED_CHALLENGE.originalTitle).toBe(ENGLISH_CHALLENGE.title); }); // TODO: reinstate this after alpha testing. // it('throws an error if the numbers of tests do not match', () => {