Fix isBeta challenges displaying without title when they should be hidden #12989

systimotic · 2017-01-30T20:58:03Z

Pre-Submission Checklist

Your pull request targets the staging branch of freeCodeCamp.
Branch starts with either fix/, feature/, or translate/ (e.g. fix/signin-issue)
You have only one commit (if not, squash them into one commit).
All new and existing tests pass the command npm test. Use git commit --amend to amend any fixes.

Type of Change

Small bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds new functionality)
Breaking change (fix or feature that would change existing functionality)
Add new translation (feature adding new translations)

Checklist:

Tested changes locally.
Closes currently open issue (replace XXXX with an issue no):
Closes Missing Challenges #12941
Closes [beta][UI] Missing challenge in Map, Slice and Splice #12842
Original: Map has missing challenges #11547

Description

Woo! So this is probably the biggest change I have made to date. A lot of hours went into understanding what's going on here.

Challenges with isBeta: true would display in production, but without title.
This was because the challenges weren't being filtered from the blocks. The titles were taken from the searchNames, which were filtered, so no title showed up.

@BerkeleyTrue This is my first time making a change this "major", and my first time changing the tests. Your review would be much appreciated.

Bouncey

Some great work here, just a few pointers for keeping it functional 😃

common/app/routes/challenges/utils.js

