refactor: files{} -> challengeFiles[], and key -> fileKey (#43023)

* fix(client): fix client

* fix propType and add comment

* revert user.json prettification

* slight type refactor and payload correction

Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>

* update ChallengeFile type imports

* add cypress test for code-storage

* update test and storage epic

* fix Shaun's tired brain's logic

* refactor with suggestions

Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>

* update codeReset

* increate cypress timeout because firefox is slow

* remove unused import to make linter happy

* use focus on editor

Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>

* use more specific seletor for cypress editor test

* account for silly null challengeFiles

Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>

Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>

curriculum/getChallenges.js

@@ -2,7 +2,7 @@ const fs = require('fs');
 const path = require('path');
 const util = require('util');
 const yaml = require('js-yaml');
-const { findIndex, reduce, toString } = require('lodash');
+const { findIndex } = require('lodash');
 const readDirP = require('readdirp');
 const { helpCategoryMap } = require('../client/utils/challenge-types');
 const { showUpcomingChanges } = require('../config/env.json');
@@ -306,44 +306,21 @@ ${getFullPath('english')}
   return prepareChallenge(challenge);
 }
 
-// TODO: tests and more descriptive name.
-function filesToObject(files) {
-  return reduce(
-    files,
-    (map, file) => {
-      map[file.key] = {
-        ...file,
-        head: arrToString(file.head),
-        contents: arrToString(file.contents),
-        tail: arrToString(file.tail)
-      };
-      return map;
-    },
-    {}
-  );
-}
-
 // gets the challenge ready for sourcing into Gatsby
 function prepareChallenge(challenge) {
-  if (challenge.files) {
-    challenge.files = filesToObject(challenge.files);
-    challenge.files = Object.keys(challenge.files)
-      .filter(key => challenge.files[key])
-      .map(key => challenge.files[key])
-      .reduce(
-        (files, file) => ({
-          ...files,
-          [file.key]: {
-            ...createPoly(file),
-            seed: file.contents.slice(0)
+  if (challenge.challengeFiles) {
+    challenge.challengeFiles = challenge.challengeFiles.reduce(
+      (challengeFiles, challengeFile) => {
+        return [
+          ...challengeFiles,
+          {
+            ...createPoly(challengeFile),
+            seed: challengeFile.contents.slice(0)
           }
-        }),
-        {}
-      );
-  }
-
-  if (challenge.solutionFiles) {
-    challenge.solutionFiles = filesToObject(challenge.solutionFiles);
+        ];
+      },
+      []
+    );
   }
   return challenge;
 }
@@ -381,10 +358,6 @@ function getBlockNameFromPath(filePath) {
   return block;
 }
 
-function arrToString(arr) {
-  return Array.isArray(arr) ? arr.join('\n') : toString(arr);
-}
-
 exports.hasEnglishSource = hasEnglishSource;
 exports.parseTranslation = parseTranslation;
 exports.createChallenge = createChallenge;

curriculum/schema/challengeSchema.js

@@ -6,7 +6,7 @@ const { challengeTypes } = require('../../client/utils/challenge-types');
 const slugRE = new RegExp('^[a-z0-9-]+$');
 
 const fileJoi = Joi.object().keys({
-  key: Joi.string(),
+  fileKey: Joi.string(),
   ext: Joi.string(),
   name: Joi.string(),
   editableRegionBoundaries: [Joi.array().items(Joi.number())],
@@ -37,12 +37,7 @@ const schema = Joi.object()
       then: Joi.string().allow(''),
       otherwise: Joi.string().required()
     }),
-    files: Joi.object().keys({
-      indexcss: fileJoi,
-      indexhtml: fileJoi,
-      indexjs: fileJoi,
-      indexjsx: fileJoi
-    }),
+    challengeFiles: Joi.array().items(fileJoi),
     guideUrl: Joi.string().uri({ scheme: 'https' }),
     helpCategory: Joi.valid(
       'JavaScript',
@@ -76,15 +71,7 @@ const schema = Joi.object()
         crossDomain: Joi.bool()
       })
     ),
-    solutions: Joi.array().items(
-      Joi.object().keys({
-        indexcss: fileJoi,
-        indexhtml: fileJoi,
-        indexjs: fileJoi,
-        indexjsx: fileJoi,
-        indexpy: fileJoi
-      })
-    ),
+    solutions: Joi.array().items(Joi.array().items(fileJoi)),
     superBlock: Joi.string().regex(slugRE),
     superOrder: Joi.number(),
     suborder: Joi.number(),

