From f54b7c07f5830557a2d00aefcc0a6fb647c3e0ab Mon Sep 17 00:00:00 2001 From: Stuart Taylor Date: Tue, 12 Jun 2018 16:50:35 +0100 Subject: [PATCH] fix(settings): fix-up user objects for solutions (#17556) --- .../routes/Profile/components/Timeline.jsx | 27 +------ .../Settings/components/Cert-Settings.jsx | 8 +- .../Settings/components/JSAlgoAndDSForm.jsx | 7 +- .../Settings/components/SolutionViewer.jsx | 73 ++++++++++++------ .../Settings/utils/buildUserProjectsMap.js | 2 +- common/models/user.js | 76 +++++++++++-------- common/models/user.json | 38 ++++++---- common/utils/index.js | 7 ++ server/boot/certificate.js | 6 +- server/boot/challenge.js | 51 +++++++++++-- server/services/user.js | 12 ++- 11 files changed, 200 insertions(+), 107 deletions(-) diff --git a/common/app/routes/Profile/components/Timeline.jsx b/common/app/routes/Profile/components/Timeline.jsx index 0a98b9cbf0..c340ee8acc 100644 --- a/common/app/routes/Profile/components/Timeline.jsx +++ b/common/app/routes/Profile/components/Timeline.jsx @@ -64,7 +64,7 @@ class Timeline extends PureComponent { renderCompletion(completed) { const { idToNameMap } = this.props; - const { id, completedDate, solution, files } = completed; + const { id, completedDate } = completed; const challengeDashedName = idToNameMap[id]; return ( @@ -80,28 +80,7 @@ class Timeline extends PureComponent { } - - {/* eslint-disable no-nested-ternary */ - files ? ( - - ) : solution ? ( - - ) : '' - } - + ); } @@ -142,7 +121,7 @@ class Timeline extends PureComponent { Challenge - First Completed + Completed diff --git a/common/app/routes/Settings/components/Cert-Settings.jsx b/common/app/routes/Settings/components/Cert-Settings.jsx index 86d45afdd2..2b56e23e94 100644 --- a/common/app/routes/Settings/components/Cert-Settings.jsx +++ b/common/app/routes/Settings/components/Cert-Settings.jsx @@ -100,12 +100,15 @@ const propTypes = { superBlock: PropTypes.string, updateUserBackend: PropTypes.func.isRequired, userProjects: PropTypes.objectOf( - PropTypes.objectOf(PropTypes.oneOfType( + PropTypes.oneOfType( [ + // this is really messy, it should be addressed + // in completedChallenges migration to unify to one type PropTypes.string, + PropTypes.arrayOf(PropTypes.object), PropTypes.object ] - )) + ) ), username: PropTypes.string }; @@ -113,7 +116,6 @@ const propTypes = { class CertificationSettings extends PureComponent { constructor(props) { super(props); - this.buildProjectForms = this.buildProjectForms.bind(this); this.handleSubmit = this.handleSubmit.bind(this); } diff --git a/common/app/routes/Settings/components/JSAlgoAndDSForm.jsx b/common/app/routes/Settings/components/JSAlgoAndDSForm.jsx index 0749996c17..6fbb4f4e15 100644 --- a/common/app/routes/Settings/components/JSAlgoAndDSForm.jsx +++ b/common/app/routes/Settings/components/JSAlgoAndDSForm.jsx @@ -13,7 +13,12 @@ const jsFormPropTypes = { claimCert: PropTypes.func.isRequired, hardGoTo: PropTypes.func.isRequired, isCertClaimed: PropTypes.bool, - jsProjects: PropTypes.objectOf(PropTypes.object), + jsProjects: PropTypes.objectOf( + PropTypes.oneOfType( + PropTypes.arrayOf(PropTypes.object), + PropTypes.string + ) + ), projectBlockName: PropTypes.string, superBlock: PropTypes.string, username: PropTypes.string diff --git a/common/app/routes/Settings/components/SolutionViewer.jsx b/common/app/routes/Settings/components/SolutionViewer.jsx index c93aefa0d8..b3263132ec 100644 --- a/common/app/routes/Settings/components/SolutionViewer.jsx +++ b/common/app/routes/Settings/components/SolutionViewer.jsx @@ -11,35 +11,59 @@ const prismLang = { html: 'markup' }; +function getContentString(file) { + return file.trim() || '// We do not have the solution to this challenge'; +} + function SolutionViewer({ files }) { + const solutions = files && Array.isArray(files) ? + files.map(file => ( + + 
+          
+
+ + )) : ( + + 
+          
+
+ + ) + ; return (
{ - Object.keys(files) - .map(key => files[key]) - .map(file => ( - - 
-                
-
- - )) + solutions }
); @@ -47,7 +71,10 @@ function SolutionViewer({ files }) { SolutionViewer.displayName = 'SolutionViewer'; SolutionViewer.propTypes = { - files: PropTypes.object + files: PropTypes.oneOfType( + PropTypes.arrayOf(PropTypes.objectOf(PropTypes.string)), + PropTypes.string + ) }; export default SolutionViewer; diff --git a/common/app/routes/Settings/utils/buildUserProjectsMap.js b/common/app/routes/Settings/utils/buildUserProjectsMap.js index 9aeb3d4fe0..a0226b3909 100644 --- a/common/app/routes/Settings/utils/buildUserProjectsMap.js +++ b/common/app/routes/Settings/utils/buildUserProjectsMap.js @@ -21,7 +21,7 @@ export function buildUserProjectsMap(projectBlock, completedChallenges) { if (completed) { solution = 'solution' in completed ? completed.solution : - completed.files; + completed.files || ''; } return { ...solutions, diff --git a/common/models/user.js b/common/models/user.js index 0ab7363272..073ddd92aa 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -1,3 +1,10 @@ +/** + * + * Any ref to fixCompletedChallengesItem should be removed post + * a db migration to fix all completedChallenges + * + */ + import { Observable } from 'rx'; import uuid from 'uuid/v4'; import moment from 'moment'; @@ -10,6 +17,7 @@ import _ from 'lodash'; import { ObjectId } from 'mongodb'; import jwt from 'jsonwebtoken'; +import { fixCompletedChallengeItem } from '../utils'; import { themes } from '../utils/themes'; import { saveUser, observeMethod } from '../../server/utils/rx.js'; import { blacklistedUsernames } from '../../server/utils/constants.js'; @@ -24,7 +32,7 @@ import { publicUserProps } from '../../server/utils/publicUserProps'; -const debug = debugFactory('fcc:models:user'); +const log = debugFactory('fcc:models:user'); const BROWNIEPOINTS_TIMEOUT = [1, 'hour']; const createEmailError = redirectTo => wrapHandledError( @@ -47,7 +55,9 @@ function buildCompletedChallengesUpdate(completedChallenges, project) { const key = Object.keys(project)[0]; const solutions = project[key]; const solutionKeys = Object.keys(solutions); - const currentCompletedChallenges = [ ...completedChallenges ]; + const currentCompletedChallenges = [ + ...completedChallenges.map(fixCompletedChallengeItem) + ]; const currentCompletedProjects = currentCompletedChallenges .filter(({id}) => solutionKeys.includes(id)); const now = Date.now(); @@ -59,7 +69,7 @@ function buildCompletedChallengesUpdate(completedChallenges, project) { const isCurrentlyCompleted = indexOfCurrentId !== -1; if (isCurrentlyCompleted) { update[indexOfCurrentId] = { - ..._.find(update, ({id}) => id === currentId).__data, + ..._.find(update, ({id}) => id === currentId), solution: solutions[currentId] }; } @@ -298,7 +308,7 @@ module.exports = function(User) { User.observe('before delete', function(ctx, next) { const UserIdentity = User.app.models.UserIdentity; const UserCredential = User.app.models.UserCredential; - debug('removing user', ctx.where); + log('removing user', ctx.where); var id = ctx.where && ctx.where.id ? ctx.where.id : null; if (!id) { return next(); @@ -315,20 +325,20 @@ module.exports = function(User) { ) .subscribe( function(data) { - debug('deleted', data); + log('deleted', data); }, function(err) { - debug('error deleting user %s stuff', id, err); + log('error deleting user %s stuff', id, err); next(err); }, function() { - debug('user stuff deleted for user %s', id); + log('user stuff deleted for user %s', id); next(); } ); }); - debug('setting up user hooks'); + log('setting up user hooks'); // overwrite lb confirm User.confirm = function(uid, token, redirectTo) { return this.findById(uid) @@ -366,6 +376,17 @@ module.exports = function(User) { }); }; + function manualReload() { + this.reload((err, instance) => { + if (err) { + throw Error('failed to reload user instance'); + } + Object.assign(this, instance); + log('user reloaded from db'); + }); + } + User.prototype.manualReload = manualReload; + User.prototype.loginByRequest = function loginByRequest(req, res) { const { query: { @@ -423,7 +444,7 @@ module.exports = function(User) { if (!username && (!email || !isEmail(email))) { return Promise.resolve(false); } - debug('checking existence'); + log('checking existence'); // check to see if username is on blacklist if (username && blacklistedUsernames.indexOf(username) !== -1) { @@ -436,7 +457,7 @@ module.exports = function(User) { } else { where.email = email ? email.toLowerCase() : email; } - debug('where', where); + log('where', where); return User.count(where) .then(count => count > 0); }; @@ -531,10 +552,7 @@ module.exports = function(User) { } }) ) - .do(() => { - this.isDonating = true; - this.donationEmails = [ ...this.donationEmails, donation.email ]; - }); + .do(() => this.manualReload()); }; User.prototype.getEncodedEmail = function getEncodedEmail(email) { @@ -677,9 +695,7 @@ module.exports = function(User) { this.requestAuthEmail(false, newEmail), (_, message) => message ) - .do(() => { - Object.assign(this, updateConfig); - }); + .doOnNext(() => this.manualReload()); }); } else { @@ -715,15 +731,11 @@ module.exports = function(User) { Observable.from(updates) .flatMap(({ flag, newValue }) => { return Observable.fromPromise(User.doesExist(null, this.email)) - .flatMap(() => { - return this.update$({ [flag]: newValue }) - .do(() => { - this[flag] = newValue; - }); - }); + .flatMap(() => this.update$({ [flag]: newValue })); }) ); }) + .doOnNext(() => this.manualReload()) .map(() => dedent` We have successfully updated your account. `); @@ -748,9 +760,7 @@ module.exports = function(User) { updatedPortfolio[pIndex] = { ...portfolioItem }; } return this.update$({ portfolio: updatedPortfolio }) - .do(() => { - this.portfolio = updatedPortfolio; - }) + .do(() => this.manualReload()) .map(() => dedent` Your portfolio has been updated. `); @@ -779,7 +789,7 @@ module.exports = function(User) { } return this.update$(updateData); }) - .do(() => Object.assign(this, updateData)) + .doOnNext(() => this.manualReload() ) .map(() => dedent` Your projects have been updated. `); @@ -795,7 +805,7 @@ module.exports = function(User) { }; return this.update$(update) - .do(() => Object.assign(this, update)) + .doOnNext(() => this.manualReload()) .map(() => dedent` Your privacy settings have been updated. `); @@ -824,9 +834,7 @@ module.exports = function(User) { } return this.update$({ username: newUsername }) - .do(() => { - this.username = newUsername; - }) + .do(() => this.manualReload()) .map(() => dedent` Your username has been updated successfully. `); @@ -955,7 +963,7 @@ module.exports = function(User) { }, (e) => cb(e, null, dev ? { giver, receiver, data } : null), () => { - debug('brownie points assigned completed'); + log('brownie points assigned completed'); } ); }; @@ -1014,7 +1022,9 @@ module.exports = function(User) { ); return Promise.reject(err); } - return this.update$({ theme }).toPromise(); + return this.update$({ theme }) + .doOnNext(() => this.manualReload()) + .toPromise(); }; // deprecated. remove once live diff --git a/common/models/user.json b/common/models/user.json index 33fb10662b..e295d538b7 100644 --- a/common/models/user.json +++ b/common/models/user.json @@ -167,25 +167,37 @@ "description": "Camper is information security and quality assurance certified", "default": false }, - "completedChallengeCount": { - "type": "number", - "description": "generated per request, not held in db" - }, - "completedCertCount": { - "type": "number", - "description": "generated per request, not held in db" - }, - "completedProjectCount": { - "type": "number", - "description": "generated per request, not held in db" - }, "completedChallenges": { "type": [ { "completedDate": "number", "id": "string", "solution": "string", - "challengeType": "number" + "githubLink": "string", + "challengeType": "number", + "files": { + "type": [ + { + "contents": { + "type": "string", + "default": "" + }, + "ext": { + "type": "string" + }, + "path": { + "type": "string" + }, + "name": { + "type": "string" + }, + "key": { + "type": "string" + } + } + ], + "default": [] + } } ], "default": [] diff --git a/common/utils/index.js b/common/utils/index.js index 36fcadf900..4bb667e3f9 100644 --- a/common/utils/index.js +++ b/common/utils/index.js @@ -1,3 +1,5 @@ +import { pick } from 'lodash'; + export function dashify(str) { return ('' + str) .toLowerCase() @@ -8,3 +10,8 @@ export function dashify(str) { // todo: unify with server/utils/index.js:dasherize const dasherize = dashify; export { dasherize }; + +export const fixCompletedChallengeItem = obj => pick( + obj, + [ 'id', 'completedDate', 'solution', 'githubLink', 'challengeType', 'files' ] +); diff --git a/server/boot/certificate.js b/server/boot/certificate.js index 0fc8cd7b3c..eea1af73e9 100644 --- a/server/boot/certificate.js +++ b/server/boot/certificate.js @@ -355,8 +355,6 @@ export default function certificate(app) { ) .subscribe( user => { - const { isLocked, showCerts } = user.profileUI; - const profile = `/portfolio/${user.username}`; if (!user) { req.flash( 'danger', @@ -364,6 +362,8 @@ export default function certificate(app) { ); return res.redirect('/'); } + const { isLocked, showCerts } = user.profileUI; + const profile = `/portfolio/${user.username}`; if (!user.name) { req.flash( @@ -415,7 +415,7 @@ export default function certificate(app) { } if (user[certType]) { - const { completedChallenges = {} } = user; + const { completedChallenges = [] } = user; const { completedDate = new Date() } = _.find( completedChallenges, ({ id }) => certId === id ) || {}; diff --git a/server/boot/challenge.js b/server/boot/challenge.js index db68524635..b6264febc8 100644 --- a/server/boot/challenge.js +++ b/server/boot/challenge.js @@ -1,3 +1,10 @@ +/** + * + * Any ref to fixCompletedChallengesItem should be removed post + * a db migration to fix all completedChallenges + * + */ + import _ from 'lodash'; import debug from 'debug'; import accepts from 'accepts'; @@ -8,17 +15,49 @@ import { getChallengeById, cachedMap } from '../utils/map'; import { dasherize } from '../utils'; import pathMigrations from '../resources/pathMigration.json'; +import { fixCompletedChallengeItem } from '../../common/utils'; const log = debug('fcc:boot:challenges'); const learnURL = 'https://learn.freecodecamp.org'; +const jsProjects = [ +'aaa48de84e1ecc7c742e1124', +'a7f4d8f2483413a6ce226cac', +'56533eb9ac21ba0edf2244e2', +'aff0395860f5d3034dc0bfc9', +'aa2e6f85cab2ab736c9a9b24' +]; + function buildUserUpdate( user, challengeId, - completedChallenge, + _completedChallenge, timezone ) { + const { files } = _completedChallenge; + let completedChallenge = {}; + + if (jsProjects.includes(challengeId)) { + completedChallenge = { + ..._completedChallenge, + files: Object.keys(files) + .map(key => files[key]) + .map(file => _.pick( + file, + [ + 'contents', + 'key', + 'index', + 'name', + 'path', + 'ext' + ] + )) + }; + } else { + completedChallenge = _.omit(_completedChallenge, ['files']); + } let finalChallenge; const updateData = {}; const { timezone: userTimezone, completedChallenges = [] } = user; @@ -46,7 +85,7 @@ function buildUserUpdate( updateData.$set = { completedChallenges: _.uniqBy( - [finalChallenge, ...completedChallenges], + [finalChallenge, ...completedChallenges.map(fixCompletedChallengeItem)], 'id' ) }; @@ -167,6 +206,7 @@ export default function(app) { const points = alreadyCompleted ? user.points : user.points + 1; return user.update$(updateData) + .doOnNext(() => user.manualReload()) .doOnNext(({ count }) => log('%s documents updated', count)) .map(() => { if (type === 'json') { @@ -199,7 +239,7 @@ export default function(app) { return req.user.getCompletedChallenges$() .flatMap(() => { const completedDate = Date.now(); - const { id, solution, timezone } = req.body; + const { id, solution, timezone, files } = req.body; const { alreadyCompleted, @@ -207,7 +247,7 @@ export default function(app) { } = buildUserUpdate( req.user, id, - { id, solution, completedDate }, + { id, solution, completedDate, files }, timezone ); @@ -250,7 +290,7 @@ export default function(app) { const completedChallenge = _.pick( body, - [ 'id', 'solution', 'githubLink', 'challengeType' ] + [ 'id', 'solution', 'githubLink', 'challengeType', 'files' ] ); completedChallenge.completedDate = Date.now(); @@ -278,6 +318,7 @@ export default function(app) { } = buildUserUpdate(user, completedChallenge.id, completedChallenge); return user.update$(updateData) + .doOnNext(() => user.manualReload()) .doOnNext(({ count }) => log('%s documents updated', count)) .doOnNext(() => { if (type === 'json') { diff --git a/server/services/user.js b/server/services/user.js index 5b9fc17d7f..ce8c6b3c65 100644 --- a/server/services/user.js +++ b/server/services/user.js @@ -1,3 +1,10 @@ +/** + * + * Any ref to fixCompletedChallengesItem should be removed post + * a db migration to fix all completedChallenges + * + */ + import { Observable } from 'rx'; import _ from 'lodash'; @@ -6,6 +13,7 @@ import { normaliseUserFields, userPropsForSession } from '../utils/publicUserProps'; +import { fixCompletedChallengeItem } from '../../common/utils'; export default function userServices() { return { @@ -32,7 +40,9 @@ export default function userServices() { .map(({ completedChallenges, progress }) => ({ ...queryUser.toJSON(), ...progress, - completedChallenges + completedChallenges: completedChallenges.map( + fixCompletedChallengeItem + ) })) .map( user => ({