+    block: Object.keys(blocks)
+      .map(dashedName => {
+        const block = blocks[dashedName];
+        let challenges = block.challenges;


common/app/routes/challenges/utils.js

+            .filter(filter)
+            .map(challenge => challenge.dashedName);
+        }
+        block.challenges = challenges;


common/app/routes/challenges/utils.js

+      .map(dashedName => {
+        const block = blocks[dashedName];
+        let challenges = block.challenges;
+        if (Array.isArray(challenges)) {


seed/challenges/02-javascript-algorithms-and-data-structures/basic-algorithm-scripting.json

@@ -657,7 +657,6 @@
      ],
      "type": "bonfire",
      "isRequired": true,
-      "isBeta": true,


systimotic · 2017-02-19T00:01:48Z

@Bouncey Thank you so much for reviewing! I'm sorry for taking so long, I've been a bit absent in the past two weeks and I completely forgot about this. I just pushed the requested updates. How are things now?

systimotic · 2017-02-26T00:27:00Z

@Bouncey @BerkeleyTrue Can you review?

systimotic · 2017-03-05T12:39:52Z

@Bouncey @BerkeleyTrue A review would be great!

seed/challenges/02-javascript-algorithms-and-data-structures/basic-algorithm-scripting.json

@@ -657,7 +657,6 @@
      ],
      "type": "bonfire",
      "isRequired": true,
-      "isBeta": true,


BerkeleyTrue · 2017-03-07T21:42:19Z

@systimotic Sorry about that. I had requested changes a while back but I had failed to submit them.

camperbot · 2017-03-11T21:57:09Z

@systimotic updated the pull request.

systimotic · 2017-03-11T22:01:17Z

@BerkeleyTrue No worries!
That does make a whole lot of sense. I'll open an issue about the isBeta for the splicing challenge.
I updated the PR. How is it now?

common/app/routes/challenges/utils.test.js

+        .forEach(block => {
+          const challenges = actual.block[block].challenges;
+          challenges.forEach(challenge => challengesFromBlocks.push(challenge));
+        });


common/app/routes/challenges/utils.test.js

+      Object.keys(actual.block)
+        .forEach(block => {
+          challengesFromBlocksCount += actual.block[block].challenges.length;
+        });


common/app/routes/challenges/utils.test.js

+        .forEach(block => {
+          challengesFromBlocksCount += actual.block[block].challenges.length;
+        });
+      console.log(challengesFromBlocksCount, actual.block);


dhcodes · 2017-08-02T13:10:12Z

@systimotic This PR would also fix the new issue #15705. Do you plan to update this or do you want to start from scratch now that there have been a few updates to the base? I can take a stab at it as well, but looks like you're already a bit farther down the road to solving it.

camperbot · 2017-08-20T15:26:43Z

@systimotic updated the pull request.

common/app/redux/utils.js

@@ -10,12 +10,30 @@ export function filterComingSoonBetaChallenge(
 }

 export function filterComingSoonBetaFromEntities(
-  { challenge: challengeMap, ...rest },
+  { challenge: challengeMap, block: blocks, ...rest },


BerkeleyTrue · 2017-08-22T02:20:06Z

@systimotic Thanks for the PR. I think this will do for now (other than my nit pick above). The issue with this approach is now all the challenges are iterated through twice in addition to the blocks. We now have over 400 challenges and 60+ blocks, so this function went from iterating 400+ times to more like 900+ times. This is a very costly function now. Not your fault, but we will need to come up with a more integrated solution that does not involving iterating through all the challenges and instead filters out during nextChallenge and map rendering.

Other than the nitpick above code LGTM. If someone could run it and verify it's ready to merge.

…dden

camperbot · 2017-08-24T18:27:43Z

@systimotic updated the pull request.

systimotic · 2017-08-24T18:30:31Z

@BerkeleyTrue Yeah, it's not ideal. Should we stick with the workaround from #15741 instead?

Also, previously you mentioned some loops in the tests should be changed to reduce. Should I still do that?

BerkeleyTrue · 2017-09-02T03:03:54Z

Merged!!! 0129116

Happy Coding!

BerkeleyTrue added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Jan 30, 2017

dhcodes requested a review from BerkeleyTrue January 31, 2017 04:38

Bouncey suggested changes Feb 5, 2017

View reviewed changes

raisedadead added the status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. label Feb 15, 2017

systimotic force-pushed the fix/hidden-challenges branch from 930696d to 7212f15 Compare February 18, 2017 23:53

systimotic removed the status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. label Feb 19, 2017

BerkeleyTrue suggested changes Mar 7, 2017

View reviewed changes

seed/challenges/02-javascript-algorithms-and-data-structures/basic-algorithm-scripting.json Outdated

@@ -657,7 +657,6 @@

],

"type": "bonfire",

"isRequired": true,

"isBeta": true,

This comment was marked as off-topic.

Sign in to view

systimotic force-pushed the fix/hidden-challenges branch from 7212f15 to 52f63f5 Compare March 11, 2017 21:57

BerkeleyTrue suggested changes Mar 17, 2017

View reviewed changes

systimotic force-pushed the fix/hidden-challenges branch from 52f63f5 to 526e932 Compare August 20, 2017 15:26

BerkeleyTrue suggested changes Aug 22, 2017

View reviewed changes

Fix isBeta challenges displaying without title when they should be hi…

2c6f5fd

…dden

systimotic force-pushed the fix/hidden-challenges branch from 526e932 to 2c6f5fd Compare August 24, 2017 18:27

BerkeleyTrue approved these changes Sep 2, 2017

View reviewed changes

BerkeleyTrue closed this Sep 2, 2017

BerkeleyTrue removed the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Sep 2, 2017

systimotic mentioned this pull request Sep 4, 2017

Beta challenge sequence problem. #14949

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix isBeta challenges displaying without title when they should be hidden #12989

Fix isBeta challenges displaying without title when they should be hidden #12989

systimotic commented Jan 30, 2017

Bouncey left a comment

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

systimotic commented Feb 19, 2017

systimotic commented Feb 26, 2017

systimotic commented Mar 5, 2017

This comment was marked as off-topic.

BerkeleyTrue commented Mar 7, 2017

camperbot commented Mar 11, 2017

systimotic commented Mar 11, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

dhcodes commented Aug 2, 2017

camperbot commented Aug 20, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

BerkeleyTrue commented Aug 22, 2017

camperbot commented Aug 24, 2017

systimotic commented Aug 24, 2017 •

edited

BerkeleyTrue commented Sep 2, 2017

Fix isBeta challenges displaying without title when they should be hidden #12989

Fix isBeta challenges displaying without title when they should be hidden #12989

Conversation

systimotic commented Jan 30, 2017

Pre-Submission Checklist

Type of Change

Checklist:

Description

Bouncey left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

systimotic commented Feb 19, 2017

systimotic commented Feb 26, 2017

systimotic commented Mar 5, 2017

This comment was marked as off-topic.

BerkeleyTrue commented Mar 7, 2017

camperbot commented Mar 11, 2017

systimotic commented Mar 11, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

dhcodes commented Aug 2, 2017

camperbot commented Aug 20, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

BerkeleyTrue commented Aug 22, 2017

camperbot commented Aug 24, 2017

systimotic commented Aug 24, 2017 • edited

BerkeleyTrue commented Sep 2, 2017

systimotic commented Aug 24, 2017 •

edited