curriculum/test/test-challenges.js

@@ -307,16 +307,16 @@ function populateTestsForLang({ lang, challenges, meta }) {
               return;
             }
 
-            // If no .files, then no seed:
-            if (!challenge.files) return;
+            // If no .challengeFiles, then no seed:
+            if (!challenge.challengeFiles) return;
 
             // - None of the translatable comments should appear in the
             //   translations. While this is a crude check, no challenges
             //   currently have the text of a comment elsewhere. If that happens
             //   we can handle that challenge separately.
             TRANSLATABLE_COMMENTS.forEach(comment => {
-              Object.values(challenge.files).forEach(file => {
-                if (file.contents.includes(comment))
+              challenge.challengeFiles.forEach(challengeFile => {
+                if (challengeFile.contents.includes(comment))
                   throw Error(
                     `English comment '${comment}' should be replaced with its translation`
                   );
@@ -325,14 +325,16 @@ function populateTestsForLang({ lang, challenges, meta }) {
 
             // - None of the translated comment texts should appear *outside* a
             //   comment
-            Object.values(challenge.files).forEach(file => {
+            challenge.challengeFiles.forEach(challengeFile => {
               let comments = {};
 
               // We get all the actual comments using the appropriate parsers
-              if (file.ext === 'html') {
+              if (challengeFile.ext === 'html') {
                 const commentTypes = ['css', 'html', 'scriptJs'];
                 for (let type of commentTypes) {
-                  const newComments = commentExtractors[type](file.contents);
+                  const newComments = commentExtractors[type](
+                    challengeFile.contents
+                  );
                   for (const [key, value] of Object.entries(newComments)) {
                     comments[key] = comments[key]
                       ? comments[key] + value
@@ -340,7 +342,9 @@ function populateTestsForLang({ lang, challenges, meta }) {
                   }
                 }
               } else {
-                comments = commentExtractors[file.ext](file.contents);
+                comments = commentExtractors[challengeFile.ext](
+                  challengeFile.contents
+                );
               }
 
               // Then we compare the number of times each comment appears in the
@@ -409,7 +413,7 @@ ${inspect(commentMap)}
             try {
               testRunner = await createTestRunner(
                 challenge,
-                '',
+                [],
                 buildChallenge
               );
             } catch {
@@ -448,12 +452,13 @@ ${inspect(commentMap)}
             // TODO: can this be dried out, ideally by removing the redux
             // handler?
             if (nextChallenge) {
-              const solutionFiles = cloneDeep(nextChallenge.files);
-              Object.keys(solutionFiles).forEach(key => {
-                const file = solutionFiles[key];
-                file.editableContents = getLines(
-                  file.contents,
-                  challenge.files[key].editableRegionBoundaries
+              const solutionFiles = cloneDeep(nextChallenge.challengeFiles);
+              solutionFiles.forEach(challengeFile => {
+                challengeFile.editableContents = getLines(
+                  challengeFile.contents,
+                  challenge.challengeFiles.find(
+                    x => x.fileKey === challengeFile.fileKey
+                  ).editableRegionBoundaries
                 );
               });
               solutions = [solutionFiles];
@@ -470,7 +475,9 @@ ${inspect(commentMap)}
 
           const filteredSolutions = solutionsAsArrays.filter(solution => {
             return !isEmpty(
-              solution.filter(file => !noSolution.test(file.contents))
+              solution.filter(
+                challengeFile => !noSolution.test(challengeFile.contents)
+              )
             );
           });
 
@@ -505,21 +512,23 @@ ${inspect(commentMap)}
 
 async function createTestRunner(
   challenge,
-  solution,
+  solutionFiles,
   buildChallenge,
   solutionFromNext
 ) {
   const { required = [], template, removeComments } = challenge;
   // we should avoid modifying challenge, as it gets reused:
-  const files = cloneDeep(challenge.files);
-
-  Object.keys(solution).forEach(key => {
-    files[key].contents = solution[key].contents;
-    files[key].editableContents = solution[key].editableContents;
+  const challengeFiles = cloneDeep(challenge.challengeFiles);
+  solutionFiles.forEach(solutionFile => {
+    const challengeFile = challengeFiles.find(
+      x => x.fileKey === solutionFile.fileKey
+    );
+    challengeFile.contents = solutionFile.contents;
+    challengeFile.editableContents = solutionFile.editableContents;
   });
 
   const { build, sources, loadEnzyme } = await buildChallenge({
-    files,
+    challengeFiles,
     required,
     template
   });