From d3dabb1f36817518d8f02cf6a9c03d7886904200 Mon Sep 17 00:00:00 2001 From: Berkeley Martinez Date: Mon, 8 Aug 2016 16:21:04 -0700 Subject: [PATCH 1/2] Fix(challenges): completed marked at render Mark challenge completed using derived data in a selector instead of manipulating the data on user load --- common/app/redux/actions.js | 4 ---- common/app/redux/entities-reducer.js | 18 +----------------- common/app/redux/fetch-user-saga.js | 2 -- common/app/redux/types.js | 1 - .../challenges/components/map/Challenge.jsx | 15 +++++++++++++-- 5 files changed, 14 insertions(+), 26 deletions(-) diff --git a/common/app/redux/actions.js b/common/app/redux/actions.js index 83fefaf255..8e3fec9ecc 100644 --- a/common/app/redux/actions.js +++ b/common/app/redux/actions.js @@ -95,10 +95,6 @@ export const updateUserLang = createAction( (username, lang) => ({ username, lang }) ); export const updateAppLang = createAction(types.updateAppLang); -// updateCompletedChallenges(username: String) => Action -export const updateCompletedChallenges = createAction( - types.updateCompletedChallenges -); // used when server needs client to redirect export const delayedRedirect = createAction(types.delayedRedirect); diff --git a/common/app/redux/entities-reducer.js b/common/app/redux/entities-reducer.js index 18170b461f..b4d48b8a37 100644 --- a/common/app/redux/entities-reducer.js +++ b/common/app/redux/entities-reducer.js @@ -1,6 +1,6 @@ import types from './types'; -const { updateUserPoints, updateCompletedChallenges } = types; +const { updateUserPoints } = types; const initialState = { superBlock: {}, block: {}, @@ -15,22 +15,6 @@ export default function entities(state = initialState, action) { type, payload: { email, username, points, flag, languageTag } = {} } = action; - if (type === updateCompletedChallenges) { - const username = action.payload; - const completedChallengeMap = state.user[username].challengeMap || {}; - return { - ...state, - challenge: Object.keys(state.challenge) - .reduce((map, key) => { - const challenge = state.challenge[key]; - map[key] = { - ...challenge, - isCompleted: !!completedChallengeMap[challenge.id] - }; - return map; - }, {}) - }; - } if (action.meta && action.meta.entities) { return { ...state, diff --git a/common/app/redux/fetch-user-saga.js b/common/app/redux/fetch-user-saga.js index 5afa9b5872..b2b8a36495 100644 --- a/common/app/redux/fetch-user-saga.js +++ b/common/app/redux/fetch-user-saga.js @@ -3,7 +3,6 @@ import types from './types'; import { addUser, updateThisUser, - updateCompletedChallenges, createErrorObservable, showSignIn, updateTheme, @@ -24,7 +23,6 @@ export default function getUserSaga(action$, getState, { services }) { const isNightMode = user.theme === 'night'; return Observable.of( addUser(entities), - updateCompletedChallenges(result), updateThisUser(result), isNightMode ? updateTheme(user.theme) : null, isNightMode ? addThemeToBody(user.theme) : null diff --git a/common/app/redux/types.js b/common/app/redux/types.js index 1da89d89c3..defd139268 100644 --- a/common/app/redux/types.js +++ b/common/app/redux/types.js @@ -12,7 +12,6 @@ export default createTypes([ 'updateUserFlag', 'updateUserEmail', 'updateUserLang', - 'updateCompletedChallenges', 'showSignIn', 'loadCurrentChallenge', 'updateMyCurrentChallenge', diff --git a/common/app/routes/challenges/components/map/Challenge.jsx b/common/app/routes/challenges/components/map/Challenge.jsx index 202bb70e69..05f3e88c90 100644 --- a/common/app/routes/challenges/components/map/Challenge.jsx +++ b/common/app/routes/challenges/components/map/Challenge.jsx @@ -8,24 +8,35 @@ import debug from 'debug'; import { updateCurrentChallenge } from '../../redux/actions'; import { makePanelHiddenSelector } from '../../redux/selectors'; +import { userSelector } from '../../../../redux/selectors'; import { closeMapDrawer } from '../../../../redux/actions'; const bindableActions = { closeMapDrawer, updateCurrentChallenge }; const makeMapStateToProps = () => createSelector( + userSelector, (_, props) => props.dashedName, state => state.entities.challenge, makePanelHiddenSelector(), - (dashedName, challengeMap, isHidden) => { + ( + { user: { challengeMap: userChallengeMap } }, + dashedName, + challengeMap, + isHidden + ) => { const challenge = challengeMap[dashedName] || {}; + let isCompleted = false; + if (userChallengeMap) { + isCompleted = !!userChallengeMap[challenge.id]; + } return { dashedName, challenge, isHidden, + isCompleted, title: challenge.title, block: challenge.block, isLocked: challenge.isLocked, isRequired: challenge.isRequired, - isCompleted: challenge.isCompleted, isComingSoon: challenge.isComingSoon, isDev: debug.enabled('fcc:*') }; From 8be0d194a5a4f0e00b3639f7e98aea80dfe9dee9 Mon Sep 17 00:00:00 2001 From: Berkeley Martinez Date: Thu, 11 Aug 2016 16:41:03 -0700 Subject: [PATCH 2/2] Fix(challenge): update user challenge map on challenge complete --- common/app/redux/actions.js | 10 +++ common/app/redux/entities-reducer.js | 17 +++++ common/app/redux/types.js | 1 + common/app/routes/challenges/redux/actions.js | 5 +- .../challenges/redux/completion-saga.js | 65 +++++++++++-------- server/boot/challenge.js | 40 ++++++++---- 6 files changed, 98 insertions(+), 40 deletions(-) diff --git a/common/app/redux/actions.js b/common/app/redux/actions.js index 8e3fec9ecc..cc6e6867e7 100644 --- a/common/app/redux/actions.js +++ b/common/app/redux/actions.js @@ -94,6 +94,16 @@ export const updateUserLang = createAction( types.updateUserLang, (username, lang) => ({ username, lang }) ); + +// updateUserChallenge( +// username: String, +// challengeInfo: Object +// ) => Action +export const updateUserChallenge = createAction( + types.updateUserChallenge, + (username, challengeInfo) => ({ username, challengeInfo }) +); + export const updateAppLang = createAction(types.updateAppLang); // used when server needs client to redirect diff --git a/common/app/redux/entities-reducer.js b/common/app/redux/entities-reducer.js index b4d48b8a37..af6d1046b5 100644 --- a/common/app/redux/entities-reducer.js +++ b/common/app/redux/entities-reducer.js @@ -82,5 +82,22 @@ export default function entities(state = initialState, action) { } }; } + + if (action.type === types.updateUserChallenge) { + const { challengeInfo } = action.payload; + return { + ...state, + user: { + ...state.user, + [username]: { + ...state.user[username], + challengeMap: { + ...state.user[username].challengeMap, + [challengeInfo.id]: challengeInfo + } + } + } + }; + } return state; } diff --git a/common/app/redux/types.js b/common/app/redux/types.js index defd139268..c01ef12618 100644 --- a/common/app/redux/types.js +++ b/common/app/redux/types.js @@ -12,6 +12,7 @@ export default createTypes([ 'updateUserFlag', 'updateUserEmail', 'updateUserLang', + 'updateUserChallenge', 'showSignIn', 'loadCurrentChallenge', 'updateMyCurrentChallenge', diff --git a/common/app/routes/challenges/redux/actions.js b/common/app/routes/challenges/redux/actions.js index f0d9593033..05467fa999 100644 --- a/common/app/routes/challenges/redux/actions.js +++ b/common/app/routes/challenges/redux/actions.js @@ -59,7 +59,10 @@ export const updateFile = createAction( export const updateFiles = createAction(types.updateFiles); // rechallenge -export const executeChallenge = createAction(types.executeChallenge); +export const executeChallenge = createAction( + types.executeChallenge, + () => null +); export const updateMain = createAction(types.updateMain); export const frameMain = createAction(types.frameMain); diff --git a/common/app/routes/challenges/redux/completion-saga.js b/common/app/routes/challenges/redux/completion-saga.js index ba92539232..d5cad8f32f 100644 --- a/common/app/routes/challenges/redux/completion-saga.js +++ b/common/app/routes/challenges/redux/completion-saga.js @@ -2,22 +2,30 @@ import { Observable } from 'rx'; import types from './types'; import { moveToNextChallenge } from './actions'; -import { - createErrorObservable, - updateUserPoints -} from '../../../redux/actions'; -import { makeToast } from '../../../toasts/redux/actions'; import { challengeSelector } from './selectors'; import { randomCompliment } from '../../../utils/get-words'; +import { + createErrorObservable, + updateUserPoints, + updateUserChallenge +} from '../../../redux/actions'; import { backEndProject } from '../../../utils/challengeTypes'; +import { makeToast } from '../../../toasts/redux/actions'; import { postJSON$ } from '../../../../utils/ajax-stream'; -function postChallenge(url, body, username) { +function postChallenge(url, username, _csrf, challengeInfo) { + const body = { ...challengeInfo, _csrf }; const saveChallenge$ = postJSON$(url, body) .retry(3) - .map(({ points }) => { - return updateUserPoints(username, points); + .flatMap(({ points, lastUpdated, completedDate }) => { + return Observable.of( + updateUserPoints(username, points), + updateUserChallenge( + username, + { ...challengeInfo, lastUpdated, completedDate } + ) + ); }) .catch(createErrorObservable); const challengeCompleted$ = Observable.of(moveToNextChallenge()); @@ -44,12 +52,13 @@ function submitModern(type, state) { app: { user, csrfToken }, challengesApp: { files } } = state; - const body = { - id, - _csrf: csrfToken, - files - }; - return postChallenge('/modern-challenge-completed', body, user); + const challengeInfo = { id, files }; + return postChallenge( + '/modern-challenge-completed', + user, + csrfToken, + challengeInfo + ); } } return Observable.just(makeToast({ message: 'Not quite there, yet.' })); @@ -62,16 +71,16 @@ function submitProject(type, state, { solution, githubLink }) { const { app: { user, csrfToken } } = state; - const body = { - id, - challengeType, - solution, - _csrf: csrfToken - }; + const challengeInfo = { id, challengeType, solution }; if (challengeType === backEndProject) { - body.githubLink = githubLink; + challengeInfo.githubLink = githubLink; } - return postChallenge('/project-completed', body, user); + return postChallenge( + '/project-completed', + user, + csrfToken, + challengeInfo + ); } function submitSimpleChallenge(type, state) { @@ -81,11 +90,13 @@ function submitSimpleChallenge(type, state) { const { app: { user, csrfToken } } = state; - const body = { - id, - _csrf: csrfToken - }; - return postChallenge('/challenge-completed', body, user); + const challengeInfo = { id }; + return postChallenge( + '/challenge-completed', + user, + csrfToken, + challengeInfo + ); } const submitTypes = { diff --git a/server/boot/challenge.js b/server/boot/challenge.js index 0f7600585d..820cac6e53 100644 --- a/server/boot/challenge.js +++ b/server/boot/challenge.js @@ -61,7 +61,12 @@ function buildUserUpdate( log('user update data', updateData); - return { alreadyCompleted, updateData }; + return { + alreadyCompleted, + updateData, + completedDate: finalChallenge.completedDate, + lastUpdated: finalChallenge.lastUpdated + }; } export default function(app) { @@ -138,14 +143,14 @@ export default function(app) { files } = req.body; - const { alreadyCompleted, updateData } = buildUserUpdate( + const { + alreadyCompleted, + updateData, + lastUpdated + } = buildUserUpdate( user, id, - { - id, - files, - completedDate - } + { id, files, completedDate } ); const points = alreadyCompleted ? user.points : user.points + 1; @@ -156,7 +161,9 @@ export default function(app) { if (type === 'json') { return res.json({ points, - alreadyCompleted + alreadyCompleted, + completedDate, + lastUpdated }); } return res.sendStatus(200); @@ -184,7 +191,11 @@ export default function(app) { const completedDate = Date.now(); const { id, solution, timezone } = req.body; - const { alreadyCompleted, updateData } = buildUserUpdate( + const { + alreadyCompleted, + updateData, + lastUpdated + } = buildUserUpdate( req.user, id, { id, solution, completedDate }, @@ -200,7 +211,9 @@ export default function(app) { if (type === 'json') { return res.json({ points, - alreadyCompleted + alreadyCompleted, + completedDate, + lastUpdated }); } return res.sendStatus(200); @@ -253,7 +266,8 @@ export default function(app) { .flatMap(() => { const { alreadyCompleted, - updateData + updateData, + lastUpdated } = buildUserUpdate(user, completedChallenge.id, completedChallenge); return user.update$(updateData) @@ -262,7 +276,9 @@ export default function(app) { if (type === 'json') { return res.send({ alreadyCompleted, - points: alreadyCompleted ? user.points : user.points + 1 + points: alreadyCompleted ? user.points : user.points + 1, + completedDate: completedChallenge.completedDate, + lastUpdated }); } return res.status(200).send(true);