From f8699a8d5557570d4675b20a8d03f5eb99760d97 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Tue, 23 Feb 2021 05:22:48 +0100 Subject: [PATCH] refactor: simplify challenge.block usage (#41185) --- api-server/src/server/boot/challenge.js | 3 +-- api-server/src/server/boot_tests/challenge.test.js | 8 -------- client/gatsby-node.js | 3 +-- client/src/templates/Challenges/classic/Show.js | 9 ++++----- .../Challenges/components/Challenge-Description.js | 8 ++++---- .../templates/Challenges/components/Challenge-Title.js | 5 ++--- .../Challenges/components/ChallengeTitle.test.js | 5 +++-- client/src/templates/Challenges/components/Side-Panel.js | 8 +++----- .../components/__snapshots__/ChallengeTitle.test.js.snap | 4 ++-- client/src/templates/Introduction/SuperBlockIntro.js | 3 +-- client/utils/gatsby/challengePageCreator.js | 2 +- curriculum/getChallenges.js | 5 ++--- curriculum/schema/challengeSchema.js | 8 +++++--- curriculum/test/test-challenges.js | 8 +++----- 14 files changed, 32 insertions(+), 47 deletions(-) diff --git a/api-server/src/server/boot/challenge.js b/api-server/src/server/boot/challenge.js index 4e4afc3962..a0bdcc5602 100644 --- a/api-server/src/server/boot/challenge.js +++ b/api-server/src/server/boot/challenge.js @@ -13,7 +13,6 @@ import isNumeric from 'validator/lib/isNumeric'; import isURL from 'validator/lib/isURL'; import { ifNoUserSend } from '../utils/middleware'; -import { dasherize } from '../../../../utils/slugs'; import { fixCompletedChallengeItem } from '../../common/utils'; import { getChallenges } from '../utils/get-curriculum'; import { @@ -144,7 +143,7 @@ export function buildUserUpdate( export function buildChallengeUrl(challenge) { const { superBlock, block, dashedName } = challenge; - return `/learn/${dasherize(superBlock)}/${dasherize(block)}/${dashedName}`; + return `/learn/${superBlock}/${block}/${dashedName}`; } // this is only called once during boot, so it can be slow. diff --git a/api-server/src/server/boot_tests/challenge.test.js b/api-server/src/server/boot_tests/challenge.test.js index e1a4f9b069..69f7fce8a3 100644 --- a/api-server/src/server/boot_tests/challenge.test.js +++ b/api-server/src/server/boot_tests/challenge.test.js @@ -159,14 +159,6 @@ describe('boot/challenge', () => { expect(result).toEqual(requestedChallengeUrl); }); - - it('can handle non-url-compliant challenge names', () => { - const challenge = { ...mockChallenge, superBlock: 'my awesome' }; - const expected = '/learn/my-awesome/actual/challenge'; - const result = buildChallengeUrl(challenge); - - expect(result).toEqual(expected); - }); }); describe('challengeUrlResolver', () => { diff --git a/client/gatsby-node.js b/client/gatsby-node.js index eea4fe913d..7f1f559216 100644 --- a/client/gatsby-node.js +++ b/client/gatsby-node.js @@ -3,7 +3,6 @@ const env = require('../config/env'); const { createFilePath } = require('gatsby-source-filesystem'); const uniq = require('lodash/uniq'); -const { dasherize } = require('../utils/slugs'); const { blockNameify } = require('../utils/block-nameify'); const { createChallengePages, @@ -20,7 +19,7 @@ exports.onCreateNode = function onCreateNode({ node, actions, getNode }) { const { createNodeField } = actions; if (node.internal.type === 'ChallengeNode') { const { tests = [], block, dashedName, superBlock } = node; - const slug = `/learn/${superBlock}/${dasherize(block)}/${dashedName}`; + const slug = `/learn/${superBlock}/${block}/${dashedName}`; createNodeField({ node, name: 'slug', value: slug }); createNodeField({ node, name: 'blockName', value: blockNameify(block) }); createNodeField({ node, name: 'tests', value: tests }); diff --git a/client/src/templates/Challenges/classic/Show.js b/client/src/templates/Challenges/classic/Show.js index fe5bd39150..2e467bd7f9 100644 --- a/client/src/templates/Challenges/classic/Show.js +++ b/client/src/templates/Challenges/classic/Show.js @@ -24,7 +24,6 @@ import Hotkeys from '../components/Hotkeys'; import { getGuideUrl } from '../utils'; import { challengeTypes } from '../../../../utils/challengeTypes'; import { ChallengeNode } from '../../../redux/propTypes'; -import { dasherize } from '../../../../../utils/slugs'; import { createFiles, challengeFilesSelector, @@ -206,7 +205,7 @@ class ShowClassic extends Component { renderInstructionsPanel({ showToolPanel }) { const { - fields: { blockName }, + block, description, instructions, superBlock, @@ -216,12 +215,11 @@ class ShowClassic extends Component { const { forumTopicId, title } = this.getChallenge(); return ( +
{description && } {instructions && ( diff --git a/client/src/templates/Challenges/components/Challenge-Title.js b/client/src/templates/Challenges/components/Challenge-Title.js index 796f70c858..4ad7eb48a8 100644 --- a/client/src/templates/Challenges/components/Challenge-Title.js +++ b/client/src/templates/Challenges/components/Challenge-Title.js @@ -2,7 +2,6 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Link } from '../../../components/helpers/index'; -import { dasherize } from '../../../../../utils/slugs'; import './challenge-title.css'; import GreenPass from '../../../assets/icons/GreenPass'; import i18next from 'i18next'; @@ -46,9 +45,9 @@ function ChallengeTitle({ - {i18next.t(`intro:${superBlock}.blocks.${dasherize(block)}.title`)} + {i18next.t(`intro:${superBlock}.blocks.${block}.title`)}
diff --git a/client/src/templates/Challenges/components/ChallengeTitle.test.js b/client/src/templates/Challenges/components/ChallengeTitle.test.js index 54f5cd5975..acc47c48ad 100644 --- a/client/src/templates/Challenges/components/ChallengeTitle.test.js +++ b/client/src/templates/Challenges/components/ChallengeTitle.test.js @@ -6,10 +6,11 @@ import renderer from 'react-test-renderer'; import ChallengeTitle from './Challenge-Title'; const baseProps = { - block: 'fake block', + block: 'fake-block', children: 'title text', isCompleted: true, - superBlock: 'fake-superblock' + superBlock: 'fake-superblock', + translationPending: false }; describe('', () => { diff --git a/client/src/templates/Challenges/components/Side-Panel.js b/client/src/templates/Challenges/components/Side-Panel.js index 66b1492723..abdeb5c0cb 100644 --- a/client/src/templates/Challenges/components/Side-Panel.js +++ b/client/src/templates/Challenges/components/Side-Panel.js @@ -27,7 +27,6 @@ const propTypes = { guideUrl: PropTypes.string, instructions: PropTypes.string, isChallengeCompleted: PropTypes.bool, - section: PropTypes.string, showToolPanel: PropTypes.bool, superBlock: PropTypes.string, tests: PropTypes.arrayOf(PropTypes.object), @@ -41,8 +40,8 @@ export class SidePanel extends Component { const MathJax = global.MathJax; const mathJaxMountPoint = document.querySelector('#mathjax'); const mathJaxChallenge = - this.props.section === 'rosetta-code' || - this.props.section === 'project-euler'; + this.props.block === 'rosetta-code' || + this.props.block === 'project-euler'; if (MathJax) { // Configure MathJax when it's loaded and // users navigate from another challenge @@ -73,7 +72,6 @@ export class SidePanel extends Component { isChallengeCompleted, guideUrl, tests, - section, showToolPanel, superBlock, translationPending, @@ -91,9 +89,9 @@ export class SidePanel extends Component { {title}
{showToolPanel && } diff --git a/client/src/templates/Challenges/components/__snapshots__/ChallengeTitle.test.js.snap b/client/src/templates/Challenges/components/__snapshots__/ChallengeTitle.test.js.snap index 739fdf0dfb..4953c740fe 100644 --- a/client/src/templates/Challenges/components/__snapshots__/ChallengeTitle.test.js.snap +++ b/client/src/templates/Challenges/components/__snapshots__/ChallengeTitle.test.js.snap @@ -12,7 +12,7 @@ exports[` renders correctly 1`] = ` href="/learn/fake-superblock" state={ Object { - "breadcrumbBlockClick": "fake block", + "breadcrumbBlockClick": "fake-block", } } > @@ -28,7 +28,7 @@ exports[` renders correctly 1`] = ` href="/learn/fake-superblock/#fake-block" state={ Object { - "breadcrumbBlockClick": "fake block", + "breadcrumbBlockClick": "fake-block", } } /> diff --git a/client/src/templates/Introduction/SuperBlockIntro.js b/client/src/templates/Introduction/SuperBlockIntro.js index 67ae41ed11..715f22c53a 100644 --- a/client/src/templates/Introduction/SuperBlockIntro.js +++ b/client/src/templates/Introduction/SuperBlockIntro.js @@ -14,7 +14,6 @@ import Login from '../../components/Header/components/Login'; import Map from '../../components/Map'; import CertChallenge from './components/CertChallenge'; import SuperBlockIntro from './components/SuperBlockIntro'; -import { dasherize } from '../../../../utils/slugs'; import Block from './components/Block'; import { Spacer } from '../../components/helpers'; import { @@ -97,7 +96,7 @@ export class SuperBlockIntroductionPage extends Component { // if coming from breadcrumb click if (location.state && location.state.breadcrumbBlockClick) { - return dasherize(location.state.breadcrumbBlockClick); + return location.state.breadcrumbBlockClick; } // if the URL includes a hash diff --git a/client/utils/gatsby/challengePageCreator.js b/client/utils/gatsby/challengePageCreator.js index e0dffc2e1d..51628c84b0 100644 --- a/client/utils/gatsby/challengePageCreator.js +++ b/client/utils/gatsby/challengePageCreator.js @@ -68,7 +68,7 @@ exports.createChallengePages = createPage => ({ node }, index, thisArray) => { context: { challengeMeta: { superBlock, - block: block, + block, template, required, nextChallengePath: getNextChallengePath(node, index, thisArray), diff --git a/curriculum/getChallenges.js b/curriculum/getChallenges.js index 0353523936..d5a07c6f41 100644 --- a/curriculum/getChallenges.js +++ b/curriculum/getChallenges.js @@ -288,7 +288,7 @@ ${getFullPath('english')} template, time } = meta; - challenge.block = blockName; + challenge.block = dasherize(blockName); challenge.order = order; challenge.superOrder = superOrder; challenge.superBlock = superBlock; @@ -298,7 +298,7 @@ ${getFullPath('english')} challenge.template = template; challenge.time = time; challenge.helpCategory = - challenge.helpCategory || helpCategoryMap[dasherize(blockName)]; + challenge.helpCategory || helpCategoryMap[challenge.block]; challenge.translationPending = lang !== 'english' && !isAuditedCert(lang, superBlock); @@ -344,7 +344,6 @@ function prepareChallenge(challenge) { if (challenge.solutionFiles) { challenge.solutionFiles = filesToObject(challenge.solutionFiles); } - challenge.block = dasherize(challenge.block); return challenge; } diff --git a/curriculum/schema/challengeSchema.js b/curriculum/schema/challengeSchema.js index b2f8ea65bd..84566549fb 100644 --- a/curriculum/schema/challengeSchema.js +++ b/curriculum/schema/challengeSchema.js @@ -3,6 +3,8 @@ Joi.objectId = require('joi-objectid')(Joi); const { challengeTypes } = require('../../client/utils/challengeTypes'); +const slugRE = new RegExp('^[a-z0-9-]+$'); + const fileJoi = Joi.object().keys({ key: Joi.string(), ext: Joi.string(), @@ -20,7 +22,7 @@ const fileJoi = Joi.object().keys({ const schema = Joi.object() .keys({ - block: Joi.string(), + block: Joi.string().regex(slugRE), blockId: Joi.objectId(), challengeOrder: Joi.number(), challengeType: Joi.number() @@ -29,7 +31,7 @@ const schema = Joi.object() .required(), checksum: Joi.number(), // TODO: require this only for normal challenges, not certs - dashedName: Joi.string(), + dashedName: Joi.string().regex(slugRE), description: Joi.when('challengeType', { is: Joi.only([challengeTypes.step, challengeTypes.video]), then: Joi.string().allow(''), @@ -82,7 +84,7 @@ const schema = Joi.object() indexpy: fileJoi }) ), - superBlock: Joi.string(), + superBlock: Joi.string().regex(slugRE), superOrder: Joi.number(), suborder: Joi.number(), tests: Joi.array().items( diff --git a/curriculum/test/test-challenges.js b/curriculum/test/test-challenges.js index 4447e9685f..0ea05a44d1 100644 --- a/curriculum/test/test-challenges.js +++ b/curriculum/test/test-challenges.js @@ -44,7 +44,6 @@ const ChallengeTitles = require('./utils/challengeTitles'); const { challengeSchemaValidator } = require('../schema/challengeSchema'); const { challengeTypes } = require('../../client/utils/challengeTypes'); -const { dasherize } = require('../../utils/slugs'); const { toSortedArray } = require('../../utils/sort-files'); const { testedLang } = require('../utils'); @@ -197,7 +196,7 @@ async function setup() { const meta = {}; for (const challenge of challenges) { - const dashedBlockName = dasherize(challenge.block); + const dashedBlockName = challenge.block; if (!meta[dashedBlockName]) { meta[dashedBlockName] = (await getMetaForBlock( dashedBlockName @@ -256,7 +255,7 @@ function populateTestsForLang({ lang, challenges, meta }) { describe(`Check challenges (${lang})`, function() { this.timeout(5000); challenges.forEach((challenge, id) => { - const dashedBlockName = dasherize(challenge.block); + const dashedBlockName = challenge.block; describe(challenge.block || 'No block', function() { describe(challenge.title || 'No title', function() { // Note: the title in meta.json are purely for human readability and @@ -280,8 +279,7 @@ function populateTestsForLang({ lang, challenges, meta }) { throw new AssertionError(result.error); } const { id, title, block, dashedName } = challenge; - const dashedBlock = dasherize(block); - const pathAndTitle = `${dashedBlock}/${dashedName}`; + const pathAndTitle = `${block}/${dashedName}`; mongoIds.check(id, title); challengeTitles.check(title, pathAndTitle); });